Infer setter parameter types in strong mode

This change also decouples setters and getters, which matches what DDC currently does. The benefit is they're inferred as independent methods, which allows code reuse between method/getter/setters. It also prevents ordering issues that might occur otherwise (i.e. if getter inference considers setter's type, and setter inference considers the getter's type).

More context: dart-archive/dev_compiler#342 (comment)

[email protected]

Review URL: https://codereview.chromium.org//1359243003 .

Author: John Messerly

pkg/analyzer/lib/src/task/strong_mode.dart

@@ -230,37 +230,6 @@ class InstanceMemberInferrer {
         : matchingParameter.type;
   }
 
-  /**
-   * If the given [accessorElement] represents a non-synthetic instance getter
-   * for which no return type was provided, infer the return type of the getter.
-   */
-  void _inferAccessor(PropertyAccessorElement accessorElement) {
-    if (!accessorElement.isSynthetic &&
-        accessorElement.isGetter &&
-        !accessorElement.isStatic &&
-        accessorElement.hasImplicitReturnType) {
-      List<ExecutableElement> overriddenGetters = inheritanceManager
-          .lookupOverrides(
-              accessorElement.enclosingElement, accessorElement.name);
-      if (overriddenGetters.isNotEmpty && _onlyGetters(overriddenGetters)) {
-        DartType newType = _computeReturnType(overriddenGetters);
-        List<ExecutableElement> overriddenSetters = inheritanceManager
-            .lookupOverrides(
-                accessorElement.enclosingElement, accessorElement.name + '=');
-        PropertyAccessorElement setter = (accessorElement.enclosingElement
-            as ClassElement).getSetter(accessorElement.name);
-        if (setter != null) {
-          overriddenSetters.add(setter);
-        }
-        if (!_isCompatible(newType, overriddenSetters)) {
-          newType = typeProvider.dynamicType;
-        }
-        setReturnType(accessorElement, newType);
-        (accessorElement.variable as FieldElementImpl).type = newType;
-      }
-    }
-  }
-
   /**
    * Infer type information for all of the instance members in the given
    * [classElement].
@@ -290,8 +259,8 @@ class InstanceMemberInferrer {
         // Then infer the types for the members.
         //
         classElement.fields.forEach(_inferField);
-        classElement.accessors.forEach(_inferAccessor);
-        classElement.methods.forEach(_inferMethod);
+        classElement.accessors.forEach(_inferExecutable);
+        classElement.methods.forEach(_inferExecutable);
         classElement.hasBeenInferred = true;
       } finally {
         elementsBeingInferred.remove(classElement);
@@ -344,46 +313,81 @@ class InstanceMemberInferrer {
   }
 
   /**
-   * If the given [methodElement] represents a non-synthetic instance method
-   * for which no return type was provided, infer the return type of the method.
+   * If the given [element] represents a non-synthetic instance method,
+   * getter or setter, infer the return type and any parameter type(s) where
+   * they were not provided.
    */
