diff --git a/example/all.yaml b/example/all.yaml index 7f955a179..811fcb036 100644 --- a/example/all.yaml +++ b/example/all.yaml @@ -78,6 +78,7 @@ linter: - hash_and_equals - implementation_imports - implicit_call_tearoffs + - implicit_reopen - invalid_case_patterns - iterable_contains_unrelated_type - join_return_with_assignment diff --git a/lib/src/rules.dart b/lib/src/rules.dart index 2e352f0fc..5e028c5a0 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -84,6 +84,7 @@ import 'rules/flutter_style_todos.dart'; import 'rules/hash_and_equals.dart'; import 'rules/implementation_imports.dart'; import 'rules/implicit_call_tearoffs.dart'; +import 'rules/implicit_reopen.dart'; import 'rules/invalid_case_patterns.dart'; import 'rules/invariant_booleans.dart'; import 'rules/iterable_contains_unrelated_type.dart'; @@ -310,6 +311,7 @@ void registerLintRules({bool inTestMode = false}) { ..register(HashAndEquals()) ..register(ImplementationImports()) ..register(ImplicitCallTearoffs()) + ..register(ImplicitReopen()) ..register(InvariantBooleans()) ..register(InvalidCasePatterns()) ..register(IterableContainsUnrelatedType()) diff --git a/lib/src/rules/implicit_reopen.dart b/lib/src/rules/implicit_reopen.dart new file mode 100644 index 000000000..17243286e --- /dev/null +++ b/lib/src/rules/implicit_reopen.dart @@ -0,0 +1,178 @@ +// 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"Don't implicitly reopen classes."; + +/// todo(pq): link out to (upcoming) dart.dev docs. +/// https://github.com/dart-lang/site-www/issues/4497 +const _details = r''' +Using an `interface`, `base`, `final`, or `sealed` modifier on a class, +or a `base` modifier on a mixin, +authors can control whether classes and mixins allow being implemented, +extended, and/or mixed in from outside of the library where they're defined. +In some cases, it's possible for an author to inadvertently relax these controls +and implicitly "reopen" a class. (A similar reopening cannot occur with a mixin.) + +This lint guards against unintentionally reopening a class by requiring such +cases to be made explicit with the +[`@reopen`](https://pub.dev/documentation/meta/latest/meta/reopen-constant.html) +annotation in `package:meta`. + +**BAD:** +```dart +interface class I {} + +class C extends I {} // LINT +``` + +**GOOD:** +```dart +interface class I {} + +final class C extends I {} +``` + +```dart +interface class I {} + +final class C extends I {} +``` + +```dart +import 'package:meta/meta.dart'; + +interface class I {} + +@reopen +class C extends I {} +``` +'''; + +class ImplicitReopen extends LintRule { + static const LintCode code = LintCode('implicit_reopen', + "The {0} '{1}' reopens '{2}' because it is not marked '{3}'", + correctionMessage: + "Try marking '{1}' '{3}' or annotating it with '@reopen'"); + + ImplicitReopen() + : super( + name: 'implicit_reopen', + description: _desc, + details: _details, + state: State.experimental(), + group: Group.errors); + + @override + LintCode get lintCode => code; + + @override + void registerNodeProcessors( + NodeLintRegistry registry, LinterContext context) { + var visitor = _Visitor(this); + registry.addClassDeclaration(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final LintRule rule; + + _Visitor(this.rule); + + void checkElement(InterfaceElement? element, NamedCompilationUnitMember node, + {required String type}) { + if (element == null) return; + if (element.hasReopen) return; + if (element.isSealed) return; + if (element.isMixinClass) return; + + var library = element.library; + var supertype = element.superElement; + if (supertype.library != library) return; + + if (element.isBase) { + if (supertype.isFinal) { + reportLint(node, + target: element, other: supertype!, reason: 'final', type: type); + return; + } else if (supertype.isInterface) { + reportLint(node, + target: element, + other: supertype!, + reason: 'interface', + type: type); + return; + } + } else if (element.hasNoModifiers) { + if (supertype.isInterface) { + reportLint(node, + target: element, + other: supertype!, + reason: 'interface', + type: type); + return; + } + } + } + + void reportLint( + NamedCompilationUnitMember member, { + required String type, + required InterfaceElement target, + required InterfaceElement other, + required String reason, + }) { + rule.reportLintForToken(member.name, + arguments: [type, target.name, other.name, reason]); + } + + @override + void visitClassDeclaration(ClassDeclaration node) { + checkElement(node.declaredElement, node, type: 'class'); + } +} + +extension on InterfaceElement? { + bool get hasNoModifiers => !isInterface && !isBase && !isSealed && !isFinal; + + bool get isBase { + var self = this; + if (self is ClassElement) return self.isBase; + if (self is MixinElement) return self.isBase; + return false; + } + + bool get isFinal { + var self = this; + if (self is ClassElement) return self.isFinal; + return false; + } + + bool get isInterface { + var self = this; + if (self is ClassElement) return self.isInterface; + return false; + } + + bool get isSealed { + var self = this; + if (self is ClassElement) return self.isSealed; + return false; + } + + bool get isMixinClass { + var self = this; + if (self is ClassElement) return self.isMixinClass; + return false; + } + + LibraryElement? get library => this?.library; + + InterfaceElement? get superElement => this?.supertype?.element; +} diff --git a/test/rules/all.dart b/test/rules/all.dart index 2acc6cb30..8b99e61a3 100644 --- a/test/rules/all.dart +++ b/test/rules/all.dart @@ -48,6 +48,7 @@ import 'exhaustive_cases_test.dart' as exhaustive_cases; import 'file_names_test.dart' as file_names; import 'flutter_style_todos_test.dart' as flutter_style_todos; import 'hash_and_equals_test.dart' as hash_and_equals; +import 'implicit_reopen_test.dart' as implicit_reopen; import 'invalid_case_patterns_test.dart' as invalid_case_patterns; import 'join_return_with_assignment_test.dart' as join_return_with_assignment; import 'library_annotations_test.dart' as library_annotations; @@ -159,6 +160,7 @@ void main() { file_names.main(); flutter_style_todos.main(); hash_and_equals.main(); + implicit_reopen.main(); invalid_case_patterns.main(); join_return_with_assignment.main(); library_annotations.main(); diff --git a/test/rules/implicit_reopen_test.dart b/test/rules/implicit_reopen_test.dart new file mode 100644 index 000000000..6acf7d4ec --- /dev/null +++ b/test/rules/implicit_reopen_test.dart @@ -0,0 +1,254 @@ +// 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(ImplicitReopenTest); + defineReflectiveTests(ImplicitReopenInducedModifierTest); + }); +} + +@reflectiveTest +class ImplicitReopenInducedModifierTest extends LintRuleTest + with LanguageVersion300Mixin { + @override + bool get addMetaPackageDep => true; + + @override + String get lintRule => 'implicit_reopen'; + + test_inducedFinal() async { + await assertDiagnostics(r''' +final class F {} +sealed class S extends F {} +base class C extends S {} +''', [ + lint(56, 1), + ]); + } + + test_inducedFinal_base_interface() async { + await assertDiagnostics(r''' +base class C {} +interface class D {} +sealed class E extends D implements C {} +base class B extends E {} +''', [ + lint(89, 1), + ]); + } + + test_inducedFinal_baseMixin_interface() async { + await assertDiagnostics(r''' +interface class D {} +base mixin G {} +sealed class H extends D with G {} +base class B extends H {} +''', [ + lint(83, 1), + ]); + } + + test_inducedFinal_interface_base_ok() async { + await assertDiagnostics(r''' +interface class S {} +base class I {} +sealed class A extends S implements I {} +class C extends A {} +''', [ + // No lint. + error(CompileTimeErrorCode.SUBTYPE_OF_BASE_IS_NOT_BASE_FINAL_OR_SEALED, + 84, 1), + ]); + } + + test_inducedFinal_mixin_finalClass() async { + await assertDiagnostics(r''' +final class S {} +mixin M {} +sealed class A extends S with M {} +base class B extends A {} +''', [ + lint(74, 1), + ]); + } + + test_inducedInterface() async { + await assertDiagnostics(r''' +interface class I {} +sealed class S extends I {} +class C extends S {} +''', [ + lint(55, 1), + ]); + } + + test_inducedInterface_base() async { + await assertDiagnostics(r''' +interface class I {} +sealed class S extends I {} +base class C extends S {} +''', [ + lint(60, 1), + ]); + } + + test_inducedInterface_base_mixin_interface() async { + await assertDiagnostics(r''' +interface class S {} +mixin M {} +sealed class A extends S with M {} +base class C extends A {} +''', [ + lint(78, 1), + ]); + } + + test_inducedInterface_mixin_interface() async { + await assertDiagnostics(r''' +interface class S {} +mixin M {} +sealed class A extends S with M {} +class C extends A {} +''', [ + lint(73, 1), + ]); + } + + test_inducedInterface_twoLevels() async { + await assertDiagnostics(r''' +interface class I {} +sealed class S extends I {} +sealed class S2 extends S {} +class C extends S2 {} +''', [ + lint(84, 1), + ]); + } + + test_subtypingFinalError_ok() async { + await assertDiagnostics(r''' +final class F {} +sealed class S extends F {} +class C extends S {} +''', [ + // No lint. + error(CompileTimeErrorCode.SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED, + 51, 1), + ]); + } +} + +@reflectiveTest +class ImplicitReopenTest extends LintRuleTest with LanguageVersion300Mixin { + @override + bool get addMetaPackageDep => true; + + @override + String get lintRule => 'implicit_reopen'; + + test_class_classFinal_ok() async { + await assertDiagnostics(r''' +final class F {} + +class C extends F {} +''', [ + // No lint. + error(CompileTimeErrorCode.SUBTYPE_OF_FINAL_IS_NOT_BASE_FINAL_OR_SEALED, + 24, 1), + ]); + } + + test_class_classInterface() async { + await assertDiagnostics(r''' +interface class I {} + +class C extends I {} +''', [ + lint(28, 1), + ]); + } + + test_class_classInterface_outsideLib_ok() async { + newFile('$testPackageLibPath/a.dart', r''' +interface class I {} +'''); + + await assertDiagnostics(r''' +import 'a.dart'; + +class C extends I {} +''', [ + // No lint. + error(CompileTimeErrorCode.INTERFACE_CLASS_EXTENDED_OUTSIDE_OF_LIBRARY, + 34, 1), + ]); + } + + test_class_classInterface_reopened_ok() async { + await assertNoDiagnostics(r''' +import 'package:meta/meta.dart'; + +interface class I {} + +@reopen +class C extends I {} +'''); + } + + test_classBase_classFinal() async { + await assertDiagnostics(r''' +final class F {} + +base class B extends F {} +''', [ + lint(29, 1), + ]); + } + + test_classBase_classInterface() async { + await assertDiagnostics(r''' +interface class I {} + +base class B extends I {} +''', [ + lint(33, 1), + ]); + } + + test_classFinal_classInterface_ok() async { + await assertNoDiagnostics(r''' +interface class I {} + +final class C extends I {} +'''); + } + + test_classMixin_classInterface_ok() async { + await assertDiagnostics(r''' +interface class I {} + +mixin class M extends I {} +''', [ + // No lint. + error(CompileTimeErrorCode.MIXIN_CLASS_DECLARATION_EXTENDS_NOT_OBJECT, 44, + 1), + ]); + } + + test_mixin_classInterface_ok() async { + await assertDiagnostics(r''' +interface class I {} + +mixin M extends I {} +''', [ + // No lint. + error(ParserErrorCode.EXPECTED_INSTEAD, 30, 7), + ]); + } +}