Prevent parseForESLint() behavior from changing after parse() is called

lib/analyze-scope.js

@@ -318,9 +318,16 @@ module.exports = function(ast, parserOptions) {
     impliedStrict: false,
     sourceType: ast.sourceType,
     ecmaVersion: parserOptions.ecmaVersion || 6,
-    childVisitorKeys,
     fallback,
   };
+
+  if (OriginalReferencer._babelEslintPatched) {
+    options._skipBabelEslintGlobals = true;
+    return escope.analyze(ast, options);
+  }
+
+  options.childVisitorKeys = childVisitorKeys;
+
   const scopeManager = new escope.ScopeManager(options);
   const referencer = new Referencer(options, scopeManager);
 

lib/index.js

@@ -1,18 +1,14 @@
 "use strict";
 
-let patched = false;
-
 exports.parse = function(code, options) {
-  patched = true;
   return require("./parse-with-patch")(code, options);
 };
 
 exports.parseForESLint = function(code, options) {
-  if (!patched && options.eslintVisitorKeys && options.eslintScopeManager) {
+  if (options.eslintVisitorKeys && options.eslintScopeManager) {
     return require("./parse-with-scope")(code, options);
   }
 
-  patched = true;
   return { ast: require("./parse-with-patch")(code, options) };
 };
 

lib/parse-with-patch.js

@@ -60,15 +60,16 @@ function monkeypatch(modules) {
 
   var analyze = escope.analyze;
   escope.analyze = function(ast, opts) {
-    opts = opts || {};
-    opts.ecmaVersion = eslintOptions.ecmaVersion;
-    opts.sourceType = eslintOptions.sourceType;
-    if (eslintOptions.globalReturn !== undefined) {
-      opts.nodejsScope = eslintOptions.globalReturn;
+    if (!opts || !opts._skipBabelEslintGlobals) {
+      opts = opts || {};
+      opts.ecmaVersion = eslintOptions.ecmaVersion;
+      opts.sourceType = eslintOptions.sourceType;
+      if (eslintOptions.globalReturn !== undefined) {
+        opts.nodejsScope = eslintOptions.globalReturn;
+      }
     }
 
-    var results = analyze.call(this, ast, opts);
-    return results;
+    return analyze.call(this, ast, opts);
   };
 
   // if there are decorators, then visit each
@@ -352,6 +353,8 @@ function monkeypatch(modules) {
       this.close(node);
     }
   };
+
+  referencer._babelEslintPatched = true;
 }
 
 module.exports = function(code, options) {

package.json

@@ -39,9 +39,11 @@
   "devDependencies": {
     "babel-eslint": "^8.0.0",
     "dedent": "^0.7.0",
-    "eslint": "^4.12.1",
+    "eslint": "^4.14.0",
     "eslint-config-babel": "^7.0.1",
+    "eslint-old": "npm:eslint@4.13.1",
     "eslint-plugin-flowtype": "^2.30.3",
+    "eslint-plugin-import": "^2.8.0",
     "eslint-plugin-prettier": "^2.1.2",
     "espree": "^3.4.0",
     "husky": "^0.14.0",

test/babel-eslint.js

@@ -539,4 +539,31 @@ describe("Public API", () => {
       babelEslint.parseNoPatch("foo", {})
     );
   });
+
+  /*
+   * This test ensures that the enhanced referencer does not get used if eslint-scope has already been
+   * monkeypatched, because this causes some correctness issues. For example, if the enhanced referencer
+   * is used after the original referencer is monkeypatched, type annotation references are counted twice.
+   */
+  it("does not visit type annotations multiple times after monkeypatching and calling parseForESLint()", () => {
+    assertImplementsAST(
+      espree.parse("foo", { sourceType: "module" }),
+      babelEslint.parse("foo", {})
+    );
+    const parseResult = babelEslint.parseForESLint(
+      "type Foo = {}; function x(): Foo {}",
+      {
+        eslintVisitorKeys: true,
+        eslintScopeManager: true,
+      }
+    );
+    assert(parseResult.visitorKeys);
+    assert(parseResult.scopeManager);
+
+    const fooVariable = parseResult.scopeManager.getDeclaredVariables(
+      parseResult.ast.body[0]
+    )[0];
+
+    assert.strictEqual(fooVariable.references.length, 1);
+  });
 });

test/fixtures/eslint-plugin-import/.eslintrc.yml

@@ -0,0 +1,11 @@
+root: true
+
+# babel-eslint
+parser: ../../../lib/index.js
+
+# use eslint-plugin-import
+plugins:
+  - import
+rules:
+  import/no-named-as-default: error
+  no-unused-vars: error

test/fixtures/eslint-plugin-import/a.js

@@ -0,0 +1 @@
+export default function foo() { }

test/fixtures/eslint-plugin-import/b.js

@@ -0,0 +1 @@
+import foo from './a.js';

test/fixtures/eslint-plugin-import/c.js

@@ -0,0 +1,4 @@
+// @flow
+type Foo = {};
+
+const FlowTypeButton = ({ }: Foo) => { };

test/non-regression.js

@@ -1,14 +1,16 @@
 /*eslint-env mocha*/
 "use strict";
 var eslint = require("eslint");
+var oldEslint = require("eslint-old");
 var unpad = require("dedent");
 
-function verifyAndAssertMessages(
+function verifyAndAssertMessagesWithSpecificESLint(
   code,
   rules,
   expectedMessages,
   sourceType,
-  overrideConfig
+  overrideConfig,
+  linter
 ) {
   var config = {
     parser: require.resolve(".."),
@@ -34,7 +36,7 @@ function verifyAndAssertMessages(
     }
   }
 
-  var messages = eslint.linter.verify(code, config);
+  var messages = linter.verify(code, config);
 
   if (messages.length !== expectedMessages.length) {
     throw new Error(
@@ -62,6 +64,25 @@ function verifyAndAssertMessages(
   });
 }
 
+function verifyAndAssertMessages(
+  code,
+  rules,
+  expectedMessages,
+  sourceType,
+  overrideConfig
+) {
+  [new eslint.Linter(), new oldEslint.Linter()].forEach(linter => {
+    verifyAndAssertMessagesWithSpecificESLint(
+      code,
+      rules,
+      expectedMessages,
+      sourceType,
+      overrideConfig,
+      linter
+    );
+  });
+}
+
 describe("verify", () => {
   it("arrow function support (issue #1)", () => {
     verifyAndAssertMessages("describe('stuff', () => {});", {}, []);

test/z_eslint-plugin-import.js

@@ -0,0 +1,14 @@
+"use strict";
+
+const eslint = require("eslint");
+
+describe("https://github.com/babel/babel-eslint/issues/558", () => {
+  it("don't crash with eslint-plugin-import", () => {
+    const engine = new eslint.CLIEngine({ ignore: false });
+    engine.executeOnFiles([
+      "test/fixtures/eslint-plugin-import/a.js",
+      "test/fixtures/eslint-plugin-import/b.js",
+      "test/fixtures/eslint-plugin-import/c.js",
+    ]);
+  });
+});