-  void _inferMethod(MethodElement methodElement) {
-    if (methodElement.isSynthetic || methodElement.isStatic) {
+  void _inferExecutable(ExecutableElement element) {
+    if (element.isSynthetic || element.isStatic) {
       return;
     }
     List<ExecutableElement> overriddenMethods = null;
     //
     // Infer the return type.
     //
-    if (methodElement.hasImplicitReturnType) {
+    if (element.hasImplicitReturnType) {
       overriddenMethods = inheritanceManager.lookupOverrides(
-          methodElement.enclosingElement, methodElement.name);
-      if (overriddenMethods.isEmpty || !_onlyMethods(overriddenMethods)) {
+          element.enclosingElement, element.name);
+      if (overriddenMethods.isEmpty ||
+          !_allSameElementKind(element, overriddenMethods)) {
         return;
       }
-      MethodElementImpl element = methodElement as MethodElementImpl;
       setReturnType(element, _computeReturnType(overriddenMethods));
+      if (element is PropertyAccessorElement) {
+        _updateSyntheticVariableType(element);
+      }
     }
     //
     // Infer the parameter types.
     //
-    List<ParameterElement> parameters = methodElement.parameters;
+    List<ParameterElement> parameters = element.parameters;
     int length = parameters.length;
     for (int i = 0; i < length; ++i) {
       ParameterElement parameter = parameters[i];
       if (parameter is ParameterElementImpl && parameter.hasImplicitType) {
         if (overriddenMethods == null) {
           overriddenMethods = inheritanceManager.lookupOverrides(
-              methodElement.enclosingElement, methodElement.name);
+              element.enclosingElement, element.name);
         }
-        if (overriddenMethods.isEmpty || !_onlyMethods(overriddenMethods)) {
+        if (overriddenMethods.isEmpty ||
+            !_allSameElementKind(element, overriddenMethods)) {
           return;
         }
         parameter.type = _computeParameterType(parameter, i, overriddenMethods);
+        if (element is PropertyAccessorElement) {
+          _updateSyntheticVariableType(element);
+        }
       }
     }
   }
 
+  /**
+   * If the given [element] is a non-synthetic getter or setter, update its
+   * synthetic variable's type to match the getter's return type, or if no
+   * corresponding getter exists, use the setter's parameter type.
+   *
+   * In general, the type of the synthetic variable should not be used, because
+   * getters and setters are independent methods. But this logic matches what
+   * `TypeResolverVisitor.visitMethodDeclaration` would fill in there.
+   */
+  void _updateSyntheticVariableType(PropertyAccessorElement element) {
+    assert(!element.isSynthetic);
+    PropertyAccessorElement getter = element;
+    if (element.isSetter) {
+      // See if we can find any getter.
+      getter = element.correspondingGetter;
+    }
+    DartType newType;
+    if (getter != null) {
+      newType = getter.returnType;
+    } else if (element.isSetter && element.parameters.isNotEmpty) {
+      newType = element.parameters[0].type;
+    }
+    if (newType != null) {
+      (element.variable as VariableElementImpl).type = newType;
+    }
+  }
+
   /**
    * Infer type information for all of the instance members in the given
    * interface [type].
@@ -426,13 +430,9 @@ class InstanceMemberInferrer {
   /**
    * Return `true` if the list of [elements] contains only methods.
    */
-  bool _onlyMethods(List<ExecutableElement> elements) {
-    for (ExecutableElement element in elements) {
-      if (element is! MethodElement) {
-        return false;
-      }
-    }
-    return true;
+  bool _allSameElementKind(
+      ExecutableElement element, List<ExecutableElement> elements) {
+    return elements.every((e) => e.kind == element.kind);
   }
 }
 

pkg/analyzer/test/src/task/strong_mode_test.dart

@@ -458,6 +458,9 @@ class B extends A {
   var get $getterName => 1;
 }
 ''');
+    ClassElement classA = unit.getType('A');
+    FieldElement fieldA = classA.getField(getterName);
+    PropertyAccessorElement getterA = classA.getGetter(getterName);
     ClassElement classB = unit.getType('B');
     FieldElement fieldB = classB.getField(getterName);
     PropertyAccessorElement getterB = classB.getGetter(getterName);
@@ -466,8 +469,92 @@ class B extends A {
 
     inferrer.inferCompilationUnit(unit);
 
+    // Expected behavior is that the getter is inferred: getters and setters
+    // are treated as independent methods.
+    expect(fieldB.type, fieldA.type);
+    expect(getterB.returnType, getterA.returnType);
+  }
+
+  void test_inferCompilationUnit_setter_single() {
+    InstanceMemberInferrer inferrer = createInferrer;
+    String setterName = 'g';
+    CompilationUnitElement unit = resolve('''
+class A {
+  set $setterName(int x) {}
+}
+class B extends A {
+  set $setterName(x) {}
+}
+''');
+    ClassElement classA = unit.getType('A');
+    FieldElement fieldA = classA.getField(setterName);
+    PropertyAccessorElement setterA = classA.getSetter(setterName);
+    ClassElement classB = unit.getType('B');
+    FieldElement fieldB = classB.getField(setterName);
+    PropertyAccessorElement setterB = classB.getSetter(setterName);
     expect(fieldB.type.isDynamic, isTrue);
-    expect(getterB.returnType.isDynamic, isTrue);
+    expect(setterB.parameters[0].type.isDynamic, isTrue);
+
+    inferrer.inferCompilationUnit(unit);
+
+    expect(fieldB.type, fieldA.type);
+    expect(setterB.parameters[0].type, setterA.parameters[0].type);
+  }
+
+  void test_inferCompilationUnit_setter_single_generic() {
+    InstanceMemberInferrer inferrer = createInferrer;
+    String setterName = 'g';
+    CompilationUnitElement unit = resolve('''
+class A<E> {
+  set $setterName(E x) {}
+}
+class B<E> extends A<E> {
+  set $setterName(x) {}
+}
+''');
+    ClassElement classB = unit.getType('B');
+    DartType typeBE = classB.typeParameters[0].type;
+    FieldElement fieldB = classB.getField(setterName);
+    PropertyAccessorElement setterB = classB.getSetter(setterName);
+    expect(fieldB.type.isDynamic, isTrue);
+    expect(setterB.parameters[0].type.isDynamic, isTrue);
+
+    inferrer.inferCompilationUnit(unit);
+
+    expect(fieldB.type, typeBE);
+    expect(setterB.parameters[0].type, typeBE);
+  }
+
+  void test_inferCompilationUnit_setter_single_inconsistentAccessors() {
+    InstanceMemberInferrer inferrer = createInferrer;
+    String getterName = 'g';
+    CompilationUnitElement unit = resolve('''
+class A {
+  int get $getterName => 0;
+  set $getterName(String value) {}
+}
+class B extends A {
+  set $getterName(x) {}
+}
+''');
+    ClassElement classA = unit.getType('A');
+    FieldElement fieldA = classA.getField(getterName);
+    PropertyAccessorElement setterA = classA.getSetter(getterName);
+    ClassElement classB = unit.getType('B');
+    FieldElement fieldB = classB.getField(getterName);
+    PropertyAccessorElement setterB = classB.getSetter(getterName);
+    expect(fieldB.type.isDynamic, isTrue);
+    expect(setterB.parameters[0].type.isDynamic, isTrue);
+
+    inferrer.inferCompilationUnit(unit);
+
+    // Expected behavior is that the getter is inferred: getters and setters
+    // are treated as independent methods.
+    expect(setterB.parameters[0].type, setterA.parameters[0].type);
+
+    // Note that B's synthetic field type will be String. This matches what
+    // resolver would do if we explicitly typed the parameter as 'String'
+    expect(fieldB.type, setterB.parameters[0].type);
   }
 
   void test_inferCompilationUnit_invalid_inheritanceCycle() {