[cfe] Special error message for illegal override of a setter by a field

This situation would produce an error referring to the erroneous type as a return type instead of a paremeter/field type. Change-Id: Ida72b0b85b448e3a05cadf38511e7e32d19986f1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115702 Reviewed-by: Dmitry Stefantsov <dmitryas@google.com> Commit-Queue: Aske Simon Christensen <askesc@google.com>
aam · Sep 9, 2019 · 41c5b81 · 41c5b81
1 parent e9c3eeb
commit 41c5b81
Show file tree

Hide file tree

Showing 12 changed files with 145 additions and 9 deletions.
diff --git a/pkg/front_end/lib/src/fasta/builder/class_builder.dart b/pkg/front_end/lib/src/fasta/builder/class_builder.dart
@@ -101,6 +101,7 @@ import '../fasta_codes.dart'
         templateOverrideMoreRequiredArguments,
         templateOverrideTypeMismatchParameter,
         templateOverrideTypeMismatchReturnType,
+        templateOverrideTypeMismatchSetter,
         templateOverrideTypeVariablesMismatch,
         templateRedirectingFactoryIncompatibleTypeArgument,
         templateRedirectionTargetNotFound,
@@ -1138,11 +1139,20 @@ abstract class ClassBuilder extends DeclarationBuilder {
       Message message;
       int fileOffset;
       if (declaredParameter == null) {
-        message = templateOverrideTypeMismatchReturnType.withArguments(
-            declaredMemberName,
-            declaredType,
-            interfaceType,
-            interfaceMemberName);
+        if (asIfDeclaredParameter) {
+          // Setter overridden by field
+          message = templateOverrideTypeMismatchSetter.withArguments(
+              declaredMemberName,
+              declaredType,
+              interfaceType,
+              interfaceMemberName);
+        } else {
+          message = templateOverrideTypeMismatchReturnType.withArguments(
+              declaredMemberName,
+              declaredType,
+              interfaceType,
+              interfaceMemberName);
+        }
         fileOffset = declaredMember.fileOffset;
       } else {
         message = templateOverrideTypeMismatchParameter.withArguments(

diff --git a/pkg/front_end/lib/src/fasta/fasta_codes_generated.dart b/pkg/front_end/lib/src/fasta/fasta_codes_generated.dart
@@ -8133,6 +8133,54 @@ Message _withArgumentsOverrideTypeMismatchReturnType(
       });
 }
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(
+        String name,
+        DartType _type,
+        DartType _type2,
+        String
+            name2)> templateOverrideTypeMismatchSetter = const Template<
+        Message Function(
+            String name, DartType _type, DartType _type2, String name2)>(
+    messageTemplate:
+        r"""The field '#name' has type '#type', which does not match the corresponding type, '#type2', in the overridden setter, '#name2'.""",
+    withArguments: _withArgumentsOverrideTypeMismatchSetter);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<
+        Message Function(
+            String name, DartType _type, DartType _type2, String name2)>
+    codeOverrideTypeMismatchSetter = const Code<
+            Message Function(
+                String name, DartType _type, DartType _type2, String name2)>(
+        "OverrideTypeMismatchSetter", templateOverrideTypeMismatchSetter,
+        analyzerCodes: <String>["INVALID_METHOD_OVERRIDE"]);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsOverrideTypeMismatchSetter(
+    String name, DartType _type, DartType _type2, String name2) {
+  if (name.isEmpty) throw 'No name provided';
+  name = demangleMixinApplicationName(name);
+  TypeLabeler labeler = new TypeLabeler();
+  List<Object> typeParts = labeler.labelType(_type);
+  List<Object> type2Parts = labeler.labelType(_type2);
+  if (name2.isEmpty) throw 'No name provided';
+  name2 = demangleMixinApplicationName(name2);
+  String type = typeParts.join();
+  String type2 = type2Parts.join();
+  return new Message(codeOverrideTypeMismatchSetter,
+      message:
+          """The field '${name}' has type '${type}', which does not match the corresponding type, '${type2}', in the overridden setter, '${name2}'.""" +
+              labeler.originMessages,
+      arguments: {
+        'name': name,
+        'type': _type,
+        'type2': _type2,
+        'name2': name2
+      });
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Template<
     Message Function(

diff --git a/pkg/front_end/messages.status b/pkg/front_end/messages.status
@@ -419,6 +419,7 @@ OverrideMismatchNamedParameter/example: Fail
 OverrideMoreRequiredArguments/example: Fail
 OverrideTypeMismatchParameter/example: Fail
 OverrideTypeMismatchReturnType/example: Fail
+OverrideTypeMismatchSetter/example: Fail
 OverrideTypeVariablesMismatch/example: Fail
 PackageNotFound/analyzerCode: Fail
 PackageNotFound/example: Fail

diff --git a/pkg/front_end/messages.yaml b/pkg/front_end/messages.yaml
@@ -2043,6 +2043,10 @@ OverrideTypeMismatchReturnType:
   tip: "Change to a subtype of '#type2'."
   analyzerCode: INVALID_METHOD_OVERRIDE
 
+OverrideTypeMismatchSetter:
+  template: "The field '#name' has type '#type', which does not match the corresponding type, '#type2', in the overridden setter, '#name2'."
+  analyzerCode: INVALID_METHOD_OVERRIDE
+
 PartOfSelf:
   template: "A file can't be a part of itself."
   analyzerCode: PART_OF_NON_PART

diff --git a/pkg/front_end/testcases/general/override_setter_with_field.dart b/pkg/front_end/testcases/general/override_setter_with_field.dart
@@ -0,0 +1,15 @@
+// Copyright (c) 2019, 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.
+
+abstract class A {
+  void set x(Object y);
+}
+
+class B implements A {
+  int x;
+}
+
+main() {
+  new B().x = 5;
+}
diff --git a/pkg/front_end/testcases/general/override_setter_with_field.dart.outline.expect b/pkg/front_end/testcases/general/override_setter_with_field.dart.outline.expect
@@ -0,0 +1,27 @@
+library;
+//
+// Problems in library:
+//
+// pkg/front_end/testcases/general/override_setter_with_field.dart:10:7: Error: The field 'B.x' has type 'int', which does not match the corresponding type, 'Object', in the overridden setter, 'A.x'.
+//  - 'Object' is from 'dart:core'.
+//   int x;
+//       ^
+// pkg/front_end/testcases/general/override_setter_with_field.dart:6:12: Context: This is the overridden method ('x').
+//   void set x(Object y);
+//            ^
+//
+import self as self;
+import "dart:core" as core;
+
+abstract class A extends core::Object {
+  synthetic constructor •() → self::A*
+    ;
+  abstract set x(core::Object* y) → void;
+}
+class B extends core::Object implements self::A {
+  field core::int* x;
+  synthetic constructor •() → self::B*
+    ;
+}
+static method main() → dynamic
+  ;
diff --git a/pkg/front_end/testcases/general/override_setter_with_field.dart.strong.expect b/pkg/front_end/testcases/general/override_setter_with_field.dart.strong.expect
@@ -0,0 +1,30 @@
+library;
+//
+// Problems in library:
+//
+// pkg/front_end/testcases/general/override_setter_with_field.dart:10:7: Error: The field 'B.x' has type 'int', which does not match the corresponding type, 'Object', in the overridden setter, 'A.x'.
+//  - 'Object' is from 'dart:core'.
+//   int x;
+//       ^
+// pkg/front_end/testcases/general/override_setter_with_field.dart:6:12: Context: This is the overridden method ('x').
+//   void set x(Object y);
+//            ^
+//
+import self as self;
+import "dart:core" as core;
+
+abstract class A extends core::Object {
+  synthetic constructor •() → self::A*
+    : super core::Object::•()
+    ;
+  abstract set x(core::Object* y) → void;
+}
+class B extends core::Object implements self::A {
+  field core::int* x = null;
+  synthetic constructor •() → self::B*
+    : super core::Object::•()
+    ;
+}
+static method main() → dynamic {
+  new self::B::•().{self::B::x} = 5;
+}
diff --git a/pkg/front_end/testcases/outline.status b/pkg/front_end/testcases/outline.status
@@ -13,6 +13,7 @@ general/override_check_accessor_with_covariant_modifier: TypeCheckError
 general/override_check_after_inference: TypeCheckError
 general/override_check_basic: TypeCheckError
 general/override_check_with_covariant_modifier: TypeCheckError
+general/override_setter_with_field: TypeCheckError
 
 inference/conflicting_fields: TypeCheckError
 inference/conflicts_can_happen2: Fail

diff --git a/pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart.outline.expect b/pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart.outline.expect
@@ -2,9 +2,8 @@ library test;
 //
 // Problems in library:
 //
-// pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart:48:7: Error: The return type of the method 'M.y' is 'int', which does not match the return type, 'Object', of the overridden method, 'I.y'.
+// pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart:48:7: Error: The field 'M.y' has type 'int', which does not match the corresponding type, 'Object', in the overridden setter, 'I.y'.
 //  - 'Object' is from 'dart:core'.
-// Change to a subtype of 'Object'.
 //   int y;
 //       ^
 // pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart:43:12: Context: This is the overridden method ('y').

diff --git a/pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart.strong.expect b/pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart.strong.expect
@@ -2,9 +2,8 @@ library test;
 //
 // Problems in library:
 //
-// pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart:48:7: Error: The return type of the method 'M.y' is 'int', which does not match the return type, 'Object', of the overridden method, 'I.y'.
+// pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart:48:7: Error: The field 'M.y' has type 'int', which does not match the corresponding type, 'Object', in the overridden setter, 'I.y'.
 //  - 'Object' is from 'dart:core'.
-// Change to a subtype of 'Object'.
 //   int y;
 //       ^
 // pkg/front_end/testcases/runtime_checks_new/mixin_forwarding_stub_field.dart:43:12: Context: This is the overridden method ('y').

diff --git a/pkg/front_end/testcases/strong.status b/pkg/front_end/testcases/strong.status
@@ -49,6 +49,7 @@ general/override_check_accessor_with_covariant_modifier: TypeCheckError # Issue
 general/override_check_after_inference: TypeCheckError # Issue #31620
 general/override_check_basic: TypeCheckError # Issue #31620
 general/override_check_with_covariant_modifier: TypeCheckError # Issue #31620
+general/override_setter_with_field: TypeCheckError
 general/reject_generic_function_types_in_bounds: RuntimeError # Expected
 general/spread_collection: RuntimeError
 general/statements: Crash

diff --git a/pkg/front_end/testcases/text_serialization.status b/pkg/front_end/testcases/text_serialization.status
@@ -201,6 +201,7 @@ general/override_check_two_substitutions: TextSerializationFailure # Was: Pass
 general/override_check_with_covariant_modifier: TypeCheckError # Issue #31620
 general/override_inference_for_setters: TextSerializationFailure
 general/override_inference_named_parameters_ordering: TextSerializationFailure
+general/override_setter_with_field: TypeCheckError
 general/part_as_entry_point: TextSerializationFailure # Was: Pass
 general/part_as_entry_point_lib: TextSerializationFailure # Was: Pass
 general/part_not_part_of: TextSerializationFailure