Re-apply the non-breaking pieces reverted in #6112 (#6170)

src/actions/pause/mapScopes.js

@@ -251,7 +251,7 @@ function isReliableScope(scope: OriginalScope): boolean {
   }
 
   // As determined by fair dice roll.
-  return totalBindings === 0 || unknownBindings / totalBindings < 0.9;
+  return totalBindings === 0 || unknownBindings / totalBindings < 0.1;
 }
 
 function generateClientScope(

src/utils/function.js

@@ -23,7 +23,9 @@ export function findFunctionText(
     return null;
   }
 
-  const { location: { start, end } } = func;
+  const {
+    location: { start, end }
+  } = func;
   const lines = source.text.split("\n");
   const firstLine = lines[start.line - 1].slice(start.column);
   const lastLine = lines[end.line - 1].slice(0, end.column);

src/utils/pause/mapScopes/findGeneratedBindingFromPosition.js

@@ -51,58 +51,66 @@ export async function findGeneratedBindingFromPosition(
     locationType,
     sourceMaps
   );
-  const applicableBindings = filterApplicableBindings(
+  let applicableBindings = filterApplicableBindings(
     generatedAstBindings,
     generatedRanges
   );
 
+  // We can adjust this number as we go, but these are a decent start as a
+  // general heuristic to assume the bindings were bad or just map a chunk of
+  // whole line or something.
+  if (applicableBindings.length > 4) {
+    // Babel's for..of generates at least 3 bindings inside one range for
+    // block-scoped loop variables, so we shouldn't go below that.
+    applicableBindings = [];
+  }
+
   let result;
   if (bindingType === "import") {
     result = await findGeneratedImportReference(applicableBindings);
-  } else {
-    result = await findGeneratedReference(applicableBindings);
-  }
 
-  if (result) {
-    return result;
-  }
+    if (!result && pos.type === "decl") {
+      const importName = pos.importName;
+      if (typeof importName !== "string") {
+        // Should never happen, just keeping Flow happy.
+        return null;
+      }
 
-  if (bindingType === "import" && pos.type === "decl") {
-    const importName = pos.importName;
-    if (typeof importName !== "string") {
-      // Should never happen, just keeping Flow happy.
-      return null;
-    }
+      let applicableImportBindings = applicableBindings;
+      if (generatedRanges.length === 0) {
+        // If the imported name itself does not map to a useful range, fall back
+        // to resolving the bindinding using the location of the overall
+        // import declaration.
+        const declarationRanges = await getGeneratedLocationRanges(
+          source,
+          pos.declaration,
+          bindingType,
+          locationType,
+          sourceMaps
+        );
+        applicableImportBindings = filterApplicableBindings(
+          generatedAstBindings,
+          declarationRanges
+        );
+
+        if (applicableImportBindings.length > 10) {
+          // Import declarations tend to have a large number of bindings for
+          // for things like 'require' and 'interop', so this number is larger
+          // than other binding count checks.
+          applicableImportBindings = [];
+        }
+      }
 
-    let applicableImportBindings = applicableBindings;
-    if (generatedRanges.length === 0) {
-      // If the imported name itself does not map to a useful range, fall back
-      // to resolving the bindinding using the location of the overall
-      // import declaration.
-      const importRanges = await getGeneratedLocationRanges(
-        source,
-        pos.declaration,
-        bindingType,
-        locationType,
-        sourceMaps
-      );
-      applicableImportBindings = filterApplicableBindings(
-        generatedAstBindings,
-        importRanges
+      result = await findGeneratedImportDeclaration(
+        applicableImportBindings,
+        importName
       );
-
-      if (applicableImportBindings.length === 0) {
-        return null;
-      }
     }
-
-    return await findGeneratedImportDeclaration(
-      applicableImportBindings,
-      importName
-    );
+  } else {
+    result = await findGeneratedReference(applicableBindings);
   }
 
-  return null;
+  return result;
 }
 
 type ApplicableBinding = {
@@ -474,6 +482,18 @@ async function getGeneratedLocationRanges(
   const ranges = await sourceMaps.getGeneratedRanges(start, source);
 
   const resultRanges = ranges.reduce((acc, mapRange) => {
+    // Some tooling creates ranges that map a line as a whole, which is useful
+    // for step-debugging, but can easily lead to finding the wrong binding.
+    // To avoid these false-positives, we entirely ignore ranges that cover
+    // full lines.
+    if (
+      locationType === "ref" &&
+      mapRange.columnStart === 0 &&
+      mapRange.columnEnd === Infinity
+    ) {
+      return acc;
+    }
+
     const range = {
       start: {
         line: mapRange.line,

src/workers/parser/getScopes/visitor.js

@@ -453,25 +453,42 @@ const scopeCollectionVisitor = {
         };
       }
     } else if (t.isClass(node)) {
-      if (t.isClassDeclaration(node) && t.isIdentifier(node.id)) {
-        state.declarationBindingIds.add(node.id);
-        state.scope.bindings[node.id.name] = {
-          type: "let",
-          refs: [
-            {
-              type: "decl",
-              start: fromBabelLocation(node.id.loc.start, state.sourceId),
-              end: fromBabelLocation(node.id.loc.end, state.sourceId),
-              declaration: {
-                start: fromBabelLocation(node.loc.start, state.sourceId),
-                end: fromBabelLocation(node.loc.end, state.sourceId)
-              }
-            }
-          ]
+      if (t.isIdentifier(node.id)) {
+        // For decorated classes, the AST considers the first the decorator
+        // to be the start of the class. For the purposes of mapping class
+        // declarations however, we really want to look for the "class Foo"
+        // piece. To achieve that, we estimate the location of the declaration
+        // instead.
+        let declStart = node.loc.start;
+        if (node.decorators && node.decorators.length > 0) {
+          // Estimate the location of the "class" keyword since it
+          // is unlikely to be a different line than the class name.
+          declStart = {
+            line: node.id.loc.start.line,
+            column: node.id.loc.start.column - "class ".length
+          };
+        }
+
+        const declaration = {
+          start: fromBabelLocation(declStart, state.sourceId),
+          end: fromBabelLocation(node.loc.end, state.sourceId)
         };
-      }
 
-      if (t.isIdentifier(node.id)) {
+        if (t.isClassDeclaration(node)) {
+          state.declarationBindingIds.add(node.id);
+          state.scope.bindings[node.id.name] = {
+            type: "let",
+            refs: [
+              {
+                type: "decl",
+                start: fromBabelLocation(node.id.loc.start, state.sourceId),
+                end: fromBabelLocation(node.id.loc.end, state.sourceId),
+                declaration
+              }
+            ]
+          };
+        }
+
         const scope = pushTempScope(state, "block", "Class", {
           start: fromBabelLocation(node.loc.start, state.sourceId),
           end: fromBabelLocation(node.loc.end, state.sourceId)
@@ -485,10 +502,7 @@ const scopeCollectionVisitor = {
               type: "decl",
               start: fromBabelLocation(node.id.loc.start, state.sourceId),
               end: fromBabelLocation(node.id.loc.end, state.sourceId),
-              declaration: {
-                start: fromBabelLocation(node.loc.start, state.sourceId),
-                end: fromBabelLocation(node.loc.end, state.sourceId)
-              }
+              declaration
             }
           ]
         };