Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New assist to add/edit hide at import for ambiguous import #56762

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0f9bbd5
new assist to add/edit `hide` at import for ambiguous import
FMorschel Sep 20, 2024
11eed02
sorting file
FMorschel Oct 3, 2024
56ae18f
refactor to simplify user choice and automation
FMorschel Oct 10, 2024
67f93bb
readding the ambiguous import fix
FMorschel Oct 16, 2024
dddd163
better handling of different prefixes same imported library
FMorschel Oct 22, 2024
d899f28
makes two fixes - considers discussion
FMorschel Nov 1, 2024
10285ac
migrates to new element model and fixes tests
FMorschel Nov 14, 2024
a6b4f03
new assist to add/edit `hide` at import for ambiguous import
FMorschel Sep 20, 2024
0386796
sorting file
FMorschel Oct 3, 2024
bcd5d7f
refactor to simplify user choice and automation
FMorschel Oct 10, 2024
34fdb5d
readding the ambiguous import fix
FMorschel Oct 16, 2024
35adb2c
better handling of different prefixes same imported library
FMorschel Oct 22, 2024
2cbb312
makes two fixes - considers discussion
FMorschel Nov 1, 2024
1ab3585
refactors - review
FMorschel Nov 14, 2024
06dac3f
sorts test file
FMorschel Nov 14, 2024
06e1823
refactors to simplify the producers calculation - review
FMorschel Nov 14, 2024
19e133f
adds support for part files fixing
FMorschel Nov 19, 2024
ff05969
new multi level part tests
FMorschel Nov 22, 2024
5c4f733
expected result for failing test
FMorschel Nov 22, 2024
d3762fa
fixes previously failing test
FMorschel Nov 22, 2024
1fe9eda
Merge branch 'main' into fix-ambiguous-import
FMorschel Dec 2, 2024
c6f0975
Merge branch 'main' into fix-ambiguous-import
FMorschel Dec 2, 2024
d60e5f8
Merge branch 'main' into fix-ambiguous-import
FMorschel Dec 26, 2024
350976b
readded removed (by mistake) lines on merge
FMorschel Dec 26, 2024
22112c9
removes trailing white space
FMorschel Dec 26, 2024
2452a61
rename files
FMorschel Dec 26, 2024
f2291c1
docs and sorting of combinators considering lint
FMorschel Dec 26, 2024
2bd0a4a
sorting and formatting
FMorschel Dec 26, 2024
c24582c
Merge branch 'dart-lang:main' into fix-ambiguous-import
FMorschel Jan 5, 2025
bfa48dd
refactors - reviews
FMorschel Jan 5, 2025
8f1adaf
changes set->list and sorts files
FMorschel Jan 6, 2025
c93d33b
removes fix when multiple equal uri
FMorschel Jan 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,303 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:analysis_server/src/services/correction/fix.dart';
import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element2.dart';
import 'package:analyzer/source/source_range.dart';
import 'package:analyzer/src/dart/ast/extensions.dart';
import 'package:analyzer/src/utilities/extensions/results.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
import 'package:analyzer_plugin/utilities/range_factory.dart';
import 'package:collection/collection.dart';

