Skip to content

Commit

Permalink
Unresolved extensions
Browse files Browse the repository at this point in the history
* formatting

* extensions fixes:
- use source path if unable to resolve (fixes #295, #271)
- don't enforce file extensions on builtins

changelog + docs updates for #296

* increase test timeout for Travis?
  • Loading branch information
benmosher committed May 5, 2016
1 parent f75d0af commit 12f446e
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 38 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions docs/rules/extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/
})

Expand Down
2 changes: 1 addition & 1 deletion src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function constant(value) {
return () => value
}

function isBuiltIn(name) {
export function isBuiltIn(name) {
return builtinModules.indexOf(name) !== -1
}

Expand Down
20 changes: 15 additions & 5 deletions src/rules/extensions.js
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -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,
Expand Down
93 changes: 62 additions & 31 deletions tests/src/rules/extensions.js
Original file line number Diff line number Diff line change
@@ -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()

Expand All @@ -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: [
Expand All @@ -40,28 +48,28 @@ 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"',
options: [ 'always' ],
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: [
Expand All @@ -74,48 +82,71 @@ 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' ] } },
errors: [
{
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' ] } },
errors: [
{
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,
},
],
}),

],
})

0 comments on commit 12f446e

Please sign in to comment.