use Unambiguous JavaScript Grammar to detect/ignore non-ES-module imp…

…orts. (#268 and #270)

CHANGELOG.md

@@ -5,7 +5,8 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ## [Unreleased]
 ### Breaking
-- [`import/extensions`] defaults to `['.js']`.
+- [`import/extensions` setting] defaults to `['.js']`. ([#306])
+- [`import/ignore` setting] defaults to nothing, and ambiguous modules are ignored natively. This means importing from CommonJS modules will no longer be reported by [`default`], [`named`], or [`namespace`], regardless of `import/ignore`. ([#270])
 
 ## [1.10.1] - 2016-07-02
 ### Added
@@ -287,11 +288,13 @@ for info on changes for earlier releases.
 [#328]: https://github.com/benmosher/eslint-plugin-import/issues/328
 [#317]: https://github.com/benmosher/eslint-plugin-import/issues/317
 [#313]: https://github.com/benmosher/eslint-plugin-import/issues/313
+[#306]: https://github.com/benmosher/eslint-plugin-import/issues/306
 [#286]: https://github.com/benmosher/eslint-plugin-import/issues/286
 [#283]: https://github.com/benmosher/eslint-plugin-import/issues/283
 [#281]: https://github.com/benmosher/eslint-plugin-import/issues/281
 [#275]: https://github.com/benmosher/eslint-plugin-import/issues/275
 [#272]: https://github.com/benmosher/eslint-plugin-import/issues/272
+[#270]: https://github.com/benmosher/eslint-plugin-import/issues/270
 [#267]: https://github.com/benmosher/eslint-plugin-import/issues/267
 [#266]: https://github.com/benmosher/eslint-plugin-import/issues/266
 [#216]: https://github.com/benmosher/eslint-plugin-import/issues/216

README.md

@@ -199,40 +199,31 @@ You may set the following settings in your `.eslintrc`:
 A whitelist of file extensions that will be parsed as modules and inspected for
 `export`s.
 
-This will default to `['.js']` in the next major revision of this plugin, unless
-you are using the `react` shared config, in which case it is specified as `['.js', '.jsx']`.
+This defaults to `['.js']`, unless you are using the `react` shared config,
+in which case it is specified as `['.js', '.jsx']`.
 
 Note that this is different from (and likely a subset of) any `import/resolver`
 extensions settings, which may include `.json`, `.coffee`, etc. which will still
 factor into the `no-unresolved` rule.
 
-Also, `import/ignore` patterns will overrule this whitelist, so `node_modules` that
-end in `.js` will still be ignored by default.
+Also, the following `import/ignore` patterns will overrule this whitelist.
 
 #### `import/ignore`
 
 A list of regex strings that, if matched by a path, will
 not report the matching module if no `export`s are found.
 In practice, this means rules other than [`no-unresolved`](./docs/rules/no-unresolved.md#ignore) will not report on any
-`import`s with (absolute) paths matching this pattern, _unless_ `export`s were
-found when parsing. This allows you to ignore `node_modules` but still properly
-lint packages that define a [`jsnext:main`] in `package.json` (Redux, D3's v4 packages, etc.).
+`import`s with (absolute filesystem) paths matching this pattern.
 
 `no-unresolved` has its own [`ignore`](./docs/rules/no-unresolved.md#ignore) setting.
 
-**Note**: setting this explicitly will replace the default of `node_modules`, so you
-may need to include it in your own list if you still want to ignore it. Example:
-
 ```yaml
 settings:
   import/ignore:
-    - node_modules       # mostly CommonJS (ignored by default)
     - \.coffee$          # fraught with parse errors
     - \.(scss|less|css)$ # can't parse unprocessed CSS modules, either
 ```
 
-[`jsnext:main`]: https://github.com/rollup/rollup/wiki/jsnext:main
-
 #### `import/core-modules`
 
 An array of additional modules to consider as "core" modules--modules that should

docs/rules/default.md

@@ -6,12 +6,14 @@ export in the imported module.
 For [ES7], reports if a default is named and exported but is not found in the
 referenced module.
 
-Note: for modules, the plugin will find exported names (including defaults)
+Note: for packages, the plugin will find exported names
 from [`jsnext:main`], if present in `package.json`.
-Redux's npm module includes this key, and thereby is lintable, for example. Otherwise,
-the whole `node_modules` folder is ignored by default ([`import/ignore`]) as most published modules are
-formatted in CommonJS, which [at time of this writing](https://github.com/benmosher/eslint-plugin-import/issues/13)
-is not able to be analyzed for exports.
+Redux's npm module includes this key, and thereby is lintable, for example.
+
+A module path that is [ignored] or not [unambiguously an ES module] will not be reported when imported.
+
+[ignored]: ../README.md#importignore
+[unambiguously an ES module]: https://github.com/bmeck/UnambiguousJavaScriptGrammar
 
 
 ## Rule Details

docs/rules/named.md

@@ -4,11 +4,14 @@ Verifies that all named imports are part of the set of named exports in the refe
 
 For `export`, verifies that all named exports exist in the referenced module.
 
-Note: for modules, the plugin will find exported names from [`jsnext:main`], if present in `package.json`.
-Redux's npm module includes this key, and thereby is lintable, for example. Otherwise,
-the whole `node_modules` folder is ignored by default ([`import/ignore`]) as most published modules are
-formatted in CommonJS, which [at time of this writing](https://github.com/benmosher/eslint-plugin-import/issues/13)
-is not able to be analyzed for exports.
+Note: for packages, the plugin will find exported names
+from [`jsnext:main`], if present in `package.json`.
+Redux's npm module includes this key, and thereby is lintable, for example.
+
+A module path that is [ignored] or not [unambiguously an ES module] will not be reported when imported.
+
+[ignored]: ../README.md#importignore
+[unambiguously an ES module]: https://github.com/bmeck/UnambiguousJavaScriptGrammar
 
 
 ## Rule Details

docs/rules/namespace.md

@@ -8,11 +8,14 @@ Also, will report for computed references (i.e. `foo["bar"]()`).
 
 Reports on assignment to a member of an imported namespace.
 
-Note: for modules, the plugin will find exported names from [`jsnext:main`], if present in `package.json`.
-Redux's npm module includes this key, and thereby is lintable, for example. Otherwise,
-the whole `node_modules` folder is ignored by default ([`import/ignore`]) as most published modules are
-formatted in CommonJS, which [at time of this writing](https://github.com/benmosher/eslint-plugin-import/issues/13)
-is not able to be analyzed for exports.
+Note: for packages, the plugin will find exported names
+from [`jsnext:main`], if present in `package.json`.
+Redux's npm module includes this key, and thereby is lintable, for example.
+
+A module path that is [ignored] or not [unambiguously an ES module] will not be reported when imported.
+
+[ignored]: ../README.md#importignore
+[unambiguously an ES module]: https://github.com/bmeck/UnambiguousJavaScriptGrammar
 
 ## Rule Details
 

src/ExportMap.js

@@ -14,12 +14,16 @@ const hashObject = require('eslint-module-utils/hash').hashObject
 const exportCache = new Map()
 
 /**
- * detect exports without a full parse.
+ * detect possible imports/exports without a full parse.
  * used primarily to ignore the import/ignore setting, iif it looks like
  * there might be something there (i.e., jsnext:main is set).
+ *
+ * Not perfect, just a fast way to disqualify large non-ES6 modules and
+ * avoid a parse.
  * @type {RegExp}
  */
-const hasExports = new RegExp('(^|[\\n;])\\s*export\\s[\\w{*]')
+const potentiallyUnambiguousModule =
+  new RegExp(`(?:^|;)\s*(?:export|import)(?:(?:\s+\w)|(?:\s*[{*]))`)
 
 class ExportMap {
   constructor(path) {
@@ -286,18 +290,28 @@ ExportMap.for = function (path, context) {
   const content = fs.readFileSync(path, { encoding: 'utf8' })
 
   // check for and cache ignore
-  if (isIgnored(path, context) && !hasExports.test(content)) {
+  if (isIgnored(path, context) && !potentiallyUnambiguousModule.test(content)) {
     exportCache.set(cacheKey, null)
     return null
   }
 
   exportMap = ExportMap.parse(path, content, context)
+
+  // ambiguous modules return null
+  if (exportMap == null) return null
+
   exportMap.mtime = stats.mtime
 
   exportCache.set(cacheKey, exportMap)
   return exportMap
 }
 
+
+const unambiguousNodeType = /^(Exp|Imp)ort.*Declaration$/
+function isUnambiguousModule(ast) {
+  return ast.body.some(node => unambiguousNodeType.test(node.type))
+}
+
 ExportMap.parse = function (path, content, context) {
   var m = new ExportMap(path)
 
@@ -308,6 +322,8 @@ ExportMap.parse = function (path, content, context) {
     return m // can't continue
   }
 
+  if (!isUnambiguousModule(ast)) return null
+
   const docstyle = (context.settings && context.settings['import/docstyle']) || ['jsdoc']
   const docStyleParsers = {}
   docstyle.forEach(style => {

tests/src/rules/default.js

@@ -22,8 +22,7 @@ ruleTester.run('default', rule, {
     // core modules always have a default
     test({ code: 'import crypto from "crypto";' }),
 
-    test({ code: 'import common from "./common";'
-         , settings: { 'import/ignore': ['common'] } }),
+    test({ code: 'import common from "./common";' }),
 
     // es7 export syntax
     test({ code: 'export bar from "./bar"'
@@ -75,21 +74,11 @@ ruleTester.run('default', rule, {
       errors: ["Parse errors in imported module './jsx/FooES7.js': Unexpected token = (6:16)"],
     }),
 
-    test({
-      code: 'import crypto from "./common";',
-      settings: { 'import/ignore': ['foo'] },
-      errors: [{ message: 'No default export found in module.'
-               , type: 'ImportDefaultSpecifier'}]}),
     test({
       code: 'import baz from "./named-exports";',
       errors: [{ message: 'No default export found in module.'
                , type: 'ImportDefaultSpecifier'}]}),
 
-    test({
-      code: 'import bar from "./common";',
-      errors: [{ message: 'No default export found in module.'
-               , type: 'ImportDefaultSpecifier'}]}),
-
     test({
       code: "import Foo from './jsx/FooES7.js';",
       errors: ["Parse errors in imported module './jsx/FooES7.js': Unexpected token = (6:16)"],

tests/src/rules/named.js

@@ -40,9 +40,6 @@ ruleTester.run('named', rule, {
 
     test({ code: 'import { someThing } from "./test-module"' }),
 
-    // node_modules/a only exports 'foo', should be ignored though
-    test({ code: 'import { zoob } from "a"' }),
-
     // export tests
     test({ code: 'export { foo } from "./bar"' }),
     test({ code: 'export { foo as bar } from "./bar"' }),
@@ -96,23 +93,24 @@ ruleTester.run('named', rule, {
       settings: { 'import/ignore': ['common'] },
     }),
 
+    // ignore CJS by default. always ignore ignore list
+    test({ code: 'import {a, b, d} from "./common"' }),
+    test({
+      code: 'import { baz } from "./bar"',
+      settings: { 'import/ignore': ['bar'] },
+    }),
+    test({
+      code: 'import { common } from "./re-export-default"',
+    }),
+
     ...SYNTAX_CASES,
   ],
 
   invalid: [
 
-    test({ code: 'import { zoob } from "a"'
-         , settings: { 'import/ignore': [] }
-         , errors: [ error('zoob', 'a') ] }),
-
     test({ code: 'import { somethingElse } from "./test-module"'
          , errors: [ error('somethingElse', './test-module') ] }),
 
-    test({code: 'import {a, b, d} from "./common"',
-      errors: [ error('a', './common')
-              , error('b', './common')
-              , error('d', './common') ]}),
-
     test({code: 'import { baz } from "./bar"',
       errors: [error('baz', './bar')]}),
 
@@ -126,9 +124,6 @@ ruleTester.run('named', rule, {
     test({code: 'import { a } from "./default-export"',
       errors: [error('a', './default-export')]}),
 
-    test({code: 'import { a } from "./common"', args: [2, 'es6-only'],
-      errors: [error('a', './common')]}),
-
     test({code: 'import { ActionTypess } from "./qc"',
       errors: [error('ActionTypess', './qc')]}),
 
@@ -201,22 +196,13 @@ ruleTester.run('named', rule, {
       code: 'import { baz } from "es6-module"',
       errors: ["baz not found in 'es6-module'"],
     }),
-    test({
-      code: 'import { baz } from "./bar"',
-      settings: { 'import/ignore': ['bar'] },
-      errors: ["baz not found in './bar'"],
-    }),
 
     // issue #251
     test({
       code: 'import { foo, bar, bap } from "./re-export-default"',
       errors: ["bap not found in './re-export-default'"],
     }),
-    test({
-      code: 'import { common } from "./re-export-default"',
-      // todo: better error message
-      errors: ["common not found via re-export-default.js -> common.js"],
-    }),
+
 
     // #328: * exports do not include default
     test({

tests/src/rules/namespace.js

@@ -27,8 +27,7 @@ const valid = [
       ecmaFeatures: { jsx: true },
     },
   }),
-  test({ code: "import * as foo from './common';"
-       , settings: { 'import/ignore': ['common'] } }),
+  test({ code: "import * as foo from './common';" }),
 
   // destructuring namespaces
   test({ code: 'import * as names from "./named-exports";' +
@@ -91,9 +90,6 @@ const valid = [
 ]
 
 const invalid = [
-  test({code: "import * as foo from './common';",
-        errors: ["No exported names found in module './common'."]}),
-
   test({ code: "import * as names from './named-exports'; " +
                ' console.log(names.c);'
        , errors: [error('c', 'names')] }),

utils/ignore.js

@@ -16,15 +16,11 @@ function validExtensions(context) {
 }
 
 exports.default = function ignore(path, context) {
-  // ignore node_modules by default
-  const ignoreStrings = context.settings['import/ignore']
-    ? [].concat(context.settings['import/ignore'])
-    : ['node_modules']
-
   // check extension whitelist first (cheap)
   if (!validExtensions(context).has(extname(path))) return true
 
-  if (ignoreStrings.length === 0) return false
+  if (!('import/ignore' in context.settings)) return false
+  const ignoreStrings = context.settings['import/ignore']
 
   for (let i = 0; i < ignoreStrings.length; i++) {
     const regex = new RegExp(ignoreStrings[i])

utils/resolve.js

@@ -180,7 +180,7 @@ function resolve(p, context) {
     if (!erroredContexts.has(context)) {
       context.report({
         message: `Resolve error: ${err.message}`,
-        loc: { line: 1, col: 0 },
+        loc: { line: 1, column: 0 },
       })
       erroredContexts.add(context)
     }