From 201a93347e527ceeb887708715f58427799d85f6 Mon Sep 17 00:00:00 2001
From: Lee Byron <lee@leebyron.com>
Date: Mon, 18 Feb 2019 09:23:45 -0800
Subject: [PATCH] fix: Ensure correct scope references after traversal (#192)

* test: Add tests illustrating scope issue
* fix: Re-crawl scope after traversal to fix references
---
 fixtures/with-changed-scope.js |  2 ++
 package-lock.json              | 19 +++++++++++++
 package.json                   |  1 +
 src/index.js                   |  3 ++
 test/babel-plugin-istanbul.js  | 51 ++++++++++++++++++++++++++++++++++
 5 files changed, 76 insertions(+)
 create mode 100644 fixtures/with-changed-scope.js

diff --git a/fixtures/with-changed-scope.js b/fixtures/with-changed-scope.js
new file mode 100644
index 0000000..8eba71c
--- /dev/null
+++ b/fixtures/with-changed-scope.js
@@ -0,0 +1,2 @@
+import { noop } from 'lodash'
+noop(noop() ? noop : noop)
diff --git a/package-lock.json b/package-lock.json
index 5b88d60..8536378 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -988,6 +988,19 @@
         }
       }
     },
+    "babel-plugin-lodash": {
+      "version": "3.3.4",
+      "resolved": "https://registry.npmjs.org/babel-plugin-lodash/-/babel-plugin-lodash-3.3.4.tgz",
+      "integrity": "sha512-yDZLjK7TCkWl1gpBeBGmuaDIFhZKmkoL+Cu2MUUjv5VxUZx/z7tBGBCBcQs5RI1Bkz5LLmNdjx7paOyQtMovyg==",
+      "dev": true,
+      "requires": {
+        "@babel/helper-module-imports": "^7.0.0-beta.49",
+        "@babel/types": "^7.0.0-beta.49",
+        "glob": "^7.1.1",
+        "lodash": "^4.17.10",
+        "require-package-name": "^2.0.1"
+      }
+    },
     "balanced-match": {
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.0.tgz",
@@ -6636,6 +6649,12 @@
       "resolved": "https://registry.npmjs.org/require-main-filename/-/require-main-filename-1.0.1.tgz",
       "integrity": "sha1-l/cXtp1IeE9fUmpsWqj/3aBVpNE="
     },
+    "require-package-name": {
+      "version": "2.0.1",
+      "resolved": "https://registry.npmjs.org/require-package-name/-/require-package-name-2.0.1.tgz",
+      "integrity": "sha1-wR6XJ2tluOKSP3Xav1+y7ww4Qbk=",
+      "dev": true
+    },
     "require-uncached": {
       "version": "1.0.3",
       "resolved": "https://registry.npmjs.org/require-uncached/-/require-uncached-1.0.3.tgz",
diff --git a/package.json b/package.json
index 4d2c18e..34f4a7b 100644
--- a/package.json
+++ b/package.json
@@ -18,6 +18,7 @@
     "@babel/core": "^7.2.2",
     "@babel/preset-env": "^7.2.3",
     "@babel/register": "^7.0.0",
+    "babel-plugin-lodash": "^3.3.4",
     "chai": "^4.2.0",
     "coveralls": "^3.0.2",
     "cross-env": "^5.2.0",
diff --git a/src/index.js b/src/index.js
index 8843650..c1dd7df 100644
--- a/src/index.js
+++ b/src/index.js
@@ -70,6 +70,9 @@ function makeVisitor ({ types: t }) {
             inputSourceMap
           })
           this.__dv__.enter(path)
+          // Istanbul visitor may replace Identifiers and require re-crawling
+          // scope before continuing with other babel plugins.
+          path.scope.crawl()
         },
         exit (path) {
           if (!this.__dv__) {
diff --git a/test/babel-plugin-istanbul.js b/test/babel-plugin-istanbul.js
index b42cbee..2003d1d 100644
--- a/test/babel-plugin-istanbul.js
+++ b/test/babel-plugin-istanbul.js
@@ -171,4 +171,55 @@ describe('babel-plugin-istanbul', function () {
       resultAfter.code.should.match(/statementMap/)
     })
   })
+
+  describe('should leave scope with correct references', function () {
+    it('leaves scope references as Identifiers', function () {
+      // This is a unit test which inspects the root cause of the problem:
+      // scope references that are replaced to no longer be Identifiers
+      babel.transformFileSync('./fixtures/with-changed-scope.js', {
+        babelrc: false,
+        configFile: false,
+        plugins: [
+          [makeVisitor({ types: babel.types }), {
+            include: ['fixtures/with-changed-scope.js']
+          }],
+          function testScope (t) {
+            return {
+              visitor: {
+                ImportSpecifier (path) {
+                  const fileScope = path.hub.file.scope
+                  const localBinding = fileScope.getBinding(path.node.local.name)
+                  for (const localReference of localBinding.referencePaths) {
+                    localReference.type.should.equal('Identifier')
+                  }
+                }
+              }
+            }
+          }
+        ]
+      })
+    })
+
+    it('creates valid syntax with other transforming plugins', function () {
+      // This is a minimal end-to-end test which illustrates how scope
+      // reference alteration impacts the assumptions of other plugins
+      const result = babel.transformFileSync(
+        './fixtures/with-changed-scope.js', {
+          babelrc: false,
+          configFile: false,
+          plugins: [
+            [makeVisitor({ types: babel.types }), {
+              include: ['fixtures/with-changed-scope.js']
+            }],
+            'babel-plugin-lodash'
+          ]
+        })
+
+      function testSyntaxError () {
+        babel.parse(result.code)
+      }
+
+      testSyntaxError.should.not.throw()
+    })
+  })
 })