Skip to content

Commit

Permalink
[Fix] [Generic Import Callback] Make callback for all import once in …
Browse files Browse the repository at this point in the history
…rules

Fixes import-js#1230.
  • Loading branch information
ljqx authored and ljharb committed Nov 19, 2018
1 parent 2ae68c1 commit 5037470
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 92 deletions.
12 changes: 4 additions & 8 deletions src/rules/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import path from 'path';

import resolve from 'eslint-module-utils/resolve';
import { isBuiltIn, isExternalModule, isScoped, isScopedModule } from '../core/importType';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
import docsUrl from '../docsUrl';

const enumValues = { enum: [ 'always', 'ignorePackages', 'never' ] };
Expand Down Expand Up @@ -134,12 +135,10 @@ module.exports = {
return false;
}

function checkFileExtension(node) {
const { source } = node;

function checkFileExtension(source) {
// bail if the declaration doesn't have a source, e.g. "export { foo };"
if (!source) return;

const importPathWithQueryString = source.value;

// don't enforce anything on builtins
Expand Down Expand Up @@ -181,9 +180,6 @@ module.exports = {
}
}

return {
ImportDeclaration: checkFileExtension,
ExportNamedDeclaration: checkFileExtension,
};
return moduleVisitor(checkFileExtension, { commonjs: true });
},
};
22 changes: 6 additions & 16 deletions src/rules/max-dependencies.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import isStaticRequire from '../core/staticRequire';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
import docsUrl from '../docsUrl';

const DEFAULT_MAX = 10;
Expand Down Expand Up @@ -36,23 +36,13 @@ module.exports = {
const dependencies = new Set(); // keep track of dependencies
let lastNode; // keep track of the last node to report on

return {
ImportDeclaration(node) {
dependencies.add(node.source.value);
lastNode = node.source;
},

CallExpression(node) {
if (isStaticRequire(node)) {
const [ requirePath ] = node.arguments;
dependencies.add(requirePath.value);
lastNode = node;
}
},

return Object.assign({
'Program:exit': function () {
countDependencies(dependencies, lastNode, context);
},
};
}, moduleVisitor((source) => {
dependencies.add(source.value);
lastNode = source;
}, { commonjs: true }));
},
};
4 changes: 2 additions & 2 deletions src/rules/no-extraneous-dependencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ module.exports = {
allowBundledDeps: testConfig(options.bundledDependencies, filename) !== false,
};

return moduleVisitor(node => {
reportIfMissing(context, deps, depsOptions, node, node.value);
return moduleVisitor((source, node) => {
reportIfMissing(context, deps, depsOptions, node, source.value);
}, { commonjs: true });
},
};
24 changes: 4 additions & 20 deletions src/rules/no-internal-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import minimatch from 'minimatch';

import resolve from 'eslint-module-utils/resolve';
import importType from '../core/importType';
import isStaticRequire from '../core/staticRequire';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
import docsUrl from '../docsUrl';

module.exports = {
Expand Down Expand Up @@ -87,24 +87,8 @@ module.exports = {
}
}

return {
ImportDeclaration(node) {
checkImportForReaching(node.source.value, node.source);
},
ExportAllDeclaration(node) {
checkImportForReaching(node.source.value, node.source);
},
ExportNamedDeclaration(node) {
if (node.source) {
checkImportForReaching(node.source.value, node.source);
}
},
CallExpression(node) {
if (isStaticRequire(node)) {
const [ firstArgument ] = node.arguments;
checkImportForReaching(firstArgument.value, firstArgument);
}
},
};
return moduleVisitor((source) => {
checkImportForReaching(source.value, source);
}, { commonjs: true });
},
};
15 changes: 4 additions & 11 deletions src/rules/no-nodejs-modules.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import importType from '../core/importType';
import isStaticRequire from '../core/staticRequire';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
import docsUrl from '../docsUrl';

function reportIfMissing(context, node, allowed, name) {
Expand Down Expand Up @@ -35,15 +35,8 @@ module.exports = {
const options = context.options[0] || {};
const allowed = options.allow || [];

return {
ImportDeclaration: function handleImports(node) {
reportIfMissing(context, node, allowed, node.source.value);
},
CallExpression: function handleRequires(node) {
if (isStaticRequire(node)) {
reportIfMissing(context, node, allowed, node.arguments[0].value);
}
},
};
return moduleVisitor((source, node) => {
reportIfMissing(context, node, allowed, source.value);
}, { commonjs: true });
},
};
17 changes: 4 additions & 13 deletions src/rules/no-restricted-paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import containsPath from 'contains-path';
import path from 'path';

