Merge pull request #351 from benmosher/fix-prefer-default

Fix prefer default (#342, #343, #350, #351) #MostPRsEver

Author: benmosher

CHANGELOG.md

@@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 - Make [`order`]'s `newline-between` option handle multiline import statements ([#313], thanks [@singles])
 - Make [`order`]'s `newline-between` option handle not assigned import statements ([#313], thanks [@singles])
 - Make [`order`]'s `newline-between` option ignore `require` statements inside object literals ([#313], thanks [@singles])
+- [`prefer-default-export`] properly handles deep destructuring, `export * from ...`, and files with no exports. ([#342]+[#343], thanks [@scottnonnenberg])
 
 ## [1.8.0] - 2016-05-11
 ### Added
@@ -224,6 +225,7 @@ for info on changes for earlier releases.
 [`no-mutable-exports`]: ./docs/rules/no-mutable-exports.md
 [`prefer-default-export`]: ./docs/rules/prefer-default-export.md
 
+[#343]: https://github.com/benmosher/eslint-plugin-import/pull/343
 [#332]: https://github.com/benmosher/eslint-plugin-import/pull/332
 [#322]: https://github.com/benmosher/eslint-plugin-import/pull/322
 [#316]: https://github.com/benmosher/eslint-plugin-import/pull/316
@@ -250,6 +252,7 @@ for info on changes for earlier releases.
 [#164]: https://github.com/benmosher/eslint-plugin-import/pull/164
 [#157]: https://github.com/benmosher/eslint-plugin-import/pull/157
 
+[#342]: https://github.com/benmosher/eslint-plugin-import/issues/342
 [#328]: https://github.com/benmosher/eslint-plugin-import/issues/328
 [#317]: https://github.com/benmosher/eslint-plugin-import/issues/317
 [#286]: https://github.com/benmosher/eslint-plugin-import/issues/286
@@ -307,3 +310,4 @@ for info on changes for earlier releases.
 [@borisyankov]: https://github.com/borisyankov
 [@gavriguy]: https://github.com/gavriguy
 [@jkimbo]: https://github.com/jkimbo
+[@scottnonnenberg]: https://github.com/scottnonnenberg

docs/rules/prefer-default-export.md

@@ -49,3 +49,10 @@ export { foo, bar }
 const foo = 'foo';
 export { foo as default }
 ```
+
+```javascript
+// export-star.js
+
+// Any batch export will disable this rule. The remote module is not inspected.
+export * from './other-module'
+```

src/rules/newline-after-import.js

@@ -96,7 +96,9 @@ module.exports = function (context) {
             return
           }
 
-          if (nextStatement && (!nextRequireCall || !containsNodeOrEqual(nextStatement, nextRequireCall))) {
+          if (nextStatement &&
+             (!nextRequireCall || !containsNodeOrEqual(nextStatement, nextRequireCall))) {
+
             checkForNewLine(statementWithRequireCall, nextStatement, 'require')
           }
         })

src/rules/prefer-default-export.js

@@ -1,10 +1,11 @@
 'use strict'
 
 module.exports = function(context) {
-  let namedExportCount = 0
   let specifierExportCount = 0
   let hasDefaultExport = false
+  let hasStarExport = false
   let namedExportNode = null
+
   return {
     'ExportSpecifier': function(node) {
       if (node.exported.name === 'default') {
@@ -14,16 +15,43 @@ module.exports = function(context) {
         namedExportNode = node
       }
     },
+
     'ExportNamedDeclaration': function(node) {
-      namedExportCount++
+      // if there are specifiers, node.declaration should be null
+      if (!node.declaration) return
+
+      function captureDeclaration(identifierOrPattern) {
+        if (identifierOrPattern.type === 'ObjectPattern') {
+          // recursively capture
+          identifierOrPattern.properties
+            .forEach(function(property) {
+              captureDeclaration(property.value)
+            })
+        } else {
+        // assume it's a single standard identifier
+          specifierExportCount++
+        }
+      }
+
+      if (node.declaration.declarations) {
+        node.declaration.declarations.forEach(function(declaration) {
+          captureDeclaration(declaration.id)
+        })
+      }
+
       namedExportNode = node
     },
+
     'ExportDefaultDeclaration': function() {
       hasDefaultExport = true
     },
 
+    'ExportAllDeclaration': function() {
+      hasStarExport = true
+    },
+
     'Program:exit': function() {
-      if (namedExportCount === 1 &&  specifierExportCount < 2 && !hasDefaultExport) {
+      if (specifierExportCount === 1 && !hasDefaultExport && !hasStarExport) {
         context.report(namedExportNode, 'Prefer default export.')
       }
     },

tests/src/rules/prefer-default-export.js

@@ -21,10 +21,39 @@ ruleTester.run('prefer-default-export', rule, {
       code: `
         export { foo, bar }`,
       }),
+    test({
+      code: `
+        export const { foo, bar } = item;`,
+      }),
+    test({
+      code: `
+        export const { foo, bar: baz } = item;`,
+      }),
+    test({
+      code: `
+        export const { foo: { bar, baz } } = item;`,
+      }),
+    test({
+      code: `
+        export const foo = item;
+        export { item };`,
+      }),
     test({
       code: `
         export { foo as default }`,
       }),
+    test({
+      code: `
+        export * from './foo';`,
+      }),
+
+    // no exports at all
+    test({
+      code: `
+        import * as foo from './foo';`,
+      }),
+
+    // ...SYNTAX_CASES,
   ],
   invalid: [
     test({
@@ -44,5 +73,21 @@ ruleTester.run('prefer-default-export', rule, {
         message: 'Prefer default export.',
       }],
     }),
+    test({
+      code: `
+        export const { foo } = { foo: "bar" };`,
+      errors: [{
+        ruleId: 'ExportNamedDeclaration',
+        message: 'Prefer default export.',
+      }],
+    }),
+    test({
+      code: `
+        export const { foo: { bar } } = { foo: { bar: "baz" } };`,
+      errors: [{
+        ruleId: 'ExportNamedDeclaration',
+        message: 'Prefer default export.',
+      }],
+    }),
   ],
 })