Skip to content

Commit 07b7cfe

Browse files
FMorschelCommit Queue
FMorschel
authored and
Commit Queue
committed
New assist to add/edit hide at import for ambiguous import
Closes #56762 GitOrigin-RevId: af16e24 Change-Id: Ibbc49c36860ba6e58c45dfc40029e2375396c153 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386020 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]>
1 parent 8aa6b41 commit 07b7cfe

File tree

8 files changed

+1297
-10
lines changed

8 files changed

+1297
-10
lines changed

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

Lines changed: 336 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,336 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analysis_server/src/services/correction/fix.dart';
6+
import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
7+
import 'package:analyzer/dart/analysis/results.dart';
8+
import 'package:analyzer/dart/ast/ast.dart';
9+
import 'package:analyzer/dart/element/element2.dart';
10+
import 'package:analyzer/source/source_range.dart';
11+
import 'package:analyzer/src/dart/ast/extensions.dart';
12+
import 'package:analyzer/src/utilities/extensions/results.dart';
13+
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
14+
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
15+
import 'package:analyzer_plugin/utilities/range_factory.dart';
16+
import 'package:collection/collection.dart';
17+
18+
class AmbiguousImportFix extends MultiCorrectionProducer {
19+
AmbiguousImportFix({required super.context});
20+
21+
@override
22+
Future<List<ResolvedCorrectionProducer>> get producers async {
23+
var node = this.node;
24+
Element2? element;
25+
String? prefix;
26+
if (node is NamedType) {
27+
element = node.element2;
28+
prefix = node.importPrefix?.name.lexeme;
29+
} else if (node is SimpleIdentifier) {
30+
element = node.element;
31+
if (node.parent case PrefixedIdentifier(prefix: var currentPrefix)) {
32+
prefix = currentPrefix.name;
33+
}
34+
}
35+
if (element is! MultiplyDefinedElement2) {
36+
return const [];
37+
}
38+
var conflictingElements = element.conflictingElements2;
39+
var name = element.name3;
40+
if (name == null || name.isEmpty) {
41+
return const [];
42+
}
43+
44+
var (unit, importDirectives, uris) = _getImportDirectives(
45+
libraryResult,
46+
unitResult,
47+
conflictingElements,
48+
name,
49+
prefix,
50+
);
51+
52+
// If we have multiple imports of the same library, then we won't fix it.
53+
if (uris.length != uris.toSet().length) {
54+
return const [];
55+
}
56+
57+
if (unit == null || importDirectives.isEmpty || uris.isEmpty) {
58+
return const [];
59+
}
60+
61+
var producers = <ResolvedCorrectionProducer>[];
62+
var thisContext = CorrectionProducerContext.createResolved(
63+
libraryResult: libraryResult,
64+
unitResult: unit,
65+
applyingBulkFixes: applyingBulkFixes,
66+
dartFixContext: context.dartFixContext,
67+
);
68+
69+
for (var uri in uris) {
70+
var directives =
71+
importDirectives
72+
.whereNot((directive) => directive.uri.stringValue == uri)
73+
.toList();
74+
producers.add(
75+
_ImportAddHide(name, uri, prefix, directives, context: thisContext),
76+
);
77+
producers.add(
78+
_ImportRemoveShow(name, uri, prefix, directives, context: thisContext),
79+
);
80+
}
81+
return producers;
82+
}
83+
84+
/// Returns [ImportDirective]s that import the given [conflictingElements]
85+
/// into [unitResult] and the set of uris (String) that represent each of the
86+
/// import directives.
87+
///
88+
/// The uris and the import directives are both returned so that we can
89+
/// run the fix for a certain uri on all of the other import directives.
90+
///
91+
/// The resulting [ResolvedUnitResult?] is the unit that contains the import
92+
/// directives. Usually this is the unit that contains the conflicting
93+
/// element, but it could be a parent unit if the conflicting element is
94+
/// a part file and the relevant imports are in an upstream file in the
95+
/// part hierarchy (enhanced parts).
96+
(ResolvedUnitResult?, List<ImportDirective>, List<String>)
97+
_getImportDirectives(
98+
ResolvedLibraryResult libraryResult,
99+
ResolvedUnitResult? unitResult,
100+
List<Element2> conflictingElements,
101+
String name,
102+
String? prefix,
103+
) {
104+
// The uris of all import directives that import the conflicting elements.
105+
var uris = <String>[];
106+
// The import directives that import the conflicting elements.
107+
var importDirectives = <ImportDirective>[];
108+
109+
// Search in each unit up the chain for related imports.
110+
while (unitResult is ResolvedUnitResult) {
111+
for (var conflictingElement in conflictingElements) {
112+
// Find all ImportDirective that import this library in this unit
113+
// and have the same prefix.
114+
for (var directive
115+
in unitResult.unit.directives.whereType<ImportDirective>()) {
116+
var libraryImport = directive.libraryImport;
117+
if (libraryImport == null) {
118+
continue;
119+
}
120+
121+
// If the prefix is different, then this directive is not relevant.
122+
if (directive.prefix?.name != prefix) {
123+
continue;
124+
}
125+
126+
// If this library is imported directly or if the directive exports
127+
// the library for this element.
128+
var element =
129+
prefix != null
130+
? libraryImport.namespace.getPrefixed2(prefix, name)
131+
: libraryImport.namespace.get2(name);
132+
if (element == conflictingElement) {
133+
var uri = directive.uri.stringValue;
134+
if (uri != null) {
135+
uris.add(uri);
136+
importDirectives.add(directive);
137+
}
138+
}
139+
}
140+
}
141+
142+
if (importDirectives.isNotEmpty) {
143+
break;
144+
}
145+
146+
// We continue up the chain.
147+
unitResult = libraryResult.parentUnitOf(unitResult);
148+
}
149+
150+
return (unitResult, importDirectives, uris);
151+
}
152+
}
153+
154+
class _ImportAddHide extends ResolvedCorrectionProducer {
155+
final List<ImportDirective> importDirectives;
156+
final String uri;
157+
final String? prefix;
158+
final String _elementName;
159+
160+
_ImportAddHide(
161+
this._elementName,
162+
this.uri,
163+
this.prefix,
164+
this.importDirectives, {
165+
required super.context,
166+
});
167+
168+
@override
169+
CorrectionApplicability get applicability =>
170+
// TODO(applicability): comment on why.
171+
CorrectionApplicability
172+
.singleLocation;
173+
174+
@override
175+
List<String> get fixArguments {
176+
var prefix = '';
177+
if (!this.prefix.isEmptyOrNull) {
178+
prefix = ' as ${this.prefix}';
179+
}
180+
return [_elementName, uri, prefix];
181+
}
182+
183+
@override
184+
FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE;
185+
186+
@override
187+
Future<void> compute(ChangeBuilder builder) async {
188+
if (_elementName.isEmpty || uri.isEmpty) {
189+
return;
190+
}
191+
192+
var hideCombinators =
193+
<({ImportDirective directive, List<HideCombinator> hideList})>[];
194+
195+
for (var directive in importDirectives) {
196+
var show = directive.combinators.whereType<ShowCombinator>().firstOrNull;
197+
// If there is an import with a show combinator, then we don't want to
198+
// deal with this case here.
199+
if (show != null) {
200+
return;
201+
}
202+
var hide = directive.combinators.whereType<HideCombinator>().toList();
203+
hideCombinators.add((directive: directive, hideList: hide));
204+
}
205+
206+
await builder.addDartFileEdit(file, (builder) {
207+
for (var (:directive, :hideList) in hideCombinators) {
208+
for (var hide in hideList) {
209+
var allNames = [
210+
...hide.hiddenNames.map((name) => name.name),
211+
_elementName,
212+
];
213+
if (_sortCombinators) {
214+
allNames.sort();
215+
}
216+
var combinator = 'hide ${allNames.join(', ')}';
217+
builder.addSimpleReplacement(range.node(hide), combinator);
218+
}
219+
if (hideList.isEmpty) {
220+
var hideCombinator = ' hide $_elementName';
221+
builder.addSimpleInsertion(directive.end - 1, hideCombinator);
222+
}
223+
}
224+
});
225+
}
226+
}
227+
228+
class _ImportRemoveShow extends ResolvedCorrectionProducer {
229+
final List<ImportDirective> importDirectives;
230+
final String _elementName;
231+
final String uri;
232+
final String? prefix;
233+
234+
_ImportRemoveShow(
235+
this._elementName,
236+
this.uri,
237+
this.prefix,
238+
this.importDirectives, {
239+
required super.context,
240+
});
241+
242+
@override
243+
CorrectionApplicability get applicability =>
244+
// TODO(applicability): comment on why.
245+
CorrectionApplicability
246+
.singleLocation;
247+
248+
@override
249+
List<String> get fixArguments {
250+
var prefix = '';
251+
if (!this.prefix.isEmptyOrNull) {
252+
prefix = ' as ${this.prefix}';
253+
}
254+
return [_elementName, uri, prefix];
255+
}
256+
257+
@override
258+
FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW;
259+
260+
@override
261+
Future<void> compute(ChangeBuilder builder) async {
262+
if (_elementName.isEmpty || uri.isEmpty) {
263+
return;
264+
}
265+
266+
var showCombinators =
267+
<
268+
({
269+
ImportDirective directive,
270+
List<ShowCombinator> showList,
271+
List<HideCombinator> hideList,
272+
})
273+
>[];
274+
275+
for (var directive in importDirectives) {
276+
var show = directive.combinators.whereType<ShowCombinator>().toList();
277+
var hide = directive.combinators.whereType<HideCombinator>().toList();
278+
// If there is no show combinator, then we don't want to deal with this
279+
// case here.
280+
if (show.isEmpty) {
281+
return;
282+
}
283+
showCombinators.add((
284+
directive: directive,
285+
showList: show,
286+
hideList: hide,
287+
));
288+
}
289+
290+
await builder.addDartFileEdit(file, (builder) {
291+
for (var (:directive, :showList, :hideList) in showCombinators) {
292+
var noShow = true;
293+
for (var show in showList) {
294+
var allNames = [
295+
...show.shownNames
296+
.map((name) => name.name)
297+
.where((name) => name != _elementName),
298+
];
299+
if (_sortCombinators) {
300+
allNames.sort();
301+
}
302+
if (allNames.isEmpty) {
303+
builder.addDeletion(SourceRange(show.offset - 1, show.length + 1));
304+
} else {
305+
noShow = false;
306+
var combinator = 'show ${allNames.join(', ')}';
307+
var range = SourceRange(show.offset, show.length);
308+
builder.addSimpleReplacement(range, combinator);
309+
}
310+
}
311+
if (noShow) {
312+
if (hideList.isEmpty) {
313+
var hideCombinator = ' hide $_elementName';
314+
builder.addSimpleInsertion(directive.end - 1, hideCombinator);
315+
}
316+
for (var hide in hideList) {
317+
var allNames = [
318+
...hide.hiddenNames.map((name) => name.name),
319+
_elementName,
320+
];
321+
if (_sortCombinators) {
322+
allNames.sort();
323+
}
324+
var combinator = 'hide ${allNames.join(', ')}';
325+
builder.addSimpleReplacement(range.node(hide), combinator);
326+
}
327+
}
328+
}
329+
});
330+
}
331+
}
332+
333+
extension on ResolvedCorrectionProducer {
334+
bool get _sortCombinators =>
335+
getCodeStyleOptions(unitResult.file).sortCombinators;
336+
}

