From 70a59fe1880b4061cd7c0f032c0b05c728223a0f Mon Sep 17 00:00:00 2001 From: vikr01 Date: Sat, 20 Oct 2018 23:09:20 -0700 Subject: [PATCH] [fix] Fix overwriting of dynamic import() CallExpression - fix import() to work with no-cycle - add tests using multiple imports in no-cycle Fixes #1035. Fixes #1166. --- src/rules/no-cycle.js | 6 +--- tests/src/rules/no-cycle.js | 32 +++++++++++++++++++ tests/src/rules/no-relative-parent-imports.js | 8 +++++ tests/src/rules/no-unresolved.js | 9 ++++++ tests/src/rules/no-useless-path-segments.js | 23 ++++++++++++- utils/moduleVisitor.js | 2 ++ 6 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/rules/no-cycle.js b/src/rules/no-cycle.js index f769b862c..62da3643b 100644 --- a/src/rules/no-cycle.js +++ b/src/rules/no-cycle.js @@ -31,14 +31,10 @@ module.exports = { function checkSourceValue(sourceNode, importer) { const imported = Exports.get(sourceNode.value, context) - if (sourceNode.parent && sourceNode.parent.importKind === 'type') { + if (importer.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/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js index 4ee4daacb..c88c7504b 100644 --- a/tests/src/rules/no-cycle.js +++ b/tests/src/rules/no-cycle.js @@ -36,10 +36,23 @@ ruleTester.run('no-cycle', rule, { code: 'import { foo } from "./depth-two"', options: [{ maxDepth: 1 }], }), + test({ + code: 'import { foo, bar } from "./depth-two"', + options: [{ maxDepth: 1 }], + }), + test({ + code: 'import("./depth-two").then(function({ foo }){})', + options: [{ maxDepth: 1 }], + parser: 'babel-eslint', + }), test({ code: 'import type { FooType } from "./depth-one"', parser: 'babel-eslint', }), + test({ + code: 'import type { FooType, BarType } from "./depth-one"', + parser: 'babel-eslint', + }), ], invalid: [ test({ @@ -84,10 +97,29 @@ ruleTester.run('no-cycle', rule, { code: 'import { two } from "./depth-three-star"', errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)], }), + test({ + code: 'import one, { two, three } from "./depth-three-star"', + errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)], + }), test({ code: 'import { bar } from "./depth-three-indirect"', errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)], }), + test({ + code: 'import { bar } from "./depth-three-indirect"', + errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)], + parser: 'babel-eslint', + }), + test({ + code: 'import("./depth-three-star")', + errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)], + parser: 'babel-eslint', + }), + test({ + code: 'import("./depth-three-indirect")', + errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)], + parser: 'babel-eslint', + }), ], }) // }) diff --git a/tests/src/rules/no-relative-parent-imports.js b/tests/src/rules/no-relative-parent-imports.js index 897823090..281edd123 100644 --- a/tests/src/rules/no-relative-parent-imports.js +++ b/tests/src/rules/no-relative-parent-imports.js @@ -93,6 +93,14 @@ ruleTester.run('no-relative-parent-imports', rule, { line: 1, column: 17 }] + }), + test({ + code: 'import("../../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: 8 + }], }) ], }) diff --git a/tests/src/rules/no-unresolved.js b/tests/src/rules/no-unresolved.js index 9db6e1a5c..9fa7019a7 100644 --- a/tests/src/rules/no-unresolved.js +++ b/tests/src/rules/no-unresolved.js @@ -29,6 +29,8 @@ function runResolverTests(resolver) { rest({ code: "import bar from './bar.js';" }), rest({ code: "import {someThing} from './test-module';" }), rest({ code: "import fs from 'fs';" }), + rest({ code: "import('fs');" + , parser: 'babel-eslint' }), rest({ code: 'import * as foo from "a"' }), @@ -114,6 +116,13 @@ function runResolverTests(resolver) { "module 'in-alternate-root'." , type: 'Literal', }]}), + rest({ + code: "import('in-alternate-root').then(function({DEEP}){});", + errors: [{ message: 'Unable to resolve path to ' + + "module 'in-alternate-root'." + , type: 'Literal', + }], + parser: 'babel-eslint'}), rest({ code: 'export { foo } from "./does-not-exist"' , errors: ["Unable to resolve path to module './does-not-exist'."] }), diff --git a/tests/src/rules/no-useless-path-segments.js b/tests/src/rules/no-useless-path-segments.js index 923c7efe7..52e66ec6f 100644 --- a/tests/src/rules/no-useless-path-segments.js +++ b/tests/src/rules/no-useless-path-segments.js @@ -17,7 +17,6 @@ function runResolverTests(resolver) { test({ code: 'import "."' }), test({ code: 'import ".."' }), test({ code: 'import fs from "fs"' }), - test({ code: 'import fs from "fs"' }), // ES modules + noUselessIndex test({ code: 'import "../index"' }), // noUselessIndex is false by default @@ -28,6 +27,13 @@ function runResolverTests(resolver) { test({ code: 'import "./malformed.js"', options: [{ noUselessIndex: true }] }), // ./malformed directory does not exist test({ code: 'import "./malformed"', options: [{ noUselessIndex: true }] }), // ./malformed directory does not exist test({ code: 'import "./importType"', options: [{ noUselessIndex: true }] }), // ./importType.js does not exist + + test({ code: 'import(".")' + , parser: 'babel-eslint' }), + test({ code: 'import("..")' + , parser: 'babel-eslint' }), + test({ code: 'import("fs").then(function(fs){})' + , parser: 'babel-eslint' }), ], invalid: [ @@ -190,6 +196,21 @@ function runResolverTests(resolver) { options: [{ noUselessIndex: true }], errors: ['Useless path segments for "../index.js", should be ".."'], }), + test({ + code: 'import("./")', + errors: [ 'Useless path segments for "./", should be "."'], + parser: 'babel-eslint', + }), + test({ + code: 'import("../")', + errors: [ 'Useless path segments for "../", should be ".."'], + parser: 'babel-eslint', + }), + test({ + code: 'import("./deep//a")', + errors: [ 'Useless path segments for "./deep//a", should be "./deep/a"'], + parser: 'babel-eslint', + }), ], }) } diff --git a/utils/moduleVisitor.js b/utils/moduleVisitor.js index 2e736242e..bc8c91b0a 100644 --- a/utils/moduleVisitor.js +++ b/utils/moduleVisitor.js @@ -91,7 +91,9 @@ exports.default = function visitModules(visitor, options) { } if (options.commonjs || options.amd) { + const currentCallExpression = visitors['CallExpression'] visitors['CallExpression'] = function (call) { + if (currentCallExpression) currentCallExpression(call) if (options.commonjs) checkCommon(call) if (options.amd) checkAMD(call) }