Use MakeDeclaredNamesUnique in RewriteClassMembers to remove any name shadowing in classes.

This includes the cases where the class name matches the variable declaring it and any names in the rhs of a public field, such as the parameter name in an arrow function, matching a constructor parameter name.

PiperOrigin-RevId: 554505664

Author: Closure Team

src/com/google/javascript/jscomp/RewriteClassMembers.java

@@ -48,6 +48,10 @@ public RewriteClassMembers(AbstractCompiler compiler) {
 
   @Override
   public void process(Node externs, Node root) {
+    // Make all declared names unique to ensure that name shadowing can't occur in classes and
+    // public fields don't share names with constructor parameters
+    MakeDeclaredNamesUnique renamer = MakeDeclaredNamesUnique.builder().build();
+    NodeTraversal.traverseRoots(compiler, renamer, externs, root);
     NodeTraversal.traverse(compiler, root, this);
     TranspilationPasses.maybeMarkFeaturesAsTranspiledAway(
         compiler, root, Feature.PUBLIC_CLASS_FIELDS, Feature.CLASS_STATIC_BLOCK);
@@ -66,15 +70,10 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
         checkState(classNameNode != null, "Class missing a name: %s", n);
         Node classInsertionPoint = getStatementDeclaringClass(n, classNameNode);
         checkState(classInsertionPoint != null, "Class was not extracted: %s", n);
-
-        if (!n.getFirstChild().isEmpty()
-            && !classNameNode.matchesQualifiedName(n.getFirstChild())) {
-          // we do not allow `let x = class C {}` where the names inside the class can be shadowed
-          // at this time
-          t.report(n, TranspilationUtil.CANNOT_CONVERT_YET, "Classes with possible name shadowing");
-          return false;
-        }
-
+        checkState(
+            n.getFirstChild().isEmpty() || classNameNode.matchesQualifiedName(n.getFirstChild()),
+            "Class name shadows variable declaring the class: %s",
+            n);
         classStack.push(new ClassRecord(n, classNameNode.getQualifiedName(), classInsertionPoint));
         break;
       case COMPUTED_FIELD_DEF:
@@ -247,13 +246,10 @@ private void rewriteInstanceMembers(NodeTraversal t, ClassRecord record) {
 
       for (Node nameInRhs : record.referencedNamesByMember.get(instanceMember)) {
         String name = nameInRhs.getString();
-        if (ctorDefinedNames.contains(name)) {
-          t.report(
-              nameInRhs,
-              TranspilationUtil.CANNOT_CONVERT_YET,
-              "Initializer referencing identifier '" + name + "' declared in constructor");
-          return;
-        }
+        checkState(
+            !ctorDefinedNames.contains(name),
+            "Rhs of public field cannot use the same name as a constructor parameter: %s",
+            name);
       }
 
       Node thisNode = astFactory.createThisForEs6ClassMember(instanceMember);

test/com/google/javascript/jscomp/RewriteClassMembersTest.java

@@ -112,6 +112,18 @@ public void testCannotConvertYet() {
         "}")*/
         CANNOT_CONVERT_YET); // uses `this` in static block
 
+    testError(
+        lines(
+            "let C = class {",
+            "  static prop = 5;",
+            "},",
+            "D = class extends C {",
+            "  static {",
+            "    console.log(this.prop);",
+            "  }",
+            "}"),
+        CANNOT_CONVERT_YET); // uses `this` in static block
+
     testError(
         lines(
             "var z = 1", //
@@ -722,7 +734,7 @@ public void testClassStaticBlocksNoFieldAssign() {
             "{",
             "  let x = 5;",
             "  class Bar {}",
-            "  {let x = 'str';}",
+            "  {let x$jscomp$1 = 'str';}",
             "}"));
   }
 
@@ -1531,58 +1543,113 @@ public void testConstuctorAndStaticFieldDontConflict() {
         lines(
             "let x = 2;", //
             "class C {",
-            "  constructor(x) {}",
+            "  constructor(x$jscomp$1) {}",
             "}",
             "C.y = x"));
   }
 
   @Test
   public void testInstanceInitializerShadowsConstructorDeclaration() {
-    testError(
+    test(
         lines(
             "let x = 2;", //
             "class C {",
             "  y = x",
             "  constructor(x) {}",
             "}"),
-        CANNOT_CONVERT_YET);
+        lines(
+            "let x = 2;", //
+            "class C {",
+            "  constructor(x$jscomp$1) {",
+            "    this.y = x;",
+            "  }",
+            "}"));
 
-    testError(
+    test(
         lines(
             "let x = 2;", //
             "class C {",
-            "  y = x",
-            "  constructor() { let x;}",
+            "  y = x;",
+            "  constructor() { let x; }",
             "}"),
-        CANNOT_CONVERT_YET);
+        lines(
+            "let x = 2;",
+            "class C {",
+            "  constructor() {",
+            "    this.y = x;",
+            "    let x$jscomp$1;",
+            "  }",
+            "}"));
 
-    testError(
+    test(
         lines(
             "let x = 2;", //
             "class C {",
             "  y = x",
             "  constructor() { {var x;} }",
             "}"),
-        CANNOT_CONVERT_YET);
+        lines(
+            "let x = 2;",
+            "class C {",
+            "  constructor() {",
+            "    this.y = x;",
+            "    {",
+            "     var x$jscomp$1;",
+            "    }",
+            "  }",
+            "}"));
 
-    testError(
+    test(
         lines(
-            "function f() { return 4; };", //
+            "function f() { return 4; }", //
             "class C {",
             "  y = f();",
             "  constructor() {function f() { return 'str'; }}",
             "}"),
-        CANNOT_CONVERT_YET);
+        lines(
+            "function f() {",
+            "  return 4;",
+            "}",
+            "class C {",
+            "  constructor() {",
+            "    this.y = f();",
+            "    function f$jscomp$1() {",
+            "      return 'str';",
+            "    }",
+            "  }",
+            "}"));
 
-    // TODO(b/189993301): Technically this has no shadowing, so we could inline without renaming
-    testError(
+    test(
+        lines(
+            "class Foo {", //
+            "  constructor(x) {}",
+            "  y = (x) => x;",
+            "}"),
+        lines(
+            "class Foo {",
+            "  constructor(x) {",
+            "    this.y = x$jscomp$1 => {",
+            "      return x$jscomp$1;",
+            "    };",
+            "  }",
+            "}"));
+
+    test(
         lines(
             "let x = 2;", //
             "class C {",
             "  y = (x) => x;",
             "  constructor(x) {}",
             "}"),
-        CANNOT_CONVERT_YET);
+        lines(
+            "let x = 2;",
+            "class C {",
+            "  constructor(x$jscomp$2) {",
+            "    this.y = x$jscomp$1 => {",
+            "      return x$jscomp$1;",
+            "    };",
+            "  }",
+            "}"));
   }
 
   @Test
@@ -1591,15 +1658,15 @@ public void testInstanceInitializerDoesntShadowConstructorDeclaration() {
         lines(
             "let x = 2;", //
             "class C {",
-            "  y = x",
+            "  y = x;",
             "  constructor() { {let x;} }",
             "}"),
         lines(
             "let x = 2;",
             "class C {",
             "  constructor() {",
             "    this.y = x;",
-            "    {let x;}",
+            "    {let x$jscomp$1;}",
             "  }",
             "}"));
 
@@ -1615,7 +1682,7 @@ public void testInstanceInitializerDoesntShadowConstructorDeclaration() {
             "class C {",
             "  constructor() {",
             "    this.y = x;",
-            "    () => { let x; };",
+            "    () => { let x$jscomp$1; };",
             "  }",
             "}"));
 
@@ -1631,7 +1698,7 @@ public void testInstanceInitializerDoesntShadowConstructorDeclaration() {
             "class C {",
             "  constructor() {",
             "    this.y = x;",
-            "    (x) => { return 3; };",
+            "    (x$jscomp$1) => { return 3; };",
             "  }",
             "}"));
   }
@@ -1656,16 +1723,28 @@ public void testInstanceFieldInitializersDontBleedOut() {
 
   @Test
   public void testNestedClassesWithShadowingInstanceFields() {
-    testError(
+    test(
         lines(
-            "let x = 2;", //
+            "let x = 2;",
             "class C {",
             "  y = () => {",
-            "    class Foo { z = x };",
+            "    class Foo { z = x }",
             "  };",
             "  constructor(x) {}",
             "}"),
-        CANNOT_CONVERT_YET);
+        lines(
+            "let x = 2;",
+            "class C {",
+            "  constructor(x$jscomp$1) {",
+            "    this.y = () => {",
+            "      class Foo {",
+            "        constructor() {",
+            "          this.z = x;",
+            "        }",
+            "      }",
+            "    };",
+            "  }",
+            "}"));
   }
 
   // Added when fixing transpilation of real-world code that passed a class expression to a