verify that members marked as redeclaring actually do

See: #53121

Change-Id: I39294952a3adf0f037417d6e04811e3294128e6d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/320886
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>

Author: pq

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -3650,6 +3650,10 @@ WarningCode.RECEIVER_OF_TYPE_NEVER:
     We _could_ remove the thing being received (method invocation, etc.)
 WarningCode.RECORD_LITERAL_ONE_POSITIONAL_NO_TRAILING_COMMA:
   status: hasFix
+WarningCode.REDECLARE_ON_NON_REDECLARING_MEMBER:
+  status: needsFix
+  notes: |-
+    The fix is to remove the annotation.
 WarningCode.REMOVED_LINT_USE:
   status: needsFix
 WarningCode.REPLACED_LINT_USE:

pkg/analyzer/lib/dart/element/element.dart

@@ -593,6 +593,9 @@ abstract class Element implements AnalysisTarget {
   /// Whether the element has an annotation of the form `@protected`.
   bool get hasProtected;
 
+  /// Whether the element has an annotation of the form `@redeclare`.
+  bool get hasRedeclare;
+
   /// Whether the element has an annotation of the form `@reopen`.
   bool get hasReopen;
 

pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart

@@ -32,6 +32,7 @@ import 'package:analyzer/src/error/imports_verifier.dart';
 import 'package:analyzer/src/error/inheritance_override.dart';
 import 'package:analyzer/src/error/language_version_override_verifier.dart';
 import 'package:analyzer/src/error/override_verifier.dart';
+import 'package:analyzer/src/error/redeclare_verifier.dart';
 import 'package:analyzer/src/error/todo_finder.dart';
 import 'package:analyzer/src/error/unicode_text_verifier.dart';
 import 'package:analyzer/src/error/unused_local_elements_verifier.dart';
@@ -448,6 +449,12 @@ class LibraryAnalyzer {
       errorReporter,
     ));
 
+    unit.accept(RedeclareVerifier(
+      _inheritance,
+      _libraryElement,
+      errorReporter,
+    ));
+
     TodoFinder(errorReporter).findIn(unit);
     LanguageVersionOverrideVerifier(errorReporter).verify(unit);
 

pkg/analyzer/lib/src/dart/element/element.dart

@@ -2112,6 +2112,18 @@ abstract class ElementImpl implements Element {
     return false;
   }
 
