Revert "linter: Refactor prefer_collection_literals to use context type more"

This reverts commit cd8a337.

Reason for revert: lint starts barking at the wrong tree: b/298917960

Original change's description:
> linter: Refactor prefer_collection_literals to use context type more
>
> There is a basic premise in this rule which we cannot satisfy exactly:
> we need to disallow `LinkedHashSet()` unless the context type requires
> the developer to use `LinkedHashSet`. But the context type is long
> gone when the lint rule is run.
>
> This CL adds some logic to try to attempt figuring out the context
> type in the cases where users have filed bugs, but it will never be
> super accurate.
>
> Fixes https://github.com/dart-lang/linter/issues/4736
> Fixes https://github.com/dart-lang/linter/issues/3057
> Fixes https://github.com/dart-lang/linter/issues/1649
> Fixes https://github.com/dart-lang/linter/issues/2795
>
> Change-Id: I3e6c6de81084dca2825488c89830ab3e7ea63186
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323680
> Reviewed-by: Phil Quitslund <[email protected]>
> Reviewed-by: Konstantin Shcheglov <[email protected]>
> Commit-Queue: Samuel Rawlins <[email protected]>

Change-Id: I980053dd51ffd4447721e0ad7436b07ca704b554
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324021
Bot-Commit: Rubber Stamper <[email protected]>
Reviewed-by: Alexander Thomas <[email protected]>
Commit-Queue: Ilya Yanok <[email protected]>

Author: Ilya Yanok

pkg/analysis_server/test/src/services/correction/fix/convert_to_map_literal_test.dart

@@ -85,6 +85,28 @@ var m = Map<String, int>();
 ''');
     await assertHasFix('''
 var m = <String, int>{};
