Allow a simple override to make a @protected method public (#3482)
diff --git a/lib/src/rules/unnecessary_overrides.dart b/lib/src/rules/unnecessary_overrides.dart
index 7f739aa..3d2cb6a 100644
--- a/lib/src/rules/unnecessary_overrides.dart
+++ b/lib/src/rules/unnecessary_overrides.dart
@@ -40,10 +40,11 @@
It's valid to override a member in the following cases:
* if a type (return type or a parameter type) is not the exactly the same as the
-super method,
+ super member,
* if the `covariant` keyword is added to one of the parameters,
* if documentation comments are present on the member,
-* if the member has annotations other than `@override`.
+* if the member has annotations other than `@override`,
+* if the member is not annotated with `@protected`, and the super member is.
`noSuchMethod` is a special method and is not checked by this rule.
@@ -68,7 +69,9 @@
abstract class _AbstractUnnecessaryOverrideVisitor extends SimpleAstVisitor {
final LintRule rule;
- ExecutableElement? inheritedMethod;
+ /// If [declaration] is an inherited member of interest, then this is set in
+ /// [visitMethodDeclaration].
+ late ExecutableElement _inheritedMethod;
late MethodDeclaration declaration;
_AbstractUnnecessaryOverrideVisitor(this.rule);
@@ -99,17 +102,27 @@
@override
void visitMethodDeclaration(MethodDeclaration node) {
- // noSuchMethod is mandatory to proxify
+ // 'noSuchMethod' is mandatory to proxify.
if (node.name.name == 'noSuchMethod') return;
- // it's ok to override to have better documentation
+ // It's ok to override to have better documentation.
if (node.documentationComment != null) return;
- inheritedMethod = getInheritedElement(node);
+ var inheritedMethod = getInheritedElement(node);
+ if (inheritedMethod == null) return;
+ _inheritedMethod = inheritedMethod;
declaration = node;
- if (inheritedMethod != null && !_addsMetadata() && _haveSameDeclaration()) {
- node.body.accept(this);
- }
+
+ // It's ok to override to add annotations.
+ if (_addsMetadata()) return;
+
+ // It's ok to override to change the signature.
+ if (!_haveSameDeclaration()) return;
+
+ // It's ok to override to make a `@protected` method public.
+ if (_makesPublicFromProtected()) return;
+
+ node.body.accept(this);
}
@override
@@ -127,36 +140,50 @@
rule.reportLint(declaration.name);
}
+ /// Returns whether [declaration] is annotated with any metadata (other than
+ /// `@override` or `@Override`).
bool _addsMetadata() {
var metadata = declaration.declaredElement?.metadata;
if (metadata != null) {
for (var annotation in metadata) {
- if (!annotation.isOverride) {
- return true;
- }
+ if (annotation.isOverride) continue;
+ if (annotation.isProtected && _inheritedMethod.hasProtected) continue;
+
+ // Any other annotation implies a meaningful override.
+ return true;
}
}
return false;
}
+ /// Returns true if [_inheritedMethod] is `@protected` and [declaration] is
+ /// not `@protected`, and false otherwise.
+ ///
+ /// This indicates that [_inheritedMethod] may have been overridden in order
+ /// to expand its visibility.
+ bool _makesPublicFromProtected() {
+ var declaredElement = declaration.declaredElement;
+ if (declaredElement == null) return false;
+ if (declaredElement.hasProtected) {
+ return false;
+ }
+ return _inheritedMethod.hasProtected;
+ }
+
bool _haveSameDeclaration() {
var declaredElement = declaration.declaredElement;
if (declaredElement == null) {
return false;
}
- var inheritedMethod = this.inheritedMethod;
- if (inheritedMethod == null) {
- return false;
- }
- if (declaredElement.returnType != inheritedMethod.returnType) {
+ if (declaredElement.returnType != _inheritedMethod.returnType) {
return false;
}
if (declaredElement.parameters.length !=
- inheritedMethod.parameters.length) {
+ _inheritedMethod.parameters.length) {
return false;
}
- for (var i = 0; i < inheritedMethod.parameters.length; i++) {
- var superParam = inheritedMethod.parameters[i];
+ for (var i = 0; i < _inheritedMethod.parameters.length; i++) {
+ var superParam = _inheritedMethod.parameters[i];
var param = declaredElement.parameters[i];
if (param.type != superParam.type) return false;
if (param.name != superParam.name) return false;
@@ -189,7 +216,7 @@
@override
void visitPropertyAccess(PropertyAccess node) {
- if (node.propertyName.staticElement == inheritedMethod) {
+ if (node.propertyName.staticElement == _inheritedMethod) {
node.target?.accept(this);
}
}
@@ -207,7 +234,7 @@
void visitMethodInvocation(MethodInvocation node) {
var declarationParameters = declaration.parameters;
if (declarationParameters != null &&
- node.methodName.staticElement == inheritedMethod &&
+ node.methodName.staticElement == _inheritedMethod &&
DartTypeUtilities.matchesArgumentsWithParameters(
node.argumentList.arguments, declarationParameters.parameters)) {
node.target?.accept(this);
@@ -271,7 +298,7 @@
node.rightHandSide)) {
var leftPart = node.leftHandSide.unParenthesized;
if (leftPart is PropertyAccess) {
- if (node.writeElement == inheritedMethod) {
+ if (node.writeElement == _inheritedMethod) {
leftPart.target?.accept(this);
}
}
diff --git a/test_data/rules/unnecessary_overrides.dart b/test_data/rules/unnecessary_overrides.dart
index 7b852e8..1314100 100644
--- a/test_data/rules/unnecessary_overrides.dart
+++ b/test_data/rules/unnecessary_overrides.dart
@@ -4,6 +4,8 @@
// test w/ `dart test -N unnecessary_overrides`
+import 'package:meta/meta.dart';
+
class _MyAnnotation {
const _MyAnnotation();
}
@@ -122,7 +124,9 @@
class C {
num get g => 0;
set s(int v) {}
+ @protected
num m(int v) => 0;
+ @protected
num m1({int v = 20}) => 0;
num m2([int v = 20]) => 0;
num operator +(int other) => 0;
@@ -187,6 +191,12 @@
@override
num m2([int v = 10]) => super.m2(v); // OK
}
+class ProtectedMadePublic extends C {
+ @override
+ num m(int v) => super.m(v); // OK
+ @protected
+ num m1({int v = 20}) => super.m1(v: v); // LINT
+}
// noSuchMethod is allowed to proxify
class D implements C {
