[linter/analyzer_public_api] Report experimental inconsistencies.

stereotype441 · Commit Queue · commit 9a6fc48d51e0 · 2025-09-29T11:06:59.000-07:00
Modifies the `analyzer_public_api` lint so that it reports `analyzer_public_api_experimental_inconsistency` if an element in the analyzer public API that is _not_ marked experimental has a type that refers to an element in the analyzer public API that _is_ marked experimental. Such references are problematic because they can cause a client of the analyzer public API to make use of an experimental type without receiving a warning. Change-Id: I6a6a6964fd5f1d37e163dc27a1cb4e605efd2d76 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/451550 Auto-Submit: Paul Berry <paulberry@google.com> Reviewed-by: Samuel Rawlins <srawlins@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
@@ -1883,6 +1883,8 @@ LintCode.analyzer_public_api_bad_part_directive:
   status: noFix
 LintCode.analyzer_public_api_bad_type:
   status: noFix
+LintCode.analyzer_public_api_experimental_inconsistency:
+  status: noFix
 LintCode.analyzer_public_api_exports_non_public_name:
   status: noFix
 LintCode.analyzer_public_api_impl_in_public_api:
diff --git a/pkg/linter/lib/src/lint_codes.g.dart b/pkg/linter/lib/src/lint_codes.g.dart
@@ -213,6 +213,39 @@ class LinterLintCode extends LintCodeWithExpectedTypes {
     expectedTypes: [ExpectedType.string],
   );
 
+  /// Lint issued if an element in the analyzer public API makes use
+  /// of a type that's annotated `@experimental`, but the element
+  /// itself is not annotated `@experimental`.
+  ///
+  /// The reason this is a problem is that it makes it possible for
+  /// analyzer clients to implicitly reference analyzer experimental
+  /// types. This can happen in many ways; here are some examples:
+  ///
+  /// - If `C` is a non-experimental public API class that implements
+  ///   `B`, and `B` is an experimental public API class with a getter
+  ///   called `x`, then a client can access `B.x` via `C`.
+  ///
+  /// - If `f` has return type `T`, and `T` is an experimental public
+  ///   API class with a getter called `x`, then a client can access
+  ///   `T.x` via `f().x`.
+  ///
+  /// - If `f` has type `void Function(T)`, and `T` is an experimental
+  ///   public API class with a getter called `x`, then a client can
+  ///   access `T.x` via `var g = f; g = (t) { print(t.x); }`.
+  ///
+  /// Parameters:
+  /// String types: list of types, separated by `, `
+  static const LinterLintTemplate<
+    LocatableDiagnostic Function({required String types})
+  >
+  analyzerPublicApiExperimentalInconsistency = LinterLintTemplate(
+    LintNames.analyzer_public_api_experimental_inconsistency,
+    "Element makes use of experimental type(s), but is not itself marked with "
+    "`@experimental`: {0}.",
+    withArguments: _withArgumentsAnalyzerPublicApiExperimentalInconsistency,
+    expectedTypes: [ExpectedType.string],
+  );
+
   /// Lint issued if a file in the analyzer public API contains an `export`
   /// directive that exports a name that's not part of the analyzer public API.
   ///
@@ -3407,6 +3440,15 @@ class LinterLintCode extends LintCodeWithExpectedTypes {
     return LocatableDiagnosticImpl(analyzerPublicApiBadType, [types]);
   }
 