+''');
+  }
+
+  Future<void> test_typedef() async {
+    await resolveTestCode('''
+typedef M = Map<String, int>;
+var m = M();
+''');
+    await assertHasFix('''
+typedef M = Map<String, int>;
+var m = <String, int>{};
+''');
+  }
+
+  Future<void> test_typedef_declaredType() async {
+    await resolveTestCode('''
+typedef M = Map<String, int>;
+Map<String, int> m = M();
+''');
+    await assertHasFix('''
+typedef M = Map<String, int>;
+Map<String, int> m = {};
 ''');
   }
 }

pkg/linter/lib/src/rules/prefer_collection_literals.dart

@@ -4,11 +4,7 @@
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
-import 'package:analyzer/dart/element/type_provider.dart';
-// ignore: implementation_imports
-import 'package:analyzer/src/dart/element/type.dart' show InvalidTypeImpl;
 
 import '../analyzer.dart';
 import '../extensions.dart';
@@ -36,8 +32,8 @@ var coordinates = <int, int>{};
 
 **EXCEPTIONS:**
 
-When a `LinkedHashSet` or `LinkedHashMap` is expected, a collection literal is
-not preferred (or allowed).
+There are cases with `LinkedHashSet` or `LinkedHashMap` where a literal constructor
+will trigger a type error so those will be excluded from the lint.
 
 ```dart
 void main() {
@@ -76,49 +72,40 @@ class PreferCollectionLiterals extends LintRule {
   @override
   void registerNodeProcessors(
       NodeLintRegistry registry, LinterContext context) {
-    var visitor = _Visitor(this, context.typeProvider);
+    var visitor = _Visitor(this);
     registry.addInstanceCreationExpression(this, visitor);
     registry.addMethodInvocation(this, visitor);
   }
 }
 
 class _Visitor extends SimpleAstVisitor<void> {
   final LintRule rule;
-  final TypeProvider typeProvider;
-  _Visitor(this.rule, this.typeProvider);
+  _Visitor(this.rule);
 
   @override
   void visitInstanceCreationExpression(InstanceCreationExpression node) {
     var constructorName = node.constructorName.name?.name;
 
-    if (node.constructorName.type.element is TypeAliasElement) {
-      // Allow the use of typedef constructors.
-      return;
-    }
-
     // Maps.
-    if (node.isHashMap) {
-      var approximateContextType = _approximateContextType(node);
-      if (approximateContextType is InvalidType) return;
-      if (approximateContextType.isTypeHashMap) return;
-    }
-    if (node.isMap || node.isHashMap) {
+    if (_isMap(node) || _isHashMap(node)) {
+      if (_shouldSkipLinkedHashLint(node, _isTypeHashMap)) {
+        return;
+      }
       if (constructorName == null && node.argumentList.arguments.isEmpty) {
         rule.reportLint(node);
       }
       return;
     }
 
     // Sets.
-    if (node.isHashSet) {
-      var approximateContextType = _approximateContextType(node);
-      if (approximateContextType is InvalidType) return;
-      if (approximateContextType.isTypeHashSet) return;
-    }
-    if (node.isSet || node.isHashSet) {
+    if (_isSet(node) || _isHashSet(node)) {
+      if (_shouldSkipLinkedHashLint(node, _isTypeHashSet)) {
+        return;
+      }
+
       var args = node.argumentList.arguments;
       if (constructorName == null) {
-        // Allow `LinkedHashSet(equals: (a, b) => false, hashCode: (o) => 13)`.
+        // Skip: LinkedHashSet(equals: (a, b) => false, hashCode: (o) => 13)
         if (args.isEmpty) {
           rule.reportLint(node);
         }
@@ -135,7 +122,7 @@ class _Visitor extends SimpleAstVisitor<void> {
 
   @override
   void visitMethodInvocation(MethodInvocation node) {
-    // Something like `['foo', 'bar', 'baz'].toSet()`.
+    // ['foo', 'bar', 'baz'].toSet();
     if (node.methodName.name != 'toSet') {
       return;
     }
@@ -144,130 +131,64 @@ class _Visitor extends SimpleAstVisitor<void> {
     }
   }
 
-  /// A very, very rough approximation of the context type of [node].
-  ///
-  /// This approximation will never be accurate for some expressions.
-  DartType? _approximateContextType(Expression node) {
-    var ancestor = node.parent;
-    var ancestorChild = node;
-    while (ancestor != null) {
-      if (ancestor is ParenthesizedExpression) {
-        ancestorChild = ancestor;
-        ancestor = ancestor.parent;
-      } else if (ancestor is CascadeExpression &&
-          ancestorChild == ancestor.target) {
-        ancestorChild = ancestor;
-        ancestor = ancestor.parent;
-      } else {
-        break;
-      }
-    }
+  bool _isHashMap(Expression expression) =>
+      _isTypeHashMap(expression.staticType);
 
-    switch (ancestor) {
-      // TODO(srawlins): Handle [AwaitExpression], [BinaryExpression],
-      // [CascadeExpression], [ConditionalExpression], [SwitchExpressionCase],
-      // likely others. Or move everything here to an analysis phase which
-      // has the actual context type.
-      case ArgumentList():
-        // Allow `function(LinkedHashSet())` for `function(LinkedHashSet mySet)`
-        // and `function(LinkedHashMap())` for `function(LinkedHashMap myMap)`.
-        return node.staticParameterElement?.type ?? InvalidTypeImpl.instance;
-      case AssignmentExpression():
-        // Allow `x = LinkedHashMap()`.
-        return ancestor.staticType;
-      case ExpressionFunctionBody(parent: var function)
-          when function is FunctionExpression:
-        // Allow `<int, LinkedHashSet>{}.putIfAbsent(3, () => LinkedHashSet())`
-        // and `<int, LinkedHashMap>{}.putIfAbsent(3, () => LinkedHashMap())`.
-        var functionType = _approximateContextType(function);
-        return functionType is FunctionType ? functionType.returnType : null;
-      case NamedExpression():
-        // Allow `void f({required LinkedHashSet<Foo> s})`.
-        return ancestor.staticParameterElement?.type ??
-            InvalidTypeImpl.instance;
-      case ReturnStatement():
-        return _expectedReturnType(
-          ancestor.thisOrAncestorOfType<FunctionBody>(),
-        );
-      case VariableDeclaration(parent: VariableDeclarationList(:var type)):
-        // Allow `LinkedHashSet<int> s = node` and
-        // `LinkedHashMap<int> s = node`.
-        return type?.type;
-      case YieldStatement():
-        return _expectedReturnType(
-          ancestor.thisOrAncestorOfType<FunctionBody>(),
-        );
-    }
+  bool _isHashSet(Expression expression) =>
+      _isTypeHashSet(expression.staticType);
 
-    return null;
-  }
+  bool _isMap(Expression expression) =>
+      expression.staticType?.isDartCoreMap ?? false;
 
-  /// Extracts the expected type for return statements or yield statements.
-  ///
-  /// For example, for an asynchronous [body] in a function with a declared
-  /// [returnType] of `Future<int>`, this returns `int`. (Note: it would be more
-  /// accurate to use `FutureOr<int>` and an assignability check, but `int` is
-  /// an approximation that works for now; this should probably be revisited.)
-  DartType? _expectedReturnableOrYieldableType(
-    DartType? returnType,
-    FunctionBody body,
-  ) {
-    if (returnType == null || returnType is InvalidType) return null;
-    if (body.isAsynchronous) {
-      if (!body.isGenerator && returnType.isDartAsyncFuture) {
-        var typeArgs = (returnType as InterfaceType).typeArguments;
-        return typeArgs.isEmpty ? null : typeArgs.first;
+  bool _isSet(Expression expression) =>
+      expression.staticType?.isDartCoreSet ?? false;
+
+  bool _isTypeHashMap(DartType? type) =>
+      type.isSameAs('LinkedHashMap', 'dart.collection');
+
+  bool _isTypeHashSet(DartType? type) =>
+      type.isSameAs('LinkedHashSet', 'dart.collection');
+
+  bool _shouldSkipLinkedHashLint(
+      InstanceCreationExpression node, bool Function(DartType node) typeCheck) {
+    if (_isHashMap(node) || _isHashSet(node)) {
+      // Skip: LinkedHashSet<int> s =  ...; or LinkedHashMap<int> s =  ...;
+      var parent = node.parent;
+      if (parent is VariableDeclaration) {
+        var parent2 = parent.parent;
+        if (parent2 is VariableDeclarationList) {
+          var assignmentType = parent2.type?.type;
+          if (assignmentType != null && typeCheck(assignmentType)) {
+            return true;
+          }
+        }
       }
-      if (body.isGenerator && returnType.isDartAsyncStream) {
-        var typeArgs = (returnType as InterfaceType).typeArguments;
-        return typeArgs.isEmpty ? null : typeArgs.first;
+      // Skip: function(LinkedHashSet()); when function(LinkedHashSet mySet) or
+      // function(LinkedHashMap()); when function(LinkedHashMap myMap)
+      if (parent is ArgumentList) {
+        var paramType = node.staticParameterElement?.type;
+        if (paramType == null || typeCheck(paramType)) {
+          return true;
+        }
       }
-    } else {
-      if (body.isGenerator && returnType.isDartCoreIterable) {
-        var typeArgs = (returnType as InterfaceType).typeArguments;
-        return typeArgs.isEmpty ? null : typeArgs.first;
+
+      // Skip: void f({required LinkedHashSet<Foo> s})
+      if (parent is NamedExpression) {
+        var paramType = parent.staticParameterElement?.type;
+        if (paramType != null && typeCheck(paramType)) {
+          return true;
+        }
       }
-    }
-    return returnType;
-  }
 
-  /// Attempts to calculate the expected return type of the function represented
-  /// by [body], accounting for an approximation of the function's context type,
-  /// in the case of a function literal.
-  DartType? _expectedReturnType(FunctionBody? body) {
-    if (body == null) return null;
-    var parent = body.parent;
-    if (parent is FunctionExpression) {
-      var grandparent = parent.parent;
-      if (grandparent is FunctionDeclaration) {
-        var returnType = grandparent.declaredElement?.returnType;
-        return _expectedReturnableOrYieldableType(returnType, body);
+      // Skip: <int, LinkedHashSet>{}.putIfAbsent(3, () => LinkedHashSet());
+      // or <int, LinkedHashMap>{}.putIfAbsent(3, () => LinkedHashMap());
+      if (parent is ExpressionFunctionBody) {
+        var expressionType = parent.expression.staticType;
+        if (expressionType != null && typeCheck(expressionType)) {
+          return true;
+        }
       }
-      var functionType = _approximateContextType(parent);
-      if (functionType is! FunctionType) return null;
-      var returnType = functionType.returnType;
-      return _expectedReturnableOrYieldableType(returnType, body);
     }
-    if (parent is MethodDeclaration) {
-      var returnType = parent.declaredElement?.returnType;
-      return _expectedReturnableOrYieldableType(returnType, body);
-    }
-    return null;
+    return false;
   }
 }
-
-extension on Expression {
-  bool get isHashMap => staticType.isTypeHashMap;
-
-  bool get isHashSet => staticType.isTypeHashSet;
-
-  bool get isMap => staticType?.isDartCoreMap ?? false;
-
-  bool get isSet => staticType?.isDartCoreSet ?? false;
-}
-
-extension on DartType? {
-  bool get isTypeHashMap => isSameAs('LinkedHashMap', 'dart.collection');
-
-  bool get isTypeHashSet => isSameAs('LinkedHashSet', 'dart.collection');
-}