Merge d030d8e into 3704801

CHANGELOG.md

@@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ### Added
 - [`group-exports`]: make aggregate module exports valid ([#1472], thanks [@atikenny])
+- [`no-namespace`]: Make rule fixable ([#1401], thanks [@TrevorBurnham])
 
 ### Added
 - support `parseForESLint` from custom parser ([#1435], thanks [@JounQin])
@@ -617,6 +618,7 @@ for info on changes for earlier releases.
 [#1412]: https://github.com/benmosher/eslint-plugin-import/pull/1412
 [#1409]: https://github.com/benmosher/eslint-plugin-import/pull/1409
 [#1404]: https://github.com/benmosher/eslint-plugin-import/pull/1404
+[#1401]: https://github.com/benmosher/eslint-plugin-import/pull/1401
 [#1393]: https://github.com/benmosher/eslint-plugin-import/pull/1393
 [#1389]: https://github.com/benmosher/eslint-plugin-import/pull/1389
 [#1377]: https://github.com/benmosher/eslint-plugin-import/pull/1377
@@ -989,3 +991,4 @@ for info on changes for earlier releases.
 [@JounQin]: https://github.com/JounQin
 [@atikenny]: https://github.com/atikenny
 [@schmidsi]: https://github.com/schmidsi
+[@TrevorBurnham]: https://github.com/TrevorBurnham

README.md

@@ -81,7 +81,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
 * Ensure all imports appear before other statements ([`first`])
 * Ensure all exports appear after other statements ([`exports-last`])
 * Report repeated import of the same module in multiple places ([`no-duplicates`])
-* Report namespace imports ([`no-namespace`])
+* Forbid namespace (a.k.a. "wildcard" `*`) imports ([`no-namespace`])
 * Ensure consistent use of file extension within the import path ([`extensions`])
 * Enforce a convention in module import order ([`order`])
 * Enforce a newline after import statements ([`newline-after-import`])

docs/rules/no-namespace.md

@@ -1,6 +1,9 @@
 # import/no-namespace
 
-Reports if namespace import is used.
+Enforce a convention of not using namespace (a.k.a. "wildcard" `*`) imports.
+
++(fixable) The `--fix` option on the [command line] automatically fixes problems reported by this rule, provided that the namespace object is only used for direct member access, e.g. `namespace.a`.
+The `--fix` functionality for this rule requires ESLint 5 or newer.
 
 ## Rule Details
 
@@ -12,10 +15,13 @@ import { a, b }  from './bar'
 import defaultExport, { a, b }  from './foobar'
 ```
 
-...whereas here imports will be reported:
+Invalid:
 
 ```js
 import * as foo from 'foo';
+```
+
+```js
 import defaultExport, * as foo from 'foo';
 ```
 

src/rules/no-namespace.js

@@ -16,13 +16,143 @@ module.exports = {
     docs: {
       url: docsUrl('no-namespace'),
     },
+    fixable: 'code',
   },
 
   create: function (context) {
     return {
       'ImportNamespaceSpecifier': function (node) {
-        context.report(node, `Unexpected namespace import.`)
+        const scopeVariables = context.getScope().variables
+        const namespaceVariable = scopeVariables.find((variable) =>
+          variable.defs[0].node === node
+        )
+        const namespaceReferences = namespaceVariable.references
+        const namespaceIdentifiers = namespaceReferences.map(reference => reference.identifier)
+        const canFix = namespaceIdentifiers.length > 0 && !usesNamespaceAsObject(namespaceIdentifiers)
+
+        context.report({
+          node,
+          message: `Unexpected namespace import.`,
+          fix: canFix && (fixer => {
+            const scopeManager = context.getSourceCode().scopeManager
+            const fixes = []
+
+            // Pass 1: Collect variable names that are already in scope for each reference we want
+            // to transform, so that we can be sure that we choose non-conflicting import names
+            const importNameConflicts = {}
+            namespaceIdentifiers.forEach((identifier) => {
+              const parent = identifier.parent
+              if (parent && parent.type === 'MemberExpression') {
+                const importName = getMemberPropertyName(parent)
+                const localConflicts = getVariableNamesInScope(scopeManager, parent)
+                if (!importNameConflicts[importName]) {
+                  importNameConflicts[importName] = localConflicts
+                } else {
+                  localConflicts.forEach((c) => importNameConflicts[importName].add(c))
+                }
+              }
+            })
+
+            // Choose new names for each import
+            const importNames = Object.keys(importNameConflicts)
+            const importLocalNames = generateLocalNames(
+              importNames,
+              importNameConflicts,
+              namespaceVariable.name
+            )
+
+            // Replace the ImportNamespaceSpecifier with a list of ImportSpecifiers
+            const namedImportSpecifiers = importNames.map((importName) =>
+              importName === importLocalNames[importName]
+                ? importName
+                : `${importName} as ${importLocalNames[importName]}`
+            )
+            fixes.push(fixer.replaceText(node, `{ ${namedImportSpecifiers.join(', ')} }`))
+
+            // Pass 2: Replace references to the namespace with references to the named imports
+            namespaceIdentifiers.forEach((identifier) => {
+              const parent = identifier.parent
+              if (parent && parent.type === 'MemberExpression') {
+                const importName = getMemberPropertyName(parent)
+                fixes.push(fixer.replaceText(parent, importLocalNames[importName]))
+              }
+            })
+
+            return fixes
+          }),
+        })
       },
     }
   },
 }
+
+/**
+ * @param {Identifier[]} namespaceIdentifiers
+ * @returns {boolean} `true` if the namespace variable is more than just a glorified constant
+ */
+function usesNamespaceAsObject(namespaceIdentifiers) {
+  return !namespaceIdentifiers.every((identifier) => {
+    const parent = identifier.parent
+
+    // `namespace.x` or `namespace['x']`
+    return (
+      parent && parent.type === 'MemberExpression' &&
+      (parent.property.type === 'Identifier' || parent.property.type === 'Literal')
+    )
+  })
+}
+
+/**
+ * @param {MemberExpression} memberExpression
+ * @returns {string} the name of the member in the object expression, e.g. the `x` in `namespace.x`
+ */
+function getMemberPropertyName(memberExpression) {
+  return memberExpression.property.type === 'Identifier'
+    ? memberExpression.property.name
+    : memberExpression.property.value
+}
+
+/**
+ * @param {ScopeManager} scopeManager
+ * @param {ASTNode} node
+ * @return {Set<string>}
+ */
+function getVariableNamesInScope(scopeManager, node) {
+  let currentNode = node
+  let scope = scopeManager.acquire(currentNode)
+  while (scope == null) {
+    currentNode = currentNode.parent
+    scope = scopeManager.acquire(currentNode, true)
+  }
+  return new Set([
+    ...scope.variables.map(variable => variable.name),
+    ...scope.upper.variables.map(variable => variable.name),
+  ])
+}
+
+/**
+ *
+ * @param {*} names
+ * @param {*} nameConflicts
+ * @param {*} namespaceName
+ */
+function generateLocalNames(names, nameConflicts, namespaceName) {
+  const localNames = {}
+  names.forEach((name) => {
+    let localName
+    if (!nameConflicts[name].has(name)) {
+      localName = name
+    } else if (!nameConflicts[name].has(`${namespaceName}_${name}`)) {
+      localName = `${namespaceName}_${name}`
+    } else {
+      for (let i = 1; i < Infinity; i++) {
+        if (!nameConflicts[name].has(`${namespaceName}_${name}_${i}`)) {
+          localName = `${namespaceName}_${name}_${i}`
+          break
+        }
+      }
+    }
+    localNames[name] = localName
+  })
+  return localNames
+}

tests/src/rules/no-namespace.js

@@ -1,44 +1,113 @@
 import { RuleTester } from 'eslint'
+import eslintPkg from 'eslint/package.json'
+import semver from 'semver'
+import { test } from '../utils'
 
 const ERROR_MESSAGE = 'Unexpected namespace import.'
 
 const ruleTester = new RuleTester()
 
+// --fix functionality requires ESLint 5+
+const FIX_TESTS = semver.satisfies(eslintPkg.version, '>5.0.0') ? [
+  test({
+    code: `
+      import * as foo from './foo';
+      florp(foo.bar);
+      florp(foo['baz']);
+    `.trim(),
+    output: `
+      import { bar, baz } from './foo';
+      florp(bar);
+      florp(baz);
+    `.trim(),
+    errors: [ {
+      line: 1,
+      column: 8,
+      message: ERROR_MESSAGE,
+    }],
+  }),
+  test({
+    code: `
+      import * as foo from './foo';
+      const bar = 'name conflict';
+      const baz = 'name conflict';
+      const foo_baz = 'name conflict';
+      florp(foo.bar);
+      florp(foo['baz']);
+    `.trim(),
+    output: `
+      import { bar as foo_bar, baz as foo_baz_1 } from './foo';
+      const bar = 'name conflict';
+      const baz = 'name conflict';
+      const foo_baz = 'name conflict';
+      florp(foo_bar);
+      florp(foo_baz_1);
+    `.trim(),
+    errors: [ {
+      line: 1,
+      column: 8,
+      message: ERROR_MESSAGE,
+    }],
+  }),
+  test({
+    code: `
+      import * as foo from './foo';
+      function func(arg) {
+        florp(foo.func);
+        florp(foo['arg']);
+      }
+    `.trim(),
+    output: `
+      import { func as foo_func, arg as foo_arg } from './foo';
+      function func(arg) {
+        florp(foo_func);
+        florp(foo_arg);
+      }
+    `.trim(),
+    errors: [ {
+      line: 1,
+      column: 8,
+      message: ERROR_MESSAGE,
+    }],
+  }),
+] : []
+
 ruleTester.run('no-namespace', require('rules/no-namespace'), {
   valid: [
-    { code: "import { a, b } from 'foo';", parserOptions: { ecmaVersion: 2015, sourceType: 'module' } },
-    { code: "import { a, b } from './foo';", parserOptions: { ecmaVersion: 2015, sourceType: 'module' } },
-    { code: "import bar from 'bar';", parserOptions: { ecmaVersion: 2015, sourceType: 'module' } },
-    { code: "import bar from './bar';", parserOptions: { ecmaVersion: 2015, sourceType: 'module' } },
+    { code: 'import { a, b } from \'foo\';', parserOptions: { ecmaVersion: 2015, sourceType: 'module' } },
+    { code: 'import { a, b } from \'./foo\';', parserOptions: { ecmaVersion: 2015, sourceType: 'module' } },
+    { code: 'import bar from \'bar\';', parserOptions: { ecmaVersion: 2015, sourceType: 'module' } },
+    { code: 'import bar from \'./bar\';', parserOptions: { ecmaVersion: 2015, sourceType: 'module' } },
   ],
 
   invalid: [
-    {
-      code: "import * as foo from 'foo';",
+    test({
+      code: 'import * as foo from \'foo\';',
+      output: 'import * as foo from \'foo\';',
       errors: [ {
         line: 1,
         column: 8,
         message: ERROR_MESSAGE,
       } ],
-      parserOptions: { ecmaVersion: 2015, sourceType: 'module' },
-    },
-    {
-      code: "import defaultExport, * as foo from 'foo';",
+    }),
+    test({
+      code: 'import defaultExport, * as foo from \'foo\';',
+      output: 'import defaultExport, * as foo from \'foo\';',
       errors: [ {
         line: 1,
         column: 23,
         message: ERROR_MESSAGE,
       } ],
-      parserOptions: { ecmaVersion: 2015, sourceType: 'module' },
-    },
-    {
-      code: "import * as foo from './foo';",
+    }),
+    test({
+      code: 'import * as foo from \'./foo\';',
+      output: 'import * as foo from \'./foo\';',
       errors: [ {
         line: 1,
         column: 8,
         message: ERROR_MESSAGE,
       } ],
-      parserOptions: { ecmaVersion: 2015, sourceType: 'module' },
-    },
+    }),
+    ...FIX_TESTS,
   ],
 })