[cfe] Handle duplicate getables and conflicting setables

Change-Id: Iea3cb2d74799f891367c27e1d5a460aa6e7ef803
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183661
Commit-Queue: Johnni Winther <[email protected]>
Reviewed-by: Dmitry Stefantsov <[email protected]>

Author: johnniwinther

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

@@ -2726,7 +2726,12 @@ class BodyBuilder extends ScopeListener<JumpTarget>
   @override
   void handleAssignmentExpression(Token token) {
     assert(checkState(token, [
-      unionOfKinds(<ValueKind>[ValueKinds.Expression, ValueKinds.Generator]),
+      unionOfKinds(<ValueKind>[
+        ValueKinds.Expression,
+        ValueKinds.Generator,
+        // TODO(johnniwinther): Avoid problem builders here.
+        ValueKinds.ProblemBuilder
+      ]),
       unionOfKinds(<ValueKind>[
         ValueKinds.Expression, ValueKinds.Generator,
         // TODO(johnniwinther): Avoid problem builders here.

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

@@ -892,55 +892,69 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
     assert(checkForMethodVsSetterConflict != null);
 
     scope.forEachLocalSetter((String name, MemberBuilder setter) {
-      Builder member = scope.lookupLocalMember(name, setter: false);
-      if (member == null) {
+      Builder getable = scope.lookupLocalMember(name, setter: false);
+      if (getable == null) {
         // Setter without getter.
         return;
       }
-      MemberBuilderImpl setterBuilder = setter;
-      do {
-        bool conflict = checkForInstanceVsStaticConflict &&
-            member.isDeclarationInstanceMember !=
-                setterBuilder.isDeclarationInstanceMember;
-        if (member is FieldBuilder) {
-          if (member.isAssignable) {
+
+      bool isConflictingSetter = false;
+      Set<Builder> conflictingGetables = {};
+      for (Builder currentGetable = getable;
+          currentGetable != null;
+          currentGetable = currentGetable.next) {
+        if (currentGetable is FieldBuilder) {
+          if (currentGetable.isAssignable) {
             // Setter with writable field.
-            setterBuilder.isConflictingSetter = true;
-            conflict = true;
+            isConflictingSetter = true;
+            conflictingGetables.add(currentGetable);
           }
-        } else if (checkForMethodVsSetterConflict && !member.isGetter) {
+        } else if (checkForMethodVsSetterConflict && !currentGetable.isGetter) {
           // Setter with method.
-          conflict = true;
+          conflictingGetables.add(currentGetable);
+        }
+      }
+      for (MemberBuilderImpl currentSetter = setter;
+          currentSetter != null;
+          currentSetter = currentSetter.next) {
+        bool conflict = conflictingGetables.isNotEmpty;
+        for (Builder currentGetable = getable;
+            currentGetable != null;
+            currentGetable = currentGetable.next) {
+          if (checkForInstanceVsStaticConflict &&
+              currentGetable.isDeclarationInstanceMember !=
+                  currentSetter.isDeclarationInstanceMember) {
+            conflict = true;
+            conflictingGetables.add(currentGetable);
+          }
+        }
+        if (isConflictingSetter) {
+          currentSetter.isConflictingSetter = true;
         }
         if (conflict) {
-          if (setterBuilder.isConflictingSetter) {
+          if (currentSetter.isConflictingSetter) {
             sourceLibraryBuilder.addProblem(
                 templateConflictsWithImplicitSetter.withArguments(name),
-                setterBuilder.charOffset,
+                currentSetter.charOffset,
                 noLength,
-                setterBuilder.fileUri);
-            // TODO(ahe): Context argument to previous message?
-            sourceLibraryBuilder.addProblem(
-                templateConflictsWithSetter.withArguments(name),
-                member.charOffset,
-                noLength,
-                member.fileUri);
+                currentSetter.fileUri);
           } else {
             sourceLibraryBuilder.addProblem(
                 templateConflictsWithMember.withArguments(name),
-                setterBuilder.charOffset,
-                noLength,
-                setterBuilder.fileUri);
-            // TODO(ahe): Context argument to previous message?
-            sourceLibraryBuilder.addProblem(
-                templateConflictsWithSetter.withArguments(name),
-                member.charOffset,
+                currentSetter.charOffset,
                 noLength,
-                member.fileUri);
+                currentSetter.fileUri);
           }
         }
-        setterBuilder = setterBuilder.next;
-      } while (setterBuilder != null);
+      }
+      for (Builder conflictingGetable in conflictingGetables) {
+        // TODO(ahe): Context argument to previous message?
+        sourceLibraryBuilder.addProblem(
+            templateConflictsWithSetter.withArguments(name),
+            conflictingGetable.charOffset,
+            noLength,
+            conflictingGetable.fileUri);
+      }
     });
   }
 

pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart.outline.expect

@@ -241,12 +241,16 @@ library;
 //   set Foo {
 //       ^
 //
+// pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:23:7: Error: Conflicts with the implicit setter of the field 'Foo'.
+//   set Foo => 0;
+//       ^
+//
 // pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:40:10: Error: Conflicts with setter 'Foo'.
 //   int A, Foo, B;
 //          ^
 //
-// pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:23:7: Error: Conflicts with the implicit setter of the field 'Foo'.
-//   set Foo => 0;
+// pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:39:7: Error: Conflicts with setter 'Foo'.
+//   int Foo;
 //       ^
 //
 import self as self;

pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart.strong.expect

@@ -241,12 +241,16 @@ library;
 //   set Foo {
 //       ^
 //
+// pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:23:7: Error: Conflicts with the implicit setter of the field 'Foo'.
+//   set Foo => 0;
+//       ^
+//
 // pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:40:10: Error: Conflicts with setter 'Foo'.
 //   int A, Foo, B;
 //          ^
 //
-// pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:23:7: Error: Conflicts with the implicit setter of the field 'Foo'.
-//   set Foo => 0;
+// pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:39:7: Error: Conflicts with setter 'Foo'.
+//   int Foo;
 //       ^
 //
 // pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:5:13: Error: 'initializer' isn't an instance field of this class.

pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart.strong.transformed.expect

@@ -241,12 +241,16 @@ library;
 //   set Foo {
 //       ^
 //
+// pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:23:7: Error: Conflicts with the implicit setter of the field 'Foo'.
+//   set Foo => 0;
+//       ^
+//
 // pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:40:10: Error: Conflicts with setter 'Foo'.
 //   int A, Foo, B;
 //          ^
 //
-// pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:23:7: Error: Conflicts with the implicit setter of the field 'Foo'.
-//   set Foo => 0;
+// pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:39:7: Error: Conflicts with setter 'Foo'.
+//   int Foo;
 //       ^
 //
 // pkg/front_end/testcases/general/error_recovery/constructor_recovery_bad_name_general.crash_dart:5:13: Error: 'initializer' isn't an instance field of this class.

pkg/front_end/testcases/nnbd/field_vs_setter.dart

@@ -9,6 +9,14 @@ int? topLevelFieldAndDuplicateSetter;
 void set topLevelFieldAndDuplicateSetter(int? value) {}
 void set topLevelFieldAndDuplicateSetter(int? value) {}
 
+int? duplicateTopLevelFieldAndSetter1;
+final int? duplicateTopLevelFieldAndSetter1 = null;
+void set duplicateTopLevelFieldAndSetter1(int? value) {}
+
+final int? duplicateTopLevelFieldAndSetter2 = null;
+int? duplicateTopLevelFieldAndSetter2;
+void set duplicateTopLevelFieldAndSetter2(int? value) {}
+
 late final int? topLevelLateFinalFieldAndSetter;
 void set topLevelLateFinalFieldAndSetter(int? value) {}
 
@@ -24,6 +32,14 @@ class Class {
   void set instanceFieldAndDuplicateSetter(int? value) {}
   void set instanceFieldAndDuplicateSetter(int? value) {}
 
+  int? duplicateInstanceFieldAndSetter1;
+  final int? duplicateInstanceFieldAndSetter1 = null;
+  void set duplicateInstanceFieldAndSetter1(int? value) {}
+
+  final int? duplicateInstanceFieldAndSetter2 = null;
+  int? duplicateInstanceFieldAndSetter2;
+  void set duplicateInstanceFieldAndSetter2(int? value) {}
+
   late final int? instanceLateFinalFieldAndSetter;
   void set instanceLateFinalFieldAndSetter(int? value) {}
 
@@ -38,6 +54,14 @@ class Class {
   static void set staticFieldAndDuplicateSetter(int? value) {}
   static void set staticFieldAndDuplicateSetter(int? value) {}
 
+  static int? duplicateStaticFieldAndSetter1;
+  static final int? duplicateStaticFieldAndSetter1 = null;
+  static void set duplicateStaticFieldAndSetter1(int? value) {}
+
+  static final int? duplicateStaticFieldAndSetter2 = null;
+  static int? duplicateStaticFieldAndSetter2;
+  static void set duplicateStaticFieldAndSetter2(int? value) {}
+
   static late final int? staticLateFinalFieldAndSetter;
   static void set staticLateFinalFieldAndSetter(int? value) {}
 
@@ -58,6 +82,22 @@ class Class {
   int? instanceFieldAndStaticDuplicateSetter;
   static void set instanceFieldAndStaticDuplicateSetter(int? value) {}
   static void set instanceFieldAndStaticDuplicateSetter(int? value) {}
+
+  int? duplicateInstanceFieldAndStaticSetter1;
+  final int? duplicateInstanceFieldAndStaticSetter1 = null;
+  static void set duplicateInstanceFieldAndStaticSetter1(int? value) {}
+
+  final int? duplicateInstanceFieldAndStaticSetter2 = null;
+  int? duplicateInstanceFieldAndStaticSetter2;
+  static void set duplicateInstanceFieldAndStaticSetter2(int? value) {}
+
+  static int? duplicateStaticFieldAndInstanceSetter1;
+  static final int? duplicateStaticFieldAndInstanceSetter1 = null;
+  void set duplicateStaticFieldAndInstanceSetter1(int? value) {}
+
+  static final int? duplicateStaticFieldAndInstanceSetter2 = null;
+  static int? duplicateStaticFieldAndInstanceSetter2;
+  void set duplicateStaticFieldAndInstanceSetter2(int? value) {}
 }
 
 extension Extension on int? {
@@ -68,13 +108,29 @@ extension Extension on int? {
   void set extensionInstanceFieldAndDuplicateSetter(int? value) {}
   void set extensionInstanceFieldAndDuplicateSetter(int? value) {}
 
+  int? duplicateExtensionInstanceFieldAndSetter1;
+  final int? duplicateExtensionInstanceFieldAndSetter1 = null;
+  void set duplicateExtensionInstanceFieldAndSetter1(int? value) {}
+
+  final int? duplicateExtensionInstanceFieldAndSetter2 = null;
+  int? duplicateExtensionInstanceFieldAndSetter2;
+  void set duplicateExtensionInstanceFieldAndSetter2(int? value) {}
+
   static int? extensionStaticFieldAndSetter;
   static void set extensionStaticFieldAndSetter(int? value) {}
 
   static int? extensionStaticFieldAndDuplicateSetter;
   static void set extensionStaticFieldAndDuplicateSetter(int? value) {}
   static void set extensionStaticFieldAndDuplicateSetter(int? value) {}
 
+  static int? duplicateExtensionStaticFieldAndSetter1;
+  static final int? duplicateExtensionStaticFieldAndSetter1 = null;
+  static void set duplicateExtensionStaticFieldAndSetter1(int? value) {}
+
+  static final int? duplicateExtensionStaticFieldAndSetter2 = null;
+  static int? duplicateExtensionStaticFieldAndSetter2;
+  static void set duplicateExtensionStaticFieldAndSetter2(int? value) {}
+
   static late final int? extensionStaticLateFinalFieldAndSetter;
   static void set extensionStaticLateFinalFieldAndSetter(int? value) {}
 
@@ -95,6 +151,22 @@ extension Extension on int? {
   int? extensionInstanceFieldAndStaticDuplicateSetter;
   static void set extensionInstanceFieldAndStaticDuplicateSetter(int? value) {}
   static void set extensionInstanceFieldAndStaticDuplicateSetter(int? value) {}
+
+  int? duplicateExtensionInstanceFieldAndStaticSetter1;
+  final int? duplicateExtensionInstanceFieldAndStaticSetter1 = null;
+  static void set duplicateExtensionInstanceFieldAndStaticSetter1(int? value) {}
+
+  final int? duplicateExtensionInstanceFieldAndStaticSetter2 = null;
+  int? duplicateExtensionInstanceFieldAndStaticSetter2;
+  static void set duplicateExtensionInstanceFieldAndStaticSetter2(int? value) {}
+
+  static int? duplicateExtensionStaticFieldAndInstanceSetter1;
+  static final int? duplicateExtensionStaticFieldAndInstanceSetter1 = null;
+  void set duplicateExtensionStaticFieldAndInstanceSetter1(int? value) {}
+
+  static final int? duplicateExtensionStaticFieldAndInstanceSetter2 = null;
+  static int? duplicateExtensionStaticFieldAndInstanceSetter2;
+  void set duplicateExtensionStaticFieldAndInstanceSetter2(int? value) {}
 }
 
 test() {
@@ -103,6 +175,8 @@ test() {
   topLevelLateFinalFieldAndSetter = topLevelLateFinalFieldAndSetter;
   topLevelLateFinalFieldAndDuplicateSetter =
       topLevelLateFinalFieldAndDuplicateSetter;
+  duplicateTopLevelFieldAndSetter1 = duplicateTopLevelFieldAndSetter1;
+  duplicateTopLevelFieldAndSetter2 = duplicateTopLevelFieldAndSetter2;
 
   var c = new Class();
 
@@ -111,12 +185,18 @@ test() {
   c.instanceLateFinalFieldAndSetter = c.instanceLateFinalFieldAndSetter;
   c.instanceLateFinalFieldAndDuplicateSetter =
       c.instanceLateFinalFieldAndDuplicateSetter;
+  c.duplicateInstanceFieldAndStaticSetter1 =
+      c.duplicateInstanceFieldAndStaticSetter1;
+  c.duplicateInstanceFieldAndStaticSetter2 =
+      c.duplicateInstanceFieldAndStaticSetter2;
 
   Class.staticFieldAndSetter = Class.staticFieldAndSetter;
   Class.staticFieldAndDuplicateSetter = Class.staticFieldAndDuplicateSetter;
   Class.staticLateFinalFieldAndSetter = Class.staticLateFinalFieldAndSetter;
   Class.staticLateFinalFieldAndDuplicateSetter =
       Class.staticLateFinalFieldAndDuplicateSetter;
+  Class.duplicateStaticFieldAndSetter1 = Class.duplicateStaticFieldAndSetter1;
+  Class.duplicateStaticFieldAndSetter2 = Class.duplicateStaticFieldAndSetter2;
 
   c.staticFieldAndInstanceSetter = Class.staticFieldAndInstanceSetter;
   Class.staticFieldAndInstanceSetter = Class.staticFieldAndInstanceSetter;
@@ -134,10 +214,30 @@ test() {
   c.instanceFieldAndStaticDuplicateSetter =
       c.instanceFieldAndStaticDuplicateSetter;
 
+  c.duplicateStaticFieldAndInstanceSetter1 =
+      Class.duplicateStaticFieldAndInstanceSetter1;
+  Class.duplicateStaticFieldAndInstanceSetter1 =
+      Class.duplicateStaticFieldAndInstanceSetter1;
+
+  c.duplicateStaticFieldAndInstanceSetter2 =
+      Class.duplicateStaticFieldAndInstanceSetter2;
+  Class.duplicateStaticFieldAndInstanceSetter2 =
+      Class.duplicateStaticFieldAndInstanceSetter2;
+
+  Class.duplicateInstanceFieldAndStaticSetter1 =
+      0.duplicateInstanceFieldAndStaticSetter1;
+  Class.duplicateInstanceFieldAndStaticSetter2 =
+      0.duplicateInstanceFieldAndStaticSetter2;
+
   0.extensionInstanceFieldAndSetter = 0.extensionInstanceFieldAndSetter;
   0.extensionInstanceFieldAndDuplicateSetter =
       0.extensionInstanceFieldAndDuplicateSetter;
 
+  0.duplicateExtensionInstanceFieldAndSetter1 =
+      0.duplicateExtensionInstanceFieldAndSetter1;
+  0.duplicateExtensionInstanceFieldAndSetter2 =
+      0.duplicateExtensionInstanceFieldAndSetter2;
+
   Extension.extensionStaticFieldAndSetter =
       Extension.extensionStaticFieldAndSetter;
 
@@ -149,6 +249,10 @@ test() {
 
   Extension.extensionStaticLateFinalFieldAndDuplicateSetter =
       Extension.extensionStaticLateFinalFieldAndDuplicateSetter;
+  Extension.duplicateExtensionStaticFieldAndSetter1 =
+      Extension.duplicateExtensionStaticFieldAndSetter1;
+  Extension.duplicateExtensionStaticFieldAndSetter2 =
+      Extension.duplicateExtensionStaticFieldAndSetter2;
 
   0.extensionStaticFieldAndInstanceSetter =
       Extension.extensionStaticFieldAndInstanceSetter;
@@ -169,6 +273,26 @@ test() {
       0.extensionInstanceFieldAndStaticDuplicateSetter;
   0.extensionInstanceFieldAndStaticDuplicateSetter =
       0.extensionInstanceFieldAndStaticDuplicateSetter;
+
+  Extension.duplicateExtensionInstanceFieldAndStaticSetter1 =
+      0.duplicateExtensionInstanceFieldAndStaticSetter1;
+  0.duplicateExtensionInstanceFieldAndStaticSetter1 =
+      0.duplicateExtensionInstanceFieldAndStaticSetter1;
+
+  Extension.duplicateExtensionInstanceFieldAndStaticSetter2 =
+      0.duplicateExtensionInstanceFieldAndStaticSetter2;
+  0.duplicateExtensionInstanceFieldAndStaticSetter2 =
+      0.duplicateExtensionInstanceFieldAndStaticSetter2;
+
+  Extension.duplicateExtensionStaticFieldAndInstanceSetter1 =
+      Extension.duplicateExtensionStaticFieldAndInstanceSetter1;
+  0.duplicateExtensionStaticFieldAndInstanceSetter1 =
+      Extension.duplicateExtensionStaticFieldAndInstanceSetter1;
+
+  Extension.duplicateExtensionStaticFieldAndInstanceSetter2 =
+      Extension.duplicateExtensionStaticFieldAndInstanceSetter2;
+  0.duplicateExtensionStaticFieldAndInstanceSetter2 =
+      Extension.duplicateExtensionStaticFieldAndInstanceSetter2;
 }
 
 main() {}