[cfe] Check getter/setter types on top level and extension members

Closes #43714

Change-Id: I07ee20cb3a65758dda2139e965aeb4fb36cf38c6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166628
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>

pkg/front_end/lib/src/fasta/kernel/class_hierarchy_builder.dart

@@ -2879,8 +2879,7 @@ class DelayedGetterSetterCheck implements DelayedCheck {
 
   void check(ClassHierarchyBuilder hierarchy) {
     classBuilder.checkGetterSetter(hierarchy.types, getter.getMember(hierarchy),
-        setter.getMember(hierarchy),
-        isInterfaceCheck: !classBuilder.isMixinApplication);
+        setter.getMember(hierarchy));
   }
 }
 

pkg/front_end/lib/src/fasta/source/source_class_builder.dart

@@ -1126,8 +1126,7 @@ class SourceClassBuilder extends ClassBuilderImpl
     // TODO(ahe): Handle other cases: accessors, operators, and fields.
   }
 
-  void checkGetterSetter(Types types, Member getter, Member setter,
-      {bool isInterfaceCheck = false}) {
+  void checkGetterSetter(Types types, Member getter, Member setter) {
     if (getter == setter) {
       return;
     }

pkg/front_end/lib/src/fasta/source/source_extension_builder.dart

@@ -255,6 +255,14 @@ class SourceExtensionBuilder extends ExtensionBuilderImpl {
       } else if (builder is ProcedureBuilder) {
         // Check procedures
         library.checkTypesInProcedureBuilder(builder, typeEnvironment);
+        if (builder.isGetter) {
+          Builder setterDeclaration =
+              scope.lookupLocalMember(builder.name, setter: true);
+          if (setterDeclaration != null) {
+            library.checkGetterSetterTypes(
+                builder, setterDeclaration, typeEnvironment);
+          }
+        }
       } else {
         assert(false, "Unexpected member: $builder.");
       }

pkg/front_end/lib/src/fasta/source/source_library_builder.dart

@@ -67,7 +67,7 @@ import 'package:kernel/src/bounds_checks.dart'
         findTypeArgumentIssuesForInvocation,
         getGenericTypeName;
 
-import 'package:kernel/type_algebra.dart' show substitute;
+import 'package:kernel/type_algebra.dart' show Substitution, substitute;
 
 import 'package:kernel/type_environment.dart'
     show SubtypeCheckMode, TypeEnvironment;
@@ -1610,6 +1610,100 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
     return typeVariablesByName;
   }
 
+  void checkGetterSetterTypes(ProcedureBuilder getterBuilder,
+      ProcedureBuilder setterBuilder, TypeEnvironment typeEnvironment) {
+    DartType getterType;
+    List<TypeParameter> getterExtensionTypeParameters;
+    if (getterBuilder.isExtensionInstanceMember) {
+      // An extension instance getter
+      //
+      //     extension E<T> on A {
+      //       T get property => ...
+      //     }
+      //
+      // is encoded as a top level method
+      //
+      //   T# E#get#property<T#>(A #this) => ...
+      //
+      Procedure procedure = getterBuilder.procedure;
+      getterType = procedure.function.returnType;
+      getterExtensionTypeParameters = procedure.function.typeParameters;
+    } else {
+      getterType = getterBuilder.procedure.getterType;
+    }
+    DartType setterType;
+    if (setterBuilder.isExtensionInstanceMember) {
+      // An extension instance setter
+      //
+      //     extension E<T> on A {
+      //       void set property(T value) { ... }
+      //     }
+      //
+      // is encoded as a top level method
+      //
+      //   void E#set#property<T#>(A #this, T# value) { ... }
+      //
+      Procedure procedure = setterBuilder.procedure;
+      setterType = procedure.function.positionalParameters[1].type;
+      if (getterExtensionTypeParameters != null &&
+          getterExtensionTypeParameters.isNotEmpty) {
+        // We substitute the setter type parameters for the getter type
+        // parameters to check them below in a shared context.
+        List<TypeParameter> setterExtensionTypeParameters =
+            procedure.function.typeParameters;
+        assert(getterExtensionTypeParameters.length ==
+            setterExtensionTypeParameters.length);
+        setterType = Substitution.fromPairs(
+                setterExtensionTypeParameters,
+                new List<DartType>.generate(
+                    getterExtensionTypeParameters.length,
+                    (int index) => new TypeParameterType.forAlphaRenaming(
+                        setterExtensionTypeParameters[index],
+                        getterExtensionTypeParameters[index])))
+            .substituteType(setterType);
+      }
+    } else {
+      setterType = setterBuilder.procedure.setterType;
+    }
+
+    if (getterType is InvalidType || setterType is InvalidType) {
+      // Don't report a problem as something else is wrong that has already
+      // been reported.
+    } else {
+      bool isValid = typeEnvironment.isSubtypeOf(
+          getterType,
+          setterType,
+          library.isNonNullableByDefault
+              ? SubtypeCheckMode.withNullabilities
+              : SubtypeCheckMode.ignoringNullabilities);
+      if (!isValid && !library.isNonNullableByDefault) {
+        // Allow assignability in legacy libraries.
+        isValid = typeEnvironment.isSubtypeOf(
+            setterType, getterType, SubtypeCheckMode.ignoringNullabilities);
+      }
+      if (!isValid) {
+        String getterMemberName = getterBuilder.fullNameForErrors;
+        String setterMemberName = setterBuilder.fullNameForErrors;
+        Template<Message Function(DartType, String, DartType, String, bool)>
+            template = library.isNonNullableByDefault
+                ? templateInvalidGetterSetterType
+                : templateInvalidGetterSetterTypeLegacy;
+        addProblem(
+            template.withArguments(getterType, getterMemberName, setterType,
+                setterMemberName, library.isNonNullableByDefault),
+            getterBuilder.charOffset,
+            getterBuilder.name.length,
+            getterBuilder.fileUri,
+            context: [
+              templateInvalidGetterSetterTypeSetterContext
+                  .withArguments(setterMemberName)
+                  .withLocation(setterBuilder.fileUri, setterBuilder.charOffset,
+                      setterBuilder.name.length)
+            ]);
+      }
+    }
+  }
+
   void addExtensionDeclaration(
       String documentationComment,
       List<MetadataBuilder> metadata,
@@ -3579,6 +3673,14 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
         checkTypesInField(declaration, typeEnvironment);
       } else if (declaration is ProcedureBuilder) {
         checkTypesInProcedureBuilder(declaration, typeEnvironment);
+        if (declaration.isGetter) {
+          Builder setterDeclaration =
+              scope.lookupLocalMember(declaration.name, setter: true);
+          if (setterDeclaration != null) {
+            checkGetterSetterTypes(
+                declaration, setterDeclaration, typeEnvironment);
+          }
+        }
       } else if (declaration is SourceClassBuilder) {
         declaration.checkTypesInOutline(typeEnvironment);
       } else if (declaration is SourceExtensionBuilder) {

pkg/front_end/test/spell_checking_list_tests.txt

@@ -508,6 +508,14 @@ producer
 profile
 profiler
 propagated
+property2a
+property2b
+property4a
+property4b
+property5a
+property5b
+property8a
+property8b
 protected
 proved
 provider

pkg/front_end/testcases/extensions/extension_member_conflict.dart

@@ -0,0 +1,66 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+extension Extension<T> on int {
+  int get duplicateInstanceGetter => 0;
+  int get duplicateInstanceGetter => 0;
+
+  void set duplicateInstanceSetter(int value) {}
+  void set duplicateInstanceSetter(int value) {}
+
+  void duplicateInstanceMethod() {}
+  void duplicateInstanceMethod() {}
+
+  static int duplicateStaticField = 0;
+  static int duplicateStaticField = 0;
+
+  static int get duplicateStaticGetter => 0;
+  static int get duplicateStaticGetter => 0;
+
+  static void set duplicateStaticSetter(int value) {}
+  static void set duplicateStaticSetter(int value) {}
+
+  static void duplicateStaticMethod() {}
+  static void duplicateStaticMethod() {}
+
+  int get duplicateInstanceGetterPlusSetter => 0;
+  int get duplicateInstanceGetterPlusSetter => 0;
+  void set duplicateInstanceGetterPlusSetter(int value) {}
+
+  int get duplicateInstanceSetterPlusGetter => 0;
+  void set duplicateInstanceSetterPlusGetter(int value) {}
+  void set duplicateInstanceSetterPlusGetter(int value) {}
+
+  int get duplicateInstanceGetterAndSetter => 0;
+  int get duplicateInstanceGetterAndSetter => 0;
+  void set duplicateInstanceGetterAndSetter(int value) {}
+  void set duplicateInstanceGetterAndSetter(int value) {}
+
+  static int get duplicateStaticGetterPlusSetter => 0;
+  static int get duplicateStaticGetterPlusSetter => 0;
+  static void set duplicateStaticGetterPlusSetter(int value) {}
+
+  static int get duplicateStaticSetterPlusGetter => 0;
+  static void set duplicateStaticSetterPlusGetter(int value) {}
+  static void set duplicateStaticSetterPlusGetter(int value) {}
+
+  static int get duplicateStaticGetterAndSetter => 0;
+  static int get duplicateStaticGetterAndSetter => 0;
+  static void set duplicateStaticGetterAndSetter(int value) {}
+  static void set duplicateStaticGetterAndSetter(int value) {}
+
+  int get instanceGetterAndStaticSetter => 0;
+  static void set instanceGetterAndStaticSetter(int value) {}
+
+  static int get instanceSetterAndStaticGetter => 0;
+  void set instanceSetterAndStaticGetter(int value) {}
+
+  int get instanceGetterAndStaticField => 0;
+  static int instanceGetterAndStaticField = 0;
+
+  void set instanceSetterAndStaticField(int value) {}
+  static final int instanceGetterAndStaticField = 0;
+}
+
+main() {}