Allow a simple override to make a @Protected method public (#3482)

lib/src/rules/unnecessary_overrides.dart

@@ -40,10 +40,11 @@ class A extends B {
 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 @@ class UnnecessaryOverrides extends LintRule {
 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 @@ abstract class _AbstractUnnecessaryOverrideVisitor extends SimpleAstVisitor {
 
   @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 @@ abstract class _AbstractUnnecessaryOverrideVisitor extends SimpleAstVisitor {
     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;
   }
 
-  bool _haveSameDeclaration() {
+  /// 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) {
+    if (declaredElement == null) return false;
+    if (declaredElement.hasProtected) {
       return false;
     }
-    var inheritedMethod = this.inheritedMethod;
-    if (inheritedMethod == null) {
+    return _inheritedMethod.hasProtected;
+  }
+
+  bool _haveSameDeclaration() {
+    var declaredElement = declaration.declaredElement;
+    if (declaredElement == 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 @@ class _UnnecessaryGetterOverrideVisitor
 
   @override
   void visitPropertyAccess(PropertyAccess node) {
-    if (node.propertyName.staticElement == inheritedMethod) {
+    if (node.propertyName.staticElement == _inheritedMethod) {
       node.target?.accept(this);
     }
   }
@@ -207,7 +234,7 @@ class _UnnecessaryMethodOverrideVisitor
   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 @@ class _UnnecessarySetterOverrideVisitor
                 node.rightHandSide)) {
       var leftPart = node.leftHandSide.unParenthesized;
       if (leftPart is PropertyAccess) {
-        if (node.writeElement == inheritedMethod) {
+        if (node.writeElement == _inheritedMethod) {
           leftPart.target?.accept(this);
         }
       }

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 B extends A {
 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 @@ class ParameterDefaultChange extends C {
   @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 {