Skip to content

Commit f58012e

Browse files
FMorschelCommit Queue
authored andcommitted
[DAS] Fixes writeType importing shadowed type by local declaration
Fixes: #59701 Change-Id: Ief2816ee8b05296adfdbb4f2c1f01a8892711136 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/451404 Auto-Submit: Felipe Morschel <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent cc876c2 commit f58012e

File tree

6 files changed

+77
-12
lines changed

6 files changed

+77
-12
lines changed

pkg/analysis_server/test/src/services/correction/assist/add_type_annotation_test.dart

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,11 +483,9 @@ void f(int Function(int) p) {
483483
v = p;
484484
}
485485
''');
486-
// TODO(brianwilkerson): Improve `DartChangeBuilder.writeType` so that
487-
// unnecessary parameter names (`p1`) are not written.
488486
await assertHasAssist('''
489487
void f(int Function(int) p) {
490-
int Function(int p1) v;
488+
int Function(int) v;
491489
v = p;
492490
}
493491
''');
@@ -678,6 +676,34 @@ void f() {
678676
''');
679677
}
680678

679+
Future<void> test_local_shadowed() async {
680+
newFile(join(testPackageLibPath, 'a.dart'), '''
681+
class A {}
682+
''');
683+
newFile(join(testPackageLibPath, 'other.dart'), '''
684+
import 'a.dart';
685+
A getA() => A();
686+
''');
687+
await resolveTestCode('''
688+
import 'other.dart';
689+
690+
class A {}
691+
void f() {
692+
var ^v = getA();
693+
}
694+
''');
695+
await assertHasAssist('''
696+
import 'package:test/a.dart' as prefix0;
697+
698+
import 'other.dart';
699+
700+
class A {}
701+
void f() {
702+
prefix0.A v = getA();
703+
}
704+
''');
705+
}
706+
681707
Future<void> test_local_unknown() async {
682708
verifyNoTestUnitErrors = false;
683709
await resolveTestCode('''

pkg/analysis_server/test/src/services/correction/fix/add_type_annotation_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ f() {
208208
switch (A(1)) {
209209
case A(a: >0 && final b): print(b);
210210
}
211-
}
211+
}
212212
''');
213213
await assertHasFix('''
214214
class A {
@@ -219,7 +219,7 @@ f() {
219219
switch (A(1)) {
220220
case A(a: >0 && final int b): print(b);
221221
}
222-
}
222+
}
223223
''');
224224
}
225225

