[analyzer] Introduce a new annotation @visibleOutsideTemplate

The new annotation is intended to be used on members of class, enum or mixin to opt out the @visibleForTemplate visibility restriction cascaded from class- / enum- / mixin- level.

1. Throw warning if the annotation is added to a invalid target.
2. Update @visibleForTemplate diagnostics logic to opt out members annotated with @visibleOutsideTemplate.

Change-Id: Iec546fc7785cd45f39a1b2a2cc8849ef1cf9d04a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304825
Reviewed-by: Marya Belanger <mbelanger@google.com>
Auto-Submit: Ludi Zhan <ludizhan@google.com>
Commit-Queue: Ludi Zhan <ludizhan@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -3529,6 +3529,8 @@ WarningCode.INVALID_VISIBILITY_ANNOTATION:
   status: hasFix
 WarningCode.INVALID_VISIBLE_FOR_OVERRIDING_ANNOTATION:
   status: hasFix
+WarningCode.INVALID_VISIBLE_OUTSIDE_TEMPLATE_ANNOTATION:
+  status: needsEvaluation
 WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_ONE:
   status: hasFix
 WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_TWO:

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

@@ -669,6 +669,10 @@ abstract class Element implements AnalysisTarget {
   /// `@visibleForTesting`.
   bool get hasVisibleForTesting;
 
+  /// Return `true` if this element has an annotation of the form
+  /// `@visibleOutsideTemplate`.
+  bool get hasVisibleOutsideTemplate;
+
   /// The unique integer identifier of this element.
   int get id;
 
@@ -935,6 +939,10 @@ abstract class ElementAnnotation implements ConstantEvaluationTarget {
   /// visible for testing.
   bool get isVisibleForTesting;
 
+  /// Return `true` if this annotation marks the associated member as being
+  /// visible outside of template files.
+  bool get isVisibleOutsideTemplate;
+
   /// Return a representation of the value of this annotation, forcing the value
   /// to be computed if it had not previously been computed, or `null` if the
   /// value of this annotation could not be computed because of errors.

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

@@ -2052,6 +2052,11 @@ class ElementAnnotationImpl implements ElementAnnotation {
   /// visible for testing.
   static const String _visibleForTestingVariableName = 'visibleForTesting';
 
+  /// The name of the top-level variable used to mark a method as being
+  /// visible outside of template files.
+  static const String _visibleOutsideTemplateVariableName =
+      'visibleOutsideTemplate';
+
   @override
   Element? element;
 
@@ -2198,6 +2203,11 @@ class ElementAnnotationImpl implements ElementAnnotation {
   bool get isVisibleForTesting =>
       _isPackageMetaGetter(_visibleForTestingVariableName);
 
+  @override
+  bool get isVisibleOutsideTemplate => _isTopGetter(
+      libraryName: _angularMetaLibName,
+      name: _visibleOutsideTemplateVariableName);
+
   @override
   LibraryElement get library => compilationUnit.library;
 
@@ -2643,6 +2653,18 @@ abstract class ElementImpl implements Element {
     return false;
   }
 
+  @override
+  bool get hasVisibleOutsideTemplate {
+    final metadata = this.metadata;
+    for (var i = 0; i < metadata.length; i++) {
+      var annotation = metadata[i];
+      if (annotation.isVisibleOutsideTemplate) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   /// Return an identifier that uniquely identifies this element among the
   /// children of this element's parent.
   String get identifier => name!;
@@ -5259,6 +5281,9 @@ class MultiplyDefinedElementImpl implements MultiplyDefinedElement {
   @override
   bool get hasVisibleForTesting => false;
 
+  @override
+  bool get hasVisibleOutsideTemplate => false;
+
   @override
   bool get isPrivate {
     throw UnimplementedError();
@@ -5272,6 +5297,8 @@ class MultiplyDefinedElementImpl implements MultiplyDefinedElement {
 
   bool get isVisibleForTemplate => false;
 
+  bool get isVisibleOutsideTemplate => false;
+
   @override
   ElementKind get kind => ElementKind.ERROR;
 

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

@@ -596,6 +596,9 @@ abstract class Member implements Element {
   @override
   bool get hasVisibleForTesting => _declaration.hasVisibleForTesting;
 
+  @override
+  bool get hasVisibleOutsideTemplate => _declaration.hasVisibleOutsideTemplate;
+
   @override
   int get id => _declaration.id;
 

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

@@ -63,6 +63,9 @@ class AnnotationVerifier {
         element.isVisibleForTesting ||
         element.isVisibleForOverriding) {
       _checkVisibility(node, element);
+    } else if (element.isVisibleOutsideTemplate) {
+      _checkVisibility(node, element);
+      _checkVisibleOutsideTemplate(node);
     }
 
     _checkKinds(node, parent, element);
@@ -343,8 +346,8 @@ class AnnotationVerifier {
   }
 
   /// Reports a warning at [node] if it is not a valid target for a
-  /// visibility (`visibleForTemplate`, `visibileForTesting`,
-  /// `visibleForOverride`) annotation.
+  /// visibility (`visibleForTemplate`, `visibleOutsideTemplate`,
+  /// `visibileForTesting`, `visibleForOverride`) annotation.
   void _checkVisibility(Annotation node, ElementAnnotation element) {
     var parent = node.parent;
     if (parent is Declaration) {
@@ -409,6 +412,23 @@ class AnnotationVerifier {
     }
   }
 
+  /// Reports a warning at [node] if it's parent is not a valid target for an
+  /// `@visibleOutsideTemplate` annotation.
+  void _checkVisibleOutsideTemplate(Annotation node) {
+    var grandparent = node.parent.parent;
+    switch (grandparent) {
+      case ClassDeclaration(declaredElement: InterfaceElement(:var metadata)):
+      case EnumDeclaration(declaredElement: InterfaceElement(:var metadata)):
+      case MixinDeclaration(declaredElement: InterfaceElement(:var metadata)):
+        for (final annotation in metadata) {
+          if (annotation.isVisibleForTemplate) return;
+        }
+    }
+
+    _errorReporter.reportErrorForNode(
+        WarningCode.INVALID_VISIBLE_OUTSIDE_TEMPLATE_ANNOTATION, node);
+  }
+
   /// Returns an expression (for error-reporting purposes) associated with a
   /// `@useResult` `unless` argument, if the associated parameter is undefined.
   Expression? _findUndefinedUseResultParameter(

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

@@ -2070,6 +2070,17 @@ class _InvalidAccessVerifier {
     return false;
   }
 
+  bool _hasVisibleOutsideTemplate(Element element) {
+    if (element.hasVisibleOutsideTemplate) {
+      return true;
+    }
+    if (element is PropertyAccessorElement &&
+        element.variable.hasVisibleOutsideTemplate) {
+      return true;
+    }
+    return false;
+  }
+
   bool _inCommentReference(SimpleIdentifier identifier) {
     var parent = identifier.parent;
     return parent is CommentReference || parent?.parent is CommentReference;
@@ -2091,16 +2102,20 @@ class _InvalidAccessVerifier {
 
   /// Check if @visibleForTemplate is applied to the given [Element].
   ///
-  /// [ClassElement] and [EnumElement] are excluded from the @visibleForTemplate
-  /// access checks. Instead, the access restriction is cascaded to the
-  /// corresponding class members and enum constants. For other types of
-  /// elements, check if they are annotated based on `hasVisibleForTemplate`
-  /// value.
-  bool _isVisibleForTemplateApplied(Element? element) {
-    if (element is ClassElement || element is EnumElement) {
+  /// [ClassElement], [EnumElement] and [MixinElement] are excluded from the
+  /// @visibleForTemplate access checks. Instead, the access restriction is
+  /// cascaded to all the corresponding members not annotated by
+  /// @visibleOutsideTemplate.
+  /// For other types of elements, check if they are annotated based on
+  /// `hasVisibleForTemplate` value.
+  bool _isVisibleForTemplateApplied(Element element) {
+    if (element is ClassElement ||
+        element is EnumElement ||
+        element is MixinElement) {
       return false;
     } else {
-      return _hasVisibleForTemplate(element);
+      return _hasVisibleForTemplate(element) &&
+          !_hasVisibleOutsideTemplate(element);
     }
   }
 }

pkg/analyzer/lib/src/error/codes.g.dart

@@ -6452,6 +6452,15 @@ class WarningCode extends AnalyzerErrorCode {
     hasPublishedDocs: true,
   );
 
+  ///  No parameters.
+  static const WarningCode INVALID_VISIBLE_OUTSIDE_TEMPLATE_ANNOTATION =
+      WarningCode(
+    'INVALID_VISIBLE_OUTSIDE_TEMPLATE_ANNOTATION',
+    "The annotation 'visibleOutsideTemplate' can only be applied to a member "
+        "of a class, enum, or mixin that is annotated with "
+        "'visibleForTemplate'.",
+  );
+
   ///  Parameters:
   ///  0: the name of the member
   static const WarningCode MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_ONE =

pkg/analyzer/lib/src/error/error_code_values.g.dart

@@ -963,6 +963,7 @@ const List<ErrorCode> errorCodeValues = [
   WarningCode.INVALID_USE_OF_VISIBLE_FOR_TESTING_MEMBER,
   WarningCode.INVALID_VISIBILITY_ANNOTATION,
   WarningCode.INVALID_VISIBLE_FOR_OVERRIDING_ANNOTATION,
+  WarningCode.INVALID_VISIBLE_OUTSIDE_TEMPLATE_ANNOTATION,
   WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_ONE,
   WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_THREE_PLUS,
   WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_TWO,

pkg/analyzer/lib/src/test_utilities/mock_packages.dart

@@ -6,6 +6,26 @@ import 'package:analyzer/file_system/file_system.dart';
 
 /// Helper for creating mock packages.
 class MockPackages {
+  /// Create a fake 'angular' package that can be used by tests.
+  static void addAngularMetaPackageFiles(Folder rootFolder) {
+    var libFolder = rootFolder.getChildAssumingFolder('lib');
+    libFolder.getChildAssumingFile('angular_meta.dart').writeAsStringSync(r'''
+library angular.meta;
+
+const _VisibleForTemplate visibleForTemplate = const _VisibleForTemplate();
+
+const _VisibleOutsideTemplate visibleOutsideTemplate = const _VisibleOutsideTemplate();
+
+class _VisibleForTemplate {
+  const _VisibleForTemplate();
+}
+
+class _VisibleOutsideTemplate {
+  const _VisibleOutsideTemplate();
+}
+''');
+  }
+
   /// Create a fake 'ffi' package that can be used by tests.
   static void addFfiPackageFiles(Folder rootFolder) {
     var libFolder = rootFolder.getChildAssumingFolder('lib');

pkg/analyzer/messages.yaml

@@ -23009,6 +23009,69 @@ WarningCode:
 
       Remove the annotation:
 
+      ```dart
+      class C {}
+      ```
+  INVALID_VISIBLE_OUTSIDE_TEMPLATE_ANNOTATION:
+    problemMessage: "The annotation 'visibleOutsideTemplate' can only be applied to a member of a class, enum, or mixin that is annotated with 'visibleForTemplate'."
+    hasPublishedDocs: false
+    comment: No parameters.
+    documentation: |-
+      #### Description
+
+      The analyzer produces this diagnostic when the `@visibleOutsideTemplate`
+      annotation is used incorrectly. This annotation is only meant to annotate
+      members of a class, enum, or mixin that has the `@visibleForTemplate`
+      annotation, to opt those members out of the visibility restrictions that
+      `@visibleForTemplate` imposes.
+
+      #### Examples
+
+      The following code produces this diagnostic because there is no
+      `@visibleForTemplate` annotation at the class level:
+
+      ```dart
+      import 'package:angular_meta/angular_meta.dart';
+
+      class C {
+        [!@visibleOutsideTemplate!]
+        int m() {
+          return 1;
+        }
+      }
+      ```
+
+      The following code produces this diagnostic because the annotation is on
+      a class declaration, not a member of a class, enum, or mixin:
+
+      ```dart
+      import 'package:angular_meta/angular_meta.dart';
+
+      [!@visibleOutsideTemplate!]
+      class C {}
+      ```
+
+      #### Common fixes
+
+      If the class is only visible so that templates can reference it, then add
+      the `@visibleForTemplate` annotation to the class:
+
+      ```dart
+      import 'package:angular_meta/angular_meta.dart';
+
+      @visibleForTemplate
+      class C {
+        @visibleOutsideTemplate
+        int m() {
+          return 1;
+        }
+      }
+      ```
+
+      If the `@visibleOutsideTemplate` annotation is on anything other than a
+      member of a class, enum, or mixin with the `@visibleForTemplate`
+      annotation, remove the annotation:
+
       ```dart
       class C {}
       ```

pkg/analyzer/test/src/dart/resolution/context_collection_resolution.dart

@@ -437,6 +437,7 @@ class PubPackageResolutionTest extends ContextResolutionTest {
   void writeTestPackageConfig(
     PackageConfigFileBuilder config, {
     String? languageVersion,
+    bool angularMeta = false,
     bool ffi = false,
     bool js = false,
     bool meta = false,
@@ -450,6 +451,14 @@ class PubPackageResolutionTest extends ContextResolutionTest {
       languageVersion: languageVersion ?? testPackageLanguageVersion,
     );
 
+    if (angularMeta) {
+      var angularMetaPath = '/packages/angular_meta';
+      MockPackages.addAngularMetaPackageFiles(
+        getFolder(angularMetaPath),
+      );
+      config.add(name: 'angular_meta', rootPath: angularMetaPath);
+    }
+
     if (ffi) {
       var ffiPath = '/packages/ffi';
       MockPackages.addFfiPackageFiles(