[M] Change in dart/sdk[main]: [analyzer] introduce `UNNECESSARY_SET_LITERAL`
Ahmed Ashour (Gerrit)
Ahmed Ashour has uploaded this change for review.
[analyzer] introduce `UNNECESSARY_SET_LITERAL`
Fixes #50900
Change-Id: I7c7535e3fa7dcd25bf11034790a14b1bdfc50a77
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/diagnostics/test_all.dart
A pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
7 files changed, 125 insertions(+), 7 deletions(-)
diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
index e8e1748..3d22663 100644
--- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
+++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
@@ -1629,6 +1629,10 @@
status: hasFix
HintCode.UNNECESSARY_QUESTION_MARK:
status: hasFix
+HintCode.UNNECESSARY_SET_LITERAL:
+ status: needsFix
+ notes: |-
+ Remove the set literal.
HintCode.UNNECESSARY_TYPE_CHECK_FALSE:
status: hasFix
HintCode.UNNECESSARY_TYPE_CHECK_TRUE:
diff --git a/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart b/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
index cc68cbd..eb02b3f 100644
--- a/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
+++ b/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
@@ -1135,6 +1135,14 @@
);
/// No parameters.
+ static const HintCode UNNECESSARY_SET_LITERAL = HintCode(
+ 'UNNECESSARY_SET_LITERAL',
+ "Unneeded set literal for a single expression in a function expression "
+ "with a 'void' return type.",
+ correctionMessage: "Try removing the set literal around th expression.",
+ );
+
+ /// No parameters.
static const HintCode UNNECESSARY_TYPE_CHECK_FALSE = HintCode(
'UNNECESSARY_TYPE_CHECK',
"Unnecessary type check; the result is always 'false'.",
diff --git a/pkg/analyzer/lib/src/error/best_practices_verifier.dart b/pkg/analyzer/lib/src/error/best_practices_verifier.dart
index 7e468b3..28ee153 100644
--- a/pkg/analyzer/lib/src/error/best_practices_verifier.dart
+++ b/pkg/analyzer/lib/src/error/best_practices_verifier.dart
@@ -585,8 +585,10 @@
@override
void visitFunctionExpression(FunctionExpression node) {
+ var body = node.body;
if (node.parent is! FunctionDeclaration) {
- _checkForMissingReturn(node.body, node);
+ _checkForMissingReturn(body, node);
+ _checkForUnnecessarySetLiteral(body, node);
}
if (!(node as FunctionExpressionImpl).wasFunctionTypeSupplied) {
_checkStrictInferenceInParameters(node.parameters, body: node.body);
@@ -1553,6 +1555,22 @@
return false;
}
+ void _checkForUnnecessarySetLiteral(
+ FunctionBody body, FunctionExpression node) {
+ if (body is ExpressionFunctionBodyImpl) {
+ var parameterType = node.staticParameterElement?.type;
+ if (parameterType is FunctionType && parameterType.returnType.isVoid) {
+ var expression = body.expression;
+ if (expression is SetOrMapLiteralImpl &&
+ expression.isSet &&
+ expression.elements.length == 1) {
+ _errorReporter.reportErrorForNode(
+ HintCode.UNNECESSARY_SET_LITERAL, expression);
+ }
+ }
+ }
+ }
+
void _checkRequiredParameter(FormalParameterList node) {
final requiredParameters =
node.parameters.where((p) => p.declaredElement?.hasRequired == true);
diff --git a/pkg/analyzer/lib/src/error/error_code_values.g.dart b/pkg/analyzer/lib/src/error/error_code_values.g.dart
index 2a67b49..ad8b7da 100644
--- a/pkg/analyzer/lib/src/error/error_code_values.g.dart
+++ b/pkg/analyzer/lib/src/error/error_code_values.g.dart
@@ -669,6 +669,7 @@
HintCode.UNNECESSARY_NULL_COMPARISON_FALSE,
HintCode.UNNECESSARY_NULL_COMPARISON_TRUE,
HintCode.UNNECESSARY_QUESTION_MARK,
+ HintCode.UNNECESSARY_SET_LITERAL,
HintCode.UNNECESSARY_TYPE_CHECK_FALSE,
HintCode.UNNECESSARY_TYPE_CHECK_TRUE,
HintCode.UNREACHABLE_SWITCH_CASE,
diff --git a/pkg/analyzer/messages.yaml b/pkg/analyzer/messages.yaml
index 56df52a..6cdd659 100644
--- a/pkg/analyzer/messages.yaml
+++ b/pkg/analyzer/messages.yaml
@@ -1021,7 +1021,7 @@
```
ASYNC_FOR_IN_WRONG_CONTEXT:
problemMessage: The async for-in loop can only be used in an async function.
- correctionMessage: "Try marking the function body with either 'async' or 'async*', or removing the 'await' before the for-in loop."
+ correctionMessage: Try marking the function body with either 'async' or 'async*', or removing the 'await' before the for-in loop.
hasPublishedDocs: true
comment: No parameters.
documentation: |-
@@ -1122,13 +1122,13 @@
```
AWAIT_IN_WRONG_CONTEXT:
problemMessage: The await expression can only be used in an async function.
- correctionMessage: "Try marking the function body with either 'async' or 'async*'."
+ correctionMessage: Try marking the function body with either 'async' or 'async*'.
comment: |-
16.30 Await Expressions: It is a compile-time error if the function
immediately enclosing _a_ is not declared asynchronous. (Where _a_ is the
await expression.)
BODY_MIGHT_COMPLETE_NORMALLY:
- problemMessage: "The body might complete normally, causing 'null' to be returned, but the return type, '{0}', is a potentially non-nullable type."
+ problemMessage: The body might complete normally, causing 'null' to be returned, but the return type, '{0}', is a potentially non-nullable type.
correctionMessage: Try adding either a return or a throw statement at the end.
hasPublishedDocs: true
comment: |-
@@ -1207,7 +1207,7 @@
}
```
BREAK_LABEL_ON_SWITCH_MEMBER:
- problemMessage: "A break label resolves to the 'case' or 'default' statement."
+ problemMessage: A break label resolves to the 'case' or 'default' statement.
hasPublishedDocs: true
comment: No parameters.
documentation: |-
@@ -1264,7 +1264,7 @@
}
```
BUILT_IN_IDENTIFIER_AS_TYPE:
- problemMessage: "The built-in identifier '{0}' can't be used as a type."
+ problemMessage: The built-in identifier '{0}' can't be used as a type.
correctionMessage: Try correcting the name to match an existing type.
hasPublishedDocs: true
comment: |-
@@ -6927,7 +6927,7 @@
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)."
+ 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
@@ -20614,6 +20614,11 @@
```dart
dynamic x;
```
+ UNNECESSARY_SET_LITERAL:
+ problemMessage: Unneeded set literal for a single expression in a function expression with a 'void' return type.
+ correctionMessage: Try removing the set literal around th expression.
+ hasPublishedDocs: false
+ comment: No parameters.
UNNECESSARY_TYPE_CHECK_FALSE:
sharedName: UNNECESSARY_TYPE_CHECK
problemMessage: "Unnecessary type check; the result is always 'false'."
diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart
index 0263ccc..9de9d6a 100644
--- a/pkg/analyzer/test/src/diagnostics/test_all.dart
+++ b/pkg/analyzer/test/src/diagnostics/test_all.dart
@@ -810,6 +810,7 @@
as unnecessary_null_check_pattern;
import 'unnecessary_null_comparison_test.dart' as unnecessary_null_comparison;
import 'unnecessary_question_mark_test.dart' as unnecessary_question_mark;
+import 'unnecessary_set_literal_test.dart' as unnecessary_set_literal;
import 'unnecessary_type_check_test.dart' as unnecessary_type_check;
import 'unqualified_reference_to_non_local_static_member_test.dart'
as unqualified_reference_to_non_local_static_member;
@@ -1384,6 +1385,7 @@
unnecessary_null_check_pattern.main();
unnecessary_null_comparison.main();
unnecessary_question_mark.main();
+ unnecessary_set_literal.main();
unnecessary_type_check.main();
unqualified_reference_to_non_local_static_member.main();
unqualified_reference_to_static_member_of_extended_type.main();
diff --git a/pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart b/pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
new file mode 100644
index 0000000..b908d55
--- /dev/null
+++ b/pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
@@ -0,0 +1,69 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// 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/src/dart/error/hint_codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+ defineReflectiveSuite(() {
+ defineReflectiveTests(UnnecessarySetLiteralTest);
+ });
+}
+
+@reflectiveTest
+class UnnecessarySetLiteralTest extends PubPackageResolutionTest {
+ test_blockFunctionBody() async {
+ await assertNoErrorsInCode(r'''
+void g(void Function(int) fun) {}
+
+void f() {
+ g((value) {1;});
+}
+''');
+ }
+
+ test_closure_map() async {
+ await assertNoErrorsInCode(r'''
+void g(void Function(int) fun) {}
+
+void f() {
+ g((value) => {1: 2});
+}
+''');
+ }
+
+ test_closure_multipleElements() async {
+ await assertNoErrorsInCode(r'''
+void g(void Function(int) fun) {}
+
+void f() {
+ g((value) => {1, 2});
+}
+''');
+ }
+
+ test_closure_object() async {
+ await assertNoErrorsInCode(r'''
+void g(Object Function(int) fun) {}
+
+void f() {
+ g((value) => {1});
+}
+''');
+ }
+
+ test_closure_void() async {
+ await assertErrorsInCode(r'''
+void g(void Function(int) fun) {}
+
+void f() {
+ g((value) => {1});
+}
+''', [
+ error(HintCode.UNNECESSARY_SET_LITERAL, 61, 3),
+ ]);
+ }
+}
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Ahmed Ashour uploaded patch set #2 to this change.
[analyzer] introduce `UNNECESSARY_SET_LITERAL`
Fixes #50900
Change-Id: I7c7535e3fa7dcd25bf11034790a14b1bdfc50a77
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/diagnostics/test_all.dart
A pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
7 files changed, 125 insertions(+), 7 deletions(-)
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Attention is currently required from: Ahmed Ashour.
1 comment:
Patchset:
I would prefer to resolve the question of the breadth of the warning before we try to implement it.
Discussion in the issue.
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
Attention is currently required from: Ahmed Ashour, Samuel Rawlins.
go/dart-cbuild result: FAILURE (REGRESSIONS DETECTED)
Details: https://goto.google.com/dart-cbuild/find/79f690bf00b102ec3e79a278db0829e83f2a5c43
Bugs: go/dart-cbuild-bug/79f690bf00b102ec3e79a278db0829e83f2a5c43
Ahmed Ashour (Gerrit)
Attention is currently required from: Ahmed Ashour, Samuel Rawlins.
Ahmed Ashour uploaded patch set #3 to this change.
[analyzer] introduce `UNNECESSARY_SET_LITERAL`
Fixes #50900
Change-Id: I7c7535e3fa7dcd25bf11034790a14b1bdfc50a77
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/diagnostics/test_all.dart
A pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
7 files changed, 136 insertions(+), 7 deletions(-)
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Attention is currently required from: Samuel Rawlins.
1 comment:
Patchset:
Only expressions are handled, PTAL.
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
Attention is currently required from: Samuel Rawlins.
go/dart-cbuild result: FAILURE (REGRESSIONS DETECTED)
Details: https://goto.google.com/dart-cbuild/find/764bbe4273acadf94a4c0612c8d0bf1793b8d451
Bugs: go/dart-cbuild-bug/764bbe4273acadf94a4c0612c8d0bf1793b8d451
Samuel Rawlins (Gerrit)
Attention is currently required from: Ahmed Ashour.
4 comments:
File pkg/analyzer/lib/src/dart/error/hint_codes.g.dart:
Patch Set #3, Line 1142:
correctionMessage: "Try removing the set literal around th expression.",nit: "the"
File pkg/analyzer/lib/src/error/best_practices_verifier.dart:
Patch Set #3, Line 1558:
void _checkForUnnecessarySetLiteral(Would you mind adding a comment above this explaining what we're checking? Especially because the name doesn't convey that this is specifically checking for `=> { ... }`
File pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart:
Patch Set #3, Line 10:
main() {I think we also need to test async functions with `Future<void>`, maybe `FutureOr<void>`. This is legal:
```
Future<void> f() async => { print(1) };
```Generators, (sync*, async*) however cannot use `=>` so don't need to cover those.
Patch Set #3, Line 29:
await assertNoErrorsInCode(r'''Curious, why don't these tests just use one function?
```
void f() => {1: 2};
```
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Attention is currently required from: Ahmed Ashour, Samuel Rawlins.
Ahmed Ashour uploaded patch set #4 to this change.
[analyzer] introduce `UNNECESSARY_SET_LITERAL`
Fixes #50900
Change-Id: I7c7535e3fa7dcd25bf11034790a14b1bdfc50a77
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/diagnostics/test_all.dart
A pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
7 files changed, 207 insertions(+), 7 deletions(-)
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Attention is currently required from: Samuel Rawlins.
4 comments:
File pkg/analyzer/lib/src/dart/error/hint_codes.g.dart:
Patch Set #3, Line 1142:
correctionMessage: "Try removing the set literal around th expression.",nit: "the"
Done
File pkg/analyzer/lib/src/error/best_practices_verifier.dart:
Patch Set #3, Line 1558:
void _checkForUnnecessarySetLiteral(
Would you mind adding a comment above this explaining what we're checking? Especially because the na […]
Done
File pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart:
Patch Set #3, Line 10:
main() {
I think we also need to test async functions with `Future<void>`, maybe `FutureOr<void>`. […]
Thanks, that was missing. Done.
Patch Set #3, Line 29:
await assertNoErrorsInCode(r'''
Curious, why don't these tests just use one function? […]
If I correctly understand, having only `void f() => {1: 2};` has a parent of `FunctionDeclaration`, and this doesn't call `_checkForUnnecessarySetLiteral`, which is the implementation in question, i.e.:
```dart
if (node.parent is! FunctionDeclaration) {
_checkForMissingReturn(body, node);
_checkForUnnecessarySetLiteral(body, node);
}
```In other words, this test case is exactly as the one which triggers the hint, but returns a map instead of the set.
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Attention is currently required from: Samuel Rawlins.
Ahmed Ashour uploaded patch set #5 to this change.
[analyzer] introduce `UNNECESSARY_SET_LITERAL`
Fixes #50900
Change-Id: I7c7535e3fa7dcd25bf11034790a14b1bdfc50a77
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/diagnostics/test_all.dart
A pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
7 files changed, 201 insertions(+), 7 deletions(-)
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Attention is currently required from: Samuel Rawlins.
Ahmed Ashour uploaded patch set #6 to this change.
[analyzer] introduce `UNNECESSARY_SET_LITERAL`
Fixes #50900
Change-Id: I7c7535e3fa7dcd25bf11034790a14b1bdfc50a77
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/diagnostics/test_all.dart
A pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
7 files changed, 233 insertions(+), 7 deletions(-)
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Samuel Rawlins (Gerrit)
Attention is currently required from: Ahmed Ashour.
2 comments:
File pkg/analyzer/messages.yaml:
Patch Set #6, Line 20637:
problemMessage: Unneeded set literal for a single expression in a function expression with a 'void' return type.I think we can improve the message here. I think this message now has a focus on correctness but it doesn't really help someone who doesn't understand what they've written with `=> { ... }`. This one is really hard to write concisely...
Maybe something like, "Braces unnecessarily wrap this expression in a set literal." I think we don't need to go into the specifics of void-returning. I think you quickly get into the weeds ("Well usually the type system protects you from using these set literal braces, but in the case of void, we're pretty loosey goosey, so that setters (which must return void) are still allowed to use arrow functions ...").
File pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart:
Patch Set #3, Line 29:
await assertNoErrorsInCode(r'''If I correctly understand, having only `void f() => {1: 2};` has a parent of `FunctionDeclaration`, […]
Oh, in that case I think there is a bug in the implementation. For example, the new Hint should be reported for:
```
void f() => {1};
```
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Attention is currently required from: Ahmed Ashour, Samuel Rawlins.
Ahmed Ashour uploaded patch set #7 to this change.
[analyzer] introduce `UNNECESSARY_SET_LITERAL`
Fixes #50900
Change-Id: I7c7535e3fa7dcd25bf11034790a14b1bdfc50a77
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/diagnostics/test_all.dart
A pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
7 files changed, 323 insertions(+), 7 deletions(-)
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Attention is currently required from: Samuel Rawlins.
2 comments:
File pkg/analyzer/messages.yaml:
Patch Set #6, Line 20637:
problemMessage: Unneeded set literal for a single expression in a function expression with a 'void' return type.
I think we can improve the message here. […]
Yes, that would be much better.
File pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart:
Patch Set #3, Line 29:
await assertNoErrorsInCode(r'''
Oh, in that case I think there is a bug in the implementation. […]
I see, initially it was for only closures. PTAL.
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
Attention is currently required from: Samuel Rawlins.
go/dart-cbuild result: FAILURE (REGRESSIONS DETECTED)
Details: https://goto.google.com/dart-cbuild/find/e31b3a27769a4cbfb183ce992043ba5cfbc169b3
Bugs: go/dart-cbuild-bug/e31b3a27769a4cbfb183ce992043ba5cfbc169b3
Samuel Rawlins (Gerrit)
Patchset:
Note, this will take a bit to land, as I have to prepare internal code. Also need to check flutter, etc.
File pkg/analyzer/lib/src/error/best_practices_verifier.dart:
Patch Set #7, Line 1564:
FunctionBody body, FunctionExpression node) {This function is implemented so carefully, I love it. Thanks!
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Samuel Rawlins (Gerrit)
Attention is currently required from: Ahmed Ashour.
Patch set 7:Code-Review +1
Samuel Rawlins (Gerrit)
Attention is currently required from: Ahmed Ashour.
Patch set 7:Commit-Queue +2
Samuel Rawlins (Gerrit)
Attention is currently required from: Ahmed Ashour.
1 comment:
Patchset:
Looks like one new small test issue
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Attention is currently required from: Ahmed Ashour, Samuel Rawlins.
Ahmed Ashour uploaded patch set #8 to this change.
[analyzer] introduce `UNNECESSARY_SET_LITERAL`
Fixes #50900
Change-Id: I7c7535e3fa7dcd25bf11034790a14b1bdfc50a77
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analysis_server/test/src/services/correction/fix/convert_for_each_to_for_loop_test.dart
M pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/diagnostics/test_all.dart
A pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
8 files changed, 327 insertions(+), 9 deletions(-)
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
Ahmed Ashour (Gerrit)
Attention is currently required from: Samuel Rawlins.
1 comment:
Patchset:
Looks like one new small test issue
Done, PTAL.
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
Attention is currently required from: Samuel Rawlins.
go/dart-cbuild result: FAILURE (NO REGRESSIONS DETECTED)
Details: https://goto.google.com/dart-cbuild/find/6b2b6df0ff641163ac7406444838017b61fdead4
Samuel Rawlins (Gerrit)
Attention is currently required from: Ahmed Ashour, Samuel Rawlins.
Patch set 8:Commit-Queue +2
Commit Queue (Gerrit)
Commit Queue submitted this change.
7 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: pkg/analysis_server/test/src/services/correction/fix/convert_for_each_to_for_loop_test.dart
Insertions: 44, Deletions: 0.
The diff is too large to show. Please review the diff.
```
[analyzer] introduce `UNNECESSARY_SET_LITERAL`
Fixes #50900
Change-Id: I7c7535e3fa7dcd25bf11034790a14b1bdfc50a77
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279962
Commit-Queue: Samuel Rawlins <sraw...@google.com>
Reviewed-by: Samuel Rawlins <sraw...@google.com>
---
M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
M pkg/analysis_server/test/src/services/correction/fix/convert_for_each_to_for_loop_test.dart
M pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
M pkg/analyzer/lib/src/error/best_practices_verifier.dart
M pkg/analyzer/lib/src/error/error_code_values.g.dart
M pkg/analyzer/messages.yaml
M pkg/analyzer/test/src/diagnostics/test_all.dart
A pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
8 files changed, 330 insertions(+), 9 deletions(-)
diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
index 51f682f..f7449f6 100644
--- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
+++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
@@ -1547,6 +1547,10 @@
status: hasFix
HintCode.UNNECESSARY_QUESTION_MARK:
status: hasFix
+HintCode.UNNECESSARY_SET_LITERAL:
+ status: needsFix
+ notes: |-
+ Remove the set literal.
HintCode.UNNECESSARY_TYPE_CHECK_FALSE:
status: hasFix
HintCode.UNNECESSARY_TYPE_CHECK_TRUE:
diff --git a/pkg/analysis_server/test/src/services/correction/fix/convert_for_each_to_for_loop_test.dart b/pkg/analysis_server/test/src/services/correction/fix/convert_for_each_to_for_loop_test.dart
index d6bf55a..6fd48ec 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/convert_for_each_to_for_loop_test.dart
+++ b/pkg/analysis_server/test/src/services/correction/fix/convert_for_each_to_for_loop_test.dart
@@ -4,6 +4,7 @@
import 'package:analysis_server/src/services/correction/fix.dart';
import 'package:analysis_server/src/services/linter/lint_names.dart';
+import 'package:analyzer/error/error.dart';
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -284,7 +285,8 @@
list.forEach((x) => {print('')});
}
''');
- await assertNoFix();
+ await assertNoFix(
+ errorFilter: (error) => error.errorCode.type == ErrorType.LINT);
}
Future<void> test_setLiteral_multiple() async {
@@ -317,6 +319,6 @@
<int>{x};
}
}
-''');
+''', errorFilter: (error) => error.errorCode.type == ErrorType.LINT);
}
}
diff --git a/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart b/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
index ee1cb96..ac2e89f 100644
--- a/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
+++ b/pkg/analyzer/lib/src/dart/error/hint_codes.g.dart
@@ -807,6 +807,13 @@
);
/// No parameters.
+ static const HintCode UNNECESSARY_SET_LITERAL = HintCode(
+ 'UNNECESSARY_SET_LITERAL',
+ "Braces unnecessarily wrap this expression in a set literal.",
+ correctionMessage: "Try removing the set literal around the expression.",
+ );
+
+ /// No parameters.
static const HintCode UNNECESSARY_TYPE_CHECK_FALSE = HintCode(
'UNNECESSARY_TYPE_CHECK',
"Unnecessary type check; the result is always 'false'.",
diff --git a/pkg/analyzer/lib/src/error/best_practices_verifier.dart b/pkg/analyzer/lib/src/error/best_practices_verifier.dart
index 627e45f..2a43cd5 100644
--- a/pkg/analyzer/lib/src/error/best_practices_verifier.dart
+++ b/pkg/analyzer/lib/src/error/best_practices_verifier.dart
@@ -585,12 +585,14 @@
@override
void visitFunctionExpression(FunctionExpression node) {
+ var body = node.body;
if (node.parent is! FunctionDeclaration) {
- _checkForMissingReturn(node.body, node);
+ _checkForMissingReturn(body, node);
}
if (!(node as FunctionExpressionImpl).wasFunctionTypeSupplied) {
_checkStrictInferenceInParameters(node.parameters, body: node.body);
}
+ _checkForUnnecessarySetLiteral(body, node);
super.visitFunctionExpression(node);
}
@@ -1555,6 +1557,47 @@
return false;
}
+ /// Generate hints related to returning a set literal in an
+ /// [ExpressionFunctionBody], having a single expression,
+ /// for a function of `void` return type.
+ void _checkForUnnecessarySetLiteral(
+ FunctionBody body, FunctionExpression node) {
+ if (body is ExpressionFunctionBodyImpl) {
+ var parameterType = node.staticParameterElement?.type;
+
+ DartType? returnType;
+ if (parameterType is FunctionType) {
+ returnType = parameterType.returnType;
+ } else {
+ var parent = node.parent;
+ if (parent is! FunctionDeclaration) return;
+ returnType = parent.returnType?.type;
+ }
+ if (returnType == null) return;
+
+ bool isReturnVoid;
+ if (returnType.isVoid) {
+ isReturnVoid = true;
+ } else if (returnType is ParameterizedType &&
+ (returnType.isDartAsyncFuture || returnType.isDartAsyncFutureOr)) {
+ var typeArguments = returnType.typeArguments;
+ isReturnVoid = typeArguments.length == 1 && typeArguments.first.isVoid;
+ } else {
+ isReturnVoid = false;
+ }
+ if (isReturnVoid) {
+ var expression = body.expression;
+ if (expression is SetOrMapLiteralImpl && expression.isSet) {
+ var elements = expression.elements;
+ if (elements.length == 1 && elements.first is Expression) {
+ _errorReporter.reportErrorForNode(
+ HintCode.UNNECESSARY_SET_LITERAL, expression);
+ }
+ }
+ }
+ }
+ }
+
void _checkRequiredParameter(FormalParameterList node) {
final requiredParameters =
node.parameters.where((p) => p.declaredElement?.hasRequired == true);
diff --git a/pkg/analyzer/lib/src/error/error_code_values.g.dart b/pkg/analyzer/lib/src/error/error_code_values.g.dart
index 36a172b..e60dde1 100644
--- a/pkg/analyzer/lib/src/error/error_code_values.g.dart
+++ b/pkg/analyzer/lib/src/error/error_code_values.g.dart
@@ -648,6 +648,7 @@
HintCode.UNNECESSARY_NULL_COMPARISON_FALSE,
HintCode.UNNECESSARY_NULL_COMPARISON_TRUE,
HintCode.UNNECESSARY_QUESTION_MARK,
+ HintCode.UNNECESSARY_SET_LITERAL,
HintCode.UNNECESSARY_TYPE_CHECK_FALSE,
HintCode.UNNECESSARY_TYPE_CHECK_TRUE,
HintCode.UNREACHABLE_SWITCH_CASE,
diff --git a/pkg/analyzer/messages.yaml b/pkg/analyzer/messages.yaml
index ee556a3..5092ee1 100644
--- a/pkg/analyzer/messages.yaml
+++ b/pkg/analyzer/messages.yaml
@@ -1021,7 +1021,7 @@
```
ASYNC_FOR_IN_WRONG_CONTEXT:
problemMessage: The async for-in loop can only be used in an async function.
- correctionMessage: "Try marking the function body with either 'async' or 'async*', or removing the 'await' before the for-in loop."
+ correctionMessage: Try marking the function body with either 'async' or 'async*', or removing the 'await' before the for-in loop.
hasPublishedDocs: true
comment: No parameters.
documentation: |-
@@ -1122,7 +1122,7 @@
```
AWAIT_IN_WRONG_CONTEXT:
problemMessage: The await expression can only be used in an async function.
- correctionMessage: "Try marking the function body with either 'async' or 'async*'."
+ correctionMessage: Try marking the function body with either 'async' or 'async*'.
comment: |-
16.30 Await Expressions: It is a compile-time error if the function
immediately enclosing _a_ is not declared asynchronous. (Where _a_ is the
@@ -1140,7 +1140,7 @@
Parameters:
0: the name of the base mixin being implemented
BODY_MIGHT_COMPLETE_NORMALLY:
- problemMessage: "The body might complete normally, causing 'null' to be returned, but the return type, '{0}', is a potentially non-nullable type."
+ problemMessage: The body might complete normally, causing 'null' to be returned, but the return type, '{0}', is a potentially non-nullable type.
correctionMessage: Try adding either a return or a throw statement at the end.
hasPublishedDocs: true
comment: |-
@@ -1219,7 +1219,7 @@
}
```
BREAK_LABEL_ON_SWITCH_MEMBER:
- problemMessage: "A break label resolves to the 'case' or 'default' statement."
+ problemMessage: A break label resolves to the 'case' or 'default' statement.
hasPublishedDocs: true
comment: No parameters.
documentation: |-
@@ -1276,7 +1276,7 @@
}
```
BUILT_IN_IDENTIFIER_AS_TYPE:
- problemMessage: "The built-in identifier '{0}' can't be used as a type."
+ problemMessage: The built-in identifier '{0}' can't be used as a type.
correctionMessage: Try correcting the name to match an existing type.
hasPublishedDocs: true
comment: |-
@@ -6975,7 +6975,7 @@
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)."
+ 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
@@ -20058,6 +20058,11 @@
```dart
dynamic x;
```
+ UNNECESSARY_SET_LITERAL:
+ problemMessage: Braces unnecessarily wrap this expression in a set literal.
+ correctionMessage: Try removing the set literal around the expression.
+ hasPublishedDocs: false
+ comment: No parameters.
UNNECESSARY_TYPE_CHECK_FALSE:
sharedName: UNNECESSARY_TYPE_CHECK
problemMessage: "Unnecessary type check; the result is always 'false'."
diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart
index 460123b..907bab5 100644
--- a/pkg/analyzer/test/src/diagnostics/test_all.dart
+++ b/pkg/analyzer/test/src/diagnostics/test_all.dart
@@ -827,6 +827,7 @@
as unnecessary_null_check_pattern;
import 'unnecessary_null_comparison_test.dart' as unnecessary_null_comparison;
import 'unnecessary_question_mark_test.dart' as unnecessary_question_mark;
+import 'unnecessary_set_literal_test.dart' as unnecessary_set_literal;
import 'unnecessary_type_check_test.dart' as unnecessary_type_check;
import 'unqualified_reference_to_non_local_static_member_test.dart'
as unqualified_reference_to_non_local_static_member;
@@ -1410,6 +1411,7 @@
unnecessary_null_check_pattern.main();
unnecessary_null_comparison.main();
unnecessary_question_mark.main();
+ unnecessary_set_literal.main();
unnecessary_type_check.main();
unqualified_reference_to_non_local_static_member.main();
unqualified_reference_to_static_member_of_extended_type.main();
diff --git a/pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart b/pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
new file mode 100644
index 0000000..6bc86a9
--- /dev/null
+++ b/pkg/analyzer/test/src/diagnostics/unnecessary_set_literal_test.dart
@@ -0,0 +1,243 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// 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/src/dart/error/hint_codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+ defineReflectiveSuite(() {
+ defineReflectiveTests(UnnecessarySetLiteralTest);
+ });
+}
+
+@reflectiveTest
+class UnnecessarySetLiteralTest extends PubPackageResolutionTest {
+ test_blockFunctionBody() async {
+ await assertNoErrorsInCode(r'''
+void g(void Function() fun) {}
+
+void f() {
+ g(() {1;});
+}
+''');
+ }
+
+ test_expressionFunctionBody_dynamic() async {
+ await assertNoErrorsInCode(r'''
+void g(Function() fun) {}
+
+void f() {
+ g(() => {1});
+}
+''');
+ }
+
+ test_expressionFunctionBody_future() async {
+ await assertNoErrorsInCode(r'''
+void g(Future Function() fun) {}
+
+void f() {
+ g(() async => {1});
+}
+''');
+ }
+
+ test_expressionFunctionBody_future_object() async {
+ await assertNoErrorsInCode(r'''
+void g(Future<Object> Function() fun) {}
+
+void f() {
+ g(() async => {1});
+}
+''');
+ }
+
+ test_expressionFunctionBody_future_void() async {
+ await assertErrorsInCode(r'''
+void g(Future<void> Function() fun) {}
+
+void f() {
+ g(() async => {1});
+}
+''', [
+ error(HintCode.UNNECESSARY_SET_LITERAL, 67, 3),
+ ]);
+ }
+
+ test_expressionFunctionBody_futureOr() async {
+ await assertNoErrorsInCode(r'''
+import 'dart:async';
+
+void g(FutureOr Function() fun) {}
+
+void f() {
+ g(() async => {1});
+}
+''');
+ }
+
+ test_expressionFunctionBody_futureOr_object() async {
+ await assertNoErrorsInCode(r'''
+import 'dart:async';
+
+void g(FutureOr<Object> Function() fun) {}
+
+void f() {
+ g(() async => {1});
+}
+''');
+ }
+
+ test_expressionFunctionBody_futureOr_void() async {
+ await assertErrorsInCode(r'''
+import 'dart:async';
+
+void g(FutureOr<void> Function() fun) {}
+
+void f() {
+ g(() async => {1});
+}
+''', [
+ error(HintCode.UNNECESSARY_SET_LITERAL, 91, 3),
+ ]);
+ }
+
+ test_expressionFunctionBody_map() async {
+ await assertNoErrorsInCode(r'''
+void g(void Function() fun) {}
+
+void f() {
+ g(() => {1: 2});
+}
+''');
+ }
+
+ test_expressionFunctionBody_multipleElements() async {
+ await assertNoErrorsInCode(r'''
+void g(void Function() fun) {}
+
+void f() {
+ g(() => {1, 2});
+}
+''');
+ }
+
+ test_expressionFunctionBody_object() async {
+ await assertNoErrorsInCode(r'''
+void g(Object Function() fun) {}
+
+void f() {
+ g(() => {1});
+}
+''');
+ }
+
+ test_expressionFunctionBody_statement() async {
+ await assertNoErrorsInCode(r'''
+void g(void Function(bool) fun) {}
+
+void f() {
+ g((value) => {if (value) print('')});
+}
+''');
+ }
+
+ test_expressionFunctionBody_void() async {
+ await assertErrorsInCode(r'''
+void g(void Function() fun) {}
+
+void f() {
+ g(() => {1});
+}
+''', [
+ error(HintCode.UNNECESSARY_SET_LITERAL, 53, 3),
+ ]);
+ }
+
+ test_functionDeclaration_dynamic() async {
+ await assertNoErrorsInCode(r'''
+f() => {1};
+''');
+ }
+
+ test_functionDeclaration_future() async {
+ await assertNoErrorsInCode(r'''
+Future f() async => {1};
+''');
+ }
+
+ test_functionDeclaration_future_object() async {
+ await assertNoErrorsInCode(r'''
+Future<Object> f() async => {1};
+''');
+ }
+
+ test_functionDeclaration_future_void() async {
+ await assertErrorsInCode(r'''
+Future<void> f() async => {1};
+''', [
+ error(HintCode.UNNECESSARY_SET_LITERAL, 26, 3),
+ ]);
+ }
+
+ test_functionDeclaration_futureOr() async {
+ await assertNoErrorsInCode(r'''
+import 'dart:async';
+
+FutureOr f() async => {1};
+''');
+ }
+
+ test_functionDeclaration_futureOr_object() async {
+ await assertNoErrorsInCode(r'''
+import 'dart:async';
+
+FutureOr<Object> f() async => {1};
+''');
+ }
+
+ test_functionDeclaration_futureOr_void() async {
+ await assertErrorsInCode(r'''
+import 'dart:async';
+
+FutureOr<void> f() async => {1};
+''', [
+ error(HintCode.UNNECESSARY_SET_LITERAL, 50, 3),
+ ]);
+ }
+
+ test_functionDeclaration_map() async {
+ await assertNoErrorsInCode(r'''
+void f() => {1: 2};
+''');
+ }
+
+ test_functionDeclaration_multipleElements() async {
+ await assertNoErrorsInCode(r'''
+void f() => {1, 2};
+''');
+ }
+
+ test_functionDeclaration_object() async {
+ await assertNoErrorsInCode(r'''
+Object f() => {1};
+''');
+ }
+
+ test_functionDeclaration_statement() async {
+ await assertNoErrorsInCode(r'''
+void f(bool value) => {if (value) print('')};
+''');
+ }
+
+ test_functionDeclaration_void() async {
+ await assertErrorsInCode(r'''
+void f() => {1};
+''', [
+ error(HintCode.UNNECESSARY_SET_LITERAL, 12, 3),
+ ]);
+ }
+}
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/b5a8206c8b03f1b9cbc0372ea703bdd11df3ca42
Oleh Prypin (Gerrit)
1 comment:
File pkg/analyzer/lib/src/error/best_practices_verifier.dart:
Patch Set #9, Line 1592:
if (elements.length == 1 && elements.first is Expression) {Maybe we could expand this check to any length, not just length of 1.
Let's discuss that on the issue.
https://github.com/dart-lang/sdk/issues/50900
To view, visit change 279962. To unsubscribe, or for help writing mail filters, visit settings.