diff --git a/pkg/analysis_server/lib/src/services/correction/dart/add_redeclare.dart b/pkg/analysis_server/lib/src/services/correction/dart/add_redeclare.dart new file mode 100644 index 000000000000..3f624f74115b --- /dev/null +++ b/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 compute(ChangeBuilder builder) async { + var member = node.thisOrAncestorOfType(); + 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'); + }); + }); + } +} diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml index 7f8c2fa83b35..80fda32ed9f3 100644 --- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml +++ b/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: diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index 7ec3c19cc0a0..f1ae1fccb992 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/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, diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 9c1b13b1beb7..6a659683e25c 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/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, ], diff --git a/pkg/analysis_server/lib/src/services/linter/lint_names.dart b/pkg/analysis_server/lib/src/services/linter/lint_names.dart index 99595bb4a8ec..397a8424e09e 100644 --- a/pkg/analysis_server/lib/src/services/linter/lint_names.dart +++ b/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'; diff --git a/pkg/analysis_server/test/src/services/correction/fix/add_redeclare_test.dart b/pkg/analysis_server/test/src/services/correction/fix/add_redeclare_test.dart new file mode 100644 index 000000000000..dbe8163e15dd --- /dev/null +++ b/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 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 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 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() {} +} +'''); + } +} diff --git a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart b/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart index 1f21411e7e22..a5a3b0f95b0d 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart +++ b/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 get experiments => const []; + List get experiments => const ['inline-class']; /// Return the lint code being tested. String? get lintCode => null; diff --git a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart index 4507e4c88e39..d0991a1a09a4 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart +++ b/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(); diff --git a/pkg/linter/example/all.yaml b/pkg/linter/example/all.yaml index 63510240ea3b..fb47d3e3fdc2 100644 --- a/pkg/linter/example/all.yaml +++ b/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 diff --git a/pkg/linter/lib/src/rules.dart b/pkg/linter/lib/src/rules.dart index 5adcef427cc9..e10258929780 100644 --- a/pkg/linter/lib/src/rules.dart +++ b/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()) diff --git a/pkg/linter/lib/src/rules/annotate_redeclares.dart b/pkg/linter/lib/src/rules/annotate_redeclares.dart new file mode 100644 index 000000000000..ccdafced2418 --- /dev/null +++ b/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 { + final LintRule rule; + final LinterContext context; + + _Visitor(this.rule, this.context); + + @override + void visitExtensionTypeDeclaration(ExtensionTypeDeclaration node) { + node.members.whereType().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)); + } +} diff --git a/pkg/linter/test/rules/all.dart b/pkg/linter/test/rules/all.dart index 72427bace80f..6319a8f31635 100644 --- a/pkg/linter/test/rules/all.dart +++ b/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(); diff --git a/pkg/linter/test/rules/annotate_redeclares_test.dart b/pkg/linter/test/rules/annotate_redeclares_test.dart new file mode 100644 index 000000000000..d999841dc952 --- /dev/null +++ b/pkg/linter/test/rules/annotate_redeclares_test.dart @@ -0,0 +1,64 @@ +// 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:test_reflective_loader/test_reflective_loader.dart'; + +import '../rule_test_support.dart'; + +main() { + defineReflectiveSuite(() { + defineReflectiveTests(AnnotateRedeclaresTest); + }); +} + +@reflectiveTest +class AnnotateRedeclaresTest extends LintRuleTest { + @override + bool get addMetaPackageDep => true; + + @override + String get lintRule => 'annotate_redeclares'; + + test_method() async { + await assertDiagnostics(r''' +class A { + void m() {} +} + +extension type E(A a) implements A { + void m() {} +} +''', [ + lint(71, 1), + ]); + } + + test_method_annotated() async { + await assertNoDiagnostics(r''' +import 'package:meta/meta.dart'; +class A { + void m() {} +} + +extension type E(A a) implements A { + @redeclare + void m() {} +} +'''); + } + + test_setter() async { + await assertDiagnostics(r''' +class A { + int i = 0; +} + +extension type E(A a) implements A { + set i(int i) {} +} +''', [ + lint(69, 1), + ]); + } +} diff --git a/pkg/linter/tool/machine/rules.json b/pkg/linter/tool/machine/rules.json index 785388f96825..354300885879 100644 --- a/pkg/linter/tool/machine/rules.json +++ b/pkg/linter/tool/machine/rules.json @@ -628,6 +628,17 @@ "details": "**DO** annotate overridden methods and fields.\n\nThis practice improves code readability and helps protect against\nunintentionally overriding superclass members.\n\n**BAD:**\n```dart\nclass Cat {\n int get lives => 9;\n}\n\nclass Lucky extends Cat {\n final int lives = 14;\n}\n```\n\n**GOOD:**\n```dart\nabstract class Dog {\n String get breed;\n void bark() {}\n}\n\nclass Husky extends Dog {\n @override\n final String breed = 'Husky';\n @override\n void bark() {}\n}\n```\n\n", "sinceDartSdk": "2.0.0" }, + { + "name": "annotate_redeclares", + "description": "Annotate redeclared members.", + "group": "style", + "state": "experimental", + "incompatible": [], + "sets": [], + "fixStatus": "hasFix", + "details": "**DO** annotate redeclared members.\n\nThis practice improves code readability and helps protect against\nunintentionally redeclaring members or being surprised when a member ceases to\nredeclare (due for example to a rename refactoring).\n\n**BAD:**\n```dart\nclass C {\n void f() { }\n}\n\nextension type E(C c) implements C {\n void f() {\n ...\n }\n}\n```\n\n**GOOD:**\n```dart\nimport 'package:meta/meta.dart';\n\nclass C {\n void f() { }\n}\n\nextension type E(C c) implements C {\n @redeclare\n void f() {\n ...\n }\n}\n```\n", + "sinceDartSdk": "Unreleased" + }, { "name": "avoid_annotating_with_dynamic", "description": "Avoid annotating with dynamic when not required.",