Add analysis hint for invalid use of @visibleForTemplate code.

Bug: #33353
Change-Id: Iaafccc3dca6b8d87bd54ed721871c72e9ac456c8
Reviewed-on: https://dart-review.googlesource.com/68432
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: srawlins

pkg/analyzer/lib/dart/element/element.dart

@@ -649,6 +649,12 @@ abstract class Element implements AnalysisTarget {
    */
   bool get hasRequired;
 
+  /**
+   * Return `true` if this element has an annotation of the form
+   * `@visibleForTemplate`.
+   */
+  bool get hasVisibleForTemplate;
+
   /**
    * Return `true` if this element has an annotation of the form
    * `@visibleForTesting`.
@@ -948,6 +954,12 @@ abstract class ElementAnnotation implements ConstantEvaluationTarget {
    */
   bool get isRequired;
 
+  /**
+   * Return `true` if this annotation marks the associated member as being
+   * visible for template files.
+   */
+  bool get isVisibleForTemplate;
+
   /**
    * Return `true` if this annotation marks the associated member as being
    * visible for testing.

pkg/analyzer/lib/error/error.dart

@@ -305,6 +305,7 @@ const List<ErrorCode> errorCodeValues = const [
   HintCode.INVALID_METHOD_OVERRIDE_TYPE_PARAMETER_BOUND,
   HintCode.INVALID_REQUIRED_PARAM,
   HintCode.INVALID_USE_OF_PROTECTED_MEMBER,
+  HintCode.INVALID_USE_OF_VISIBLE_FOR_TEMPLATE_MEMBER,
   HintCode.INVALID_USE_OF_VISIBLE_FOR_TESTING_MEMBER,
   HintCode.IS_DOUBLE,
   HintCode.IS_INT,

pkg/analyzer/lib/src/dart/element/element.dart

@@ -2906,6 +2906,12 @@ class ElementAnnotationImpl implements ElementAnnotation {
    */
   static String _MUST_CALL_SUPER_VARIABLE_NAME = "mustCallSuper";
 
+  /**
+   * The name of `angular.meta` library, used to define angular analysis
+   * annotations.
+   */
+  static String _NG_META_LIB_NAME = "angular.meta";
+
   /**
    * The name of the top-level variable used to mark a method as being expected
    * to override an inherited method.
@@ -2935,6 +2941,10 @@ class ElementAnnotationImpl implements ElementAnnotation {
    */
   static String _REQUIRED_VARIABLE_NAME = "required";
 
+  /// The name of the top-level variable used to mark a method as being
+  /// visible for templates.
+  static String _VISIBLE_FOR_TEMPLATE_VARIABLE_NAME = "visibleForTemplate";
+
   /// The name of the top-level variable used to mark a method as being
   /// visible for testing.
   static String _VISIBLE_FOR_TESTING_VARIABLE_NAME = "visibleForTesting";
@@ -3065,6 +3075,12 @@ class ElementAnnotationImpl implements ElementAnnotation {
           element.name == _REQUIRED_VARIABLE_NAME &&
           element.library?.name == _META_LIB_NAME;
 
+  @override
+  bool get isVisibleForTemplate =>
+      element is PropertyAccessorElement &&
+      element.name == _VISIBLE_FOR_TEMPLATE_VARIABLE_NAME &&
+      element.library?.name == _NG_META_LIB_NAME;
+
   @override
   bool get isVisibleForTesting =>
       element is PropertyAccessorElement &&
@@ -3281,6 +3297,10 @@ abstract class ElementImpl implements Element {
   bool get hasRequired =>
       metadata.any((ElementAnnotation annotation) => annotation.isRequired);
 
+  @override
+  bool get hasVisibleForTemplate => metadata
+      .any((ElementAnnotation annotation) => annotation.isVisibleForTemplate);
+
   @override
   bool get hasVisibleForTesting => metadata
       .any((ElementAnnotation annotation) => annotation.isVisibleForTesting);
@@ -3382,6 +3402,9 @@ abstract class ElementImpl implements Element {
     setModifier(Modifier.SYNTHETIC, isSynthetic);
   }
 
+  bool get isVisibleForTemplate => metadata
+      .any((ElementAnnotation annotation) => annotation.isVisibleForTemplate);
+
   @override
   bool get isVisibleForTesting => metadata
       .any((ElementAnnotation annotation) => annotation.isVisibleForTesting);
@@ -7665,6 +7688,9 @@ class MultiplyDefinedElementImpl implements MultiplyDefinedElement {
   @override
   bool get hasRequired => false;
 
+  @override
+  bool get hasVisibleForTemplate => false;
+
   @override
   bool get hasVisibleForTesting => false;
 
@@ -7704,6 +7730,8 @@ class MultiplyDefinedElementImpl implements MultiplyDefinedElement {
   @override
   bool get isSynthetic => true;
 
+  bool get isVisibleForTemplate => false;
+
   @override
   bool get isVisibleForTesting => false;
 

pkg/analyzer/lib/src/dart/element/handle.dart

@@ -380,6 +380,9 @@ abstract class ElementHandle implements Element {
   @override
   bool get hasRequired => actualElement.hasRequired;
 
+  @override
+  bool get hasVisibleForTemplate => actualElement.hasVisibleForTemplate;
+
   @override
   bool get hasVisibleForTesting => actualElement.hasVisibleForTesting;
 
@@ -413,6 +416,9 @@ abstract class ElementHandle implements Element {
   @override
   bool get isSynthetic => actualElement.isSynthetic;
 
+  @override
+  bool get isVisibleForTemplate => actualElement.hasVisibleForTemplate;
+
   @override
   bool get isVisibleForTesting => actualElement.hasVisibleForTesting;
 

pkg/analyzer/lib/src/dart/element/member.dart

@@ -418,6 +418,9 @@ abstract class Member implements Element {
   @override
   bool get hasRequired => _baseElement.hasRequired;
 
+  @override
+  bool get hasVisibleForTemplate => _baseElement.hasVisibleForTemplate;
+
   @override
   bool get hasVisibleForTesting => _baseElement.hasVisibleForTesting;
 
@@ -454,6 +457,9 @@ abstract class Member implements Element {
   @override
   bool get isSynthetic => _baseElement.isSynthetic;
 
+  @override
+  bool get isVisibleForTemplate => _baseElement.hasVisibleForTemplate;
+
   @override
   bool get isVisibleForTesting => _baseElement.hasVisibleForTesting;
 

pkg/analyzer/lib/src/dart/error/hint_codes.dart

@@ -309,6 +309,16 @@ class HintCode extends ErrorCode {
       "The member '{0}' can only be used within instance members of subclasses "
       "of '{1}'.");
 
+  /// This hint is generated anywhere where a member annotated with
+  /// `@visibleForTemplate` is used outside of a "template" Dart file.
+  ///
+  /// Parameters:
+  /// 0: the name of the member
+  /// 1: the name of the defining class
+  static const HintCode INVALID_USE_OF_VISIBLE_FOR_TEMPLATE_MEMBER =
+      const HintCode('INVALID_USE_OF_VISIBLE_FOR_TEMPLATE_MEMBER',
+          "The member '{0}' can only be used within '{1}' or a template library.");
+
   /// This hint is generated anywhere where a member annotated with
   /// `@visibleForTesting` is used outside the defining library, or a test.
   ///

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

@@ -233,6 +233,8 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
 
   static String _TO_INT_METHOD_NAME = "toInt";
 
+  static final _templateExtension = '.template';
+
   static final _testDir = '${path.separator}test${path.separator}';
 
   static final _testingDir = '${path.separator}testing${path.separator}';
@@ -849,9 +851,13 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
   ///   getter/setter, method closure or invocation accessed outside a subclass,
   ///   or accessed outside the library wherein the identifier is declared, or
   /// * if the given identifier is a closure, field, getter, setter, method
+  ///   closure or invocation which is annotated with `visibleForTemplate`, and
+  ///   is accessed outside of the defining library, and the current library
+  ///   does not have the suffix '.template' in its source path, or
+  /// * if the given identifier is a closure, field, getter, setter, method
   ///   closure or invocation which is annotated with `visibleForTesting`, and
   ///   is accessed outside of the defining library, and the current library
-  ///   does not have the word 'test' in its name.
+  ///   does not have a directory named 'test' or 'testing' in its path.
   void _checkForInvalidAccess(SimpleIdentifier identifier) {
     if (identifier.inDeclarationContext()) {
       return;
@@ -871,6 +877,21 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
       return false;
     }
 
+    bool isVisibleForTemplate(Element element) {
+      if (element == null) {
+        return false;
+      }
+      if (element.hasVisibleForTemplate) {
+        return true;
+      }
+      if (element is PropertyAccessorElement &&
+          element.enclosingElement is ClassElement &&
+          element.variable.hasVisibleForTemplate) {
+        return true;
+      }
+      return false;
+    }
+
     bool isVisibleForTesting(Element element) {
       if (element == null) {
         return false;
@@ -897,12 +918,19 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
         identifier.parent is Combinator &&
         identifier.parent.parent is ExportDirective;
 
+    bool inTemplateSource(LibraryElement library) =>
+        library.definingCompilationUnit.source.fullName
+            .contains(_templateExtension);
+
     bool inTestDirectory(LibraryElement library) =>
         library.definingCompilationUnit.source.fullName.contains(_testDir) ||
         library.definingCompilationUnit.source.fullName.contains(_testingDir);
 
     Element element = identifier.staticElement;
-    if (!isProtected(element) && !isVisibleForTesting(element)) {
+    if (!isProtected(element) &&
+        !isVisibleForTemplate(element) &&
+        !isVisibleForTesting(element)) {
+      // Without any of these annotations, the access is valid.
       return;
     }
 
@@ -920,26 +948,43 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
         return;
       }
     }
+    if (isVisibleForTemplate(element)) {
+      if (inCurrentLibrary(element) ||
+          inTemplateSource(_currentLibrary) ||
+          inExportDirective(identifier) ||
+          inCommentReference(identifier)) {
+        // The access is valid; even if [element] is also marked `protected`,
+        // the "visibilities" are unioned.
+        return;
+      }
+    }
     if (isVisibleForTesting(element)) {
       if (inCurrentLibrary(element) ||
           inTestDirectory(_currentLibrary) ||
           inExportDirective(identifier) ||
           inCommentReference(identifier)) {
-        // The access is valid; even if [element] is also marked
-        // `protected`, the "visibilities" are unioned.
+        // The access is valid; even if [element] is also marked `protected`,
+        // the "visibilities" are unioned.
         return;
       }
     }
 
     // At this point, [identifier] was not cleared as protected access, nor
-    // cleared as access for testing. Report the appropriate violation(s).
+    // cleared as access for templates or testing. Report the appropriate
+    // violation(s).
     Element definingClass = element.enclosingElement;
     if (isProtected(element)) {
       _errorReporter.reportErrorForNode(
           HintCode.INVALID_USE_OF_PROTECTED_MEMBER,
           identifier,
           [identifier.name.toString(), definingClass.name]);
     }
+    if (isVisibleForTemplate(element)) {
+      _errorReporter.reportErrorForNode(
+          HintCode.INVALID_USE_OF_VISIBLE_FOR_TEMPLATE_MEMBER,
+          identifier,
+          [identifier.name.toString(), definingClass.name]);
+    }
     if (isVisibleForTesting(element)) {
       _errorReporter.reportErrorForNode(
           HintCode.INVALID_USE_OF_VISIBLE_FOR_TESTING_MEMBER,