Skip to content

Commit f07e042

Browse files
srawlinsCommit Queue
srawlins
authored and
Commit Queue
committed
linter: Fix avoid_types_on_closures for inference cases
Fixes https://github.com/dart-lang/linter/issues/1099 Fixes https://github.com/dart-lang/linter/issues/3330 We just add a check that the (approximate) context type is a function type. Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Change-Id: I56fe14ff8852375754fdaf6b92b3c632b7df9c95 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367982 Reviewed-by: Phil Quitslund <pquitslund@google.com> Commit-Queue: Samuel Rawlins <srawlins@google.com>
1 parent aa28753 commit f07e042

File tree

4 files changed

+165
-82
lines changed

4 files changed

+165
-82
lines changed

‎pkg/analysis_server/test/src/services/correction/fix/remove_type_annotation_test.dart

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,16 @@ class AvoidTypesOnClosureParametersBulkTest extends BulkFixProcessorTest {
150150

151151
Future<void> test_singleFile() async {
152152
await resolveTestCode('''
153-
var x = ({Future<int> defaultValue}) => null;
154-
var y = (Future<int> defaultValue) => null;
153+
void f(List<Future<int>> list) {
154+
list.forEach((Future<int> defaultValue) {});
155+
list.forEach((Future<int> defaultValue) { defaultValue; });
156+
}
155157
''');
156158
await assertHasFix('''
157-
var x = ({defaultValue}) => null;
158-
var y = (defaultValue) => null;
159+
void f(List<Future<int>> list) {
160+
list.forEach((defaultValue) {});
161+
list.forEach((defaultValue) { defaultValue; });
162+
}
159163
''');
160164
}
161165
}
@@ -167,28 +171,58 @@ class AvoidTypesOnClosureParametersTest extends RemoveTypeAnnotationTest {
167171

168172
Future<void> test_namedParameter() async {
169173
await resolveTestCode('''
170-
var x = ({Future<int>? defaultValue}) => null;
174+
void f(C c) {
175+
c.forEach(({Future<int>? p}) {});
176+
}
177+
class C {
178+
void forEach(void Function({Future<int>? p})) {}
179+
}
171180
''');
172181
await assertHasFix('''
173-
var x = ({defaultValue}) => null;
182+
void f(C c) {
183+
c.forEach(({p}) {});
184+
}
185+
class C {
186+
void forEach(void Function({Future<int>? p})) {}
187+
}
174188
''');
175189
}
176190

177191
Future<void> test_normalParameter() async {
178192
await resolveTestCode('''
179-
var x = (Future<int> defaultValue) => null;
193+
void f(C c) {
194+
c.forEach((Future<int>? p) {});
195+
}
196+
class C {
197+
void forEach(void Function(Future<int>? p)) {}
198+
}
180199
''');
181200
await assertHasFix('''
182-
var x = (defaultValue) => null;
201+
void f(C c) {
202+
c.forEach((p) {});
203+
}
204+
class C {
205+
void forEach(void Function(Future<int>? p)) {}
206+
}
183207
''');
184208
}
185209

186210
Future<void> test_optionalParameter() async {
187211
await resolveTestCode('''
188-
var x = ([Future<int>? defaultValue]) => null;
212+
void f(C c) {
213+
c.forEach(([Future<int>? p]) {});
214+
}
215+
class C {
216+
void forEach(void Function([Future<int>? p])) {}
217+
}
189218
''');
190219
await assertHasFix('''
191-
var x = ([defaultValue]) => null;
220+
void f(C c) {
221+
c.forEach(([p]) {});
222+
}
223+
class C {
224+
void forEach(void Function([Future<int>? p])) {}
225+
}
192226
''');
193227
}
194228
}

‎pkg/analysis_server/test/src/services/correction/fix/replace_with_identifier_test.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,14 @@ class ReplaceWithIdentifierTest extends FixProcessorLintTest {
2525

2626
Future<void> test_functionTypedFormalParameter() async {
2727
await resolveTestCode('''
28-
var functionWithFunction = (int f(int x)) => f(0);
28+
void f(List<int Function(int)> list) {
29+
list.forEach((int p(int x)) {p(0);});
30+
}
2931
''');
3032
await assertHasFix('''
31-
var functionWithFunction = (f) => f(0);
33+
void f(List<int Function(int)> list) {
34+
list.forEach((p) {p(0);});
35+
}
3236
''');
3337
}
3438
}

‎pkg/linter/lib/src/rules/avoid_types_on_closure_parameters.dart

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:analyzer/dart/ast/visitor.dart';
77
import 'package:analyzer/dart/element/type.dart';
88

