Skip to content

Commit 443e8cc

Browse files
FMorschelCommit Queue
FMorschel
authored and
Commit Queue
committed
[DAS] Fixes multiple commas by Add missing required arguments
This also refactors the correction producer so that it is a bit easier to read and maintain. Fixes: #61531 Change-Id: I54656f3e917764881bc4817f7b9692bb9fd973b5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450360 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Auto-Submit: Felipe Morschel <[email protected]>
1 parent b7df75c commit 443e8cc

File tree

2 files changed

+145
-76
lines changed

2 files changed

+145
-76
lines changed

‎pkg/analysis_server/lib/src/services/correction/dart/add_missing_required_argument.dart‎

Lines changed: 120 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'package:_fe_analyzer_shared/src/scanner/token.dart';
65
import 'package:analysis_server/src/services/completion/dart/utilities.dart';
76
import 'package:analysis_server/src/services/correction/fix.dart';
87
import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
98
import 'package:analysis_server_plugin/src/correction/fix_generators.dart';
109
import 'package:analyzer/dart/ast/ast.dart';
10+
import 'package:analyzer/dart/ast/token.dart';
1111
import 'package:analyzer/dart/element/element.dart';
1212
import 'package:analyzer/diagnostic/diagnostic.dart';
1313
import 'package:analyzer/error/error.dart';
@@ -68,7 +68,7 @@ class AddMissingRequiredArgument extends ResolvedCorrectionProducer {
6868
}
6969

