Validation of `@protected` method invocations.

pkg/analyzer/lib/src/generated/resolver.dart

 // Copyright (c) 2014, 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.
 
 library analyzer.src.generated.resolver;
 
 import 'dart:collection';
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
@@ skipping 188 matching lines @@
     // This was determined to not be a good hint, see: dartbug.com/16029
     //checkForOverridingPrivateMember(node);
     _checkForMissingReturn(node.returnType, node.body);
     _checkForUnnecessaryNoSuchMethod(node);
     return super.visitMethodDeclaration(node);
   }
 
   @override
   Object visitMethodInvocation(MethodInvocation node) {
     _checkForCanBeNullAfterNullAware(node.realTarget, node.operator);
+    _checkForInvalidProtectedMethodCalls(node);
     return super.visitMethodInvocation(node);
   }
 
   @override
   Object visitPostfixExpression(PostfixExpression node) {
     _checkForDeprecatedMemberUse(node.bestElement, node);
     return super.visitPostfixExpression(node);
   }
 
   @override
@@ skipping 382 matching lines @@
       if (!_typeSystem.isAssignableTo(bestRightType, leftType)) {
         _errorReporter.reportTypeErrorForNode(
             HintCode.INVALID_ASSIGNMENT, rhs, [bestRightType, leftType]);
         return true;
       }
     }
     return false;
   }
 
   /**
+   * Produces a hint if the given invocation is of a protected method outside
+   * a subclass instance method.
+   */
+  void _checkForInvalidProtectedMethodCalls(MethodInvocation node) {
+    Element element = node.methodName.staticElement;

[Brian Wilkerson, 2016/02/23 17:45:56]

Given that this is a hint, I suspect that we want to use "bestElement" rather
than "staticElement".

[pquitslund, 2016/02/23 22:59:07]

Done.

+    if (element == null || !element.isProtected) {
+      return;
+    }
+
+    ClassElement definingClass = element.enclosingElement;
+
+    MethodDeclaration decl =
+        node.getAncestor((AstNode node) => node is MethodDeclaration);
+    if (decl == null) {
+      _errorReporter.reportErrorForNode(
+          HintCode.INVALID_USE_OF_PROTECTED_MEMBER,
+          node,
+          [node.methodName.toString(), definingClass.name]);
+      return;
+    }
+
+    ClassElement invokingClass = decl.element?.enclosingElement;
+    if (invokingClass != null &&
+        !invokingClass.type.isSubtypeOf(definingClass.type)) {

[Brian Wilkerson, 2016/02/23 17:45:56]

The comment for the annotation says "extend or mix in", but the use of
"isSubtypeOf" will also allow classes that implement the interface of the
defining class, such as:

class Foo {
  @protected
  void m1() {}
}

class Bar implements Foo {
  void m2() {
    m1(); // Expect a hint here.
  }
}

I think we need to explicitly walk the superclass chain (looking at mixins) in
order to get the right behavior.

[pquitslund, 2016/02/23 22:59:07]

Fixed.

+      _errorReporter.reportErrorForNode(
+          HintCode.INVALID_USE_OF_PROTECTED_MEMBER,
+          node,
+          [node.methodName.toString(), definingClass.name]);
+    }
+  }
+
+  /**
    * Check that the imported library does not define a loadLibrary function. The import has already
    * been determined to be deferred when this is called.
    *
    * @param node the import directive to evaluate
    * @param importElement the [ImportElement] retrieved from the node
    * @return `true` if and only if an error code is generated on the passed node
    * See [CompileTimeErrorCode.IMPORT_DEFERRED_LIBRARY_WITH_LOAD_FUNCTION].
    */
   bool _checkForLoadLibraryFunction(
       ImportDirective node, ImportElement importElement) {
@@ skipping 7759 matching lines @@
     safelyVisit(node.target);
     safelyVisit(node.typeArguments);
     node.accept(elementResolver);
     _inferFunctionExpressionsParametersTypes(node.argumentList);
     _inferArgumentTypesFromContext(node);
     safelyVisit(node.argumentList);
     node.accept(typeAnalyzer);
     return null;
   }
 
-  void _inferArgumentTypesFromContext(InvocationExpression node) {
-    DartType contextType = node.staticInvokeType;
-    if (contextType is FunctionType) {
-      DartType originalType = node.function.staticType;
-      DartType returnContextType = InferenceContext.getType(node);
-      TypeSystem ts = typeSystem;
-      if (returnContextType != null &&
-          node.typeArguments == null &&
-          originalType is FunctionType &&
-          originalType.typeFormals.isNotEmpty &&
-          ts is StrongTypeSystemImpl) {
-
-        contextType = ts.inferGenericFunctionCall(typeProvider, originalType,
-            DartType.EMPTY_LIST, DartType.EMPTY_LIST, returnContextType);
-      }
-
-      InferenceContext.setType(node.argumentList, contextType);
-    }
-  }
-
   @override
   Object visitNamedExpression(NamedExpression node) {
     InferenceContext.setType(node.expression, InferenceContext.getType(node));
     return super.visitNamedExpression(node);
   }
 
   @override
   Object visitNode(AstNode node) {
     node.visitChildren(this);
     node.accept(elementResolver);
@@ skipping 307 matching lines @@
         List<ParameterElement> onDataParameters = onDataType.parameters;
         if (onDataParameters == null || onDataParameters.isEmpty) {
           return null;
         }
         return onDataParameters[0].type;
       }
     }
     return null;
   }
 
+  void _inferArgumentTypesFromContext(InvocationExpression node) {
+    DartType contextType = node.staticInvokeType;
+    if (contextType is FunctionType) {
+      DartType originalType = node.function.staticType;
+      DartType returnContextType = InferenceContext.getType(node);
+      TypeSystem ts = typeSystem;
+      if (returnContextType != null &&
+          node.typeArguments == null &&
+          originalType is FunctionType &&
+          originalType.typeFormals.isNotEmpty &&
+          ts is StrongTypeSystemImpl) {
+        contextType = ts.inferGenericFunctionCall(typeProvider, originalType,
+            DartType.EMPTY_LIST, DartType.EMPTY_LIST, returnContextType);
+      }
+
+      InferenceContext.setType(node.argumentList, contextType);
+    }
+  }
+
   void _inferFormalParameterList(FormalParameterList node, DartType type) {
     if (typeAnalyzer.inferFormalParameterList(node, type)) {
       // TODO(leafp): This gets dropped on the floor if we're in the field
       // inference task.  We should probably keep these infos.
       inferenceContext.recordInference(node.parent, type);
     }
   }
 
   /**
    * If given "mayBeClosure" is [FunctionExpression] without explicit parameters types and its
@@ skipping 4053 matching lines @@
     nonFields.add(node);
     return null;
   }
 
   @override
   Object visitNode(AstNode node) => node.accept(TypeResolverVisitor_this);
 
   @override
   Object visitWithClause(WithClause node) => null;
 }