From c24f36d47095a933203069e8cd52b69aa7fa4eda Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Mon, 5 Dec 2022 22:32:19 +0000 Subject: [PATCH] [analysis_server] Use public (non-src) URIs for Flutter snippet imports Fixes https://github.com/dart-lang/sdk/issues/49081. Change-Id: I0734b4f45c72d70f7b32640bed6b6ec2e8130c01 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/273841 Reviewed-by: Brian Wilkerson Commit-Queue: Brian Wilkerson --- pkg/analysis_server/lib/src/cider/fixes.dart | 2 +- .../src/lsp/handlers/handler_completion.dart | 3 + .../lib/src/services/correction/fix.dart | 2 +- .../dart/flutter_stateful_widget.dart | 4 +- ...lutter_stateful_widget_with_animation.dart | 4 +- .../dart/flutter_stateless_widget.dart | 4 +- .../services/snippets/snippet_producer.dart | 35 +++++++-- .../test/lsp/completion_dart_test.dart | 7 +- .../dart/flutter_stateful_widget_test.dart | 43 +++-------- ...widget_with_animation_controller_test.dart | 47 +++--------- .../dart/flutter_stateless_widget_test.dart | 50 ++++--------- .../services/snippets/dart/test_support.dart | 29 ++++++++ .../src/dart/analysis/file_state_filter.dart | 24 +++++++ .../src/services}/top_level_declarations.dart | 54 +++++++++++--- .../change_builder/change_builder_core.dart | 7 +- .../change_builder/change_builder_dart.dart | 72 ++++++++++++++----- .../change_builder/change_builder_core.dart | 6 +- 17 files changed, 241 insertions(+), 152 deletions(-) rename pkg/{analysis_server/lib/src/services/correction/fix/dart => analyzer/lib/src/services}/top_level_declarations.dart (55%) diff --git a/pkg/analysis_server/lib/src/cider/fixes.dart b/pkg/analysis_server/lib/src/cider/fixes.dart index 60d04c3d0a9..b2cbecf0cab 100644 --- a/pkg/analysis_server/lib/src/cider/fixes.dart +++ b/pkg/analysis_server/lib/src/cider/fixes.dart @@ -5,7 +5,6 @@ import 'package:analysis_server/plugin/edit/fix/fix_core.dart'; import 'package:analysis_server/src/services/correction/change_workspace.dart'; import 'package:analysis_server/src/services/correction/fix.dart'; -import 'package:analysis_server/src/services/correction/fix/dart/top_level_declarations.dart'; import 'package:analysis_server/src/services/correction/fix_internal.dart'; import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/element/element.dart'; @@ -15,6 +14,7 @@ import 'package:analyzer/source/line_info.dart'; import 'package:analyzer/src/dart/analysis/file_state.dart'; import 'package:analyzer/src/dart/analysis/performance_logger.dart'; import 'package:analyzer/src/dart/micro/resolve_file.dart'; +import 'package:analyzer/src/services/top_level_declarations.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_workspace.dart'; class CiderErrorFixes { diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart index 25cc471127c..c24299d2a5a 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart +++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart @@ -519,6 +519,9 @@ class CompletionHandler extends MessageHandler try { unrankedResults = await performance.runAsync('getSnippets', (performance) async { + // TODO(dantup): Pass `fuzzy` into here so we can filter snippets + // before computing them to avoid looking up Element->Public Library + // if they won't be included. final snippets = await _getDartSnippetItems( clientCapabilities: capabilities, unit: unit, diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index b0184a2bcd3..633a16c1c39 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -4,13 +4,13 @@ import 'package:analysis_server/plugin/edit/fix/fix_dart.dart'; import 'package:analysis_server/src/services/correction/fix/dart/extensions.dart'; -import 'package:analysis_server/src/services/correction/fix/dart/top_level_declarations.dart'; import 'package:analysis_server/src/services/correction/fix_internal.dart'; import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/error/error.dart'; import 'package:analyzer/instrumentation/service.dart'; import 'package:analyzer/src/error/codes.dart'; +import 'package:analyzer/src/services/top_level_declarations.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_workspace.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; diff --git a/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateful_widget.dart b/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateful_widget.dart index 7babe62c453..00a79a8f7b2 100644 --- a/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateful_widget.dart +++ b/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateful_widget.dart @@ -31,7 +31,9 @@ class FlutterStatefulWidget extends FlutterSnippetProducer final classStatefulWidget = this.classStatefulWidget!; final classState = this.classState!; - await builder.addDartFileEdit(request.filePath, (builder) { + await builder.addDartFileEdit(request.filePath, (builder) async { + await addImports(builder); + builder.addReplacement(request.replacementRange, (builder) { // Write the StatefulWidget class builder.writeClassDeclaration( diff --git a/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateful_widget_with_animation.dart b/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateful_widget_with_animation.dart index 48377f218f5..c9709934ed1 100644 --- a/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateful_widget_with_animation.dart +++ b/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateful_widget_with_animation.dart @@ -37,7 +37,9 @@ class FlutterStatefulWidgetWithAnimationController final classSingleTickerProviderStateMixin = this.classSingleTickerProviderStateMixin!; - await builder.addDartFileEdit(request.filePath, (builder) { + await builder.addDartFileEdit(request.filePath, (builder) async { + await addImports(builder); + builder.addReplacement(request.replacementRange, (builder) { // Write the StatefulWidget class builder.writeClassDeclaration( diff --git a/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateless_widget.dart b/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateless_widget.dart index e7711d36a29..86ca320bec0 100644 --- a/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateless_widget.dart +++ b/pkg/analysis_server/lib/src/services/snippets/dart/flutter_stateless_widget.dart @@ -28,7 +28,9 @@ class FlutterStatelessWidget extends FlutterSnippetProducer // Checked by isValid(). final classStatelessWidget = this.classStatelessWidget!; - await builder.addDartFileEdit(request.filePath, (builder) { + await builder.addDartFileEdit(request.filePath, (builder) async { + await addImports(builder); + builder.addReplacement(request.replacementRange, (builder) { builder.writeClassDeclaration( widgetClassName, diff --git a/pkg/analysis_server/lib/src/services/snippets/snippet_producer.dart b/pkg/analysis_server/lib/src/services/snippets/snippet_producer.dart index fde61f41119..5f04c486bc8 100644 --- a/pkg/analysis_server/lib/src/services/snippets/snippet_producer.dart +++ b/pkg/analysis_server/lib/src/services/snippets/snippet_producer.dart @@ -13,6 +13,8 @@ import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/src/dart/analysis/session_helper.dart'; import 'package:analyzer/src/lint/linter.dart'; +import 'package:analyzer_plugin/src/utilities/change_builder/change_builder_dart.dart' + show DartFileEditBuilderImpl; import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart'; import 'package:meta/meta.dart'; @@ -50,13 +52,38 @@ abstract class FlutterSnippetProducer extends DartSnippetProducer { late ClassElement? classWidget; late ClassElement? classPlaceholder; + /// Elements that need to be imported for generated code to be valid. + /// + /// Calling [getClass] or [getMixin] records elements in this set. + /// Calling [addImports] will add any required imports to the supplied + /// builder. + final Set _requiredElementImports = {}; + FlutterSnippetProducer(super.request); - Future getClass(String name) => - sessionHelper.getClass(flutter.widgetsUri, name); + /// Adds public imports for any elements fetched by [getClass] and [getMixin] + /// to [builder]. + Future addImports(DartFileEditBuilder builder) async { + final dartBuilder = builder as DartFileEditBuilderImpl; + await Future.wait( + _requiredElementImports.map(dartBuilder.importElementLibrary)); + } - Future getMixin(String name) => - sessionHelper.getMixin(flutter.widgetsUri, name); + Future getClass(String name) async { + final class_ = await sessionHelper.getClass(flutter.widgetsUri, name); + if (class_ != null) { + _requiredElementImports.add(class_); + } + return class_; + } + + Future getMixin(String name) async { + final mixin = await sessionHelper.getMixin(flutter.widgetsUri, name); + if (mixin != null) { + _requiredElementImports.add(mixin); + } + return mixin; + } DartType getType( InterfaceElement classElement, [ diff --git a/pkg/analysis_server/test/lsp/completion_dart_test.dart b/pkg/analysis_server/test/lsp/completion_dart_test.dart index 03bb692f9fb..1624c59952a 100644 --- a/pkg/analysis_server/test/lsp/completion_dart_test.dart +++ b/pkg/analysis_server/test/lsp/completion_dart_test.dart @@ -3731,8 +3731,7 @@ void f() { class FlutterSnippetCompletionTest extends SnippetCompletionTest { /// Standard import statements expected for basic Widgets. String get expectedImports => ''' -import 'package:flutter/src/widgets/framework.dart'; -import 'package:flutter/src/widgets/placeholder.dart';'''; +import 'package:flutter/widgets.dart';'''; /// Nullability suffix expected in this test class. /// @@ -4007,9 +4006,7 @@ class FlutterSnippetCompletionWithoutNullSafetyTest extends FlutterSnippetCompletionTest { @override String get expectedImports => ''' -import 'package:flutter/src/foundation/key.dart'; -import 'package:flutter/src/widgets/framework.dart'; -import 'package:flutter/src/widgets/placeholder.dart';'''; +import 'package:flutter/widgets.dart';'''; @override String get expectedNullableSuffix => ''; diff --git a/pkg/analysis_server/test/services/snippets/dart/flutter_stateful_widget_test.dart b/pkg/analysis_server/test/services/snippets/dart/flutter_stateful_widget_test.dart index 93d38e58c30..1381f635c03 100644 --- a/pkg/analysis_server/test/services/snippets/dart/flutter_stateful_widget_test.dart +++ b/pkg/analysis_server/test/services/snippets/dart/flutter_stateful_widget_test.dart @@ -4,6 +4,7 @@ import 'package:analysis_server/src/protocol_server.dart'; import 'package:analysis_server/src/services/snippets/dart/flutter_stateful_widget.dart'; +import 'package:analyzer/src/test_utilities/test_code_format.dart'; import 'package:test/test.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; @@ -38,9 +39,7 @@ class FlutterStatefulWidgetTest extends FlutterSnippetProducerTest { code = SourceEdit.applySequence(code, edit.edits); } expect(code, ''' -import 'package:flutter/src/foundation/key.dart'; -import 'package:flutter/src/widgets/framework.dart'; -import 'package:flutter/src/widgets/placeholder.dart'; +import 'package:flutter/widgets.dart'; class MyWidget extends StatefulWidget { const MyWidget({Key? key}) : super(key: key); @@ -69,44 +68,22 @@ class _MyWidgetState extends State { final snippet = await expectValidSnippet('^'); expect(snippet.prefix, prefix); expect(snippet.label, label); - var code = ''; - expect(snippet.change.edits, hasLength(1)); - for (var edit in snippet.change.edits) { - code = SourceEdit.applySequence(code, edit.edits); - } - expect(code, ''' -import 'package:flutter/src/widgets/framework.dart'; -import 'package:flutter/src/widgets/placeholder.dart'; + final expected = TestCode.parse(''' +import 'package:flutter/widgets.dart'; -class MyWidget extends StatefulWidget { - const MyWidget({super.key}); +class /*0*/MyWidget extends StatefulWidget { + const /*1*/MyWidget({super.key}); @override - State createState() => _MyWidgetState(); + State createState() => _/*3*/MyWidgetState(); } -class _MyWidgetState extends State { +class _/*4*/MyWidgetState extends State { @override Widget build(BuildContext context) { - return const Placeholder(); + return /*[0*/const Placeholder()/*0]*/; } }'''); - expect(snippet.change.selection!.file, testFile); - expect(snippet.change.selection!.offset, 358); - expect(snippet.change.selectionLength, 19); - expect(snippet.change.linkedEditGroups.map((group) => group.toJson()), [ - { - 'positions': [ - {'file': testFile, 'offset': 115}, - {'file': testFile, 'offset': 157}, - {'file': testFile, 'offset': 201}, - {'file': testFile, 'offset': 229}, - {'file': testFile, 'offset': 256}, - {'file': testFile, 'offset': 284}, - ], - 'length': 8, - 'suggestions': [] - } - ]); + assertFlutterSnippetChange(snippet.change, 'MyWidget', expected); } } diff --git a/pkg/analysis_server/test/services/snippets/dart/flutter_stateful_widget_with_animation_controller_test.dart b/pkg/analysis_server/test/services/snippets/dart/flutter_stateful_widget_with_animation_controller_test.dart index 64e0de83588..e6d8cb15799 100644 --- a/pkg/analysis_server/test/services/snippets/dart/flutter_stateful_widget_with_animation_controller_test.dart +++ b/pkg/analysis_server/test/services/snippets/dart/flutter_stateful_widget_with_animation_controller_test.dart @@ -4,6 +4,7 @@ import 'package:analysis_server/src/protocol_server.dart'; import 'package:analysis_server/src/services/snippets/dart/flutter_stateful_widget_with_animation.dart'; +import 'package:analyzer/src/test_utilities/test_code_format.dart'; import 'package:test/test.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; @@ -39,11 +40,7 @@ class FlutterStatefulWidgetWithAnimationControllerTest code = SourceEdit.applySequence(code, edit.edits); } expect(code, ''' -import 'package:flutter/src/animation/animation_controller.dart'; -import 'package:flutter/src/foundation/key.dart'; -import 'package:flutter/src/widgets/framework.dart'; -import 'package:flutter/src/widgets/placeholder.dart'; -import 'package:flutter/src/widgets/ticker_provider.dart'; +import 'package:flutter/widgets.dart'; class MyWidget extends StatefulWidget { const MyWidget({Key? key}) : super(key: key); @@ -87,25 +84,17 @@ class _MyWidgetState extends State final snippet = await expectValidSnippet('^'); expect(snippet.prefix, prefix); expect(snippet.label, label); - var code = ''; - expect(snippet.change.edits, hasLength(1)); - for (var edit in snippet.change.edits) { - code = SourceEdit.applySequence(code, edit.edits); - } - expect(code, ''' -import 'package:flutter/src/animation/animation_controller.dart'; -import 'package:flutter/src/widgets/framework.dart'; -import 'package:flutter/src/widgets/placeholder.dart'; -import 'package:flutter/src/widgets/ticker_provider.dart'; + final expected = TestCode.parse(''' +import 'package:flutter/widgets.dart'; -class MyWidget extends StatefulWidget { - const MyWidget({super.key}); +class /*0*/MyWidget extends StatefulWidget { + const /*1*/MyWidget({super.key}); @override - State createState() => _MyWidgetState(); + State createState() => _/*3*/MyWidgetState(); } -class _MyWidgetState extends State +class _/*4*/MyWidgetState extends State with SingleTickerProviderStateMixin { late AnimationController _controller; @@ -123,25 +112,9 @@ class _MyWidgetState extends State @override Widget build(BuildContext context) { - return const Placeholder(); + return /*[0*/const Placeholder()/*0]*/; } }'''); - expect(snippet.change.selection!.file, testFile); - expect(snippet.change.selection!.offset, 761); - expect(snippet.change.selectionLength, 19); - expect(snippet.change.linkedEditGroups.map((group) => group.toJson()), [ - { - 'positions': [ - {'file': testFile, 'offset': 240}, - {'file': testFile, 'offset': 282}, - {'file': testFile, 'offset': 326}, - {'file': testFile, 'offset': 354}, - {'file': testFile, 'offset': 381}, - {'file': testFile, 'offset': 409}, - ], - 'length': 8, - 'suggestions': [] - } - ]); + assertFlutterSnippetChange(snippet.change, 'MyWidget', expected); } } diff --git a/pkg/analysis_server/test/services/snippets/dart/flutter_stateless_widget_test.dart b/pkg/analysis_server/test/services/snippets/dart/flutter_stateless_widget_test.dart index dd88d45c3c3..439878a694c 100644 --- a/pkg/analysis_server/test/services/snippets/dart/flutter_stateless_widget_test.dart +++ b/pkg/analysis_server/test/services/snippets/dart/flutter_stateless_widget_test.dart @@ -2,8 +2,8 @@ // 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/protocol_server.dart'; import 'package:analysis_server/src/services/snippets/dart/flutter_stateless_widget.dart'; +import 'package:analyzer/src/test_utilities/test_code_format.dart'; import 'package:test/test.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; @@ -32,24 +32,18 @@ class FlutterStatelessWidgetTest extends FlutterSnippetProducerTest { final snippet = await expectValidSnippet('^'); expect(snippet.prefix, prefix); expect(snippet.label, label); - var code = ''; - expect(snippet.change.edits, hasLength(1)); - for (var edit in snippet.change.edits) { - code = SourceEdit.applySequence(code, edit.edits); - } - expect(code, ''' -import 'package:flutter/src/foundation/key.dart'; -import 'package:flutter/src/widgets/framework.dart'; -import 'package:flutter/src/widgets/placeholder.dart'; + final expected = TestCode.parse(''' +import 'package:flutter/widgets.dart'; -class MyWidget extends StatelessWidget { - const MyWidget({Key? key}) : super(key: key); +class /*0*/MyWidget extends StatelessWidget { + const /*1*/MyWidget({Key? key}) : super(key: key); @override Widget build(BuildContext context) { - return const Placeholder(); + return /*[0*/const Placeholder()/*0]*/; } }'''); + assertFlutterSnippetChange(snippet.change, 'MyWidget', expected); } Future test_notValid_notFlutterProject() async { @@ -64,35 +58,17 @@ class MyWidget extends StatelessWidget { final snippet = await expectValidSnippet('^'); expect(snippet.prefix, prefix); expect(snippet.label, label); - var code = ''; - expect(snippet.change.edits, hasLength(1)); - for (var edit in snippet.change.edits) { - code = SourceEdit.applySequence(code, edit.edits); - } - expect(code, ''' -import 'package:flutter/src/widgets/framework.dart'; -import 'package:flutter/src/widgets/placeholder.dart'; + final expected = TestCode.parse(''' +import 'package:flutter/widgets.dart'; -class MyWidget extends StatelessWidget { - const MyWidget({super.key}); +class /*0*/MyWidget extends StatelessWidget { + const /*1*/MyWidget({super.key}); @override Widget build(BuildContext context) { - return const Placeholder(); + return /*[0*/const Placeholder()/*0]*/; } }'''); - expect(snippet.change.selection!.file, testFile); - expect(snippet.change.selection!.offset, 244); - expect(snippet.change.selectionLength, 19); - expect(snippet.change.linkedEditGroups.map((group) => group.toJson()), [ - { - 'positions': [ - {'file': testFile, 'offset': 115}, - {'file': testFile, 'offset': 158}, - ], - 'length': 8, - 'suggestions': [] - } - ]); + assertFlutterSnippetChange(snippet.change, 'MyWidget', expected); } } diff --git a/pkg/analysis_server/test/services/snippets/dart/test_support.dart b/pkg/analysis_server/test/services/snippets/dart/test_support.dart index 2d0a5950394..5c206e7ea75 100644 --- a/pkg/analysis_server/test/services/snippets/dart/test_support.dart +++ b/pkg/analysis_server/test/services/snippets/dart/test_support.dart @@ -2,9 +2,11 @@ // 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/protocol_server.dart'; import 'package:analysis_server/src/services/snippets/dart_snippet_request.dart'; import 'package:analysis_server/src/services/snippets/snippet.dart'; import 'package:analysis_server/src/services/snippets/snippet_manager.dart'; +import 'package:analyzer/src/test_utilities/test_code_format.dart'; import 'package:test/test.dart'; import '../../../abstract_single_unit.dart'; @@ -52,6 +54,33 @@ abstract class DartSnippetProducerTest extends AbstractSingleUnitTest { } abstract class FlutterSnippetProducerTest extends DartSnippetProducerTest { + /// Asserts that [change] matches the code in [expected], has a selection + /// matching its range and a single linked edit group containing all of its + /// positions. + void assertFlutterSnippetChange( + SourceChange change, + String linkedGroupText, + TestCode expected, + ) { + expect(change.edits, hasLength(1)); + final code = SourceEdit.applySequence('', change.edits.single.edits); + expect(code, expected.code); + + expect(change.selection!.file, testFile); + expect(change.selection!.offset, expected.range.sourceRange.offset); + expect(change.selectionLength, expected.range.sourceRange.length); + expect(change.linkedEditGroups.map((group) => group.toJson()), [ + { + 'positions': [ + for (final position in expected.positions) + {'file': testFile, 'offset': position.offset}, + ], + 'length': linkedGroupText.length, + 'suggestions': [] + } + ]); + } + /// Checks snippets can produce edits where the imports and snippet will be /// inserted at the same location. /// diff --git a/pkg/analyzer/lib/src/dart/analysis/file_state_filter.dart b/pkg/analyzer/lib/src/dart/analysis/file_state_filter.dart index 5396bc0cc4a..81e213aee02 100644 --- a/pkg/analyzer/lib/src/dart/analysis/file_state_filter.dart +++ b/pkg/analyzer/lib/src/dart/analysis/file_state_filter.dart @@ -18,6 +18,14 @@ abstract class FileStateFilter { } } + /// Return a filter of files in the package named [packageName]. + factory FileStateFilter.packageName( + String? packageName, { + required bool excludeSrc, + }) { + return _PackageNameFilter(packageName, excludeSrc: excludeSrc); + } + bool shouldInclude(FileState file); } @@ -32,6 +40,22 @@ class _AnyFilter implements FileStateFilter { } } +/// Matches any file in the package [packageName]. +/// +/// If [packageName] is `null`, matches files that also have no `packageName`. +class _PackageNameFilter implements FileStateFilter { + final String? packageName; + final bool excludeSrc; + + _PackageNameFilter(this.packageName, {required this.excludeSrc}); + + @override + bool shouldInclude(FileState file) { + var uri = file.uriProperties; + return uri.packageName == packageName && !(uri.isSrc && excludeSrc); + } +} + class _PubFilter implements FileStateFilter { final PubWorkspacePackage targetPackage; final String? targetPackageName; diff --git a/pkg/analysis_server/lib/src/services/correction/fix/dart/top_level_declarations.dart b/pkg/analyzer/lib/src/services/top_level_declarations.dart similarity index 55% rename from pkg/analysis_server/lib/src/services/correction/fix/dart/top_level_declarations.dart rename to pkg/analyzer/lib/src/services/top_level_declarations.dart index 520fe283869..0b9e97d6424 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix/dart/top_level_declarations.dart +++ b/pkg/analyzer/lib/src/services/top_level_declarations.dart @@ -19,6 +19,42 @@ class TopLevelDeclarations { return analysisContext as DriverBasedAnalysisContext; } + /// Return the first public library that that exports (but does not necessary + /// declare) [element]. + Future publiclyExporting(Element element) async { + var declarationFilePath = element.source?.fullName; + if (declarationFilePath == null) { + return null; + } + + var analysisDriver = _analysisContext.driver; + var fsState = analysisDriver.fsState; + await analysisDriver.discoverAvailableFiles(); + + var declarationFile = fsState.getFileForPath(declarationFilePath); + var declarationPackage = declarationFile.uriProperties.packageName; + + for (var file in fsState.knownFiles.toList()) { + var uri = file.uriProperties; + // Only search the package that contains the declaration and its public + // libraries. + if (uri.packageName != declarationPackage || uri.isSrc) { + continue; + } + + var elementResult = await analysisDriver.getLibraryByUri(file.uriStr); + if (elementResult is! LibraryElementResult) { + continue; + } + + if (_findElement(elementResult.element, element.displayName) != null) { + return elementResult.element; + } + } + + return null; + } + /// Return the mapping from a library (that is available to this context) to /// a top-level declaration that is exported (not necessary declared) by this /// library, and has the requested base name. For getters and setters the @@ -55,17 +91,15 @@ class TopLevelDeclarations { LibraryElement libraryElement, String baseName, ) { - void addSingle(String name) { - var element = libraryElement.exportNamespace.get(name); - if (element is PropertyAccessorElement) { - element = element.variable; - } - if (element != null) { - result[libraryElement] = element; - } + var element = _findElement(libraryElement, baseName); + if (element != null) { + result[libraryElement] = element; } + } - addSingle(baseName); - addSingle('$baseName='); + static Element? _findElement(LibraryElement libraryElement, String name) { + var element = libraryElement.exportNamespace.get(name) ?? + libraryElement.exportNamespace.get('$name='); + return element is PropertyAccessorElement ? element.variable : element; } } diff --git a/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_core.dart b/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_core.dart index b9c1eb26ed9..d4c89bf2de2 100644 --- a/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_core.dart +++ b/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_core.dart @@ -2,6 +2,7 @@ // 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 'dart:async'; import 'dart:collection'; import 'package:analyzer/dart/analysis/results.dart'; @@ -118,8 +119,8 @@ class ChangeBuilderImpl implements ChangeBuilder { } @override - Future addDartFileEdit( - String path, void Function(DartFileEditBuilder builder) buildFileEdit, + Future addDartFileEdit(String path, + FutureOr Function(DartFileEditBuilder builder) buildFileEdit, {ImportPrefixGenerator? importPrefixGenerator, bool createEditsForImports = true}) async { if (_genericFileEditBuilders.containsKey(path)) { @@ -147,7 +148,7 @@ class ChangeBuilderImpl implements ChangeBuilder { } if (builder != null) { builder.importPrefixGenerator = importPrefixGenerator; - buildFileEdit(builder); + await buildFileEdit(builder); } } diff --git a/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart b/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart index 47d9a538164..b11edb57657 100644 --- a/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart +++ b/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart @@ -14,6 +14,7 @@ import 'package:analyzer/dart/element/type_provider.dart'; import 'package:analyzer/src/dart/ast/utilities.dart'; import 'package:analyzer/src/dart/element/type.dart'; import 'package:analyzer/src/generated/source.dart'; +import 'package:analyzer/src/services/top_level_declarations.dart'; import 'package:analyzer_plugin/protocol/protocol_common.dart' hide Element, ElementKind; import 'package:analyzer_plugin/src/utilities/change_builder/change_builder_core.dart'; @@ -1172,7 +1173,7 @@ class DartEditBuilderImpl extends EditBuilderImpl implements DartEditBuilder { if (import != null) { var prefix = import.prefix; if (prefix != null) { - write(prefix.element.displayName); + write(prefix); write('.'); } } else { @@ -1401,7 +1402,11 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl /// A mapping from libraries that need to be imported in order to make visible /// the names used in generated code, to information about these imports. - Map librariesToImport = {}; + Map librariesToImport = {}; + + /// A mapping of [Element]s to pending imports that will be added to make + /// them visible in the generated code. + final Map _elementLibrariesToImport = {}; /// Initialize a newly created builder to build a source file edit within the /// change being built by the given [changeBuilder]. The file being edited has @@ -1470,6 +1475,9 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl for (var entry in librariesToImport.entries) { copy.librariesToImport[entry.key] = entry.value; } + for (var entry in _elementLibrariesToImport.entries) { + copy._elementLibrariesToImport[entry.key] = entry.value; + } return copy; } @@ -1520,6 +1528,34 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl ); } + /// Arrange to have an import added that makes [element] available. + /// + /// If [element] is already available in the current library, does nothing. + /// + /// If the library [element] is declared in is inside the `src` folder, will + /// try to locate a public URI to import instead. + Future importElementLibrary(Element element) async { + // TODO(dantup): Add the ability to pass a cache in to this function so + // multiple callers can avoid looking up the same elements. + if (_isDefinedLocally(element) || _getImportElement(element) != null) { + return; + } + + var libraryWithElement = + await TopLevelDeclarations(resolvedUnit).publiclyExporting(element); + if (libraryWithElement != null) { + _elementLibrariesToImport[element] = + _importLibrary(libraryWithElement.source.uri); + return; + } + + // If we didn't find one, use the original URI. + var uri = element.source?.uri; + if (uri != null) { + _importLibrary(uri); + } + } + @override String importLibrary(Uri uri, {String? prefix}) { return _importLibrary(uri, prefix: prefix).uriText; @@ -1590,7 +1626,7 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl } /// Adds edits ensure that all the [imports] are imported into the library. - void _addLibraryImports(Iterable<_LibraryToImport> imports) { + void _addLibraryImports(Iterable<_LibraryImport> imports) { // Prepare information about existing imports. LibraryDirective? libraryDirective; var importDirectives = []; @@ -1614,7 +1650,7 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl importList.sort((a, b) => a.uriText.compareTo(b.uriText)); var quote = codeStyleOptions.preferredQuoteForUris(importDirectives); - void writeImport(EditBuilder builder, _LibraryToImport import) { + void writeImport(EditBuilder builder, _LibraryImport import) { builder.write('import $quote'); builder.write(import.uriText); builder.write(quote); @@ -1817,17 +1853,21 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl ); } - /// Return the import element used to import the given [element] into the - /// target library, or `null` if the element was not imported, such as when - /// the element is declared in the same library. - LibraryImportElement? _getImportElement(Element element) { + /// Return information about the library used to import the given [element] + /// into the target library, or `null` if the element was not imported, such + /// as when the element is declared in the same library. + /// + /// The result may be an existing import, or one that is pending. + _LibraryImport? _getImportElement(Element element) { for (var import in resolvedUnit.libraryElement.libraryImports) { var definedNames = import.namespace.definedNames; if (definedNames.containsValue(element)) { - return import; + return _LibraryImport(import.librarySource.uri.toString(), + import.prefix?.element.displayName); } } - return null; + + return _elementLibrariesToImport[element]; } Iterable _getImportsForUri(Uri uri) sync* { @@ -1878,7 +1918,7 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl /// /// [uri] may be converted from an absolute URI to a relative URI depending on /// user preferences/lints unless [forceAbsolute] or [forceRelative] are `true`. - _LibraryToImport _importLibrary( + _LibraryImport _importLibrary( Uri uri, { String? prefix, bool forceAbsolute = false, @@ -1890,7 +1930,7 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl forceAbsolute: forceAbsolute, forceRelative: forceRelative); prefix ??= importPrefixGenerator != null ? importPrefixGenerator!(uri) : null; - import = _LibraryToImport(uriText, prefix); + import = _LibraryImport(uriText, prefix); (libraryChangeBuilder ?? this).librariesToImport[uri] = import; } return import; @@ -1995,19 +2035,19 @@ class _EnclosingElementFinder { } } -/// Information about a new library to import. -class _LibraryToImport { +/// Information about a library import. +class _LibraryImport { final String uriText; final String? prefix; - _LibraryToImport(this.uriText, this.prefix); + _LibraryImport(this.uriText, this.prefix); @override int get hashCode => uriText.hashCode; @override bool operator ==(other) { - return other is _LibraryToImport && + return other is _LibraryImport && other.uriText == uriText && other.prefix == prefix; } diff --git a/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_core.dart b/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_core.dart index 139215d823c..24ee6565a47 100644 --- a/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_core.dart +++ b/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_core.dart @@ -2,6 +2,8 @@ // 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 'dart:async'; + import 'package:analyzer/dart/analysis/session.dart'; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer_plugin/protocol/protocol_common.dart'; @@ -40,8 +42,8 @@ abstract class ChangeBuilder { /// /// Setting [createEditsForImports] to `false` will prevent edits being /// produced to add `import` statements for any unimported types. - Future addDartFileEdit( - String path, void Function(DartFileEditBuilder builder) buildFileEdit, + Future addDartFileEdit(String path, + FutureOr Function(DartFileEditBuilder builder) buildFileEdit, {ImportPrefixGenerator importPrefixGenerator, bool createEditsForImports = true});