Skip to content

Commit d469271

Browse files
FMorschelCommit Queue
FMorschel
authored and
Commit Queue
committed
[DAS] Make missing_required_argument fix work for multiple arguments
[email protected] Fixes: #46915 Fixes: #38876 Change-Id: I966462c38bb45773072729a9495f53b448574208 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410740 Reviewed-by: Phil Quitslund <[email protected]> Auto-Submit: Felipe Morschel <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Phil Quitslund <[email protected]>
1 parent 91a653b commit d469271

File tree

3 files changed

+130
-82
lines changed

3 files changed

+130
-82
lines changed

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

Lines changed: 123 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,44 @@ import 'package:_fe_analyzer_shared/src/scanner/token.dart';
66
import 'package:analysis_server/src/services/completion/dart/utilities.dart';
77
import 'package:analysis_server/src/services/correction/fix.dart';
88
import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
9+
import 'package:analysis_server_plugin/src/correction/fix_generators.dart';
910
import 'package:analyzer/dart/ast/ast.dart';
1011
import 'package:analyzer/dart/element/element2.dart';
12+
import 'package:analyzer/error/error.dart';
1113
import 'package:analyzer/src/utilities/extensions/flutter.dart';
1214
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
1315
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
1416
import 'package:collection/collection.dart';
1517

