From 691ec932b487af7d89c51f9843f24b131b571884 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 6 Nov 2020 16:39:20 -0800 Subject: [PATCH 1/4] Fix false positives in prefer_collection_literals Towards #1649 Allow using `LinkedHashSet` and `LinkedHashMap` for named arguments, assignment to a variable, and when used in a binary expression where a static type is pushed down to the arguments. Refactor `_shouldSkipLinkedHashLint`. Extract a method to determine the type that is enforced statically for the expression rather than duplicate the subsequent check for each type of parent node. --- CHANGELOG.md | 4 ++ lib/src/rules/prefer_collection_literals.dart | 57 +++++++++---------- pubspec.yaml | 2 +- test/rules/prefer_collection_literals.dart | 23 ++++++-- 4 files changed, 50 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45973b4db..d5150eb97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 0.1.125-dev + +* fixed false positives in `prefer_colleciton_literals`. + # 0.1.124 * fixed false positives in `prefer_constructors_over_static_methods` diff --git a/lib/src/rules/prefer_collection_literals.dart b/lib/src/rules/prefer_collection_literals.dart index a94beb59f..a1c12bd3c 100644 --- a/lib/src/rules/prefer_collection_literals.dart +++ b/lib/src/rules/prefer_collection_literals.dart @@ -142,35 +142,34 @@ class _Visitor extends SimpleAstVisitor { bool _shouldSkipLinkedHashLint( InstanceCreationExpression node, bool Function(DartType node) typeCheck) { - if (_isHashMap(node) || _isHashSet(node)) { - // Skip: LinkedHashSet s = ...; or LinkedHashMap 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; - } - } - } - // Skip: function(LinkedHashSet()); when function(LinkedHashSet mySet) or - // function(LinkedHashMap()); when function(LinkedHashMap myMap) - if (parent is ArgumentList) { - final paramType = parent.arguments.first.staticParameterElement?.type; - if (paramType != null && !typeCheck(paramType)) { - return true; - } - } - // Skip: {}.putIfAbsent(3, () => LinkedHashSet()); - // or {}.putIfAbsent(3, () => LinkedHashMap()); - if (parent is ExpressionFunctionBody) { - var expressionType = parent.expression.staticType; - if (expressionType != null && !typeCheck(expressionType)) { - return true; - } - } + if (!_isHashMap(node) && !_isHashSet(node)) return false; + var parent = node.parent; + var staticType = _enforcedType(parent); + return staticType != null && !typeCheck(staticType); + } +} + +/// Returns the static type which is pushed into an expression by it's parent. +DartType _enforcedType(AstNode parent) { + if (parent is VariableDeclaration) { + var parent2 = parent.parent; + if (parent2 is VariableDeclarationList) { + return parent2.type?.type; } - return false; + } + if (parent is ArgumentList) { + return parent.arguments.first.staticParameterElement?.type; + } + if (parent is NamedExpression) { + return parent.staticType; + } + if (parent is ExpressionFunctionBody) { + return parent.expression.staticType; + } + if (parent is AssignmentExpression) { + return parent.leftHandSide.staticType; + } + if (parent is BinaryExpression) { + return parent.staticType; } } diff --git a/pubspec.yaml b/pubspec.yaml index 47127b617..c3d20674e 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: linter -version: 0.1.124 +version: 0.1.125-dev description: >- The implementation of the lint rules supported by the analyzer framework. diff --git a/test/rules/prefer_collection_literals.dart b/test/rules/prefer_collection_literals.dart index 22948a096..e1cc874ad 100644 --- a/test/rules/prefer_collection_literals.dart +++ b/test/rules/prefer_collection_literals.dart @@ -10,7 +10,7 @@ import 'dart:collection'; void main() { var listToLint = new List(); //LINT var mapToLint = new Map(); // LINT - var LinkedHashMapToLint = new LinkedHashMap(); // LINT + var linkedHashMapToLint = new LinkedHashMap(); // LINT var m1 = Map.unmodifiable({}); //OK var m2 = Map.fromIterable([]); //OK @@ -50,31 +50,42 @@ void main() { Set ss5 = LinkedHashSet(); // LINT LinkedHashSet ss6 = LinkedHashSet(); // OK + ss6 = LinkedHashSet(); // OK + ss6 = null ?? LinkedHashSet(); // OK printSet(Set()); // LINT printSet(LinkedHashSet()); // LINT printHashSet(LinkedHashSet()); // OK + printHashSetNamedArgument(ids: LinkedHashSet()); // OK Set ss7 = LinkedHashSet.from([1, 2, 3]); // LINT - LinkedHashSet ss8 = LinkedHashSet.from([1, 2, 3]); // OK + LinkedHashSet ss8 = LinkedHashSet.from([1, 2, 3]); // OK Iterable iter = Iterable.empty(); // OK var sss = Set.from(iter); // OK - LinkedHashSet sss1 = >{}.putIfAbsent(3, () => LinkedHashSet()); // OK + LinkedHashSet sss1 = >{} + .putIfAbsent(3, () => LinkedHashSet()); // OK - var lhs = LinkedHashSet(equals: (a, b) => false, hashCode: (o) => 13)..addAll({}); // OK + var lhs = LinkedHashSet(equals: (a, b) => false, hashCode: (o) => 13) + ..addAll({}); // OK LinkedHashMap hashMap = LinkedHashMap(); // OK + hashMap = LinkedHashMap(); // OK + hashMap = null ?? LinkedHashMap(); // OK printMap(Map()); // LINT printMap(LinkedHashMap()); // LINT printHashMap(LinkedHashMap()); // OK + printHashMapNamedArgument(map: LinkedHashMap()); // OK - LinkedHashMap lhm = >{}.putIfAbsent(3, () => LinkedHashMap()); // OK + LinkedHashMap lhm = >{} + .putIfAbsent(3, () => LinkedHashMap()); // OK } void printSet(Set ids) => print('$ids!'); void printHashSet(LinkedHashSet ids) => printSet(ids); +void printHashSetNamedArgument({LinkedHashSet ids}) => printSet(ids); void printMap(Map map) => print('$map!'); -void printHashMap(LinkedHashMap map) => printMap(map); \ No newline at end of file +void printHashMap(LinkedHashMap map) => printMap(map); +void printHashMapNamedArgument({LinkedHashMap map}) => printMap(map); From 49cb6f1405aeda9a047799d5b349d0b0abee8a5c Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 6 Nov 2020 16:56:36 -0800 Subject: [PATCH 2/4] Update version.dart --- lib/src/version.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/version.dart b/lib/src/version.dart index c15adf2d2..11a5bae0a 100644 --- a/lib/src/version.dart +++ b/lib/src/version.dart @@ -3,4 +3,4 @@ // BSD-style license that can be found in the LICENSE file. /// Package version. Synchronized w/ pubspec.yaml. -const String version = '0.1.124'; +const String version = '0.1.125-dev'; From 9222e139bcf27e4fbeaee0b15fa96806ff39b1f6 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 6 Nov 2020 16:58:01 -0800 Subject: [PATCH 3/4] Missing return --- lib/src/rules/prefer_collection_literals.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/src/rules/prefer_collection_literals.dart b/lib/src/rules/prefer_collection_literals.dart index a1c12bd3c..d9d02981c 100644 --- a/lib/src/rules/prefer_collection_literals.dart +++ b/lib/src/rules/prefer_collection_literals.dart @@ -42,7 +42,7 @@ will trigger a type error so those will be excluded from the lint. void main() { LinkedHashSet linkedHashSet = LinkedHashSet.from([1, 2, 3]); // OK LinkedHashMap linkedHashMap = LinkedHashMap(); // OK - + printSet(LinkedHashSet()); // LINT printHashSet(LinkedHashSet()); // OK @@ -150,7 +150,7 @@ class _Visitor extends SimpleAstVisitor { } /// Returns the static type which is pushed into an expression by it's parent. -DartType _enforcedType(AstNode parent) { +DartType/*?*/ _enforcedType(AstNode parent) { if (parent is VariableDeclaration) { var parent2 = parent.parent; if (parent2 is VariableDeclarationList) { @@ -172,4 +172,5 @@ DartType _enforcedType(AstNode parent) { if (parent is BinaryExpression) { return parent.staticType; } + return null; } From b9a720246a6d268082588ac59849287a0a7f673f Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 6 Nov 2020 17:09:46 -0800 Subject: [PATCH 4/4] Dartfmt --- lib/src/rules/prefer_collection_literals.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/rules/prefer_collection_literals.dart b/lib/src/rules/prefer_collection_literals.dart index d9d02981c..ce7bcf1e9 100644 --- a/lib/src/rules/prefer_collection_literals.dart +++ b/lib/src/rules/prefer_collection_literals.dart @@ -150,7 +150,7 @@ class _Visitor extends SimpleAstVisitor { } /// Returns the static type which is pushed into an expression by it's parent. -DartType/*?*/ _enforcedType(AstNode parent) { +DartType /*?*/ _enforcedType(AstNode parent) { if (parent is VariableDeclaration) { var parent2 = parent.parent; if (parent2 is VariableDeclarationList) {