From 0f9bbd538f94080cedae9afc1e9588573241e8d6 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:13:24 -0300 Subject: [PATCH 01/28] new assist to add/edit `hide` at import for ambiguous import --- .../correction/dart/import_add_hide.dart | 112 ++++++ .../services/correction/error_fix_status.yaml | 9 +- .../lib/src/services/correction/fix.dart | 5 + .../src/services/correction/fix_internal.dart | 4 + .../correction/fix/import_add_hide_test.dart | 326 ++++++++++++++++++ .../src/services/correction/fix/test_all.dart | 2 + 6 files changed, 453 insertions(+), 5 deletions(-) create mode 100644 pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart create mode 100644 pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart new file mode 100644 index 000000000000..58c3584c0297 --- /dev/null +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -0,0 +1,112 @@ +// 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/ast/ast.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/source/source_range.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; + +class ImportAddHide extends MultiCorrectionProducer { + ImportAddHide({required super.context}); + + @override + Future> get producers async { + var node = this.node; + Element? element; + if (node is NamedType) { + element = node.element; + } else if (node is SimpleIdentifier) { + element = node.staticElement; + } + if (element is! MultiplyDefinedElement) { + return const []; + } + var elements = element.conflictingElements; + var pairs = {}; + for (var element in elements) { + var library = element.enclosingElement3?.library; + // find all ImportDirective that import this library in this unit + for (var directive in unit.directives.whereType()) { + var imported = directive.element?.importedLibrary; + if (imported == null) { + continue; + } + if (imported == library) { + pairs[directive] = element; + } + // If the directive exports the library, then the library is also + // imported. + if (imported.exportedLibraries.contains(library)) { + pairs[directive] = element; + } + } + } + return [ + for (var MapEntry(key: import, value: element) in pairs.entries) + _ImportAddHide(import, element, context: context), + ]; + } +} + +class _ImportAddHide extends ResolvedCorrectionProducer { + _ImportAddHide(this.importDirective, this.element, {required super.context}); + + final ImportDirective importDirective; + final Element element; + + @override + CorrectionApplicability get applicability => + // TODO(applicability): comment on why. + CorrectionApplicability.singleLocation; + + @override + FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; + + @override + List get fixArguments { + var aliasStr = importDirective.prefix?.name; + var alias = ''; + if (aliasStr != null) { + alias = " as '$aliasStr'"; + } + return [elementName, importStr, alias]; + } + + String get importStr => importDirective.uri.stringValue ?? ''; + String get elementName => element.name ?? ''; + + @override + Future compute(ChangeBuilder builder) async { + if (elementName.isEmpty || importStr.isEmpty) { + return; + } + + if (importDirective.combinators.whereType().isNotEmpty) { + return; + } + + var hide = importDirective.combinators.whereType(); + if (hide.isNotEmpty) { + return await builder.addDartFileEdit(file, (builder) { + var hideCombinator = hide.first; + var allNames = [elementName]; + for (var name in hideCombinator.hiddenNames) { + allNames.add(name.name); + } + allNames.sort(); + var combinator = 'hide ${allNames.join(', ')}'; + var range = SourceRange(hideCombinator.offset, hideCombinator.length); + builder.addSimpleReplacement(range, combinator); + }); + } + + await builder.addDartFileEdit(file, (builder) { + var hideCombinator = ' hide $elementName'; + builder.addSimpleInsertion(importDirective.end - 1, hideCombinator); + }); + } +} diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml index 269a45fb4f79..21081ad63c7a 100644 --- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml +++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml @@ -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: |- diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index 845b93b4a408..77b0c1526633 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -874,6 +874,11 @@ abstract final class DartFixKind { DartFixKindPriority.standard + 4, "Import library '{0}' with prefix '{1}'", ); + static const IMPORT_LIBRARY_HIDE = FixKind( + 'dart.fix.import.libraryHide', + DartFixKindPriority.standard, + "Add 'hide {0}' to library '{1}'{2} import", + ); static const INLINE_INVOCATION = FixKind( 'dart.fix.inlineInvocation', DartFixKindPriority.standard - 20, diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 07e97880e59a..6bcb68170762 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -101,6 +101,7 @@ import 'package:analysis_server/src/services/correction/dart/extend_class_for_mi import 'package:analysis_server/src/services/correction/dart/extract_local_variable.dart'; import 'package:analysis_server/src/services/correction/dart/flutter_remove_widget.dart'; import 'package:analysis_server/src/services/correction/dart/ignore_diagnostic.dart'; +import 'package:analysis_server/src/services/correction/dart/import_add_hide.dart'; import 'package:analysis_server/src/services/correction/dart/import_library.dart'; import 'package:analysis_server/src/services/correction/dart/inline_invocation.dart'; import 'package:analysis_server/src/services/correction/dart/inline_typedef.dart'; @@ -530,6 +531,9 @@ final _builtInLintProducers = >{ }; final _builtInNonLintMultiProducers = { + CompileTimeErrorCode.AMBIGUOUS_IMPORT: [ + ImportAddHide.new, + ], CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO: [ AddExtensionOverride.new, ], diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart new file mode 100644 index 000000000000..5361a887c02a --- /dev/null +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -0,0 +1,326 @@ +// Copyright (c) 2018, 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:analyzer/src/error/codes.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import 'fix_processor.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(ImportAddHideTest); + }); +} + +@reflectiveTest +class ImportAddHideTest extends FixProcessorTest { + @override + FixKind get kind => DartFixKind.IMPORT_LIBRARY_HIDE; + + Future test_doubleAliasedImports() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' as i; +import 'lib2.dart' as i; + +void f(i.N? n) { + print(n); +} +'''); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Add 'hide N' to library 'lib1.dart' as 'i' import", + "Add 'hide N' to library 'lib2.dart' as 'i' import", + ]); + await assertHasFix(''' +import 'lib1.dart' as i hide N; +import 'lib2.dart' as i; + +void f(i.N? n) { + print(n); +} +''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' as 'i' import"); + await assertHasFix(''' +import 'lib1.dart' as i; +import 'lib2.dart' as i hide N; + +void f(i.N? n) { + print(n); +} +''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' as 'i' import"); + } + + Future test_DoubleImports() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib2.dart'; + +void f(N? n) { + print(n); +} +'''); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Add 'hide N' to library 'lib1.dart' import", + "Add 'hide N' to library 'lib2.dart' import", + ]); + await assertHasFix(''' +import 'lib1.dart' hide N; +import 'lib2.dart'; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' import"); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib2.dart' hide N; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' import"); + } + + Future test_doubleImports_bothShow() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' show N; +import 'lib2.dart' show N; + +void f(N? n) { + print(n); +} +'''); + await assertNoFix(); + } + + Future test_doubleImports_constant() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' show foo; +import 'lib2.dart'; + +void f() { + print(foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart' show foo; +import 'lib2.dart' hide foo; + +void f() { + print(foo); +} +''', matchFixMessage: "Add 'hide foo' to library 'lib2.dart' import"); + } + + Future test_doubleImports_exportedByImport() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +export 'lib3.dart';'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +mixin M {}'''); + newFile('$testPackageLibPath/lib3.dart', ''' +library lib3; +mixin M {}'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib2.dart' show M; + +class C with M {} +'''); + await assertHasFix(''' +import 'lib1.dart' hide M; +import 'lib2.dart' show M; + +class C with M {} +''', matchFixMessage: "Add 'hide M' to library 'lib1.dart' import", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); + } + + Future test_doubleImports_extension() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +extension E on int { + bool get isDivisibleByThree => this % 3 == 0; +}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +extension E on int { + bool get isDivisibleByThree => this % 3 == 0; +}'''); + await resolveTestCode(''' +import 'lib1.dart' show E; +import 'lib2.dart'; + +void foo(int i) { + print(E(i.isDivisibleByThree)); +} +'''); + await assertHasFix(''' +import 'lib1.dart' show E; +import 'lib2.dart' hide E; + +void foo(int i) { + print(E(i.isDivisibleByThree)); +} +''', matchFixMessage: "Add 'hide E' to library 'lib2.dart' import", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); + } + + Future test_doubleImports_function() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +void bar() {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +void bar() {}'''); + await resolveTestCode(''' +import 'lib1.dart' show bar; +import 'lib2.dart'; + +void foo() { + bar(); +} +'''); + await assertHasFix(''' +import 'lib1.dart' show bar; +import 'lib2.dart' hide bar; + +void foo() { + bar(); +} +''', matchFixMessage: "Add 'hide bar' to library 'lib2.dart' import"); + } + + Future test_doubleImports_mixin() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +mixin M {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +mixin M {}'''); + await resolveTestCode(''' +import 'lib1.dart' show M; +import 'lib2.dart'; + +class C with M {} +'''); + await assertHasFix(''' +import 'lib1.dart' show M; +import 'lib2.dart' hide M; + +class C with M {} +''', matchFixMessage: "Add 'hide M' to library 'lib2.dart' import", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); + } + + Future test_doubleImports_oneHide() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +class M {} class N {} class O {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' hide M, O; +import 'lib2.dart' show N; + +void f(N? n) { + print(n); +} +'''); + await assertHasFix(''' +import 'lib1.dart' hide M, N, O; +import 'lib2.dart' show N; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' import"); + } + + Future test_doubleImports_oneShow() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' show N; +import 'lib2.dart'; + +void f(N? n) { + print(n); +} +'''); + await assertHasFix(''' +import 'lib1.dart' show N; +import 'lib2.dart' hide N; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' import"); + } + + Future test_doubleImports_variable() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +var foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +var foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' show foo; +import 'lib2.dart'; + +void f() { + print(foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart' show foo; +import 'lib2.dart' hide foo; + +void f() { + print(foo); +} +''', matchFixMessage: "Add 'hide foo' to library 'lib2.dart' import"); + } +} diff --git a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart index ac465354a9ba..6465d938bb88 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart @@ -132,6 +132,7 @@ import 'fix_processor_map_test.dart' as fix_processor_map; import 'fix_test.dart' as fix; import 'format_file_test.dart' as format_file; import 'ignore_diagnostic_test.dart' as ignore_error; +import 'import_add_hide_test.dart' as import_add_hide; import 'import_library_hide_test.dart' as import_library_hide; import 'import_library_prefix_test.dart' as import_library_prefix; import 'import_library_project_test.dart' as import_library_project; @@ -416,6 +417,7 @@ void main() { fix_processor_map.main(); format_file.main(); ignore_error.main(); + import_add_hide.main(); import_library_hide.main(); import_library_prefix.main(); import_library_project.main(); From 11eed02d603f98a4af77dc7861a4b6c9e72133fa Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 3 Oct 2024 09:01:49 -0300 Subject: [PATCH 02/28] sorting file --- .../correction/dart/import_add_hide.dart | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 58c3584c0297..f74a5e4e84ba 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -53,19 +53,16 @@ class ImportAddHide extends MultiCorrectionProducer { } class _ImportAddHide extends ResolvedCorrectionProducer { - _ImportAddHide(this.importDirective, this.element, {required super.context}); - final ImportDirective importDirective; final Element element; + _ImportAddHide(this.importDirective, this.element, {required super.context}); + @override CorrectionApplicability get applicability => // TODO(applicability): comment on why. CorrectionApplicability.singleLocation; - @override - FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; - @override List get fixArguments { var aliasStr = importDirective.prefix?.name; @@ -73,15 +70,18 @@ class _ImportAddHide extends ResolvedCorrectionProducer { if (aliasStr != null) { alias = " as '$aliasStr'"; } - return [elementName, importStr, alias]; + return [_elementName, _importStr, alias]; } - String get importStr => importDirective.uri.stringValue ?? ''; - String get elementName => element.name ?? ''; + @override + FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; + + String get _elementName => element.name ?? ''; + String get _importStr => importDirective.uri.stringValue ?? ''; @override Future compute(ChangeBuilder builder) async { - if (elementName.isEmpty || importStr.isEmpty) { + if (_elementName.isEmpty || _importStr.isEmpty) { return; } @@ -93,7 +93,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { if (hide.isNotEmpty) { return await builder.addDartFileEdit(file, (builder) { var hideCombinator = hide.first; - var allNames = [elementName]; + var allNames = [_elementName]; for (var name in hideCombinator.hiddenNames) { allNames.add(name.name); } @@ -105,7 +105,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { } await builder.addDartFileEdit(file, (builder) { - var hideCombinator = ' hide $elementName'; + var hideCombinator = ' hide $_elementName'; builder.addSimpleInsertion(importDirective.end - 1, hideCombinator); }); } From 56ae18f62551e46be9c59a910586f74bf9eeb0f9 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 10 Oct 2024 15:46:27 -0300 Subject: [PATCH 03/28] refactor to simplify user choice and automation - Removed verbose process for user. - Refactored to allow user to specify desired elements explicitly. - The fix now automatically hide everything else --- .../correction/dart/import_add_hide.dart | 130 ++++++++---- .../lib/src/services/correction/fix.dart | 2 +- .../correction/fix/import_add_hide_test.dart | 193 ++++++++++++------ 3 files changed, 229 insertions(+), 96 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index f74a5e4e84ba..38bacd7a3092 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -9,6 +9,7 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:collection/collection.dart'; class ImportAddHide extends MultiCorrectionProducer { ImportAddHide({required super.context}); @@ -26,37 +27,66 @@ class ImportAddHide extends MultiCorrectionProducer { return const []; } var elements = element.conflictingElements; - var pairs = {}; + var records = <({Element element, String uri, String? prefix}), + List>{}; for (var element in elements) { var library = element.enclosingElement3?.library; + if (library == null) { + continue; + } + + var directives = []; // find all ImportDirective that import this library in this unit for (var directive in unit.directives.whereType()) { + // Get import directive that var imported = directive.element?.importedLibrary; if (imported == null) { continue; } if (imported == library) { - pairs[directive] = element; + directives.add(directive); } // If the directive exports the library, then the library is also // imported. if (imported.exportedLibraries.contains(library)) { - pairs[directive] = element; + directives.add(directive); + } + } + for (var directive in directives) { + var uri = directive.uri.stringValue; + var prefix = directive.prefix?.name; + if (uri != null) { + records[(element: element, uri: uri, prefix: prefix)] = directives; } } } - return [ - for (var MapEntry(key: import, value: element) in pairs.entries) - _ImportAddHide(import, element, context: context), - ]; + if (records.entries.isEmpty) { + return const []; + } + + var producers = []; + for (var MapEntry(key: key) in records.entries) { + var directives = records.entries + .whereNot((e) => e.key == key) + .expand((e) => e.value) + .whereNot((d) => + (d.prefix?.name == key.prefix) && (d.uri.stringValue == key.uri)) + .toSet(); + producers.add(_ImportAddHide(key.element, key.uri, key.prefix, directives, + context: context)); + } + return producers; } } class _ImportAddHide extends ResolvedCorrectionProducer { - final ImportDirective importDirective; + final Set importDirectives; final Element element; + final String uri; + final String? prefix; - _ImportAddHide(this.importDirective, this.element, {required super.context}); + _ImportAddHide(this.element, this.uri, this.prefix, this.importDirectives, + {required super.context}); @override CorrectionApplicability get applicability => @@ -65,48 +95,76 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override List get fixArguments { - var aliasStr = importDirective.prefix?.name; - var alias = ''; - if (aliasStr != null) { - alias = " as '$aliasStr'"; + // Any import directive that imports the element is directly showing it? + var removeShow = false; + for (var directives in importDirectives + .map((d) => d.combinators.whereType()) + .where((d) => d.isNotEmpty)) { + for (var name in directives.first.shownNames) { + if (name.name == _elementName) { + removeShow = true; + break; + } + } } - return [_elementName, _importStr, alias]; + // If there is a show combinator explicitly showing the element, then + // we should remove it. + var removeShowStr = removeShow ? ' (removing \'show\')' : ''; + var prefix = ''; + if ((this.prefix ?? '') != '') { + prefix = ' as ${this.prefix}'; + } + return [_elementName, uri, prefix, removeShowStr]; } @override FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; String get _elementName => element.name ?? ''; - String get _importStr => importDirective.uri.stringValue ?? ''; @override Future compute(ChangeBuilder builder) async { - if (_elementName.isEmpty || _importStr.isEmpty) { - return; - } - - if (importDirective.combinators.whereType().isNotEmpty) { + if (_elementName.isEmpty || uri.isEmpty) { return; } - var hide = importDirective.combinators.whereType(); - if (hide.isNotEmpty) { - return await builder.addDartFileEdit(file, (builder) { - var hideCombinator = hide.first; - var allNames = [_elementName]; - for (var name in hideCombinator.hiddenNames) { - allNames.add(name.name); + for (var directive in importDirectives) { + var show = directive.combinators.whereType(); + var hide = directive.combinators.whereType(); + await builder.addDartFileEdit(file, (builder) { + if (show.isNotEmpty) { + var showCombinator = show.first; + var allNames = []; + for (var show in showCombinator.shownNames) { + if (show.name == _elementName) continue; + allNames.add(show.name); + } + allNames.sort(); + if (allNames.isEmpty) { + builder.addDeletion(SourceRange( + showCombinator.offset - 1, showCombinator.length + 1)); + } else { + var combinator = 'show ${allNames.join(', ')}'; + var range = + SourceRange(showCombinator.offset, showCombinator.length); + builder.addSimpleReplacement(range, combinator); + } + } + if (hide.isNotEmpty) { + var hideCombinator = hide.first; + var allNames = [_elementName]; + for (var name in hideCombinator.hiddenNames) { + allNames.add(name.name); + } + allNames.sort(); + var combinator = 'hide ${allNames.join(', ')}'; + var range = SourceRange(hideCombinator.offset, hideCombinator.length); + builder.addSimpleReplacement(range, combinator); + } else { + var hideCombinator = ' hide $_elementName'; + builder.addSimpleInsertion(directive.end - 1, hideCombinator); } - allNames.sort(); - var combinator = 'hide ${allNames.join(', ')}'; - var range = SourceRange(hideCombinator.offset, hideCombinator.length); - builder.addSimpleReplacement(range, combinator); }); } - - await builder.addDartFileEdit(file, (builder) { - var hideCombinator = ' hide $_elementName'; - builder.addSimpleInsertion(importDirective.end - 1, hideCombinator); - }); } } diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index 77b0c1526633..f15cdeec491c 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -877,7 +877,7 @@ abstract final class DartFixKind { static const IMPORT_LIBRARY_HIDE = FixKind( 'dart.fix.import.libraryHide', DartFixKindPriority.standard, - "Add 'hide {0}' to library '{1}'{2} import", + "Use '{0}' from '{1}'{2}{3}", ); static const INLINE_INVOCATION = FixKind( 'dart.fix.inlineInvocation', diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 5361a887c02a..9a58122bbc92 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -22,10 +22,8 @@ class ImportAddHideTest extends FixProcessorTest { Future test_doubleAliasedImports() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' as i; @@ -38,33 +36,31 @@ void f(i.N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Add 'hide N' to library 'lib1.dart' as 'i' import", - "Add 'hide N' to library 'lib2.dart' as 'i' import", + "Use 'N' from 'lib1.dart' as i", + "Use 'N' from 'lib2.dart' as i", ]); await assertHasFix(''' -import 'lib1.dart' as i hide N; -import 'lib2.dart' as i; +import 'lib1.dart' as i; +import 'lib2.dart' as i hide N; void f(i.N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' as 'i' import"); +''', matchFixMessage: "Use 'N' from 'lib1.dart' as i"); await assertHasFix(''' -import 'lib1.dart' as i; -import 'lib2.dart' as i hide N; +import 'lib1.dart' as i hide N; +import 'lib2.dart' as i; void f(i.N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' as 'i' import"); +''', matchFixMessage: "Use 'N' from 'lib2.dart' as i"); } Future test_DoubleImports() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -77,8 +73,8 @@ void f(N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Add 'hide N' to library 'lib1.dart' import", - "Add 'hide N' to library 'lib2.dart' import", + "Use 'N' from 'lib1.dart'", + "Use 'N' from 'lib2.dart'", ]); await assertHasFix(''' import 'lib1.dart' hide N; @@ -87,7 +83,7 @@ import 'lib2.dart'; void f(N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' import"); +''', matchFixMessage: "Use 'N' from 'lib2.dart'"); await assertHasFix(''' import 'lib1.dart'; import 'lib2.dart' hide N; @@ -95,15 +91,13 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'N' from 'lib1.dart'"); } Future test_doubleImports_bothShow() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N; @@ -113,18 +107,37 @@ void f(N? n) { print(n); } '''); - await assertNoFix(); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Use 'N' from 'lib1.dart' (removing 'show')", + "Use 'N' from 'lib2.dart' (removing 'show')", + ]); + await assertHasFix(''' +import 'lib1.dart' hide N; +import 'lib2.dart' show N; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Use 'N' from 'lib2.dart' (removing 'show')"); + await assertHasFix(''' +import 'lib1.dart' show N; +import 'lib2.dart' hide N; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Use 'N' from 'lib1.dart' (removing 'show')"); } Future test_doubleImports_constant() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; const foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; const foo = 0;'''); await resolveTestCode(''' -import 'lib1.dart' show foo; +import 'lib1.dart'; import 'lib2.dart'; void f() { @@ -132,55 +145,49 @@ void f() { } '''); await assertHasFix(''' -import 'lib1.dart' show foo; +import 'lib1.dart'; import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Add 'hide foo' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); } Future test_doubleImports_exportedByImport() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; export 'lib3.dart';'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; mixin M {}'''); newFile('$testPackageLibPath/lib3.dart', ''' -library lib3; mixin M {}'''); await resolveTestCode(''' import 'lib1.dart'; -import 'lib2.dart' show M; +import 'lib2.dart'; class C with M {} '''); await assertHasFix(''' import 'lib1.dart' hide M; -import 'lib2.dart' show M; +import 'lib2.dart'; class C with M {} -''', matchFixMessage: "Add 'hide M' to library 'lib1.dart' import", - errorFilter: (error) { +''', matchFixMessage: "Use 'M' from 'lib2.dart'", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } Future test_doubleImports_extension() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); await resolveTestCode(''' -import 'lib1.dart' show E; +import 'lib1.dart'; import 'lib2.dart'; void foo(int i) { @@ -188,27 +195,24 @@ void foo(int i) { } '''); await assertHasFix(''' -import 'lib1.dart' show E; +import 'lib1.dart'; import 'lib2.dart' hide E; void foo(int i) { print(E(i.isDivisibleByThree)); } -''', matchFixMessage: "Add 'hide E' to library 'lib2.dart' import", - errorFilter: (error) { +''', matchFixMessage: "Use 'E' from 'lib1.dart'", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } Future test_doubleImports_function() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; void bar() {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; void bar() {}'''); await resolveTestCode(''' -import 'lib1.dart' show bar; +import 'lib1.dart'; import 'lib2.dart'; void foo() { @@ -216,49 +220,44 @@ void foo() { } '''); await assertHasFix(''' -import 'lib1.dart' show bar; +import 'lib1.dart'; import 'lib2.dart' hide bar; void foo() { bar(); } -''', matchFixMessage: "Add 'hide bar' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'bar' from 'lib1.dart'"); } Future test_doubleImports_mixin() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; mixin M {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; mixin M {}'''); await resolveTestCode(''' -import 'lib1.dart' show M; +import 'lib1.dart'; import 'lib2.dart'; class C with M {} '''); await assertHasFix(''' -import 'lib1.dart' show M; +import 'lib1.dart'; import 'lib2.dart' hide M; class C with M {} -''', matchFixMessage: "Add 'hide M' to library 'lib2.dart' import", - errorFilter: (error) { +''', matchFixMessage: "Use 'M' from 'lib1.dart'", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } Future test_doubleImports_oneHide() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class M {} class N {} class O {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' hide M, O; -import 'lib2.dart' show N; +import 'lib2.dart'; void f(N? n) { print(n); @@ -266,20 +265,18 @@ void f(N? n) { '''); await assertHasFix(''' import 'lib1.dart' hide M, N, O; -import 'lib2.dart' show N; +import 'lib2.dart'; void f(N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' import"); +''', matchFixMessage: "Use 'N' from 'lib2.dart'"); } Future test_doubleImports_oneShow() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N; @@ -296,15 +293,13 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'N' from 'lib1.dart'"); } Future test_doubleImports_variable() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; var foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; var foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' show foo; @@ -321,6 +316,86 @@ import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Add 'hide foo' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); + } + + Future test_show_prefixed() async { + newFile('$testPackageLibPath/lib1.dart', ''' +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +'''); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Use 'N' from 'lib1.dart' as l (removing 'show')", + "Use 'N' from 'lib2.dart' as l (removing 'show')", + ]); + await assertHasFix(''' +import 'lib1.dart' as l hide N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Use 'N' from 'lib2.dart' as l (removing 'show')"); + await assertHasFix(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l hide N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Use 'N' from 'lib1.dart' as l (removing 'show')"); + } + + Future test_tripleImports() async { + newFile('$testPackageLibPath/lib1.dart', ''' +export 'lib3.dart';'''); + newFile('$testPackageLibPath/lib2.dart', ''' +mixin M {}'''); + newFile('$testPackageLibPath/lib3.dart', ''' +mixin M {}'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib2.dart'; +import 'lib3.dart'; + +class C with M {} +'''); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib2.dart' hide M; +import 'lib3.dart' hide M; + +class C with M {} +''', matchFixMessage: "Use 'M' from 'lib1.dart'", errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); + await assertHasFix(''' +import 'lib1.dart' hide M; +import 'lib2.dart'; +import 'lib3.dart' hide M; + +class C with M {} +''', matchFixMessage: "Use 'M' from 'lib2.dart'", errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); + await assertHasFix(''' +import 'lib1.dart' hide M; +import 'lib2.dart' hide M; +import 'lib3.dart'; + +class C with M {} +''', matchFixMessage: "Use 'M' from 'lib3.dart'", errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); } } From 67f93bbac359b9bb459537254028827ec8a4933d Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Wed, 16 Oct 2024 10:20:54 -0300 Subject: [PATCH 04/28] readding the ambiguous import fix --- .../lib/src/services/correction/fix_internal.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 6bcb68170762..98d639082fc6 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -531,15 +531,15 @@ final _builtInLintProducers = >{ }; final _builtInNonLintMultiProducers = { - CompileTimeErrorCode.AMBIGUOUS_IMPORT: [ - ImportAddHide.new, - ], CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO: [ AddExtensionOverride.new, ], CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE: [ AddExtensionOverride.new, ], + CompileTimeErrorCode.AMBIGUOUS_IMPORT: [ + ImportAddHide.new, + ], CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [DataDriven.new], CompileTimeErrorCode.CAST_TO_NON_TYPE: [ DataDriven.new, From dddd1639a169d26d64e2c9f41aa76cf0766c5990 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Tue, 22 Oct 2024 10:37:24 -0300 Subject: [PATCH 05/28] better handling of different prefixes same imported library --- .../correction/dart/import_add_hide.dart | 11 +++- .../correction/fix/import_add_hide_test.dart | 50 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 38bacd7a3092..6455ccae5129 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -18,10 +18,15 @@ class ImportAddHide extends MultiCorrectionProducer { Future> get producers async { var node = this.node; Element? element; + String? prefix; if (node is NamedType) { element = node.element; + prefix = node.importPrefix?.name.lexeme; } else if (node is SimpleIdentifier) { element = node.staticElement; + if (node.parent case PrefixedIdentifier(prefix: var currentPrefix)) { + prefix = currentPrefix.name; + } } if (element is! MultiplyDefinedElement) { return const []; @@ -37,18 +42,20 @@ class ImportAddHide extends MultiCorrectionProducer { var directives = []; // find all ImportDirective that import this library in this unit + // and have the same prefix for (var directive in unit.directives.whereType()) { // Get import directive that var imported = directive.element?.importedLibrary; if (imported == null) { continue; } - if (imported == library) { + if (imported == library && directive.prefix?.name == prefix) { directives.add(directive); } // If the directive exports the library, then the library is also // imported. - if (imported.exportedLibraries.contains(library)) { + if (imported.exportedLibraries.contains(library) && + directive.prefix?.name == prefix) { directives.add(directive); } } diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 9a58122bbc92..808e37df2053 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -154,6 +154,56 @@ void f() { ''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); } + Future test_tripleImports_oneAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' as lib; +import 'lib1.dart'; +import 'lib2.dart'; + +void f() { + print(foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart' as lib; +import 'lib1.dart'; +import 'lib2.dart' hide foo; + +void f() { + print(foo); +} +''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); + } + + Future test_tripleImports_twoAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib1.dart' as lib; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib1.dart' as lib; +import 'lib2.dart' as lib hide foo; + +void f() { + print(lib.foo); +} +''', matchFixMessage: "Use 'foo' from 'lib1.dart' as lib"); + } + Future test_doubleImports_exportedByImport() async { newFile('$testPackageLibPath/lib1.dart', ''' export 'lib3.dart';'''); From d899f28cbea3dab583bbd80eace977209f45ffe6 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Fri, 1 Nov 2024 18:01:39 -0300 Subject: [PATCH 06/28] makes two fixes - considers discussion --- .../correction/dart/import_add_hide.dart | 136 ++++-- .../lib/src/services/correction/fix.dart | 15 +- .../src/services/correction/fix_internal.dart | 2 +- .../correction/fix/import_add_hide_test.dart | 388 +++++++++++------- 4 files changed, 351 insertions(+), 190 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 6455ccae5129..50ab58d2c0b8 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -11,8 +11,8 @@ import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dar import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; import 'package:collection/collection.dart'; -class ImportAddHide extends MultiCorrectionProducer { - ImportAddHide({required super.context}); +class AmbiguousImportFix extends MultiCorrectionProducer { + AmbiguousImportFix({required super.context}); @override Future> get producers async { @@ -81,6 +81,9 @@ class ImportAddHide extends MultiCorrectionProducer { .toSet(); producers.add(_ImportAddHide(key.element, key.uri, key.prefix, directives, context: context)); + producers.add(_ImportRemoveShow( + key.element, key.uri, key.prefix, directives, + context: context)); } return producers; } @@ -102,26 +105,11 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override List get fixArguments { - // Any import directive that imports the element is directly showing it? - var removeShow = false; - for (var directives in importDirectives - .map((d) => d.combinators.whereType()) - .where((d) => d.isNotEmpty)) { - for (var name in directives.first.shownNames) { - if (name.name == _elementName) { - removeShow = true; - break; - } - } - } - // If there is a show combinator explicitly showing the element, then - // we should remove it. - var removeShowStr = removeShow ? ' (removing \'show\')' : ''; var prefix = ''; if ((this.prefix ?? '') != '') { prefix = ' as ${this.prefix}'; } - return [_elementName, uri, prefix, removeShowStr]; + return [_elementName, uri, prefix]; } @override @@ -135,43 +123,103 @@ class _ImportAddHide extends ResolvedCorrectionProducer { return; } + var hideCombinator = + <({ImportDirective directive, HideCombinator? hide})>[]; + for (var directive in importDirectives) { - var show = directive.combinators.whereType(); - var hide = directive.combinators.whereType(); - await builder.addDartFileEdit(file, (builder) { - if (show.isNotEmpty) { - var showCombinator = show.first; - var allNames = []; - for (var show in showCombinator.shownNames) { - if (show.name == _elementName) continue; - allNames.add(show.name); - } - allNames.sort(); - if (allNames.isEmpty) { - builder.addDeletion(SourceRange( - showCombinator.offset - 1, showCombinator.length + 1)); - } else { - var combinator = 'show ${allNames.join(', ')}'; - var range = - SourceRange(showCombinator.offset, showCombinator.length); - builder.addSimpleReplacement(range, combinator); - } - } - if (hide.isNotEmpty) { - var hideCombinator = hide.first; + var show = directive.combinators.whereType().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().firstOrNull; + hideCombinator.add((directive: directive, hide: hide)); + } + + await builder.addDartFileEdit(file, (builder) { + for (var (:directive, :hide) in hideCombinator) { + if (hide != null) { var allNames = [_elementName]; - for (var name in hideCombinator.hiddenNames) { + for (var name in hide.hiddenNames) { allNames.add(name.name); } allNames.sort(); var combinator = 'hide ${allNames.join(', ')}'; - var range = SourceRange(hideCombinator.offset, hideCombinator.length); + var range = SourceRange(hide.offset, hide.length); builder.addSimpleReplacement(range, combinator); } else { var hideCombinator = ' hide $_elementName'; builder.addSimpleInsertion(directive.end - 1, hideCombinator); } - }); + } + }); + } +} + +class _ImportRemoveShow extends ResolvedCorrectionProducer { + final Set importDirectives; + final Element element; + final String uri; + final String? prefix; + + _ImportRemoveShow(this.element, this.uri, this.prefix, this.importDirectives, + {required super.context}); + + @override + CorrectionApplicability get applicability => + // TODO(applicability): comment on why. + CorrectionApplicability.singleLocation; + + @override + List get fixArguments { + var prefix = ''; + if ((this.prefix ?? '') != '') { + prefix = ' as ${this.prefix}'; + } + return [_elementName, uri, prefix]; + } + + @override + FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; + + String get _elementName => element.name ?? ''; + + @override + Future compute(ChangeBuilder builder) async { + if (_elementName.isEmpty || uri.isEmpty) { + return; } + + var showCombinator = <({ImportDirective directive, ShowCombinator show})>[]; + for (var directive in importDirectives) { + var show = directive.combinators.whereType().firstOrNull; + // If there is no show combinator, then we don't want to deal with this + // case here. + if (show == null) { + return; + } + showCombinator.add((directive: directive, show: show)); + } + + await builder.addDartFileEdit(file, (builder) { + for (var (:directive, :show) in showCombinator) { + var allNames = []; + for (var show in show.shownNames) { + if (show.name == _elementName) continue; + allNames.add(show.name); + } + 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 { + var combinator = 'show ${allNames.join(', ')}'; + var range = SourceRange(show.offset, show.length); + builder.addSimpleReplacement(range, combinator); + } + } + }); } } diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index f15cdeec491c..6d30d60989bf 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -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, @@ -864,6 +869,11 @@ abstract final class DartFixKind { DartFixKindPriority.standard + 1, "Import library '{0}' with prefix '{1}'", ); + 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, @@ -874,11 +884,6 @@ abstract final class DartFixKind { DartFixKindPriority.standard + 4, "Import library '{0}' with prefix '{1}'", ); - static const IMPORT_LIBRARY_HIDE = FixKind( - 'dart.fix.import.libraryHide', - DartFixKindPriority.standard, - "Use '{0}' from '{1}'{2}{3}", - ); static const INLINE_INVOCATION = FixKind( 'dart.fix.inlineInvocation', DartFixKindPriority.standard - 20, diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 98d639082fc6..4a604d682b39 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -538,7 +538,7 @@ final _builtInNonLintMultiProducers = { AddExtensionOverride.new, ], CompileTimeErrorCode.AMBIGUOUS_IMPORT: [ - ImportAddHide.new, + AmbiguousImportFix.new, ], CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [DataDriven.new], CompileTimeErrorCode.CAST_TO_NON_TYPE: [ diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 808e37df2053..1e7f7de66ded 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -12,6 +12,7 @@ import 'fix_processor.dart'; void main() { defineReflectiveSuite(() { defineReflectiveTests(ImportAddHideTest); + defineReflectiveTests(ImportRemoveShowTest); }); } @@ -20,7 +21,7 @@ class ImportAddHideTest extends FixProcessorTest { @override FixKind get kind => DartFixKind.IMPORT_LIBRARY_HIDE; - Future test_doubleAliasedImports() async { + Future test_double_aliased() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -36,8 +37,8 @@ void f(i.N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Use 'N' from 'lib1.dart' as i", - "Use 'N' from 'lib2.dart' as i", + "Hide others to use 'N' from 'lib1.dart' as i", + "Hide others to use 'N' from 'lib2.dart' as i", ]); await assertHasFix(''' import 'lib1.dart' as i; @@ -46,7 +47,7 @@ import 'lib2.dart' as i hide N; void f(i.N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib1.dart' as i"); +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart' as i"); await assertHasFix(''' import 'lib1.dart' as i hide N; import 'lib2.dart' as i; @@ -54,10 +55,10 @@ import 'lib2.dart' as i; void f(i.N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib2.dart' as i"); +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart' as i"); } - Future test_DoubleImports() async { + Future test_double() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -73,8 +74,8 @@ void f(N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Use 'N' from 'lib1.dart'", - "Use 'N' from 'lib2.dart'", + "Hide others to use 'N' from 'lib1.dart'", + "Hide others to use 'N' from 'lib2.dart'", ]); await assertHasFix(''' import 'lib1.dart' hide N; @@ -83,7 +84,7 @@ import 'lib2.dart'; void f(N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib2.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); await assertHasFix(''' import 'lib1.dart'; import 'lib2.dart' hide N; @@ -91,47 +92,10 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); } - Future test_doubleImports_bothShow() async { - newFile('$testPackageLibPath/lib1.dart', ''' -class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' -class N {}'''); - await resolveTestCode(''' -import 'lib1.dart' show N; -import 'lib2.dart' show N; - -void f(N? n) { - print(n); -} -'''); - await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 2, - matchFixMessages: [ - "Use 'N' from 'lib1.dart' (removing 'show')", - "Use 'N' from 'lib2.dart' (removing 'show')", - ]); - await assertHasFix(''' -import 'lib1.dart' hide N; -import 'lib2.dart' show N; - -void f(N? n) { - print(n); -} -''', matchFixMessage: "Use 'N' from 'lib2.dart' (removing 'show')"); - await assertHasFix(''' -import 'lib1.dart' show N; -import 'lib2.dart' hide N; - -void f(N? n) { - print(n); -} -''', matchFixMessage: "Use 'N' from 'lib1.dart' (removing 'show')"); - } - - Future test_doubleImports_constant() async { + Future test_double_constant() async { newFile('$testPackageLibPath/lib1.dart', ''' const foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -151,60 +115,10 @@ import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); } - Future test_tripleImports_oneAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart' as lib; -import 'lib1.dart'; -import 'lib2.dart'; - -void f() { - print(foo); -} -'''); - await assertHasFix(''' -import 'lib1.dart' as lib; -import 'lib1.dart'; -import 'lib2.dart' hide foo; - -void f() { - print(foo); -} -''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); - } - - Future test_tripleImports_twoAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart'; -import 'lib1.dart' as lib; -import 'lib2.dart' as lib; - -void f() { - print(lib.foo); -} -'''); - await assertHasFix(''' -import 'lib1.dart'; -import 'lib1.dart' as lib; -import 'lib2.dart' as lib hide foo; - -void f() { - print(lib.foo); -} -''', matchFixMessage: "Use 'foo' from 'lib1.dart' as lib"); - } - - Future test_doubleImports_exportedByImport() async { + Future test_double_exportedByImport() async { newFile('$testPackageLibPath/lib1.dart', ''' export 'lib3.dart';'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -222,12 +136,13 @@ import 'lib1.dart' hide M; import 'lib2.dart'; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib2.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib2.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } - Future test_doubleImports_extension() async { + Future test_double_extension() async { newFile('$testPackageLibPath/lib1.dart', ''' extension E on int { bool get isDivisibleByThree => this % 3 == 0; @@ -251,12 +166,13 @@ import 'lib2.dart' hide E; void foo(int i) { print(E(i.isDivisibleByThree)); } -''', matchFixMessage: "Use 'E' from 'lib1.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'E' from 'lib1.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } - Future test_doubleImports_function() async { + Future test_double_function() async { newFile('$testPackageLibPath/lib1.dart', ''' void bar() {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -276,10 +192,10 @@ import 'lib2.dart' hide bar; void foo() { bar(); } -''', matchFixMessage: "Use 'bar' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'bar' from 'lib1.dart'"); } - Future test_doubleImports_mixin() async { + Future test_double_mixin() async { newFile('$testPackageLibPath/lib1.dart', ''' mixin M {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -295,12 +211,13 @@ import 'lib1.dart'; import 'lib2.dart' hide M; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib1.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } - Future test_doubleImports_oneHide() async { + Future test_double_oneHide() async { newFile('$testPackageLibPath/lib1.dart', ''' class M {} class N {} class O {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -320,10 +237,10 @@ import 'lib2.dart'; void f(N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib2.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); } - Future test_doubleImports_oneShow() async { + Future test_double_oneShow() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -343,10 +260,10 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); } - Future test_doubleImports_variable() async { + Future test_double_variable() async { newFile('$testPackageLibPath/lib1.dart', ''' var foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -366,7 +283,7 @@ import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); } Future test_show_prefixed() async { @@ -376,37 +293,16 @@ class N {}'''); class N {}'''); await resolveTestCode(''' import 'lib1.dart' as l show N; -import 'lib2.dart' as l show N; +import 'lib2.dart' as l; void f(l.N? n) { print(n); } '''); - await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 2, - matchFixMessages: [ - "Use 'N' from 'lib1.dart' as l (removing 'show')", - "Use 'N' from 'lib2.dart' as l (removing 'show')", - ]); - await assertHasFix(''' -import 'lib1.dart' as l hide N; -import 'lib2.dart' as l show N; - -void f(l.N? n) { - print(n); -} -''', matchFixMessage: "Use 'N' from 'lib2.dart' as l (removing 'show')"); - await assertHasFix(''' -import 'lib1.dart' as l show N; -import 'lib2.dart' as l hide N; - -void f(l.N? n) { - print(n); -} -''', matchFixMessage: "Use 'N' from 'lib1.dart' as l (removing 'show')"); + await assertNoFix(); } - Future test_tripleImports() async { + Future test_triple() async { newFile('$testPackageLibPath/lib1.dart', ''' export 'lib3.dart';'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -426,7 +322,8 @@ import 'lib2.dart' hide M; import 'lib3.dart' hide M; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib1.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); await assertHasFix(''' @@ -435,7 +332,8 @@ import 'lib2.dart'; import 'lib3.dart' hide M; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib2.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib2.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); await assertHasFix(''' @@ -444,8 +342,218 @@ import 'lib2.dart' hide M; import 'lib3.dart'; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib3.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib3.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } + + Future test_triple_oneAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' as lib; +import 'lib1.dart'; +import 'lib2.dart'; + +void f() { + print(foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart' as lib; +import 'lib1.dart'; +import 'lib2.dart' hide foo; + +void f() { + print(foo); +} +''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); + } + + Future test_triple_twoAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib1.dart' as lib; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib1.dart' as lib; +import 'lib2.dart' as lib hide foo; + +void f() { + print(lib.foo); +} +''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart' as lib"); + } +} + +@reflectiveTest +class ImportRemoveShowTest extends FixProcessorTest { + @override + FixKind get kind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; + + Future test_triple_twoAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib1.dart' as lib show foo; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib1.dart' as lib hide foo; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +''', matchFixMessage: "Remove show to use 'foo' from 'lib1.dart' as lib"); + } + + Future test_double_oneHide() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' hide foo; +import 'lib2.dart' show foo; + +void f() { + print(foo); +} +'''); + await assertNoFix(); + } + + Future test_double_aliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +'''); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Remove show to use 'N' from 'lib1.dart' as l", + "Remove show to use 'N' from 'lib2.dart' as l", + ]); + await assertHasFix(''' +import 'lib1.dart' as l hide N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart' as l"); + await assertHasFix(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l hide N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Remove show to use 'N' from 'lib1.dart' as l"); + } + + Future test_double() async { + newFile('$testPackageLibPath/lib1.dart', ''' +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' show N; +import 'lib2.dart' show N; + +void f(l.N? n) { + print(n); +} +'''); + await assertHasFix(''' +import 'lib1.dart' as l hide N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart' as l"); + } + + Future test_moreShow() async { + newFile('$testPackageLibPath/lib1.dart', ''' +class N {} +class M {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' show N, M; +import 'lib2.dart'; + +void f(N? n) { + print(n); +} +'''); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 1, + matchFixMessages: [ + "Remove show to use 'N' from 'lib2.dart'", + ]); + await assertHasFix(''' +import 'lib1.dart' show M; +import 'lib2.dart'; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart'"); + } + + Future test_one_show() async { + newFile('$testPackageLibPath/lib1.dart', ''' +var foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +var foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' show foo; +import 'lib2.dart'; + +void f() { + print(foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart' hide foo; +import 'lib2.dart'; + +void f() { + print(foo); +} +''', matchFixMessage: "Remove show to use 'foo' from 'lib2.dart'"); + } } From 10285ac6149175553462d366ddb976ca666d7a8a Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 14 Nov 2024 08:40:00 -0300 Subject: [PATCH 07/28] migrates to new element model and fixes tests --- .../correction/dart/import_add_hide.dart | 32 +++++++++---------- .../correction/fix/import_add_hide_test.dart | 21 ++++++++---- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 50ab58d2c0b8..80e32d69e9cf 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -5,7 +5,7 @@ import 'package:analysis_server/src/services/correction/fix.dart'; import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/element2.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; @@ -17,25 +17,25 @@ class AmbiguousImportFix extends MultiCorrectionProducer { @override Future> get producers async { var node = this.node; - Element? element; + Element2? element; String? prefix; if (node is NamedType) { - element = node.element; + element = node.element2; prefix = node.importPrefix?.name.lexeme; } else if (node is SimpleIdentifier) { - element = node.staticElement; + element = node.element; if (node.parent case PrefixedIdentifier(prefix: var currentPrefix)) { prefix = currentPrefix.name; } } - if (element is! MultiplyDefinedElement) { + if (element is! MultiplyDefinedElement2) { return const []; } - var elements = element.conflictingElements; - var records = <({Element element, String uri, String? prefix}), + var elements = element.conflictingElements2; + var records = <({Element2 element, String uri, String? prefix}), List>{}; for (var element in elements) { - var library = element.enclosingElement3?.library; + var library = element.enclosingElement2?.library2; if (library == null) { continue; } @@ -45,7 +45,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { // and have the same prefix for (var directive in unit.directives.whereType()) { // Get import directive that - var imported = directive.element?.importedLibrary; + var imported = directive.libraryImport?.importedLibrary2; if (imported == null) { continue; } @@ -54,7 +54,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { } // If the directive exports the library, then the library is also // imported. - if (imported.exportedLibraries.contains(library) && + if (imported.exportedLibraries2.contains(library) && directive.prefix?.name == prefix) { directives.add(directive); } @@ -91,7 +91,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { class _ImportAddHide extends ResolvedCorrectionProducer { final Set importDirectives; - final Element element; + final Element2 element; final String uri; final String? prefix; @@ -115,7 +115,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; - String get _elementName => element.name ?? ''; + String get _elementName => element.name3 ?? ''; @override Future compute(ChangeBuilder builder) async { @@ -128,7 +128,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { for (var directive in importDirectives) { var show = directive.combinators.whereType().firstOrNull; - // If there is an import with a show combinator, then we don't want to + // If there is an import with a show combinator, then we don't want to // deal with this case here. if (show != null) { return; @@ -159,7 +159,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { class _ImportRemoveShow extends ResolvedCorrectionProducer { final Set importDirectives; - final Element element; + final Element2 element; final String uri; final String? prefix; @@ -183,7 +183,7 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { @override FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; - String get _elementName => element.name ?? ''; + String get _elementName => element.name3 ?? ''; @override Future compute(ChangeBuilder builder) async { @@ -194,7 +194,7 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { var showCombinator = <({ImportDirective directive, ShowCombinator show})>[]; for (var directive in importDirectives) { var show = directive.combinators.whereType().firstOrNull; - // If there is no show combinator, then we don't want to deal with this + // If there is no show combinator, then we don't want to deal with this // case here. if (show == null) { return; diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 1e7f7de66ded..d2aad0444829 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -299,7 +299,14 @@ void f(l.N? n) { print(n); } '''); - await assertNoFix(); + await assertHasFix(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l hide N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart' as l"); } Future test_triple() async { @@ -426,7 +433,7 @@ import 'lib2.dart' as lib; void f() { print(lib.foo); } -''', matchFixMessage: "Remove show to use 'foo' from 'lib1.dart' as lib"); +''', matchFixMessage: "Remove show to use 'foo' from 'lib2.dart' as lib"); } Future test_double_oneHide() async { @@ -491,18 +498,18 @@ class N {}'''); import 'lib1.dart' show N; import 'lib2.dart' show N; -void f(l.N? n) { +void f(N? n) { print(n); } '''); await assertHasFix(''' -import 'lib1.dart' as l hide N; -import 'lib2.dart' as l show N; +import 'lib1.dart' hide N; +import 'lib2.dart' show N; -void f(l.N? n) { +void f(N? n) { print(n); } -''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart' as l"); +''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart'"); } Future test_moreShow() async { From a6b4f038adf003016b0651b8d3b733ce29fd4c96 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:13:24 -0300 Subject: [PATCH 08/28] new assist to add/edit `hide` at import for ambiguous import --- .../correction/dart/import_add_hide.dart | 209 +++------ .../lib/src/services/correction/fix.dart | 5 + .../src/services/correction/fix_internal.dart | 3 + .../correction/fix/import_add_hide_test.dart | 402 ++++-------------- 4 files changed, 137 insertions(+), 482 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 80e32d69e9cf..58c3584c0297 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -5,221 +5,108 @@ import 'package:analysis_server/src/services/correction/fix.dart'; import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/element/element2.dart'; +import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; -import 'package:collection/collection.dart'; -class AmbiguousImportFix extends MultiCorrectionProducer { - AmbiguousImportFix({required super.context}); +class ImportAddHide extends MultiCorrectionProducer { + ImportAddHide({required super.context}); @override Future> get producers async { var node = this.node; - Element2? element; - String? prefix; + Element? element; 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; - } + } else if (node is SimpleIdentifier) { + element = node.staticElement; } - if (element is! MultiplyDefinedElement2) { + if (element is! MultiplyDefinedElement) { return const []; } - var elements = element.conflictingElements2; - var records = <({Element2 element, String uri, String? prefix}), - List>{}; + var elements = element.conflictingElements; + var pairs = {}; for (var element in elements) { - var library = element.enclosingElement2?.library2; - if (library == null) { - continue; - } - - var directives = []; + var library = element.enclosingElement3?.library; // find all ImportDirective that import this library in this unit - // and have the same prefix for (var directive in unit.directives.whereType()) { - // Get import directive that - var imported = directive.libraryImport?.importedLibrary2; + var imported = directive.element?.importedLibrary; if (imported == null) { continue; } - if (imported == library && directive.prefix?.name == prefix) { - directives.add(directive); + if (imported == library) { + pairs[directive] = element; } // If the directive exports the library, then the library is also // imported. - if (imported.exportedLibraries2.contains(library) && - directive.prefix?.name == prefix) { - directives.add(directive); - } - } - for (var directive in directives) { - var uri = directive.uri.stringValue; - var prefix = directive.prefix?.name; - if (uri != null) { - records[(element: element, uri: uri, prefix: prefix)] = directives; + if (imported.exportedLibraries.contains(library)) { + pairs[directive] = element; } } } - if (records.entries.isEmpty) { - return const []; - } - - var producers = []; - for (var MapEntry(key: key) in records.entries) { - var directives = records.entries - .whereNot((e) => e.key == key) - .expand((e) => e.value) - .whereNot((d) => - (d.prefix?.name == key.prefix) && (d.uri.stringValue == key.uri)) - .toSet(); - producers.add(_ImportAddHide(key.element, key.uri, key.prefix, directives, - context: context)); - producers.add(_ImportRemoveShow( - key.element, key.uri, key.prefix, directives, - context: context)); - } - return producers; + return [ + for (var MapEntry(key: import, value: element) in pairs.entries) + _ImportAddHide(import, element, context: context), + ]; } } class _ImportAddHide extends ResolvedCorrectionProducer { - final Set importDirectives; - final Element2 element; - final String uri; - final String? prefix; + _ImportAddHide(this.importDirective, this.element, {required super.context}); - _ImportAddHide(this.element, this.uri, this.prefix, this.importDirectives, - {required super.context}); + final ImportDirective importDirective; + final Element element; @override CorrectionApplicability get applicability => // TODO(applicability): comment on why. CorrectionApplicability.singleLocation; - @override - List get fixArguments { - var prefix = ''; - if ((this.prefix ?? '') != '') { - prefix = ' as ${this.prefix}'; - } - return [_elementName, uri, prefix]; - } - @override FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; - String get _elementName => element.name3 ?? ''; - - @override - Future compute(ChangeBuilder builder) async { - if (_elementName.isEmpty || uri.isEmpty) { - return; - } - - var hideCombinator = - <({ImportDirective directive, HideCombinator? hide})>[]; - - for (var directive in importDirectives) { - var show = directive.combinators.whereType().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().firstOrNull; - hideCombinator.add((directive: directive, hide: hide)); - } - - await builder.addDartFileEdit(file, (builder) { - for (var (:directive, :hide) in hideCombinator) { - if (hide != null) { - var allNames = [_elementName]; - for (var name in hide.hiddenNames) { - allNames.add(name.name); - } - allNames.sort(); - var combinator = 'hide ${allNames.join(', ')}'; - var range = SourceRange(hide.offset, hide.length); - builder.addSimpleReplacement(range, combinator); - } else { - var hideCombinator = ' hide $_elementName'; - builder.addSimpleInsertion(directive.end - 1, hideCombinator); - } - } - }); - } -} - -class _ImportRemoveShow extends ResolvedCorrectionProducer { - final Set importDirectives; - final Element2 element; - final String uri; - final String? prefix; - - _ImportRemoveShow(this.element, this.uri, this.prefix, this.importDirectives, - {required super.context}); - - @override - CorrectionApplicability get applicability => - // TODO(applicability): comment on why. - CorrectionApplicability.singleLocation; - @override List get fixArguments { - var prefix = ''; - if ((this.prefix ?? '') != '') { - prefix = ' as ${this.prefix}'; + var aliasStr = importDirective.prefix?.name; + var alias = ''; + if (aliasStr != null) { + alias = " as '$aliasStr'"; } - return [_elementName, uri, prefix]; + return [elementName, importStr, alias]; } - @override - FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; - - String get _elementName => element.name3 ?? ''; + String get importStr => importDirective.uri.stringValue ?? ''; + String get elementName => element.name ?? ''; @override Future compute(ChangeBuilder builder) async { - if (_elementName.isEmpty || uri.isEmpty) { + if (elementName.isEmpty || importStr.isEmpty) { return; } - var showCombinator = <({ImportDirective directive, ShowCombinator show})>[]; - for (var directive in importDirectives) { - var show = directive.combinators.whereType().firstOrNull; - // If there is no show combinator, then we don't want to deal with this - // case here. - if (show == null) { - return; - } - showCombinator.add((directive: directive, show: show)); + if (importDirective.combinators.whereType().isNotEmpty) { + return; } - await builder.addDartFileEdit(file, (builder) { - for (var (:directive, :show) in showCombinator) { - var allNames = []; - for (var show in show.shownNames) { - if (show.name == _elementName) continue; - allNames.add(show.name); + var hide = importDirective.combinators.whereType(); + if (hide.isNotEmpty) { + return await builder.addDartFileEdit(file, (builder) { + var hideCombinator = hide.first; + var allNames = [elementName]; + for (var name in hideCombinator.hiddenNames) { + allNames.add(name.name); } 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 { - var combinator = 'show ${allNames.join(', ')}'; - var range = SourceRange(show.offset, show.length); - builder.addSimpleReplacement(range, combinator); - } - } + var combinator = 'hide ${allNames.join(', ')}'; + var range = SourceRange(hideCombinator.offset, hideCombinator.length); + builder.addSimpleReplacement(range, combinator); + }); + } + + await builder.addDartFileEdit(file, (builder) { + var hideCombinator = ' hide $elementName'; + builder.addSimpleInsertion(importDirective.end - 1, hideCombinator); }); } } diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index 6d30d60989bf..f8c844dd5e48 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -884,6 +884,11 @@ abstract final class DartFixKind { DartFixKindPriority.standard + 4, "Import library '{0}' with prefix '{1}'", ); + static const IMPORT_LIBRARY_HIDE = FixKind( + 'dart.fix.import.libraryHide', + DartFixKindPriority.standard, + "Add 'hide {0}' to library '{1}'{2} import", + ); static const INLINE_INVOCATION = FixKind( 'dart.fix.inlineInvocation', DartFixKindPriority.standard - 20, diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 4a604d682b39..623ef582a3a0 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -531,6 +531,9 @@ final _builtInLintProducers = >{ }; final _builtInNonLintMultiProducers = { + CompileTimeErrorCode.AMBIGUOUS_IMPORT: [ + ImportAddHide.new, + ], CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO: [ AddExtensionOverride.new, ], diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index d2aad0444829..5361a887c02a 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -12,7 +12,6 @@ import 'fix_processor.dart'; void main() { defineReflectiveSuite(() { defineReflectiveTests(ImportAddHideTest); - defineReflectiveTests(ImportRemoveShowTest); }); } @@ -21,10 +20,12 @@ class ImportAddHideTest extends FixProcessorTest { @override FixKind get kind => DartFixKind.IMPORT_LIBRARY_HIDE; - Future test_double_aliased() async { + Future test_doubleAliasedImports() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' as i; @@ -37,31 +38,33 @@ void f(i.N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Hide others to use 'N' from 'lib1.dart' as i", - "Hide others to use 'N' from 'lib2.dart' as i", + "Add 'hide N' to library 'lib1.dart' as 'i' import", + "Add 'hide N' to library 'lib2.dart' as 'i' import", ]); await assertHasFix(''' -import 'lib1.dart' as i; -import 'lib2.dart' as i hide N; +import 'lib1.dart' as i hide N; +import 'lib2.dart' as i; void f(i.N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart' as i"); +''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' as 'i' import"); await assertHasFix(''' -import 'lib1.dart' as i hide N; -import 'lib2.dart' as i; +import 'lib1.dart' as i; +import 'lib2.dart' as i hide N; void f(i.N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart' as i"); +''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' as 'i' import"); } - Future test_double() async { + Future test_DoubleImports() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -74,8 +77,8 @@ void f(N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Hide others to use 'N' from 'lib1.dart'", - "Hide others to use 'N' from 'lib2.dart'", + "Add 'hide N' to library 'lib1.dart' import", + "Add 'hide N' to library 'lib2.dart' import", ]); await assertHasFix(''' import 'lib1.dart' hide N; @@ -84,7 +87,7 @@ import 'lib2.dart'; void f(N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); +''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' import"); await assertHasFix(''' import 'lib1.dart'; import 'lib2.dart' hide N; @@ -92,16 +95,36 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); +''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' import"); + } + + Future test_doubleImports_bothShow() async { + newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' show N; +import 'lib2.dart' show N; + +void f(N? n) { + print(n); +} +'''); + await assertNoFix(); } - Future test_double_constant() async { + Future test_doubleImports_constant() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; const foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; const foo = 0;'''); await resolveTestCode(''' -import 'lib1.dart'; +import 'lib1.dart' show foo; import 'lib2.dart'; void f() { @@ -109,50 +132,55 @@ void f() { } '''); await assertHasFix(''' -import 'lib1.dart'; +import 'lib1.dart' show foo; import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); +''', matchFixMessage: "Add 'hide foo' to library 'lib2.dart' import"); } - Future test_double_exportedByImport() async { + Future test_doubleImports_exportedByImport() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; export 'lib3.dart';'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; mixin M {}'''); newFile('$testPackageLibPath/lib3.dart', ''' +library lib3; mixin M {}'''); await resolveTestCode(''' import 'lib1.dart'; -import 'lib2.dart'; +import 'lib2.dart' show M; class C with M {} '''); await assertHasFix(''' import 'lib1.dart' hide M; -import 'lib2.dart'; +import 'lib2.dart' show M; class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib2.dart'", +''', matchFixMessage: "Add 'hide M' to library 'lib1.dart' import", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } - Future test_double_extension() async { + Future test_doubleImports_extension() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); await resolveTestCode(''' -import 'lib1.dart'; +import 'lib1.dart' show E; import 'lib2.dart'; void foo(int i) { @@ -160,25 +188,27 @@ void foo(int i) { } '''); await assertHasFix(''' -import 'lib1.dart'; +import 'lib1.dart' show E; import 'lib2.dart' hide E; void foo(int i) { print(E(i.isDivisibleByThree)); } -''', matchFixMessage: "Hide others to use 'E' from 'lib1.dart'", +''', matchFixMessage: "Add 'hide E' to library 'lib2.dart' import", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } - Future test_double_function() async { + Future test_doubleImports_function() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; void bar() {}'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; void bar() {}'''); await resolveTestCode(''' -import 'lib1.dart'; +import 'lib1.dart' show bar; import 'lib2.dart'; void foo() { @@ -186,45 +216,49 @@ void foo() { } '''); await assertHasFix(''' -import 'lib1.dart'; +import 'lib1.dart' show bar; import 'lib2.dart' hide bar; void foo() { bar(); } -''', matchFixMessage: "Hide others to use 'bar' from 'lib1.dart'"); +''', matchFixMessage: "Add 'hide bar' to library 'lib2.dart' import"); } - Future test_double_mixin() async { + Future test_doubleImports_mixin() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; mixin M {}'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; mixin M {}'''); await resolveTestCode(''' -import 'lib1.dart'; +import 'lib1.dart' show M; import 'lib2.dart'; class C with M {} '''); await assertHasFix(''' -import 'lib1.dart'; +import 'lib1.dart' show M; import 'lib2.dart' hide M; class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", +''', matchFixMessage: "Add 'hide M' to library 'lib2.dart' import", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } - Future test_double_oneHide() async { + Future test_doubleImports_oneHide() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; class M {} class N {} class O {}'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' hide M, O; -import 'lib2.dart'; +import 'lib2.dart' show N; void f(N? n) { print(n); @@ -232,18 +266,20 @@ void f(N? n) { '''); await assertHasFix(''' import 'lib1.dart' hide M, N, O; -import 'lib2.dart'; +import 'lib2.dart' show N; void f(N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); +''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' import"); } - Future test_double_oneShow() async { + Future test_doubleImports_oneShow() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N; @@ -260,13 +296,15 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); +''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' import"); } - Future test_double_variable() async { + Future test_doubleImports_variable() async { newFile('$testPackageLibPath/lib1.dart', ''' +library lib1; var foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' +library lib2; var foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' show foo; @@ -283,284 +321,6 @@ import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); - } - - Future test_show_prefixed() async { - newFile('$testPackageLibPath/lib1.dart', ''' -class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' -class N {}'''); - await resolveTestCode(''' -import 'lib1.dart' as l show N; -import 'lib2.dart' as l; - -void f(l.N? n) { - print(n); -} -'''); - await assertHasFix(''' -import 'lib1.dart' as l show N; -import 'lib2.dart' as l hide N; - -void f(l.N? n) { - print(n); -} -''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart' as l"); - } - - Future test_triple() async { - newFile('$testPackageLibPath/lib1.dart', ''' -export 'lib3.dart';'''); - newFile('$testPackageLibPath/lib2.dart', ''' -mixin M {}'''); - newFile('$testPackageLibPath/lib3.dart', ''' -mixin M {}'''); - await resolveTestCode(''' -import 'lib1.dart'; -import 'lib2.dart'; -import 'lib3.dart'; - -class C with M {} -'''); - await assertHasFix(''' -import 'lib1.dart'; -import 'lib2.dart' hide M; -import 'lib3.dart' hide M; - -class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); - await assertHasFix(''' -import 'lib1.dart' hide M; -import 'lib2.dart'; -import 'lib3.dart' hide M; - -class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib2.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); - await assertHasFix(''' -import 'lib1.dart' hide M; -import 'lib2.dart' hide M; -import 'lib3.dart'; - -class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib3.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); - } - - Future test_triple_oneAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart' as lib; -import 'lib1.dart'; -import 'lib2.dart'; - -void f() { - print(foo); -} -'''); - await assertHasFix(''' -import 'lib1.dart' as lib; -import 'lib1.dart'; -import 'lib2.dart' hide foo; - -void f() { - print(foo); -} -''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); - } - - Future test_triple_twoAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart'; -import 'lib1.dart' as lib; -import 'lib2.dart' as lib; - -void f() { - print(lib.foo); -} -'''); - await assertHasFix(''' -import 'lib1.dart'; -import 'lib1.dart' as lib; -import 'lib2.dart' as lib hide foo; - -void f() { - print(lib.foo); -} -''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart' as lib"); - } -} - -@reflectiveTest -class ImportRemoveShowTest extends FixProcessorTest { - @override - FixKind get kind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; - - Future test_triple_twoAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart'; -import 'lib1.dart' as lib show foo; -import 'lib2.dart' as lib; - -void f() { - print(lib.foo); -} -'''); - await assertHasFix(''' -import 'lib1.dart'; -import 'lib1.dart' as lib hide foo; -import 'lib2.dart' as lib; - -void f() { - print(lib.foo); -} -''', matchFixMessage: "Remove show to use 'foo' from 'lib2.dart' as lib"); - } - - Future test_double_oneHide() async { - newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart' hide foo; -import 'lib2.dart' show foo; - -void f() { - print(foo); -} -'''); - await assertNoFix(); - } - - Future test_double_aliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' -class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' -class N {}'''); - await resolveTestCode(''' -import 'lib1.dart' as l show N; -import 'lib2.dart' as l show N; - -void f(l.N? n) { - print(n); -} -'''); - await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 2, - matchFixMessages: [ - "Remove show to use 'N' from 'lib1.dart' as l", - "Remove show to use 'N' from 'lib2.dart' as l", - ]); - await assertHasFix(''' -import 'lib1.dart' as l hide N; -import 'lib2.dart' as l show N; - -void f(l.N? n) { - print(n); -} -''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart' as l"); - await assertHasFix(''' -import 'lib1.dart' as l show N; -import 'lib2.dart' as l hide N; - -void f(l.N? n) { - print(n); -} -''', matchFixMessage: "Remove show to use 'N' from 'lib1.dart' as l"); - } - - Future test_double() async { - newFile('$testPackageLibPath/lib1.dart', ''' -class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' -class N {}'''); - await resolveTestCode(''' -import 'lib1.dart' show N; -import 'lib2.dart' show N; - -void f(N? n) { - print(n); -} -'''); - await assertHasFix(''' -import 'lib1.dart' hide N; -import 'lib2.dart' show N; - -void f(N? n) { - print(n); -} -''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart'"); - } - - Future test_moreShow() async { - newFile('$testPackageLibPath/lib1.dart', ''' -class N {} -class M {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' -class N {}'''); - await resolveTestCode(''' -import 'lib1.dart' show N, M; -import 'lib2.dart'; - -void f(N? n) { - print(n); -} -'''); - await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 1, - matchFixMessages: [ - "Remove show to use 'N' from 'lib2.dart'", - ]); - await assertHasFix(''' -import 'lib1.dart' show M; -import 'lib2.dart'; - -void f(N? n) { - print(n); -} -''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart'"); - } - - Future test_one_show() async { - newFile('$testPackageLibPath/lib1.dart', ''' -var foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -var foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart' show foo; -import 'lib2.dart'; - -void f() { - print(foo); -} -'''); - await assertHasFix(''' -import 'lib1.dart' hide foo; -import 'lib2.dart'; - -void f() { - print(foo); -} -''', matchFixMessage: "Remove show to use 'foo' from 'lib2.dart'"); +''', matchFixMessage: "Add 'hide foo' to library 'lib2.dart' import"); } } From 0386796916d7496ba2b3514e07ae196d33a13f52 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 3 Oct 2024 09:01:49 -0300 Subject: [PATCH 09/28] sorting file --- .../correction/dart/import_add_hide.dart | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 58c3584c0297..f74a5e4e84ba 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -53,19 +53,16 @@ class ImportAddHide extends MultiCorrectionProducer { } class _ImportAddHide extends ResolvedCorrectionProducer { - _ImportAddHide(this.importDirective, this.element, {required super.context}); - final ImportDirective importDirective; final Element element; + _ImportAddHide(this.importDirective, this.element, {required super.context}); + @override CorrectionApplicability get applicability => // TODO(applicability): comment on why. CorrectionApplicability.singleLocation; - @override - FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; - @override List get fixArguments { var aliasStr = importDirective.prefix?.name; @@ -73,15 +70,18 @@ class _ImportAddHide extends ResolvedCorrectionProducer { if (aliasStr != null) { alias = " as '$aliasStr'"; } - return [elementName, importStr, alias]; + return [_elementName, _importStr, alias]; } - String get importStr => importDirective.uri.stringValue ?? ''; - String get elementName => element.name ?? ''; + @override + FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; + + String get _elementName => element.name ?? ''; + String get _importStr => importDirective.uri.stringValue ?? ''; @override Future compute(ChangeBuilder builder) async { - if (elementName.isEmpty || importStr.isEmpty) { + if (_elementName.isEmpty || _importStr.isEmpty) { return; } @@ -93,7 +93,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { if (hide.isNotEmpty) { return await builder.addDartFileEdit(file, (builder) { var hideCombinator = hide.first; - var allNames = [elementName]; + var allNames = [_elementName]; for (var name in hideCombinator.hiddenNames) { allNames.add(name.name); } @@ -105,7 +105,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { } await builder.addDartFileEdit(file, (builder) { - var hideCombinator = ' hide $elementName'; + var hideCombinator = ' hide $_elementName'; builder.addSimpleInsertion(importDirective.end - 1, hideCombinator); }); } From bcd5d7fb1574c202607ff9dca3e63775a6e05503 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 10 Oct 2024 15:46:27 -0300 Subject: [PATCH 10/28] refactor to simplify user choice and automation - Removed verbose process for user. - Refactored to allow user to specify desired elements explicitly. - The fix now automatically hide everything else --- .../correction/dart/import_add_hide.dart | 130 ++++++++---- .../lib/src/services/correction/fix.dart | 2 +- .../correction/fix/import_add_hide_test.dart | 193 ++++++++++++------ 3 files changed, 229 insertions(+), 96 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index f74a5e4e84ba..38bacd7a3092 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -9,6 +9,7 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:collection/collection.dart'; class ImportAddHide extends MultiCorrectionProducer { ImportAddHide({required super.context}); @@ -26,37 +27,66 @@ class ImportAddHide extends MultiCorrectionProducer { return const []; } var elements = element.conflictingElements; - var pairs = {}; + var records = <({Element element, String uri, String? prefix}), + List>{}; for (var element in elements) { var library = element.enclosingElement3?.library; + if (library == null) { + continue; + } + + var directives = []; // find all ImportDirective that import this library in this unit for (var directive in unit.directives.whereType()) { + // Get import directive that var imported = directive.element?.importedLibrary; if (imported == null) { continue; } if (imported == library) { - pairs[directive] = element; + directives.add(directive); } // If the directive exports the library, then the library is also // imported. if (imported.exportedLibraries.contains(library)) { - pairs[directive] = element; + directives.add(directive); + } + } + for (var directive in directives) { + var uri = directive.uri.stringValue; + var prefix = directive.prefix?.name; + if (uri != null) { + records[(element: element, uri: uri, prefix: prefix)] = directives; } } } - return [ - for (var MapEntry(key: import, value: element) in pairs.entries) - _ImportAddHide(import, element, context: context), - ]; + if (records.entries.isEmpty) { + return const []; + } + + var producers = []; + for (var MapEntry(key: key) in records.entries) { + var directives = records.entries + .whereNot((e) => e.key == key) + .expand((e) => e.value) + .whereNot((d) => + (d.prefix?.name == key.prefix) && (d.uri.stringValue == key.uri)) + .toSet(); + producers.add(_ImportAddHide(key.element, key.uri, key.prefix, directives, + context: context)); + } + return producers; } } class _ImportAddHide extends ResolvedCorrectionProducer { - final ImportDirective importDirective; + final Set importDirectives; final Element element; + final String uri; + final String? prefix; - _ImportAddHide(this.importDirective, this.element, {required super.context}); + _ImportAddHide(this.element, this.uri, this.prefix, this.importDirectives, + {required super.context}); @override CorrectionApplicability get applicability => @@ -65,48 +95,76 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override List get fixArguments { - var aliasStr = importDirective.prefix?.name; - var alias = ''; - if (aliasStr != null) { - alias = " as '$aliasStr'"; + // Any import directive that imports the element is directly showing it? + var removeShow = false; + for (var directives in importDirectives + .map((d) => d.combinators.whereType()) + .where((d) => d.isNotEmpty)) { + for (var name in directives.first.shownNames) { + if (name.name == _elementName) { + removeShow = true; + break; + } + } } - return [_elementName, _importStr, alias]; + // If there is a show combinator explicitly showing the element, then + // we should remove it. + var removeShowStr = removeShow ? ' (removing \'show\')' : ''; + var prefix = ''; + if ((this.prefix ?? '') != '') { + prefix = ' as ${this.prefix}'; + } + return [_elementName, uri, prefix, removeShowStr]; } @override FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; String get _elementName => element.name ?? ''; - String get _importStr => importDirective.uri.stringValue ?? ''; @override Future compute(ChangeBuilder builder) async { - if (_elementName.isEmpty || _importStr.isEmpty) { - return; - } - - if (importDirective.combinators.whereType().isNotEmpty) { + if (_elementName.isEmpty || uri.isEmpty) { return; } - var hide = importDirective.combinators.whereType(); - if (hide.isNotEmpty) { - return await builder.addDartFileEdit(file, (builder) { - var hideCombinator = hide.first; - var allNames = [_elementName]; - for (var name in hideCombinator.hiddenNames) { - allNames.add(name.name); + for (var directive in importDirectives) { + var show = directive.combinators.whereType(); + var hide = directive.combinators.whereType(); + await builder.addDartFileEdit(file, (builder) { + if (show.isNotEmpty) { + var showCombinator = show.first; + var allNames = []; + for (var show in showCombinator.shownNames) { + if (show.name == _elementName) continue; + allNames.add(show.name); + } + allNames.sort(); + if (allNames.isEmpty) { + builder.addDeletion(SourceRange( + showCombinator.offset - 1, showCombinator.length + 1)); + } else { + var combinator = 'show ${allNames.join(', ')}'; + var range = + SourceRange(showCombinator.offset, showCombinator.length); + builder.addSimpleReplacement(range, combinator); + } + } + if (hide.isNotEmpty) { + var hideCombinator = hide.first; + var allNames = [_elementName]; + for (var name in hideCombinator.hiddenNames) { + allNames.add(name.name); + } + allNames.sort(); + var combinator = 'hide ${allNames.join(', ')}'; + var range = SourceRange(hideCombinator.offset, hideCombinator.length); + builder.addSimpleReplacement(range, combinator); + } else { + var hideCombinator = ' hide $_elementName'; + builder.addSimpleInsertion(directive.end - 1, hideCombinator); } - allNames.sort(); - var combinator = 'hide ${allNames.join(', ')}'; - var range = SourceRange(hideCombinator.offset, hideCombinator.length); - builder.addSimpleReplacement(range, combinator); }); } - - await builder.addDartFileEdit(file, (builder) { - var hideCombinator = ' hide $_elementName'; - builder.addSimpleInsertion(importDirective.end - 1, hideCombinator); - }); } } diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index f8c844dd5e48..d4e76eac650a 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -887,7 +887,7 @@ abstract final class DartFixKind { static const IMPORT_LIBRARY_HIDE = FixKind( 'dart.fix.import.libraryHide', DartFixKindPriority.standard, - "Add 'hide {0}' to library '{1}'{2} import", + "Use '{0}' from '{1}'{2}{3}", ); static const INLINE_INVOCATION = FixKind( 'dart.fix.inlineInvocation', diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 5361a887c02a..9a58122bbc92 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -22,10 +22,8 @@ class ImportAddHideTest extends FixProcessorTest { Future test_doubleAliasedImports() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' as i; @@ -38,33 +36,31 @@ void f(i.N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Add 'hide N' to library 'lib1.dart' as 'i' import", - "Add 'hide N' to library 'lib2.dart' as 'i' import", + "Use 'N' from 'lib1.dart' as i", + "Use 'N' from 'lib2.dart' as i", ]); await assertHasFix(''' -import 'lib1.dart' as i hide N; -import 'lib2.dart' as i; +import 'lib1.dart' as i; +import 'lib2.dart' as i hide N; void f(i.N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' as 'i' import"); +''', matchFixMessage: "Use 'N' from 'lib1.dart' as i"); await assertHasFix(''' -import 'lib1.dart' as i; -import 'lib2.dart' as i hide N; +import 'lib1.dart' as i hide N; +import 'lib2.dart' as i; void f(i.N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' as 'i' import"); +''', matchFixMessage: "Use 'N' from 'lib2.dart' as i"); } Future test_DoubleImports() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -77,8 +73,8 @@ void f(N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Add 'hide N' to library 'lib1.dart' import", - "Add 'hide N' to library 'lib2.dart' import", + "Use 'N' from 'lib1.dart'", + "Use 'N' from 'lib2.dart'", ]); await assertHasFix(''' import 'lib1.dart' hide N; @@ -87,7 +83,7 @@ import 'lib2.dart'; void f(N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' import"); +''', matchFixMessage: "Use 'N' from 'lib2.dart'"); await assertHasFix(''' import 'lib1.dart'; import 'lib2.dart' hide N; @@ -95,15 +91,13 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'N' from 'lib1.dart'"); } Future test_doubleImports_bothShow() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N; @@ -113,18 +107,37 @@ void f(N? n) { print(n); } '''); - await assertNoFix(); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Use 'N' from 'lib1.dart' (removing 'show')", + "Use 'N' from 'lib2.dart' (removing 'show')", + ]); + await assertHasFix(''' +import 'lib1.dart' hide N; +import 'lib2.dart' show N; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Use 'N' from 'lib2.dart' (removing 'show')"); + await assertHasFix(''' +import 'lib1.dart' show N; +import 'lib2.dart' hide N; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Use 'N' from 'lib1.dart' (removing 'show')"); } Future test_doubleImports_constant() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; const foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; const foo = 0;'''); await resolveTestCode(''' -import 'lib1.dart' show foo; +import 'lib1.dart'; import 'lib2.dart'; void f() { @@ -132,55 +145,49 @@ void f() { } '''); await assertHasFix(''' -import 'lib1.dart' show foo; +import 'lib1.dart'; import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Add 'hide foo' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); } Future test_doubleImports_exportedByImport() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; export 'lib3.dart';'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; mixin M {}'''); newFile('$testPackageLibPath/lib3.dart', ''' -library lib3; mixin M {}'''); await resolveTestCode(''' import 'lib1.dart'; -import 'lib2.dart' show M; +import 'lib2.dart'; class C with M {} '''); await assertHasFix(''' import 'lib1.dart' hide M; -import 'lib2.dart' show M; +import 'lib2.dart'; class C with M {} -''', matchFixMessage: "Add 'hide M' to library 'lib1.dart' import", - errorFilter: (error) { +''', matchFixMessage: "Use 'M' from 'lib2.dart'", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } Future test_doubleImports_extension() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); await resolveTestCode(''' -import 'lib1.dart' show E; +import 'lib1.dart'; import 'lib2.dart'; void foo(int i) { @@ -188,27 +195,24 @@ void foo(int i) { } '''); await assertHasFix(''' -import 'lib1.dart' show E; +import 'lib1.dart'; import 'lib2.dart' hide E; void foo(int i) { print(E(i.isDivisibleByThree)); } -''', matchFixMessage: "Add 'hide E' to library 'lib2.dart' import", - errorFilter: (error) { +''', matchFixMessage: "Use 'E' from 'lib1.dart'", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } Future test_doubleImports_function() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; void bar() {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; void bar() {}'''); await resolveTestCode(''' -import 'lib1.dart' show bar; +import 'lib1.dart'; import 'lib2.dart'; void foo() { @@ -216,49 +220,44 @@ void foo() { } '''); await assertHasFix(''' -import 'lib1.dart' show bar; +import 'lib1.dart'; import 'lib2.dart' hide bar; void foo() { bar(); } -''', matchFixMessage: "Add 'hide bar' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'bar' from 'lib1.dart'"); } Future test_doubleImports_mixin() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; mixin M {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; mixin M {}'''); await resolveTestCode(''' -import 'lib1.dart' show M; +import 'lib1.dart'; import 'lib2.dart'; class C with M {} '''); await assertHasFix(''' -import 'lib1.dart' show M; +import 'lib1.dart'; import 'lib2.dart' hide M; class C with M {} -''', matchFixMessage: "Add 'hide M' to library 'lib2.dart' import", - errorFilter: (error) { +''', matchFixMessage: "Use 'M' from 'lib1.dart'", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } Future test_doubleImports_oneHide() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class M {} class N {} class O {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' hide M, O; -import 'lib2.dart' show N; +import 'lib2.dart'; void f(N? n) { print(n); @@ -266,20 +265,18 @@ void f(N? n) { '''); await assertHasFix(''' import 'lib1.dart' hide M, N, O; -import 'lib2.dart' show N; +import 'lib2.dart'; void f(N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib1.dart' import"); +''', matchFixMessage: "Use 'N' from 'lib2.dart'"); } Future test_doubleImports_oneShow() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N; @@ -296,15 +293,13 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Add 'hide N' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'N' from 'lib1.dart'"); } Future test_doubleImports_variable() async { newFile('$testPackageLibPath/lib1.dart', ''' -library lib1; var foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' -library lib2; var foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' show foo; @@ -321,6 +316,86 @@ import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Add 'hide foo' to library 'lib2.dart' import"); +''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); + } + + Future test_show_prefixed() async { + newFile('$testPackageLibPath/lib1.dart', ''' +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +'''); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Use 'N' from 'lib1.dart' as l (removing 'show')", + "Use 'N' from 'lib2.dart' as l (removing 'show')", + ]); + await assertHasFix(''' +import 'lib1.dart' as l hide N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Use 'N' from 'lib2.dart' as l (removing 'show')"); + await assertHasFix(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l hide N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Use 'N' from 'lib1.dart' as l (removing 'show')"); + } + + Future test_tripleImports() async { + newFile('$testPackageLibPath/lib1.dart', ''' +export 'lib3.dart';'''); + newFile('$testPackageLibPath/lib2.dart', ''' +mixin M {}'''); + newFile('$testPackageLibPath/lib3.dart', ''' +mixin M {}'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib2.dart'; +import 'lib3.dart'; + +class C with M {} +'''); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib2.dart' hide M; +import 'lib3.dart' hide M; + +class C with M {} +''', matchFixMessage: "Use 'M' from 'lib1.dart'", errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); + await assertHasFix(''' +import 'lib1.dart' hide M; +import 'lib2.dart'; +import 'lib3.dart' hide M; + +class C with M {} +''', matchFixMessage: "Use 'M' from 'lib2.dart'", errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); + await assertHasFix(''' +import 'lib1.dart' hide M; +import 'lib2.dart' hide M; +import 'lib3.dart'; + +class C with M {} +''', matchFixMessage: "Use 'M' from 'lib3.dart'", errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); } } From 34fdb5d725aaffc02bf34a6848bba830bed808a1 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Wed, 16 Oct 2024 10:20:54 -0300 Subject: [PATCH 11/28] readding the ambiguous import fix --- .../src/services/correction/fix_internal.dart | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 623ef582a3a0..dab651a8947c 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -106,7 +106,6 @@ import 'package:analysis_server/src/services/correction/dart/import_library.dart import 'package:analysis_server/src/services/correction/dart/inline_invocation.dart'; import 'package:analysis_server/src/services/correction/dart/inline_typedef.dart'; import 'package:analysis_server/src/services/correction/dart/insert_body.dart'; -import 'package:analysis_server/src/services/correction/dart/insert_on_keyword.dart'; import 'package:analysis_server/src/services/correction/dart/insert_semicolon.dart'; import 'package:analysis_server/src/services/correction/dart/make_class_abstract.dart'; import 'package:analysis_server/src/services/correction/dart/make_conditional_on_debug_mode.dart'; @@ -379,7 +378,6 @@ final _builtInLintProducers = >{ LinterLintCode.null_closures: [ReplaceNullWithClosure.new], LinterLintCode.omit_local_variable_types: [ReplaceWithVar.new], LinterLintCode.omit_obvious_local_variable_types: [ReplaceWithVar.new], - LinterLintCode.omit_obvious_property_types: [ReplaceWithVar.new], LinterLintCode.prefer_adjacent_string_concatenation: [RemoveOperator.new], LinterLintCode.prefer_collection_literals: [ ConvertToMapLiteral.new, @@ -462,9 +460,6 @@ final _builtInLintProducers = >{ LinterLintCode.specify_nonobvious_local_variable_types: [ AddTypeAnnotation.bulkFixable, ], - LinterLintCode.specify_nonobvious_property_types: [ - AddTypeAnnotation.bulkFixable, - ], LinterLintCode.type_annotate_public_apis: [AddTypeAnnotation.bulkFixable], LinterLintCode.type_init_formals: [RemoveTypeAnnotation.other], LinterLintCode.type_literal_in_constant_pattern: [ @@ -531,9 +526,6 @@ final _builtInLintProducers = >{ }; final _builtInNonLintMultiProducers = { - CompileTimeErrorCode.AMBIGUOUS_IMPORT: [ - ImportAddHide.new, - ], CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_TWO: [ AddExtensionOverride.new, ], @@ -541,7 +533,7 @@ final _builtInNonLintMultiProducers = { AddExtensionOverride.new, ], CompileTimeErrorCode.AMBIGUOUS_IMPORT: [ - AmbiguousImportFix.new, + ImportAddHide.new, ], CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [DataDriven.new], CompileTimeErrorCode.CAST_TO_NON_TYPE: [ @@ -1146,11 +1138,7 @@ final _builtInNonLintProducers = >{ ParserErrorCode.EXPECTED_SWITCH_EXPRESSION_BODY: [InsertBody.new], ParserErrorCode.EXPECTED_SWITCH_STATEMENT_BODY: [InsertBody.new], ParserErrorCode.EXPECTED_TRY_STATEMENT_BODY: [InsertBody.new], - ParserErrorCode.EXPECTED_TOKEN: [ - InsertSemicolon.new, - ReplaceWithArrow.new, - InsertOnKeyword.new, - ], + ParserErrorCode.EXPECTED_TOKEN: [InsertSemicolon.new, ReplaceWithArrow.new], ParserErrorCode.EXTENSION_AUGMENTATION_HAS_ON_CLAUSE: [RemoveOnClause.new], ParserErrorCode.EXTENSION_DECLARES_CONSTRUCTOR: [RemoveConstructor.new], ParserErrorCode.EXTERNAL_CLASS: [RemoveLexeme.modifier], From 35adb2c9b7c614d30e6ee108f4c4a4e7bd5d60ca Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Tue, 22 Oct 2024 10:37:24 -0300 Subject: [PATCH 12/28] better handling of different prefixes same imported library --- .../correction/dart/import_add_hide.dart | 11 +++- .../correction/fix/import_add_hide_test.dart | 50 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 38bacd7a3092..6455ccae5129 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -18,10 +18,15 @@ class ImportAddHide extends MultiCorrectionProducer { Future> get producers async { var node = this.node; Element? element; + String? prefix; if (node is NamedType) { element = node.element; + prefix = node.importPrefix?.name.lexeme; } else if (node is SimpleIdentifier) { element = node.staticElement; + if (node.parent case PrefixedIdentifier(prefix: var currentPrefix)) { + prefix = currentPrefix.name; + } } if (element is! MultiplyDefinedElement) { return const []; @@ -37,18 +42,20 @@ class ImportAddHide extends MultiCorrectionProducer { var directives = []; // find all ImportDirective that import this library in this unit + // and have the same prefix for (var directive in unit.directives.whereType()) { // Get import directive that var imported = directive.element?.importedLibrary; if (imported == null) { continue; } - if (imported == library) { + if (imported == library && directive.prefix?.name == prefix) { directives.add(directive); } // If the directive exports the library, then the library is also // imported. - if (imported.exportedLibraries.contains(library)) { + if (imported.exportedLibraries.contains(library) && + directive.prefix?.name == prefix) { directives.add(directive); } } diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 9a58122bbc92..808e37df2053 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -154,6 +154,56 @@ void f() { ''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); } + Future test_tripleImports_oneAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' as lib; +import 'lib1.dart'; +import 'lib2.dart'; + +void f() { + print(foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart' as lib; +import 'lib1.dart'; +import 'lib2.dart' hide foo; + +void f() { + print(foo); +} +''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); + } + + Future test_tripleImports_twoAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib1.dart' as lib; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib1.dart' as lib; +import 'lib2.dart' as lib hide foo; + +void f() { + print(lib.foo); +} +''', matchFixMessage: "Use 'foo' from 'lib1.dart' as lib"); + } + Future test_doubleImports_exportedByImport() async { newFile('$testPackageLibPath/lib1.dart', ''' export 'lib3.dart';'''); From 2cbb3121c2fd20ffafab7848d49dbf12338b4517 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Fri, 1 Nov 2024 18:01:39 -0300 Subject: [PATCH 13/28] makes two fixes - considers discussion --- .../correction/dart/import_add_hide.dart | 136 ++++-- .../lib/src/services/correction/fix.dart | 5 - .../src/services/correction/fix_internal.dart | 2 +- .../correction/fix/import_add_hide_test.dart | 388 +++++++++++------- 4 files changed, 341 insertions(+), 190 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 6455ccae5129..50ab58d2c0b8 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -11,8 +11,8 @@ import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dar import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; import 'package:collection/collection.dart'; -class ImportAddHide extends MultiCorrectionProducer { - ImportAddHide({required super.context}); +class AmbiguousImportFix extends MultiCorrectionProducer { + AmbiguousImportFix({required super.context}); @override Future> get producers async { @@ -81,6 +81,9 @@ class ImportAddHide extends MultiCorrectionProducer { .toSet(); producers.add(_ImportAddHide(key.element, key.uri, key.prefix, directives, context: context)); + producers.add(_ImportRemoveShow( + key.element, key.uri, key.prefix, directives, + context: context)); } return producers; } @@ -102,26 +105,11 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override List get fixArguments { - // Any import directive that imports the element is directly showing it? - var removeShow = false; - for (var directives in importDirectives - .map((d) => d.combinators.whereType()) - .where((d) => d.isNotEmpty)) { - for (var name in directives.first.shownNames) { - if (name.name == _elementName) { - removeShow = true; - break; - } - } - } - // If there is a show combinator explicitly showing the element, then - // we should remove it. - var removeShowStr = removeShow ? ' (removing \'show\')' : ''; var prefix = ''; if ((this.prefix ?? '') != '') { prefix = ' as ${this.prefix}'; } - return [_elementName, uri, prefix, removeShowStr]; + return [_elementName, uri, prefix]; } @override @@ -135,43 +123,103 @@ class _ImportAddHide extends ResolvedCorrectionProducer { return; } + var hideCombinator = + <({ImportDirective directive, HideCombinator? hide})>[]; + for (var directive in importDirectives) { - var show = directive.combinators.whereType(); - var hide = directive.combinators.whereType(); - await builder.addDartFileEdit(file, (builder) { - if (show.isNotEmpty) { - var showCombinator = show.first; - var allNames = []; - for (var show in showCombinator.shownNames) { - if (show.name == _elementName) continue; - allNames.add(show.name); - } - allNames.sort(); - if (allNames.isEmpty) { - builder.addDeletion(SourceRange( - showCombinator.offset - 1, showCombinator.length + 1)); - } else { - var combinator = 'show ${allNames.join(', ')}'; - var range = - SourceRange(showCombinator.offset, showCombinator.length); - builder.addSimpleReplacement(range, combinator); - } - } - if (hide.isNotEmpty) { - var hideCombinator = hide.first; + var show = directive.combinators.whereType().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().firstOrNull; + hideCombinator.add((directive: directive, hide: hide)); + } + + await builder.addDartFileEdit(file, (builder) { + for (var (:directive, :hide) in hideCombinator) { + if (hide != null) { var allNames = [_elementName]; - for (var name in hideCombinator.hiddenNames) { + for (var name in hide.hiddenNames) { allNames.add(name.name); } allNames.sort(); var combinator = 'hide ${allNames.join(', ')}'; - var range = SourceRange(hideCombinator.offset, hideCombinator.length); + var range = SourceRange(hide.offset, hide.length); builder.addSimpleReplacement(range, combinator); } else { var hideCombinator = ' hide $_elementName'; builder.addSimpleInsertion(directive.end - 1, hideCombinator); } - }); + } + }); + } +} + +class _ImportRemoveShow extends ResolvedCorrectionProducer { + final Set importDirectives; + final Element element; + final String uri; + final String? prefix; + + _ImportRemoveShow(this.element, this.uri, this.prefix, this.importDirectives, + {required super.context}); + + @override + CorrectionApplicability get applicability => + // TODO(applicability): comment on why. + CorrectionApplicability.singleLocation; + + @override + List get fixArguments { + var prefix = ''; + if ((this.prefix ?? '') != '') { + prefix = ' as ${this.prefix}'; + } + return [_elementName, uri, prefix]; + } + + @override + FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; + + String get _elementName => element.name ?? ''; + + @override + Future compute(ChangeBuilder builder) async { + if (_elementName.isEmpty || uri.isEmpty) { + return; } + + var showCombinator = <({ImportDirective directive, ShowCombinator show})>[]; + for (var directive in importDirectives) { + var show = directive.combinators.whereType().firstOrNull; + // If there is no show combinator, then we don't want to deal with this + // case here. + if (show == null) { + return; + } + showCombinator.add((directive: directive, show: show)); + } + + await builder.addDartFileEdit(file, (builder) { + for (var (:directive, :show) in showCombinator) { + var allNames = []; + for (var show in show.shownNames) { + if (show.name == _elementName) continue; + allNames.add(show.name); + } + 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 { + var combinator = 'show ${allNames.join(', ')}'; + var range = SourceRange(show.offset, show.length); + builder.addSimpleReplacement(range, combinator); + } + } + }); } } diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index d4e76eac650a..6d30d60989bf 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -884,11 +884,6 @@ abstract final class DartFixKind { DartFixKindPriority.standard + 4, "Import library '{0}' with prefix '{1}'", ); - static const IMPORT_LIBRARY_HIDE = FixKind( - 'dart.fix.import.libraryHide', - DartFixKindPriority.standard, - "Use '{0}' from '{1}'{2}{3}", - ); static const INLINE_INVOCATION = FixKind( 'dart.fix.inlineInvocation', DartFixKindPriority.standard - 20, diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index dab651a8947c..f2ba868dc3b2 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -533,7 +533,7 @@ final _builtInNonLintMultiProducers = { AddExtensionOverride.new, ], CompileTimeErrorCode.AMBIGUOUS_IMPORT: [ - ImportAddHide.new, + AmbiguousImportFix.new, ], CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [DataDriven.new], CompileTimeErrorCode.CAST_TO_NON_TYPE: [ diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 808e37df2053..1e7f7de66ded 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -12,6 +12,7 @@ import 'fix_processor.dart'; void main() { defineReflectiveSuite(() { defineReflectiveTests(ImportAddHideTest); + defineReflectiveTests(ImportRemoveShowTest); }); } @@ -20,7 +21,7 @@ class ImportAddHideTest extends FixProcessorTest { @override FixKind get kind => DartFixKind.IMPORT_LIBRARY_HIDE; - Future test_doubleAliasedImports() async { + Future test_double_aliased() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -36,8 +37,8 @@ void f(i.N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Use 'N' from 'lib1.dart' as i", - "Use 'N' from 'lib2.dart' as i", + "Hide others to use 'N' from 'lib1.dart' as i", + "Hide others to use 'N' from 'lib2.dart' as i", ]); await assertHasFix(''' import 'lib1.dart' as i; @@ -46,7 +47,7 @@ import 'lib2.dart' as i hide N; void f(i.N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib1.dart' as i"); +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart' as i"); await assertHasFix(''' import 'lib1.dart' as i hide N; import 'lib2.dart' as i; @@ -54,10 +55,10 @@ import 'lib2.dart' as i; void f(i.N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib2.dart' as i"); +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart' as i"); } - Future test_DoubleImports() async { + Future test_double() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -73,8 +74,8 @@ void f(N? n) { await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Use 'N' from 'lib1.dart'", - "Use 'N' from 'lib2.dart'", + "Hide others to use 'N' from 'lib1.dart'", + "Hide others to use 'N' from 'lib2.dart'", ]); await assertHasFix(''' import 'lib1.dart' hide N; @@ -83,7 +84,7 @@ import 'lib2.dart'; void f(N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib2.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); await assertHasFix(''' import 'lib1.dart'; import 'lib2.dart' hide N; @@ -91,47 +92,10 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); } - Future test_doubleImports_bothShow() async { - newFile('$testPackageLibPath/lib1.dart', ''' -class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' -class N {}'''); - await resolveTestCode(''' -import 'lib1.dart' show N; -import 'lib2.dart' show N; - -void f(N? n) { - print(n); -} -'''); - await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 2, - matchFixMessages: [ - "Use 'N' from 'lib1.dart' (removing 'show')", - "Use 'N' from 'lib2.dart' (removing 'show')", - ]); - await assertHasFix(''' -import 'lib1.dart' hide N; -import 'lib2.dart' show N; - -void f(N? n) { - print(n); -} -''', matchFixMessage: "Use 'N' from 'lib2.dart' (removing 'show')"); - await assertHasFix(''' -import 'lib1.dart' show N; -import 'lib2.dart' hide N; - -void f(N? n) { - print(n); -} -''', matchFixMessage: "Use 'N' from 'lib1.dart' (removing 'show')"); - } - - Future test_doubleImports_constant() async { + Future test_double_constant() async { newFile('$testPackageLibPath/lib1.dart', ''' const foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -151,60 +115,10 @@ import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); } - Future test_tripleImports_oneAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart' as lib; -import 'lib1.dart'; -import 'lib2.dart'; - -void f() { - print(foo); -} -'''); - await assertHasFix(''' -import 'lib1.dart' as lib; -import 'lib1.dart'; -import 'lib2.dart' hide foo; - -void f() { - print(foo); -} -''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); - } - - Future test_tripleImports_twoAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart'; -import 'lib1.dart' as lib; -import 'lib2.dart' as lib; - -void f() { - print(lib.foo); -} -'''); - await assertHasFix(''' -import 'lib1.dart'; -import 'lib1.dart' as lib; -import 'lib2.dart' as lib hide foo; - -void f() { - print(lib.foo); -} -''', matchFixMessage: "Use 'foo' from 'lib1.dart' as lib"); - } - - Future test_doubleImports_exportedByImport() async { + Future test_double_exportedByImport() async { newFile('$testPackageLibPath/lib1.dart', ''' export 'lib3.dart';'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -222,12 +136,13 @@ import 'lib1.dart' hide M; import 'lib2.dart'; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib2.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib2.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } - Future test_doubleImports_extension() async { + Future test_double_extension() async { newFile('$testPackageLibPath/lib1.dart', ''' extension E on int { bool get isDivisibleByThree => this % 3 == 0; @@ -251,12 +166,13 @@ import 'lib2.dart' hide E; void foo(int i) { print(E(i.isDivisibleByThree)); } -''', matchFixMessage: "Use 'E' from 'lib1.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'E' from 'lib1.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } - Future test_doubleImports_function() async { + Future test_double_function() async { newFile('$testPackageLibPath/lib1.dart', ''' void bar() {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -276,10 +192,10 @@ import 'lib2.dart' hide bar; void foo() { bar(); } -''', matchFixMessage: "Use 'bar' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'bar' from 'lib1.dart'"); } - Future test_doubleImports_mixin() async { + Future test_double_mixin() async { newFile('$testPackageLibPath/lib1.dart', ''' mixin M {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -295,12 +211,13 @@ import 'lib1.dart'; import 'lib2.dart' hide M; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib1.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } - Future test_doubleImports_oneHide() async { + Future test_double_oneHide() async { newFile('$testPackageLibPath/lib1.dart', ''' class M {} class N {} class O {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -320,10 +237,10 @@ import 'lib2.dart'; void f(N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib2.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); } - Future test_doubleImports_oneShow() async { + Future test_double_oneShow() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -343,10 +260,10 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', matchFixMessage: "Use 'N' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); } - Future test_doubleImports_variable() async { + Future test_double_variable() async { newFile('$testPackageLibPath/lib1.dart', ''' var foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -366,7 +283,7 @@ import 'lib2.dart' hide foo; void f() { print(foo); } -''', matchFixMessage: "Use 'foo' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); } Future test_show_prefixed() async { @@ -376,37 +293,16 @@ class N {}'''); class N {}'''); await resolveTestCode(''' import 'lib1.dart' as l show N; -import 'lib2.dart' as l show N; +import 'lib2.dart' as l; void f(l.N? n) { print(n); } '''); - await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 2, - matchFixMessages: [ - "Use 'N' from 'lib1.dart' as l (removing 'show')", - "Use 'N' from 'lib2.dart' as l (removing 'show')", - ]); - await assertHasFix(''' -import 'lib1.dart' as l hide N; -import 'lib2.dart' as l show N; - -void f(l.N? n) { - print(n); -} -''', matchFixMessage: "Use 'N' from 'lib2.dart' as l (removing 'show')"); - await assertHasFix(''' -import 'lib1.dart' as l show N; -import 'lib2.dart' as l hide N; - -void f(l.N? n) { - print(n); -} -''', matchFixMessage: "Use 'N' from 'lib1.dart' as l (removing 'show')"); + await assertNoFix(); } - Future test_tripleImports() async { + Future test_triple() async { newFile('$testPackageLibPath/lib1.dart', ''' export 'lib3.dart';'''); newFile('$testPackageLibPath/lib2.dart', ''' @@ -426,7 +322,8 @@ import 'lib2.dart' hide M; import 'lib3.dart' hide M; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib1.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); await assertHasFix(''' @@ -435,7 +332,8 @@ import 'lib2.dart'; import 'lib3.dart' hide M; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib2.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib2.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); await assertHasFix(''' @@ -444,8 +342,218 @@ import 'lib2.dart' hide M; import 'lib3.dart'; class C with M {} -''', matchFixMessage: "Use 'M' from 'lib3.dart'", errorFilter: (error) { +''', matchFixMessage: "Hide others to use 'M' from 'lib3.dart'", + errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } + + Future test_triple_oneAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' as lib; +import 'lib1.dart'; +import 'lib2.dart'; + +void f() { + print(foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart' as lib; +import 'lib1.dart'; +import 'lib2.dart' hide foo; + +void f() { + print(foo); +} +''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); + } + + Future test_triple_twoAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib1.dart' as lib; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib1.dart' as lib; +import 'lib2.dart' as lib hide foo; + +void f() { + print(lib.foo); +} +''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart' as lib"); + } +} + +@reflectiveTest +class ImportRemoveShowTest extends FixProcessorTest { + @override + FixKind get kind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; + + Future test_triple_twoAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib1.dart' as lib show foo; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib1.dart' as lib hide foo; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +''', matchFixMessage: "Remove show to use 'foo' from 'lib1.dart' as lib"); + } + + Future test_double_oneHide() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' hide foo; +import 'lib2.dart' show foo; + +void f() { + print(foo); +} +'''); + await assertNoFix(); + } + + Future test_double_aliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +'''); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Remove show to use 'N' from 'lib1.dart' as l", + "Remove show to use 'N' from 'lib2.dart' as l", + ]); + await assertHasFix(''' +import 'lib1.dart' as l hide N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart' as l"); + await assertHasFix(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l hide N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Remove show to use 'N' from 'lib1.dart' as l"); + } + + Future test_double() async { + newFile('$testPackageLibPath/lib1.dart', ''' +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' show N; +import 'lib2.dart' show N; + +void f(l.N? n) { + print(n); +} +'''); + await assertHasFix(''' +import 'lib1.dart' as l hide N; +import 'lib2.dart' as l show N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart' as l"); + } + + Future test_moreShow() async { + newFile('$testPackageLibPath/lib1.dart', ''' +class N {} +class M {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' show N, M; +import 'lib2.dart'; + +void f(N? n) { + print(n); +} +'''); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 1, + matchFixMessages: [ + "Remove show to use 'N' from 'lib2.dart'", + ]); + await assertHasFix(''' +import 'lib1.dart' show M; +import 'lib2.dart'; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart'"); + } + + Future test_one_show() async { + newFile('$testPackageLibPath/lib1.dart', ''' +var foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +var foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' show foo; +import 'lib2.dart'; + +void f() { + print(foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart' hide foo; +import 'lib2.dart'; + +void f() { + print(foo); +} +''', matchFixMessage: "Remove show to use 'foo' from 'lib2.dart'"); + } } From 1ab3585db0a2adc2b36bb23d6fa09b683f9a1f79 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 14 Nov 2024 09:01:59 -0300 Subject: [PATCH 14/28] refactors - review --- .../correction/dart/import_add_hide.dart | 110 ++++++++++-------- 1 file changed, 61 insertions(+), 49 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 50ab58d2c0b8..e303a318266a 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -5,75 +5,88 @@ import 'package:analysis_server/src/services/correction/fix.dart'; import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/element/element.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_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; import 'package:collection/collection.dart'; +typedef _CorrectionProducerParameters = ({ + Element2 element, + String uri, + String? prefix, +}); + class AmbiguousImportFix extends MultiCorrectionProducer { AmbiguousImportFix({required super.context}); @override Future> get producers async { var node = this.node; - Element? element; + Element2? element; String? prefix; if (node is NamedType) { - element = node.element; + element = node.element2; prefix = node.importPrefix?.name.lexeme; } else if (node is SimpleIdentifier) { - element = node.staticElement; + element = node.element; if (node.parent case PrefixedIdentifier(prefix: var currentPrefix)) { prefix = currentPrefix.name; } } - if (element is! MultiplyDefinedElement) { + if (element is! MultiplyDefinedElement2) { return const []; } - var elements = element.conflictingElements; - var records = <({Element element, String uri, String? prefix}), - List>{}; - for (var element in elements) { - var library = element.enclosingElement3?.library; + var conflictingElements = element.conflictingElements2; + + // This stores the parameters for the CorrectionProducers along with the + // list of ImportDirectives that import the declaration. + var fixes = <_CorrectionProducerParameters, List>{}; + for (var conflictingElement in conflictingElements) { + var library = conflictingElement.enclosingElement2?.library2; if (library == null) { continue; } var directives = []; - // find all ImportDirective that import this library in this unit - // and have the same prefix + // Find all ImportDirective that import this library in this unit + // and have the same prefix. for (var directive in unit.directives.whereType()) { - // Get import directive that - var imported = directive.element?.importedLibrary; + var imported = directive.libraryImport?.importedLibrary2; if (imported == null) { continue; } - if (imported == library && directive.prefix?.name == prefix) { - directives.add(directive); + // If the prefix is different, then this directive is not relevant. + if (directive.prefix?.name != prefix) { + continue; } - // If the directive exports the library, then the library is also - // imported. - if (imported.exportedLibraries.contains(library) && - directive.prefix?.name == prefix) { + + // If this library is imported directly or if the directive exports the + // library for this element. + if (imported == library || + imported.exportedLibraries2.contains(library)) { directives.add(directive); } } + for (var directive in directives) { var uri = directive.uri.stringValue; var prefix = directive.prefix?.name; if (uri != null) { - records[(element: element, uri: uri, prefix: prefix)] = directives; + var key = (element: conflictingElement, uri: uri, prefix: prefix); + fixes[key] = directives; } } } - if (records.entries.isEmpty) { + + if (fixes.isEmpty) { return const []; } var producers = []; - for (var MapEntry(key: key) in records.entries) { - var directives = records.entries + for (var key in fixes.keys) { + var directives = fixes.entries .whereNot((e) => e.key == key) .expand((e) => e.value) .whereNot((d) => @@ -91,7 +104,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { class _ImportAddHide extends ResolvedCorrectionProducer { final Set importDirectives; - final Element element; + final Element2 element; final String uri; final String? prefix; @@ -106,7 +119,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override List get fixArguments { var prefix = ''; - if ((this.prefix ?? '') != '') { + if (!this.prefix.isEmptyOrNull) { prefix = ' as ${this.prefix}'; } return [_elementName, uri, prefix]; @@ -115,7 +128,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; - String get _elementName => element.name ?? ''; + String get _elementName => element.name3 ?? ''; @override Future compute(ChangeBuilder builder) async { @@ -123,28 +136,27 @@ class _ImportAddHide extends ResolvedCorrectionProducer { return; } - var hideCombinator = + var hideCombinators = <({ImportDirective directive, HideCombinator? hide})>[]; for (var directive in importDirectives) { var show = directive.combinators.whereType().firstOrNull; - // If there is an import with a show combinator, then we don't want to + // 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().firstOrNull; - hideCombinator.add((directive: directive, hide: hide)); + hideCombinators.add((directive: directive, hide: hide)); } await builder.addDartFileEdit(file, (builder) { - for (var (:directive, :hide) in hideCombinator) { + for (var (:directive, :hide) in hideCombinators) { if (hide != null) { - var allNames = [_elementName]; - for (var name in hide.hiddenNames) { - allNames.add(name.name); - } - allNames.sort(); + var allNames = [ + _elementName, + ...hide.hiddenNames.map((name) => name.name), + ]..sort(); var combinator = 'hide ${allNames.join(', ')}'; var range = SourceRange(hide.offset, hide.length); builder.addSimpleReplacement(range, combinator); @@ -159,7 +171,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { class _ImportRemoveShow extends ResolvedCorrectionProducer { final Set importDirectives; - final Element element; + final Element2 element; final String uri; final String? prefix; @@ -174,7 +186,7 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { @override List get fixArguments { var prefix = ''; - if ((this.prefix ?? '') != '') { + if (!this.prefix.isEmptyOrNull) { prefix = ' as ${this.prefix}'; } return [_elementName, uri, prefix]; @@ -183,7 +195,7 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { @override FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; - String get _elementName => element.name ?? ''; + String get _elementName => element.name3 ?? ''; @override Future compute(ChangeBuilder builder) async { @@ -191,25 +203,25 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { return; } - var showCombinator = <({ImportDirective directive, ShowCombinator show})>[]; + var showCombinators = + <({ImportDirective directive, ShowCombinator show})>[]; for (var directive in importDirectives) { var show = directive.combinators.whereType().firstOrNull; - // If there is no show combinator, then we don't want to deal with this + // If there is no show combinator, then we don't want to deal with this // case here. if (show == null) { return; } - showCombinator.add((directive: directive, show: show)); + showCombinators.add((directive: directive, show: show)); } await builder.addDartFileEdit(file, (builder) { - for (var (:directive, :show) in showCombinator) { - var allNames = []; - for (var show in show.shownNames) { - if (show.name == _elementName) continue; - allNames.add(show.name); - } - allNames.sort(); + for (var (:directive, :show) in showCombinators) { + var allNames = [ + ...show.shownNames + .map((name) => name.name) + .where((name) => name != _elementName), + ]..sort(); if (allNames.isEmpty) { builder.addDeletion(SourceRange(show.offset - 1, show.length + 1)); var hideCombinator = ' hide $_elementName'; From 06dac3fd1334045c73eac7501582e245771cbc1f Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 14 Nov 2024 13:39:28 -0300 Subject: [PATCH 15/28] sorts test file --- .../correction/fix/import_add_hide_test.dart | 155 +++++++++--------- 1 file changed, 81 insertions(+), 74 deletions(-) diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 1e7f7de66ded..74db33312ff4 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -21,78 +21,78 @@ class ImportAddHideTest extends FixProcessorTest { @override FixKind get kind => DartFixKind.IMPORT_LIBRARY_HIDE; - Future test_double_aliased() async { + Future test_double() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' class N {}'''); await resolveTestCode(''' -import 'lib1.dart' as i; -import 'lib2.dart' as i; +import 'lib1.dart'; +import 'lib2.dart'; -void f(i.N? n) { +void f(N? n) { print(n); } '''); await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Hide others to use 'N' from 'lib1.dart' as i", - "Hide others to use 'N' from 'lib2.dart' as i", + "Hide others to use 'N' from 'lib1.dart'", + "Hide others to use 'N' from 'lib2.dart'", ]); await assertHasFix(''' -import 'lib1.dart' as i; -import 'lib2.dart' as i hide N; +import 'lib1.dart' hide N; +import 'lib2.dart'; -void f(i.N? n) { +void f(N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart' as i"); +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); await assertHasFix(''' -import 'lib1.dart' as i hide N; -import 'lib2.dart' as i; +import 'lib1.dart'; +import 'lib2.dart' hide N; -void f(i.N? n) { +void f(N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart' as i"); +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); } - Future test_double() async { + Future test_double_aliased() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' class N {}'''); await resolveTestCode(''' -import 'lib1.dart'; -import 'lib2.dart'; +import 'lib1.dart' as i; +import 'lib2.dart' as i; -void f(N? n) { +void f(i.N? n) { print(n); } '''); await assertHasFixesWithoutApplying( expectedNumberOfFixesForKind: 2, matchFixMessages: [ - "Hide others to use 'N' from 'lib1.dart'", - "Hide others to use 'N' from 'lib2.dart'", + "Hide others to use 'N' from 'lib1.dart' as i", + "Hide others to use 'N' from 'lib2.dart' as i", ]); await assertHasFix(''' -import 'lib1.dart' hide N; -import 'lib2.dart'; +import 'lib1.dart' as i; +import 'lib2.dart' as i hide N; -void f(N? n) { +void f(i.N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart' as i"); await assertHasFix(''' -import 'lib1.dart'; -import 'lib2.dart' hide N; +import 'lib1.dart' as i hide N; +import 'lib2.dart' as i; -void f(N? n) { +void f(i.N? n) { print(n); } -''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart' as i"); } Future test_double_constant() async { @@ -299,7 +299,14 @@ void f(l.N? n) { print(n); } '''); - await assertNoFix(); + await assertHasFix(''' +import 'lib1.dart' as l show N; +import 'lib2.dart' as l hide N; + +void f(l.N? n) { + print(n); +} +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart' as l"); } Future test_triple() async { @@ -404,45 +411,27 @@ class ImportRemoveShowTest extends FixProcessorTest { @override FixKind get kind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; - Future test_triple_twoAliased() async { + Future test_double() async { newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); +class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); +class N {}'''); await resolveTestCode(''' -import 'lib1.dart'; -import 'lib1.dart' as lib show foo; -import 'lib2.dart' as lib; +import 'lib1.dart' show N; +import 'lib2.dart' show N; -void f() { - print(lib.foo); +void f(N? n) { + print(n); } '''); await assertHasFix(''' -import 'lib1.dart'; -import 'lib1.dart' as lib hide foo; -import 'lib2.dart' as lib; - -void f() { - print(lib.foo); -} -''', matchFixMessage: "Remove show to use 'foo' from 'lib1.dart' as lib"); - } - - Future test_double_oneHide() async { - newFile('$testPackageLibPath/lib1.dart', ''' -const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -const foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart' hide foo; -import 'lib2.dart' show foo; +import 'lib1.dart' hide N; +import 'lib2.dart' show N; -void f() { - print(foo); +void f(N? n) { + print(n); } -'''); - await assertNoFix(); +''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart'"); } Future test_double_aliased() async { @@ -482,27 +471,20 @@ void f(l.N? n) { ''', matchFixMessage: "Remove show to use 'N' from 'lib1.dart' as l"); } - Future test_double() async { + Future test_double_oneHide() async { newFile('$testPackageLibPath/lib1.dart', ''' -class N {}'''); +const foo = 0;'''); newFile('$testPackageLibPath/lib2.dart', ''' -class N {}'''); +const foo = 0;'''); await resolveTestCode(''' -import 'lib1.dart' show N; -import 'lib2.dart' show N; +import 'lib1.dart' hide foo; +import 'lib2.dart' show foo; -void f(l.N? n) { - print(n); +void f() { + print(foo); } '''); - await assertHasFix(''' -import 'lib1.dart' as l hide N; -import 'lib2.dart' as l show N; - -void f(l.N? n) { - print(n); -} -''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart' as l"); + await assertNoFix(); } Future test_moreShow() async { @@ -556,4 +538,29 @@ void f() { } ''', matchFixMessage: "Remove show to use 'foo' from 'lib2.dart'"); } + + Future test_triple_twoAliased() async { + newFile('$testPackageLibPath/lib1.dart', ''' +const foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +const foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib1.dart' as lib show foo; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart'; +import 'lib1.dart' as lib hide foo; +import 'lib2.dart' as lib; + +void f() { + print(lib.foo); +} +''', matchFixMessage: "Remove show to use 'foo' from 'lib2.dart' as lib"); + } } From 06e18233ac107853de7b9833297950b1e86ba567 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:17:53 -0300 Subject: [PATCH 16/28] refactors to simplify the producers calculation - review --- .../correction/dart/import_add_hide.dart | 73 ++++++++----------- 1 file changed, 30 insertions(+), 43 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index e303a318266a..27ce38cd090e 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -12,12 +12,6 @@ import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dar import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; import 'package:collection/collection.dart'; -typedef _CorrectionProducerParameters = ({ - Element2 element, - String uri, - String? prefix, -}); - class AmbiguousImportFix extends MultiCorrectionProducer { AmbiguousImportFix({required super.context}); @@ -39,20 +33,24 @@ class AmbiguousImportFix extends MultiCorrectionProducer { return const []; } var conflictingElements = element.conflictingElements2; + var name = element.name3; + if (name == null || name.isEmpty) { + return const []; + } - // This stores the parameters for the CorrectionProducers along with the - // list of ImportDirectives that import the declaration. - var fixes = <_CorrectionProducerParameters, List>{}; + var uris = {}; + var importDirectives = {}; for (var conflictingElement in conflictingElements) { var library = conflictingElement.enclosingElement2?.library2; if (library == null) { continue; } - var directives = []; // Find all ImportDirective that import this library in this unit // and have the same prefix. - for (var directive in unit.directives.whereType()) { + for (var directive in unit.directives + .whereType() + .whereNot(importDirectives.contains)) { var imported = directive.libraryImport?.importedLibrary2; if (imported == null) { continue; @@ -66,49 +64,41 @@ class AmbiguousImportFix extends MultiCorrectionProducer { // library for this element. if (imported == library || imported.exportedLibraries2.contains(library)) { - directives.add(directive); - } - } - - for (var directive in directives) { - var uri = directive.uri.stringValue; - var prefix = directive.prefix?.name; - if (uri != null) { - var key = (element: conflictingElement, uri: uri, prefix: prefix); - fixes[key] = directives; + var uri = directive.uri.stringValue; + if (uri != null) { + uris.add(uri); + importDirectives.add(directive); + } } } } - if (fixes.isEmpty) { + if (uris.isEmpty) { return const []; } - var producers = []; - for (var key in fixes.keys) { - var directives = fixes.entries - .whereNot((e) => e.key == key) - .expand((e) => e.value) - .whereNot((d) => - (d.prefix?.name == key.prefix) && (d.uri.stringValue == key.uri)) + var producers = {}; + for (var uri in uris) { + var directives = importDirectives + .where((directive) => directive.uri.stringValue != uri) .toSet(); - producers.add(_ImportAddHide(key.element, key.uri, key.prefix, directives, - context: context)); - producers.add(_ImportRemoveShow( - key.element, key.uri, key.prefix, directives, - context: context)); + producers.addAll([ + _ImportAddHide(name, uri, prefix, directives, context: context), + _ImportRemoveShow(name, uri, prefix, directives, context: context) + ]); } - return producers; + return producers.toList(); } } class _ImportAddHide extends ResolvedCorrectionProducer { final Set importDirectives; - final Element2 element; final String uri; final String? prefix; + final String _elementName; - _ImportAddHide(this.element, this.uri, this.prefix, this.importDirectives, + _ImportAddHide( + this._elementName, this.uri, this.prefix, this.importDirectives, {required super.context}); @override @@ -128,8 +118,6 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_HIDE; - String get _elementName => element.name3 ?? ''; - @override Future compute(ChangeBuilder builder) async { if (_elementName.isEmpty || uri.isEmpty) { @@ -171,11 +159,12 @@ class _ImportAddHide extends ResolvedCorrectionProducer { class _ImportRemoveShow extends ResolvedCorrectionProducer { final Set importDirectives; - final Element2 element; + final String _elementName; final String uri; final String? prefix; - _ImportRemoveShow(this.element, this.uri, this.prefix, this.importDirectives, + _ImportRemoveShow( + this._elementName, this.uri, this.prefix, this.importDirectives, {required super.context}); @override @@ -195,8 +184,6 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { @override FixKind get fixKind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; - String get _elementName => element.name3 ?? ''; - @override Future compute(ChangeBuilder builder) async { if (_elementName.isEmpty || uri.isEmpty) { From 19e133fc6efbdbf87457a9c3967636ee0064a642 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Tue, 19 Nov 2024 14:44:10 -0300 Subject: [PATCH 17/28] adds support for part files fixing --- .../correction/dart/import_add_hide.dart | 117 ++++++++++++------ .../correction/fix/import_add_hide_test.dart | 47 +++++++ 2 files changed, 124 insertions(+), 40 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 27ce38cd090e..3950a3f56ac4 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -4,10 +4,12 @@ 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:collection/collection.dart'; @@ -38,56 +40,91 @@ class AmbiguousImportFix extends MultiCorrectionProducer { return const []; } - var uris = {}; - var importDirectives = {}; - for (var conflictingElement in conflictingElements) { - var library = conflictingElement.enclosingElement2?.library2; - if (library == null) { - continue; + var (uris, importDirectives) = _getImportDirectives( + libraryResult, + unitResult, + conflictingElements, + prefix, + ); + + var producers = {}; + for (var uri in uris) { + for (var MapEntry(:key, :value) in importDirectives.entries) { + value = value + .whereNot((directive) => directive.uri.stringValue == uri) + .toSet(); + var thisContext = CorrectionProducerContext.createResolved( + libraryResult: libraryResult, + unitResult: key, + applyingBulkFixes: applyingBulkFixes, + dartFixContext: context.dartFixContext, + diagnostic: diagnostic, + selectionLength: selectionLength, + selectionOffset: selectionOffset, + ); + producers.addAll([ + _ImportAddHide(name, uri, prefix, value, context: thisContext), + _ImportRemoveShow(name, uri, prefix, value, context: thisContext) + ]); } + } + return producers.toList(); + } - // Find all ImportDirective that import this library in this unit - // and have the same prefix. - for (var directive in unit.directives - .whereType() - .whereNot(importDirectives.contains)) { - var imported = directive.libraryImport?.importedLibrary2; - if (imported == null) { - continue; - } - // If the prefix is different, then this directive is not relevant. - if (directive.prefix?.name != prefix) { + /// Returns [ImportDirective]s that import the given [conflictingElements] + /// into [unitResult]. + (Set, Map>) + _getImportDirectives( + ResolvedLibraryResult libraryResult, + ResolvedUnitResult? unitResult, + List conflictingElements, + String? prefix, + ) { + var uris = {}; + var importDirectives = >{}; + // Search in each unit up the chain for related imports. + while (unitResult is ResolvedUnitResult) { + for (var conflictingElement in conflictingElements) { + var library = conflictingElement.enclosingElement2?.library2; + if (library == null) { continue; } - // If this library is imported directly or if the directive exports the - // library for this element. - if (imported == library || - imported.exportedLibraries2.contains(library)) { - var uri = directive.uri.stringValue; - if (uri != null) { - uris.add(uri); - importDirectives.add(directive); + // Find all ImportDirective that import this library in this unit + // and have the same prefix. + for (var directive in unitResult.unit.directives + .whereType() + .whereNot(importDirectives.containsValue)) { + var imported = directive.libraryImport?.importedLibrary2; + if (imported == 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 (imported == library || + imported.exportedLibraries2.contains(library)) { + var uri = directive.uri.stringValue; + if (uri != null) { + uris.add(uri); + if (importDirectives[unitResult] == null) { + importDirectives[unitResult] = {directive}; + } else { + importDirectives[unitResult]?.add(directive); + } + } } } } + // We continue up the chain. + unitResult = libraryResult.parentUnitOf(unitResult); } - if (uris.isEmpty) { - return const []; - } - - var producers = {}; - for (var uri in uris) { - var directives = importDirectives - .where((directive) => directive.uri.stringValue != uri) - .toSet(); - producers.addAll([ - _ImportAddHide(name, uri, prefix, directives, context: context), - _ImportRemoveShow(name, uri, prefix, directives, context: context) - ]); - } - return producers.toList(); + return (uris, importDirectives); } } diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 74db33312ff4..435882462c23 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -263,6 +263,53 @@ void f(N? n) { ''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); } + Future test_double_part() async { + newFile('$testPackageLibPath/lib1.dart', ''' +class N {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + newFile('$testPackageLibPath/imports.dart', ''' +import 'lib1.dart'; +import 'lib2.dart'; + +part 'test.dart'; +'''); + await resolveTestCode(''' +part of 'imports.dart'; + +void f(N? n) { + print(n); +} +'''); + await assertHasFixesWithoutApplying( + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Hide others to use 'N' from 'lib1.dart'", + "Hide others to use 'N' from 'lib2.dart'", + ], + ); + await assertHasFix( + ''' +import 'lib1.dart' hide N; +import 'lib2.dart'; + +part 'test.dart'; +''', + target: '$testPackageLibPath/imports.dart', + matchFixMessage: "Hide others to use 'N' from 'lib2.dart'", + ); + await assertHasFix( + ''' +import 'lib1.dart'; +import 'lib2.dart' hide N; + +part 'test.dart'; +''', + target: '$testPackageLibPath/imports.dart', + matchFixMessage: "Hide others to use 'N' from 'lib1.dart'", + ); + } + Future test_double_variable() async { newFile('$testPackageLibPath/lib1.dart', ''' var foo = 0;'''); From ff0596947877d26c09af4786b23f68c49c3927b9 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Fri, 22 Nov 2024 09:45:50 -0300 Subject: [PATCH 18/28] new multi level part tests --- .../correction/dart/import_add_hide.dart | 60 ++++---- .../correction/fix/import_add_hide_test.dart | 133 ++++++++++++++---- 2 files changed, 133 insertions(+), 60 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart index 3950a3f56ac4..fa742020a66f 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart @@ -40,48 +40,49 @@ class AmbiguousImportFix extends MultiCorrectionProducer { return const []; } - var (uris, importDirectives) = _getImportDirectives( + var (uris, unit, importDirectives) = _getImportDirectives( libraryResult, unitResult, conflictingElements, prefix, ); + if (unit == null || importDirectives.isEmpty || uris.isEmpty) { + return const []; + } + var producers = {}; for (var uri in uris) { - for (var MapEntry(:key, :value) in importDirectives.entries) { - value = value - .whereNot((directive) => directive.uri.stringValue == uri) - .toSet(); - var thisContext = CorrectionProducerContext.createResolved( - libraryResult: libraryResult, - unitResult: key, - applyingBulkFixes: applyingBulkFixes, - dartFixContext: context.dartFixContext, - diagnostic: diagnostic, - selectionLength: selectionLength, - selectionOffset: selectionOffset, - ); - producers.addAll([ - _ImportAddHide(name, uri, prefix, value, context: thisContext), - _ImportRemoveShow(name, uri, prefix, value, context: thisContext) - ]); - } + var thisContext = CorrectionProducerContext.createResolved( + libraryResult: libraryResult, + unitResult: unit, + applyingBulkFixes: applyingBulkFixes, + dartFixContext: context.dartFixContext, + diagnostic: diagnostic, + selectionLength: selectionLength, + selectionOffset: selectionOffset, + ); + var directives = importDirectives + .whereNot((directive) => directive.uri.stringValue == uri) + .toSet(); + producers.addAll([ + _ImportAddHide(name, uri, prefix, directives, context: thisContext), + _ImportRemoveShow(name, uri, prefix, directives, context: thisContext), + ]); } return producers.toList(); } /// Returns [ImportDirective]s that import the given [conflictingElements] /// into [unitResult]. - (Set, Map>) - _getImportDirectives( + (Set, ResolvedUnitResult?, Set) _getImportDirectives( ResolvedLibraryResult libraryResult, ResolvedUnitResult? unitResult, List conflictingElements, String? prefix, ) { var uris = {}; - var importDirectives = >{}; + var importDirectives = {}; // Search in each unit up the chain for related imports. while (unitResult is ResolvedUnitResult) { for (var conflictingElement in conflictingElements) { @@ -94,7 +95,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { // and have the same prefix. for (var directive in unitResult.unit.directives .whereType() - .whereNot(importDirectives.containsValue)) { + .whereNot(importDirectives.contains)) { var imported = directive.libraryImport?.importedLibrary2; if (imported == null) { continue; @@ -111,20 +112,21 @@ class AmbiguousImportFix extends MultiCorrectionProducer { var uri = directive.uri.stringValue; if (uri != null) { uris.add(uri); - if (importDirectives[unitResult] == null) { - importDirectives[unitResult] = {directive}; - } else { - importDirectives[unitResult]?.add(directive); - } + importDirectives.add(directive); } } } } + + if (importDirectives.isNotEmpty) { + break; + } + // We continue up the chain. unitResult = libraryResult.parentUnitOf(unitResult); } - return (uris, importDirectives); + return (uris, unitResult, importDirectives); } } diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index 435882462c23..f8045cb8fa63 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -263,19 +263,109 @@ void f(N? n) { ''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); } - Future test_double_part() async { + Future test_double_variable() async { + newFile('$testPackageLibPath/lib1.dart', ''' +var foo = 0;'''); + newFile('$testPackageLibPath/lib2.dart', ''' +var foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' show foo; +import 'lib2.dart'; + +void f() { + print(foo); +} +'''); + await assertHasFix(''' +import 'lib1.dart' show foo; +import 'lib2.dart' hide foo; + +void f() { + print(foo); +} +''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); + } + + @FailingTest(reason: 'No error produced') + Future test_multiLevelParts() async { + // Create a tree of files that all import 'dart:math' and ensure we find + // only the import from the parent (not a grandparent, sibling, or child). + // + // - lib declares Random + // + // - root has import + // - level1_other has import + // - level1 has imports, is the used reference + // - level2_other has import + // - test has reference <-- testing this + // - level3_other has import + + newFile('$testPackageLibPath/lib.dart', ''' +class Random {} +'''); + + newFile('$testPackageLibPath/root.dart', ''' +import 'dart:math'; +part 'level1_other.dart'; +part 'level1.dart'; +'''); + + newFile('$testPackageLibPath/level1_other.dart', ''' +part of 'root.dart'; +import 'dart:math'; +'''); + + newFile('$testPackageLibPath/level1.dart', ''' +part of 'root.dart'; +import 'dart:math'; +import 'lib.dart'; +part 'level2_other.dart'; +part 'test.dart'; +'''); + + newFile('$testPackageLibPath/level2_other.dart', ''' +part of 'level1.dart'; +import 'dart:math'; +'''); + + newFile('$testPackageLibPath/level3_other.dart', ''' +part of 'test.dart'; +import 'dart:math'; +'''); + + await resolveTestCode(''' +part of 'level1.dart'; +part 'level3_other.dart'; + +Random? r; +'''); + + await assertHasFix( + ''' +part of 'root.dart'; +import 'dart:math'; +import 'lib.dart'; +part 'level2_other.dart'; +part 'test.dart'; +''', + target: 'lib.dart', + ); + } + + Future test_part() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); newFile('$testPackageLibPath/lib2.dart', ''' class N {}'''); - newFile('$testPackageLibPath/imports.dart', ''' + newFile('$testPackageLibPath/other.dart', ''' import 'lib1.dart'; import 'lib2.dart'; - part 'test.dart'; '''); await resolveTestCode(''' -part of 'imports.dart'; +part of 'other.dart'; +import 'lib1.dart'; +import 'lib2.dart'; void f(N? n) { print(n); @@ -290,49 +380,30 @@ void f(N? n) { ); await assertHasFix( ''' +part of 'other.dart'; import 'lib1.dart' hide N; import 'lib2.dart'; -part 'test.dart'; +void f(N? n) { + print(n); +} ''', - target: '$testPackageLibPath/imports.dart', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'", ); await assertHasFix( ''' +part of 'other.dart'; import 'lib1.dart'; import 'lib2.dart' hide N; -part 'test.dart'; +void f(N? n) { + print(n); +} ''', - target: '$testPackageLibPath/imports.dart', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'", ); } - Future test_double_variable() async { - newFile('$testPackageLibPath/lib1.dart', ''' -var foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' -var foo = 0;'''); - await resolveTestCode(''' -import 'lib1.dart' show foo; -import 'lib2.dart'; - -void f() { - print(foo); -} -'''); - await assertHasFix(''' -import 'lib1.dart' show foo; -import 'lib2.dart' hide foo; - -void f() { - print(foo); -} -''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); - } - Future test_show_prefixed() async { newFile('$testPackageLibPath/lib1.dart', ''' class N {}'''); From 5c4f7338f6cb50c0b89c00fd4ca4beb8f5b1f895 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Fri, 22 Nov 2024 09:58:13 -0300 Subject: [PATCH 19/28] expected result for failing test --- .../src/services/correction/fix/import_add_hide_test.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index f8045cb8fa63..fdfa7154db70 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -343,12 +343,13 @@ Random? r; await assertHasFix( ''' part of 'root.dart'; -import 'dart:math'; +import 'dart:math' hide Random; import 'lib.dart'; part 'level2_other.dart'; part 'test.dart'; ''', - target: 'lib.dart', + target: '$testPackageLibPath/level1.dart', + matchFixMessage: "Hide others to use 'Random' from 'lib.dart'", ); } From d3762faf06889c6136b5d610defa97149e449195 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Fri, 22 Nov 2024 10:56:40 -0300 Subject: [PATCH 20/28] fixes previously failing test --- .../correction/fix/import_add_hide_test.dart | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart index fdfa7154db70..bebf15af984e 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart @@ -286,12 +286,12 @@ void f() { ''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); } - @FailingTest(reason: 'No error produced') Future test_multiLevelParts() async { // Create a tree of files that all import 'dart:math' and ensure we find // only the import from the parent (not a grandparent, sibling, or child). // - // - lib declares Random + // - lib1 declares A + // - lib2 declares A // // - root has import // - level1_other has import @@ -300,56 +300,60 @@ void f() { // - test has reference <-- testing this // - level3_other has import - newFile('$testPackageLibPath/lib.dart', ''' -class Random {} + newFile('$testPackageLibPath/lib1.dart', ''' +class A {} +'''); + + newFile('$testPackageLibPath/lib2.dart', ''' +class A {} '''); newFile('$testPackageLibPath/root.dart', ''' -import 'dart:math'; +import 'lib1.dart'; part 'level1_other.dart'; part 'level1.dart'; '''); newFile('$testPackageLibPath/level1_other.dart', ''' part of 'root.dart'; -import 'dart:math'; +import 'lib1.dart'; '''); newFile('$testPackageLibPath/level1.dart', ''' part of 'root.dart'; -import 'dart:math'; -import 'lib.dart'; +import 'lib1.dart'; +import 'lib2.dart'; part 'level2_other.dart'; part 'test.dart'; '''); newFile('$testPackageLibPath/level2_other.dart', ''' part of 'level1.dart'; -import 'dart:math'; +import 'lib1.dart'; '''); newFile('$testPackageLibPath/level3_other.dart', ''' part of 'test.dart'; -import 'dart:math'; +import 'lib1.dart'; '''); await resolveTestCode(''' part of 'level1.dart'; part 'level3_other.dart'; -Random? r; +A? a; '''); await assertHasFix( ''' part of 'root.dart'; -import 'dart:math' hide Random; -import 'lib.dart'; +import 'lib1.dart' hide A; +import 'lib2.dart'; part 'level2_other.dart'; part 'test.dart'; ''', target: '$testPackageLibPath/level1.dart', - matchFixMessage: "Hide others to use 'Random' from 'lib.dart'", + matchFixMessage: "Hide others to use 'A' from 'lib2.dart'", ); } From 350976bbfdae8e61691685861f13d49245148f2b Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 26 Dec 2024 09:40:26 -0300 Subject: [PATCH 21/28] readded removed (by mistake) lines on merge --- .../lib/src/services/correction/fix_internal.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 4d1f25ba2181..1e5649a3c709 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -106,6 +106,7 @@ import 'package:analysis_server/src/services/correction/dart/import_library.dart import 'package:analysis_server/src/services/correction/dart/inline_invocation.dart'; import 'package:analysis_server/src/services/correction/dart/inline_typedef.dart'; import 'package:analysis_server/src/services/correction/dart/insert_body.dart'; +import 'package:analysis_server/src/services/correction/dart/insert_on_keyword.dart'; import 'package:analysis_server/src/services/correction/dart/insert_semicolon.dart'; import 'package:analysis_server/src/services/correction/dart/make_class_abstract.dart'; import 'package:analysis_server/src/services/correction/dart/make_conditional_on_debug_mode.dart'; @@ -381,6 +382,7 @@ final _builtInLintProducers = >{ LinterLintCode.null_closures: [ReplaceNullWithClosure.new], LinterLintCode.omit_local_variable_types: [ReplaceWithVar.new], LinterLintCode.omit_obvious_local_variable_types: [ReplaceWithVar.new], + LinterLintCode.omit_obvious_property_types: [ReplaceWithVar.new], LinterLintCode.prefer_adjacent_string_concatenation: [RemoveOperator.new], LinterLintCode.prefer_collection_literals: [ ConvertToMapLiteral.new, @@ -1150,7 +1152,11 @@ final _builtInNonLintProducers = >{ ParserErrorCode.EXPECTED_SWITCH_EXPRESSION_BODY: [InsertBody.new], ParserErrorCode.EXPECTED_SWITCH_STATEMENT_BODY: [InsertBody.new], ParserErrorCode.EXPECTED_TRY_STATEMENT_BODY: [InsertBody.new], - ParserErrorCode.EXPECTED_TOKEN: [InsertSemicolon.new, ReplaceWithArrow.new], + ParserErrorCode.EXPECTED_TOKEN: [ + InsertSemicolon.new, + ReplaceWithArrow.new, + InsertOnKeyword.new, + ], ParserErrorCode.EXTENSION_AUGMENTATION_HAS_ON_CLAUSE: [RemoveOnClause.new], ParserErrorCode.EXTENSION_DECLARES_CONSTRUCTOR: [RemoveConstructor.new], ParserErrorCode.EXTERNAL_CLASS: [RemoveLexeme.modifier], From 22112c9a88aa5a3cd3d121021fb9c04e4cdbdc13 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 26 Dec 2024 09:46:06 -0300 Subject: [PATCH 22/28] removes trailing white space --- .../lib/src/services/correction/fix_internal.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 1e5649a3c709..62a7eda0fff1 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -1153,7 +1153,7 @@ final _builtInNonLintProducers = >{ ParserErrorCode.EXPECTED_SWITCH_STATEMENT_BODY: [InsertBody.new], ParserErrorCode.EXPECTED_TRY_STATEMENT_BODY: [InsertBody.new], ParserErrorCode.EXPECTED_TOKEN: [ - InsertSemicolon.new, + InsertSemicolon.new, ReplaceWithArrow.new, InsertOnKeyword.new, ], From 2452a61ac56db77dbf1f0dbe220f8fa05281b035 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 26 Dec 2024 09:56:21 -0300 Subject: [PATCH 23/28] rename files --- .../dart/{import_add_hide.dart => ambiguous_import_fix.dart} | 0 .../lib/src/services/correction/fix_internal.dart | 2 +- ...port_add_hide_test.dart => ambiguous_import_fix_test.dart} | 0 .../test/src/services/correction/fix/test_all.dart | 4 ++-- 4 files changed, 3 insertions(+), 3 deletions(-) rename pkg/analysis_server/lib/src/services/correction/dart/{import_add_hide.dart => ambiguous_import_fix.dart} (100%) rename pkg/analysis_server/test/src/services/correction/fix/{import_add_hide_test.dart => ambiguous_import_fix_test.dart} (100%) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart b/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart similarity index 100% rename from pkg/analysis_server/lib/src/services/correction/dart/import_add_hide.dart rename to pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 62a7eda0fff1..05740abf7731 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -101,7 +101,7 @@ import 'package:analysis_server/src/services/correction/dart/extend_class_for_mi import 'package:analysis_server/src/services/correction/dart/extract_local_variable.dart'; import 'package:analysis_server/src/services/correction/dart/flutter_remove_widget.dart'; import 'package:analysis_server/src/services/correction/dart/ignore_diagnostic.dart'; -import 'package:analysis_server/src/services/correction/dart/import_add_hide.dart'; +import 'package:analysis_server/src/services/correction/dart/ambiguous_import_fix.dart'; import 'package:analysis_server/src/services/correction/dart/import_library.dart'; import 'package:analysis_server/src/services/correction/dart/inline_invocation.dart'; import 'package:analysis_server/src/services/correction/dart/inline_typedef.dart'; diff --git a/pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart b/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart similarity index 100% rename from pkg/analysis_server/test/src/services/correction/fix/import_add_hide_test.dart rename to pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart diff --git a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart index 6465d938bb88..0dc51472cbbc 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart @@ -132,7 +132,7 @@ import 'fix_processor_map_test.dart' as fix_processor_map; import 'fix_test.dart' as fix; import 'format_file_test.dart' as format_file; import 'ignore_diagnostic_test.dart' as ignore_error; -import 'import_add_hide_test.dart' as import_add_hide; +import 'ambiguous_import_fix_test.dart' as ambiguous_import_fix; import 'import_library_hide_test.dart' as import_library_hide; import 'import_library_prefix_test.dart' as import_library_prefix; import 'import_library_project_test.dart' as import_library_project; @@ -417,7 +417,7 @@ void main() { fix_processor_map.main(); format_file.main(); ignore_error.main(); - import_add_hide.main(); + ambiguous_import_fix.main(); import_library_hide.main(); import_library_prefix.main(); import_library_project.main(); From f2291c13a44ec54208d71568b108b6274e12827a Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 26 Dec 2024 15:12:49 -0300 Subject: [PATCH 24/28] docs and sorting of combinators considering lint --- .../correction/dart/ambiguous_import_fix.dart | 33 +++++++++-- .../correction/dart/import_library.dart | 1 + .../fix/ambiguous_import_fix_test.dart | 56 +++++++++++++++++++ .../lib/dart/analysis/code_style_options.dart | 3 + .../analysis_options/code_style_options.dart | 3 + 5 files changed, 92 insertions(+), 4 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart b/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart index fa742020a66f..7cd87e9d6ae1 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart @@ -74,14 +74,26 @@ class AmbiguousImportFix extends MultiCorrectionProducer { } /// Returns [ImportDirective]s that import the given [conflictingElements] - /// into [unitResult]. + /// 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 + /// library hierarchy (enhanced parts). (Set, ResolvedUnitResult?, Set) _getImportDirectives( ResolvedLibraryResult libraryResult, ResolvedUnitResult? unitResult, List conflictingElements, String? prefix, ) { + // The uris of all import directives that import the conflicting elements. var uris = {}; + // The import directives that import the conflicting elements. var importDirectives = {}; // Search in each unit up the chain for related imports. while (unitResult is ResolvedUnitResult) { @@ -181,9 +193,13 @@ class _ImportAddHide extends ResolvedCorrectionProducer { for (var (:directive, :hide) in hideCombinators) { if (hide != null) { var allNames = [ - _elementName, ...hide.hiddenNames.map((name) => name.name), - ]..sort(); + _elementName, + ]; + if (_sortCombinators) { + allNames.sort(); + } + // TODO(FMorschel): Use the utility function instead of ', '. var combinator = 'hide ${allNames.join(', ')}'; var range = SourceRange(hide.offset, hide.length); builder.addSimpleReplacement(range, combinator); @@ -247,12 +263,16 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { ...show.shownNames .map((name) => name.name) .where((name) => name != _elementName), - ]..sort(); + ]; + 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); @@ -261,3 +281,8 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { }); } } + +extension on ResolvedCorrectionProducer { + bool get _sortCombinators => + getCodeStyleOptions(unitResult.file).combinatorsOrdering; +} diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart index 605a83bdfa58..4c373a055659 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart @@ -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; diff --git a/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart b/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart index bebf15af984e..9f761967eaed 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart @@ -5,6 +5,7 @@ import 'package:analysis_server/src/services/correction/fix.dart'; import 'package:analyzer/src/error/codes.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:linter/src/lint_names.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; import 'fix_processor.dart'; @@ -226,6 +227,30 @@ class N {}'''); import 'lib1.dart' hide M, O; import 'lib2.dart'; +void f(N? n) { + print(n); +} +'''); + await assertHasFix(''' +import 'lib1.dart' hide M, O, N; +import 'lib2.dart'; + +void f(N? n) { + print(n); +} +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); + } + + Future test_double_oneHide_sort() async { + createAnalysisOptionsFile(lints: [LintNames.combinators_ordering]); + newFile('$testPackageLibPath/lib1.dart', ''' +class M {} class N {} class O {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' hide M, O; +import 'lib2.dart'; + void f(N? n) { print(n); } @@ -639,6 +664,37 @@ void f(N? n) { ''', matchFixMessage: "Remove show to use 'N' from 'lib2.dart'"); } + Future test_moreShow_sort() async { + createAnalysisOptionsFile(lints: [LintNames.combinators_ordering]); + newFile('$testPackageLibPath/lib1.dart', ''' +class N {} +class M {} +class O {}'''); + newFile('$testPackageLibPath/lib2.dart', ''' +class N {}'''); + await resolveTestCode(''' +import 'lib1.dart' show N, O, M; +import 'lib2.dart'; + +void f(N? n, O? o) { + print(n); +} +'''); + await assertHasFix( + ''' +import 'lib1.dart' show M, O; +import 'lib2.dart'; + +void f(N? n, O? o) { + print(n); +} +''', + errorFilter: (error) => + error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT, + matchFixMessage: "Remove show to use 'N' from 'lib2.dart'", + ); + } + Future test_one_show() async { newFile('$testPackageLibPath/lib1.dart', ''' var foo = 0;'''); diff --git a/pkg/analyzer/lib/dart/analysis/code_style_options.dart b/pkg/analyzer/lib/dart/analysis/code_style_options.dart index 09d077d0e9a8..4d56e9b3cbba 100644 --- a/pkg/analyzer/lib/dart/analysis/code_style_options.dart +++ b/pkg/analyzer/lib/dart/analysis/code_style_options.dart @@ -13,6 +13,9 @@ abstract class CodeStyleOptions { /// should be inserted in function calls and declarations. bool get addTrailingCommas; + /// Whether combinators should be ordered alphabetically. + bool get combinatorsOrdering; + /// Whether local variables should be `final` inside a for-loop. bool get finalInForEach; diff --git a/pkg/analyzer/lib/src/analysis_options/code_style_options.dart b/pkg/analyzer/lib/src/analysis_options/code_style_options.dart index 241bdf3cbc9e..ff225c8ce8b4 100644 --- a/pkg/analyzer/lib/src/analysis_options/code_style_options.dart +++ b/pkg/analyzer/lib/src/analysis_options/code_style_options.dart @@ -19,6 +19,9 @@ class CodeStyleOptionsImpl implements CodeStyleOptions { @override bool get addTrailingCommas => _isLintEnabled('require_trailing_commas'); + @override + bool get combinatorsOrdering => _isLintEnabled('combinators_ordering'); + @override bool get finalInForEach => _isLintEnabled('prefer_final_in_for_each'); From 2bd0a4a01043ed196be398a70f75a7d6dd3f6d05 Mon Sep 17 00:00:00 2001 From: FMorschel <52160996+FMorschel@users.noreply.github.com> Date: Thu, 26 Dec 2024 15:18:02 -0300 Subject: [PATCH 25/28] sorting and formatting --- .../lib/src/services/correction/fix_internal.dart | 6 +++--- .../test/src/services/correction/fix/test_all.dart | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 05740abf7731..654bdccac124 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -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'; @@ -101,7 +102,6 @@ import 'package:analysis_server/src/services/correction/dart/extend_class_for_mi import 'package:analysis_server/src/services/correction/dart/extract_local_variable.dart'; import 'package:analysis_server/src/services/correction/dart/flutter_remove_widget.dart'; import 'package:analysis_server/src/services/correction/dart/ignore_diagnostic.dart'; -import 'package:analysis_server/src/services/correction/dart/ambiguous_import_fix.dart'; import 'package:analysis_server/src/services/correction/dart/import_library.dart'; import 'package:analysis_server/src/services/correction/dart/inline_invocation.dart'; import 'package:analysis_server/src/services/correction/dart/inline_typedef.dart'; @@ -1102,8 +1102,8 @@ final _builtInNonLintProducers = >{ // updated so that only the appropriate subset is generated. QualifyReference.new, ], - CompileTimeErrorCode - .UNQUALIFIED_REFERENCE_TO_STATIC_MEMBER_OF_EXTENDED_TYPE: [ + CompileTimeErrorCode.UNQUALIFIED_REFERENCE_TO_STATIC_MEMBER_OF_EXTENDED_TYPE: + [ // TODO(brianwilkerson): Consider adding fixes to create a field, getter, // method or setter. The existing producers would need to be updated so // that only the appropriate subset is generated. diff --git a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart index 0dc51472cbbc..3d1b1b63ce2b 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart @@ -50,6 +50,7 @@ import 'add_super_parameter_test.dart' as add_super_parameter; import 'add_switch_case_break_test.dart' as add_switch_case_break; import 'add_trailing_comma_test.dart' as add_trailing_comma; import 'add_type_annotation_test.dart' as add_type_annotation; +import 'ambiguous_import_fix_test.dart' as ambiguous_import_fix; import 'analysis_options/test_all.dart' as analysis_options; import 'bulk_fix_processor_test.dart' as bulk_fix_processor; import 'change_argument_name_test.dart' as change_argument_name; @@ -132,7 +133,6 @@ import 'fix_processor_map_test.dart' as fix_processor_map; import 'fix_test.dart' as fix; import 'format_file_test.dart' as format_file; import 'ignore_diagnostic_test.dart' as ignore_error; -import 'ambiguous_import_fix_test.dart' as ambiguous_import_fix; import 'import_library_hide_test.dart' as import_library_hide; import 'import_library_prefix_test.dart' as import_library_prefix; import 'import_library_project_test.dart' as import_library_project; From bfa48ddfa1a806f34f852a03365cd81174a515d8 Mon Sep 17 00:00:00 2001 From: FMorschel Date: Sat, 4 Jan 2025 23:29:28 -0300 Subject: [PATCH 26/28] refactors - reviews --- .../correction/dart/ambiguous_import_fix.dart | 87 +++++----- .../fix/ambiguous_import_fix_test.dart | 148 ++++++++++-------- .../lib/dart/analysis/code_style_options.dart | 7 +- .../analysis_options/code_style_options.dart | 6 +- 4 files changed, 142 insertions(+), 106 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart b/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart index 7cd87e9d6ae1..1b341590f259 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart @@ -12,6 +12,7 @@ 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 { @@ -40,7 +41,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { return const []; } - var (uris, unit, importDirectives) = _getImportDirectives( + var (unit, importDirectives, uris) = _getImportDirectives( libraryResult, unitResult, conflictingElements, @@ -51,26 +52,27 @@ class AmbiguousImportFix extends MultiCorrectionProducer { return const []; } - var producers = {}; + var producers = []; + var thisContext = CorrectionProducerContext.createResolved( + libraryResult: libraryResult, + unitResult: unit, + applyingBulkFixes: applyingBulkFixes, + dartFixContext: context.dartFixContext, + ); + for (var uri in uris) { - var thisContext = CorrectionProducerContext.createResolved( - libraryResult: libraryResult, - unitResult: unit, - applyingBulkFixes: applyingBulkFixes, - dartFixContext: context.dartFixContext, - diagnostic: diagnostic, - selectionLength: selectionLength, - selectionOffset: selectionOffset, - ); - var directives = importDirectives - .whereNot((directive) => directive.uri.stringValue == uri) - .toSet(); - producers.addAll([ + var directives = + importDirectives + .whereNot((directive) => directive.uri.stringValue == uri) + .toSet(); + producers.add( _ImportAddHide(name, uri, prefix, directives, context: thisContext), + ); + producers.add( _ImportRemoveShow(name, uri, prefix, directives, context: thisContext), - ]); + ); } - return producers.toList(); + return producers; } /// Returns [ImportDirective]s that import the given [conflictingElements] @@ -84,8 +86,8 @@ class AmbiguousImportFix extends MultiCorrectionProducer { /// 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 - /// library hierarchy (enhanced parts). - (Set, ResolvedUnitResult?, Set) _getImportDirectives( + /// part hierarchy (enhanced parts). + (ResolvedUnitResult?, Set, Set) _getImportDirectives( ResolvedLibraryResult libraryResult, ResolvedUnitResult? unitResult, List conflictingElements, @@ -95,23 +97,24 @@ class AmbiguousImportFix extends MultiCorrectionProducer { var uris = {}; // The import directives that import the conflicting elements. var importDirectives = {}; + + 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) { - var library = conflictingElement.enclosingElement2?.library2; - if (library == null) { - continue; - } - // Find all ImportDirective that import this library in this unit // and have the same prefix. - for (var directive in unitResult.unit.directives - .whereType() - .whereNot(importDirectives.contains)) { - var imported = directive.libraryImport?.importedLibrary2; - if (imported == null) { + for (var directive + in unitResult.unit.directives.whereType()) { + 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; @@ -119,8 +122,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { // If this library is imported directly or if the directive exports the // library for this element. - if (imported == library || - imported.exportedLibraries2.contains(library)) { + if (libraryImport.namespace.get2(name) == conflictingElement) { var uri = directive.uri.stringValue; if (uri != null) { uris.add(uri); @@ -138,7 +140,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { unitResult = libraryResult.parentUnitOf(unitResult); } - return (uris, unitResult, importDirectives); + return (unitResult, importDirectives, uris); } } @@ -149,8 +151,12 @@ class _ImportAddHide extends ResolvedCorrectionProducer { final String _elementName; _ImportAddHide( - this._elementName, this.uri, this.prefix, this.importDirectives, - {required super.context}); + this._elementName, + this.uri, + this.prefix, + this.importDirectives, { + required super.context, + }); @override CorrectionApplicability get applicability => @@ -201,8 +207,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { } // TODO(FMorschel): Use the utility function instead of ', '. var combinator = 'hide ${allNames.join(', ')}'; - var range = SourceRange(hide.offset, hide.length); - builder.addSimpleReplacement(range, combinator); + builder.addSimpleReplacement(range.node(hide), combinator); } else { var hideCombinator = ' hide $_elementName'; builder.addSimpleInsertion(directive.end - 1, hideCombinator); @@ -219,8 +224,12 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { final String? prefix; _ImportRemoveShow( - this._elementName, this.uri, this.prefix, this.importDirectives, - {required super.context}); + this._elementName, + this.uri, + this.prefix, + this.importDirectives, { + required super.context, + }); @override CorrectionApplicability get applicability => @@ -284,5 +293,5 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { extension on ResolvedCorrectionProducer { bool get _sortCombinators => - getCodeStyleOptions(unitResult.file).combinatorsOrdering; + getCodeStyleOptions(unitResult.file).sortCombinators; } diff --git a/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart b/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart index 9f761967eaed..1c23cb759fb5 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart @@ -23,9 +23,9 @@ class ImportAddHideTest extends FixProcessorTest { FixKind get kind => DartFixKind.IMPORT_LIBRARY_HIDE; Future test_double() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -60,9 +60,9 @@ void f(N? n) { } Future test_double_aliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' as i; @@ -97,9 +97,9 @@ void f(i.N? n) { } Future test_double_constant() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart'; @@ -120,35 +120,61 @@ void f() { } Future test_double_exportedByImport() async { - newFile('$testPackageLibPath/lib1.dart', ''' -export 'lib3.dart';'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' mixin M {}'''); - newFile('$testPackageLibPath/lib3.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' mixin M {}'''); + newFile(join(testPackageLibPath,'lib3.dart'), ''' +export 'lib2.dart';'''); await resolveTestCode(''' import 'lib1.dart'; -import 'lib2.dart'; +import 'lib3.dart'; class C with M {} '''); await assertHasFix(''' import 'lib1.dart' hide M; -import 'lib2.dart'; +import 'lib3.dart'; class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib2.dart'", +''', matchFixMessage: "Hide others to use 'M' from 'lib3.dart'", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }); + } + + Future test_double_doubleExportedByImport() async { + newFile(join(testPackageLibPath,'lib1.dart'), ''' +mixin M {}'''); + newFile(join(testPackageLibPath,'lib2.dart'), ''' +mixin M {}'''); + newFile(join(testPackageLibPath,'lib3.dart'), ''' +export 'lib2.dart';'''); + newFile(join(testPackageLibPath, 'lib4.dart'), ''' +export 'lib3.dart';'''); + await resolveTestCode(''' +import 'lib1.dart'; +import 'lib4.dart'; + +class C with M {} +'''); + await assertHasFix(''' +import 'lib1.dart' hide M; +import 'lib4.dart'; + +class C with M {} +''', matchFixMessage: "Hide others to use 'M' from 'lib4.dart'", errorFilter: (error) { return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; }); } Future test_double_extension() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); @@ -174,9 +200,9 @@ void foo(int i) { } Future test_double_function() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' void bar() {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' void bar() {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -197,9 +223,9 @@ void foo() { } Future test_double_mixin() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' mixin M {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' mixin M {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -219,9 +245,9 @@ class C with M {} } Future test_double_oneHide() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class M {} class N {} class O {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' hide M, O; @@ -243,9 +269,9 @@ void f(N? n) { Future test_double_oneHide_sort() async { createAnalysisOptionsFile(lints: [LintNames.combinators_ordering]); - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class M {} class N {} class O {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' hide M, O; @@ -266,9 +292,9 @@ void f(N? n) { } Future test_double_oneShow() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N; @@ -289,9 +315,9 @@ void f(N? n) { } Future test_double_variable() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' var foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' var foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' show foo; @@ -325,26 +351,26 @@ void f() { // - test has reference <-- testing this // - level3_other has import - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class A {} '''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class A {} '''); - newFile('$testPackageLibPath/root.dart', ''' + newFile(join(testPackageLibPath,'root.dart'), ''' import 'lib1.dart'; part 'level1_other.dart'; part 'level1.dart'; '''); - newFile('$testPackageLibPath/level1_other.dart', ''' + newFile(join(testPackageLibPath,'level1_other.dart'), ''' part of 'root.dart'; import 'lib1.dart'; '''); - newFile('$testPackageLibPath/level1.dart', ''' + newFile(join(testPackageLibPath,'level1.dart'), ''' part of 'root.dart'; import 'lib1.dart'; import 'lib2.dart'; @@ -352,12 +378,12 @@ part 'level2_other.dart'; part 'test.dart'; '''); - newFile('$testPackageLibPath/level2_other.dart', ''' + newFile(join(testPackageLibPath,'level2_other.dart'), ''' part of 'level1.dart'; import 'lib1.dart'; '''); - newFile('$testPackageLibPath/level3_other.dart', ''' + newFile(join(testPackageLibPath,'level3_other.dart'), ''' part of 'test.dart'; import 'lib1.dart'; '''); @@ -377,17 +403,17 @@ import 'lib2.dart'; part 'level2_other.dart'; part 'test.dart'; ''', - target: '$testPackageLibPath/level1.dart', + target: join(testPackageLibPath,'level1.dart'), matchFixMessage: "Hide others to use 'A' from 'lib2.dart'", ); } Future test_part() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); - newFile('$testPackageLibPath/other.dart', ''' + newFile(join(testPackageLibPath,'other.dart'), ''' import 'lib1.dart'; import 'lib2.dart'; part 'test.dart'; @@ -435,9 +461,9 @@ void f(N? n) { } Future test_show_prefixed() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' as l show N; @@ -458,11 +484,11 @@ void f(l.N? n) { } Future test_triple() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' export 'lib3.dart';'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' mixin M {}'''); - newFile('$testPackageLibPath/lib3.dart', ''' + newFile(join(testPackageLibPath,'lib3.dart'), ''' mixin M {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -504,9 +530,9 @@ class C with M {} } Future test_triple_oneAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' as lib; @@ -529,9 +555,9 @@ void f() { } Future test_triple_twoAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart'; @@ -560,9 +586,9 @@ class ImportRemoveShowTest extends FixProcessorTest { FixKind get kind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; Future test_double() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N; @@ -583,9 +609,9 @@ void f(N? n) { } Future test_double_aliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class N {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' as l show N; @@ -620,9 +646,9 @@ void f(l.N? n) { } Future test_double_oneHide() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' hide foo; @@ -636,10 +662,10 @@ void f() { } Future test_moreShow() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class N {} class M {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N, M; @@ -666,11 +692,11 @@ void f(N? n) { Future test_moreShow_sort() async { createAnalysisOptionsFile(lints: [LintNames.combinators_ordering]); - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' class N {} class M {} class O {}'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N, O, M; @@ -696,9 +722,9 @@ void f(N? n, O? o) { } Future test_one_show() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' var foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' var foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' show foo; @@ -719,9 +745,9 @@ void f() { } Future test_triple_twoAliased() async { - newFile('$testPackageLibPath/lib1.dart', ''' + newFile(join(testPackageLibPath,'lib1.dart'), ''' const foo = 0;'''); - newFile('$testPackageLibPath/lib2.dart', ''' + newFile(join(testPackageLibPath,'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart'; diff --git a/pkg/analyzer/lib/dart/analysis/code_style_options.dart b/pkg/analyzer/lib/dart/analysis/code_style_options.dart index 4d56e9b3cbba..bae080839fc3 100644 --- a/pkg/analyzer/lib/dart/analysis/code_style_options.dart +++ b/pkg/analyzer/lib/dart/analysis/code_style_options.dart @@ -13,9 +13,6 @@ abstract class CodeStyleOptions { /// should be inserted in function calls and declarations. bool get addTrailingCommas; - /// Whether combinators should be ordered alphabetically. - bool get combinatorsOrdering; - /// Whether local variables should be `final` inside a for-loop. bool get finalInForEach; @@ -31,6 +28,10 @@ abstract class CodeStyleOptions { /// The preferred quote based on the enabled lints, otherwise a single quote. String get preferredQuoteForStrings; + /// Whether combinators should be ordered alphabetically. Difined by + /// `combinators_ordering`. + bool get sortCombinators; + /// Whether constructors should be sorted first, before other class members. bool get sortConstructorsFirst; diff --git a/pkg/analyzer/lib/src/analysis_options/code_style_options.dart b/pkg/analyzer/lib/src/analysis_options/code_style_options.dart index ff225c8ce8b4..95fecedc655c 100644 --- a/pkg/analyzer/lib/src/analysis_options/code_style_options.dart +++ b/pkg/analyzer/lib/src/analysis_options/code_style_options.dart @@ -19,9 +19,6 @@ class CodeStyleOptionsImpl implements CodeStyleOptions { @override bool get addTrailingCommas => _isLintEnabled('require_trailing_commas'); - @override - bool get combinatorsOrdering => _isLintEnabled('combinators_ordering'); - @override bool get finalInForEach => _isLintEnabled('prefer_final_in_for_each'); @@ -38,6 +35,9 @@ class CodeStyleOptionsImpl implements CodeStyleOptions { @override String get preferredQuoteForStrings => _lintQuote() ?? "'"; + @override + bool get sortCombinators => _isLintEnabled('combinators_ordering'); + @override bool get sortConstructorsFirst => _isLintEnabled('sort_constructors_first'); From 8f1adaf45fca765f934a9acd4f826f66bf36a9e5 Mon Sep 17 00:00:00 2001 From: FMorschel Date: Mon, 6 Jan 2025 14:10:26 -0300 Subject: [PATCH 27/28] changes set->list and sorts files --- .../correction/dart/ambiguous_import_fix.dart | 23 +- .../src/services/correction/fix_internal.dart | 8 +- .../fix/ambiguous_import_fix_test.dart | 285 ++++++++++-------- 3 files changed, 167 insertions(+), 149 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart b/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart index 1b341590f259..7fbf3ba1555a 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart @@ -64,7 +64,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { var directives = importDirectives .whereNot((directive) => directive.uri.stringValue == uri) - .toSet(); + .toList(); producers.add( _ImportAddHide(name, uri, prefix, directives, context: thisContext), ); @@ -87,16 +87,17 @@ class AmbiguousImportFix extends MultiCorrectionProducer { /// 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?, Set, Set) _getImportDirectives( + (ResolvedUnitResult?, List, List) + _getImportDirectives( ResolvedLibraryResult libraryResult, ResolvedUnitResult? unitResult, List conflictingElements, String? prefix, ) { // The uris of all import directives that import the conflicting elements. - var uris = {}; + var uris = []; // The import directives that import the conflicting elements. - var importDirectives = {}; + var importDirectives = []; var name = conflictingElements.firstOrNull?.name3; if (name == null || name.isEmpty) { @@ -145,7 +146,7 @@ class AmbiguousImportFix extends MultiCorrectionProducer { } class _ImportAddHide extends ResolvedCorrectionProducer { - final Set importDirectives; + final List importDirectives; final String uri; final String? prefix; final String _elementName; @@ -160,8 +161,9 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override CorrectionApplicability get applicability => - // TODO(applicability): comment on why. - CorrectionApplicability.singleLocation; + // TODO(applicability): comment on why. + CorrectionApplicability + .singleLocation; @override List get fixArguments { @@ -218,7 +220,7 @@ class _ImportAddHide extends ResolvedCorrectionProducer { } class _ImportRemoveShow extends ResolvedCorrectionProducer { - final Set importDirectives; + final List importDirectives; final String _elementName; final String uri; final String? prefix; @@ -233,8 +235,9 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { @override CorrectionApplicability get applicability => - // TODO(applicability): comment on why. - CorrectionApplicability.singleLocation; + // TODO(applicability): comment on why. + CorrectionApplicability + .singleLocation; @override List get fixArguments { diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 654bdccac124..d7f9f08d5159 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -541,9 +541,7 @@ final _builtInNonLintMultiProducers = { CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS_THREE_OR_MORE: [ AddExtensionOverride.new, ], - CompileTimeErrorCode.AMBIGUOUS_IMPORT: [ - AmbiguousImportFix.new, - ], + CompileTimeErrorCode.AMBIGUOUS_IMPORT: [AmbiguousImportFix.new], CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE: [DataDriven.new], CompileTimeErrorCode.CAST_TO_NON_TYPE: [ DataDriven.new, @@ -1102,8 +1100,8 @@ final _builtInNonLintProducers = >{ // updated so that only the appropriate subset is generated. QualifyReference.new, ], - CompileTimeErrorCode.UNQUALIFIED_REFERENCE_TO_STATIC_MEMBER_OF_EXTENDED_TYPE: - [ + CompileTimeErrorCode + .UNQUALIFIED_REFERENCE_TO_STATIC_MEMBER_OF_EXTENDED_TYPE: [ // TODO(brianwilkerson): Consider adding fixes to create a field, getter, // method or setter. The existing producers would need to be updated so // that only the appropriate subset is generated. diff --git a/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart b/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart index 1c23cb759fb5..8673b187c6bf 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart @@ -23,9 +23,9 @@ class ImportAddHideTest extends FixProcessorTest { FixKind get kind => DartFixKind.IMPORT_LIBRARY_HIDE; Future test_double() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class N {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -36,11 +36,12 @@ void f(N? n) { } '''); await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 2, - matchFixMessages: [ - "Hide others to use 'N' from 'lib1.dart'", - "Hide others to use 'N' from 'lib2.dart'", - ]); + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Hide others to use 'N' from 'lib1.dart'", + "Hide others to use 'N' from 'lib2.dart'", + ], + ); await assertHasFix(''' import 'lib1.dart' hide N; import 'lib2.dart'; @@ -60,9 +61,9 @@ void f(N? n) { } Future test_double_aliased() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class N {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' as i; @@ -73,11 +74,12 @@ void f(i.N? n) { } '''); await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 2, - matchFixMessages: [ - "Hide others to use 'N' from 'lib1.dart' as i", - "Hide others to use 'N' from 'lib2.dart' as i", - ]); + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Hide others to use 'N' from 'lib1.dart' as i", + "Hide others to use 'N' from 'lib2.dart' as i", + ], + ); await assertHasFix(''' import 'lib1.dart' as i; import 'lib2.dart' as i hide N; @@ -97,9 +99,9 @@ void f(i.N? n) { } Future test_double_constant() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' const foo = 0;'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart'; @@ -119,62 +121,68 @@ void f() { ''', matchFixMessage: "Hide others to use 'foo' from 'lib1.dart'"); } - Future test_double_exportedByImport() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + Future test_double_doubleExportedByImport() async { + newFile(join(testPackageLibPath, 'lib1.dart'), ''' mixin M {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' mixin M {}'''); - newFile(join(testPackageLibPath,'lib3.dart'), ''' + newFile(join(testPackageLibPath, 'lib3.dart'), ''' export 'lib2.dart';'''); + newFile(join(testPackageLibPath, 'lib4.dart'), ''' +export 'lib3.dart';'''); await resolveTestCode(''' import 'lib1.dart'; -import 'lib3.dart'; +import 'lib4.dart'; class C with M {} '''); - await assertHasFix(''' + await assertHasFix( + ''' import 'lib1.dart' hide M; -import 'lib3.dart'; +import 'lib4.dart'; class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib3.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); +''', + matchFixMessage: "Hide others to use 'M' from 'lib4.dart'", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }, + ); } - Future test_double_doubleExportedByImport() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + Future test_double_exportedByImport() async { + newFile(join(testPackageLibPath, 'lib1.dart'), ''' mixin M {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' mixin M {}'''); - newFile(join(testPackageLibPath,'lib3.dart'), ''' + newFile(join(testPackageLibPath, 'lib3.dart'), ''' export 'lib2.dart';'''); - newFile(join(testPackageLibPath, 'lib4.dart'), ''' -export 'lib3.dart';'''); await resolveTestCode(''' import 'lib1.dart'; -import 'lib4.dart'; +import 'lib3.dart'; class C with M {} '''); - await assertHasFix(''' + await assertHasFix( + ''' import 'lib1.dart' hide M; -import 'lib4.dart'; +import 'lib3.dart'; class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib4.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); +''', + matchFixMessage: "Hide others to use 'M' from 'lib3.dart'", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }, + ); } Future test_double_extension() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' extension E on int { bool get isDivisibleByThree => this % 3 == 0; }'''); @@ -186,23 +194,26 @@ void foo(int i) { print(E(i.isDivisibleByThree)); } '''); - await assertHasFix(''' + await assertHasFix( + ''' import 'lib1.dart'; import 'lib2.dart' hide E; void foo(int i) { print(E(i.isDivisibleByThree)); } -''', matchFixMessage: "Hide others to use 'E' from 'lib1.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); +''', + matchFixMessage: "Hide others to use 'E' from 'lib1.dart'", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }, + ); } Future test_double_function() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' void bar() {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' void bar() {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -223,9 +234,9 @@ void foo() { } Future test_double_mixin() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' mixin M {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' mixin M {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -233,21 +244,24 @@ import 'lib2.dart'; class C with M {} '''); - await assertHasFix(''' + await assertHasFix( + ''' import 'lib1.dart'; import 'lib2.dart' hide M; class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); +''', + matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }, + ); } Future test_double_oneHide() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class M {} class N {} class O {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' hide M, O; @@ -269,9 +283,9 @@ void f(N? n) { Future test_double_oneHide_sort() async { createAnalysisOptionsFile(lints: [LintNames.combinators_ordering]); - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class M {} class N {} class O {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' hide M, O; @@ -292,9 +306,9 @@ void f(N? n) { } Future test_double_oneShow() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class N {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N; @@ -315,9 +329,9 @@ void f(N? n) { } Future test_double_variable() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' var foo = 0;'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' var foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' show foo; @@ -351,26 +365,26 @@ void f() { // - test has reference <-- testing this // - level3_other has import - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class A {} '''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class A {} '''); - newFile(join(testPackageLibPath,'root.dart'), ''' + newFile(join(testPackageLibPath, 'root.dart'), ''' import 'lib1.dart'; part 'level1_other.dart'; part 'level1.dart'; '''); - newFile(join(testPackageLibPath,'level1_other.dart'), ''' + newFile(join(testPackageLibPath, 'level1_other.dart'), ''' part of 'root.dart'; import 'lib1.dart'; '''); - newFile(join(testPackageLibPath,'level1.dart'), ''' + newFile(join(testPackageLibPath, 'level1.dart'), ''' part of 'root.dart'; import 'lib1.dart'; import 'lib2.dart'; @@ -378,12 +392,12 @@ part 'level2_other.dart'; part 'test.dart'; '''); - newFile(join(testPackageLibPath,'level2_other.dart'), ''' + newFile(join(testPackageLibPath, 'level2_other.dart'), ''' part of 'level1.dart'; import 'lib1.dart'; '''); - newFile(join(testPackageLibPath,'level3_other.dart'), ''' + newFile(join(testPackageLibPath, 'level3_other.dart'), ''' part of 'test.dart'; import 'lib1.dart'; '''); @@ -403,17 +417,17 @@ import 'lib2.dart'; part 'level2_other.dart'; part 'test.dart'; ''', - target: join(testPackageLibPath,'level1.dart'), + target: join(testPackageLibPath, 'level1.dart'), matchFixMessage: "Hide others to use 'A' from 'lib2.dart'", ); } Future test_part() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class N {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); - newFile(join(testPackageLibPath,'other.dart'), ''' + newFile(join(testPackageLibPath, 'other.dart'), ''' import 'lib1.dart'; import 'lib2.dart'; part 'test.dart'; @@ -434,8 +448,7 @@ void f(N? n) { "Hide others to use 'N' from 'lib2.dart'", ], ); - await assertHasFix( - ''' + await assertHasFix(''' part of 'other.dart'; import 'lib1.dart' hide N; import 'lib2.dart'; @@ -443,11 +456,8 @@ import 'lib2.dart'; void f(N? n) { print(n); } -''', - matchFixMessage: "Hide others to use 'N' from 'lib2.dart'", - ); - await assertHasFix( - ''' +''', matchFixMessage: "Hide others to use 'N' from 'lib2.dart'"); + await assertHasFix(''' part of 'other.dart'; import 'lib1.dart'; import 'lib2.dart' hide N; @@ -455,15 +465,13 @@ import 'lib2.dart' hide N; void f(N? n) { print(n); } -''', - matchFixMessage: "Hide others to use 'N' from 'lib1.dart'", - ); +''', matchFixMessage: "Hide others to use 'N' from 'lib1.dart'"); } Future test_show_prefixed() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class N {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' as l show N; @@ -484,11 +492,11 @@ void f(l.N? n) { } Future test_triple() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' export 'lib3.dart';'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' mixin M {}'''); - newFile(join(testPackageLibPath,'lib3.dart'), ''' + newFile(join(testPackageLibPath, 'lib3.dart'), ''' mixin M {}'''); await resolveTestCode(''' import 'lib1.dart'; @@ -497,42 +505,51 @@ import 'lib3.dart'; class C with M {} '''); - await assertHasFix(''' + await assertHasFix( + ''' import 'lib1.dart'; import 'lib2.dart' hide M; import 'lib3.dart' hide M; class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); - await assertHasFix(''' +''', + matchFixMessage: "Hide others to use 'M' from 'lib1.dart'", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }, + ); + await assertHasFix( + ''' import 'lib1.dart' hide M; import 'lib2.dart'; import 'lib3.dart' hide M; class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib2.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); - await assertHasFix(''' +''', + matchFixMessage: "Hide others to use 'M' from 'lib2.dart'", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }, + ); + await assertHasFix( + ''' import 'lib1.dart' hide M; import 'lib2.dart' hide M; import 'lib3.dart'; class C with M {} -''', matchFixMessage: "Hide others to use 'M' from 'lib3.dart'", - errorFilter: (error) { - return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; - }); +''', + matchFixMessage: "Hide others to use 'M' from 'lib3.dart'", + errorFilter: (error) { + return error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT; + }, + ); } Future test_triple_oneAliased() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' const foo = 0;'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' as lib; @@ -555,9 +572,9 @@ void f() { } Future test_triple_twoAliased() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' const foo = 0;'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart'; @@ -586,9 +603,9 @@ class ImportRemoveShowTest extends FixProcessorTest { FixKind get kind => DartFixKind.IMPORT_LIBRARY_REMOVE_SHOW; Future test_double() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class N {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N; @@ -609,9 +626,9 @@ void f(N? n) { } Future test_double_aliased() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class N {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' as l show N; @@ -622,11 +639,12 @@ void f(l.N? n) { } '''); await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 2, - matchFixMessages: [ - "Remove show to use 'N' from 'lib1.dart' as l", - "Remove show to use 'N' from 'lib2.dart' as l", - ]); + expectedNumberOfFixesForKind: 2, + matchFixMessages: [ + "Remove show to use 'N' from 'lib1.dart' as l", + "Remove show to use 'N' from 'lib2.dart' as l", + ], + ); await assertHasFix(''' import 'lib1.dart' as l hide N; import 'lib2.dart' as l show N; @@ -646,9 +664,9 @@ void f(l.N? n) { } Future test_double_oneHide() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' const foo = 0;'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' hide foo; @@ -662,10 +680,10 @@ void f() { } Future test_moreShow() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class N {} class M {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N, M; @@ -676,10 +694,9 @@ void f(N? n) { } '''); await assertHasFixesWithoutApplying( - expectedNumberOfFixesForKind: 1, - matchFixMessages: [ - "Remove show to use 'N' from 'lib2.dart'", - ]); + expectedNumberOfFixesForKind: 1, + matchFixMessages: ["Remove show to use 'N' from 'lib2.dart'"], + ); await assertHasFix(''' import 'lib1.dart' show M; import 'lib2.dart'; @@ -692,11 +709,11 @@ void f(N? n) { Future test_moreShow_sort() async { createAnalysisOptionsFile(lints: [LintNames.combinators_ordering]); - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' class N {} class M {} class O {}'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' class N {}'''); await resolveTestCode(''' import 'lib1.dart' show N, O, M; @@ -715,16 +732,16 @@ void f(N? n, O? o) { print(n); } ''', - errorFilter: (error) => - error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT, + errorFilter: + (error) => error.errorCode == CompileTimeErrorCode.AMBIGUOUS_IMPORT, matchFixMessage: "Remove show to use 'N' from 'lib2.dart'", ); } Future test_one_show() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' var foo = 0;'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' var foo = 0;'''); await resolveTestCode(''' import 'lib1.dart' show foo; @@ -745,9 +762,9 @@ void f() { } Future test_triple_twoAliased() async { - newFile(join(testPackageLibPath,'lib1.dart'), ''' + newFile(join(testPackageLibPath, 'lib1.dart'), ''' const foo = 0;'''); - newFile(join(testPackageLibPath,'lib2.dart'), ''' + newFile(join(testPackageLibPath, 'lib2.dart'), ''' const foo = 0;'''); await resolveTestCode(''' import 'lib1.dart'; From c93d33ba8af571b4c274ff382e34f04ed89fcf31 Mon Sep 17 00:00:00 2001 From: FMorschel Date: Mon, 6 Jan 2025 19:07:57 -0300 Subject: [PATCH 28/28] removes fix when multiple equal uri --- .../correction/dart/ambiguous_import_fix.dart | 15 +++--- .../fix/ambiguous_import_fix_test.dart | 46 +++++++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart b/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart index 7fbf3ba1555a..faa4b0f885b6 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/ambiguous_import_fix.dart @@ -48,6 +48,11 @@ class AmbiguousImportFix extends MultiCorrectionProducer { 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 []; } @@ -161,9 +166,8 @@ class _ImportAddHide extends ResolvedCorrectionProducer { @override CorrectionApplicability get applicability => - // TODO(applicability): comment on why. - CorrectionApplicability - .singleLocation; + // TODO(applicability): comment on why. + CorrectionApplicability.singleLocation; @override List get fixArguments { @@ -235,9 +239,8 @@ class _ImportRemoveShow extends ResolvedCorrectionProducer { @override CorrectionApplicability get applicability => - // TODO(applicability): comment on why. - CorrectionApplicability - .singleLocation; + // TODO(applicability): comment on why. + CorrectionApplicability.singleLocation; @override List get fixArguments { diff --git a/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart b/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart index 8673b187c6bf..c2b57143b792 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/ambiguous_import_fix_test.dart @@ -150,6 +150,29 @@ class C with M {} ); } + Future test_double_equal_importUris() async { + // https://github.com/dart-lang/sdk/issues/56830#issuecomment-2573945155 + newFile(join(testPackageLibPath, 'lib1.dart'), ''' +var foo = 0; +var bar = 0; +var baz = 0; +'''); + newFile(join(testPackageLibPath, 'lib2.dart'), ''' +var foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' hide bar; +import 'lib1.dart' hide baz; +import 'lib2.dart'; + +void f() { + print(bar); + print(baz); + print(foo); +} +'''); + await assertNoFix(); + } + Future test_double_exportedByImport() async { newFile(join(testPackageLibPath, 'lib1.dart'), ''' mixin M {}'''); @@ -663,6 +686,29 @@ void f(l.N? n) { ''', matchFixMessage: "Remove show to use 'N' from 'lib1.dart' as l"); } + Future test_double_equal_importUris() async { + // https://github.com/dart-lang/sdk/issues/56830#issuecomment-2573945155 + newFile(join(testPackageLibPath, 'lib1.dart'), ''' +var foo = 0; +var bar = 0; +var baz = 0; +'''); + newFile(join(testPackageLibPath, 'lib2.dart'), ''' +var foo = 0;'''); + await resolveTestCode(''' +import 'lib1.dart' show bar, foo; +import 'lib1.dart' show baz, foo; +import 'lib2.dart'; + +void f() { + print(bar); + print(baz); + print(foo); +} +'''); + await assertNoFix(); + } + Future test_double_oneHide() async { newFile(join(testPackageLibPath, 'lib1.dart'), ''' const foo = 0;''');