From 70474daf88ffb56c5c7e9f2807eef0fdbcc9b8dc Mon Sep 17 00:00:00 2001 From: futpib Date: Fri, 3 Jul 2020 11:57:20 +0300 Subject: [PATCH 01/23] Stub initial implementation --- index.js | 1 + readme.md | 2 + rules/import-style.js | 26 ++++++++++++ test/import-style.js | 96 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+) create mode 100644 rules/import-style.js create mode 100644 test/import-style.js diff --git a/index.js b/index.js index 1c0b88e4b1..84fc9b0526 100644 --- a/index.js +++ b/index.js @@ -27,6 +27,7 @@ module.exports = { 'unicorn/explicit-length-check': 'error', 'unicorn/filename-case': 'error', 'unicorn/import-index': 'error', + 'unicorn/import-style': 'error', 'unicorn/new-for-builtins': 'error', 'unicorn/no-abusive-eslint-disable': 'error', 'unicorn/no-array-instanceof': 'error', diff --git a/readme.md b/readme.md index 37d1a14fb1..ce72a40bf1 100644 --- a/readme.md +++ b/readme.md @@ -43,6 +43,7 @@ Configure it in `package.json`. "unicorn/explicit-length-check": "error", "unicorn/filename-case": "error", "unicorn/import-index": "error", + "unicorn/import-style": "error", "unicorn/new-for-builtins": "error", "unicorn/no-abusive-eslint-disable": "error", "unicorn/no-array-instanceof": "error", @@ -106,6 +107,7 @@ Configure it in `package.json`. - [explicit-length-check](docs/rules/explicit-length-check.md) - Enforce explicitly comparing the `length` property of a value. *(partly fixable)* - [filename-case](docs/rules/filename-case.md) - Enforce a case style for filenames. - [import-index](docs/rules/import-index.md) - Enforce importing index files with `.`. *(fixable)* +- [import-style](docs/rules/import-style.md) - Enforce default or named import per module. - [new-for-builtins](docs/rules/new-for-builtins.md) - Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. *(fixable)* - [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) - Enforce specifying rules to disable in `eslint-disable` comments. - [no-array-instanceof](docs/rules/no-array-instanceof.md) - Require `Array.isArray()` instead of `instanceof Array`. *(fixable)* diff --git a/rules/import-style.js b/rules/import-style.js new file mode 100644 index 0000000000..236709f684 --- /dev/null +++ b/rules/import-style.js @@ -0,0 +1,26 @@ +'use strict'; +const getDocumentationUrl = require('./utils/get-documentation-url'); + +const DEFAULT_MESSAGE_ID = 'importStyleDefault'; +const NAMESPACE_MESSAGE_ID = 'importStyleNamespace'; +const NAMED_MESSAGE_ID = 'importStyleNamed'; + +const create = context => { + return { + }; +}; + +module.exports = { + create, + meta: { + type: 'problem', + docs: { + url: getDocumentationUrl(__filename) + }, + messages: { + [DEFAULT_MESSAGE_ID]: 'Use default import for module `{{module}}`.', + [NAMESPACE_MESSAGE_ID]: 'Use namespace import for module `{{parameter}}`.', + [NAMED_MESSAGE_ID]: 'Use named import for module `{{parameter}}`.', + } + } +}; diff --git a/test/import-style.js b/test/import-style.js new file mode 100644 index 0000000000..97e1a2cb54 --- /dev/null +++ b/test/import-style.js @@ -0,0 +1,96 @@ +import test from 'ava'; +import avaRuleTester from 'eslint-ava-rule-tester'; +import {outdent} from 'outdent'; +import rule from '../rules/import-style'; + +const ruleTester = avaRuleTester(test, { + parserOptions: { + sourceType: 'module', + ecmaVersion: 2020 + } +}); + +const defaultError = { + messageId: 'importStyleDefault', + data: {module: 'default'}, +}; + +const namespaceError = { + messageId: 'importStyleNamespace', + data: {module: 'namespace'}, +}; + +const namedError = { + messageId: 'importStyleNamed', + data: {module: 'named'}, +}; + +ruleTester.run('import-style', rule, { + valid: [ + `const x = require('default')`, + `const {default: x} = require('default')`, + `import x from 'default'`, + + `const x = require('namespace')`, + `import * as x from 'namespace'`, + + `const {x} = require('named')`, + `import {x} from 'named'`, + ], + invalid: [ + { + code: `import * as x from 'default'`, + errors: [defaultError] + }, + { + code: `const {x} = require('default')`, + errors: [defaultError] + }, + { + code: `const {x: y} = require('default')`, + errors: [defaultError] + }, + { + code: `import {x} = require('default')`, + errors: [defaultError] + }, + { + code: `import {x as y} from 'default'`, + errors: [defaultError] + }, + + { + code: `const {default: x} = require('namespace')`, + errors: [namespaceError] + }, + { + code: `import x from 'namespace'`, + errors: [namespaceError] + }, + { + code: `const {x} = require('namespace')`, + errors: [namespaceError] + }, + { + code: `import {x} from 'namespace'`, + errors: [namespaceError] + }, + + { + code: `const x = require('named')`, + errors: [namedError] + }, + { + code: `const {default: x} = require('named')`, + errors: [namedError] + }, + { + code: `import x from 'named'`, + errors: [namedError] + }, + { + code: `import * as x from 'named'`, + errors: [namedError] + }, + ] +}); From 16b095ef83719de8722565d9197a71caeb838035 Mon Sep 17 00:00:00 2001 From: futpib Date: Fri, 3 Jul 2020 11:57:47 +0300 Subject: [PATCH 02/23] feedback commit --- docs/rules/import-style.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 docs/rules/import-style.md diff --git a/docs/rules/import-style.md b/docs/rules/import-style.md new file mode 100644 index 0000000000..fbc5661485 --- /dev/null +++ b/docs/rules/import-style.md @@ -0,0 +1,22 @@ +# Whether to allow default imports or destructuring/named imports + +Sometimes a module contains unrelated functions like `util`, thus it is a good practice to enforce destructuring or named imports here, other times in modules like `path` it is good to do default import as they have similar functions, likely to be utilised. By default `path` and `chalk` are enforced to have default export and `util`, `lodash` and `underscore` are having named export. But you can easily override these properties by passing `false` for respective module. + +## Fail + +```js +const util = require('util'); + +import util from 'util'; + +import * as util from 'util'; +``` + + +## Pass + +```js +const {promisify} = require('util'); + +import {promisify} from 'util'; +``` From 0fcf6cdf74be60f8c53f1f933be80b1a393f92ae Mon Sep 17 00:00:00 2001 From: futpib Date: Fri, 3 Jul 2020 13:22:05 +0300 Subject: [PATCH 03/23] Handle import declaration --- rules/import-style.js | 120 +++++++++++++++++++++-- test/import-style.js | 217 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 322 insertions(+), 15 deletions(-) diff --git a/rules/import-style.js b/rules/import-style.js index 236709f684..bea5fc0bd2 100644 --- a/rules/import-style.js +++ b/rules/import-style.js @@ -1,12 +1,122 @@ 'use strict'; +const {getStringIfConstant} = require('eslint-utils'); + const getDocumentationUrl = require('./utils/get-documentation-url'); -const DEFAULT_MESSAGE_ID = 'importStyleDefault'; -const NAMESPACE_MESSAGE_ID = 'importStyleNamespace'; -const NAMED_MESSAGE_ID = 'importStyleNamed'; +const getAllowedImportStyles = (styles, moduleName) => { + const importStyles = styles[moduleName]; + + if (Array.isArray(importStyles)) { + return importStyles; + } + + if (typeof importStyles === 'string') { + return [ importStyles ]; + } + + return null; +} + +const getActualImportDeclarationStyles = importDeclaration => { + const { specifiers } = importDeclaration; + + if (specifiers.length === 0) { + return [ 'unassigned' ]; + } + + const styles = new Set(); + + for (const specifier of specifiers) { + if (specifier.type === 'ImportDefaultSpecifier') { + styles.add('default'); + continue; + } + + if (specifier.type === 'ImportNamespaceSpecifier') { + styles.add('namespace'); + continue; + } + + if (specifier.type === 'ImportSpecifier') { + styles.add('named'); + continue; + } + } + + return [ ...styles ]; +}; + +const joinOr = words => { + return words + .map((word, index) => { + if (index === (words.length - 1)) { + return word; + } + + if (index === (words.length - 2)) { + return word + ' or'; + } + + return word + ','; + }) + .join(' '); +} + +const MESSAGE_ID = 'importStyle'; + +const defaultStyles = { + util: 'named', + path: 'default', + chalk: 'default', +}; const create = context => { + let [ + { + styles = {} + } = {} + ] = context.options; + + styles = { + ...defaultStyles, + ...styles, + }; + + const report = (node, moduleName, actualImportStyles, allowedImportStyles) => { + const data = { + allowedStyles: joinOr(allowedImportStyles), + moduleName, + }; + + context.report({ + node, + messageId: MESSAGE_ID, + data, + }); + }; + return { + ImportDeclaration(node) { + const moduleName = getStringIfConstant(node.source, context.getScope()); + + if (!moduleName) { + return; + } + + const allowedImportStyles = getAllowedImportStyles(styles, moduleName); + + if (!allowedImportStyles || allowedImportStyles.length === 0) { + return; + } + + const actualImportStyles = getActualImportDeclarationStyles(node); + + if (actualImportStyles.every(style => allowedImportStyles.includes(style))) { + return; + } + + report(node, moduleName, actualImportStyles, allowedImportStyles); + }, }; }; @@ -18,9 +128,7 @@ module.exports = { url: getDocumentationUrl(__filename) }, messages: { - [DEFAULT_MESSAGE_ID]: 'Use default import for module `{{module}}`.', - [NAMESPACE_MESSAGE_ID]: 'Use namespace import for module `{{parameter}}`.', - [NAMED_MESSAGE_ID]: 'Use named import for module `{{parameter}}`.', + [MESSAGE_ID]: 'Use {{allowedStyles}} import for module `{{moduleName}}`.', } } }; diff --git a/test/import-style.js b/test/import-style.js index 97e1a2cb54..56b528a465 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -10,38 +10,181 @@ const ruleTester = avaRuleTester(test, { } }); +const options = { + styles: { + unassigned: 'unassigned', + default: 'default', + namespace: 'namespace', + named: 'named', + }, +}; + +const unassignedError = { + messageId: 'importStyle', + data: { + allowedStyles: 'unassigned', + moduleName: 'unassigned', + }, +}; + const defaultError = { - messageId: 'importStyleDefault', - data: {module: 'default'}, + messageId: 'importStyle', + data: { + allowedStyles: 'default', + moduleName: 'default' + }, }; const namespaceError = { - messageId: 'importStyleNamespace', - data: {module: 'namespace'}, + messageId: 'importStyle', + data: { + allowedStyles: 'namespace', + moduleName: 'namespace' + }, }; const namedError = { - messageId: 'importStyleNamed', - data: {module: 'named'}, + messageId: 'importStyle', + data: { + allowedStyles: 'named', + moduleName: 'named' + }, }; +const addOptions = test => { + if (typeof test === 'string') { + test = { + code: test, + }; + } + + return { + options: [ options ], + ...test + }; +} + ruleTester.run('import-style', rule, { valid: [ + `require('unassigned')`, + `import 'unassigned'`, + `import('unassigned')`, + `const x = require('default')`, `const {default: x} = require('default')`, `import x from 'default'`, + outdent` + async () => { + const {default: x} = await import('default'); + } + `, `const x = require('namespace')`, `import * as x from 'namespace'`, + outdent` + async () => { + const x = await import('namespace'); + } + `, `const {x} = require('named')`, + `const {x: y} = require('named')`, `import {x} from 'named'`, - ], + `import {x as y} from 'named'`, + outdent` + async () => { + const {x} = await import('named'); + } + `, + outdent` + async () => { + const {x: y} = await import('named'); + } + `, + ].map(addOptions), + invalid: [ + { + code: `const {x} = require('unassigned')`, + errors: [unassignedError] + }, + { + code: `const {default: x} = require('unassigned')`, + errors: [unassignedError] + }, + { + code: `import x from 'unassigned'`, + errors: [unassignedError] + }, + { + code: outdent` + async () => { + const {default: x} = await import('unassigned'); + } + `, + errors: [unassignedError] + }, + { + code: `const x = require('unassigned')`, + errors: [unassignedError] + }, + { + code: `import * as x from 'unassigned'`, + errors: [unassignedError] + }, + { + code: outdent` + async () => { + const x = await import('unassigned'); + } + `, + errors: [unassignedError] + }, + { + code: `const {x} = require('unassigned')`, + errors: [unassignedError] + }, + { + code: `const {x: y} = require('unassigned')`, + errors: [unassignedError] + }, + { + code: `import {x} from 'unassigned'`, + errors: [unassignedError] + }, + { + code: `import {x as y} from 'unassigned'`, + errors: [unassignedError] + }, + { + code: outdent` + async () => { + const {x} = await import('unassigned'); + } + `, + errors: [unassignedError] + }, + { + code: outdent` + async () => { + const {x: y} = await import('unassigned'); + } + `, + errors: [unassignedError] + }, + { code: `import * as x from 'default'`, errors: [defaultError] }, + { + code: outdent` + async () => { + const x = await import('default'); + } + `, + errors: [defaultError] + }, { code: `const {x} = require('default')`, errors: [defaultError] @@ -51,13 +194,29 @@ ruleTester.run('import-style', rule, { errors: [defaultError] }, { - code: `import {x} = require('default')`, + code: `import {x} from 'default'`, errors: [defaultError] }, { code: `import {x as y} from 'default'`, errors: [defaultError] }, + { + code: outdent` + async () => { + const {x} = await import('default'); + } + `, + errors: [defaultError] + }, + { + code: outdent` + async () => { + const {x: y} = await import('default'); + } + `, + errors: [defaultError] + }, { code: `const {default: x} = require('namespace')`, @@ -71,10 +230,34 @@ ruleTester.run('import-style', rule, { code: `const {x} = require('namespace')`, errors: [namespaceError] }, + { + code: `const {x: y} = require('namespace')`, + errors: [namespaceError] + }, { code: `import {x} from 'namespace'`, errors: [namespaceError] }, + { + code: `import {x as y} from 'namespace'`, + errors: [namespaceError] + }, + { + code: outdent` + async () => { + const {x} = await import('namespace'); + } + `, + errors: [namespaceError] + }, + { + code: outdent` + async () => { + const {x: y} = await import('namespace'); + } + `, + errors: [namespaceError] + }, { code: `const x = require('named')`, @@ -88,9 +271,25 @@ ruleTester.run('import-style', rule, { code: `import x from 'named'`, errors: [namedError] }, + { + code: outdent` + async () => { + const {default: x} = await import('named'); + } + `, + errors: [namedError] + }, { code: `import * as x from 'named'`, errors: [namedError] }, - ] + { + code: outdent` + async () => { + const x = await import('named'); + } + `, + errors: [namedError] + }, + ].map(addOptions) }); From 93e72083099ee83864158e4f6c7bcd2c91593d41 Mon Sep 17 00:00:00 2001 From: futpib Date: Fri, 3 Jul 2020 15:10:18 +0300 Subject: [PATCH 04/23] Upgrade eslint-template-visitor --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 153710638d..0c66def885 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "ci-info": "^2.0.0", "clean-regexp": "^1.0.0", "eslint-ast-utils": "^1.1.0", - "eslint-template-visitor": "^2.0.0", + "eslint-template-visitor": "^2.2.1", "eslint-utils": "^2.1.0", "import-modules": "^2.0.0", "lodash": "^4.17.15", From a70428c43473d393561246554915edc14d19ea76 Mon Sep 17 00:00:00 2001 From: futpib Date: Fri, 3 Jul 2020 23:24:27 +0300 Subject: [PATCH 05/23] Convert options to object-style, add more tests --- rules/import-style.js | 264 ++++++++++++++++++++++++++++++++++------ test/import-style.js | 274 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 447 insertions(+), 91 deletions(-) diff --git a/rules/import-style.js b/rules/import-style.js index bea5fc0bd2..c323756e6b 100644 --- a/rules/import-style.js +++ b/rules/import-style.js @@ -1,27 +1,15 @@ 'use strict'; +const {defaultsDeep} = require('lodash'); const {getStringIfConstant} = require('eslint-utils'); +const eslintTemplateVisitor = require('eslint-template-visitor'); const getDocumentationUrl = require('./utils/get-documentation-url'); -const getAllowedImportStyles = (styles, moduleName) => { - const importStyles = styles[moduleName]; - - if (Array.isArray(importStyles)) { - return importStyles; - } - - if (typeof importStyles === 'string') { - return [ importStyles ]; - } - - return null; -} - const getActualImportDeclarationStyles = importDeclaration => { - const { specifiers } = importDeclaration; + const {specifiers} = importDeclaration; if (specifiers.length === 0) { - return [ 'unassigned' ]; + return ['unassigned']; } const styles = new Set(); @@ -38,12 +26,41 @@ const getActualImportDeclarationStyles = importDeclaration => { } if (specifier.type === 'ImportSpecifier') { + if (specifier.imported.type === 'Identifier' && specifier.imported.name === 'default') { + styles.add('default'); + continue; + } + styles.add('named'); continue; } } - return [ ...styles ]; + return [...styles]; +}; + +const getActualAssignmentTargetImportStyles = assignmentTarget => { + if (assignmentTarget.type === 'Identifier') { + return ['namespace']; + } + + if (assignmentTarget.type === 'ObjectPattern') { + const styles = new Set(); + + for (const property of assignmentTarget.properties) { + if (property.key.type === 'Identifier') { + if (property.key.name === 'default') { + styles.add('default'); + } else { + styles.add('named'); + } + } + } + + return [...styles]; + } + + return []; }; const joinOr = words => { @@ -60,66 +77,234 @@ const joinOr = words => { return word + ','; }) .join(' '); -} +}; const MESSAGE_ID = 'importStyle'; -const defaultStyles = { - util: 'named', - path: 'default', - chalk: 'default', +// Keep this alphabetically sorted for easier maintenance +const defaultStyles = { + chalk: { + default: true + }, + path: { + default: true + }, + util: { + named: true + } }; +const templates = eslintTemplateVisitor({ + parserOptions: { + sourceType: 'module', + ecmaVersion: 2018 + } +}); + +const variableDeclarationVariable = templates.variableDeclarationVariable(); +const assignmentTargetVariable = templates.variable(); +const moduleNameVariable = templates.variable(); + +const assignedDynamicImportTemplate = templates.template`async () => { + ${variableDeclarationVariable} ${assignmentTargetVariable} = await import(${moduleNameVariable}); +}`.narrow('BlockStatement > :has(AwaitExpression)'); + +const assignedRequireTemplate = templates.template` + ${variableDeclarationVariable} ${assignmentTargetVariable} = require(${moduleNameVariable}); +`; + const create = context => { let [ { - styles = {} + styles = {}, + extendDefaultStyles = true, + checkImport = true, + checkDynamicImport = true, + checkRequire = true } = {} ] = context.options; - styles = { - ...defaultStyles, - ...styles, - }; + styles = extendDefaultStyles ? + defaultsDeep({}, styles, defaultStyles) : + styles; + + styles = new Map( + Object.entries(styles).map( + ([moduleName, styles]) => + [moduleName, new Map(Object.entries(styles))] + ) + ); + + const report = (node, moduleName, actualImportStyles, allowedImportStyles, isRequire = false) => { + if (!allowedImportStyles || allowedImportStyles.size === 0) { + return; + } + + let effectiveAllowedImportStyles = allowedImportStyles; + + if (isRequire) { + // For `require`, `'default'` style allows both `x = require('x')` (`'namespace'` style) and + // `{default: x} = require('x')` (`'default'` style) since we don't know in advance + // whether `'x'` is a compiled ES6 module (with `default` key) or a CommonJS module and `require` + // does not provide any automatic interop for this, so the user may have to use either of theese. + if (allowedImportStyles.get('default') && !allowedImportStyles.get('namespace')) { + effectiveAllowedImportStyles = new Map(allowedImportStyles); + effectiveAllowedImportStyles.set('namespace', true); + } + } + + if (actualImportStyles.every(style => effectiveAllowedImportStyles.get(style))) { + return; + } - const report = (node, moduleName, actualImportStyles, allowedImportStyles) => { const data = { - allowedStyles: joinOr(allowedImportStyles), - moduleName, + allowedStyles: joinOr([...allowedImportStyles.keys()]), + moduleName }; context.report({ node, messageId: MESSAGE_ID, - data, + data }); }; - return { + return templates.visitor({ ImportDeclaration(node) { + if (!checkImport) { + return; + } + const moduleName = getStringIfConstant(node.source, context.getScope()); if (!moduleName) { return; } - const allowedImportStyles = getAllowedImportStyles(styles, moduleName); + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = getActualImportDeclarationStyles(node); + + report(node, moduleName, actualImportStyles, allowedImportStyles); + }, + + 'ExpressionStatement > ImportExpression'(node) { + if (!checkDynamicImport) { + return; + } + + const moduleName = getStringIfConstant(node.source, context.getScope()); + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = ['unassigned']; - if (!allowedImportStyles || allowedImportStyles.length === 0) { + report(node, moduleName, actualImportStyles, allowedImportStyles); + }, + + [assignedDynamicImportTemplate](node) { + if (!checkDynamicImport) { return; } - const actualImportStyles = getActualImportDeclarationStyles(node); + const assignmentTargetNode = assignedDynamicImportTemplate.context.getMatch(assignmentTargetVariable); + const moduleNameNode = assignedDynamicImportTemplate.context.getMatch(moduleNameVariable); + const moduleName = getStringIfConstant(moduleNameNode, context.getScope()); - if (actualImportStyles.every(style => allowedImportStyles.includes(style))) { + if (!moduleName) { return; } + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = getActualAssignmentTargetImportStyles(assignmentTargetNode); + report(node, moduleName, actualImportStyles, allowedImportStyles); }, - }; + + 'ExpressionStatement > CallExpression[callee.name=\'require\']'(node) { + if (!checkRequire) { + return; + } + + if (node.arguments.length !== 1) { + return; + } + + const moduleName = getStringIfConstant(node.arguments[0], context.getScope()); + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = ['unassigned']; + + report(node, moduleName, actualImportStyles, allowedImportStyles, true); + }, + + [assignedRequireTemplate](node) { + if (!checkRequire) { + return; + } + + const assignmentTargetNode = assignedRequireTemplate.context.getMatch(assignmentTargetVariable); + const moduleNameNode = assignedRequireTemplate.context.getMatch(moduleNameVariable); + const moduleName = getStringIfConstant(moduleNameNode, context.getScope()); + + if (!moduleName) { + return; + } + + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = getActualAssignmentTargetImportStyles(assignmentTargetNode); + + report(node, moduleName, actualImportStyles, allowedImportStyles, true); + } + }); }; +const schema = [ + { + type: 'object', + properties: { + checkImport: { + type: 'boolean' + }, + checkDynamicImport: { + type: 'boolean' + }, + checkRequire: { + type: 'boolean' + }, + extendDefaultStyles: { + type: 'boolean' + }, + styles: { + $ref: '#/items/0/definitions/moduleStyles' + } + }, + additionalProperties: false, + definitions: { + moduleStyles: { + type: 'object', + additionalProperties: { + $ref: '#/items/0/definitions/styles' + } + }, + styles: { + anyOf: [ + { + enum: [ + false + ] + }, + { + $ref: '#/items/0/definitions/booleanObject' + } + ] + }, + booleanObject: { + type: 'object', + additionalProperties: { + type: 'boolean' + } + } + } + } +]; + module.exports = { create, meta: { @@ -128,7 +313,8 @@ module.exports = { url: getDocumentationUrl(__filename) }, messages: { - [MESSAGE_ID]: 'Use {{allowedStyles}} import for module `{{moduleName}}`.', - } + [MESSAGE_ID]: 'Use {{allowedStyles}} import for module `{{moduleName}}`.' + }, + schema } }; diff --git a/test/import-style.js b/test/import-style.js index 56b528a465..3f125953d6 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -12,19 +12,27 @@ const ruleTester = avaRuleTester(test, { const options = { styles: { - unassigned: 'unassigned', - default: 'default', - namespace: 'namespace', - named: 'named', - }, + unassigned: { + unassigned: true + }, + default: { + default: true + }, + namespace: { + namespace: true + }, + named: { + named: true + } + } }; const unassignedError = { messageId: 'importStyle', data: { allowedStyles: 'unassigned', - moduleName: 'unassigned', - }, + moduleName: 'unassigned' + } }; const defaultError = { @@ -32,7 +40,7 @@ const defaultError = { data: { allowedStyles: 'default', moduleName: 'default' - }, + } }; const namespaceError = { @@ -40,7 +48,7 @@ const namespaceError = { data: { allowedStyles: 'namespace', moduleName: 'namespace' - }, + } }; const namedError = { @@ -48,49 +56,49 @@ const namedError = { data: { allowedStyles: 'named', moduleName: 'named' - }, + } }; -const addOptions = test => { +const addDefaultOptions = test => { if (typeof test === 'string') { test = { - code: test, + code: test }; } return { - options: [ options ], + options: [options], ...test }; -} +}; ruleTester.run('import-style', rule, { valid: [ - `require('unassigned')`, - `import 'unassigned'`, - `import('unassigned')`, + 'require(\'unassigned\')', + 'import \'unassigned\'', + 'import(\'unassigned\')', - `const x = require('default')`, - `const {default: x} = require('default')`, - `import x from 'default'`, + 'const x = require(\'default\')', + 'const {default: x} = require(\'default\')', + 'import x from \'default\'', outdent` async () => { const {default: x} = await import('default'); } `, - `const x = require('namespace')`, - `import * as x from 'namespace'`, + 'const x = require(\'namespace\')', + 'import * as x from \'namespace\'', outdent` async () => { const x = await import('namespace'); } `, - `const {x} = require('named')`, - `const {x: y} = require('named')`, - `import {x} from 'named'`, - `import {x as y} from 'named'`, + 'const {x} = require(\'named\')', + 'const {x: y} = require(\'named\')', + 'import {x} from \'named\'', + 'import {x as y} from \'named\'', outdent` async () => { const {x} = await import('named'); @@ -101,19 +109,91 @@ ruleTester.run('import-style', rule, { const {x: y} = await import('named'); } `, - ].map(addOptions), + + { + code: 'import {inspect} from \'util\'', + options: [] + }, + { + code: 'const {inspect} = require(\'util\')', + options: [] + }, + { + code: 'import chalk from \'chalk\'', + options: [] + }, + { + code: 'import {default as chalk} from \'chalk\'', + options: [] + }, + { + code: 'const {inspect} = require(\'util\')', + options: [] + }, + + { + code: 'require(\'chalk\')', + options: [{ + styles: {}, + extendDefaultStyles: false + }] + }, + { + code: 'import \'chalk\'', + options: [{ + checkImport: false + }] + }, + { + code: outdent` + async () => { + const {red} = await import('chalk'); + } + `, + options: [{ + checkDynamicImport: false + }] + }, + { + code: 'import(\'chalk\')', + options: [{ + checkDynamicImport: false + }] + }, + { + code: 'require(\'chalk\')', + options: [{ + checkRequire: false + }] + }, + { + code: 'const {red} = require(\'chalk\')', + options: [{ + checkRequire: false + }] + }, + + 'require(1, 2, 3)', + 'require(variable)', + 'const x = require(variable)', + outdent` + async () => { + const {red} = await import(variable); + } + ` + ].map(test => addDefaultOptions(test)), invalid: [ { - code: `const {x} = require('unassigned')`, + code: 'const {x} = require(\'unassigned\')', errors: [unassignedError] }, { - code: `const {default: x} = require('unassigned')`, + code: 'const {default: x} = require(\'unassigned\')', errors: [unassignedError] }, { - code: `import x from 'unassigned'`, + code: 'import x from \'unassigned\'', errors: [unassignedError] }, { @@ -125,11 +205,11 @@ ruleTester.run('import-style', rule, { errors: [unassignedError] }, { - code: `const x = require('unassigned')`, + code: 'const x = require(\'unassigned\')', errors: [unassignedError] }, { - code: `import * as x from 'unassigned'`, + code: 'import * as x from \'unassigned\'', errors: [unassignedError] }, { @@ -141,19 +221,19 @@ ruleTester.run('import-style', rule, { errors: [unassignedError] }, { - code: `const {x} = require('unassigned')`, + code: 'const {x} = require(\'unassigned\')', errors: [unassignedError] }, { - code: `const {x: y} = require('unassigned')`, + code: 'const {x: y} = require(\'unassigned\')', errors: [unassignedError] }, { - code: `import {x} from 'unassigned'`, + code: 'import {x} from \'unassigned\'', errors: [unassignedError] }, { - code: `import {x as y} from 'unassigned'`, + code: 'import {x as y} from \'unassigned\'', errors: [unassignedError] }, { @@ -174,7 +254,19 @@ ruleTester.run('import-style', rule, { }, { - code: `import * as x from 'default'`, + code: 'require(\'default\')', + errors: [defaultError] + }, + { + code: 'import \'default\'', + errors: [defaultError] + }, + { + code: 'import(\'default\')', + errors: [defaultError] + }, + { + code: 'import * as x from \'default\'', errors: [defaultError] }, { @@ -186,19 +278,19 @@ ruleTester.run('import-style', rule, { errors: [defaultError] }, { - code: `const {x} = require('default')`, + code: 'const {x} = require(\'default\')', errors: [defaultError] }, { - code: `const {x: y} = require('default')`, + code: 'const {x: y} = require(\'default\')', errors: [defaultError] }, { - code: `import {x} from 'default'`, + code: 'import {x} from \'default\'', errors: [defaultError] }, { - code: `import {x as y} from 'default'`, + code: 'import {x as y} from \'default\'', errors: [defaultError] }, { @@ -219,27 +311,39 @@ ruleTester.run('import-style', rule, { }, { - code: `const {default: x} = require('namespace')`, + code: 'require(\'namespace\')', + errors: [namespaceError] + }, + { + code: 'import \'namespace\'', + errors: [namespaceError] + }, + { + code: 'import(\'namespace\')', errors: [namespaceError] }, { - code: `import x from 'namespace'`, + code: 'const {default: x} = require(\'namespace\')', errors: [namespaceError] }, { - code: `const {x} = require('namespace')`, + code: 'import x from \'namespace\'', errors: [namespaceError] }, { - code: `const {x: y} = require('namespace')`, + code: 'const {x} = require(\'namespace\')', errors: [namespaceError] }, { - code: `import {x} from 'namespace'`, + code: 'const {x: y} = require(\'namespace\')', errors: [namespaceError] }, { - code: `import {x as y} from 'namespace'`, + code: 'import {x} from \'namespace\'', + errors: [namespaceError] + }, + { + code: 'import {x as y} from \'namespace\'', errors: [namespaceError] }, { @@ -260,15 +364,27 @@ ruleTester.run('import-style', rule, { }, { - code: `const x = require('named')`, + code: 'require(\'named\')', + errors: [namedError] + }, + { + code: 'import \'named\'', + errors: [namedError] + }, + { + code: 'import(\'named\')', + errors: [namedError] + }, + { + code: 'const x = require(\'named\')', errors: [namedError] }, { - code: `const {default: x} = require('named')`, + code: 'const {default: x} = require(\'named\')', errors: [namedError] }, { - code: `import x from 'named'`, + code: 'import x from \'named\'', errors: [namedError] }, { @@ -280,7 +396,7 @@ ruleTester.run('import-style', rule, { errors: [namedError] }, { - code: `import * as x from 'named'`, + code: 'import * as x from \'named\'', errors: [namedError] }, { @@ -291,5 +407,59 @@ ruleTester.run('import-style', rule, { `, errors: [namedError] }, - ].map(addOptions) + + { + code: 'import util from \'util\'', + options: [], + errors: [{}] + }, + { + code: 'import * as util from \'util\'', + options: [], + errors: [{}] + }, + { + code: 'const util = require(\'util\')', + options: [], + errors: [{}] + }, + { + code: 'require(\'util\')', + options: [], + errors: [{}] + }, + { + code: 'import {red} from \'chalk\'', + options: [], + errors: [{}] + }, + { + code: 'import {red as green} from \'chalk\'', + options: [], + errors: [{}] + }, + { + code: outdent` + async () => { + const {red} = await import('chalk'); + } + `, + options: [], + errors: [{}] + }, + + { + code: 'require(\'no-unassigned\')', + options: [{ + styles: { + 'no-unassigned': { + named: true, + namespace: true, + default: true + } + } + }], + errors: [{}] + } + ].map(test => addDefaultOptions(test)) }); From 590a341f34140ddf5cae3ddec126f814a15b4ac2 Mon Sep 17 00:00:00 2001 From: futpib Date: Fri, 3 Jul 2020 23:40:34 +0300 Subject: [PATCH 06/23] Fix test coverage --- rules/import-style.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rules/import-style.js b/rules/import-style.js index c323756e6b..711b37b44d 100644 --- a/rules/import-style.js +++ b/rules/import-style.js @@ -60,6 +60,10 @@ const getActualAssignmentTargetImportStyles = assignmentTarget => { return [...styles]; } + // Next line is not test-coverable until unforceable changes to the language + // like an addition of new AST node types usable in `const __HERE__ = foo;`. + // An exotic custom parser or a bug in one could cover it too. + /* istanbul ignore next */ return []; }; @@ -177,10 +181,6 @@ const create = context => { const moduleName = getStringIfConstant(node.source, context.getScope()); - if (!moduleName) { - return; - } - const allowedImportStyles = styles.get(moduleName); const actualImportStyles = getActualImportDeclarationStyles(node); From 78b73d3c3bd1c46d8b4a918ec8a0a13752ff09cd Mon Sep 17 00:00:00 2001 From: futpib Date: Sat, 4 Jul 2020 00:16:22 +0300 Subject: [PATCH 07/23] Fix docs --- docs/rules/import-style.md | 69 ++++++++++++++++++++++++++++++++++++-- readme.md | 2 +- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/docs/rules/import-style.md b/docs/rules/import-style.md index fbc5661485..731893b925 100644 --- a/docs/rules/import-style.md +++ b/docs/rules/import-style.md @@ -1,6 +1,12 @@ -# Whether to allow default imports or destructuring/named imports +# Enforce specific import styles per module -Sometimes a module contains unrelated functions like `util`, thus it is a good practice to enforce destructuring or named imports here, other times in modules like `path` it is good to do default import as they have similar functions, likely to be utilised. By default `path` and `chalk` are enforced to have default export and `util`, `lodash` and `underscore` are having named export. But you can easily override these properties by passing `false` for respective module. +Sometimes a module contains unrelated functions like `util`, thus it is a good practice to enforce destructuring or named imports here, other times in modules like `path` it is good to do default import as they have similar functions, all likely to be utilised. + +This rule defines 4 import typoes: +* `unassigned` - `import 'foo'` or `require('foo')` +* `default` - `import path from 'path'` or `const path = require('path')` +* `namespace` - `import * as path from 'path'` or `const path = require('path')` +* `named` - `import {inspect} from 'util'` or `const {inspect} = require('util')` ## Fail @@ -20,3 +26,62 @@ const {promisify} = require('util'); import {promisify} from 'util'; ``` + +## Options + +### styles + +Type: `object` + +You can extend default import styles per module by passing the `styles` option. + +Default options per module are: +* `util` - `named` only +* `path` - `default` only +* `chalk` - `default` only + +The example below: +- disables any restrictions on the `utils` module imports +- allows `named` import from the `path` module (by default only `default` import of `path` is allowed) + +```js +"unicorn/import-style": [ + "error", + { + "styles": { + "utils": false, + "path": { + "named": true + } + } + } +] +``` + +### extendDefaultStyles + +Type: `boolean`
+Default: `true` + +Pass `"extendDefaultStyles": false` to override the default `styles` option completely. + +### checkImport + +Type: `boolean`
+Default: `true` + +Pass `"checkImport": false` to disable linting of static import statements (like `import 'foo'`) completely. + +### checkDynamicImport + +Type: `boolean`
+Default: `true` + +Pass `"checkDynamicImport": false` to disable linting of dynamic import statements (like `import('foo')`) completely. + +### checkRequire + +Type: `boolean`
+Default: `true` + +Pass `"checkRequire": false` to disable linting of `require` calls completely. diff --git a/readme.md b/readme.md index ce72a40bf1..974a04387d 100644 --- a/readme.md +++ b/readme.md @@ -107,7 +107,7 @@ Configure it in `package.json`. - [explicit-length-check](docs/rules/explicit-length-check.md) - Enforce explicitly comparing the `length` property of a value. *(partly fixable)* - [filename-case](docs/rules/filename-case.md) - Enforce a case style for filenames. - [import-index](docs/rules/import-index.md) - Enforce importing index files with `.`. *(fixable)* -- [import-style](docs/rules/import-style.md) - Enforce default or named import per module. +- [import-style](docs/rules/import-style.md) - Enforce specific import styles per module. - [new-for-builtins](docs/rules/new-for-builtins.md) - Enforce the use of `new` for all builtins, except `String`, `Number`, `Boolean`, `Symbol` and `BigInt`. *(fixable)* - [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) - Enforce specifying rules to disable in `eslint-disable` comments. - [no-array-instanceof](docs/rules/no-array-instanceof.md) - Require `Array.isArray()` instead of `instanceof Array`. *(fixable)* From 7517336229b6e6801c291dfcfc318cab9f233de9 Mon Sep 17 00:00:00 2001 From: futpib Date: Sat, 4 Jul 2020 00:17:44 +0300 Subject: [PATCH 08/23] Fix docs --- docs/rules/import-style.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/import-style.md b/docs/rules/import-style.md index 731893b925..c47a6c29f7 100644 --- a/docs/rules/import-style.md +++ b/docs/rules/import-style.md @@ -42,7 +42,7 @@ Default options per module are: The example below: - disables any restrictions on the `utils` module imports -- allows `named` import from the `path` module (by default only `default` import of `path` is allowed) +- allows `named` import (leaving `default` allowed too) from the `path` module (by default only `default` import of `path` is allowed) ```js "unicorn/import-style": [ From 98f0beae568a945c3b1dd8400f7a76adf2b1dcf6 Mon Sep 17 00:00:00 2001 From: futpib Date: Sat, 4 Jul 2020 15:36:58 +0300 Subject: [PATCH 09/23] Update import-style.md --- docs/rules/import-style.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/rules/import-style.md b/docs/rules/import-style.md index c47a6c29f7..c096bced10 100644 --- a/docs/rules/import-style.md +++ b/docs/rules/import-style.md @@ -41,7 +41,7 @@ Default options per module are: * `chalk` - `default` only The example below: -- disables any restrictions on the `utils` module imports +- disables any restrictions on the `util` module imports - allows `named` import (leaving `default` allowed too) from the `path` module (by default only `default` import of `path` is allowed) ```js @@ -49,7 +49,7 @@ The example below: "error", { "styles": { - "utils": false, + "util": false, "path": { "named": true } From de84da72afce7e23e6c1f8a85cf18471cbcf6965 Mon Sep 17 00:00:00 2001 From: futpib Date: Mon, 6 Jul 2020 05:25:38 +0300 Subject: [PATCH 10/23] Update import-style.md --- docs/rules/import-style.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rules/import-style.md b/docs/rules/import-style.md index c096bced10..903a22ba1d 100644 --- a/docs/rules/import-style.md +++ b/docs/rules/import-style.md @@ -2,7 +2,7 @@ Sometimes a module contains unrelated functions like `util`, thus it is a good practice to enforce destructuring or named imports here, other times in modules like `path` it is good to do default import as they have similar functions, all likely to be utilised. -This rule defines 4 import typoes: +This rule defines 4 import types: * `unassigned` - `import 'foo'` or `require('foo')` * `default` - `import path from 'path'` or `const path = require('path')` * `namespace` - `import * as path from 'path'` or `const path = require('path')` @@ -70,14 +70,14 @@ Pass `"extendDefaultStyles": false` to override the default `styles` option comp Type: `boolean`
Default: `true` -Pass `"checkImport": false` to disable linting of static import statements (like `import 'foo'`) completely. +Pass `"checkImport": false` to disable linting of static import statements (like `import ... from 'foo'` or `import 'foo'`) completely. ### checkDynamicImport Type: `boolean`
Default: `true` -Pass `"checkDynamicImport": false` to disable linting of dynamic import statements (like `import('foo')`) completely. +Pass `"checkDynamicImport": false` to disable linting of dynamic import statements (like `await import('foo')`) completely. ### checkRequire From 3540802832531e368c1c7a16abaa9b86386e659d Mon Sep 17 00:00:00 2001 From: futpib Date: Mon, 6 Jul 2020 05:35:24 +0300 Subject: [PATCH 11/23] Fix a test being too lax --- test/import-style.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/import-style.js b/test/import-style.js index 3f125953d6..b794d776d5 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -459,7 +459,13 @@ ruleTester.run('import-style', rule, { } } }], - errors: [{}] + errors: [{ + messageId: 'importStyle', + data: { + allowedStyles: 'named, namespace or default', + moduleName: 'no-unassigned' + } + }] } ].map(test => addDefaultOptions(test)) }); From 634081c9e18cdae4b40e2ad3d6e5a06bba2d8261 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Sat, 18 Jul 2020 00:44:36 +0800 Subject: [PATCH 12/23] Update import-style.md --- docs/rules/import-style.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/rules/import-style.md b/docs/rules/import-style.md index 903a22ba1d..42f9563487 100644 --- a/docs/rules/import-style.md +++ b/docs/rules/import-style.md @@ -1,6 +1,6 @@ # Enforce specific import styles per module -Sometimes a module contains unrelated functions like `util`, thus it is a good practice to enforce destructuring or named imports here, other times in modules like `path` it is good to do default import as they have similar functions, all likely to be utilised. +Sometimes a module contains unrelated functions like `util`, thus it is a good practice to enforce destructuring or named imports here, other times in modules like `path` it is good to use default import as they have similar functions, all likely to be utilized. This rule defines 4 import types: * `unassigned` - `import 'foo'` or `require('foo')` @@ -18,7 +18,6 @@ import util from 'util'; import * as util from 'util'; ``` - ## Pass ```js @@ -41,8 +40,8 @@ Default options per module are: * `chalk` - `default` only The example below: -- disables any restrictions on the `util` module imports -- allows `named` import (leaving `default` allowed too) from the `path` module (by default only `default` import of `path` is allowed) +- Disables any restrictions on the `util` module imports. +- Allows `named` import (leaving `default` allowed too) from the `path` module (by default only `default` import of `path` is allowed). ```js "unicorn/import-style": [ @@ -60,28 +59,28 @@ The example below: ### extendDefaultStyles -Type: `boolean`
+Type: `boolean`\ Default: `true` Pass `"extendDefaultStyles": false` to override the default `styles` option completely. ### checkImport -Type: `boolean`
+Type: `boolean`\ Default: `true` Pass `"checkImport": false` to disable linting of static import statements (like `import ... from 'foo'` or `import 'foo'`) completely. ### checkDynamicImport -Type: `boolean`
+Type: `boolean`\ Default: `true` Pass `"checkDynamicImport": false` to disable linting of dynamic import statements (like `await import('foo')`) completely. ### checkRequire -Type: `boolean`
+Type: `boolean`\ Default: `true` Pass `"checkRequire": false` to disable linting of `require` calls completely. From e701f05ae8091879bbe09878411490013879f554 Mon Sep 17 00:00:00 2001 From: futpib Date: Sat, 18 Jul 2020 05:19:40 +0300 Subject: [PATCH 13/23] Address review comments --- rules/import-style.js | 114 ++++++++++++++++++++---------------------- 1 file changed, 55 insertions(+), 59 deletions(-) diff --git a/rules/import-style.js b/rules/import-style.js index 711b37b44d..c87f3ee224 100644 --- a/rules/import-style.js +++ b/rules/import-style.js @@ -173,86 +173,82 @@ const create = context => { }); }; - return templates.visitor({ - ImportDeclaration(node) { - if (!checkImport) { - return; - } + let visitor = {}; - const moduleName = getStringIfConstant(node.source, context.getScope()); + if (checkImport) { + visitor = { + ...visitor, - const allowedImportStyles = styles.get(moduleName); - const actualImportStyles = getActualImportDeclarationStyles(node); + ImportDeclaration(node) { + const moduleName = getStringIfConstant(node.source, context.getScope()); - report(node, moduleName, actualImportStyles, allowedImportStyles); - }, + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = getActualImportDeclarationStyles(node); - 'ExpressionStatement > ImportExpression'(node) { - if (!checkDynamicImport) { - return; + report(node, moduleName, actualImportStyles, allowedImportStyles); } + }; + } - const moduleName = getStringIfConstant(node.source, context.getScope()); - const allowedImportStyles = styles.get(moduleName); - const actualImportStyles = ['unassigned']; - - report(node, moduleName, actualImportStyles, allowedImportStyles); - }, + if (checkDynamicImport) { + visitor = { + ...visitor, - [assignedDynamicImportTemplate](node) { - if (!checkDynamicImport) { - return; - } + 'ExpressionStatement > ImportExpression'(node) { + const moduleName = getStringIfConstant(node.source, context.getScope()); + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = ['unassigned']; - const assignmentTargetNode = assignedDynamicImportTemplate.context.getMatch(assignmentTargetVariable); - const moduleNameNode = assignedDynamicImportTemplate.context.getMatch(moduleNameVariable); - const moduleName = getStringIfConstant(moduleNameNode, context.getScope()); + report(node, moduleName, actualImportStyles, allowedImportStyles); + }, - if (!moduleName) { - return; - } + [assignedDynamicImportTemplate](node) { + const assignmentTargetNode = assignedDynamicImportTemplate.context.getMatch(assignmentTargetVariable); + const moduleNameNode = assignedDynamicImportTemplate.context.getMatch(moduleNameVariable); + const moduleName = getStringIfConstant(moduleNameNode, context.getScope()); - const allowedImportStyles = styles.get(moduleName); - const actualImportStyles = getActualAssignmentTargetImportStyles(assignmentTargetNode); + if (!moduleName) { + return; + } - report(node, moduleName, actualImportStyles, allowedImportStyles); - }, + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = getActualAssignmentTargetImportStyles(assignmentTargetNode); - 'ExpressionStatement > CallExpression[callee.name=\'require\']'(node) { - if (!checkRequire) { - return; + report(node, moduleName, actualImportStyles, allowedImportStyles); } + }; + } - if (node.arguments.length !== 1) { - return; - } + if (checkRequire) { + visitor = { + ...visitor, - const moduleName = getStringIfConstant(node.arguments[0], context.getScope()); - const allowedImportStyles = styles.get(moduleName); - const actualImportStyles = ['unassigned']; + 'ExpressionStatement > CallExpression[callee.name=\'require\'][arguments.length=1]'(node) { + const moduleName = getStringIfConstant(node.arguments[0], context.getScope()); + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = ['unassigned']; - report(node, moduleName, actualImportStyles, allowedImportStyles, true); - }, + report(node, moduleName, actualImportStyles, allowedImportStyles, true); + }, - [assignedRequireTemplate](node) { - if (!checkRequire) { - return; - } + [assignedRequireTemplate](node) { + const assignmentTargetNode = assignedRequireTemplate.context.getMatch(assignmentTargetVariable); + const moduleNameNode = assignedRequireTemplate.context.getMatch(moduleNameVariable); + const moduleName = getStringIfConstant(moduleNameNode, context.getScope()); - const assignmentTargetNode = assignedRequireTemplate.context.getMatch(assignmentTargetVariable); - const moduleNameNode = assignedRequireTemplate.context.getMatch(moduleNameVariable); - const moduleName = getStringIfConstant(moduleNameNode, context.getScope()); + if (!moduleName) { + return; + } - if (!moduleName) { - return; - } + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = getActualAssignmentTargetImportStyles(assignmentTargetNode); - const allowedImportStyles = styles.get(moduleName); - const actualImportStyles = getActualAssignmentTargetImportStyles(assignmentTargetNode); + report(node, moduleName, actualImportStyles, allowedImportStyles, true); + } + }; + } - report(node, moduleName, actualImportStyles, allowedImportStyles, true); - } - }); + return templates.visitor(visitor); }; const schema = [ From 30e37e1c6f80fc9956d2bd3bd34f0d118b36bf77 Mon Sep 17 00:00:00 2001 From: futpib Date: Sat, 18 Jul 2020 23:37:04 +0300 Subject: [PATCH 14/23] Add a test --- test/import-style.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/import-style.js b/test/import-style.js index b794d776d5..2eec5eb1af 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -173,6 +173,18 @@ ruleTester.run('import-style', rule, { }] }, + { + code: 'import util, {inspect} from \'named-or-default\'', + options: [{ + styles: { + 'named-or-default': { + named: true, + default: true + } + } + }] + }, + 'require(1, 2, 3)', 'require(variable)', 'const x = require(variable)', @@ -408,6 +420,15 @@ ruleTester.run('import-style', rule, { errors: [namedError] }, + { + code: 'import util, {inspect} from \'named\'', + errors: [namedError] + }, + { + code: 'import util, {inspect} from \'default\'', + errors: [defaultError] + }, + { code: 'import util from \'util\'', options: [], From 6fe4d3535888afd766b55e53c2cfb9b7ab181a87 Mon Sep 17 00:00:00 2001 From: futpib Date: Sat, 18 Jul 2020 23:57:09 +0300 Subject: [PATCH 15/23] Add `export ... from ...` support --- docs/rules/import-style.md | 7 +++++ rules/import-style.js | 52 ++++++++++++++++++++++++++++++++++++ test/import-style.js | 54 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+) diff --git a/docs/rules/import-style.md b/docs/rules/import-style.md index 42f9563487..eea8dc7e23 100644 --- a/docs/rules/import-style.md +++ b/docs/rules/import-style.md @@ -78,6 +78,13 @@ Default: `true` Pass `"checkDynamicImport": false` to disable linting of dynamic import statements (like `await import('foo')`) completely. +### checkExportFrom + +Type: `boolean`\ +Default: `false` + +Pass `"checkExportFrom": true` to enable linting of export-from statements (like `export ... from 'foo'`). + ### checkRequire Type: `boolean`\ diff --git a/rules/import-style.js b/rules/import-style.js index c87f3ee224..16e271ea06 100644 --- a/rules/import-style.js +++ b/rules/import-style.js @@ -39,6 +39,30 @@ const getActualImportDeclarationStyles = importDeclaration => { return [...styles]; }; +const getActualExportDeclarationStyles = exportDeclaration => { + const {specifiers} = exportDeclaration; + + if (specifiers.length === 0) { + return ['unassigned']; + } + + const styles = new Set(); + + for (const specifier of specifiers) { + if (specifier.type === 'ExportSpecifier') { + if (specifier.exported.type === 'Identifier' && specifier.exported.name === 'default') { + styles.add('default'); + continue; + } + + styles.add('named'); + continue; + } + } + + return [...styles]; +}; + const getActualAssignmentTargetImportStyles = assignmentTarget => { if (assignmentTarget.type === 'Identifier') { return ['namespace']; @@ -124,6 +148,7 @@ const create = context => { extendDefaultStyles = true, checkImport = true, checkDynamicImport = true, + checkExportFrom = false, checkRequire = true } = {} ] = context.options; @@ -219,6 +244,30 @@ const create = context => { }; } + if (checkExportFrom) { + visitor = { + ...visitor, + + ExportAllDeclaration(node) { + const moduleName = getStringIfConstant(node.source, context.getScope()); + + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = ['namespace']; + + report(node, moduleName, actualImportStyles, allowedImportStyles); + }, + + ExportNamedDeclaration(node) { + const moduleName = getStringIfConstant(node.source, context.getScope()); + + const allowedImportStyles = styles.get(moduleName); + const actualImportStyles = getActualExportDeclarationStyles(node); + + report(node, moduleName, actualImportStyles, allowedImportStyles); + } + }; + } + if (checkRequire) { visitor = { ...visitor, @@ -261,6 +310,9 @@ const schema = [ checkDynamicImport: { type: 'boolean' }, + checkExportFrom: { + type: 'boolean' + }, checkRequire: { type: 'boolean' }, diff --git a/test/import-style.js b/test/import-style.js index 2eec5eb1af..014c7474a3 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -11,6 +11,7 @@ const ruleTester = avaRuleTester(test, { }); const options = { + checkExportFrom: true, styles: { unassigned: { unassigned: true @@ -77,6 +78,7 @@ ruleTester.run('import-style', rule, { 'require(\'unassigned\')', 'import \'unassigned\'', 'import(\'unassigned\')', + 'export {} from \'unassigned\'', 'const x = require(\'default\')', 'const {default: x} = require(\'default\')', @@ -86,6 +88,7 @@ ruleTester.run('import-style', rule, { const {default: x} = await import('default'); } `, + 'export {default} from \'default\'', 'const x = require(\'namespace\')', 'import * as x from \'namespace\'', @@ -94,6 +97,7 @@ ruleTester.run('import-style', rule, { const x = await import('namespace'); } `, + 'export * from \'namespace\'', 'const {x} = require(\'named\')', 'const {x: y} = require(\'named\')', @@ -109,6 +113,8 @@ ruleTester.run('import-style', rule, { const {x: y} = await import('named'); } `, + 'export {x} from \'named\'', + 'export {x as y} from \'named\'', { code: 'import {inspect} from \'util\'', @@ -264,6 +270,22 @@ ruleTester.run('import-style', rule, { `, errors: [unassignedError] }, + { + code: 'export * from \'unassigned\'', + errors: [unassignedError] + }, + { + code: 'export {x} from \'unassigned\'', + errors: [unassignedError] + }, + { + code: 'export {x as y} from \'unassigned\'', + errors: [unassignedError] + }, + { + code: 'export {default} from \'unassigned\'', + errors: [unassignedError] + }, { code: 'require(\'default\')', @@ -321,6 +343,18 @@ ruleTester.run('import-style', rule, { `, errors: [defaultError] }, + { + code: 'export * from \'default\'', + errors: [defaultError] + }, + { + code: 'export {x} from \'default\'', + errors: [defaultError] + }, + { + code: 'export {x as y} from \'default\'', + errors: [defaultError] + }, { code: 'require(\'namespace\')', @@ -374,6 +408,18 @@ ruleTester.run('import-style', rule, { `, errors: [namespaceError] }, + { + code: 'export {x} from \'namespace\'', + errors: [namespaceError] + }, + { + code: 'export {x as y} from \'namespace\'', + errors: [namespaceError] + }, + { + code: 'export {default} from \'namespace\'', + errors: [namespaceError] + }, { code: 'require(\'named\')', @@ -419,6 +465,14 @@ ruleTester.run('import-style', rule, { `, errors: [namedError] }, + { + code: 'export * from \'named\'', + errors: [namedError] + }, + { + code: 'export {default} from \'named\'', + errors: [namedError] + }, { code: 'import util, {inspect} from \'named\'', From 4c667479aac7ecfe1a803b7b0210475747a58843 Mon Sep 17 00:00:00 2001 From: futpib Date: Sun, 19 Jul 2020 00:09:47 +0300 Subject: [PATCH 16/23] Add a test --- test/import-style.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/import-style.js b/test/import-style.js index 014c7474a3..8170cbe782 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -136,6 +136,10 @@ ruleTester.run('import-style', rule, { code: 'const {inspect} = require(\'util\')', options: [] }, + { + code: 'export {promisify, callbackify} from \'util\'', + options: [] + }, { code: 'require(\'chalk\')', From c5ede137f28aafc8024af3a851fefcf4e5833409 Mon Sep 17 00:00:00 2001 From: futpib Date: Wed, 22 Jul 2020 03:39:02 +0300 Subject: [PATCH 17/23] Add tests suggested in review --- rules/import-style.js | 4 ++++ test/import-style.js | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/rules/import-style.js b/rules/import-style.js index 16e271ea06..27a893ff8d 100644 --- a/rules/import-style.js +++ b/rules/import-style.js @@ -69,6 +69,10 @@ const getActualAssignmentTargetImportStyles = assignmentTarget => { } if (assignmentTarget.type === 'ObjectPattern') { + if (assignmentTarget.properties.length === 0) { + return ['unassigned']; + } + const styles = new Set(); for (const property of assignmentTarget.properties) { diff --git a/test/import-style.js b/test/import-style.js index 8170cbe782..bc8da3e596 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -76,7 +76,9 @@ const addDefaultOptions = test => { ruleTester.run('import-style', rule, { valid: [ 'require(\'unassigned\')', + 'const {} = require(\'unassigned\')', 'import \'unassigned\'', + 'import {} from \'unassigned\'', 'import(\'unassigned\')', 'export {} from \'unassigned\'', @@ -295,10 +297,18 @@ ruleTester.run('import-style', rule, { code: 'require(\'default\')', errors: [defaultError] }, + { + code: 'const {} = require(\'default\')', + errors: [defaultError] + }, { code: 'import \'default\'', errors: [defaultError] }, + { + code: 'import {} from \'default\'', + errors: [defaultError] + }, { code: 'import(\'default\')', errors: [defaultError] @@ -364,10 +374,18 @@ ruleTester.run('import-style', rule, { code: 'require(\'namespace\')', errors: [namespaceError] }, + { + code: 'const {} = require(\'namespace\')', + errors: [namespaceError] + }, { code: 'import \'namespace\'', errors: [namespaceError] }, + { + code: 'import {} from \'namespace\'', + errors: [namespaceError] + }, { code: 'import(\'namespace\')', errors: [namespaceError] @@ -429,10 +447,18 @@ ruleTester.run('import-style', rule, { code: 'require(\'named\')', errors: [namedError] }, + { + code: 'const {} = require(\'named\')', + errors: [namedError] + }, { code: 'import \'named\'', errors: [namedError] }, + { + code: 'import {} from \'named\'', + errors: [namedError] + }, { code: 'import(\'named\')', errors: [namedError] From ca97a1582226f729acc485ff895a3d86efe5ec7b Mon Sep 17 00:00:00 2001 From: futpib Date: Wed, 22 Jul 2020 03:45:11 +0300 Subject: [PATCH 18/23] Add visualizer tests --- test/import-style.js | 22 +++++++++ test/snapshots/import-style.js.md | 70 ++++++++++++++++++++++++++++ test/snapshots/import-style.js.snap | Bin 0 -> 451 bytes 3 files changed, 92 insertions(+) create mode 100644 test/snapshots/import-style.js.md create mode 100644 test/snapshots/import-style.js.snap diff --git a/test/import-style.js b/test/import-style.js index bc8da3e596..c5bee1eba8 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -2,6 +2,7 @@ import test from 'ava'; import avaRuleTester from 'eslint-ava-rule-tester'; import {outdent} from 'outdent'; import rule from '../rules/import-style'; +import visualizeRuleTester from './utils/visualize-rule-tester'; const ruleTester = avaRuleTester(test, { parserOptions: { @@ -574,3 +575,24 @@ ruleTester.run('import-style', rule, { } ].map(test => addDefaultOptions(test)) }); + +const visualizeTester = visualizeRuleTester(test, { + parserOptions: { + sourceType: 'module', + ecmaVersion: 2020 + } +}); + +visualizeTester.run('consistent-function-scoping', rule, [ + 'import util from \'util\'', + 'import * as util from \'util\'', + 'const util = require(\'util\')', + 'require(\'util\')', + 'import {red} from \'chalk\'', + 'import {red as green} from \'chalk\'', + outdent` + async () => { + const {red} = await import('chalk'); + } + `, +]); diff --git a/test/snapshots/import-style.js.md b/test/snapshots/import-style.js.md new file mode 100644 index 0000000000..e6a0c1454c --- /dev/null +++ b/test/snapshots/import-style.js.md @@ -0,0 +1,70 @@ +# Snapshot report for `test/import-style.js` + +The actual snapshot is saved in `import-style.js.snap`. + +Generated by [AVA](https://avajs.dev). + +## consistent-function-scoping - #1 + +> Snapshot 1 + + `␊ + > 1 | import util from 'util'␊ + | ^^^^^^^^^^^^^^^^^^^^^^^ Use named import for module `util`.␊ + ` + +## consistent-function-scoping - #2 + +> Snapshot 1 + + `␊ + > 1 | import * as util from 'util'␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use named import for module `util`.␊ + ` + +## consistent-function-scoping - #3 + +> Snapshot 1 + + `␊ + > 1 | const util = require('util')␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use named import for module `util`.␊ + ` + +## consistent-function-scoping - #4 + +> Snapshot 1 + + `␊ + > 1 | require('util')␊ + | ^^^^^^^^^^^^^^^ Use named import for module `util`.␊ + ` + +## consistent-function-scoping - #5 + +> Snapshot 1 + + `␊ + > 1 | import {red} from 'chalk'␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^ Use default import for module `chalk`.␊ + ` + +## consistent-function-scoping - #6 + +> Snapshot 1 + + `␊ + > 1 | import {red as green} from 'chalk'␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use default import for module `chalk`.␊ + ` + +## consistent-function-scoping - #7 + +> Snapshot 1 + + `␊ + 1 | async () => {␊ + > 2 | const {red} = await import('chalk');␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use default import for module `chalk`.␊ + 3 | }␊ + ` diff --git a/test/snapshots/import-style.js.snap b/test/snapshots/import-style.js.snap new file mode 100644 index 0000000000000000000000000000000000000000..a2daca5a1f5c48ab687cf2ff392577e2fb217571 GIT binary patch literal 451 zcmV;!0X+UeRzV400Q|y{Kd95ua9+ zS*I+zzTS0VHzQcojER9E>S_G72Zxfgx42ARsQq}Q2_sl^9}qVl+o4>RrL;V3mA{DC zBc5-JV9`uw1_oxJ%RzvVl|hg(k;_iOP@zU4Gq)hWs6?T(Br``LttdZNK^??Y=TZQI z8ihFQKq0g^RUt1iH#G&OKP|sVAvZszG$&Od0c1de9v6yr*{IfODI^w?XeWghC+FuC zL%nIMP?TC&npu>p0kKz;CKd-mEhfq?(v8nR4bbYM)RbCiU?yiI=42BLO|X?IscDI& zIVIRE1)G9yYZa!gpqNZAN=?lp&3cO5!8C^fXgF)r006ji$4>wN literal 0 HcmV?d00001 From b3b8b88ff4d0dd6187d41af1ac403dd11db89a61 Mon Sep 17 00:00:00 2001 From: futpib Date: Wed, 22 Jul 2020 03:49:10 +0300 Subject: [PATCH 19/23] Add a static value test --- test/import-style.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/import-style.js b/test/import-style.js index c5bee1eba8..410d8577c4 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -534,6 +534,11 @@ ruleTester.run('import-style', rule, { options: [], errors: [{}] }, + { + code: 'require(\'ut\' + \'il\')', + options: [], + errors: [{}] + }, { code: 'import {red} from \'chalk\'', options: [], From 0d97a94af39cf2a2f249142612989c7e3bad9347 Mon Sep 17 00:00:00 2001 From: futpib Date: Wed, 22 Jul 2020 03:55:36 +0300 Subject: [PATCH 20/23] Linter fix --- test/import-style.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/import-style.js b/test/import-style.js index 410d8577c4..de4edd6afc 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -599,5 +599,5 @@ visualizeTester.run('consistent-function-scoping', rule, [ async () => { const {red} = await import('chalk'); } - `, + ` ]); From d10e7335ba308fba3f123636f00ea7d8013733b2 Mon Sep 17 00:00:00 2001 From: futpib Date: Mon, 27 Jul 2020 12:13:15 +0300 Subject: [PATCH 21/23] Add a test suggested in review --- test/import-style.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/import-style.js b/test/import-style.js index de4edd6afc..9d739e056a 100644 --- a/test/import-style.js +++ b/test/import-style.js @@ -201,6 +201,7 @@ ruleTester.run('import-style', rule, { 'require(1, 2, 3)', 'require(variable)', 'const x = require(variable)', + 'const x = require(\'unassigned\').x', outdent` async () => { const {red} = await import(variable); From 9c47f0161171310200140361cd844d58d120727d Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Mon, 27 Jul 2020 20:14:18 +0200 Subject: [PATCH 22/23] Update import-style.md --- docs/rules/import-style.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/rules/import-style.md b/docs/rules/import-style.md index eea8dc7e23..a2b236db0a 100644 --- a/docs/rules/import-style.md +++ b/docs/rules/import-style.md @@ -1,12 +1,12 @@ # Enforce specific import styles per module -Sometimes a module contains unrelated functions like `util`, thus it is a good practice to enforce destructuring or named imports here, other times in modules like `path` it is good to use default import as they have similar functions, all likely to be utilized. +Sometimes a module contains unrelated functions, like `util`, thus it is a good practice to enforce destructuring or named imports here. Other times, in modules like `path`, it is good to use default import as they have similar functions, all likely to be utilized. This rule defines 4 import types: -* `unassigned` - `import 'foo'` or `require('foo')` -* `default` - `import path from 'path'` or `const path = require('path')` -* `namespace` - `import * as path from 'path'` or `const path = require('path')` -* `named` - `import {inspect} from 'util'` or `const {inspect} = require('util')` +- `unassigned` - `import 'foo'` or `require('foo')` +- `default` - `import path from 'path'` or `const path = require('path')` +- `namespace` - `import * as path from 'path'` or `const path = require('path')` +- `named` - `import {inspect} from 'util'` or `const {inspect} = require('util')` ## Fail @@ -35,9 +35,9 @@ Type: `object` You can extend default import styles per module by passing the `styles` option. Default options per module are: -* `util` - `named` only -* `path` - `default` only -* `chalk` - `default` only +- `util` - `named` only +- `path` - `default` only +- `chalk` - `default` only The example below: - Disables any restrictions on the `util` module imports. From 8a25f364e48044ad76b6f45ae1655aed784da6e4 Mon Sep 17 00:00:00 2001 From: futpib Date: Thu, 30 Jul 2020 03:04:49 +0300 Subject: [PATCH 23/23] Update import-style.md --- docs/rules/import-style.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/import-style.md b/docs/rules/import-style.md index a2b236db0a..139540c43c 100644 --- a/docs/rules/import-style.md +++ b/docs/rules/import-style.md @@ -2,7 +2,7 @@ Sometimes a module contains unrelated functions, like `util`, thus it is a good practice to enforce destructuring or named imports here. Other times, in modules like `path`, it is good to use default import as they have similar functions, all likely to be utilized. -This rule defines 4 import types: +This rule defines 4 import styles: - `unassigned` - `import 'foo'` or `require('foo')` - `default` - `import path from 'path'` or `const path = require('path')` - `namespace` - `import * as path from 'path'` or `const path = require('path')`