From 54d86c8a64dc21c14088fcdd8fc3935206e8347a Mon Sep 17 00:00:00 2001 From: vikr01 Date: Thu, 25 Oct 2018 01:44:45 -0700 Subject: [PATCH] [New] `named`: add `commonjs` option --- CHANGELOG.md | 4 +- src/rules/named.js | 80 ++++++++++++++++++++++++++++++++++++---- tests/src/rules/named.js | 59 +++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41799fb6d..493c70940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Added - [`no-dynamic-require`]: add option `esmodule` ([#1223], thanks [@vikr01]) +- [`named`]: add `commonjs` option ([#1222], thanks [@vikr01]) ### Fixed - [`no-duplicates`]: ensure autofix avoids excessive newlines ([#2028], thanks [@ertrzyiks]) @@ -891,7 +892,6 @@ for info on changes for earlier releases. [#1619]: https://github.com/benmosher/eslint-plugin-import/pull/1619 [#1612]: https://github.com/benmosher/eslint-plugin-import/pull/1612 [#1611]: https://github.com/benmosher/eslint-plugin-import/pull/1611 -[#1223]: https://github.com/benmosher/eslint-plugin-import/pull/1223 [#1605]: https://github.com/benmosher/eslint-plugin-import/pull/1605 [#1586]: https://github.com/benmosher/eslint-plugin-import/pull/1586 [#1572]: https://github.com/benmosher/eslint-plugin-import/pull/1572 @@ -963,6 +963,8 @@ for info on changes for earlier releases. [#1235]: https://github.com/benmosher/eslint-plugin-import/pull/1235 [#1234]: https://github.com/benmosher/eslint-plugin-import/pull/1234 [#1232]: https://github.com/benmosher/eslint-plugin-import/pull/1232 +[#1223]: https://github.com/benmosher/eslint-plugin-import/pull/1223 +[#1222]: https://github.com/benmosher/eslint-plugin-import/pull/1222 [#1218]: https://github.com/benmosher/eslint-plugin-import/pull/1218 [#1176]: https://github.com/benmosher/eslint-plugin-import/pull/1176 [#1163]: https://github.com/benmosher/eslint-plugin-import/pull/1163 diff --git a/src/rules/named.js b/src/rules/named.js index bcf143beb..2770d51fa 100644 --- a/src/rules/named.js +++ b/src/rules/named.js @@ -8,10 +8,22 @@ module.exports = { docs: { url: docsUrl('named'), }, - schema: [], + schema: [ + { + type: 'object', + properties: { + commonjs: { + type: 'boolean', + }, + }, + additionalProperties: false, + }, + ], }, create: function (context) { + const options = context.options[0] || {}; + function checkSpecifiers(key, type, node) { // ignore local exports and type imports/exports if ( @@ -38,12 +50,11 @@ module.exports = { } node.specifiers.forEach(function (im) { - if (im.type !== type) { - return; - } - - // ignore type imports - if (im.importKind === 'type' || im.importKind === 'typeof') { + if ( + im.type !== type + // ignore type imports + || im.importKind === 'type' || im.importKind === 'typeof' + ) { return; } @@ -63,10 +74,65 @@ module.exports = { }); } + function checkRequire(node) { + if ( + !options.commonjs + || node.type !== 'VariableDeclarator' + // return if it's not an object destructure or it's an empty object destructure + || !node.id || node.id.type !== 'ObjectPattern' || node.id.properties.length === 0 + // return if there is no call expression on the right side + || !node.init || node.init.type !== 'CallExpression' + ) { + return; + } + + const call = node.init; + const [source] = call.arguments; + const variableImports = node.id.properties; + const variableExports = Exports.get(source.value, context); + + if ( + // return if it's not a commonjs require statement + call.callee.type !== 'Identifier' || call.callee.name !== 'require' || call.arguments.length !== 1 + // return if it's not a string source + || source.type !== 'Literal' + || variableExports == null + ) { + return; + } + + if (variableExports.errors.length) { + variableExports.reportErrors(context, node); + return; + } + + variableImports.forEach(function (im) { + if (im.type !== 'Property' || !im.key || im.key.type !== 'Identifier') { + return; + } + + const deepLookup = variableExports.hasDeep(im.key.name); + + if (!deepLookup.found) { + if (deepLookup.path.length > 1) { + const deepPath = deepLookup.path + .map(i => path.relative(path.dirname(context.getFilename()), i.path)) + .join(' -> '); + + context.report(im.key, `${im.key.name} not found via ${deepPath}`); + } else { + context.report(im.key, im.key.name + ' not found in \'' + source.value + '\''); + } + } + }); + } + return { ImportDeclaration: checkSpecifiers.bind(null, 'imported', 'ImportSpecifier'), ExportNamedDeclaration: checkSpecifiers.bind(null, 'local', 'ExportSpecifier'), + + VariableDeclarator: checkRequire, }; }, }; diff --git a/tests/src/rules/named.js b/tests/src/rules/named.js index f09ee2059..57e40c91a 100644 --- a/tests/src/rules/named.js +++ b/tests/src/rules/named.js @@ -146,6 +146,41 @@ ruleTester.run('named', rule, { code: 'import { common } from "./re-export-default"', }), + // destructured requires with commonjs option + test({ + code: 'const { destructuredProp } = require("./named-exports")', + options: [{ commonjs: true }], + }), + test({ + code: 'let { arrayKeyProp } = require("./named-exports")', + options: [{ commonjs: true }], + }), + test({ + code: 'const { deepProp } = require("./named-exports")', + options: [{ commonjs: true }], + }), + + test({ + code: 'const { foo, bar } = require("./re-export-names")', + options: [{ commonjs: true }], + }), + + test({ + code: 'const { baz } = require("./bar")', + errors: [error('baz', './bar')], + }), + + test({ + code: 'const { baz } = require("./bar")', + errors: [error('baz', './bar')], + options: [{ commonjs: false }], + }), + + test({ + code: 'const { default: defExport } = require("./bar")', + options: [{ commonjs: true }], + }), + ...SYNTAX_CASES, ], @@ -201,6 +236,30 @@ ruleTester.run('named', rule, { errors: ['baz not found via broken-trampoline.js -> named-exports.js'], }), + test({ + code: 'const { baz } = require("./bar")', + errors: [error('baz', './bar')], + options: [{ commonjs: true }], + }), + + test({ + code: 'let { baz } = require("./bar")', + errors: [error('baz', './bar')], + options: [{ commonjs: true }], + }), + + test({ + code: 'const { baz: bar, bop } = require("./bar"), { a } = require("./re-export-names")', + errors: [error('baz', './bar'), error('bop', './bar'), error('a', './re-export-names')], + options: [{ commonjs: true }], + }), + + test({ + code: 'const { default: defExport } = require("./named-exports")', + errors: [error('default', './named-exports')], + options: [{ commonjs: true }], + }), + // parse errors // test({ // code: "import { a } from './test.coffee';",