class AmbiguousImportFix extends MultiCorrectionProducer {
AmbiguousImportFix({required super.context});

@override
Future<List<ResolvedCorrectionProducer>> get producers async {
var node = this.node;
Element2? element;
String? prefix;
if (node is NamedType) {
element = node.element2;
prefix = node.importPrefix?.name.lexeme;
} else if (node is SimpleIdentifier) {
element = node.element;
if (node.parent case PrefixedIdentifier(prefix: var currentPrefix)) {
prefix = currentPrefix.name;
}
}
if (element is! MultiplyDefinedElement2) {
return const [];
}
var conflictingElements = element.conflictingElements2;
var name = element.name3;
if (name == null || name.isEmpty) {
return const [];
}

var (unit, importDirectives, uris) = _getImportDirectives(
libraryResult,
unitResult,
conflictingElements,
prefix,
);

// If we have multiple imports of the same library, then we won't fix it.
if (uris.length != uris.toSet().length) {
return const [];
}

if (unit == null || importDirectives.isEmpty || uris.isEmpty) {
return const [];
}

var producers = <ResolvedCorrectionProducer>[];
var thisContext = CorrectionProducerContext.createResolved(
libraryResult: libraryResult,
unitResult: unit,
applyingBulkFixes: applyingBulkFixes,
dartFixContext: context.dartFixContext,
);

for (var uri in uris) {
var directives =
importDirectives
.whereNot((directive) => directive.uri.stringValue == uri)
.toList();
producers.add(
_ImportAddHide(name, uri, prefix, directives, context: thisContext),
);
producers.add(
_ImportRemoveShow(name, uri, prefix, directives, context: thisContext),
);
}
return producers;
}

/// Returns [ImportDirective]s that import the given [conflictingElements]
/// into [unitResult] and the set of uris (String) that represent each of the
/// import directives.
///
/// The uris and the import directives are both returned so that we can
/// run the fix for a certain uri on all of the other import directives.
///
/// The resulting [ResolvedUnitResult?] is the unit that contains the import
/// directives. Usually this is the unit that contains the conflicting
/// element, but it could be a parent unit if the conflicting element is
/// a part file and the relevant imports are in an upstream file in the
/// part hierarchy (enhanced parts).
(ResolvedUnitResult?, List<ImportDirective>, List<String>)
_getImportDirectives(
ResolvedLibraryResult libraryResult,
ResolvedUnitResult? unitResult,
List<Element2> conflictingElements,
String? prefix,
) {
// The uris of all import directives that import the conflicting elements.
var uris = <String>[];
// The import directives that import the conflicting elements.
var importDirectives = <ImportDirective>[];

var name = conflictingElements.firstOrNull?.name3;
if (name == null || name.isEmpty) {
return (null, importDirectives, uris);
}

// Search in each unit up the chain for related imports.
while (unitResult is ResolvedUnitResult) {
for (var conflictingElement in conflictingElements) {
// Find all ImportDirective that import this library in this unit
// and have the same prefix.
for (var directive
in unitResult.unit.directives.whereType<ImportDirective>()) {
var libraryImport = directive.libraryImport;
if (libraryImport == null) {
continue;
}

// If the prefix is different, then this directive is not relevant.
if (directive.prefix?.name != prefix) {
continue;
}

// If this library is imported directly or if the directive exports the
// library for this element.
if (libraryImport.namespace.get2(name) == conflictingElement) {
var uri = directive.uri.stringValue;
if (uri != null) {
uris.add(uri);
importDirectives.add(directive);
}
}
}
}

if (importDirectives.isNotEmpty) {
break;
}

// We continue up the chain.
unitResult = libraryResult.parentUnitOf(unitResult);
}

return (unitResult, importDirectives, uris);
}
}

class _ImportAddHide extends ResolvedCorrectionProducer {
final List<ImportDirective> importDirectives;
final String uri;
final String? prefix;
final String _elementName;

_ImportAddHide(
this._elementName,
this.uri,
this.prefix,
this.importDirectives, {
required super.context,
});

@override
CorrectionApplicability get applicability =>
// TODO(applicability): comment on why.
CorrectionApplicability.singleLocation;

@override
List<String> get fixArguments {
var prefix = '';
if (!this.prefix.isEmptyOrNull) {
prefix = ' as ${this.prefix}';
}
return [_elementName, uri, prefix];
}

@override
FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE;

@override
Future<void> compute(ChangeBuilder builder) async {
if (_elementName.isEmpty || uri.isEmpty) {
return;
}

var hideCombinators =
<({ImportDirective directive, HideCombinator? hide})>[];

for (var directive in importDirectives) {
var show = directive.combinators.whereType<ShowCombinator>().firstOrNull;
// If there is an import with a show combinator, then we don't want to
// deal with this case here.
if (show != null) {
return;
}
var hide = directive.combinators.whereType<HideCombinator>().firstOrNull;
hideCombinators.add((directive: directive, hide: hide));
}

await builder.addDartFileEdit(file, (builder) {
for (var (:directive, :hide) in hideCombinators) {
if (hide != null) {
var allNames = [
...hide.hiddenNames.map((name) => name.name),
_elementName,
];
if (_sortCombinators) {
allNames.sort();
}
// TODO(FMorschel): Use the utility function instead of ', '.
var combinator = 'hide ${allNames.join(', ')}';
builder.addSimpleReplacement(range.node(hide), combinator);
} else {
var hideCombinator = ' hide $_elementName';
builder.addSimpleInsertion(directive.end - 1, hideCombinator);
}
}
});
}
}

