diff --git a/CHANGELOG.md b/CHANGELOG.md index d384d5c390..3969ea3aac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,26 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## [Unreleased] +## [2.14.0] - 2018-08-13 +* 69e0187 (HEAD -> master, source/master, origin/master, origin/HEAD) Merge pull request #1151 from jf248/jsx +|\ +| * e30a757 (source/pr/1151, fork/jsx) Add JSX check to namespace rule +|/ +* 8252344 (source/pr/1148) Add error to output when module loaded as resolver has invalid API +### Added +- [`no-useless-path-segments`]: add commonJS (CJS) support ([#1128], thanks [@1pete]) +- [`namespace`]: add JSX check ([#1151], thanks [@jf248]) + +### Fixed +- [`no-cycle`]: ignore Flow imports ([#1126], thanks [@gajus]) +- fix Flow type imports ([#1106], thanks [@syymza]) +- [`no-relative-parent-imports`]: resolve paths ([#1135], thanks [@chrislloyd]) +- [`import/order`]: fix autofixer when using typescript-eslint-parser ([#1137], thanks [@justinanastos]) +- repeat fix from [#797] for [#717], in another place (thanks [@ljharb]) + +### Refactors +- add explicit support for RestElement alongside ExperimentalRestProperty (thanks [@ljharb]) + ## [2.13.0] - 2018-06-24 ### Added - Add ESLint 5 support ([#1122], thanks [@ai] and [@ljharb]) @@ -473,7 +493,13 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md +[#1151]: https://github.com/benmosher/eslint-plugin-import/pull/1151 +[#1137]: https://github.com/benmosher/eslint-plugin-import/pull/1137 +[#1135]: https://github.com/benmosher/eslint-plugin-import/pull/1135 +[#1128]: https://github.com/benmosher/eslint-plugin-import/pull/1128 +[#1126]: https://github.com/benmosher/eslint-plugin-import/pull/1126 [#1122]: https://github.com/benmosher/eslint-plugin-import/pull/1122 +[#1106]: https://github.com/benmosher/eslint-plugin-import/pull/1106 [#1093]: https://github.com/benmosher/eslint-plugin-import/pull/1093 [#1085]: https://github.com/benmosher/eslint-plugin-import/pull/1085 [#1068]: https://github.com/benmosher/eslint-plugin-import/pull/1068 @@ -486,6 +512,7 @@ for info on changes for earlier releases. [#858]: https://github.com/benmosher/eslint-plugin-import/pull/858 [#843]: https://github.com/benmosher/eslint-plugin-import/pull/843 [#871]: https://github.com/benmosher/eslint-plugin-import/pull/871 +[#797]: https://github.com/benmosher/eslint-plugin-import/pull/797 [#744]: https://github.com/benmosher/eslint-plugin-import/pull/744 [#742]: https://github.com/benmosher/eslint-plugin-import/pull/742 [#737]: https://github.com/benmosher/eslint-plugin-import/pull/737 @@ -558,6 +585,7 @@ for info on changes for earlier releases. [#842]: https://github.com/benmosher/eslint-plugin-import/issues/842 [#839]: https://github.com/benmosher/eslint-plugin-import/issues/839 [#720]: https://github.com/benmosher/eslint-plugin-import/issues/720 +[#717]: https://github.com/benmosher/eslint-plugin-import/issues/717 [#686]: https://github.com/benmosher/eslint-plugin-import/issues/686 [#671]: https://github.com/benmosher/eslint-plugin-import/issues/671 [#660]: https://github.com/benmosher/eslint-plugin-import/issues/660 @@ -619,7 +647,8 @@ for info on changes for earlier releases. [#119]: https://github.com/benmosher/eslint-plugin-import/issues/119 [#89]: https://github.com/benmosher/eslint-plugin-import/issues/89 -[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v2.12.0...HEAD +[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v2.13.0...HEAD +[2.13.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.12.0...v2.13.0 [2.12.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.11.0...v2.12.0 [2.11.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.10.0...v2.11.0 [2.10.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.9.0...v2.10.0 @@ -733,3 +762,8 @@ for info on changes for earlier releases. [@hulkish]: https://github.com/hulkish [@chrislloyd]: https://github.com/chrislloyd [@ai]: https://github.com/ai +[@syymza]: https://github.com/syymza +[@justinanastos]: https://github.com/justinanastos +[@1pete]: https://github.com/1pete +[@gajus]: https://github.com/gajus +[@jf248]: https://github.com/jf248 diff --git a/package.json b/package.json index 7d39395a76..810084436e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-import", - "version": "2.13.0", + "version": "2.14.0", "description": "Import with sanity.", "engines": { "node": ">=4" diff --git a/src/ExportMap.js b/src/ExportMap.js index 1cb5dc3e9c..66b212a211 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -38,7 +38,12 @@ export default class ExportMap { get size() { let size = this.namespace.size + this.reexports.size - this.dependencies.forEach(dep => size += dep().size) + this.dependencies.forEach(dep => { + const d = dep() + // CJS / ignored dependencies won't exist (#717) + if (d == null) return + size += d.size + }) return size } diff --git a/src/rules/named.js b/src/rules/named.js index 8c2acd714e..57e4f1d9ef 100644 --- a/src/rules/named.js +++ b/src/rules/named.js @@ -30,6 +30,9 @@ module.exports = { node.specifiers.forEach(function (im) { if (im.type !== type) return + // ignore type imports + if (im.importKind === 'type') return + const deepLookup = imports.hasDeep(im[key].name) if (!deepLookup.found) { diff --git a/src/rules/namespace.js b/src/rules/namespace.js index 93e5891594..bbba2ce2ef 100644 --- a/src/rules/namespace.js +++ b/src/rules/namespace.js @@ -44,7 +44,7 @@ module.exports = { return { // pick up all imports at body entry time, to properly respect hoisting - 'Program': function ({ body }) { + Program: function ({ body }) { function processBodyStatement(declaration) { if (declaration.type !== 'ImportDeclaration') return @@ -58,7 +58,7 @@ module.exports = { return } - for (let specifier of declaration.specifiers) { + for (const specifier of declaration.specifiers) { switch (specifier.type) { case 'ImportNamespaceSpecifier': if (!imports.size) { @@ -83,7 +83,7 @@ module.exports = { }, // same as above, but does not add names to local map - 'ExportNamespaceSpecifier': function (namespace) { + ExportNamespaceSpecifier: function (namespace) { var declaration = importDeclaration(context) var imports = Exports.get(declaration.source.value, context) @@ -102,7 +102,7 @@ module.exports = { // todo: check for possible redefinition - 'MemberExpression': function (dereference) { + MemberExpression: function (dereference) { if (dereference.object.type !== 'Identifier') return if (!namespaces.has(dereference.object.name)) return @@ -146,7 +146,7 @@ module.exports = { }, - 'VariableDeclarator': function ({ id, init }) { + VariableDeclarator: function ({ id, init }) { if (init == null) return if (init.type !== 'Identifier') return if (!namespaces.has(init.name)) return @@ -160,8 +160,12 @@ module.exports = { if (pattern.type !== 'ObjectPattern') return - for (let property of pattern.properties) { - if (property.type === 'ExperimentalRestProperty' || !property.key) { + for (const property of pattern.properties) { + if ( + property.type === 'ExperimentalRestProperty' + || property.type === 'RestElement' + || !property.key + ) { continue } @@ -189,6 +193,17 @@ module.exports = { testKey(id, namespaces.get(init.name)) }, + + JSXMemberExpression: function({object, property}) { + if (!namespaces.has(object.name)) return + var namespace = namespaces.get(object.name) + if (!namespace.has(property.name)) { + context.report({ + node: property, + message: makeMessage(property, [object.name]), + }) + } + }, } }, } diff --git a/src/rules/no-cycle.js b/src/rules/no-cycle.js index bbc251e388..1a70db2c70 100644 --- a/src/rules/no-cycle.js +++ b/src/rules/no-cycle.js @@ -30,6 +30,14 @@ module.exports = { function checkSourceValue(sourceNode, importer) { const imported = Exports.get(sourceNode.value, context) + if (sourceNode.parent && sourceNode.parent.importKind === 'type') { + return // no Flow import resolution + } + + if (sourceNode._babelType === 'Literal') { + return // no Flow import resolution, workaround for ESLint < 5.x + } + if (imported == null) { return // no-unresolved territory } diff --git a/src/rules/no-relative-parent-imports.js b/src/rules/no-relative-parent-imports.js index 3153eeb784..6b58c97f5a 100644 --- a/src/rules/no-relative-parent-imports.js +++ b/src/rules/no-relative-parent-imports.js @@ -1,6 +1,7 @@ import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor' import docsUrl from '../docsUrl' -import { basename } from 'path' +import { basename, dirname, relative } from 'path' +import resolve from 'eslint-module-utils/resolve' import importType from '../core/importType' @@ -14,11 +15,24 @@ module.exports = { create: function noRelativePackages(context) { const myPath = context.getFilename() - if (myPath === '') return {} // can't cycle-check a non-file + if (myPath === '') return {} // can't check a non-file function checkSourceValue(sourceNode) { const depPath = sourceNode.value - if (importType(depPath, context) === 'parent') { + + if (importType(depPath, context) === 'external') { // ignore packages + return + } + + const absDepPath = resolve(depPath, context) + + if (!absDepPath) { // unable to resolve path + return + } + + const relDepPath = relative(dirname(myPath), absDepPath) + + if (importType(relDepPath, context) === 'parent') { context.report({ node: sourceNode, message: 'Relative imports from parent directories are not allowed. ' + diff --git a/src/rules/no-useless-path-segments.js b/src/rules/no-useless-path-segments.js index b9c4eedda0..5872b2d1c3 100644 --- a/src/rules/no-useless-path-segments.js +++ b/src/rules/no-useless-path-segments.js @@ -39,6 +39,16 @@ module.exports = { url: docsUrl('no-useless-path-segments'), }, + schema: [ + { + type: 'object', + properties: { + commonjs: { type: 'boolean' }, + }, + additionalProperties: false, + }, + ], + fixable: 'code', }, diff --git a/src/rules/order.js b/src/rules/order.js index 81babd7fde..f925a20eb4 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -97,8 +97,8 @@ function findRootNode(node) { function findEndOfLineWithComments(sourceCode, node) { const tokensToEndOfLine = takeTokensAfterWhile(sourceCode, node, commentOnSameLineAs(node)) let endOfTokens = tokensToEndOfLine.length > 0 - ? tokensToEndOfLine[tokensToEndOfLine.length - 1].end - : node.end + ? tokensToEndOfLine[tokensToEndOfLine.length - 1].range[1] + : node.range[1] let result = endOfTokens for (let i = endOfTokens; i < sourceCode.text.length; i++) { if (sourceCode.text[i] === '\n') { @@ -121,7 +121,7 @@ function commentOnSameLineAs(node) { function findStartOfLineWithComments(sourceCode, node) { const tokensToEndOfLine = takeTokensBeforeWhile(sourceCode, node, commentOnSameLineAs(node)) - let startOfTokens = tokensToEndOfLine.length > 0 ? tokensToEndOfLine[0].start : node.start + let startOfTokens = tokensToEndOfLine.length > 0 ? tokensToEndOfLine[0].range[0] : node.range[0] let result = startOfTokens for (let i = startOfTokens - 1; i > 0; i--) { if (sourceCode.text[i] !== ' ' && sourceCode.text[i] !== '\t') { @@ -296,11 +296,11 @@ function fixNewLineAfterImport(context, previousImport) { const tokensToEndOfLine = takeTokensAfterWhile( context.getSourceCode(), prevRoot, commentOnSameLineAs(prevRoot)) - let endOfLine = prevRoot.end + let endOfLine = prevRoot.range[1] if (tokensToEndOfLine.length > 0) { - endOfLine = tokensToEndOfLine[tokensToEndOfLine.length - 1].end + endOfLine = tokensToEndOfLine[tokensToEndOfLine.length - 1].range[1] } - return (fixer) => fixer.insertTextAfterRange([prevRoot.start, endOfLine], '\n') + return (fixer) => fixer.insertTextAfterRange([prevRoot.range[0], endOfLine], '\n') } function removeNewLineAfterImport(context, currentImport, previousImport) { diff --git a/tests/files/flowtypes.js b/tests/files/flowtypes.js index 7ada3482b1..2df2471475 100644 --- a/tests/files/flowtypes.js +++ b/tests/files/flowtypes.js @@ -10,3 +10,6 @@ export type MyType = { export interface MyInterface {} export class MyClass {} + +export opaque type MyOpaqueType: string = string; + diff --git a/tests/files/foo-bar-resolver-invalid.js b/tests/files/foo-bar-resolver-invalid.js new file mode 100644 index 0000000000..a6213d6678 --- /dev/null +++ b/tests/files/foo-bar-resolver-invalid.js @@ -0,0 +1 @@ +exports = {}; diff --git a/tests/src/core/resolve.js b/tests/src/core/resolve.js index 3c15303edf..b9a9063243 100644 --- a/tests/src/core/resolve.js +++ b/tests/src/core/resolve.js @@ -110,6 +110,22 @@ describe('resolve', function () { expect(testContextReports[0].loc).to.eql({ line: 1, column: 0 }) }) + it('reports loaded resolver with invalid interface', function () { + const resolverName = './foo-bar-resolver-invalid'; + const testContext = utils.testContext({ 'import/resolver': resolverName }); + const testContextReports = [] + testContext.report = function (reportInfo) { + testContextReports.push(reportInfo) + } + testContextReports.length = 0 + expect(resolve( '../files/foo' + , Object.assign({}, testContext, { getFilename: function () { return utils.getFilename('foo.js') } }) + )).to.equal(undefined) + expect(testContextReports[0]).to.be.an('object') + expect(testContextReports[0].message).to.equal(`Resolve error: ${resolverName} with invalid interface loaded as resolver`) + expect(testContextReports[0].loc).to.eql({ line: 1, column: 0 }) + }) + it('respects import/resolve extensions', function () { const testContext = utils.testContext({ 'import/resolve': { 'extensions': ['.jsx'] }}) @@ -119,7 +135,6 @@ describe('resolve', function () { }) it('reports load exception in a user resolver', function () { - const testContext = utils.testContext({ 'import/resolver': './load-error-resolver' }) const testContextReports = [] testContext.report = function (reportInfo) { diff --git a/tests/src/rules/named.js b/tests/src/rules/named.js index 4fdd3434f9..cb1a5b843b 100644 --- a/tests/src/rules/named.js +++ b/tests/src/rules/named.js @@ -72,6 +72,14 @@ ruleTester.run('named', rule, { code: 'import type { MissingType } from "./flowtypes"', parser: 'babel-eslint', }), + test({ + code: 'import type { MyOpaqueType } from "./flowtypes"', + parser: 'babel-eslint', + }), + test({ + code: 'import { type MyOpaqueType, MyClass } from "./flowtypes"', + parser: 'babel-eslint', + }), // TypeScript test({ @@ -262,6 +270,12 @@ ruleTester.run('named', rule, { }], }), + test({ + code: 'import { type MyOpaqueType, MyMissingClass } from "./flowtypes"', + parser: 'babel-eslint', + errors: ["MyMissingClass not found in './flowtypes'"], + }), + // jsnext test({ code: '/*jsnext*/ import { createSnorlax } from "redux"', diff --git a/tests/src/rules/namespace.js b/tests/src/rules/namespace.js index 1cfee2b54d..7fa8cfcdb9 100644 --- a/tests/src/rules/namespace.js +++ b/tests/src/rules/namespace.js @@ -104,6 +104,16 @@ const valid = [ parser: 'babel-eslint', }), + // JSX + test({ + code: 'import * as Names from "./named-exports"; const Foo = ', + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + }), + ...SYNTAX_CASES, ] @@ -185,6 +195,17 @@ const invalid = [ errors: [`'default' not found in imported namespace 'ree'.`], }), + // JSX + test({ + code: 'import * as Names from "./named-exports"; const Foo = ', + errors: [error('e', 'Names')], + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + }), + ] /////////////////////// diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js index ae45ba36ec..4ee4daacb6 100644 --- a/tests/src/rules/no-cycle.js +++ b/tests/src/rules/no-cycle.js @@ -36,6 +36,10 @@ ruleTester.run('no-cycle', rule, { code: 'import { foo } from "./depth-two"', options: [{ maxDepth: 1 }], }), + test({ + code: 'import type { FooType } from "./depth-one"', + parser: 'babel-eslint', + }), ], invalid: [ test({ diff --git a/tests/src/rules/no-relative-parent-imports.js b/tests/src/rules/no-relative-parent-imports.js index 6d7a2c2fae..8978230904 100644 --- a/tests/src/rules/no-relative-parent-imports.js +++ b/tests/src/rules/no-relative-parent-imports.js @@ -38,9 +38,18 @@ ruleTester.run('no-relative-parent-imports', rule, { test({ code: 'import("./app/index.js")', }), + test({ + code: 'import(".")', + }), + test({ + code: 'import("path")', + }), test({ code: 'import("package")', }), + test({ + code: 'import("@scope/package")', + }), ], invalid: [ @@ -69,5 +78,21 @@ ruleTester.run('no-relative-parent-imports', rule, { column: 8, } ], }), + test({ + code: 'import foo from "./../plugin.js"', + errors: [ { + message: 'Relative imports from parent directories are not allowed. Please either pass what you\'re importing through at runtime (dependency injection), move `index.js` to same directory as `./../plugin.js` or consider making `./../plugin.js` a package.', + line: 1, + column: 17 + }] + }), + test({ + code: 'import foo from "../../api/service"', + errors: [ { + message: 'Relative imports from parent directories are not allowed. Please either pass what you\'re importing through at runtime (dependency injection), move `index.js` to same directory as `../../api/service` or consider making `../../api/service` a package.', + line: 1, + column: 17 + }] + }) ], }) diff --git a/tests/src/rules/no-useless-path-segments.js b/tests/src/rules/no-useless-path-segments.js index 1f4229f5ee..ed20440012 100644 --- a/tests/src/rules/no-useless-path-segments.js +++ b/tests/src/rules/no-useless-path-segments.js @@ -7,6 +7,10 @@ const rule = require('rules/no-useless-path-segments') function runResolverTests(resolver) { ruleTester.run(`no-useless-path-segments (${resolver})`, rule, { valid: [ + // commonjs with default options + test({ code: 'require("./../files/malformed.js")' }), + + // esmodule test({ code: 'import "./malformed.js"' }), test({ code: 'import "./test-module"' }), test({ code: 'import "./bar/"' }), @@ -16,6 +20,49 @@ function runResolverTests(resolver) { ], invalid: [ + // commonjs + test({ + code: 'require("./../files/malformed.js")', + options: [{ commonjs: true }], + errors: [ 'Useless path segments for "./../files/malformed.js", should be "../files/malformed.js"'], + }), + test({ + code: 'require("./../files/malformed")', + options: [{ commonjs: true }], + errors: [ 'Useless path segments for "./../files/malformed", should be "../files/malformed"'], + }), + test({ + code: 'require("../files/malformed.js")', + options: [{ commonjs: true }], + errors: [ 'Useless path segments for "../files/malformed.js", should be "./malformed.js"'], + }), + test({ + code: 'require("../files/malformed")', + options: [{ commonjs: true }], + errors: [ 'Useless path segments for "../files/malformed", should be "./malformed"'], + }), + test({ + code: 'require("./test-module/")', + options: [{ commonjs: true }], + errors: [ 'Useless path segments for "./test-module/", should be "./test-module"'], + }), + test({ + code: 'require("./")', + options: [{ commonjs: true }], + errors: [ 'Useless path segments for "./", should be "."'], + }), + test({ + code: 'require("../")', + options: [{ commonjs: true }], + errors: [ 'Useless path segments for "../", should be ".."'], + }), + test({ + code: 'require("./deep//a")', + options: [{ commonjs: true }], + errors: [ 'Useless path segments for "./deep//a", should be "./deep/a"'], + }), + + // esmodule test({ code: 'import "./../files/malformed.js"', errors: [ 'Useless path segments for "./../files/malformed.js", should be "../files/malformed.js"'], diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index fb3b788448..23487dac91 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -1260,5 +1260,21 @@ ruleTester.run('order', rule, { message: '`fs` import should occur before import of `async`', }], })), + // fix incorrect order with typescript-eslint-parser + test({ + code: ` + var async = require('async'); + var fs = require('fs'); + `, + output: ` + var fs = require('fs'); + var async = require('async'); + `, + parser: 'typescript-eslint-parser', + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), ], }) diff --git a/utils/ignore.js b/utils/ignore.js index 88e4080dda..91cc731a81 100644 --- a/utils/ignore.js +++ b/utils/ignore.js @@ -24,8 +24,11 @@ function makeValidExtensionSet(settings) { // all alternate parser extensions are also valid if ('import/parsers' in settings) { for (let parser in settings['import/parsers']) { - settings['import/parsers'][parser] - .forEach(ext => exts.add(ext)) + const parserSettings = settings['import/parsers'][parser] + if (!Array.isArray(parserSettings)) { + throw new TypeError('"settings" for ' + parser + ' must be an array') + } + parserSettings.forEach(ext => exts.add(ext)) } } diff --git a/utils/resolve.js b/utils/resolve.js index b280ca2cfa..87a1eaea81 100644 --- a/utils/resolve.js +++ b/utils/resolve.js @@ -160,8 +160,19 @@ function requireResolver(name, sourceFile) { if (!resolver) { throw new Error(`unable to load resolver "${name}".`) + } + if (!isResolverValid(resolver)) { + throw new Error(`${name} with invalid interface loaded as resolver`) + } + + return resolver +} + +function isResolverValid(resolver) { + if (resolver.interfaceVersion === 2) { + return resolver.resolve && typeof resolver.resolve === 'function' } else { - return resolver; + return resolver.resolveImport && typeof resolver.resolveImport === 'function' } }