Typecheck implementations of @abstract Symbol-keyed properties/methods

Also simplify @interface override checks to use the new getOwnPropertyKeys() helper.

PiperOrigin-RevId: 790750515

Author: lauraharker

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

@@ -2499,19 +2499,19 @@ private boolean couldBeAConstructor(JSType type) {
   private void checkInterfaceConflictProperties(
       Node n,
       String functionName,
-      Map<String, ObjectType> properties,
-      Map<String, ObjectType> currentProperties,
+      Map<Property.Key, ObjectType> properties,
+      Map<Property.Key, ObjectType> currentProperties,
       ObjectType interfaceType) {
     ObjectType implicitProto = interfaceType.getImplicitPrototype();
-    Set<String> currentPropertyNames;
+    ImmutableSet<Property.Key> currentPropertyNames;
     if (implicitProto == null) {
       // This can be the case if interfaceType is proxy to a non-existent
       // object (which is a bad type annotation, but shouldn't crash).
       currentPropertyNames = ImmutableSet.of();
     } else {
-      currentPropertyNames = implicitProto.getOwnPropertyNames();
+      currentPropertyNames = implicitProto.getOwnPropertyKeys();
     }
-    for (String name : currentPropertyNames) {
+    for (Property.Key name : currentPropertyNames) {
       ObjectType oType = properties.get(name);
       currentProperties.put(name, interfaceType);
       if (oType != null) {
@@ -2531,7 +2531,7 @@ private void checkInterfaceConflictProperties(
                 n,
                 INCOMPATIBLE_EXTENDED_PROPERTY_TYPE,
                 functionName,
-                name,
+                name.humanReadableName(),
                 oType.toString(),
                 interfaceType.toString()));
       }
@@ -2695,8 +2695,8 @@ private void checkInterface(Node n, FunctionType functionType) {
     // Check whether the extended interfaces have any conflicts
     if (functionType.getExtendedInterfacesCount() > 1) {
       // Only check when extending more than one interfaces
-      LinkedHashMap<String, ObjectType> properties = new LinkedHashMap<>();
-      LinkedHashMap<String, ObjectType> currentProperties = new LinkedHashMap<>();
+      LinkedHashMap<Property.Key, ObjectType> properties = new LinkedHashMap<>();
+      LinkedHashMap<Property.Key, ObjectType> currentProperties = new LinkedHashMap<>();
       for (ObjectType interfaceType : functionType.getExtendedInterfaces()) {
         currentProperties.clear();
         checkInterfaceConflictProperties(

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

@@ -52,12 +52,10 @@
 import com.google.javascript.rhino.jstype.JSType.SubtypingMode;
 import com.google.javascript.rhino.jstype.JSTypeNative;
 import com.google.javascript.rhino.jstype.JSTypeRegistry;
-import com.google.javascript.rhino.jstype.KnownSymbolType;
 import com.google.javascript.rhino.jstype.NamedType;
 import com.google.javascript.rhino.jstype.ObjectType;
 import com.google.javascript.rhino.jstype.Property;
 import com.google.javascript.rhino.jstype.Property.OwnedProperty;
-import com.google.javascript.rhino.jstype.Property.StringKey;
 import com.google.javascript.rhino.jstype.TemplateType;
 import com.google.javascript.rhino.jstype.TemplateTypeMap;
 import com.google.javascript.rhino.jstype.TemplateTypeReplacer;
@@ -950,20 +948,13 @@ void expectAllInterfaceProperties(Node n, FunctionType type) {
   private void expectInterfaceProperties(
       Node n, ObjectType instance, ObjectType ancestorInterface) {
     // Case: `/** @interface */ class Foo { constructor() { this.prop; } }`
-    for (String prop : ancestorInterface.getOwnPropertyNames()) {
-      expectInterfaceProperty(n, instance, ancestorInterface, new StringKey(prop));
-    }
-    for (KnownSymbolType symbol : ancestorInterface.getOwnPropertyKnownSymbols()) {
-      expectInterfaceProperty(n, instance, ancestorInterface, new Property.SymbolKey(symbol));
+    for (Property.Key prop : ancestorInterface.getOwnPropertyKeys()) {
+      expectInterfaceProperty(n, instance, ancestorInterface, prop);
     }
     if (ancestorInterface.getImplicitPrototype() != null) {
       // Case: `/** @interface */ class Foo { prop() { } }`
-      for (String prop : ancestorInterface.getImplicitPrototype().getOwnPropertyNames()) {
-        expectInterfaceProperty(n, instance, ancestorInterface, new StringKey(prop));
-      }
-      for (KnownSymbolType symbol :
-          ancestorInterface.getImplicitPrototype().getOwnPropertyKnownSymbols()) {
-        expectInterfaceProperty(n, instance, ancestorInterface, new Property.SymbolKey(symbol));
+      for (Property.Key prop : ancestorInterface.getImplicitPrototype().getOwnPropertyKeys()) {
+        expectInterfaceProperty(n, instance, ancestorInterface, prop);
       }
     }
   }
@@ -1049,15 +1040,15 @@ void checkPropertyType(
   void expectAbstractMethodsImplemented(Node n, FunctionType ctorType) {
     checkArgument(ctorType.isConstructor());
 
-    Map<String, ObjectType> abstractMethodSuperTypeMap = new LinkedHashMap<>();
+    Map<Property.Key, ObjectType> abstractMethodSuperTypeMap = new LinkedHashMap<>();
     FunctionType currSuperCtor = ctorType.getSuperClassConstructor();
     if (currSuperCtor == null || !currSuperCtor.isAbstract()) {
       return;
     }
 
     while (currSuperCtor != null && currSuperCtor.isAbstract()) {
       ObjectType superType = currSuperCtor.getInstanceType();
-      for (String prop : currSuperCtor.getPrototype().getOwnPropertyNames()) {
+      for (Property.Key prop : currSuperCtor.getPrototype().getOwnPropertyKeys()) {
         FunctionType maybeAbstractMethod = superType.findPropertyType(prop).toMaybeFunctionType();
         if (maybeAbstractMethod != null
             && maybeAbstractMethod.isAbstract()
@@ -1070,7 +1061,7 @@ void expectAbstractMethodsImplemented(Node n, FunctionType ctorType) {
 
     ObjectType instance = ctorType.getInstanceType();
     for (var entry : abstractMethodSuperTypeMap.entrySet()) {
-      String method = entry.getKey();
+      Property.Key method = entry.getKey();
       ObjectType superType = entry.getValue();
       FunctionType abstractMethod = instance.findPropertyType(method).toMaybeFunctionType();
       if (abstractMethod == null || abstractMethod.isAbstract()) {
@@ -1080,7 +1071,7 @@ void expectAbstractMethodsImplemented(Node n, FunctionType ctorType) {
             JSError.make(
                 n,
                 ABSTRACT_METHOD_NOT_IMPLEMENTED,
-                method,
+                method.humanReadableName(),
                 superType.toString(),
                 instance.toString()));
       }

src/com/google/javascript/rhino/jstype/ObjectType.java

@@ -54,7 +54,6 @@
 import com.google.javascript.rhino.JSDocInfo;
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.jstype.Property.OwnedProperty;
-import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 import org.jspecify.annotations.Nullable;
@@ -674,6 +673,22 @@ public Set<String> getOwnPropertyNames() {
     return getPropertyMap().getOwnPropertyNames();
   }
 
+  /**
+   * Returns the names of all the properties directly on this type.
+   */
+  public final ImmutableSet<Property.Key> getOwnPropertyKeys() {
+    Set<String> ownPropertyNames = this.getOwnPropertyNames();
+    Set<KnownSymbolType> ownSymbols = this.getOwnPropertyKnownSymbols();
+    ImmutableSet.Builder<Property.Key> ownPropertyKeys = ImmutableSet.builder();
+    for (String propertyName : ownPropertyNames) {
+      ownPropertyKeys.add(new Property.StringKey(propertyName));
+    }
+    for (KnownSymbolType symbol : ownSymbols) {
+      ownPropertyKeys.add(new Property.SymbolKey(symbol));
+    }
+    return ownPropertyKeys.build();
+  }
+
   /**
    * Checks whether the property's type is inferred.
    */

src/com/google/javascript/rhino/testing/TypeSubject.java

@@ -349,6 +349,10 @@ public void isConstructorFor(String name) {
           .isEqualTo(name);
     }
 
+    public void isAbstract() {
+      check("isAbstract()").that(actualFunctionType().isAbstract()).isTrue();
+    }
+
     public void hasPrimitiveId(ClosurePrimitive id) {
       check("getClosurePrimitive()")
           .that(actualNonNull().toMaybeFunctionType().getClosurePrimitive())

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

@@ -453,6 +453,21 @@ class C extends B {
         .run();
   }
 
+  @Test
+  public void testAbstractMethodInEs6NotImplemented_symbolProp() {
+    newTest()
+        .addSource(
+            """
+            /** @abstract */ class A {
+              /** @abstract */ [Symbol.iterator]() {}
+            }
+            class B extends A {}
+            """)
+        .addDiagnostic("property Symbol.iterator on abstract class A is not implemented by type B")
+        .includeDefaultExterns()
+        .run();
+  }
+
   @Test
   public void testAbstractMethodHandling1() {
     newTest()
@@ -609,6 +624,35 @@ function Baz() {}
         .run();
   }
 
+  @Test
+  public void testOverriddenReturnDoesntMatchOnAbstractClass_symbolProp() {
+    newTest()
+        .addSource(
+            """
+            /** @interface */ function IFoo() {}
+            /** @return {number} */ IFoo.prototype[Symbol.iterator] = function() {}
+            /** @constructor */ function Foo() {}
+            /** @return {string} */ Foo.prototype[Symbol.iterator] = function() {}
+            /** @constructor @extends {Foo} */ function Bar() {}
+            /**
+             * @constructor
+             * @abstract
+             * @extends {Bar}
+             * @implements {IFoo}
+             */
+            function Baz() {}
+            /** @return {string} */
+            function test() { return (/** @type {Baz} */ (null))[Symbol.iterator](); }
+            """)
+        .addDiagnostic(
+"""
+mismatch of the Symbol.iterator property on type Baz and the type of the property it overrides from interface IFoo
+original: function(this:IFoo): number
+override: function(this:Foo): string
+""")
+        .run();
+  }
+
   @Test
   public void testAbstractMethodCall1() {
     // Converted from Closure style "goog.base" super call

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

@@ -57,6 +57,7 @@
 import com.google.javascript.rhino.jstype.KnownSymbolType;
 import com.google.javascript.rhino.jstype.NamedType;
 import com.google.javascript.rhino.jstype.ObjectType;
+import com.google.javascript.rhino.jstype.Property;
 import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.HashMap;
@@ -3338,6 +3339,37 @@ class Foo {
         .toStringIsEqualTo("function(this:Foo): symbol");
   }
 
+  @Test
+  public void testClassWithWellKnownSymbolTypedMethod_abstract() {
+    testSame(
+        externs(
+            new TestExternsBuilder().addIterable().build(),
+            """
+            /** @const {symbol}  */
+            Symbol.dispose;
+            """),
+        srcs(
+            """
+            /** @abstract */
+            class Foo {
+              /**
+               * @param {string} x
+               * @param {number} y
+               * @abstract
+               */
+              [Symbol.iterator](x, y) {}
+            }
+            """));
+
+    FunctionType foo = (FunctionType) findNameType("Foo", globalScope);
+    KnownSymbolType symbolIteratorType =
+        (KnownSymbolType) findNameType("Symbol.iterator", globalScope);
+    assertType(foo.getInstanceType()).hasProperty(symbolIteratorType);
+    assertType(foo.getInstanceType().getPropertyType(new Property.SymbolKey(symbolIteratorType)))
+        .isFunctionTypeThat()
+        .isAbstract();
+  }
+
   @Test
   public void testClassDeclarationWithDucplicatePrototypeMethodAfterward() {
     // Given