+annotate_redeclares

Fixes: dart-lang/linter#4747

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

pkg/analysis_server/lib/src/services/correction/dart/add_redeclare.dart

@@ -0,0 +1,40 @@
+// 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:analysis_server/src/services/correction/dart/abstract_producer.dart';
+import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+
+class AddRedeclare extends ResolvedCorrectionProducer {
+  @override
+  bool get canBeAppliedInBulk => true;
+
+  @override
+  bool get canBeAppliedToFile => true;
+
+  @override
+  FixKind get fixKind => DartFixKind.ADD_REDECLARE;
+
+  @override
+  FixKind get multiFixKind => DartFixKind.ADD_REDECLARE_MULTI;
+
+  @override
+  Future<void> compute(ChangeBuilder builder) async {
+    var member = node.thisOrAncestorOfType<ClassMember>();
+    if (member == null) return;
+
+    var token = member.firstTokenAfterCommentAndMetadata;
+    var indent = utils.getIndent(1);
+    await builder.addDartFileEdit(file, (builder) {
+      builder.addInsertion(token.offset, (builder) {
+        builder.write('@');
+        builder.writeImportedName(
+            [Uri.parse('package:meta/meta.dart')], 'redeclare');
+        builder.write('$eol$indent');
+      });
+    });
+  }
+}

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

@@ -1848,6 +1848,8 @@ LintCode.always_use_package_imports:
   status: hasFix
 LintCode.annotate_overrides:
   status: hasFix
+LintCode.annotate_redeclares:
+  status: hasFix
 LintCode.avoid_annotating_with_dynamic:
   status: hasFix
 LintCode.avoid_as:

pkg/analysis_server/lib/src/services/correction/fix.dart

@@ -258,6 +258,16 @@ class DartFixKind {
     DartFixKindPriority.IN_FILE,
     "Add '@override' annotations everywhere in file",
   );
+  static const ADD_REDECLARE = FixKind(
+    'dart.fix.add.redeclare',
+    DartFixKindPriority.DEFAULT,
+    "Add '@redeclare' annotation",
+  );
+  static const ADD_REDECLARE_MULTI = FixKind(
+    'dart.fix.add.redeclare.multi',
+    DartFixKindPriority.IN_FILE,
+    "Add '@redeclare' annotations everywhere in file",
+  );
   static const ADD_REOPEN = FixKind(
     'dart.fix.add.reopen',
     DartFixKindPriority.DEFAULT,

pkg/analysis_server/lib/src/services/correction/fix_internal.dart

@@ -30,6 +30,7 @@ import 'package:analysis_server/src/services/correction/dart/add_missing_switch_
 import 'package:analysis_server/src/services/correction/dart/add_ne_null.dart';
 import 'package:analysis_server/src/services/correction/dart/add_null_check.dart';
 import 'package:analysis_server/src/services/correction/dart/add_override.dart';
+import 'package:analysis_server/src/services/correction/dart/add_redeclare.dart';
 import 'package:analysis_server/src/services/correction/dart/add_reopen.dart';
 import 'package:analysis_server/src/services/correction/dart/add_required.dart';
 import 'package:analysis_server/src/services/correction/dart/add_required_keyword.dart';
@@ -442,6 +443,9 @@ class FixProcessor extends BaseProcessor {
     LintNames.annotate_overrides: [
       AddOverride.new,
     ],
+    LintNames.annotate_redeclares: [
+      AddRedeclare.new,
+    ],
     LintNames.avoid_annotating_with_dynamic: [
       RemoveTypeAnnotation.other,
     ],

pkg/analysis_server/lib/src/services/linter/lint_names.dart

@@ -15,6 +15,7 @@ class LintNames {
   static const String always_specify_types = 'always_specify_types';
   static const String always_use_package_imports = 'always_use_package_imports';
   static const String annotate_overrides = 'annotate_overrides';
+  static const String annotate_redeclares = 'annotate_redeclares';
   static const String avoid_annotating_with_dynamic =
       'avoid_annotating_with_dynamic';
   static const String avoid_empty_else = 'avoid_empty_else';

pkg/analysis_server/test/src/services/correction/fix/add_redeclare_test.dart

@@ -0,0 +1,125 @@
+// 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:analysis_server/src/services/correction/fix.dart';
+import 'package:analysis_server/src/services/linter/lint_names.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import 'fix_processor.dart';
+
+void main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(AddRedeclareBulkTest);
+    defineReflectiveTests(AddRedeclareTest);
+  });
+}
+
+@reflectiveTest
+class AddRedeclareBulkTest extends BulkFixProcessorTest {
+  @override
+  String get lintCode => LintNames.annotate_redeclares;
+
+  @override
+  void setUp() {
+    super.setUp();
+    writeTestPackageConfig(meta: true);
+  }
+
+  Future<void> test_singleFile() async {
+    await resolveTestCode('''
+class C {
+  void f() {}
+  void g() {}
+}
+
+extension type E(C c) implements C {
+  /// Comment.
+  void f() {}
+  void g() {}
+}
+''');
+    await assertHasFix('''
+import 'package:meta/meta.dart';
+
+class C {
+  void f() {}
+  void g() {}
+}
+
+extension type E(C c) implements C {
+  /// Comment.
+  @redeclare
+  void f() {}
+  @redeclare
+  void g() {}
+}
+''');
+  }
+}
+
+@reflectiveTest
+class AddRedeclareTest extends FixProcessorLintTest {
+  @override
+  FixKind get kind => DartFixKind.ADD_REDECLARE;
+
+  @override
+  String get lintCode => LintNames.annotate_redeclares;
+
+  @override
+  void setUp() {
+    super.setUp();
+    writeTestPackageConfig(meta: true);
+  }
+
+  Future<void> test_method() async {
+    await resolveTestCode('''
+class C {
+  void f() {}
+}
+
+extension type E(C c) implements C {
+  void f() {}
+}
+''');
+    await assertHasFix('''
+import 'package:meta/meta.dart';
+
+class C {
+  void f() {}
+}
+
+extension type E(C c) implements C {
+  @redeclare
+  void f() {}
+}
+''');
+  }
+
+  Future<void> test_method_withComment() async {
+    await resolveTestCode('''
+class C {
+  void f() {}
+}
+
+extension type E(C c) implements C {
+  /// Comment.
+  void f() {}
+}
+''');
+    await assertHasFix('''
+import 'package:meta/meta.dart';
+
+class C {
+  void f() {}
+}
+
+extension type E(C c) implements C {
+  /// Comment.
+  @redeclare
+  void f() {}
+}
+''');
+  }
+}

pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart

@@ -87,7 +87,7 @@ abstract class BulkFixProcessorTest extends AbstractSingleUnitTest {
   late BulkFixProcessor processor;
 
   @override
-  List<String> get experiments => const [];
+  List<String> get experiments => const ['inline-class'];
 
   /// Return the lint code being tested.
   String? get lintCode => null;

pkg/analysis_server/test/src/services/correction/fix/test_all.dart

@@ -36,6 +36,7 @@ import 'add_missing_switch_cases_test.dart' as add_missing_switch_cases;
 import 'add_ne_null_test.dart' as add_ne_null;
 import 'add_null_check_test.dart' as add_null_check;
 import 'add_override_test.dart' as add_override;
+import 'add_redeclare_test.dart' as add_redeclare;
 import 'add_reopen_test.dart' as add_reopen;
 import 'add_required_test.dart' as add_required;
 import 'add_return_null_test.dart' as add_return_null;
@@ -306,6 +307,7 @@ void main() {
     add_ne_null.main();
     add_null_check.main();
     add_override.main();
+    add_redeclare.main();
     add_reopen.main();
     add_required.main();
     add_return_null.main();

pkg/linter/example/all.yaml

@@ -8,6 +8,7 @@ linter:
     - always_specify_types
     - always_use_package_imports
     - annotate_overrides
+    - annotate_redeclares
     - avoid_annotating_with_dynamic
     - avoid_bool_literals_in_conditional_expressions
     - avoid_catches_without_on_clauses

pkg/linter/lib/src/rules.dart

@@ -10,6 +10,7 @@ import 'rules/always_require_non_null_named_parameters.dart';
 import 'rules/always_specify_types.dart';
 import 'rules/always_use_package_imports.dart';
 import 'rules/annotate_overrides.dart';
+import 'rules/annotate_redeclares.dart';
 import 'rules/avoid_annotating_with_dynamic.dart';
 import 'rules/avoid_as.dart';
 import 'rules/avoid_bool_literals_in_conditional_expressions.dart';
@@ -241,6 +242,7 @@ void registerLintRules({bool inTestMode = false}) {
     ..register(AlwaysSpecifyTypes())
     ..register(AlwaysUsePackageImports())
     ..register(AnnotateOverrides())
+    ..register(AnnotateRedeclares())
     ..register(AvoidAnnotatingWithDynamic())
     ..register(AvoidAs())
     ..register(AvoidBoolLiteralsInConditionalExpressions())

pkg/linter/lib/src/rules/annotate_redeclares.dart

@@ -0,0 +1,110 @@
+// 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 '../analyzer.dart';
+
+const _desc = r'Annotate redeclared members.';
+
+const _details = r'''
+**DO** annotate redeclared members.
+
+This practice improves code readability and helps protect against
+unintentionally redeclaring members or being surprised when a member ceases to
+redeclare (due for example to a rename refactoring).
+
+**BAD:**
+```dart
+class C {
+  void f() { }
+}
+
+extension type E(C c) implements C {
+  void f() {
+    ...
+  }
+}
+```
+
+**GOOD:**
+```dart
+import 'package:meta/meta.dart';
+
+class C {
+  void f() { }
+}
+
+extension type E(C c) implements C {
+  @redeclare
+  void f() {
+    ...
+  }
+}
+```
+''';
+
+class AnnotateRedeclares extends LintRule {
+  static const LintCode code = LintCode('annotate_redeclares',
+      "The member '{0}' is redeclaring but isn't annotated with '@redeclare'.",
+      correctionMessage: "Try adding the '@redeclare' annotation.");
+
+  AnnotateRedeclares()
+      : super(
+            name: 'annotate_redeclares',
+            description: _desc,
+            details: _details,
+            group: Group.style,
+            state: State.experimental());
+
+  @override
+  LintCode get lintCode => code;
+
+  @override
+  void registerNodeProcessors(
+      NodeLintRegistry registry, LinterContext context) {
+    var visitor = _Visitor(this, context);
+    registry.addExtensionTypeDeclaration(this, visitor);
+  }
+}
+
+class _Visitor extends SimpleAstVisitor<void> {
+  final LintRule rule;
+  final LinterContext context;
+
+  _Visitor(this.rule, this.context);
+
+  @override
+  void visitExtensionTypeDeclaration(ExtensionTypeDeclaration node) {
+    node.members.whereType<MethodDeclaration>().forEach(_check);
+  }
+
+  void _check(MethodDeclaration node) {
+    if (node.isStatic) return;
+    var parent = node.parent;
+    // Shouldn't happen.
+    if (parent is! ExtensionTypeDeclaration) return;
+
+    var element = node.declaredElement;
+    if (element == null || element.hasRedeclare) return;
+
+    var extensionType = parent.declaredElement;
+    if (extensionType == null) return;
+
+    if (_redeclaresMember(element, extensionType)) {
+      rule.reportLintForToken(node.name, arguments: [element.displayName]);
+    }
+  }
+
+  /// Return `true` if the [member] redeclares a member from a superinterface.
+  bool _redeclaresMember(
+      ExecutableElement member, InterfaceElement extensionType) {
+    // todo(pq): unify with similar logic in `redeclare_verifier` and move to inheritanceManager
+    var uri = member.library.source.uri;
+    var interface = context.inheritanceManager.getInterface(extensionType);
+    return interface.redeclared.containsKey(Name(uri, member.name));
+  }
+}

pkg/linter/test/rules/all.dart

@@ -5,6 +5,7 @@
 import 'always_specify_types_test.dart' as always_specify_types;
 import 'always_use_package_imports_test.dart' as always_use_package_imports;
 import 'annotate_overrides_test.dart' as annotate_overrides;
+import 'annotate_redeclares_test.dart' as annotate_redeclares;
 import 'avoid_annotating_with_dynamic_test.dart'
     as avoid_annotating_with_dynamic;
 import 'avoid_equals_and_hash_code_on_mutable_classes_test.dart'
@@ -245,6 +246,7 @@ void main() {
   always_specify_types.main();
   always_use_package_imports.main();
   annotate_overrides.main();
+  annotate_redeclares.main();
   avoid_annotating_with_dynamic.main();
   avoid_equals_and_hash_code_on_mutable_classes.main();
   avoid_escaping_inner_quotes.main();