class _ImportRemoveShow extends ResolvedCorrectionProducer {
final List<ImportDirective> importDirectives;
final String _elementName;
final String uri;
final String? prefix;

_ImportRemoveShow(
this._elementName,
this.uri,
this.prefix,
this.importDirectives, {
required super.context,
});

@override
CorrectionApplicability get applicability =>
// TODO(applicability): comment on why.
CorrectionApplicability.singleLocation;

@override
List<String> get fixArguments {
var prefix = '';
if (!this.prefix.isEmptyOrNull) {
prefix = ' as ${this.prefix}';
}
return [_elementName, uri, prefix];
}

@override
FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW;

@override
Future<void> compute(ChangeBuilder builder) async {
if (_elementName.isEmpty || uri.isEmpty) {
return;
}

var showCombinators =
<({ImportDirective directive, ShowCombinator show})>[];
for (var directive in importDirectives) {
var show = directive.combinators.whereType<ShowCombinator>().firstOrNull;
// If there is no show combinator, then we don't want to deal with this
// case here.
if (show == null) {
return;
}
showCombinators.add((directive: directive, show: show));
}

await builder.addDartFileEdit(file, (builder) {
for (var (:directive, :show) in showCombinators) {
var allNames = [
...show.shownNames
.map((name) => name.name)
.where((name) => name != _elementName),
];
if (_sortCombinators) {
allNames.sort();
}
if (allNames.isEmpty) {
builder.addDeletion(SourceRange(show.offset - 1, show.length + 1));
var hideCombinator = ' hide $_elementName';
builder.addSimpleInsertion(directive.end - 1, hideCombinator);
} else {
// TODO(FMorschel): Use the utility function instead of ', '.
var combinator = 'show ${allNames.join(', ')}';
var range = SourceRange(show.offset, show.length);
builder.addSimpleReplacement(range, combinator);
}
}
});
}
}

extension on ResolvedCorrectionProducer {
bool get _sortCombinators =>
getCodeStyleOptions(unitResult.file).sortCombinators;
}
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,7 @@ class _ImportLibraryCombinator extends ResolvedCorrectionProducer {
}
var newCombinatorCode = '';
if (finalNames.isNotEmpty) {
// TODO(FMorschel): Use the utility function instead of ', '.
newCombinatorCode = ' ${keyword.lexeme} ${finalNames.join(', ')}';
}
var libraryPath = unitResult.libraryElement2.firstFragment.source.fullName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,11 @@ CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE:
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO:
status: hasFix
CompileTimeErrorCode.AMBIGUOUS_IMPORT:
status: needsFix
status: hasFix
notes: |-
1. For each imported name, add a fix to hide the name.
2. For each imported name, add a fix to add a prefix. We wouldn't be able to
add the prefix everywhere, but could add it wherever the name was already
unambiguous.
For each imported name, add a fix to add a prefix. We wouldn't be able to
add the prefix everywhere, but could add it wherever the name was already
unambiguous.
CompileTimeErrorCode.AMBIGUOUS_SET_OR_MAP_LITERAL_BOTH:
status: noFix
notes: |-
Expand Down
10 changes: 10 additions & 0 deletions pkg/analysis_server/lib/src/services/correction/fix.dart
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,11 @@ abstract final class DartFixKind {
DartFixKindPriority.standard + 5,
"Update library '{0}' import",
);
static const IMPORT_LIBRARY_HIDE = FixKind(
'dart.fix.import.libraryHide',
DartFixKindPriority.standard,
"Hide others to use '{0}' from '{1}'{2}",
);
static const IMPORT_LIBRARY_PREFIX = FixKind(
'dart.fix.import.libraryPrefix',
DartFixKindPriority.standard + 5,
Expand Down Expand Up @@ -894,6 +899,11 @@ abstract final class DartFixKind {
DartFixKindPriority.standard + 1,
"Import library '{0}' with 'show'",
);
static const IMPORT_LIBRARY_REMOVE_SHOW = FixKind(
'dart.fix.import.libraryRemoveShow',
DartFixKindPriority.standard - 1,
"Remove show to use '{0}' from '{1}'{2}",
);
static const IMPORT_LIBRARY_SDK = FixKind(
'dart.fix.import.librarySdk',
DartFixKindPriority.standard + 4,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import 'package:analysis_server/src/services/correction/dart/add_super_parameter
import 'package:analysis_server/src/services/correction/dart/add_switch_case_break.dart';
import 'package:analysis_server/src/services/correction/dart/add_trailing_comma.dart';
import 'package:analysis_server/src/services/correction/dart/add_type_annotation.dart';
import 'package:analysis_server/src/services/correction/dart/ambiguous_import_fix.dart';
import 'package:analysis_server/src/services/correction/dart/change_argument_name.dart';
import 'package:analysis_server/src/services/correction/dart/change_to.dart';
import 'package:analysis_server/src/services/correction/dart/change_to_nearest_precise_value.dart';
Expand Down Expand Up @@ -540,6 +541,7 @@ final _builtInNonLintMultiProducers = {
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE: [
AddExtensionOverride.new,
],
CompileTimeErrorCode.AMBIGUOUS_IMPORT: [AmbiguousImportFix.new],
CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [DataDriven.new],
CompileTimeErrorCode.CAST_TO_NON_TYPE: [
DataDriven.new,
Expand Down
Loading