pkg/analysis_server/test/src/services/correction/fix/convert_to_function_declaration_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ void f() {
156156
''');
157157
await assertHasFix('''
158158
void f() {
159-
int v1(int Function(String p1)? p) {
159+
int v1(int Function(String)? p) {
160160
return p?.call('') ?? 0;
161161
}
162162
v1((s) => 0);

pkg/analyzer_plugin/api.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1731,7 +1731,7 @@ package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart:
17311731
writeConstructorDeclaration (method: void Function(String, {ArgumentList? argumentList, void Function()? bodyWriter, String? classNameGroupName, String? constructorName, String? constructorNameGroupName, List<String>? fieldNames, void Function()? initializerWriter, bool isConst, void Function()? parameterWriter}))
17321732
writeFieldDeclaration (method: void Function(String, {void Function()? initializerWriter, bool isConst, bool isFinal, bool isStatic, String? nameGroupName, DartType? type, String? typeGroupName}))
17331733
writeFormalParameter (method: void Function(String, {bool isCovariant, bool isRequiredNamed, bool isRequiredType, deprecated ExecutableElement? methodBeingCopied, String? nameGroupName, DartType? type, String? typeGroupName, List<TypeParameterElement>? typeParametersInScope}))
1734-
writeFormalParameters (method: void Function(Iterable<FormalParameterElement>, {String? groupNamePrefix, bool includeDefaultValues, deprecated ExecutableElement? methodBeingCopied, bool requiredTypes, List<TypeParameterElement>? typeParametersInScope}))
1734+
writeFormalParameters (method: void Function(Iterable<FormalParameterElement>, {bool fillParameterNames, String? groupNamePrefix, bool includeDefaultValues, deprecated ExecutableElement? methodBeingCopied, bool requiredTypes, List<TypeParameterElement>? typeParametersInScope}))
17351735
writeFunctionDeclaration (method: void Function(String, {void Function()? bodyWriter, bool isStatic, String? nameGroupName, void Function()? parameterWriter, DartType? returnType, String? returnTypeGroupName}))
17361736
writeGetterDeclaration (method: void Function(String, {void Function() bodyWriter, bool isStatic, String nameGroupName, DartType returnType, String returnTypeGroupName}))
17371737
writeImportedName (method: void Function(List<Uri>, String))

pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ class DartEditBuilderImpl extends EditBuilderImpl implements DartEditBuilder {
333333
ExecutableElement? methodBeingCopied,
334334
List<TypeParameterElement>? typeParametersInScope,
335335
String? groupNamePrefix,
336+
bool fillParameterNames = true,
336337
bool includeDefaultValues = true,
337338
bool requiredTypes = false,
338339
}) {
@@ -364,13 +365,13 @@ class DartEditBuilderImpl extends EditBuilderImpl implements DartEditBuilder {
364365
}
365366
// Parameter.
366367
var name = parameter.name;
367-
if (name == null || name == '') {
368-
name = _generateUniqueName(parameterNames, 'p');
368+
if ((name == null || name == '') && fillParameterNames) {
369+
name = _generateUniqueName(parameterNames);
369370
parameterNames.add(name);
370371
}
371372
var groupPrefix = groupNamePrefix != null ? '$groupNamePrefix:' : '';
372373
writeFormalParameter(
373-
name,
374+
name ?? '',
374375
isCovariant: parameter.isCovariant,
375376
isRequiredNamed: parameter.isRequiredNamed,
376377
typeParametersInScope: typeParametersInScope,
@@ -1191,7 +1192,7 @@ class DartEditBuilderImpl extends EditBuilderImpl implements DartEditBuilder {
11911192

11921193
/// Generates a name that does not occur in [existingNames] that begins with
11931194
/// the given [prefix].
1194-
String _generateUniqueName(Set<String> existingNames, String prefix) {
1195+
String _generateUniqueName(Set<String> existingNames, {String prefix = 'p'}) {
11951196
var index = 1;
11961197
var name = '$prefix$index';
11971198
while (existingNames.contains(name)) {
@@ -1459,7 +1460,16 @@ class DartEditBuilderImpl extends EditBuilderImpl implements DartEditBuilder {
14591460
if (import == null) {
14601461
var library = element.library?.uri;
14611462
if (library != null) {
1462-
import = _dartFileEditBuilder._importLibrary(library);
1463+
var shadowed =
1464+
_dartFileEditBuilder.resolvedLibrary.element.publicNamespace.get2(
1465+
element.displayName,
1466+
) !=
1467+
null;
1468+
String? prefix;
1469+
if (shadowed) {
1470+
prefix = _dartFileEditBuilder._defaultImportPrefixFor(library);
1471+
}
1472+
import = _dartFileEditBuilder._importLibrary(library, prefix: prefix);
14631473
}
14641474
}
14651475
if (import == null) {
@@ -1561,6 +1571,7 @@ class DartEditBuilderImpl extends EditBuilderImpl implements DartEditBuilder {
15611571
type.formalParameters,
15621572
typeParametersInScope: typeParametersInScope,
15631573
includeDefaultValues: false,
1574+
fillParameterNames: false,
15641575
);
15651576
if (type.nullabilitySuffix == NullabilitySuffix.question) {
15661577
write('?');
@@ -2606,6 +2617,33 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl
26062617
}, insertBeforeExisting: true);
26072618
}
26082619

2620+
String _defaultImportPrefixFor(Uri uri) {
2621+
if (importPrefixGenerator != null) {
2622+
return importPrefixGenerator!(uri);
2623+
}
2624+
// TODO(FMorschel): Think of a way to identify if the current editing range
2625+
// already contains a variable with the same name as the generated prefix.
2626+
// This only accounts for top-level names.
2627+
var existingNames = {
2628+
...resolvedLibrary.element.exportNamespace.definedNames2.keys,
2629+
};
2630+
for (var unit in resolvedLibrary.units) {
2631+
existingNames.addAll(
2632+
unit.unit.directives
2633+
.whereType<ImportDirective>()
2634+
.map((d) => d.prefix?.name)
2635+
.nonNulls,
2636+
);
2637+
}
2638+
var suffix = 0;
2639+
var prefix = 'prefix$suffix';
2640+
while (existingNames.contains(prefix)) {
2641+
suffix++;
2642+
prefix = 'prefix$suffix';
2643+
}
2644+
return prefix;
2645+
}
2646+
26092647
/// Returns information about the library used to import the given [element]
26102648
/// into the target library, or `null` if the element was not imported, such
26112649
/// as when the element is declared in the same library.

pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ abstract class DartEditBuilder implements EditBuilder {
185185
ExecutableElement? methodBeingCopied,
186186
List<TypeParameterElement>? typeParametersInScope,
187187
String? groupNamePrefix,
188+
bool fillParameterNames = true,
188189
bool includeDefaultValues = true,
189190
bool requiredTypes,
190191
});

0 commit comments

Comments
 (0)