99
import '../analyzer.dart';
10+
import '../extensions.dart';
1011

1112
const _desc = r'Avoid annotating types for function expression parameters.';
1213

@@ -55,10 +56,10 @@ class AvoidTypesOnClosureParameters extends LintRule {
5556
}
5657
}
5758

58-
class AvoidTypesOnClosureParametersVisitor extends SimpleAstVisitor {
59-
LintRule rule;
59+
class _Visitor extends SimpleAstVisitor<void> {
60+
final LintRule rule;
6061

61-
AvoidTypesOnClosureParametersVisitor(this.rule);
62+
_Visitor(this.rule);
6263

6364
@override
6465
void visitDefaultFormalParameter(DefaultFormalParameter node) {
@@ -67,9 +68,8 @@ class AvoidTypesOnClosureParametersVisitor extends SimpleAstVisitor {
6768

6869
@override
6970
void visitFunctionExpression(FunctionExpression node) {
70-
if (node.parent is FunctionDeclaration) {
71-
return;
72-
}
71+
var contextType = node.approximateContextType;
72+
if (contextType is! FunctionType) return;
7373
var parameterList = node.parameters?.parameters;
7474
if (parameterList != null) {
7575
for (var parameter in parameterList) {
@@ -91,15 +91,3 @@ class AvoidTypesOnClosureParametersVisitor extends SimpleAstVisitor {
9191
}
9292
}
9393
}
94-
95-
class _Visitor extends SimpleAstVisitor<void> {
96-
final LintRule rule;
97-
98-
_Visitor(this.rule);
99-
100-
@override
101-
void visitFunctionExpression(FunctionExpression node) {
102-
var visitor = AvoidTypesOnClosureParametersVisitor(rule);
103-
visitor.visitFunctionExpression(node);
104-
}
105-
}

‎pkg/linter/test/rules/avoid_types_on_closure_parameters_test.dart

Lines changed: 109 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -35,91 +35,148 @@ void f(List<int> list) {
3535
]);
3636
}
3737

38-
test_initializerInDeclaration_namedOptional() async {
39-
// TODO(srawlins): Any test which just assigns a function literal to a
40-
// variable should be converted to something like an argument, or let
41-
// downwards inference work in some other way.
42-
// See https://github.com/dart-lang/linter/issues/1099.
38+
test_assignedToFunctionTypedTarget() async {
39+
await assertDiagnostics(r'''
40+
void f(C c) {
41+
c.onFoo = (int p) {};
42+
}
43+
class C {
44+
void Function(int p)? onFoo;
45+
}
46+
''', [
47+
lint(27, 3),
48+
]);
49+
}
50+
51+
test_assignedToNonFunctionTypedTarget() async {
52+
await assertNoDiagnostics(r'''
53+
var onFoo = (int p) {};
54+
''');
55+
}
56+
57+
test_closureIsArgument_dartCoreFunctionType() async {
58+
await assertNoDiagnostics(r'''
59+
void f(Future<void> future) {
60+
future.then((_) {}, onError: (e, st) {});
61+
}
62+
''');
63+
}
64+
65+
test_closureIsArgument_namedOptional() async {
4366
await assertNoDiagnostics(r'''
44-
var f = ({e}) => e.isEven;
67+
void f(C c) {
68+
c.map(({e}) => e?.isEven);
69+
}
70+
class C {
71+
void map(void Function({int? e})) {}
72+
}
4573
''');
4674
}
4775

48-
test_initializerInDeclaration_optionalNullable() async {
49-
// TODO(srawlins): Any test which just assigns a function literal to a
50-
// variable should be converted to something like an argument, or let
51-
// downwards inference work in some other way.
52-
// See https://github.com/dart-lang/linter/issues/1099.
76+
test_closureIsArgument_optionalNullable() async {
5377
await assertNoDiagnostics(r'''
54-
var f = ([e]) => e.name;
78+
void f(C c) {
79+
c.map(([e]) => e?.isEven);
80+
}
81+
class C {
82+
void map(void Function([int? e])) {}
83+
}
5584
''');
5685
}
5786

58-
test_initializerInDeclaration_optionalWithDefault() async {
59-
// TODO(srawlins): Any test which just assigns a function literal to a
60-
// variable should be converted to something like an argument, or let
61-
// downwards inference work in some other way.
62-
// See https://github.com/dart-lang/linter/issues/1099.
87+
test_closureIsArgument_optionalWithDefault() async {
6388
await assertNoDiagnostics(r'''
64-
var f = ({e = ''}) => e;
89+
void f(C c) {
90+
c.map(({e = 7}) => e?.isEven);
91+
}
92+
class C {
93+
void map(void Function({int? e})) {}
94+
}
6595
''');
6696
}
6797

68-
test_initializerInDeclaration_parameterIsTyped_dynamic() async {
69-
// TODO(srawlins): Any test which just assigns a function literal to a
70-
// variable should be converted to something like an argument, or let
71-
// downwards inference work in some other way.
72-
// See https://github.com/dart-lang/linter/issues/1099.
98+
test_closureIsArgument_parameterIsTyped_dynamic() async {
7399
await assertNoDiagnostics(r'''
74-
var goodName5 = (dynamic person) => person.name;
100+
void f(C c) {
101+
c.map((dynamic p) => p);
102+
}
103+
class C {
104+
void map(int Function(dynamic p)) {}
105+
}
75106
''');
76107
}
77108

78-
test_initializerInDeclaration_parameterIsTyped_functionType() async {
79-
// TODO(srawlins): Any test which just assigns a function literal to a
80-
// variable should be converted to something like an argument, or let
81-
// downwards inference work in some other way.
82-
// See https://github.com/dart-lang/linter/issues/1099.
109+
test_closureIsArgument_parameterIsTyped_functionType() async {
83110
await assertDiagnostics(r'''
84-
var functionWithFunction = (int f(int x)) => f(0);
111+
void f(List<int Function(int)> list) {
112+
list.map((int p(int x)) => p(0));
113+
}
85114
''', [
86-
lint(28, 12),
115+
lint(51, 12),
87116
]);
88117
}
89118

90-
test_initializerInDeclaration_parameterIsTyped_namedRequired() async {
91-
// TODO(srawlins): Any test which just assigns a function literal to a
92-
// variable should be converted to something like an argument, or let
93-
// downwards inference work in some other way.
94-
// See https://github.com/dart-lang/linter/issues/1099.
119+
test_closureIsArgument_parameterIsTyped_namedRequired() async {
95120
await assertDiagnostics(r'''
96-
var f = ({required int e}) => e.isEven;
121+
void f(C c) {
122+
c.map(({required int p}) => p);
123+
}
124+
class C {
125+
void map(int Function({required int p})) {}
126+
}
97127
''', [
98-
lint(19, 3),
128+
lint(33, 3),
99129
]);
100130
}
101131

102-
test_initializerInDeclaration_parameterIsTyped_optionalNullable() async {
103-
// TODO(srawlins): Any test which just assigns a function literal to a
104-
// variable should be converted to something like an argument, or let
105-
// downwards inference work in some other way.
106-
// See https://github.com/dart-lang/linter/issues/1099.
132+
test_closureIsArgument_parameterIsTyped_optionalNullable() async {
107133
await assertDiagnostics(r'''
108-
var f = ([int? e]) => e?.isEven;
134+
void f(C c) {
135+
c.map(([int? p]) => p);
136+
}
137+
class C {
138+
void map(int? Function([int? p])) {}
139+
}
109140
''', [
110-
lint(10, 4),
141+
lint(24, 4),
111142
]);
112143
}
113144

114-
test_initializerInDeclaration_parameterIsTyped_optionalWithDefault() async {
115-
// TODO(srawlins): Any test which just assigns a function literal to a
116-
// variable should be converted to something like an argument, or let
117-
// downwards inference work in some other way.
118-
// See https://github.com/dart-lang/linter/issues/1099.
145+
test_closureIsArgument_parameterIsTyped_optionalWithDefault() async {
119146
await assertDiagnostics(r'''
120-
var f = ([String e = '']) => e;
147+
void f(C c) {
148+
c.map(([int p = 0]) => p);
149+
}
150+
class C {
151+
void map(int? Function([int p])) {}
152+
}
121153
''', [
122-
lint(10, 6),
154+
lint(24, 3),
123155
]);
124156
}
157+
158+
test_parameterIsNotInClosure_inFunction() async {
159+
await assertNoDiagnostics(r'''
160+
void f(int p) {}
161+
''');
162+
}
163+
164+
test_parameterIsNotInClosure_inLocalFunction() async {
165+
await assertNoDiagnostics(r'''
166+
void f(List<int> list) {
167+
list.map((e) {
168+
void g(int p) {}
169+
return g(e);
170+
});
171+
}
172+
''');
173+
}
174+
175+
test_parameterIsNotInClosure_inMethod() async {
176+
await assertNoDiagnostics(r'''
177+
class C {
178+
void f(int p) {}
179+
}
180+
''');
181+
}
125182
}

0 commit comments

Comments
 (0)