lint: correct some simple unnecessary_parenthesis cases found internally
Fixes https://github.com/dart-lang/linter/issues/4354
Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try
Change-Id: Ie0138093467ff266d4f06c70f280660c4fc0783d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/376540
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
diff --git a/pkg/analyzer/lib/src/lint/pub.dart b/pkg/analyzer/lib/src/lint/pub.dart
index 626c5a8..254fcfc 100644
--- a/pkg/analyzer/lib/src/lint/pub.dart
+++ b/pkg/analyzer/lib/src/lint/pub.dart
@@ -143,7 +143,7 @@
PSEntry(this.key, this.value);
@override
- String toString() => '${key != null ? ('$key: ') : ''}$value';
+ String toString() => '${key != null ? '$key: ' : ''}$value';
}
/// Representation of environment in `pubspec.yaml`.
diff --git a/pkg/linter/lib/src/extensions.dart b/pkg/linter/lib/src/extensions.dart
index 03cb544..1f75352 100644
--- a/pkg/linter/lib/src/extensions.dart
+++ b/pkg/linter/lib/src/extensions.dart
@@ -100,29 +100,29 @@
return null;
}
+ /// Whether the expression is null-aware, or if one of its recursive targets
+ /// is null-aware.
+ bool get containsNullAwareInvocationInChain {
+ var node = this;
+ if (node is PropertyAccess) {
+ if (node.isNullAware) return true;
+ return node.target.containsNullAwareInvocationInChain;
+ } else if (node is MethodInvocation) {
+ if (node.isNullAware) return true;
+ return node.target.containsNullAwareInvocationInChain;
+ } else if (node is IndexExpression) {
+ if (node.isNullAware) return true;
+ return node.target.containsNullAwareInvocationInChain;
+ }
+ return false;
+ }
+
bool get isFieldNameShortcut {
var node = this;
if (node is NullCheckPattern) node = node.parent;
if (node is NullAssertPattern) node = node.parent;
return node is PatternField && node.name != null && node.name?.name == null;
}
-
- /// Return `true` if the expression is null aware, or if one of its recursive
- /// targets is null aware.
- bool containsNullAwareInvocationInChain() {
- var node = this;
- if (node is PropertyAccess) {
- if (node.isNullAware) return true;
- return node.target.containsNullAwareInvocationInChain();
- } else if (node is MethodInvocation) {
- if (node.isNullAware) return true;
- return node.target.containsNullAwareInvocationInChain();
- } else if (node is IndexExpression) {
- if (node.isNullAware) return true;
- return node.target.containsNullAwareInvocationInChain();
- }
- return false;
- }
}
extension BlockExtension on Block {
diff --git a/pkg/linter/lib/src/rules/avoid_function_literals_in_foreach_calls.dart b/pkg/linter/lib/src/rules/avoid_function_literals_in_foreach_calls.dart
index 1731b23..13a2fd7 100644
--- a/pkg/linter/lib/src/rules/avoid_function_literals_in_foreach_calls.dart
+++ b/pkg/linter/lib/src/rules/avoid_function_literals_in_foreach_calls.dart
@@ -67,7 +67,7 @@
node.argumentList.arguments.isNotEmpty &&
node.argumentList.arguments.first is FunctionExpression &&
_isIterable(target.staticType) &&
- !node.containsNullAwareInvocationInChain() &&
+ !node.containsNullAwareInvocationInChain &&
!_hasMethodChaining(node) &&
!_isInsideCascade(node)) {
rule.reportLint(node.function);
diff --git a/pkg/linter/lib/src/rules/unnecessary_lambdas.dart b/pkg/linter/lib/src/rules/unnecessary_lambdas.dart
index 9bfb7b7..33cdec7 100644
--- a/pkg/linter/lib/src/rules/unnecessary_lambdas.dart
+++ b/pkg/linter/lib/src/rules/unnecessary_lambdas.dart
@@ -204,7 +204,7 @@
}
var checker = _FinalExpressionChecker(parameters);
- if (!node.containsNullAwareInvocationInChain() &&
+ if (!node.containsNullAwareInvocationInChain &&
checker.isFinalNode(node.target) &&
node.methodName.staticElement.isFinal &&
node.typeArguments == null) {
diff --git a/pkg/linter/lib/src/rules/unnecessary_parenthesis.dart b/pkg/linter/lib/src/rules/unnecessary_parenthesis.dart
index 38fd7f8..ec6bbac 100644
--- a/pkg/linter/lib/src/rules/unnecessary_parenthesis.dart
+++ b/pkg/linter/lib/src/rules/unnecessary_parenthesis.dart
@@ -104,8 +104,8 @@
// `(List<int>).toString()` is OK.
if (expression is TypeLiteral) return;
- if (expression is SimpleIdentifier ||
- expression.containsNullAwareInvocationInChain()) {
+ if (expression.isOneToken ||
+ expression.containsNullAwareInvocationInChain) {
if (parent is PropertyAccess) {
var name = parent.propertyName.name;
if (name == 'hashCode' || name == 'runtimeType') {
@@ -233,9 +233,10 @@
// inside one of the following nodes, the readability is not affected.
if (parent is! AssignmentExpression &&
parent is! ConstructorFieldInitializer &&
- parent is! VariableDeclaration &&
parent is! ExpressionFunctionBody &&
+ parent is! RecordLiteral &&
parent is! ReturnStatement &&
+ parent is! VariableDeclaration &&
parent is! YieldStatement &&
!node.isArgument) {
return;
@@ -350,4 +351,15 @@
bool get isArgument =>
parent is ArgumentList ||
(parent is NamedExpression && parent?.parent is ArgumentList);
+
+ /// Whether this expression is a sigle token.
+ ///
+ /// This excludes type literals because they often need to be parenthesized.
+ bool get isOneToken =>
+ this is SimpleIdentifier ||
+ this is StringLiteral ||
+ this is IntegerLiteral ||
+ this is DoubleLiteral ||
+ this is NullLiteral ||
+ this is BooleanLiteral;
}
diff --git a/pkg/linter/test/rules/unnecessary_parenthesis_test.dart b/pkg/linter/test/rules/unnecessary_parenthesis_test.dart
index bf5eb07..cce1ae4 100644
--- a/pkg/linter/test/rules/unnecessary_parenthesis_test.dart
+++ b/pkg/linter/test/rules/unnecessary_parenthesis_test.dart
@@ -57,6 +57,17 @@
''');
}
+ test_binaryExpressionInside_constructorFieldInitializer() async {
+ await assertDiagnostics(r'''
+class C {
+ bool f;
+ C() : f = (true && false);
+}
+''', [
+ lint(32, 15),
+ ]);
+ }
+
test_binaryExpressionInside_namedArgument() async {
await assertDiagnostics(r'''
void f({required int p}) {
@@ -83,6 +94,16 @@
''');
}
+ test_binaryExpressionInside_recordLiteral() async {
+ await assertDiagnostics(r'''
+Record f() {
+ return (1, (2 + 2));
+}
+''', [
+ lint(26, 7),
+ ]);
+ }
+
test_binaryExpressionInside_returnExpression() async {
await assertDiagnostics(r'''
bool f() {
@@ -93,6 +114,26 @@
]);
}
+ /// https://github.com/dart-lang/linter/issues/4041
+ test_cascadeAssignmentInside_nullAware() async {
+ await assertNoDiagnostics(r'''
+class A {
+ var b = false;
+ void m() {}
+ set setter(int i) {}
+}
+
+void f(A? a) {
+ (a?..b = true)?.m();
+ (a?..b = true)?.setter = 1;
+}
+
+void g(List<int>? list) {
+ (list?..[0] = 1)?.length;
+}
+''');
+ }
+
test_conditionalExpressionInside_argument() async {
await assertDiagnostics(r'''
void f(int p) {
@@ -141,26 +182,6 @@
''');
}
- test_constructorFieldInitializer_binaryInside() async {
- await assertDiagnostics(r'''
-class C {
- bool f;
- C() : f = (true && false);
-}
-''', [
- lint(32, 15),
- ]);
- }
-
- test_constructorFieldInitializer_equalityInside() async {
- await assertNoDiagnostics(r'''
-class C {
- bool f;
- C() : f = (1 == 2);
-}
-''');
- }
-
test_constructorFieldInitializer_functionExpressionInAssignment() async {
await assertNoDiagnostics(r'''
class C {
@@ -171,29 +192,6 @@
''');
}
- /// https://github.com/dart-lang/linter/issues/1395
- test_constructorFieldInitializer_functionExpressionInCascade() async {
- await assertNoDiagnostics(r'''
-class C {
- Object f;
-
- C() : f = (C()..f = () => 42);
-}
-''');
- }
-
- /// https://github.com/dart-lang/linter/issues/1395
- test_constructorFieldInitializer_functionExpressionInCascade2() async {
- await assertNoDiagnostics(r'''
-class C {
- dynamic f;
-
- C()
- : f = (C()..f = (C()..f = () => 42));
-}
-''');
- }
-
test_constructorFieldInitializer_functionExpressionInNullAware() async {
await assertNoDiagnostics(r'''
class C {
@@ -215,6 +213,15 @@
''');
}
+ test_equalityInside_constructorFieldInitializer() async {
+ await assertNoDiagnostics(r'''
+class C {
+ bool f;
+ C() : f = (1 == 2);
+}
+''');
+ }
+
test_equalityInside_expressionBody() async {
await assertDiagnostics(r'''
bool f() => (1 == 1);
@@ -223,15 +230,7 @@
]);
}
- test_inListLiteral() async {
- await assertDiagnostics(r'''
-final items = [1, (DateTime.now())];
-''', [
- lint(18, 16),
- ]);
- }
-
- test_isExpressionInside_targetOfMethodInvocation() async {
+ test_expressionInside_targetOfMethodInvocation() async {
await assertNoDiagnostics(r'''
void f(Object o) {
(o is num).toString();
@@ -239,6 +238,37 @@
''');
}
+ /// https://github.com/dart-lang/linter/issues/1395
+ test_functionExpressionInCascade2Inside_constructorFieldInitializer() async {
+ await assertNoDiagnostics(r'''
+class C {
+ dynamic f;
+
+ C()
+ : f = (C()..f = (C()..f = () => 42));
+}
+''');
+ }
+
+ /// https://github.com/dart-lang/linter/issues/1395
+ test_functionExpressionInCascadeInside_constructorFieldInitializer() async {
+ await assertNoDiagnostics(r'''
+class C {
+ Object f;
+
+ C() : f = (C()..f = () => 42);
+}
+''');
+ }
+
+ test_listLiteral() async {
+ await assertDiagnostics(r'''
+final items = [1, (DateTime.now())];
+''', [
+ lint(18, 16),
+ ]);
+ }
+
test_notEqualInside_returnExpression() async {
await assertNoDiagnostics(r'''
bool f() {
@@ -247,24 +277,15 @@
''');
}
- /// https://github.com/dart-lang/linter/issues/4041
- test_nullAware_cascadeAssignment() async {
- await assertNoDiagnostics(r'''
-class A {
- var b = false;
- void m() {}
- set setter(int i) {}
+ @FailingTest(issue: 'https://github.com/dart-lang/linter/issues/4062')
+ test_parenthesizedPattern_nonPatternOutside() async {
+ await assertDiagnostics(r'''
+void f(num n) {
+ if (1 case (int())) {}
}
-
-void f(A? a) {
- (a?..b = true)?.m();
- (a?..b = true)?.setter = 1;
-}
-
-void g(List<int>? list) {
- (list?..[0] = 1)?.length;
-}
-''');
+''', [
+ lint(20, 7),
+ ]);
}
test_positionalArgument() async {
@@ -293,7 +314,17 @@
''');
}
- test_record_assignment() async {
+ test_propertyAccessInside_recordLiteral() async {
+ await assertDiagnostics(r'''
+Record f() {
+ return (1.isEven, (2.isEven));
+}
+''', [
+ lint(33, 10),
+ ]);
+ }
+
+ test_recordInside_assignment() async {
await assertDiagnostics(r'''
void f() {
(int,) r = ((3,));
@@ -303,7 +334,7 @@
]);
}
- test_record_namedParam() async {
+ test_recordInside_namedParam() async {
await assertDiagnostics(r'''
void f() {
g(i: ((3,)));
@@ -315,7 +346,7 @@
]);
}
- test_record_param() async {
+ test_recordInside_param() async {
await assertDiagnostics(r'''
void f() {
g(((3,)));
@@ -340,7 +371,7 @@
]);
}
- test_singleElementRecordWithNoTrailingComma_namedParam() async {
+ test_singleElementRecordWithNoTrailingCommaInside_namedArgument() async {
await assertDiagnostics(r'''
void f() {
g(i: (3));
@@ -357,7 +388,7 @@
}
/// https://github.com/dart-lang/linter/issues/4876
- test_singleElementRecordWithNoTrailingComma_param() async {
+ test_singleElementRecordWithNoTrailingCommaInside_positionalArgument() async {
await assertDiagnostics(r'''
void f() {
g((3));
@@ -389,15 +420,17 @@
''');
}
- test_switchExpression_expressionStatement() async {
- await assertNoDiagnostics(r'''
-void f(Object? x) {
- (switch (x) { _ => 0 });
+ test_stringLiteralInside() async {
+ await assertDiagnostics(r'''
+void f() {
+ '' + ('');
}
-''');
+''', [
+ lint(18, 4),
+ ]);
}
- test_switchExpression_invocationArgument() async {
+ test_switchExpressionInside_argument() async {
await assertDiagnostics(r'''
void f(Object? x) {
print((switch (x) { _ => 0 }));
@@ -407,7 +440,15 @@
]);
}
- test_switchExpression_variableDeclaration() async {
+ test_switchExpressionInside_expressionStatement() async {
+ await assertNoDiagnostics(r'''
+void f(Object? x) {
+ (switch (x) { _ => 0 });
+}
+''');
+ }
+
+ test_switchExpressionInside_variableDeclaration() async {
await assertDiagnostics(r'''
void f(Object? x) {
final v = (switch (x) { _ => 0 });