+  static LocatableDiagnostic
+  _withArgumentsAnalyzerPublicApiExperimentalInconsistency({
+    required String types,
+  }) {
+    return LocatableDiagnosticImpl(analyzerPublicApiExperimentalInconsistency, [
+      types,
+    ]);
+  }
+
   static LocatableDiagnostic
   _withArgumentsAnalyzerPublicApiExportsNonPublicName({
     required String elements,
diff --git a/pkg/linter/lib/src/lint_names.g.dart b/pkg/linter/lib/src/lint_names.g.dart
@@ -44,6 +44,9 @@ abstract final class LintNames {
   static const String analyzer_public_api_bad_type =
       'analyzer_public_api_bad_type';
 
+  static const String analyzer_public_api_experimental_inconsistency =
+      'analyzer_public_api_experimental_inconsistency';
+
   static const String analyzer_public_api_exports_non_public_name =
       'analyzer_public_api_exports_non_public_name';
 
diff --git a/pkg/linter/lib/src/rules/analyzer_public_api.dart b/pkg/linter/lib/src/rules/analyzer_public_api.dart
@@ -32,6 +32,7 @@ class AnalyzerPublicApi extends MultiAnalysisRule {
   List<DiagnosticCode> get diagnosticCodes => [
     LinterLintCode.analyzerPublicApiBadPartDirective,
     LinterLintCode.analyzerPublicApiBadType,
+    LinterLintCode.analyzerPublicApiExperimentalInconsistency,
     LinterLintCode.analyzerPublicApiExportsNonPublicName,
     LinterLintCode.analyzerPublicApiImplInPublicApi,
   ];
@@ -48,6 +49,16 @@ class AnalyzerPublicApi extends MultiAnalysisRule {
   }
 }
 
+/// Types of problematic uses of types that can be detected by this lint.
+enum _ProblematicTypeUseKind {
+  /// Use of a non-public type by a part of the analyzer public API.
+  useOfNonPublicType,
+
+  /// Use of an experimental type by a non-experimental part of the analyzer
+  /// public API.
+  useOfExperimentalType,
+}
+
 /// Lightweight object representing a public import.
 class _PublicImport {
   final Element? Function(String name) lookup;
@@ -130,7 +141,7 @@ class _Visitor extends SimpleAstVisitor<void> {
     }
   }
 
-  void _checkMember(Fragment fragment) {
+  void _checkMember(Fragment fragment, {required bool isParentExperimental}) {
     if (fragment is ConstructorFragment &&
         fragment.element.enclosingElement is EnumElement) {
       // Enum constructors aren't callable from outside of the enum, so they
@@ -143,7 +154,12 @@ class _Visitor extends SimpleAstVisitor<void> {
       return;
     }
     if (fragment is ExecutableFragment) {
-      _checkType(fragment.element.type, fragment: fragment);
+      _checkType(
+        fragment.element.type,
+        fragment: fragment,
+        isUsageSiteExperimental:
+            isParentExperimental || fragment.element.metadata.hasExperimental,
+      );
     }
   }
 
@@ -158,95 +174,136 @@ class _Visitor extends SimpleAstVisitor<void> {
         diagnosticCode: LinterLintCode.analyzerPublicApiImplInPublicApi,
       );
     }
+    var isUsageSiteExperimental = fragment.element.metadata.hasExperimental;
     switch (fragment) {
       case ExtensionFragment(name: null):
         // Unnamed extensions are not public, so ignore.
         break;
       case InstanceFragment(:var typeParameters, :var children):
         for (var typeParameter in typeParameters) {
-          _checkTypeParameter(typeParameter, fragment: fragment);
+          _checkTypeParameter(
+            typeParameter,
+            fragment: fragment,
+            isUsageSiteExperimental: isUsageSiteExperimental,
+          );
         }
         if (fragment is InterfaceFragment) {
           var element = fragment.element;
-          _checkType(element.supertype, fragment: fragment);
+          _checkType(
+            element.supertype,
+            fragment: fragment,
+            isUsageSiteExperimental: isUsageSiteExperimental,
+          );
           for (var t in element.interfaces) {
-            _checkType(t, fragment: fragment);
+            _checkType(
+              t,
+              fragment: fragment,
+              isUsageSiteExperimental: isUsageSiteExperimental,
+            );
           }
           for (var t in element.mixins) {
-            _checkType(t, fragment: fragment);
+            _checkType(
+              t,
+              fragment: fragment,
+              isUsageSiteExperimental: isUsageSiteExperimental,
+            );
           }
         }
         if (fragment case ExtensionFragment(:var element)) {
-          _checkType(element.extendedType, fragment: fragment);
+          _checkType(
+            element.extendedType,
+            fragment: fragment,
+            isUsageSiteExperimental: isUsageSiteExperimental,
+          );
         }
         if (fragment case MixinFragment(:var superclassConstraints)) {
           for (var t in superclassConstraints) {
-            _checkType(t, fragment: fragment);
+            _checkType(
+              t,
+              fragment: fragment,
+              isUsageSiteExperimental: isUsageSiteExperimental,
+            );
           }
         }
-        children.forEach(_checkMember);
+        for (var child in children) {
+          _checkMember(child, isParentExperimental: isUsageSiteExperimental);
+        }
       case ExecutableFragment():
-        _checkType(fragment.element.type, fragment: fragment);
+        _checkType(
+          fragment.element.type,
+          fragment: fragment,
+          isUsageSiteExperimental: isUsageSiteExperimental,
+        );
       case TypeAliasFragment(:var element, :var typeParameters):
         var aliasedType = element.aliasedType;
-        _checkType(aliasedType, fragment: fragment);
+        _checkType(
+          aliasedType,
+          fragment: fragment,
+          isUsageSiteExperimental: isUsageSiteExperimental,
+        );
         if (typeParameters.isNotEmpty &&
             aliasedType is FunctionType &&
             aliasedType.typeParameters.isEmpty) {
           // Sometimes `aliasedType` doesn't have the type parameters. Not sure
           // why.
           // TODO(paulberry): consider fixing this in the analyzer.
           for (var typeParameter in typeParameters) {
-            _checkTypeParameter(typeParameter, fragment: fragment);
+            _checkTypeParameter(
+              typeParameter,
+              fragment: fragment,
+              isUsageSiteExperimental: isUsageSiteExperimental,
+            );
           }
         }
     }
   }
 
-  void _checkType(DartType? type, {required Fragment fragment}) {
+  void _checkType(
+    DartType? type, {
+    required Fragment fragment,
+    required bool isUsageSiteExperimental,
+  }) {
     if (type == null) return;
-    var problems = _problemsForAnalyzerPublicApi(type);
-    if (problems.isEmpty) return;
-    int offset;
-    int length;
-    while (true) {
-      if (fragment.nameOffset != null) {
-        offset = fragment.nameOffset!;
-        length = fragment.name!.length;
-        break;
-      } else if (fragment case PropertyAccessorFragment()
-          when fragment.element.variable.firstFragment.nameOffset != null) {
-        offset = fragment.element.variable.firstFragment.nameOffset!;
-        length = fragment.element.variable.name!.length;
-        break;
-      } else if (fragment is ConstructorFragment &&
-          fragment.typeNameOffset != null) {
-        offset = fragment.typeNameOffset!;
-        length = fragment.enclosingFragment!.name!.length;
-        break;
-      } else if (fragment.enclosingFragment case var enclosingFragment?) {
-        fragment = enclosingFragment;
-      } else {
-        // This should never happen. But if it does, make sure we generate a
-        // lint anyway.
-        offset = 0;
-        length = 1;
-        break;
-      }
-    }
-    rule.reportAtOffset(
-      offset,
-      length,
-      diagnosticCode: LinterLintCode.analyzerPublicApiBadType,
-      arguments: [problems.join(', ')],
+    var nonPublicProblems = _problemsForAnalyzerPublicApi(
+      type,
+      kind: _ProblematicTypeUseKind.useOfNonPublicType,
     );
+    var experimentalProblems = isUsageSiteExperimental
+        ? const <String>{}
+        : _problemsForAnalyzerPublicApi(
+            type,
+            kind: _ProblematicTypeUseKind.useOfExperimentalType,
+          );
+    late var offsetAndLength = _offsetAndLengthForFragment(fragment);
+    if (nonPublicProblems.isNotEmpty) {
+      rule.reportAtOffset(
+        offsetAndLength.offset,
+        offsetAndLength.length,
+        diagnosticCode: LinterLintCode.analyzerPublicApiBadType,
+        arguments: [nonPublicProblems.join(', ')],
+      );
+    }
+    if (experimentalProblems.isNotEmpty) {
+      rule.reportAtOffset(
+        offsetAndLength.offset,
+        offsetAndLength.length,
+        diagnosticCode:
+            LinterLintCode.analyzerPublicApiExperimentalInconsistency,
+        arguments: [nonPublicProblems.join(', ')],
+      );
+    }
   }
 
   void _checkTypeParameter(
     TypeParameterFragment typeParameter, {
     required Fragment fragment,
+    required bool isUsageSiteExperimental,
   }) {
-    _checkType(typeParameter.element.bound, fragment: fragment);
+    _checkType(
+      typeParameter.element.bound,
+      fragment: fragment,
+      isUsageSiteExperimental: isUsageSiteExperimental,
+    );
   }
 
   /// Whether [element] is visible through at least one public import.
@@ -270,20 +327,56 @@ class _Visitor extends SimpleAstVisitor<void> {
     return false;
   }
 
-  Set<String> _problemsForAnalyzerPublicApi(DartType type) {
+  ({int length, int offset}) _offsetAndLengthForFragment(Fragment fragment) {
+    while (true) {
+      if (fragment.nameOffset != null) {
+        return (offset: fragment.nameOffset!, length: fragment.name!.length);
+      } else if (fragment case PropertyAccessorFragment()
+          when fragment.element.variable.firstFragment.nameOffset != null) {
+        return (
+          offset: fragment.element.variable.firstFragment.nameOffset!,
+          length: fragment.element.variable.name!.length,
+        );
+      } else if (fragment is ConstructorFragment &&
+          fragment.typeNameOffset != null) {
+        return (
+          offset: fragment.typeNameOffset!,
+          length: fragment.enclosingFragment!.name!.length,
+        );
+      } else if (fragment.enclosingFragment case var enclosingFragment?) {
+        // ignore: parameter_assignments
+        fragment = enclosingFragment;
+      } else {
+        // This should never happen. But if it does, make sure we generate a
+        // lint anyway.
+        return (offset: 0, length: 1);
+      }
+    }
+  }
+
+  Set<String> _problemsForAnalyzerPublicApi(
+    DartType type, {
+    required _ProblematicTypeUseKind kind,
+  }) {
     switch (type) {
       case RecordType(:var positionalFields, :var namedFields):
         return {
           for (var f in positionalFields)
-            ..._problemsForAnalyzerPublicApi(f.type),
-          for (var f in namedFields) ..._problemsForAnalyzerPublicApi(f.type),
+            ..._problemsForAnalyzerPublicApi(f.type, kind: kind),
+          for (var f in namedFields)
+            ..._problemsForAnalyzerPublicApi(f.type, kind: kind),
         };
       case InterfaceType(:var element, :var typeArguments):
+        var isProblematicElement = switch (kind) {
+          _ProblematicTypeUseKind.useOfNonPublicType =>
+            !_isPubliclyImported(element) && !element.isOkForAnalyzerPublicApi,
+          _ProblematicTypeUseKind.useOfExperimentalType =>
+            element.metadata.hasExperimental,
+        };
         return {
-          if (!_isPubliclyImported(element) &&
-              !element.isOkForAnalyzerPublicApi)
-            element.name!,
-          for (var t in typeArguments) ..._problemsForAnalyzerPublicApi(t),
+          if (isProblematicElement) element.name!,
+          for (var t in typeArguments)
+            ..._problemsForAnalyzerPublicApi(t, kind: kind),
         };
       case NeverType():
       case DynamicType():
@@ -297,11 +390,12 @@ class _Visitor extends SimpleAstVisitor<void> {
         :var formalParameters,
       ):
         return {
-          ..._problemsForAnalyzerPublicApi(returnType),
+          ..._problemsForAnalyzerPublicApi(returnType, kind: kind),
           for (var p in typeParameters)
-            if (p.bound != null) ..._problemsForAnalyzerPublicApi(p.bound!),
+            if (p.bound != null)
+              ..._problemsForAnalyzerPublicApi(p.bound!, kind: kind),
           for (var p in formalParameters)
-            ..._problemsForAnalyzerPublicApi(p.type),
+            ..._problemsForAnalyzerPublicApi(p.type, kind: kind),
         };
       default:
         throw StateError('Unexpected type $runtimeType');
diff --git a/pkg/linter/messages.yaml b/pkg/linter/messages.yaml
diff --git a/pkg/linter/test/rules/analyzer_public_api_test.dart b/pkg/linter/test/rules/analyzer_public_api_test.dart
