Ignore abstract classes, mixins, sealed classes.
Bug: https://github.com/dart-lang/sdk/issues/52965, https://github.com/dart-lang/sdk/issues/55550
Change-Id: I8ec70f909f55663eb299764f0402c5a992729846
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364206
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Matan Lurey <matanl@google.com>
diff --git a/pkg/analyzer/lib/src/error/inheritance_override.dart b/pkg/analyzer/lib/src/error/inheritance_override.dart
index 3550558..ed327b5 100644
--- a/pkg/analyzer/lib/src/error/inheritance_override.dart
+++ b/pkg/analyzer/lib/src/error/inheritance_override.dart
@@ -886,6 +886,14 @@
/// Verify that [classElement] complies with all `@mustBeOverridden`-annotated
/// members in all of its supertypes.
void _verifyMustBeOverridden() {
+ var classElement = this.classElement;
+ if (classElement is! ClassElement ||
+ classElement.isAbstract ||
+ classElement.isSealed) {
+ // We only care about concrete classes.
+ return;
+ }
+
var noSuchMethodDeclaration =
classElement.getMethod(FunctionElement.NO_SUCH_METHOD_METHOD_NAME);
if (noSuchMethodDeclaration != null &&
diff --git a/pkg/analyzer/test/src/diagnostics/missing_override_of_must_be_overridden_test.dart b/pkg/analyzer/test/src/diagnostics/missing_override_of_must_be_overridden_test.dart
index 0ac2f3c..de148c1 100644
--- a/pkg/analyzer/test/src/diagnostics/missing_override_of_must_be_overridden_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/missing_override_of_must_be_overridden_test.dart
@@ -234,8 +234,8 @@
]);
}
- test_method_hasAbstractOverride() async {
- await assertErrorsInCode('''
+ test_method_hasAbstractOverride_isOkBecauseNotConcreteClass() async {
+ await assertNoErrorsInCode('''
import 'package:meta/meta.dart';
class A {
@@ -246,9 +246,7 @@
abstract class B extends A {
void m();
}
-''', [
- error(WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_ONE, 96, 1),
- ]);
+''');
}
test_method_hasConcreteOverride() async {
@@ -354,26 +352,6 @@
]);
}
- test_method_multipleDirectSuperclass() async {
- await assertErrorsInCode('''
-import 'package:meta/meta.dart';
-
-class A {
- @mustBeOverridden
- void m() {}
-}
-
-class B {
- @mustBeOverridden
- void m() {}
-}
-
-abstract class C implements A, B {}
-''', [
- error(WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_ONE, 143, 1),
- ]);
- }
-
test_method_notVisible() async {
newFile('$testPackageLibPath/a.dart', '''
import 'package:meta/meta.dart';
@@ -391,8 +369,21 @@
''');
}
- test_method_superconstraint() async {
- await assertErrorsInCode('''
+ test_method_sealedClassIsImplicitlyAbstract() async {
+ await assertNoErrorsInCode('''
+import 'package:meta/meta.dart';
+
+class A {
+ @mustBeOverridden
+ void m() {}
+}
+
+sealed class B extends A {}
+''');
+ }
+
+ test_method_superconstraint_isOkBecauseMixinsAreNotConcrete() async {
+ await assertNoErrorsInCode('''
import 'package:meta/meta.dart';
class A {
@@ -401,9 +392,7 @@
}
mixin M on A {}
-''', [
- error(WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_ONE, 87, 1),
- ]);
+''');
}
test_operator_directSuperclass() async {
diff --git a/pkg/meta/CHANGELOG.md b/pkg/meta/CHANGELOG.md
index 53b3c33..9910c53 100644
--- a/pkg/meta/CHANGELOG.md
+++ b/pkg/meta/CHANGELOG.md
@@ -1,3 +1,37 @@
+## 1.15.0-dev
+
+- Updated `@mustBeOverridden` to only flag missing overrides in concrete
+ classes; in other words, abstract classes (including implicitly abstract, i.e
+ `sealed`) and mixin declarations are _no longer required_ to provide an
+ implementation:
+
+ ```dart
+ import 'package:meta/meta.dart';
+
+ abstract class Base {
+ @mustBeOverridden
+ void foo() {}
+ }
+
+ class Derived extends Base {
+ // ERROR: Missing implementation of `foo`.
+ }
+
+ abstract class Abstract extends Base {
+ // No error.
+ }
+
+ sealed class Sealed extends Base {
+ // No error.
+ }
+
+ mixin Mixin on Base {
+ // No error.
+ }
+ ```
+
+ See <https://github.com/dart-lang/sdk/issues/52965> for more information.
+
## 1.14.0
- Introduce `TargetKind.constructor`, to indicate that an annotation is valid on
@@ -41,6 +75,7 @@
);
}
```
+
- Introduce `@mustBeConst` to annotate parameters which only accept constant
arguments.
diff --git a/pkg/meta/lib/meta.dart b/pkg/meta/lib/meta.dart
index f7fe16a..e3292b7 100644
--- a/pkg/meta/lib/meta.dart
+++ b/pkg/meta/lib/meta.dart
@@ -267,8 +267,39 @@
const _MustBeConst mustBeConst = _MustBeConst();
/// Used to annotate an instance member `m` declared on a class or mixin `C`.
-/// Indicates that every subclass of `C`, concrete or abstract, must directly
-/// override `m`.
+/// Indicates that every concrete subclass of `C` must directly override `m`.
+///
+/// The intention of this annotation is to "re-abtract" a member that was
+/// previously concrete, and to ensure that subclasses provide their own
+/// implementation of the member. For example:
+///
+/// ```dart
+/// base class Entity {
+/// @mustBeOverridden
+/// String toString();
+/// }
+///
+/// abstract class AbstractEntity extends Entity {
+/// // OK: AbstractEntity is abstract.
+/// }
+///
+/// sealed class SealedEntity extends Entity {
+/// // OK: SealedEntity is sealed, which implies abstract.
+/// }
+///
+/// mixin MixinEntity on Entity {
+/// // OK: MixinEntity is abstract.
+/// }
+///
+/// class Person extends Entity {
+/// // ERROR: Missing new implementation of 'toString'.
+/// }
+///
+/// class Animal extends Entity {
+/// // OK: Animal provides its own implementation of 'toString'.
+/// String toString() => 'Animal';
+/// }
+/// ```
///
/// This annotation places no restrictions on the overriding members. In
/// particular, it does not require that the overriding members invoke the
@@ -281,7 +312,7 @@
/// (a method, operator, field, getter, or setter) of a class or of a mixin,
/// or
/// * the annotation is associated with a member `m` in class or mixin `C`, and
-/// there is a class or mixin `D` which is a subclass of `C` (directly or
+/// there is a concrete class `D` which is a subclass of `C` (directly or
/// indirectly), and `D` does not directly declare a concrete override of `m`
/// and does not directly declare a concrete override of `noSuchMethod`.
const _MustBeOverridden mustBeOverridden = _MustBeOverridden();
