Add rule no-named-as-default-member

Fixes import-js#205.

CHANGELOG.md

@@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 ### Added
 - report resolver errors at the top of the linted file
 - add `no-namespace` rule
+- add `no-named-as-default-member` rule
 
 ## [1.4.0] - 2016-03-25
 ### Added

README.md

@@ -26,8 +26,10 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
 Helpful warnings:
 
 * Report use of exported name as identifier of default export ([`no-named-as-default`])
+* Report use of exported name as property of default export ([`no-named-as-default-member`])
 
 [`no-named-as-default`]: ./docs/rules/no-named-as-default.md
+[`no-named-as-default-member`]: ./docs/rules/no-named-as-default-member.md
 
 Style guide:
 

docs/rules/no-named-as-default-member.md

@@ -0,0 +1,47 @@
+# no-named-as-default-member
+
+Reports use of an exported name as a property on the default export.
+
+Rationale: Accessing a property that has a name that is shared by an exported
+name from the same module is likely to be a mistake.
+
+Named import syntax looks very similar to destructuring assignment. It's easy to
+make the (incorrect) assumption that named exports are also accessible as
+properties of the default export.
+
+Furthermore, [in Babel 5 this is actually how things worked][blog]. This was
+fixed in Babel 6. Before upgrading an existing codebase to Babel 6, it can be
+useful to run this lint rule.
+
+
+[blog]: https://medium.com/@kentcdodds/misunderstanding-es6-modules-upgrading-babel-tears-and-a-solution-ad2d5ab93ce0
+
+
+## Rule Details
+
+Given:
+```js
+// foo.js
+export default 'foo';
+export const bar = 'baz';
+```
+
+...this would be valid:
+```js
+import foo, {bar} from './foo.js';
+```
+
+...and the following would be reported:
+```js
+// Caution: `foo` also has a named export `bar`.
+// Check if you meant to write `import {bar} from './foo.js'` instead.
+import foo from './foo.js';
+const bar = foo.bar;
+```
+
+```js
+// Caution: `foo` also has a named export `bar`.
+// Check if you meant to write `import {bar} from './foo.js'` instead.
+import foo from './foo.js';
+const {bar} = foo;
+```

src/index.js

@@ -7,6 +7,7 @@ export const rules = {
   'export': require('./rules/export'),
 
   'no-named-as-default': require('./rules/no-named-as-default'),
+  'no-named-as-default-member': require('./rules/no-named-as-default-member'),
 
   'no-commonjs': require('./rules/no-commonjs'),
   'no-amd': require('./rules/no-amd'),

src/rules/no-named-as-default-member.js

@@ -0,0 +1,90 @@
+/**
+ * @fileoverview Rule to warn about potentially confused use of name exports
+ * @author Desmond Brand
+ * @copyright 2016 Desmond Brand. All rights reserved.
+ * See LICENSE in root directory for full license.
+ */
+
+import 'es6-symbol/implement'
+import Map from 'es6-map'
+
+import Exports from '../core/getExports'
+import importDeclaration from '../importDeclaration'
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = function(context) {
+
+  const fileImports = new Map()
+  const allPropertyLookups = new Map()
+
+  function handleImportDefault(node) {
+    const declaration = importDeclaration(context)
+    const exportMap = Exports.get(declaration.source.value, context)
+    if (exportMap == null) return
+
+    if (exportMap.errors.length) {
+      exportMap.reportErrors(context, declaration)
+      return
+    }
+
+    fileImports.set(node.local.name, {
+      exportMap,
+      sourcePath: declaration.source.value,
+    })
+  }
+
+  function storePropertyLookup(objectName, propName, node) {
+    const lookups = allPropertyLookups.get(objectName) || []
+    lookups.push({node, propName})
+    allPropertyLookups.set(objectName, lookups)
+  }
+
+  function handlePropLookup(node) {
+    const objectName = node.object.name
+    const propName = node.property.name
+    storePropertyLookup(objectName, propName, node)
+  }
+
+  function handleDestructuringAssignment(node) {
+    const isDestructure = (
+      node.id.type === 'ObjectPattern' && node.init.type === 'Identifier'
+    )
+    if (!isDestructure) return
+
+    const objectName = node.init.name
+    for (const {key} of node.id.properties) {
+      storePropertyLookup(objectName, key.name, key)
+    }
+  }
+
+  function handleProgramExit() {
+    allPropertyLookups.forEach((lookups, objectName) => {
+      const fileImport = fileImports.get(objectName)
+      if (fileImport == null) return
+
+      for (const {propName, node} of lookups) {
+        if (!fileImport.exportMap.namespace.has(propName)) continue
+
+        context.report({
+          node,
+          message: (
+            `Caution: \`${objectName}\` also has a named export ` +
+            `\`${propName}\`. Check if you meant to write ` +
+            `\`import {${propName}} from '${fileImport.sourcePath}'\` ` +
+            'instead.'
+          ),
+        })
+      }
+    })
+  }
+
+  return {
+    'ImportDefaultSpecifier': handleImportDefault,
+    'MemberExpression': handlePropLookup,
+    'VariableDeclarator': handleDestructuringAssignment,
+    'Program:exit': handleProgramExit,
+  }
+}

tests/src/rules/no-named-as-default-member.js

@@ -0,0 +1,57 @@
+import {test} from '../utils'
+import {RuleTester} from 'eslint'
+import rule from 'rules/no-named-as-default-member'
+
+const ruleTester = new RuleTester()
+
+ruleTester.run('no-named-as-default-member', rule, {
+  valid: [
+    test({code: 'import bar, {foo} from "./bar";'}),
+    test({code: 'import bar from "./bar"; const baz = bar.baz'}),
+    test({code: 'import {foo} from "./bar"; const baz = foo.baz;'}),
+    test({code: 'import * as named from "./named-exports"; const a = named.a'}),
+  ],
+
+  invalid: [
+    test({
+      code: 'import bar from "./bar"; const foo = bar.foo;',
+      errors: [{
+        message: (
+          'Caution: `bar` also has a named export `foo`. ' +
+          'Check if you meant to write `import {foo} from \'./bar\'` instead.'
+        ),
+        type: 'MemberExpression',
+      }],
+    }),
+    test({
+      code: 'import bar from "./bar"; bar.foo();',
+      errors: [{
+        message: (
+          'Caution: `bar` also has a named export `foo`. ' +
+          'Check if you meant to write `import {foo} from \'./bar\'` instead.'
+        ),
+        type: 'MemberExpression',
+      }],
+    }),
+    test({
+      code: 'import bar from "./bar"; const {foo} = bar;',
+      errors: [{
+        message: (
+          'Caution: `bar` also has a named export `foo`. ' +
+          'Check if you meant to write `import {foo} from \'./bar\'` instead.'
+        ),
+        type: 'Identifier',
+      }],
+    }),
+    test({
+      code: 'import bar from "./bar"; const {foo: foo2, baz} = bar;',
+      errors: [{
+        message: (
+          'Caution: `bar` also has a named export `foo`. ' +
+          'Check if you meant to write `import {foo} from \'./bar\'` instead.'
+        ),
+        type: 'Identifier',
+      }],
+    }),
+  ],
+})