1618
class AddMissingRequiredArgument extends ResolvedCorrectionProducer {
17-
/// The name of the parameter that was missing.
18-
String _missingParameterName = '';
19+
/// The number of the parameters missing.
20+
late int _missingParameters;
1921

2022
AddMissingRequiredArgument({required super.context});
2123

2224
@override
2325
CorrectionApplicability get applicability =>
24-
// Not a stand-alone fix; requires follow-up actions.
25-
CorrectionApplicability
26-
.singleLocation;
26+
// Not a stand-alone fix; requires follow-up actions.
27+
CorrectionApplicability.singleLocation;
2728

2829
@override
29-
List<String> get fixArguments => [_missingParameterName];
30+
List<String> get fixArguments => ['$_missingParameters', _plural];
3031

3132
@override
3233
FixKind get fixKind => DartFixKind.ADD_MISSING_REQUIRED_ARGUMENT;
3334

35+
/// All the error codes that this fix can be applied to.
36+
List<ErrorCode> get _errorCodesWhereThisIsValid {
37+
var producerGenerators = [AddMissingRequiredArgument.new];
38+
var nonLintProducers = registeredFixGenerators.nonLintProducers;
39+
return [
40+
for (var MapEntry(:key, :value) in nonLintProducers.entries)
41+
if (value.containsAny(producerGenerators)) key,
42+
];
43+
}
44+
45+
String get _plural => _missingParameters == 1 ? '' : 's';
46+
3447
@override
3548
Future<void> compute(ChangeBuilder builder) async {
3649
InstanceCreationExpression? creation;
@@ -54,90 +67,126 @@ class AddMissingRequiredArgument extends ResolvedCorrectionProducer {
5467
}
5568

5669
var diagnostic = this.diagnostic;
57-
if (diagnostic == null) {
70+
if (diagnostic is! AnalysisError) {
5871
return;
5972
}
6073

61-
if (targetElement is ExecutableElement2 && argumentList != null) {
62-
// Format: "Missing required argument 'foo'."
63-
var messageParts = diagnostic.problemMessage
64-
.messageText(includeUrl: false)
65-
.split("'");
66-
if (messageParts.length < 2) {
67-
return;
68-
}
69-
_missingParameterName = messageParts[1];
74+
var validErrors = _errorCodesWhereThisIsValid;
75+
var errors = unitResult.errors.where(
76+
(e) => diagnostic.sameRangeAs(e) && validErrors.contains(e.errorCode),
77+
);
7078

71-
var missingParameter = targetElement.formalParameters.firstWhereOrNull(
72-
(p) => p.name3 == _missingParameterName,
73-
);
74-
if (missingParameter == null) {
75-
return;
76-
}
79+
// Should not happen since the current diagnostic is in the list of errors
80+
// where this fix is valid.
81+
if (errors.isEmpty) {
82+
errors = [diagnostic];
83+
}
7784

78-
int offset;
79-
var hasTrailingComma = false;
80-
var insertBetweenParams = false;
81-
var arguments = argumentList.arguments;
82-
if (arguments.isEmpty) {
83-
offset = argumentList.leftParenthesis.end;
84-
} else {
85-
var lastArgument = arguments.last;
86-
offset = lastArgument.end;
87-
hasTrailingComma = lastArgument.endToken.next!.type == TokenType.COMMA;
88-
89-
if (lastArgument is NamedExpression && creation.isWidgetExpression) {
90-
if (lastArgument.isChildArgument || lastArgument.isChildrenArgument) {
91-
offset = lastArgument.offset;
92-
hasTrailingComma = true;
93-
insertBetweenParams = true;
94-
}
85+
// This should only trigger once and the disposition of the valid
86+
// diagnostics in the unit should always be the same.
87+
if (errors.first != diagnostic) {
88+
return;
89+
}
90+
91+
_missingParameters = errors.length;
92+
93+
for (var (index, diagnostic) in errors.indexed) {
94+
if (targetElement is ExecutableElement2 && argumentList != null) {
95+
// Format: "Missing required argument 'foo'."
96+
var messageParts = diagnostic.problemMessage
97+
.messageText(includeUrl: false)
98+
.split("'");
99+
if (messageParts.length < 2) {
100+
return;
95101
}
96-
}
97102

98-
var codeStyleOptions = getCodeStyleOptions(unitResult.file);
99-
var defaultValue = getDefaultStringParameterValue(
100-
missingParameter,
101-
codeStyleOptions.preferredQuoteForStrings,
102-
);
103+
var parameterName = messageParts[1];
104+
105+
var missingParameter = targetElement.formalParameters.firstWhereOrNull(
106+
(p) => p.name3 == parameterName,
107+
);
108+
if (missingParameter == null) {
109+
return;
110+
}
103111

104-
await builder.addDartFileEdit(file, (builder) {
105-
builder.addInsertion(offset, (builder) {
106-
if (arguments.isNotEmpty && !insertBetweenParams) {
107-
builder.write(', ');
112+
int offset;
113+
var hasTrailingComma = false;
114+
var insertBetweenParams = false;
115+
var arguments = argumentList.arguments;
116+
if (arguments.isEmpty) {
117+
offset = argumentList.leftParenthesis.end;
118+
} else {
119+
var lastArgument = arguments.last;
120+
offset = lastArgument.end;
121+
hasTrailingComma =
122+
lastArgument.endToken.next!.type == TokenType.COMMA;
123+
124+
if (lastArgument is NamedExpression && creation.isWidgetExpression) {
125+
if (lastArgument.isChildArgument ||
126+
lastArgument.isChildrenArgument) {
127+
offset = lastArgument.offset;
128+
hasTrailingComma = true;
129+
insertBetweenParams = true;
130+
}
108131
}
132+
}
109133

110-
builder.write('$_missingParameterName: ');
134+
var codeStyleOptions = getCodeStyleOptions(unitResult.file);
135+
var defaultValue = getDefaultStringParameterValue(
136+
missingParameter,
137+
codeStyleOptions.preferredQuoteForStrings,
138+
);
111139

112-
// Use defaultValue.cursorPosition if it's not null.
113-
if (defaultValue != null) {
114-
var text = defaultValue.text;
115-
var cursorPosition = defaultValue.cursorPosition;
116-
if (cursorPosition != null) {
117-
builder.write(text.substring(0, cursorPosition));
118-
builder.selectHere();
119-
builder.write(text.substring(cursorPosition));
120-
} else {
121-
builder.addSimpleLinkedEdit('VALUE', text);
140+
await builder.addDartFileEdit(file, (builder) {
141+
builder.addInsertion(offset, (builder) {
142+
if ((arguments.isNotEmpty || index > 0) && !insertBetweenParams) {
143+
builder.write(', ');
122144
}
123-
} else {
124-
builder.addSimpleLinkedEdit('VALUE', 'null');
125-
}
126145

127-
if (creation.isWidgetExpression) {
128-
// Insert a trailing comma after Flutter instance creation params.
129-
if (!hasTrailingComma) {
130-
builder.write(',');
131-
} else if (insertBetweenParams) {
132-
builder.writeln(',');
146+
builder.write('$parameterName: ');
147+
148+
// Use defaultValue.cursorPosition if it's not null.
149+
if (defaultValue != null) {
150+
var text = defaultValue.text;
151+
var cursorPosition = defaultValue.cursorPosition;
152+
if (cursorPosition != null) {
153+
builder.write(text.substring(0, cursorPosition));
154+
builder.selectHere();
155+
builder.write(text.substring(cursorPosition));
156+
} else {
157+
builder.addSimpleLinkedEdit('VALUE', text);
158+
}
159+
} else {
160+
builder.addSimpleLinkedEdit('VALUE', 'null');
161+
}
133162

134-
// Insert indent before the child: or children: param.
135-
var indent = utils.getLinePrefix(offset);
136-
builder.write(indent);
163+
if (creation.isWidgetExpression) {
164+
// Insert a trailing comma after Flutter instance creation params.
165+
if (!hasTrailingComma) {
166+
builder.write(',');
167+
} else if (insertBetweenParams) {
168+
builder.writeln(',');
169+
170+
// Insert indent before the child: or children: param.
171+
var indent = utils.getLinePrefix(offset);
172+
builder.write(indent);
173+
}
137174
}
138-
}
175+
});
139176
});
140-
});
177+
}
141178
}
142179
}
143180
}
181+
182+
extension<T> on Iterable<T> {
183+
bool containsAny(Iterable<T> values) {
184+
return values.any((v) => contains(v));
185+
}
186+
}
187+
188+
extension on AnalysisError {
189+
bool sameRangeAs(AnalysisError other) {
190+
return offset == other.offset && length == other.length;
191+
}
192+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ abstract final class DartFixKind {
210210
static const ADD_MISSING_REQUIRED_ARGUMENT = FixKind(
211211
'dart.fix.add.missingRequiredArgument',
212212
70,
213-
"Add required argument '{0}'",
213+
'Add {0} required argument{1}',
214214
);
215215
static const ADD_MISSING_SWITCH_CASES = FixKind(
216216
'dart.fix.add.missingSwitchCases',

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -344,24 +344,23 @@ void f() {
344344
await assertHasFix('''
345345
test({required int a, required int bcd}) {}
346346
void f() {
347-
test(a: null);
347+
test(a: null, bcd: null);
348348
}
349349
''', errorFilter: (error) => error.message.contains("'a'"));
350350
}
351351

352+
/// Asserts that no fix is available for other diagnostics than the first one.
353+
///
354+
/// This has no fix because the [test_multiple_1of2] should trigger the fix
355+
/// since it is the first error in the list.
352356
Future<void> test_multiple_2of2() async {
353357
await resolveTestCode('''
354358
test({required int a, required int bcd}) {}
355359
void f() {
356360
test();
357361
}
358362
''');
359-
await assertHasFix('''
360-
test({required int a, required int bcd}) {}
361-
void f() {
362-
test(bcd: null);
363-
}
364-
''', errorFilter: (error) => error.message.contains("'bcd'"));
363+
await assertNoFix(errorFilter: (error) => error.message.contains("'bcd'"));
365364
}
366365

367366
Future<void> test_nonNullable() async {

0 commit comments

Comments
 (0)