analyzer: skip CodeChecker for null safe libraries

In some profiling, I saw that CodeChecker accounted for ~10% of the time
spent in `LibraryAnalyzer._computeVerifyErrors`.

The only errors which are reported any longer in CodeChecker are
pre-null safe errors, and the TOP_LEVEL_CYCLE error, which is moved to
resolution.

Change-Id: I9ad38a1c76490a95cbb8464465e37a07622b4f97
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/240643
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart

@@ -440,12 +440,14 @@ class LibraryAnalyzer {
   void _computeVerifyErrors(FileState file, CompilationUnit unit) {
     ErrorReporter errorReporter = _getErrorReporter(file);
 
-    CodeChecker checker = CodeChecker(
-      _typeProvider,
-      _typeSystem,
-      errorReporter,
-    );
-    checker.visitCompilationUnit(unit);
+    if (!unit.featureSet.isEnabled(Feature.non_nullable)) {
+      CodeChecker checker = CodeChecker(
+        _typeProvider,
+        _typeSystem,
+        errorReporter,
+      );
+      checker.visitCompilationUnit(unit);
+    }
 
     //
     // Validate the directives.

pkg/analyzer/lib/src/dart/micro/library_analyzer.dart

@@ -361,12 +361,14 @@ class LibraryAnalyzer {
   void _computeVerifyErrors(FileState file, CompilationUnit unit) {
     ErrorReporter errorReporter = _getErrorReporter(file);
 
-    CodeChecker checker = CodeChecker(
-      _typeProvider,
-      _typeSystem,
-      errorReporter,
-    );
-    checker.visitCompilationUnit(unit);
+    if (!unit.featureSet.isEnabled(Feature.non_nullable)) {
+      CodeChecker checker = CodeChecker(
+        _typeProvider,
+        _typeSystem,
+        errorReporter,
+      );
+      checker.visitCompilationUnit(unit);
+    }
 
     //
     // Validate the directives.

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

@@ -7688,6 +7688,8 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
    * Parameters:
    * 0: the type of the function
    * 1: the expected function type
+   *
+   * This error is only reported in libraries which are not null safe.
    */
   static const CompileTimeErrorCode INVALID_CAST_FUNCTION =
       CompileTimeErrorCode(
@@ -7700,6 +7702,8 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
    * Parameters:
    * 0: the type of the torn-off function expression
    * 1: the expected function type
+   *
+   * This error is only reported in libraries which are not null safe.
    */
   static const CompileTimeErrorCode INVALID_CAST_FUNCTION_EXPR =
       CompileTimeErrorCode(
@@ -7713,6 +7717,8 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
    * Parameters:
    * 0: the type of the literal
    * 1: the expected type
+   *
+   * This error is only reported in libraries which are not null safe.
    */
   static const CompileTimeErrorCode INVALID_CAST_LITERAL = CompileTimeErrorCode(
     'INVALID_CAST_LITERAL',
@@ -7723,6 +7729,8 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
    * Parameters:
    * 0: the type of the list literal
    * 1: the expected type
+   *
+   * This error is only reported in libraries which are not null safe.
    */
   static const CompileTimeErrorCode INVALID_CAST_LITERAL_LIST =
       CompileTimeErrorCode(
@@ -7736,6 +7744,8 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
    * Parameters:
    * 0: the type of the map literal
    * 1: the expected type
+   *
+   * This error is only reported in libraries which are not null safe.
    */
   static const CompileTimeErrorCode INVALID_CAST_LITERAL_MAP =
       CompileTimeErrorCode(
@@ -7749,6 +7759,8 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
    * Parameters:
    * 0: the type of the set literal
    * 1: the expected type
+   *
+   * This error is only reported in libraries which are not null safe.
    */
   static const CompileTimeErrorCode INVALID_CAST_LITERAL_SET =
       CompileTimeErrorCode(
@@ -7762,6 +7774,8 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
    * Parameters:
    * 0: the type of the torn-off method
    * 1: the expected function type
+   *
+   * This error is only reported in libraries which are not null safe.
    */
   static const CompileTimeErrorCode INVALID_CAST_METHOD = CompileTimeErrorCode(
     'INVALID_CAST_METHOD',
@@ -7774,6 +7788,8 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
    * Parameters:
    * 0: the type of the instantiated object
    * 1: the expected type
+   *
+   * This error is only reported in libraries which are not null safe.
    */
   static const CompileTimeErrorCode INVALID_CAST_NEW_EXPR =
       CompileTimeErrorCode(

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

@@ -73,6 +73,7 @@ import 'package:analyzer/src/generated/static_type_analyzer.dart';
 import 'package:analyzer/src/generated/this_access_tracker.dart';
 import 'package:analyzer/src/generated/utilities_dart.dart';
 import 'package:analyzer/src/generated/variable_type_provider.dart';
+import 'package:analyzer/src/task/inference_error.dart';
 import 'package:analyzer/src/util/ast_data_extractor.dart';
 import 'package:meta/meta.dart';
 
@@ -2488,6 +2489,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
           isLate: parent.isLate,
           isImplicitlyTyped: declaredType == null);
     }
+    _checkTopLevelCycle(node);
   }
 
   @override
@@ -2537,6 +2539,27 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
     _yieldStatementResolver.resolve(node);
   }
 
+  void _checkTopLevelCycle(VariableDeclaration node) {
+    var element = node.declaredElement;
+    if (element is! PropertyInducingElementImpl) {
+      return;
+    }
+    // Errors on const are reported separately with
+    // [CompileTimeErrorCode.RECURSIVE_COMPILE_TIME_CONSTANT].
+    if (element.isConst) {
+      return;
+    }
+    var error = element.typeInferenceError;
+    if (error == null) {
+      return;
+    }
+    if (error.kind == TopLevelInferenceErrorKind.dependencyCycle) {
+      var argumentsText = error.arguments.join(', ');
+      errorReporter.reportErrorForNode(CompileTimeErrorCode.TOP_LEVEL_CYCLE,
+          node.name, [node.name.name, argumentsText]);
+    }
+  }
+
   /// Creates a union of `T | Future<T>`, unless `T` is already a
   /// future-union, in which case it simply returns `T`.
   DartType _createFutureOr(DartType type) {

pkg/analyzer/lib/src/task/strong/checker.dart

@@ -2,7 +2,6 @@
 // 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.
 
-import 'package:analyzer/dart/analysis/features.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
@@ -13,11 +12,9 @@ import 'package:analyzer/dart/element/type_provider.dart';
 import 'package:analyzer/error/error.dart';
 import 'package:analyzer/error/listener.dart';
 import 'package:analyzer/src/dart/ast/extensions.dart';
-import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type_system.dart';
 import 'package:analyzer/src/error/codes.dart' show CompileTimeErrorCode;
-import 'package:analyzer/src/task/inference_error.dart';
 
 Element? _getKnownElement(Expression expression) {
   if (expression is ParenthesizedExpression) {
@@ -34,25 +31,15 @@ Element? _getKnownElement(Expression expression) {
   return null;
 }
 
-/// Checks the body of functions and properties.
+/// Checks the body of functions and properties for implicit cast errors in
+/// pre-null safe libraries.
 class CodeChecker extends RecursiveAstVisitor {
   final TypeSystemImpl _typeSystem;
   final TypeProvider _typeProvider;
   final ErrorReporter _errorReporter;
 
-  late final FeatureSet _featureSet;
-
   CodeChecker(this._typeProvider, this._typeSystem, this._errorReporter);
 
-  bool get _isNonNullableByDefault =>
-      _featureSet.isEnabled(Feature.non_nullable);
-
-  NullabilitySuffix get _noneOrStarSuffix {
-    return _isNonNullableByDefault
-        ? NullabilitySuffix.none
-        : NullabilitySuffix.star;
-  }
-
   void checkArgument(Expression arg, DartType expectedType) {
     // Preserve named argument structure, so their immediate parent is the
     // method invocation.
@@ -172,7 +159,6 @@ class CodeChecker extends RecursiveAstVisitor {
 
   @override
   void visitCompilationUnit(CompilationUnit node) {
-    _featureSet = node.featureSet;
     node.visitChildren(this);
   }
 
@@ -385,28 +371,6 @@ class CodeChecker extends RecursiveAstVisitor {
     node.visitChildren(this);
   }
 
-  @override
-  void visitVariableDeclaration(VariableDeclaration node) {
-    var element = node.declaredElement;
-    if (element is PropertyInducingElementImpl) {
-      var error = element.typeInferenceError;
-      if (error != null) {
-        if (error.kind == TopLevelInferenceErrorKind.dependencyCycle) {
-          // Errors on const should have been reported with
-          // [CompileTimeErrorCode.RECURSIVE_COMPILE_TIME_CONSTANT].
-          if (!element.isConst) {
-            _recordMessage(
-              node.name,
-              CompileTimeErrorCode.TOP_LEVEL_CYCLE,
-              [element.name, error.arguments],
-            );
-          }
-        }
-      }
-    }
-    node.visitChildren(this);
-  }
-
   @override
   void visitVariableDeclarationList(VariableDeclarationList node) {
     var type = node.type;
@@ -520,10 +484,6 @@ class CodeChecker extends RecursiveAstVisitor {
       bool forSpread = false,
       bool forSpreadKey = false,
       bool forSpreadValue = false}) {
-    if (_isNonNullableByDefault) {
-      return;
-    }
-
     expr = expr.unParenthesized;
     if (_needsImplicitCast(expr, to: to, from: from) == true) {
       _recordImplicitCast(expr, to,
@@ -627,7 +587,7 @@ class CodeChecker extends RecursiveAstVisitor {
         // Ensure it's at least a Stream / Iterable.
         return expectedElement.instantiate(
           typeArguments: [_typeProvider.dynamicType],
-          nullabilitySuffix: _noneOrStarSuffix,
+          nullabilitySuffix: NullabilitySuffix.star,
         );
       } else {
         // Analyzer will provide a separate error if expected type
@@ -836,7 +796,7 @@ class CodeChecker extends RecursiveAstVisitor {
     if (elementType == null) {
       var sequenceType = sequenceElement.instantiate(
         typeArguments: [_typeProvider.dynamicType],
-        nullabilitySuffix: _noneOrStarSuffix,
+        nullabilitySuffix: NullabilitySuffix.star,
       );
 
       if (_typeSystem.isSubtypeOf(sequenceType, iterableType)) {

pkg/analyzer/messages.yaml

@@ -6771,48 +6771,64 @@ CompileTimeErrorCode:
       Parameters:
       0: the type of the function
       1: the expected function type
+
+      This error is only reported in libraries which are not null safe.
   INVALID_CAST_FUNCTION_EXPR:
     problemMessage: "The function expression type '{0}' isn't of type '{1}'. This means its parameter or return type doesn't match what is expected. Consider changing parameter type(s) or the returned type(s)."
     comment: |-
       Parameters:
       0: the type of the torn-off function expression
       1: the expected function type
+
+      This error is only reported in libraries which are not null safe.
   INVALID_CAST_LITERAL:
     problemMessage: "The literal '{0}' with type '{1}' isn't of expected type '{2}'."
     comment: |-
       Parameters:
       0: the type of the literal
       1: the expected type
+
+      This error is only reported in libraries which are not null safe.
   INVALID_CAST_LITERAL_LIST:
     problemMessage: "The list literal type '{0}' isn't of expected type '{1}'. The list's type can be changed with an explicit generic type argument or by changing the element types."
     comment: |-
       Parameters:
       0: the type of the list literal
       1: the expected type
+
+      This error is only reported in libraries which are not null safe.
   INVALID_CAST_LITERAL_MAP:
     problemMessage: "The map literal type '{0}' isn't of expected type '{1}'. The map's type can be changed with an explicit generic type arguments or by changing the key and value types."
     comment: |-
       Parameters:
       0: the type of the map literal
       1: the expected type
+
+      This error is only reported in libraries which are not null safe.
   INVALID_CAST_LITERAL_SET:
     problemMessage: "The set literal type '{0}' isn't of expected type '{1}'. The set's type can be changed with an explicit generic type argument or by changing the element types."
     comment: |-
       Parameters:
       0: the type of the set literal
       1: the expected type
+
+      This error is only reported in libraries which are not null safe.
   INVALID_CAST_METHOD:
     problemMessage: "The method tear-off '{0}' has type '{1}' that isn't of expected type '{2}'. This means its parameter or return type doesn't match what is expected."
     comment: |-
       Parameters:
       0: the type of the torn-off method
       1: the expected function type
+
+      This error is only reported in libraries which are not null safe.
   INVALID_CAST_NEW_EXPR:
     problemMessage: "The constructor returns type '{0}' that isn't of expected type '{1}'."
     comment: |-
       Parameters:
       0: the type of the instantiated object
       1: the expected type
+
+      This error is only reported in libraries which are not null safe.
   INVALID_CONSTANT:
     problemMessage: Invalid constant value.
     comment: |-