Skip to content

Commit a1eec6b

Browse files
asashourCommit Queue
authored and
Commit Queue
committed
[analysis_server] RemoveLeadingUnderscore to handle conflicting names
Fixes #50142 Change-Id: I68c225e0528c643a29f2454607464d3896f25d36 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/262606 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent 5b11919 commit a1eec6b

File tree

3 files changed

+120
-3
lines changed

3 files changed

+120
-3
lines changed

pkg/analysis_server/lib/src/services/correction/dart/remove_leading_underscore.dart

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,22 @@ class RemoveLeadingUnderscore extends CorrectionProducer {
5757
// Find references to the identifier.
5858
List<SimpleIdentifier>? references;
5959
if (element is LocalVariableElement) {
60-
var root = node.thisOrAncestorOfType<Block>();
61-
if (root != null) {
62-
references = findLocalElementReferences(root, element);
60+
var block = node.thisOrAncestorOfType<Block>();
61+
if (block != null) {
62+
references = findLocalElementReferences(block, element);
63+
64+
var declaration = block.thisOrAncestorOfType<MethodDeclaration>() ??
65+
block.thisOrAncestorOfType<FunctionDeclaration>();
66+
67+
if (declaration != null) {
68+
if (isDeclaredIn(declaration, newName)) {
69+
var suffix = -1;
70+
do {
71+
suffix++;
72+
} while (isDeclaredIn(declaration, '$newName$suffix'));
73+
newName = '$newName$suffix';
74+
}
75+
}
6376
}
6477
} else if (element is ParameterElement) {
6578
if (!element.isNamed) {

pkg/analysis_server/lib/src/services/correction/util.dart

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,33 @@ bool hasDisplayName(Element? element, String name) {
466466
return element?.displayName == name;
467467
}
468468

469+
/// Return whether the specified [name] is declared inside the [root] node
470+
/// or not.
471+
bool isDeclaredIn(AstNode root, String name) {
472+
bool isDeclaredIn(FormalParameterList? parameters) {
473+
if (parameters != null) {
474+
for (var parameter in parameters.parameters) {
475+
if (parameter.name?.lexeme == name) {
476+
return true;
477+
}
478+
}
479+
}
480+
return false;
481+
}
482+
483+
if (root is MethodDeclaration && isDeclaredIn(root.parameters)) {
484+
return true;
485+
}
486+
if (root is FunctionDeclaration &&
487+
isDeclaredIn(root.functionExpression.parameters)) {
488+
return true;
489+
}
490+
491+
var collector = _DeclarationCollector(name);
492+
root.accept(collector);
493+
return collector.isDeclared;
494+
}
495+
469496
/// Checks if given [DartNode] is the left hand side of an assignment, or a
470497
/// declaration of a variable.
471498
bool isLeftHandOfAssignment(SimpleIdentifier node) {
@@ -1542,6 +1569,20 @@ class _CollectReferencedUnprefixedNames extends RecursiveAstVisitor {
15421569
}
15431570
}
15441571

1572+
class _DeclarationCollector extends RecursiveAstVisitor<void> {
1573+
final String name;
1574+
bool isDeclared = false;
1575+
1576+
_DeclarationCollector(this.name);
1577+
1578+
@override
1579+
void visitVariableDeclaration(VariableDeclaration node) {
1580+
if (node.name.lexeme == name) {
1581+
isDeclared = true;
1582+
}
1583+
}
1584+
}
1585+
15451586
class _ElementReferenceCollector extends RecursiveAstVisitor<void> {
15461587
final Element element;
15471588
final List<SimpleIdentifier> references = [];

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,69 @@ void f() {
8989
''');
9090
}
9191

92+
Future<void> test_localVariable_conflictWithParameter() async {
93+
await resolveTestCode('''
94+
class A {
95+
void m({int? foo}) {
96+
var _foo = 1;
97+
print(foo);
98+
print(_foo);
99+
}
100+
}
101+
''');
102+
await assertHasFix('''
103+
class A {
104+
void m({int? foo}) {
105+
var foo0 = 1;
106+
print(foo);
107+
print(foo0);
108+
}
109+
}
110+
''');
111+
}
112+
113+
Future<void> test_localVariable_conflictWithVariable() async {
114+
await resolveTestCode('''
115+
void f() {
116+
var _foo = 1;
117+
var foo = true;
118+
print(_foo);
119+
print(foo);
120+
}
121+
''');
122+
await assertHasFix('''
123+
void f() {
124+
var foo0 = 1;
125+
var foo = true;
126+
print(foo0);
127+
print(foo);
128+
}
129+
''');
130+
}
131+
132+
Future<void> test_localVariable_conflictWithVariable_existing() async {
133+
await resolveTestCode('''
134+
void f() {
135+
var _foo = 1;
136+
var foo = true;
137+
var foo0 = true;
138+
print(_foo);
139+
print(foo);
140+
print(foo0);
141+
}
142+
''');
143+
await assertHasFix('''
144+
void f() {
145+
var foo1 = 1;
146+
var foo = true;
147+
var foo0 = true;
148+
print(foo1);
149+
print(foo);
150+
print(foo0);
151+
}
152+
''');
153+
}
154+
92155
Future<void> test_parameter_closure() async {
93156
await resolveTestCode('''
94157
void f() {

0 commit comments

Comments
 (0)