‎pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,11 @@ CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE:
156156
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO:
157157
status: hasFix
158158
CompileTimeErrorCode.AMBIGUOUS_IMPORT:
159-
status: needsFix
159+
status: hasFix
160160
notes: |-
161-
1. For each imported name, add a fix to hide the name.
162-
2. For each imported name, add a fix to add a prefix. We wouldn't be able to
163-
add the prefix everywhere, but could add it wherever the name was already
164-
unambiguous.
161+
For each imported name, add a fix to add a prefix. We wouldn't be able to
162+
add the prefix everywhere, but could add it wherever the name was already
163+
unambiguous.
165164
CompileTimeErrorCode.AMBIGUOUS_SET_OR_MAP_LITERAL_BOTH:
166165
status: noFix
167166
notes: |-

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,11 @@ abstract final class DartFixKind {
834834
DartFixKindPriority.standard + 5,
835835
"Update library '{0}' import",
836836
);
837+
static const IMPORT_LIBRARY_HIDE = FixKind(
838+
'dart.fix.import.libraryHide',
839+
DartFixKindPriority.standard,
840+
"Hide others to use '{0}' from '{1}'{2}",
841+
);
837842
static const IMPORT_LIBRARY_PREFIX = FixKind(
838843
'dart.fix.import.libraryPrefix',
839844
DartFixKindPriority.standard + 5,
@@ -899,6 +904,11 @@ abstract final class DartFixKind {
899904
DartFixKindPriority.standard + 1,
900905
"Import library '{0}' with 'show'",
901906
);
907+
static const IMPORT_LIBRARY_REMOVE_SHOW = FixKind(
908+
'dart.fix.import.libraryRemoveShow',
909+
DartFixKindPriority.standard - 1,
910+
"Remove show to use '{0}' from '{1}'{2}",
911+
);
902912
static const IMPORT_LIBRARY_SDK = FixKind(
903913
'dart.fix.import.librarySdk',
904914
DartFixKindPriority.standard + 4,
@@ -909,16 +919,16 @@ abstract final class DartFixKind {
909919
DartFixKindPriority.standard + 4,
910920
"Import library '{0}' with prefix '{1}'",
911921
);
912-
static const IMPORT_LIBRARY_SDK_PREFIXED_SHOW = FixKind(
913-
'dart.fix.import.librarySdkPrefixedShow',
914-
DartFixKindPriority.standard + 4,
915-
"Import library '{0}' with prefix '{1}' and 'show'",
916-
);
917922
static const IMPORT_LIBRARY_SDK_SHOW = FixKind(
918923
'dart.fix.import.librarySdkShow',
919924
DartFixKindPriority.standard + 4,
920925
"Import library '{0}' with 'show'",
921926
);
927+
static const IMPORT_LIBRARY_SDK_PREFIXED_SHOW = FixKind(
928+
'dart.fix.import.librarySdkPrefixedShow',
929+
DartFixKindPriority.standard + 4,
930+
"Import library '{0}' with prefix '{1}' and 'show'",
931+
);
922932
static const INLINE_INVOCATION = FixKind(
923933
'dart.fix.inlineInvocation',
924934
DartFixKindPriority.standard - 20,

0 commit comments

Comments
 (0)