Merge branch 'master' into no-unused-modules-dynamic-imports

Author: maxkomarychev

CHANGELOG.md

@@ -5,6 +5,11 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).
 
 ## [Unreleased]
+### Fixed
+- [`order`]: fix `isExternalModule` detect on windows ([#1651], thanks [@fisker])
+- [`order`]: recognize ".." as a "parent" path ([#1658], thanks [@golopot])
+- [`no-duplicates`]: fix fixer on cases with default import ([#1666], thanks [@golopot])
+- [`no-unused-modules`]: Handle `export { default } from` syntax ([#1631], thanks [@richardxia])
 
 ## [2.20.1] - 2020-02-01
 ### Fixed
@@ -652,7 +657,11 @@ for info on changes for earlier releases.
 
 [`memo-parser`]: ./memo-parser/README.md
 
+[#1666]: https://github.com/benmosher/eslint-plugin-import/pull/1666
+[#1658]: https://github.com/benmosher/eslint-plugin-import/pull/1658
+[#1651]: https://github.com/benmosher/eslint-plugin-import/pull/1651
 [#1635]: https://github.com/benmosher/eslint-plugin-import/issues/1635
+[#1631]: https://github.com/benmosher/eslint-plugin-import/issues/1631
 [#1625]: https://github.com/benmosher/eslint-plugin-import/pull/1625
 [#1620]: https://github.com/benmosher/eslint-plugin-import/pull/1620
 [#1619]: https://github.com/benmosher/eslint-plugin-import/pull/1619
@@ -1105,3 +1114,5 @@ for info on changes for earlier releases.
 [@kentcdodds]: https://github.com/kentcdodds
 [@IvanGoncharov]: https://github.com/IvanGoncharov
 [@wschurman]: https://github.com/wschurman
+[@fisker]: https://github.com/fisker
+[@richardxia]: https://github.com/richardxia

appveyor.yml

@@ -8,7 +8,7 @@ environment:
   # - nodejs_version: "4"
 
 matrix:
-  fast_finish: true
+  fast_finish: false
 
   # allow_failures:
   #   - nodejs_version: "4" # for eslint 5
@@ -27,6 +27,9 @@ install:
       if ($env:nodejs_version -eq "4") {
         npm install -g npm@3;
       }
+      if ($env:nodejs_version -in @("8", "10", "12")) {
+        npm install -g npm@6.10.3;
+      }
   - npm install
 
   # fix symlinks

resolvers/node/CHANGELOG.md

@@ -4,10 +4,12 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).
 
 ## Unreleased
+### Added
+- add `.node` extension ([#1663])
 
 ## v0.3.2 - 2018-01-05
 ### Added
-- `.mjs` extension detected by default to support `experimental-modules` (#939)
+- `.mjs` extension detected by default to support `experimental-modules` ([#939])
 
 ### Deps
 - update `debug`, `resolve`
@@ -42,6 +44,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 [#438]: https://github.com/benmosher/eslint-plugin-import/pull/438
 
+[#1663]: https://github.com/benmosher/eslint-plugin-import/issues/1663
 [#939]: https://github.com/benmosher/eslint-plugin-import/issues/939
 [#531]: https://github.com/benmosher/eslint-plugin-import/issues/531
 [#437]: https://github.com/benmosher/eslint-plugin-import/issues/437

resolvers/node/index.js

@@ -28,7 +28,7 @@ function opts(file, config) {
   return Object.assign({
       // more closely matches Node (#333)
       // plus 'mjs' for native modules! (#939)
-      extensions: ['.mjs', '.js', '.json'],
+      extensions: ['.mjs', '.js', '.json', '.node'],
     },
     config,
     {

resolvers/node/test/dot-node.node

@@ -40,10 +40,21 @@ describe("default options", function () {
       .equal(path.resolve(__dirname, './native.mjs'))
   })
 
+  it("finds .node modules, with lowest precedence", function () {
+    expect(node.resolve('./native.node', './test/file.js'))
+      .to.have.property('path')
+      .equal(path.resolve(__dirname, './native.node'))
+  })
+
+  it("finds .node modules", function () {
+    expect(node.resolve('./dot-node', './test/file.js'))
+      .to.have.property('path')
+      .equal(path.resolve(__dirname, './dot-node.node'))
+  })
+
   it("still finds .js if explicit", function () {
     expect(node.resolve('./native.js', './test/file.js'))
       .to.have.property('path')
       .equal(path.resolve(__dirname, './native.js'))
   })
-
 })

resolvers/node/test/native.node

@@ -29,15 +29,16 @@ function isExternalPath(path, name, settings) {
 }
 
 function isSubpath(subpath, path) {
-  const normSubpath = subpath.replace(/[/]$/, '')
+  const normPath = path.replace(/\\/g, '/')
+  const normSubpath = subpath.replace(/\\/g, '/').replace(/\/$/, '')
   if (normSubpath.length === 0) {
     return false
   }
-  const left = path.indexOf(normSubpath)
+  const left = normPath.indexOf(normSubpath)
   const right = left + normSubpath.length
   return left !== -1 &&
-        (left === 0 || normSubpath[0] !== '/' && path[left - 1] === '/') &&
-        (right >= path.length || path[right] === '/')
+        (left === 0 || normSubpath[0] !== '/' && normPath[left - 1] === '/') &&
+        (right >= normPath.length || normPath[right] === '/')
 }
 
 const externalModuleRegExp = /^\w/
@@ -67,7 +68,7 @@ function isInternalModule(name, settings, path) {
 }
 
 function isRelativeToParent(name) {
-  return /^\.\.[\\/]/.test(name)
+  return/^\.\.$|^\.\.[\\/]/.test(name)
 }
 
 const indexFiles = ['.', './', './index', './index.js']

resolvers/node/test/paths.js

@@ -136,8 +136,13 @@ function getFix(first, rest, sourceCode) {
         fixes.push(fixer.insertTextBefore(closeBrace, specifiersText))
       }
     } else if (!shouldAddDefault && openBrace == null && shouldAddSpecifiers) {
-      // `import './foo'` → `import {...} from './foo'`
-      fixes.push(fixer.insertTextAfter(firstToken, ` {${specifiersText}} from`))
+      if (first.specifiers.length === 0) {
+        // `import './foo'` → `import {...} from './foo'`
+        fixes.push(fixer.insertTextAfter(firstToken, ` {${specifiersText}} from`))
+      } else {
+        // `import def from './foo'` → `import def, {...} from './foo'`
+        fixes.push(fixer.insertTextAfter(first.specifiers[0], `, {${specifiersText}}`))
+      }
     } else if (!shouldAddDefault && openBrace != null && closeBrace != null) {
       // `import {...} './foo'` → `import {..., ...} from './foo'`
       fixes.push(fixer.insertTextBefore(closeBrace, specifiersText))

src/core/importType.js

@@ -66,8 +66,54 @@ const CLASS_DECLARATION = 'ClassDeclaration'
 const DEFAULT = 'default'
 const TYPE_ALIAS = 'TypeAlias'
 
+/**
+ * List of imports per file.
+ *
+ * Represented by a two-level Map to a Set of identifiers. The upper-level Map
+ * keys are the paths to the modules containing the imports, while the
+ * lower-level Map keys are the paths to the files which are being imported
+ * from. Lastly, the Set of identifiers contains either names being imported
+ * or a special AST node name listed above (e.g ImportDefaultSpecifier).
+ *
+ * For example, if we have a file named foo.js containing:
+ *
+ *   import { o2 } from './bar.js';
+ *
+ * Then we will have a structure that looks like:
+ *
+ *   Map { 'foo.js' => Map { 'bar.js' => Set { 'o2' } } }
+ *
+ * @type {Map<string, Map<string, Set<string>>>}
+ */
 const importList = new Map()
+
+/**
+ * List of exports per file.
+ *
+ * Represented by a two-level Map to an object of metadata. The upper-level Map
+ * keys are the paths to the modules containing the exports, while the
+ * lower-level Map keys are the specific identifiers or special AST node names
+ * being exported. The leaf-level metadata object at the moment only contains a
+ * `whereUsed` propoerty, which contains a Set of paths to modules that import
+ * the name.
+ *
+ * For example, if we have a file named bar.js containing the following exports:
+ *
+ *   const o2 = 'bar';
+ *   export { o2 };
+ *
+ * And a file named foo.js containing the following import:
+ *
+ *   import { o2 } from './bar.js';
+ *
+ * Then we will have a structure that looks like:
+ *
+ *   Map { 'bar.js' => Map { 'o2' => { whereUsed: Set { 'foo.js' } } } }
+ *
+ * @type {Map<string, Map<string, object>>}
+ */
 const exportList = new Map()
+
 const ignoredFiles = new Set()
 const filesOutsideSrc = new Set()
 
@@ -453,9 +499,12 @@ module.exports = {
         }
       }
 
-      const exportStatement = exports.get(exportedValue)
+      // exportsList will always map any imported value of 'default' to 'ImportDefaultSpecifier'
+      const exportsKey = exportedValue === DEFAULT ? IMPORT_DEFAULT_SPECIFIER : exportedValue
+
+      const exportStatement = exports.get(exportsKey)
 
-      const value = exportedValue === IMPORT_DEFAULT_SPECIFIER ? DEFAULT : exportedValue
+      const value = exportsKey === IMPORT_DEFAULT_SPECIFIER ? DEFAULT : exportsKey
 
       if (typeof exportStatement !== 'undefined'){
         if (exportStatement.whereUsed.size < 1) {

src/rules/no-duplicates.js

@@ -11,3 +11,4 @@ import {q} from './file-q'
 export * from './file-n'
 export { default, o0, o3 } from './file-o'
 export { p } from './file-p'
+import s from './file-s'

src/rules/no-unused-modules.js

@@ -0,0 +1 @@
+export { default } from './file-o'

tests/files/no-unused-modules/file-0.js

@@ -1,7 +1,7 @@
 import { expect } from 'chai'
 import * as path from 'path'
 
-import importType from 'core/importType'
+import importType, {isExternalModule} from 'core/importType'
 
 import { testContext, testFilePath } from '../utils'
 
@@ -180,6 +180,12 @@ describe('importType(name)', function () {
   })
 
   it('returns "external" for a scoped module from a symlinked directory which partial path is contained in "external-module-folders" (webpack resolver)', function() {
+    const originalFoldersContext = testContext({
+      'import/resolver': 'webpack',
+      'import/external-module-folders': [],
+    })
+    expect(importType('@test-scope/some-module', originalFoldersContext)).to.equal('internal')
+
     const foldersContext = testContext({
       'import/resolver': 'webpack',
       'import/external-module-folders': ['files/symlinked-module'],
@@ -224,4 +230,11 @@ describe('importType(name)', function () {
     })
     expect(importType('@test-scope/some-module', foldersContext)).to.equal('external')
   })
+
+  it('`isExternalModule` works with windows directory separator', function() {
+    expect(isExternalModule('foo', {}, 'E:\\path\\to\\node_modules\\foo')).to.equal(true)
+    expect(isExternalModule('foo', {
+      'import/external-module-folders': ['E:\\path\\to\\node_modules'],
+    }, 'E:\\path\\to\\node_modules\\foo')).to.equal(true)
+  })
 })

tests/files/no-unused-modules/file-s.js

@@ -168,6 +168,12 @@ ruleTester.run('no-duplicates', rule, {
       errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
     }),
 
+    test({
+      code: "import def from './foo'; import {x} from './foo'",
+      output: "import def, {x} from './foo'; ",
+      errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
+    }),
+
     test({
       code: "import {x} from './foo'; import def from './foo'",
       output: "import def, {x} from './foo'; ",

tests/src/core/importType.js

@@ -117,7 +117,8 @@ ruleTester.run('no-unused-modules', rule, {
            import * as l from './file-l'
            export * from './file-n'
            export { default, o0, o3 } from './file-o'
-           export { p } from './file-p'`,
+           export { p } from './file-p'
+           import s from './file-s'`,
            filename: testFilePath('./no-unused-modules/file-0.js'),
            errors: [
              error(`exported declaration 'default' not used within other modules`),
@@ -206,7 +207,11 @@ typescriptRuleTester.run('no-unused-modules', rule, {
 
 // // test for export from
 ruleTester.run('no-unused-modules', rule, {
-  valid: [],
+  valid: [
+    test({ options: unusedExportsOptions,
+           code: `export { default } from './file-o'`,
+           filename: testFilePath('./no-unused-modules/file-s.js')}),
+  ],
   invalid: [
     test({ options: unusedExportsOptions,
            code: `export { k } from '${testFilePath('./no-unused-modules/file-k.js')}'`,

tests/src/rules/no-duplicates.js

@@ -19,6 +19,7 @@ ruleTester.run('order', rule, {
         var relParent1 = require('../foo');
         var relParent2 = require('../foo/bar');
         var relParent3 = require('../');
+        var relParent4 = require('..');
         var sibling = require('./foo');
         var index = require('./');`,
       }),
@@ -196,7 +197,13 @@ ruleTester.run('order', rule, {
         import { Input } from '-/components/Input';
         import { Button } from '-/components/Button';
 
-        import { add } from './helper';`,
+        import p from '..';
+        import q from '../';
+
+        import { add } from './helper';
+
+        import i from '.';
+        import j from './';`,
       options: [
         {
           'newlines-between': 'always',
@@ -2002,6 +2009,25 @@ ruleTester.run('order', rule, {
         message: '`foo` import should occur before import of `Bar`',
       }],
     }),
+    // Alphabetize with parent paths
+    test({
+      code: `
+        import a from '../a';
+        import p from '..';
+      `,
+      output: `
+        import p from '..';
+        import a from '../a';
+      `,
+      options: [{
+        groups: ['external', 'index'],
+        alphabetize: {order: 'asc'},
+      }],
+      errors: [{
+        ruleID: 'order',
+        message: '`..` import should occur before import of `../a`',
+      }],
+    }),
     // Alphabetize with require
     test({
       code: `

tests/src/rules/no-unused-modules.js

@@ -5,6 +5,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ## Unreleased
 
+### Added
+[New] Print more helpful info if parsing fails ([#1671], thanks [@kaiyoma])
+
 ## v2.5.2 - 2020-01-12
 
 ### Fixed
@@ -70,6 +73,7 @@ Yanked due to critical issue with cache key resulting from #839.
 ### Fixed
 - `unambiguous.test()` regex is now properly in multiline mode
 
+[#1671]: https://github.com/benmosher/eslint-plugin-import/pull/1671
 [#1606]: https://github.com/benmosher/eslint-plugin-import/pull/1606
 [#1602]: https://github.com/benmosher/eslint-plugin-import/pull/1602
 [#1591]: https://github.com/benmosher/eslint-plugin-import/pull/1591
@@ -94,3 +98,4 @@ Yanked due to critical issue with cache key resulting from #839.
 [@arcanis]: https://github.com/arcanis
 [@sompylasar]: https://github.com/sompylasar
 [@iamnapo]: https://github.com/iamnapo
+[@kaiyoma]: https://github.com/kaiyoma

tests/src/rules/order.js

@@ -76,7 +76,9 @@ exports.default = function parse(path, content, context) {
         visitorKeys: keysFromParser(parserPath, parser, parserRaw),
       }
     } catch (e) {
-      //
+      console.warn()
+      console.warn('Error while parsing ' + parserOptions.filePath)
+      console.warn('Line ' + e.lineNumber + ', column ' + e.column + ': ' + e.message)
     }
     if (!ast || typeof ast !== 'object') {
       console.warn(