import resolve from 'eslint-module-utils/resolve';
import isStaticRequire from '../core/staticRequire';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
import docsUrl from '../docsUrl';
import importType from '../core/importType';

Expand Down Expand Up @@ -109,17 +109,8 @@ module.exports = {
});
}

return {
ImportDeclaration(node) {
checkForRestrictedImportPath(node.source.value, node.source);
},
CallExpression(node) {
if (isStaticRequire(node)) {
const [ firstArgument ] = node.arguments;

checkForRestrictedImportPath(firstArgument.value, firstArgument);
}
},
};
return moduleVisitor((source) => {
checkForRestrictedImportPath(source.value, source);
}, { commonjs: true });
},
};
15 changes: 4 additions & 11 deletions src/rules/no-self-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import resolve from 'eslint-module-utils/resolve';
import isStaticRequire from '../core/staticRequire';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
import docsUrl from '../docsUrl';

function isImportingSelf(context, node, requireName) {
Expand All @@ -31,15 +31,8 @@ module.exports = {
schema: [],
},
create: function (context) {
return {
ImportDeclaration(node) {
isImportingSelf(context, node, node.source.value);
},
CallExpression(node) {
if (isStaticRequire(node)) {
isImportingSelf(context, node, node.arguments[0].value);
}
},
};
return moduleVisitor((source, node) => {
isImportingSelf(context, node, source.value);
}, { commonjs: true });
},
};
15 changes: 4 additions & 11 deletions src/rules/no-webpack-loader-syntax.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import isStaticRequire from '../core/staticRequire';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
import docsUrl from '../docsUrl';

function reportIfNonStandard(context, node, name) {
Expand All @@ -19,15 +19,8 @@ module.exports = {
},

create: function (context) {
return {
ImportDeclaration: function handleImports(node) {
reportIfNonStandard(context, node, node.source.value);
},
CallExpression: function handleRequires(node) {
if (isStaticRequire(node)) {
reportIfNonStandard(context, node, node.arguments[0].value);
}
},
};
return moduleVisitor((source, node) => {
reportIfNonStandard(context, node, source.value);
}, { commonjs: true });
},
};
93 changes: 93 additions & 0 deletions tests/src/rules/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,22 @@ ruleTester.run('extensions', rule, {
options: [ 'never', { ignorePackages: true } ],
}),

test({
code: `
import foo from './foo.js'
import bar from './bar.json'
import Component from './Component.jsx'
`,
errors: [
{
message: 'Unexpected use of file extension "jsx" for "./Component.jsx"',
line: 4,
column: 31,
},
],
options: [ 'always', { pattern: { jsx: 'never' } } ],
}),

// export (#964)
test({
code: [
Expand Down Expand Up @@ -444,6 +460,48 @@ ruleTester.run('extensions', rule, {
},
],
}),
// require (#1230)
test({
code: [
'const { foo } = require("./foo")',
'export { foo }',
].join('\n'),
options: [ 'always' ],
errors: [
{
message: 'Missing file extension for "./foo"',
line: 1,
column: 25,
},
],
}),
test({
code: [
'const { foo } = require("./foo.js")',
'export { foo }',
].join('\n'),
options: [ 'never' ],
errors: [
{
message: 'Unexpected use of file extension "js" for "./foo.js"',
line: 1,
column: 25,
},
],
}),

// export { } from
test({
code: 'export { foo } from "./foo"',
options: [ 'always' ],
errors: [
{
message: 'Missing file extension for "./foo"',
line: 1,
column: 21,
},
],
}),
test({
code: 'import foo from "@/ImNotAScopedModule"',
options: ['always'],
Expand All @@ -454,5 +512,40 @@ ruleTester.run('extensions', rule, {
},
],
}),
test({
code: 'export { foo } from "./foo.js"',
options: [ 'never' ],
errors: [
{
message: 'Unexpected use of file extension "js" for "./foo.js"',
line: 1,
column: 21,
},
],
}),

// export * from
test({
code: 'export * from "./foo"',
options: [ 'always' ],
errors: [
{
message: 'Missing file extension for "./foo"',
line: 1,
column: 15,
},
],
}),
test({
code: 'export * from "./foo.js"',
options: [ 'never' ],
errors: [
{
message: 'Unexpected use of file extension "js" for "./foo.js"',
line: 1,
column: 15,
},
],
}),
],
});

0 comments on commit 5037470

Please sign in to comment.