diff --git a/CHANGELOG.md b/CHANGELOG.md index cbf13112c..81c3a3da5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - Added an `optionalDependencies` option to [`no-extraneous-dependencies`] to allow/forbid optional dependencies ([#266], thanks [@jfmengels]). - Added `newlines-between` option to [`order`] rule ([#298], thanks [@singles]) +### Fixed +- [`extensions`]: fallback to source path for extension enforcement if imported + module is not resolved. Also, never report for builtins (i.e. `path`). ([#296]) + ## resolvers/webpack/0.2.4 - 2016-04-29 ### Changed - automatically find webpack config with `interpret`-able extensions ([#287], thanks [@taion]) @@ -191,6 +195,7 @@ for info on changes for earlier releases. [`named`]: ./docs/rules/named.md [`newline-after-import`]: ./docs/rules/newline-after-import.md +[#296]: https://github.com/benmosher/eslint-plugin-import/pull/296 [#289]: https://github.com/benmosher/eslint-plugin-import/pull/289 [#288]: https://github.com/benmosher/eslint-plugin-import/pull/288 [#287]: https://github.com/benmosher/eslint-plugin-import/pull/287 diff --git a/docs/rules/extensions.md b/docs/rules/extensions.md index 1bd8dbb6b..4fe70eea9 100644 --- a/docs/rules/extensions.md +++ b/docs/rules/extensions.md @@ -54,6 +54,8 @@ import bar from './bar'; import Component from './Component' import express from 'express/index'; + +import * as path from 'path'; ``` The following patterns are considered problems when configuration set to "always": @@ -78,6 +80,8 @@ import bar from './bar.json'; import Component from './Component.jsx' import express from 'express/index.js'; + +import * as path from 'path'; ``` ## When Not To Use It diff --git a/gulpfile.js b/gulpfile.js index e7790a812..74a3eaaea 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -91,7 +91,7 @@ var reporter = 'spec' gulp.task('test', ['pretest'], function () { return gulp.src('tests/lib/**/*.js', { read: false }) - .pipe(mocha({ reporter: reporter, grep: process.env.TEST_GREP })) + .pipe(mocha({ reporter: reporter, grep: process.env.TEST_GREP, timeout: 5000 })) // NODE_PATH=./lib mocha --recursive --reporter dot tests/lib/ }) diff --git a/src/core/importType.js b/src/core/importType.js index 133acec60..d913d110c 100644 --- a/src/core/importType.js +++ b/src/core/importType.js @@ -8,7 +8,7 @@ function constant(value) { return () => value } -function isBuiltIn(name) { +export function isBuiltIn(name) { return builtinModules.indexOf(name) !== -1 } diff --git a/src/rules/extensions.js b/src/rules/extensions.js index 5a8c04edd..175dc1d9d 100644 --- a/src/rules/extensions.js +++ b/src/rules/extensions.js @@ -1,7 +1,9 @@ import path from 'path' -import resolve from '../core/resolve' import endsWith from 'lodash.endswith' +import resolve from '../core/resolve' +import { isBuiltIn } from '../core/importType' + module.exports = function (context) { const configuration = context.options[0] || 'never' @@ -24,17 +26,25 @@ module.exports = function (context) { function checkFileExtension(node) { const { source } = node const importPath = source.value + + // don't enforce anything on builtins + if (isBuiltIn(importPath)) return + const resolvedPath = resolve(importPath, context) - const extension = path.extname(resolvedPath).substring(1) - if (!endsWith(importPath, extension)) { + // get extension from resolved path, if possible. + // for unresolved, use source value. + const extension = path.extname(resolvedPath || importPath).substring(1) + + if (!extension || !endsWith(importPath, extension)) { if (isUseOfExtensionEnforced(extension)) { context.report({ node: source, - message: `Missing file extension "${extension}" for "${importPath}"`, + message: + `Missing file extension ${extension ? `"${extension}" ` : ''}for "${importPath}"`, }) } - } else { + } else if (extension) { if (!isUseOfExtensionEnforced(extension) && isResolvableWithoutExtension(importPath)) { context.report({ node: source, diff --git a/tests/src/rules/extensions.js b/tests/src/rules/extensions.js index 62b05ca0e..74f8c1473 100644 --- a/tests/src/rules/extensions.js +++ b/tests/src/rules/extensions.js @@ -1,6 +1,6 @@ import { RuleTester } from 'eslint' -import rule from 'rules/extensions'; -import { test } from '../utils'; +import rule from 'rules/extensions' +import { test } from '../utils' const ruleTester = new RuleTester() @@ -10,28 +10,36 @@ ruleTester.run('extensions', rule, { test({ code: 'import dot from "./file.with.dot"' }), test({ code: 'import a from "a/index.js"', - options: [ 'always' ] + options: [ 'always' ], }), test({ code: 'import dot from "./file.with.dot.js"', - options: [ 'always' ] + options: [ 'always' ], }), test({ code: [ 'import a from "a"', 'import packageConfig from "./package.json"', ].join('\n'), - options: [ { json: 'always', js: 'never' } ] + options: [ { json: 'always', js: 'never' } ], }), test({ code: [ 'import lib from "./bar"', 'import component from "./bar.jsx"', - 'import data from "./bar.json"' + 'import data from "./bar.json"', ].join('\n'), options: [ 'never' ], - settings: { 'import/resolve': { 'extensions': [ '.js', '.jsx', '.json' ] } } - }) + settings: { 'import/resolve': { 'extensions': [ '.js', '.jsx', '.json' ] } }, + }), + + // unresolved (#271/#295) + test({ code: 'import path from "path"' }), + test({ code: 'import path from "path"', options: [ 'never' ] }), + test({ code: 'import path from "path"', options: [ 'always' ] }), + test({ code: 'import thing from "./fake-file.js"', options: [ 'always' ] }), + test({ code: 'import thing from "non-package"', options: [ 'never' ] }), + ], invalid: [ @@ -40,8 +48,8 @@ ruleTester.run('extensions', rule, { errors: [ { message: 'Unexpected use of file extension "js" for "a/index.js"', line: 1, - column: 15 - } ] + column: 15, + } ], }), test({ code: 'import a from "a"', @@ -49,19 +57,19 @@ ruleTester.run('extensions', rule, { errors: [ { message: 'Missing file extension "js" for "a"', line: 1, - column: 15 - } ] + column: 15, + } ], }), test({ code: 'import dot from "./file.with.dot"', - options: [ "always" ], + options: [ 'always' ], errors: [ { message: 'Missing file extension "js" for "./file.with.dot"', line: 1, - column: 17 - } - ] + column: 17, + }, + ], }), test({ code: [ @@ -74,20 +82,20 @@ ruleTester.run('extensions', rule, { { message: 'Unexpected use of file extension "js" for "a/index.js"', line: 1, - column: 15 + column: 15, }, { message: 'Missing file extension "json" for "./package"', line: 2, - column: 27 - } - ] + column: 27, + }, + ], }), test({ code: [ 'import lib from "./bar.js"', 'import component from "./bar.jsx"', - 'import data from "./bar.json"' + 'import data from "./bar.json"', ].join('\n'), options: [ 'never' ], settings: { 'import/resolve': { 'extensions': [ '.js', '.jsx', '.json' ] } }, @@ -95,15 +103,15 @@ ruleTester.run('extensions', rule, { { message: 'Unexpected use of file extension "js" for "./bar.js"', line: 1, - column: 17 - } - ] + column: 17, + }, + ], }), test({ code: [ 'import lib from "./bar.js"', 'import component from "./bar.jsx"', - 'import data from "./bar.json"' + 'import data from "./bar.json"', ].join('\n'), options: [ { json: 'always', js: 'never', jsx: 'never' } ], settings: { 'import/resolve': { 'extensions': [ '.js', '.jsx', '.json' ] } }, @@ -111,11 +119,34 @@ ruleTester.run('extensions', rule, { { message: 'Unexpected use of file extension "js" for "./bar.js"', line: 1, - column: 17 - } - ] - }) + column: 17, + }, + ], + }), - ] -}) + // unresolved (#271/#295) + test({ + code: 'import thing from "./fake-file.js"', + options: [ 'never' ], + errors: [ + { + message: 'Unexpected use of file extension "js" for "./fake-file.js"', + line: 1, + column: 19, + }, + ], + }), + test({ + code: 'import thing from "non-package"', + options: [ 'always' ], + errors: [ + { + message: 'Missing file extension for "non-package"', + line: 1, + column: 19, + }, + ], + }), + ], +})