7070
var diagnostic = this.diagnostic;
71-
if (diagnostic is! Diagnostic) {
71+
if (diagnostic == null) {
7272
return;
7373
}
7474

@@ -80,6 +80,7 @@ class AddMissingRequiredArgument extends ResolvedCorrectionProducer {
8080
// Should not happen since the current diagnostic is in the list of errors
8181
// where this fix is valid.
8282
if (diagnostics.isEmpty) {
83+
assert(false, 'No valid diagnostics found, but expected at least one.');
8384
diagnostics = [diagnostic];
8485
}
8586

@@ -91,92 +92,135 @@ class AddMissingRequiredArgument extends ResolvedCorrectionProducer {
9192

9293
_missingParameters = diagnostics.length;
9394

94-
for (var (index, diagnostic) in diagnostics.indexed) {
95-
if (targetElement is ExecutableElement && argumentList != null) {
96-
// Format: "Missing required argument 'foo'."
97-
var messageParts = diagnostic.problemMessage
98-
.messageText(includeUrl: false)
99-
.split("'");
100-
if (messageParts.length < 2) {
101-
return;
95+
if (argumentList == null || targetElement is! ExecutableElement) {
96+
return;
97+
}
98+
var arguments = argumentList.arguments;
99+
100+
int offset;
101+
var insertLeadingComma = false;
102+
var insertFlutterTrailingComma = true;
103+
var insertBetweenFlutterParams = false;
104+
if (arguments.isEmpty) {
105+
offset = argumentList.leftParenthesis.end;
106+
} else {
107+
var lastArgument = arguments.last;
108+
offset = lastArgument.end;
109+
insertLeadingComma = true;
110+
if (creation.isWidgetExpression) {
111+
// If the last argument is a `child:` or `children:` parameter in a
112+
// Flutter widget, insert the new parameter(s) before it.
113+
if (lastArgument is NamedExpression) {
114+
if (lastArgument.isChildArgument || lastArgument.isChildrenArgument) {
115+
offset = lastArgument.offset;
116+
insertBetweenFlutterParams = true;
117+
// Insert a leading comma if we are have a case like:
118+
// ```dart
119+
// MyWidget(other: 1, child: ...,)
120+
// ```
121+
// so that we get:
122+
// ```dart
123+
// MyWidget(other: 1, newParam: ..., child: ...,)
124+
// ```
125+
// since the offset is at the end of `other: 1`.
126+
insertLeadingComma = arguments.length > 1;
127+
}
102128
}
129+
if (lastArgument.endToken.next case Token(
130+
type: TokenType.COMMA,
131+
) when !insertBetweenFlutterParams) {
132+
// If there is a trailing comma after the last argument, don't add
133+
// another one.
134+
insertFlutterTrailingComma = false;
135+
}
136+
}
137+
}
103138

104-
var parameterName = messageParts[1];
139+
Map<String, DefaultArgument?> parameters = {};
140+
for (var diagnostic in diagnostics) {
141+
// Format: "Missing required argument 'foo'."
142+
var messageParts = diagnostic.problemMessage
143+
.messageText(includeUrl: false)
144+
.split("'");
145+
if (messageParts.length < 2) {
146+
return;
147+
}
148+
149+
var parameterName = messageParts[1];
105150

106-
var missingParameter = targetElement.formalParameters.firstWhereOrNull(
107-
(p) => p.name == parameterName,
151+
var missingParameter = targetElement.formalParameters.firstWhereOrNull(
152+
(p) => p.name == parameterName,
153+
);
154+
if (missingParameter == null) {
155+
assert(
156+
false,
157+
'Could not find parameter $parameterName, but expected since it was '
158+
'reported as missing.',
108159
);
109-
if (missingParameter == null) {
110-
return;
111-
}
160+
continue;
161+
}
112162

113-
int offset;
114-
var hasTrailingComma = false;
115-
var insertBetweenParams = false;
116-
var arguments = argumentList.arguments;
117-
if (arguments.isEmpty) {
118-
offset = argumentList.leftParenthesis.end;
119-
} else {
120-
var lastArgument = arguments.last;
121-
offset = lastArgument.end;
122-
hasTrailingComma =
123-
lastArgument.endToken.next!.type == TokenType.COMMA;
124-
125-
if (lastArgument is NamedExpression && creation.isWidgetExpression) {
126-
if (lastArgument.isChildArgument ||
127-
lastArgument.isChildrenArgument) {
128-
offset = lastArgument.offset;
129-
hasTrailingComma = true;
130-
insertBetweenParams = true;
131-
}
132-
}
133-
}
163+
var codeStyleOptions = getCodeStyleOptions(unitResult.file);
164+
var defaultValue = getDefaultStringParameterValue(
165+
missingParameter,
166+
codeStyleOptions.preferredQuoteForStrings,
167+
);
134168

135-
var codeStyleOptions = getCodeStyleOptions(unitResult.file);
136-
var defaultValue = getDefaultStringParameterValue(
137-
missingParameter,
138-
codeStyleOptions.preferredQuoteForStrings,
139-
);
169+
parameters[parameterName] = defaultValue;
170+
}
140171

141-
await builder.addDartFileEdit(file, (builder) {
142-
builder.addInsertion(offset, (builder) {
143-
if ((arguments.isNotEmpty || index > 0) && !insertBetweenParams) {
144-
builder.write(', ');
145-
}
172+
var parametersLength = parameters.length;
146173

147-
builder.write('$parameterName: ');
148-
149-
// Use defaultValue.cursorPosition if it's not null.
150-
if (defaultValue != null) {
151-
var text = defaultValue.text;
152-
var cursorPosition = defaultValue.cursorPosition;
153-
if (cursorPosition != null) {
154-
builder.write(text.substring(0, cursorPosition));
155-
builder.selectHere();
156-
builder.write(text.substring(cursorPosition));
157-
} else {
158-
builder.addSimpleLinkedEdit('VALUE', text);
159-
}
174+
await builder.addDartFileEdit(file, (builder) {
175+
builder.addInsertion(offset, (builder) {
176+
if (insertLeadingComma) {
177+
builder.write(', ');
178+
}
179+
var entries = parameters.entries.toList();
180+
var childOrChildren = entries.firstWhereOrNull(
181+
(e) => const {'child', 'children'}.contains(e.key),
182+
);
183+
if (childOrChildren != null) {
184+
// If there is a child: or children: parameter, insert it last.
185+
entries.remove(childOrChildren);
186+
entries.add(childOrChildren);
187+
}
188+
entries.forEachIndexed((index, entry) {
189+
var MapEntry(key: parameterName, value: defaultValue) = entry;
190+
191+
builder.write('$parameterName: ');
192+
193+
// Use defaultValue.cursorPosition if it's not null.
194+
if (defaultValue != null) {
195+
var text = defaultValue.text;
196+
var cursorPosition = defaultValue.cursorPosition;
197+
if (cursorPosition != null) {
198+
builder.write(text.substring(0, cursorPosition));
199+
builder.selectHere();
200+
builder.write(text.substring(cursorPosition));
160201
} else {
161-
builder.addSimpleLinkedEdit('VALUE', 'null');
202+
builder.addSimpleLinkedEdit('VALUE', text);
162203
}
204+
} else {
205+
builder.addSimpleLinkedEdit('VALUE', 'null');
206+
}
163207

164-
if (creation.isWidgetExpression) {
165-
// Insert a trailing comma after Flutter instance creation params.
166-
if (!hasTrailingComma) {
167-
builder.write(',');
168-
} else if (insertBetweenParams) {
169-
builder.writeln(',');
170-
171-
// Insert indent before the child: or children: param.
172-
var indent = utils.getLinePrefix(offset);
173-
builder.write(indent);
174-
}
175-
}
176-
});
208+
if (index < parametersLength - 1) {
209+
builder.write(', ');
210+
} else if (insertBetweenFlutterParams) {
211+
// Insert a trailing comma after Flutter instance creation params.
212+
builder.writeln(',');
213+
// Insert indent before the child: or children: param.
214+
var indent = utils.getLinePrefix(offset);
215+
builder.write(indent);
216+
} else if (creation.isWidgetExpression &&
217+
insertFlutterTrailingComma) {
218+
// If this is a Flutter widget, format with a trailing comma.
219+
builder.write(',');
220+
}
177221
});
178-
}
179-
}
222+
});
223+
});
180224
}
181225
}
182226

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,31 @@ void f() {
319319
''');
320320
}
321321

322+
Future<void> test_flutter_closure_children() async {
323+
await resolveTestCode('''
324+
import 'package:flutter/widgets.dart';
325+
326+
class MyWidget extends Widget {
327+
MyWidget({required List<Widget> children, required void Function()? fn});
328+
}
329+
330+
build() {
331+
return new MyWidget();
332+
}
333+
''');
334+
await assertHasFix('''
335+
import 'package:flutter/widgets.dart';
336+
337+
class MyWidget extends Widget {
338+
MyWidget({required List<Widget> children, required void Function()? fn});
339+
}
340+
341+
build() {
342+
return new MyWidget(fn: () { }, children: [],);
343+
}
344+
''', errorFilter: (diagnostic) => diagnostic.message.contains("'children'"));
345+
}
346+
322347
Future<void> test_functionType_noParameterName() async {
323348
await resolveTestCode('''
324349
void foo({required void Function(int) f}) {}

0 commit comments

Comments
 (0)