[analyzer] fix #33119, add strict-raw-types flag

When the flag is enabled, warnings are issued for raw types, for
example `List`.

Raw types (those without explicit type arguments) are allowed if any
of these are true, otherwise they are reported:

- none of the type arguments are `dynamic`.
- the type was inferred from a downwards context.
- the class/typedef has `@optionalTypeArgs` from package:meta.

Change-Id: I1e70dba5b3bf6118b274bbf72843e3578dbaa8fd
Reviewed-on: https://dart-review.googlesource.com/c/75629
Auto-Submit: Jenny Messerly <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: Jenny Messerly

pkg/analyzer/lib/error/error.dart

@@ -325,6 +325,7 @@ const List<ErrorCode> errorCodeValues = const [
   HintCode.OVERRIDE_ON_NON_OVERRIDING_SETTER,
   HintCode.PACKAGE_IMPORT_CONTAINS_DOT_DOT,
   HintCode.SDK_VERSION_ASYNC_EXPORTED_FROM_CORE,
+  HintCode.STRICT_RAW_TYPE,
   HintCode.SUBTYPE_OF_SEALED_CLASS,
   HintCode.TYPE_CHECK_IS_NOT_NULL,
   HintCode.TYPE_CHECK_IS_NULL,

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

@@ -188,6 +188,14 @@ 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.

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

@@ -4666,6 +4666,10 @@ class StaticWarningCode extends ErrorCode {
 
 /**
  * This class has Strong Mode specific error codes.
+ * 
+ * "Strong Mode" was the prototype for Dart 2's sound type system. Many of these
+ * errors became part of Dart 2. Some of them are optional flags, used for
+ * stricter checking.
  *
  * These error codes tend to use the same message across different severity
  * levels, so they are grouped for clarity.

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

@@ -414,6 +414,12 @@ class ElementResolver extends SimpleAstVisitor<void> {
     _resolveMetadataForParameter(node);
   }
 
+  @override
+  void visitGenericTypeAlias(GenericTypeAlias node) {
+    resolveMetadata(node);
+    return null;
+  }
+
   @override
   void visitImportDirective(ImportDirective node) {
     SimpleIdentifier prefixNode = node.prefix;

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

@@ -1511,6 +1511,15 @@ class AnalysisOptionsImpl implements AnalysisOptions {
    */
   bool strictInference = false;
 
+  /**
+   * Whether raw types (types without explicit type arguments, such as `List`)
+   * should be reported as potential problems.
+   * 
+   * Raw types are a common source of `dynamic` being introduced implicitly.
+   * This often leads to cast failures later on in the program.
+   */
+  bool strictRawTypes = false;
+
   /**
    * Initialize a newly created set of analysis options to have their default
    * values.
@@ -1542,6 +1551,7 @@ class AnalysisOptionsImpl implements AnalysisOptions {
       implicitCasts = options.implicitCasts;
       implicitDynamic = options.implicitDynamic;
       strictInference = options.strictInference;
+      strictRawTypes = options.strictRawTypes;
     }
     trackCacheDependencies = options.trackCacheDependencies;
     disableCacheFlushing = options.disableCacheFlushing;
@@ -1717,6 +1727,7 @@ class AnalysisOptionsImpl implements AnalysisOptions {
       buffer.addBool(implicitCasts);
       buffer.addBool(implicitDynamic);
       buffer.addBool(strictInference);
+      buffer.addBool(strictRawTypes);
       buffer.addBool(strongModeHints);
       buffer.addBool(useFastaParser);
 
@@ -1801,6 +1812,7 @@ class AnalysisOptionsImpl implements AnalysisOptions {
     implicitCasts = true;
     implicitDynamic = true;
     strictInference = false;
+    strictRawTypes = false;
     lint = false;
     _lintRules = null;
     patchPaths = {};

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

@@ -1000,6 +1000,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       _checkTypeArgumentCount(typeArguments, 1,
           StaticTypeWarningCode.EXPECTED_ONE_LIST_TYPE_ARGUMENTS);
     }
+    _checkForRawTypedLiteral(node);
     _checkForImplicitDynamicTypedLiteral(node);
     _checkForListElementTypeNotAssignable(node);
 
@@ -1020,6 +1021,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       _checkTypeArgumentCount(typeArguments, 2,
           StaticTypeWarningCode.EXPECTED_TWO_MAP_TYPE_ARGUMENTS);
     }
+    _checkForRawTypedLiteral(node);
     _checkForImplicitDynamicTypedLiteral(node);
     _checkForMapTypeNotAssignable(node);
     _checkForNonConstMapAsExpressionStatement(node);
@@ -1225,6 +1227,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       _checkTypeArgumentCount(typeArguments, 1,
           StaticTypeWarningCode.EXPECTED_ONE_SET_TYPE_ARGUMENTS);
     }
+    _checkForRawTypedLiteral(node);
     _checkForImplicitDynamicTypedLiteral(node);
     _checkForSetElementTypeNotAssignable(node);
 
@@ -1246,6 +1249,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         _checkTypeArgumentCount(typeArguments, 2,
             StaticTypeWarningCode.EXPECTED_TWO_MAP_TYPE_ARGUMENTS);
       }
+      _checkForRawTypedLiteral(node);
       _checkForImplicitDynamicTypedLiteral(node);
       _checkForMapTypeNotAssignable3(node);
       _checkForNonConstMapAsExpressionStatement3(node);
@@ -1261,6 +1265,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         _checkTypeArgumentCount(typeArguments, 1,
             StaticTypeWarningCode.EXPECTED_ONE_SET_TYPE_ARGUMENTS);
       }
+      _checkForRawTypedLiteral(node);
       _checkForImplicitDynamicTypedLiteral(node);
       _checkForSetElementTypeNotAssignable3(node);
     }
@@ -1358,6 +1363,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
   @override
   void visitTypeName(TypeName node) {
     _checkForTypeArgumentNotMatchingBounds(node);
+    _checkForRawTypeName(node);
     super.visitTypeName(node);
   }
 
@@ -4506,11 +4512,9 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     Map<LibraryElement, Map<String, String>> mixedInNames =
         <LibraryElement, Map<String, String>>{};
 
-    /**
-     * Report an error and return `true` if the given [name] is a private name
-     * (which is defined in the given [library]) and it conflicts with another
-     * definition of that name inherited from the superclass.
-     */
+    /// Report an error and return `true` if the given [name] is a private name
+    /// (which is defined in the given [library]) and it conflicts with another
+    /// definition of that name inherited from the superclass.
     bool isConflictingName(
         String name, LibraryElement library, TypeName typeName) {
       if (Identifier.isPrivateName(name)) {
@@ -4998,6 +5002,93 @@ 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.metadata.isNotEmpty) {
+        for (var annotation in element.metadata) {
+          var e = annotation.element;
+          // TODO(jmesserly): similar "package:meta" annotations are added to
+          // the element as boolean getters, that may be worth considering.
+          if (e?.name == 'optionalTypeArgs' &&
+              e.librarySource.uri.toString() == 'package:meta/meta.dart') {
+            // Type is marked with `@optionalTypeArgs`: not an error.
+            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.
+  ///
+  /// This checks if [node] refers to a generic type and does not have explicit
+  /// or inferred type arguments. When that happens, it reports error code
+  /// [StrongModeCode.STRICT_RAW_TYPE].
+  void _checkForRawTypeName(TypeName node) {
+    if (!_options.strictRawTypes || node == null) return;
+    if (node.typeArguments != null) {
+      // Type has explicit type arguments.
+      return;
+    }
+    var parent = node.parent;
+    InstanceCreationExpression inferenceContextNode;
+    if (parent is ConstructorName) {
+      var grandparent = parent.parent;
+      if (grandparent is InstanceCreationExpression) {
+        inferenceContextNode = grandparent;
+      }
+    }
+    return _checkForRawTypeErrors(
+        node, node.type, node.name.staticElement, inferenceContextNode);
+  }
+
   /**
    * Check whether the given constructor [declaration] is the redirecting
    * generative constructor and references itself directly or indirectly. The

pkg/analyzer/lib/src/summary/link.dart

@@ -427,6 +427,9 @@ class AnalysisOptionsForLink implements AnalysisOptionsImpl {
   @override
   bool get strictInference => false;
 
+  @override
+  bool get strictRawTypes => false;
+
   @deprecated
   @override
   bool get previewDart2 => true;

pkg/analyzer/lib/src/task/options.dart

@@ -60,6 +60,7 @@ class AnalyzerOptions {
 
   // Language options (see AnalysisOptionsImpl for documentation).
   static const String strictInference = 'strict-inference';
+  static const String strictRawTypes = 'strict-raw-types';
 
   /// Ways to say `ignore`.
   static const List<String> ignoreSynonyms = const ['ignore', 'false'];
@@ -94,6 +95,7 @@ class AnalyzerOptions {
   /// Supported `analyzer` language options.
   static const List<String> languageOptions = const [
     strictInference,
+    strictRawTypes
   ];
 }
 
@@ -704,6 +706,9 @@ class _OptionsProcessor {
       if (feature == AnalyzerOptions.strictInference) {
         options.strictInference = boolValue;
       }
+      if (feature == AnalyzerOptions.strictRawTypes) {
+        options.strictRawTypes = boolValue;
+      }
     }
   }
 

pkg/analyzer/test/src/context/builder_test.dart

@@ -953,6 +953,7 @@ environment:
     expect(actual.implicitCasts, expected.implicitCasts);
     expect(actual.implicitDynamic, expected.implicitDynamic);
     expect(actual.strictInference, expected.strictInference);
+    expect(actual.strictRawTypes, expected.strictRawTypes);
     expect(actual.trackCacheDependencies, expected.trackCacheDependencies);
     expect(actual.disableCacheFlushing, expected.disableCacheFlushing);
   }

pkg/analyzer/test/src/task/options_test.dart

@@ -373,7 +373,7 @@ analyzer:
 analyzer:
   language:
     unsupported: true
-''', [AnalysisOptionsWarningCode.UNSUPPORTED_OPTION_WITH_LEGAL_VALUE]);
+''', [AnalysisOptionsWarningCode.UNSUPPORTED_OPTION_WITH_LEGAL_VALUES]);
   }
 
   test_analyzer_strong_mode_unsupported_value() {