Modify TypedScopeCreator to not throw an error when there's a getter or member function in a public field.

PiperOrigin-RevId: 550633891

Author: Closure Team

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

@@ -789,8 +789,7 @@ CompilerInput getCompilerInput() {
       return compiler.getInput(inputId);
     }
 
-    @Nullable
-    Module getModule() {
+    @Nullable Module getModule() {
       return this.currentScope.getModule();
     }
 
@@ -2358,8 +2357,7 @@ private TypedScope getLValueRootScope(Node n) {
      * @param declaredRValueTypeSupplier A supplier for the declared type of the rvalue, used for
      *     destructuring declarations where we have to do additional work on the rvalue.
      */
-    @Nullable
-    JSType getDeclaredType(
+    @Nullable JSType getDeclaredType(
         JSDocInfo info,
         Node lValue,
         @Nullable Node rValue,
@@ -3244,7 +3242,7 @@ void visitPostorder(NodeTraversal t, Node n, Node parent) {
 
   /**
    * Scope builder subclass for function scopes, which only contain bleeding function names and
-   * parameter names. The main function body is handled by the a NormalScopeBuilder on the function
+   * parameter names. The main function body is handled by the NormalScopeBuilder on the function
    * block.
    */
   private final class FunctionScopeBuilder extends AbstractScopeBuilder {
@@ -3523,11 +3521,13 @@ void visitPostorder(NodeTraversal t, Node n, Node parent) {
             .defineSlot();
       } else if (NodeUtil.isEs6ConstructorMemberFunctionDef(n)) {
         // Ignore "constructor" since it has special handling in `createClassTypeFromNodes()`.
-      } else if (n.isMemberFunctionDef()) {
+      } else if (n.isMemberFunctionDef() && parent.isClassMembers()) {
         defineMemberFunction(n);
       } else if (n.isMemberFieldDef()) {
+        // public fields are roots of their own scope so the parent doesn't get passed into
+        // visitPostorder
         defineMemberField(n);
-      } else if (n.isGetterDef() || n.isSetterDef()) {
+      } else if ((n.isGetterDef() || n.isSetterDef()) && parent.isClassMembers()) {
         defineGetterSetter(n);
       }
     }
@@ -3594,8 +3594,10 @@ void defineGetterSetter(Node n) {
      * nonstatic member.
      */
     private ObjectType determineOwnerTypeForClassMember(Node member) {
-      // MEMBER_FUNCTION_DEF -> CLASS_MEMBERS -> CLASS  or
-      // GETTER_DEF -> CLASS_MEMBERS -> CLASS
+      // MEMBER_FUNCTION_DEF -> CLASS_MEMBERS -> CLASS or
+      // MEMBER_FIELD_DEF -> CLASS_MEMBERS -> CLASS or
+      // GETTER_DEF -> CLASS_MEMBERS -> CLASS or
+      // SETTER_DEF -> CLASS_MEMBERS -> CLASS
       Node ownerNode = member.getGrandparent();
       checkState(ownerNode.isClass());
       FunctionType ownerType = ownerNode.getJSType().toMaybeFunctionType();

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

@@ -3273,11 +3273,11 @@ public void testClassDeclarationWithFieldThisAndSuper() {
             "  /** @type {number} */",
             "  a = 2;",
             "  /** @type {number} */",
-            "  b = this.b;",
+            "  b = this.a;",
             "}",
             "class Bar extends Foo {",
             "  /** @type {number} */",
-            "  c = super.a + 1;",
+            "  c = this.a + 1;", // Must use `this` and not `super` to access field of parent class
             "}"));
 
     FunctionType fooCtor = globalScope.getVar("Foo").getType().assertFunctionType();
@@ -3368,11 +3368,12 @@ public void testClassDeclarationWithStaticFieldThisAndSuper() {
             "  /** @type {number} */",
             "  static a = 2;",
             "  /** @type {number} */",
-            "  static b = this.b;",
+            "  static b = this.a;",
             "}",
             "class Bar extends Foo {",
             "  /** @type {number} */",
-            "  static c = super.a + 1;",
+            "  static c = this.a + 1;", // Must use `this` and not `super` to access field of parent
+            // class
             "}"));
 
     FunctionType fooCtor = globalScope.getVar("Foo").getType().assertFunctionType();
@@ -3391,6 +3392,104 @@ public void testClassDeclarationWithStaticFieldThisAndSuper() {
     assertType(barCtor.getPropertyType("c")).isNumber();
   }
 
+  @Test
+  public void testTypeOfThisInObjectLitInPublicField() {
+    // Make sure the `this` on the RHS of a class field declaration has the appropriate type, even
+    // when it appears within an object literal
+    testSame(
+        lines(
+            "class Foo {", //
+            "  x = 3;",
+            "  bar = {",
+            "    b: this.x",
+            "  };",
+            "}"));
+
+    FunctionType fooCtor = globalScope.getVar("Foo").getType().assertFunctionType();
+    ObjectType fooInstance = fooCtor.getInstanceType();
+    Node getProp =
+        fooInstance
+            .getPropertyNode("bar")
+            .getOnlyChild() // OBJECTLIT
+            .getOnlyChild() // STRING_KEY
+            .getOnlyChild(); // GETPROP
+    assertNode(getProp).hasJSTypeThat().isNumber();
+    Node thisNode = getProp.getOnlyChild();
+    assertNode(thisNode).hasJSTypeThat().isEqualTo(fooInstance);
+
+    testSame(
+        lines(
+            "class Foo {", //
+            "  static baz = {",
+            "    e: this",
+            "  };",
+            "}"));
+
+    fooCtor = globalScope.getVar("Foo").getType().assertFunctionType();
+    thisNode =
+        fooCtor
+            .getPropertyNode("baz")
+            .getOnlyChild() // OBJECTLIT
+            .getOnlyChild() // STRING_KEY
+            .getOnlyChild(); // THIS
+    assertNode(thisNode).hasJSTypeThat().isEqualTo(fooCtor);
+  }
+
+  @Test
+  public void testGetterInObjectLitInPublicField() {
+    // Previously a bug caused getters & setters in object literals on the RHS of a field
+    // declaration to be treated as if they belonged to the class.
+    //
+    // Make sure that `this` in an object literal getter or setter has the unknown type.
+    testSame(
+        lines(
+            "class Foo {", //
+            "  bar = {",
+            "    get baz() {",
+            "      X: this;",
+            "    }",
+            "  };",
+            "}"));
+    Node thisNode = getLabeledStatement("X").statementNode.getOnlyChild();
+    // We don't create unique types for object literals, so even though we know
+    // the getter will (probably) only be called on this object, we just
+    // use unknown for the type of `this`.
+    assertNode(thisNode).hasJSTypeThat().isUnknown();
+
+    testSame(
+        lines(
+            "class Foo {", //
+            "  static bar = {",
+            "    /** @this {string} */",
+            "    get baz() { Y: this; }",
+            "  };",
+            "}"));
+
+    thisNode = getLabeledStatement("Y").statementNode.getOnlyChild();
+    assertNode(thisNode).hasJSTypeThat().isString();
+  }
+
+  @Test
+  public void testMemberFunctionInObjectLitInPublicField() {
+    // Previously a bug caused methods in object literals on the RHS of a field declaration to be
+    // treated as if they were methods on the class.
+    //
+    // Make sure that `this` in an object literal method has the unknown type.
+    testSame(
+        lines(
+            "class Foo {", //
+            "  bar = {",
+            "    /** @this {number} */",
+            "    member() {",
+            "      X: this;",
+            "    }",
+            "  };",
+            "}"));
+
+    Node thisNode = getLabeledStatement("X").statementNode.getOnlyChild();
+    assertNode(thisNode).hasJSTypeThat().isNumber();
+  }
+
   @Test
   public void testClassDeclarationWithDeprecatedField() {
     testSame(