Skip to content

Commit

Permalink
extensions fixes:
Browse files Browse the repository at this point in the history
- use source path if unable to resolve (fixes #295, #271)
- don't enforce file extensions on builtins

changelog + docs updates for #296
  • Loading branch information
benmosher committed May 3, 2016
1 parent 3561ab4 commit 779c0ea
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 7 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`newline-after-import`], new rule. ([#245], thanks [@singles])
- Added an `optionalDependencies` option to [`no-extraneous-dependencies`] to allow/forbid optional dependencies ([#266], thanks [@jfmengels]).

### 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 @@ -190,6 +194,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 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
33 changes: 32 additions & 1 deletion tests/src/rules/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ ruleTester.run('extensions', rule, {
options: [ 'never' ],
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 Down Expand Up @@ -116,6 +124,29 @@ ruleTester.run('extensions', rule, {
],
}),

// 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 779c0ea

Please sign in to comment.