+  @override
+  bool get hasRedeclare {
+    final metadata = this.metadata;
+    for (var i = 0; i < metadata.length; i++) {
+      var annotation = metadata[i];
+      if (annotation.isRedeclare) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   @override
   bool get hasReopen {
     final metadata = this.metadata;
@@ -5213,6 +5225,9 @@ class MultiplyDefinedElementImpl implements MultiplyDefinedElement {
   @override
   bool get hasProtected => false;
 
+  @override
+  bool get hasRedeclare => false;
+
   @override
   bool get hasReopen => false;
 

pkg/analyzer/lib/src/dart/element/member.dart

@@ -570,6 +570,9 @@ abstract class Member implements Element {
   @override
   bool get hasProtected => _declaration.hasProtected;
 
+  @override
+  bool get hasRedeclare => _declaration.hasRedeclare;
+
   @override
   bool get hasReopen => _declaration.hasReopen;
 

pkg/analyzer/lib/src/error/codes.g.dart

@@ -6925,6 +6925,18 @@ class WarningCode extends AnalyzerErrorCode {
     hasPublishedDocs: true,
   );
 
+  ///  An error code indicating the use of a redeclare annotation on a member that does not redeclare.
+  ///
+  ///  Parameters:
+  ///  0: the kind of member
+  static const WarningCode REDECLARE_ON_NON_REDECLARING_MEMBER = WarningCode(
+    'REDECLARE_ON_NON_REDECLARING_MEMBER',
+    "The {0} doesn't redeclare a {0} declared in a superinterface.",
+    correctionMessage:
+        "Try updating this member to match a declaration in a superinterface, "
+        "or removing the redeclare annotation.",
+  );
+
   ///  An error code indicating use of a removed lint rule.
   ///
   ///  Parameters:

pkg/analyzer/lib/src/error/error_code_values.g.dart

@@ -1013,6 +1013,7 @@ const List<ErrorCode> errorCodeValues = [
   WarningCode.PATTERN_NEVER_MATCHES_VALUE_TYPE,
   WarningCode.RECEIVER_OF_TYPE_NEVER,
   WarningCode.RECORD_LITERAL_ONE_POSITIONAL_NO_TRAILING_COMMA,
+  WarningCode.REDECLARE_ON_NON_REDECLARING_MEMBER,
   WarningCode.REMOVED_LINT_USE,
   WarningCode.REPLACED_LINT_USE,
   WarningCode.RETURN_OF_DO_NOT_STORE,

pkg/analyzer/lib/src/error/redeclare_verifier.dart

@@ -0,0 +1,92 @@
+// Copyright (c) 2023, 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.
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/error/listener.dart';
+import 'package:analyzer/src/dart/element/inheritance_manager3.dart';
+import 'package:analyzer/src/error/codes.dart';
+
+/// Instances of the class `RedeclareVerifier` visit all of the members of any
+/// extension type declarations in a compilation unit to verify that if they
+/// have a redeclare annotation it is being used correctly.
+class RedeclareVerifier extends RecursiveAstVisitor<void> {
+  /// The inheritance manager used to find redeclared members.
+  final InheritanceManager3 _inheritance;
+
+  /// The URI of the library being verified.
+  final Uri _libraryUri;
+
+  /// The error reporter used to report errors.
+  final ErrorReporter _errorReporter;
+
+  /// The current extension type.
+  InterfaceElement? _currentExtensionType;
+
+  RedeclareVerifier(
+      this._inheritance, LibraryElement library, this._errorReporter)
+      : _libraryUri = library.source.uri;
+
+  @override
+  void visitExtensionTypeDeclaration(ExtensionTypeDeclaration node) {
+    _currentExtensionType = node.declaredElement;
+    super.visitExtensionTypeDeclaration(node);
+    _currentExtensionType = null;
+  }
+
+  @override
+  void visitMethodDeclaration(MethodDeclaration node) {
+    // Only check if we're in an extension type declaration.
+    if (_currentExtensionType == null) return;
+
+    var element = node.declaredElement!;
+
+    // Static members can't redeclare.
+    if (element.isStatic) return;
+
+    if (element.hasRedeclare && !_redeclaresMember(element)) {
+      if (element is MethodElement) {
+        _errorReporter.reportErrorForToken(
+          WarningCode.REDECLARE_ON_NON_REDECLARING_MEMBER,
+          node.name,
+          [
+            'method',
+          ],
+        );
+      } else if (element is PropertyAccessorElement) {
+        if (element.isGetter) {
+          _errorReporter.reportErrorForToken(
+            WarningCode.REDECLARE_ON_NON_REDECLARING_MEMBER,
+            node.name,
+            [
+              'getter',
+            ],
+          );
+        } else {
+          _errorReporter.reportErrorForToken(
+            WarningCode.REDECLARE_ON_NON_REDECLARING_MEMBER,
+            node.name,
+            [
+              'setter',
+            ],
+          );
+        }
+      }
+    }
+  }
+
+  /// Return `true` if the [member] redeclares a member from a superinterface.
+  bool _redeclaresMember(ExecutableElement member) {
+    var currentType = _currentExtensionType;
+    if (currentType != null) {
+      var interface = _inheritance.getInterface(currentType);
+      var redeclared = interface.redeclared;
+      var name = Name(_libraryUri, member.name);
+      return redeclared.containsKey(name);
+    }
+
+    return false;
+  }
+}

pkg/analyzer/messages.yaml

@@ -24102,6 +24102,14 @@ WarningCode:
     problemMessage: "A record literal with exactly one positional field requires a trailing comma."
     correctionMessage: Try adding a trailing comma.
     hasPublishedDocs: true
+  REDECLARE_ON_NON_REDECLARING_MEMBER:
+    problemMessage: "The {0} doesn't redeclare a {0} declared in a superinterface."
+    correctionMessage: Try updating this member to match a declaration in a superinterface, or removing the redeclare annotation.
+    comment: |-
+      An error code indicating the use of a redeclare annotation on a member that does not redeclare.
+
+      Parameters:
+      0: the kind of member
   REMOVED_LINT_USE:
     problemMessage: "'{0}' was removed in Dart '{1}'"
     correctionMessage: "Remove the reference to '{0}'."

pkg/analyzer/test/src/diagnostics/invalid_annotation_target_test.dart

@@ -373,7 +373,7 @@ class C {
   int get g => 0; 
 }
 
-extension type E(C c) {
+extension type E(C c) implements C {
   @redeclare
   int get g => 0; 
 }
@@ -388,7 +388,7 @@ class C {
   void m() {}
 }
 
-extension type E(C c) {
+extension type E(C c) implements C {
   @redeclare
   void m() {}
 }
@@ -403,7 +403,7 @@ class C {
   set g(int i) {}
 }
 
-extension type E(C c) {
+extension type E(C c) implements C {
   @redeclare
   set g(int i) {}
 }

pkg/analyzer/test/src/diagnostics/redeclare_on_non_redeclaring_member_test.dart

@@ -0,0 +1,148 @@
+// Copyright (c) 2023, 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.
+
+import 'package:analyzer/src/error/codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(RedeclareOnNonRedeclaringMemberTest);
+  });
+}
+
+@reflectiveTest
+class RedeclareOnNonRedeclaringMemberTest extends PubPackageResolutionTest {
+  @override
+  List<String> get experiments => ['inline-class'];
+
+  @override
+  void setUp() {
+    super.setUp();
+    writeTestPackageConfig(PackageConfigFileBuilder(), meta: true);
+  }
+
+  test_getter() async {
+    await assertErrorsInCode(r'''
+import 'package:meta/meta.dart';
+
+class C {}
+
+extension type E(C c) implements C {
+  @redeclare
+  int get i => 0;
+}
+''', [
+      error(WarningCode.REDECLARE_ON_NON_REDECLARING_MEMBER, 106, 1),
+    ]);
+  }
+
+  test_getter_redeclares() async {
+    await assertNoErrorsInCode(r'''
+import 'package:meta/meta.dart';
+
+class C {
+  int get i => 0;
+}
+
+extension type E(C c) implements C {
+  @redeclare
+  int get i => 0;
+}
+''');
+  }
+
+  test_method() async {
+    await assertErrorsInCode(r'''
+import 'package:meta/meta.dart';
+
+class C {}
+
+extension type E(C c) implements C {
+  @redeclare
+  void n() {}
+}
+''', [
+      error(WarningCode.REDECLARE_ON_NON_REDECLARING_MEMBER, 103, 1),
+    ]);
+  }
+
+  test_method_inClass() async {
+    await assertErrorsInCode(r'''
+import 'package:meta/meta.dart';
+
+class C {}
+
+class D implements C {
+  @redeclare
+  void n() {}
+}
+''', [
+      // No REDECLARE_ON_NON_REDECLARING_MEMBER warning.
+      error(WarningCode.INVALID_ANNOTATION_TARGET, 71, 10),
+    ]);
+  }
+
+  test_method_redeclared() async {
+    await assertNoErrorsInCode(r'''
+import 'package:meta/meta.dart';
+
+class C {
+  void m() {}
+}
+
+extension type E(C c) implements C {
+  @redeclare
+  void m() {}
+}
+''');
+  }
+
+  test_method_static() async {
+    await assertErrorsInCode(r'''
+import 'package:meta/meta.dart';
+
+class C {}
+
+extension type E(C c) implements C {
+  @redeclare
+  static void n() {}
+}
+''', [
+      // No REDECLARE_ON_NON_REDECLARING_MEMBER warning.
+      error(WarningCode.INVALID_ANNOTATION_TARGET, 85, 10),
+    ]);
+  }
+
+  test_setter() async {
+    await assertErrorsInCode(r'''
+import 'package:meta/meta.dart';
+
+class C {}
+
+extension type E(C c) implements C {
+  @redeclare
+  set i(int i) {}
+}
+''', [
+      error(WarningCode.REDECLARE_ON_NON_REDECLARING_MEMBER, 102, 1),
+    ]);
+  }
+
+  test_setter_redeclares() async {
+    await assertNoErrorsInCode(r'''
+import 'package:meta/meta.dart';
+
+class C {
+  set i(int i) {}
+}
+
+extension type E(C c) implements C {
+  @redeclare
+  set i(int i) {}
+}
+''');
+  }
+}