From 046f3117b67108374ac363a512560c3f1abf9a10 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Tue, 2 Apr 2024 16:09:26 +0000 Subject: [PATCH] [analysis_server] Fix navigation on augmentation directives Change-Id: I0972d6c9aa93f1bac180504da96e63ed9f04e63e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360260 Commit-Queue: Brian Wilkerson Reviewed-by: Konstantin Shcheglov Reviewed-by: Brian Wilkerson --- .../notification_navigation_test.dart | 32 +++++ .../test/lsp/definition_test.dart | 113 ++++++++++-------- .../lib/utilities/analyzer_converter.dart | 12 +- .../utilities/navigation/navigation_dart.dart | 23 +++- 4 files changed, 121 insertions(+), 59 deletions(-) diff --git a/pkg/analysis_server/test/analysis/notification_navigation_test.dart b/pkg/analysis_server/test/analysis/notification_navigation_test.dart index ac727cae0d40..4f08a67aa3d1 100644 --- a/pkg/analysis_server/test/analysis/notification_navigation_test.dart +++ b/pkg/analysis_server/test/analysis/notification_navigation_test.dart @@ -1709,6 +1709,22 @@ class A { } } + Future test_string_augmentLibrary() async { + var augmentedCode = 'import augment "test.dart";'; + var augmentedFile = + newFile('$testPackageLibPath/augmented.dart', augmentedCode).path; + addTestFile('library augment "augmented.dart";'); + await prepareNavigation(); + assertHasRegionString('"augmented.dart"'); + assertHasFileTarget(augmentedFile, 0, 0); + } + + Future test_string_augmentLibrary_unresolvedUri() async { + addTestFile('library augment "no.dart";'); + await prepareNavigation(); + assertNoRegionString('"no.dart"'); + } + Future test_string_configuration() async { newFile('$testPackageLibPath/lib.dart', '').path; var lib2File = newFile('$testPackageLibPath/lib2.dart', '').path; @@ -1755,6 +1771,22 @@ class A { assertNoRegionString('"no.dart"'); } + Future test_string_importAugment() async { + var augmentCode = 'library augment "test.dart";'; + var augmentFile = + newFile('$testPackageLibPath/augment.dart', augmentCode).path; + addTestFile('import augment "augment.dart";'); + await prepareNavigation(); + assertHasRegionString('"augment.dart"'); + assertHasFileTarget(augmentFile, 0, 0); + } + + Future test_string_importAugment_unresolvedUri() async { + addTestFile('import augment "no.dart";'); + await prepareNavigation(); + assertNoRegionString('"no.dart"'); + } + Future test_string_part() async { var unitCode = 'part of lib; f() {}'; var unitFile = newFile('$testPackageLibPath/test_unit.dart', unitCode).path; diff --git a/pkg/analysis_server/test/lsp/definition_test.dart b/pkg/analysis_server/test/lsp/definition_test.dart index b6626321e269..6230389e900c 100644 --- a/pkg/analysis_server/test/lsp/definition_test.dart +++ b/pkg/analysis_server/test/lsp/definition_test.dart @@ -268,6 +268,46 @@ class [!A!] { await testContents(contents); } + Future test_directive_augmentLibrary() async { + await verifyDirective( + source: "library augment 'destin^ation.dart';", + destination: "import augment 'source.dart';", + ); + } + + Future test_directive_export() async { + await verifyDirective( + source: "export 'destin^ation.dart';", + ); + } + + Future test_directive_import() async { + await verifyDirective( + source: "import 'desti^nation.dart';", + ); + } + + Future test_directive_importAugment() async { + await verifyDirective( + source: "import augment 'destin^ation.dart';", + destination: "library augment 'source.dart';", + ); + } + + Future test_directive_part() async { + await verifyDirective( + source: "part 'desti^nation.dart';", + destination: "part of 'source.dart';", + ); + } + + Future test_directive_partOf() async { + await verifyDirective( + source: "part of 'destin^ation.dart';", + destination: "part 'source.dart';", + ); + } + Future test_fieldFormalParam() async { final contents = ''' class A { @@ -571,54 +611,6 @@ class A {} ); } - Future test_partFilename() async { - final mainContents = ''' -part 'pa^rt.dart'; - '''; - - final partContents = ''' -part of 'main.dart'; - '''; - - final partFilePath = join(projectFolderPath, 'lib', 'part.dart'); - final partFileUri = toUri(partFilePath); - - final mainCode = TestCode.parse(mainContents); - final partCode = TestCode.parse(partContents); - - newFile(mainFilePath, mainCode.code); - newFile(partFilePath, partCode.code); - await initialize(); - final res = - await getDefinitionAsLocation(mainFileUri, mainCode.position.position); - - expect(res.single.uri, equals(partFileUri)); - } - - Future test_partOfFilename() async { - final mainContents = ''' -part 'part.dart'; - '''; - - final partContents = ''' -part of 'ma^in.dart'; - '''; - - final partFilePath = join(projectFolderPath, 'lib', 'part.dart'); - final partFileUri = toUri(partFilePath); - - final mainCode = TestCode.parse(mainContents); - final partCode = TestCode.parse(partContents); - - newFile(mainFilePath, mainCode.code); - newFile(partFilePath, partCode.code); - await initialize(); - final res = - await getDefinitionAsLocation(partFileUri, partCode.position.position); - - expect(res.single.uri, equals(mainFileUri)); - } - Future test_sameLine() async { final contents = ''' int plusOne(int [!value!]) => 1 + val^ue; @@ -714,4 +706,29 @@ foo() { expect(loc.range, equals(code.range.range)); expect(loc.uri, equals(mainFileUri)); } + + /// Verifies that invoking Definition at `^` in [source] (which will be + /// written into `source.dart`) navigate to `destination.dart` (with the + /// content [destination]). + Future verifyDirective({ + required String source, + String destination = '', + }) async { + final destinationCode = TestCode.parse(destination); + final sourceCode = TestCode.parse(source); + + final sourceFilePath = join(projectFolderPath, 'lib', 'source.dart'); + final sourceFileUri = toUri(sourceFilePath); + final destinationFilePath = + join(projectFolderPath, 'lib', 'destination.dart'); + final destinationFileUri = toUri(destinationFilePath); + + newFile(sourceFilePath, sourceCode.code); + newFile(destinationFilePath, destinationCode.code); + await initialize(); + final res = await getDefinitionAsLocation( + sourceFileUri, sourceCode.position.position); + + expect(res.single.uri, equals(destinationFileUri)); + } } diff --git a/pkg/analyzer_plugin/lib/utilities/analyzer_converter.dart b/pkg/analyzer_plugin/lib/utilities/analyzer_converter.dart index 86e209ef2c2a..f564f702f5a6 100644 --- a/pkg/analyzer_plugin/lib/utilities/analyzer_converter.dart +++ b/pkg/analyzer_plugin/lib/utilities/analyzer_converter.dart @@ -318,21 +318,13 @@ extension ElementExtensions on analyzer.Element? { /// Return the compilation unit containing the given [element]. analyzer.CompilationUnitElement? get _unitElement { var currentElement = this; - if (currentElement is analyzer.CompilationUnitElement) { - return currentElement; - } - if (currentElement?.enclosingElement - is analyzer.LibraryOrAugmentationElement) { - currentElement = currentElement?.enclosingElement; - } - if (currentElement is analyzer.LibraryOrAugmentationElement) { - return currentElement.definingCompilationUnit; - } for (; currentElement != null; currentElement = currentElement.enclosingElement) { if (currentElement is analyzer.CompilationUnitElement) { return currentElement; + } else if (currentElement is analyzer.LibraryOrAugmentationElement) { + return currentElement.definingCompilationUnit; } } return null; diff --git a/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart b/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart index 3f7f5585d872..f78d8153e47a 100644 --- a/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart +++ b/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart @@ -219,6 +219,12 @@ class _DartNavigationComputerVisitor extends RecursiveAstVisitor { node.rightHandSide.accept(this); } + @override + void visitAugmentationImportDirective(AugmentationImportDirective node) { + super.visitAugmentationImportDirective(node); + _addUriDirectiveRegion(node, node.element?.importedAugmentation); + } + @override void visitBinaryExpression(BinaryExpression node) { node.leftOperand.accept(this); @@ -431,6 +437,18 @@ class _DartNavigationComputerVisitor extends RecursiveAstVisitor { computer._addRegionForToken(node.rightBracket, element); } + @override + void visitLibraryAugmentationDirective(LibraryAugmentationDirective node) { + super.visitLibraryAugmentationDirective(node); + var element = node.element; + var library = element?.library; + // If the library URI is unresolved, library will be the augmentation + // itself, so don't create a navigation region in that case. + if (element != library) { + _addUriDirectiveRegion(node, library); + } + } + @override void visitLibraryDirective(LibraryDirective node) { computer._addRegionForNode(node.name2, node.element); @@ -619,7 +637,10 @@ class _DartNavigationComputerVisitor extends RecursiveAstVisitor { /// If the source of the given [element] (referenced by the [node]) exists, /// then add the navigation region from the [node] to the [element]. - void _addUriDirectiveRegion(UriBasedDirective node, LibraryElement? element) { + void _addUriDirectiveRegion( + UriBasedDirective node, + LibraryOrAugmentationElement? element, + ) { var source = element?.source; if (source != null) { if (resourceProvider.getResource(source.fullName).exists) {