Re-work collection literal strict-raw-type to strict-inference

Bug: #36445 #33749
Change-Id: I127d671840bfcfe3857225932164fa4098963715
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/99085
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: srawlins

pkg/analyzer/lib/error/error.dart

@@ -296,6 +296,7 @@ const List<ErrorCode> errorCodeValues = const [
   HintCode.FILE_IMPORT_INSIDE_LIB_REFERENCES_FILE_OUTSIDE,
   HintCode.FILE_IMPORT_OUTSIDE_LIB_REFERENCES_FILE_INSIDE,
   HintCode.IMPORT_DEFERRED_LIBRARY_WITH_LOAD_FUNCTION,
+  HintCode.INFERENCE_FAILURE_ON_COLLECTION_LITERAL,
   HintCode.INFERENCE_FAILURE_ON_UNINITIALIZED_VARIABLE,
   HintCode.INVALID_FACTORY_ANNOTATION,
   HintCode.INVALID_FACTORY_METHOD_DECL,

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

@@ -177,6 +177,15 @@ class HintCode extends ErrorCode {
           correction: "Try changing the import to not be deferred, or "
               "rename the function in the imported library.");
 
+  /**
+   * When "strict-inference" is enabled, collection literal types must be
+   * inferred via the context type, or have type arguments.
+   */
+  static const HintCode INFERENCE_FAILURE_ON_COLLECTION_LITERAL = HintCode(
+      'INFERENCE_FAILURE_ON_COLLECTION_LITERAL',
+      "The type argument(s) of '{0}' cannot be inferred.",
+      correction: "Use explicit type argument(s) for '{0}'.");
+
   /**
    * When "strict-inference" in enabled, uninitialized variables must be
    * declared with a specific type.
@@ -188,14 +197,6 @@ class HintCode extends ErrorCode {
           "initializer.",
           correction: "Try specifying the type of the variable.");
 
-  /**
-   * When "strict-raw-types" is enabled, raw types must be inferred via the
-   * context type, or have type arguments other than dynamic.
-   */
-  static const HintCode STRICT_RAW_TYPE = HintCode('STRICT_RAW_TYPE',
-      "The generic type '{0}' should have explicit type arguments but doesn't.",
-      correction: "Use explicit type arguments for '{0}'.");
-
   /**
    * This hint is generated anywhere a @factory annotation is associated with
    * anything other than a method.
@@ -628,6 +629,14 @@ class HintCode extends ErrorCode {
       "but this code is required to be able to run on earlier versions.",
       correction: "Try updating the SDK constraints.");
 
+  /**
+   * When "strict-raw-types" is enabled, raw types must be inferred via the
+   * context type, or have type arguments.
+   */
+  static const HintCode STRICT_RAW_TYPE = HintCode('STRICT_RAW_TYPE',
+      "The generic type '{0}' should have explicit type arguments but doesn't.",
+      correction: "Use explicit type arguments for '{0}'.");
+
   /**
    * This hint is generated anywhere where a `@sealed` class or mixin is used as
    * a super-type of a class.

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

@@ -994,7 +994,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       _checkTypeArgumentCount(typeArguments, 1,
           StaticTypeWarningCode.EXPECTED_ONE_LIST_TYPE_ARGUMENTS);
     }
-    _checkForRawTypedLiteral(node);
+    _checkForInferenceFailureOnCollectionLiteral(node);
     _checkForImplicitDynamicTypedLiteral(node);
     _checkForListElementTypeNotAssignable(node);
 
@@ -1201,7 +1201,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         _checkTypeArgumentCount(typeArguments, 2,
             StaticTypeWarningCode.EXPECTED_TWO_MAP_TYPE_ARGUMENTS);
       }
-      _checkForRawTypedLiteral(node);
+      _checkForInferenceFailureOnCollectionLiteral(node);
       _checkForImplicitDynamicTypedLiteral(node);
       _checkForMapTypeNotAssignable(node);
       _checkForNonConstMapAsExpressionStatement3(node);
@@ -1217,7 +1217,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         _checkTypeArgumentCount(typeArguments, 1,
             StaticTypeWarningCode.EXPECTED_ONE_SET_TYPE_ARGUMENTS);
       }
-      _checkForRawTypedLiteral(node);
+      _checkForInferenceFailureOnCollectionLiteral(node);
       _checkForImplicitDynamicTypedLiteral(node);
       _checkForSetElementTypeNotAssignable3(node);
     }
@@ -3666,6 +3666,25 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         [directive.uri.stringValue]);
   }
 
+  /// Checks a collection literal for an inference failure, and reports the
+  /// appropriate error if [AnalysisOptionsImpl.strictInference] is set.
+  ///
+  /// This checks if [node] does not have explicit or inferred type arguments.
+  /// When that happens, it reports a
+  /// HintCode.INFERENCE_FAILURE_ON_COLLECTION_LITERAL error.
+  void _checkForInferenceFailureOnCollectionLiteral(TypedLiteral node) {
+    if (!_options.strictInference || node == null) return;
+    if (node.typeArguments != null) {
+      // Type has explicit type arguments.
+      return;
+    }
+    var type = node.staticType;
+    if (_isMissingTypeArguments(node, type, type.element, node)) {
+      _errorReporter.reportErrorForNode(
+          HintCode.INFERENCE_FAILURE_ON_COLLECTION_LITERAL, node, [type.name]);
+    }
+  }
+
   /**
    * Check that the given [typeReference] is not a type reference and that then
    * the [name] is reference to an instance member.
@@ -4692,60 +4711,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         CompileTimeErrorCode.PRIVATE_OPTIONAL_PARAMETER, parameter);
   }
 
-  /// Similar to [_checkForRawTypeName] but for list/map/set literals.
-  void _checkForRawTypedLiteral(TypedLiteral node) {
-    if (!_options.strictRawTypes || node == null) return;
-    if (node.typeArguments != null) {
-      // Type has explicit type arguments.
-      return;
-    }
-    var type = node.staticType;
-    return _checkForRawTypeErrors(node, type, type.element, node);
-  }
-
-  /// Given a [node] without type arguments that refers to [element], issues
-  /// an error if [type] is a generic type, and the type arguments were not
-  /// supplied from inference or a non-dynamic default instantiation.
-  ///
-  /// This function is used by other node-specific raw type checking functions
-  /// (for example [_checkForRawTypeName]), and should only be called when
-  /// we already know [AnalysisOptionsImpl.strictRawTypes] is true and [node]
-  /// has no explicit `typeArguments`.
-  ///
-  /// [inferenceContextNode] is the node that has the downwards context type,
-  /// if any. For example an [InstanceCreationExpression].
-  ///
-  /// The raw type error [HintCode.STRICT_RAW_TYPE] will *not* be reported when
-  /// any of the following are true:
-  ///
-  /// - [inferenceContextNode] has an inference context type that does not
-  ///   contain `?`
-  /// - [type] does not have any `dynamic` type arguments.
-  /// - the element is marked with `@optionalTypeArgs` from "package:meta".
-  void _checkForRawTypeErrors(AstNode node, DartType type, Element element,
-      Expression inferenceContextNode) {
-    assert(_options.strictRawTypes);
-    // Check if this type has type arguments and at least one is dynamic.
-    // If so, we may need to issue a strict-raw-types error.
-    if (type is ParameterizedType &&
-        type.typeArguments.any((t) => t.isDynamic)) {
-      // If we have an inference context node, check if the type was inferred
-      // from it. Some cases will not have a context type, such as the type
-      // annotation `List` in `List list;`
-      if (inferenceContextNode != null) {
-        var contextType = InferenceContext.getContext(inferenceContextNode);
-        if (contextType != null && UnknownInferredType.isKnown(contextType)) {
-          // Type was inferred from downwards context: not an error.
-          return;
-        }
-      }
-      if (element.hasOptionalTypeArgs) {
-        return;
-      }
-      _errorReporter.reportErrorForNode(HintCode.STRICT_RAW_TYPE, node, [type]);
-    }
-  }
-
   /// Checks a type annotation for a raw generic type, and reports the
   /// appropriate error if [AnalysisOptionsImpl.strictRawTypes] is set.
   ///
@@ -4766,8 +4731,11 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         inferenceContextNode = grandparent;
       }
     }
-    return _checkForRawTypeErrors(
-        node, node.type, node.name.staticElement, inferenceContextNode);
+    if (_isMissingTypeArguments(
+        node, node.type, node.name.staticElement, inferenceContextNode)) {
+      _errorReporter
+          .reportErrorForNode(HintCode.STRICT_RAW_TYPE, node, [node.type]);
+    }
   }
 
   /**
@@ -6111,6 +6079,46 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     return false;
   }
 
+  /// Given a [node] without type arguments that refers to [element], issues
+  /// an error if [type] is a generic type, and the type arguments were not
+  /// supplied from inference or a non-dynamic default instantiation.
+  ///
+  /// This function is used by other node-specific type checking functions, and
+  /// should only be called when [node] has no explicit `typeArguments`.
+  ///
+  /// [inferenceContextNode] is the node that has the downwards context type,
+  /// if any. For example an [InstanceCreationExpression].
+  ///
+  /// This function will return false if any of the following are true:
+  ///
+  /// - [inferenceContextNode] has an inference context type that does not
+  ///   contain `?`
+  /// - [type] does not have any `dynamic` type arguments.
+  /// - the element is marked with `@optionalTypeArgs` from "package:meta".
+  bool _isMissingTypeArguments(AstNode node, DartType type, Element element,
+      Expression inferenceContextNode) {
+    // Check if this type has type arguments and at least one is dynamic.
+    // If so, we may need to issue a strict-raw-types error.
+    if (type is ParameterizedType &&
+        type.typeArguments.any((t) => t.isDynamic)) {
+      // If we have an inference context node, check if the type was inferred
+      // from it. Some cases will not have a context type, such as the type
+      // annotation `List` in `List list;`
+      if (inferenceContextNode != null) {
+        var contextType = InferenceContext.getContext(inferenceContextNode);
+        if (contextType != null && UnknownInferredType.isKnown(contextType)) {
+          // Type was inferred from downwards context: not an error.
+          return false;
+        }
+      }
+      if (element.hasOptionalTypeArgs) {
+        return false;
+      }
+      return true;
+    }
+    return false;
+  }
+
   /**
    * Return `true` if the given 'this' [expression] is in a valid context.
    */

pkg/analyzer/test/src/task/strong/checker_test.dart

@@ -3649,6 +3649,25 @@ class Child extends Base {
 ''');
   }
 
+  test_strictInference_collectionLiterals() async {
+    addFile(r'''
+main() {
+  var emptyList = /*info:INFERENCE_FAILURE_ON_COLLECTION_LITERAL*/[];
+  var emptyMap = /*info:INFERENCE_FAILURE_ON_COLLECTION_LITERAL*/{};
+
+  var upwardsInfersDynamicList = /*info:INFERENCE_FAILURE_ON_COLLECTION_LITERAL*/[42 as dynamic];
+  var upwardsInfersDynamicSet = /*info:INFERENCE_FAILURE_ON_COLLECTION_LITERAL*/{42 as dynamic};
+
+
+  dynamic d;
+  var upwardsInfersDynamicMap1 = /*info:INFERENCE_FAILURE_ON_COLLECTION_LITERAL*/{d: 2};
+  var upwardsInfersDynamicMap2 = /*info:INFERENCE_FAILURE_ON_COLLECTION_LITERAL*/{4: d};
+  var upwardsInfersDynamicMap3 = /*info:INFERENCE_FAILURE_ON_COLLECTION_LITERAL*/{d: d};
+}
+    ''');
+    await check(strictInference: true);
+  }
+
   test_strictRawTypes_classes() async {
     addFile(r'''
 class C<T> {
@@ -3702,11 +3721,8 @@ main() {
     var upwardsInferNonDynamicIsOK = [42];
     var explicitDynamicIsOK = <dynamic>[42];
 
-    var rawList = /*info:STRICT_RAW_TYPE*/[];
     var rawListOfLists = </*info:STRICT_RAW_TYPE*/List>[];
     /*info:STRICT_RAW_TYPE*/List rawListFromType = [];
-
-    var upwardsInfersDynamic = /*info:STRICT_RAW_TYPE*/[42 as dynamic];
   }
 
   {
@@ -3716,11 +3732,8 @@ main() {
     var upwardsInferNonDynamicIsOK = [42];
     var explicitDynamicIsOK = <dynamic>[42];
 
-    var rawList = /*info:STRICT_RAW_TYPE*/[];
     var rawListOfLists = </*info:STRICT_RAW_TYPE*/List>[];
     /*info:STRICT_RAW_TYPE*/List rawListFromType = [];
-
-    var upwardsInfersDynamic = /*info:STRICT_RAW_TYPE*/[42 as dynamic];
   }
 
   {
@@ -3732,8 +3745,6 @@ main() {
 
     var rawSetOfSets = </*info:STRICT_RAW_TYPE*/Set>{};
     /*info:STRICT_RAW_TYPE*/Set rawSetFromType = {};
-
-    var upwardsInfersDynamic = /*info:STRICT_RAW_TYPE*/{42 as dynamic};
   }
 
   {
@@ -3748,11 +3759,6 @@ main() {
 
     var rawMapOfMaps = </*info:STRICT_RAW_TYPE*/Map>{};
     /*info:STRICT_RAW_TYPE*/Map rawMapFromType = {};
-
-    dynamic d;
-    var upwardsInfersDynamic1 = /*info:STRICT_RAW_TYPE*/{d: 2};
-    var upwardsInfersDynamic2 = /*info:STRICT_RAW_TYPE*/{4: d};
-    var upwardsInfersDynamic3 = /*info:STRICT_RAW_TYPE*/{d: d};
   }
 
   {
@@ -3815,15 +3821,6 @@ main() {
     await check(strictRawTypes: true);
   }
 
-  test_strictRawTypes_emptyMap() async {
-    addFile('''
-main() {
-  var rawMap = /*info:STRICT_RAW_TYPE*/{};
-}
-''');
-    await check(strictRawTypes: true);
-  }
-
   test_strictRawTypes_typedefs() async {
     addFile(r'''
 typedef T F1<T>(T _);

pkg/analyzer/test/src/task/strong/strong_test_helper.dart

@@ -280,6 +280,7 @@ class AbstractStrongTest with ResourceProviderMixin {
   Future<CompilationUnit> check(
       {bool implicitCasts: true,
       bool implicitDynamic: true,
+      bool strictInference: false,
       bool strictRawTypes: false}) async {
     _checkCalled = true;
 
@@ -290,6 +291,7 @@ class AbstractStrongTest with ResourceProviderMixin {
     analysisOptions.strongModeHints = true;
     analysisOptions.implicitCasts = implicitCasts;
     analysisOptions.implicitDynamic = implicitDynamic;
+    analysisOptions.strictInference = strictInference;
     analysisOptions.strictRawTypes = strictRawTypes;
     analysisOptions.enabledExperiments = enabledExperiments;
 
@@ -330,9 +332,11 @@ class AbstractStrongTest with ResourceProviderMixin {
           code == TodoCode.TODO) {
         return false;
       }
-      if (strictRawTypes) {
-        // When testing strict-raw-types, ignore anything else.
+      if (strictInference || strictRawTypes) {
+        // When testing strict-inference or strict-raw-types, ignore anything
+        // else.
         return code.errorSeverity.ordinal > ErrorSeverity.INFO.ordinal ||
+            code == HintCode.INFERENCE_FAILURE_ON_COLLECTION_LITERAL ||
             code == HintCode.STRICT_RAW_TYPE;
       }
       return true;
@@ -343,6 +347,7 @@ class AbstractStrongTest with ResourceProviderMixin {
     LibraryElement mainLibrary =
         resolutionMap.elementDeclaredByCompilationUnit(mainUnit).library;
     Set<LibraryElement> allLibraries = _reachableLibraries(mainLibrary);
+
     for (LibraryElement library in allLibraries) {
       for (CompilationUnitElement unit in library.units) {
         var source = unit.source;