[Analyzer] Fix error in LSP go-to-definition when target is in a "part"
Plus unrelated navigation regions (for example in comments, references
outside of the required range would be returned).
Fixes https://github.com/dart-lang/sdk/issues/28799.
Change-Id: I2fce33a3bd2678abfb40e5e9b3e3c5662e4f5608
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161948
Commit-Queue: Danny Tuppeny <danny@tuppeny.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/test/analysis/get_navigation_test.dart b/pkg/analysis_server/test/analysis/get_navigation_test.dart
index f2f2cf1..4dbe557 100644
--- a/pkg/analysis_server/test/analysis/get_navigation_test.dart
+++ b/pkg/analysis_server/test/analysis/get_navigation_test.dart
@@ -43,6 +43,29 @@
assertHasTarget('test = 0');
}
+ Future<void> test_comment_outsideReference() async {
+ addTestFile('''
+/// Returns a [String].
+String main() {
+}''');
+ await waitForTasksFinished();
+ var search = 'Returns';
+ await _getNavigation(testFile, testCode.indexOf(search), 1);
+ expect(regions, hasLength(0));
+ }
+
+ Future<void> test_comment_reference() async {
+ addTestFile('''
+/// Returns a [String].
+String main() {
+}''');
+ await waitForTasksFinished();
+ var search = '[String';
+ await _getNavigation(testFile, testCode.indexOf(search), 1);
+ expect(regions, hasLength(1));
+ assertHasRegion('String]');
+ }
+
Future<void> test_fieldType() async {
// This test mirrors test_navigation() from
// test/integration/analysis/get_navigation_test.dart
@@ -101,20 +124,6 @@
expect(testTargets[0].kind, ElementKind.LIBRARY);
}
- Future<void> test_importKeyword() async {
- addTestFile('''
-import 'dart:math';
-
-main() {
-}''');
- await waitForTasksFinished();
- await _getNavigation(testFile, 0, 1);
- expect(regions, hasLength(1));
- assertHasRegionString("'dart:math'");
- expect(testTargets, hasLength(1));
- expect(testTargets[0].kind, ElementKind.LIBRARY);
- }
-
Future<void> test_importUri() async {
addTestFile('''
import 'dart:math';
diff --git a/pkg/analysis_server/test/integration/analysis/get_navigation_test.dart b/pkg/analysis_server/test/integration/analysis/get_navigation_test.dart
index bd69280..1288adb 100644
--- a/pkg/analysis_server/test/integration/analysis/get_navigation_test.dart
+++ b/pkg/analysis_server/test/integration/analysis/get_navigation_test.dart
@@ -41,9 +41,7 @@
expect(target.startColumn, 7);
}
- @failingTest
Future<void> test_navigation_no_result() async {
- // This fails - it returns navigation results for a whitespace area (#28799).
var pathname = sourcePath('test.dart');
var text = r'''
//
diff --git a/pkg/analysis_server/test/lsp/definition_test.dart b/pkg/analysis_server/test/lsp/definition_test.dart
index a243579..d64412c 100644
--- a/pkg/analysis_server/test/lsp/definition_test.dart
+++ b/pkg/analysis_server/test/lsp/definition_test.dart
@@ -55,6 +55,26 @@
expect(loc.uri, equals(referencedFileUri.toString()));
}
+ Future<void> test_comment_adjacentReference() async {
+ /// Computing Dart navigation locates a node at the provided offset then
+ /// returns all navigation regions inside it. This test ensures we filter
+ /// out any regions that are in the same target node (the comment) but do
+ /// not span the requested offset.
+ final contents = '''
+ /// Te^st
+ ///
+ /// References [String].
+ main() {}
+ ''';
+
+ await initialize();
+ await openFile(mainFileUri, withoutMarkers(contents));
+ final res = await getDefinitionAsLocation(
+ mainFileUri, positionFromMarker(contents));
+
+ expect(res, hasLength(0));
+ }
+
Future<void> test_fromPlugins() async {
final pluginAnalyzedFilePath = join(projectFolderPath, 'lib', 'foo.foo');
final pluginAnalyzedFileUri = Uri.file(pluginAnalyzedFilePath);
@@ -176,6 +196,59 @@
expect(res, isEmpty);
}
+ /// Failing due to incorrect range because _DartNavigationCollector._getCodeLocation
+ /// does not handle parts.
+ @failingTest
+ Future<void> test_part() async {
+ final mainContents = '''
+ import 'lib.dart';
+
+ main() {
+ Icons.[[ad^d]]();
+ }
+ ''';
+
+ final libContents = '''
+ part 'part.dart';
+ ''';
+
+ final partContents = '''
+ part of 'lib.dart';
+
+ void unrelatedFunction() {}
+
+ class Icons {
+ /// `targetRange` should not include the dartDoc but should include the full
+ /// function body. `targetSelectionRange` will be just the name.
+ [[String add = "Test"]];
+ }
+
+ void otherUnrelatedFunction() {}
+ ''';
+
+ final libFileUri = Uri.file(join(projectFolderPath, 'lib', 'lib.dart'));
+ final partFileUri = Uri.file(join(projectFolderPath, 'lib', 'part.dart'));
+
+ await initialize(
+ textDocumentCapabilities:
+ withLocationLinkSupport(emptyTextDocumentClientCapabilities));
+ await openFile(mainFileUri, withoutMarkers(mainContents));
+ await openFile(libFileUri, withoutMarkers(libContents));
+ await openFile(partFileUri, withoutMarkers(partContents));
+ final res = await getDefinitionAsLocationLinks(
+ mainFileUri, positionFromMarker(mainContents));
+
+ expect(res, hasLength(1));
+ var loc = res.single;
+ expect(loc.originSelectionRange, equals(rangeFromMarkers(mainContents)));
+ expect(loc.targetUri, equals(partFileUri.toString()));
+ expect(loc.targetRange, equals(rangeFromMarkers(partContents)));
+ expect(
+ loc.targetSelectionRange,
+ equals(rangeOfString(partContents, 'add')),
+ );
+ }
+
Future<void> test_sameLine() async {
final contents = '''
int plusOne(int [[value]]) => 1 + val^ue;
diff --git a/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart b/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart
index efa84ae..cd05675 100644
--- a/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart
+++ b/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart
@@ -19,7 +19,7 @@
CompilationUnit unit,
int offset,
int length) {
- var dartCollector = _DartNavigationCollector(collector);
+ var dartCollector = _DartNavigationCollector(collector, offset, length);
var visitor = _DartNavigationComputerVisitor(resourceProvider, dartCollector);
if (offset == null || length == null) {
unit.accept(visitor);
@@ -43,8 +43,11 @@
/// A Dart specific wrapper around [NavigationCollector].
class _DartNavigationCollector {
final NavigationCollector collector;
+ final int requestedOffset;
+ final int requestedLength;
- _DartNavigationCollector(this.collector);
+ _DartNavigationCollector(
+ this.collector, this.requestedOffset, this.requestedLength);
void _addRegion(int offset, int length, Element element) {
if (element is FieldFormalParameterElement) {
@@ -56,6 +59,12 @@
if (element.location == null) {
return;
}
+ // Discard elements that don't span the offset/range given (if provided).
+ if (requestedOffset != null &&
+ (offset > requestedOffset + (requestedLength ?? 0) ||
+ offset + length < requestedOffset)) {
+ return;
+ }
var converter = AnalyzerConverter();
var kind = converter.convertElementKind(element.kind);
var location = converter.locationFromElement(element);
@@ -118,9 +127,13 @@
}
// Read the declaration so we can get the offset after the doc comments.
- final declaration = codeElement.session
- .getParsedLibrary(location.file)
- .getElementDeclaration(codeElement);
+ // TODO(dantup): Skip this for parts (getParsedLibrary will throw), but find
+ // a better solution.
+ final declaration = !codeElement.session.getFile(location.file).isPart
+ ? codeElement.session
+ .getParsedLibrary(location.file)
+ .getElementDeclaration(codeElement)
+ : null;
var node = declaration?.node;
if (node is VariableDeclaration) {
node = node.parent;
