[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 dart-lang/sdk#28799.

Change-Id: I2fce33a3bd2678abfb40e5e9b3e3c5662e4f5608
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161948
Commit-Queue: Danny Tuppeny <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: DanTup

pkg/analysis_server/test/analysis/get_navigation_test.dart

@@ -43,6 +43,29 @@ main() {
     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 @@ main() {
     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';

pkg/analysis_server/test/integration/analysis/get_navigation_test.dart

@@ -41,9 +41,7 @@ class Bar {
     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'''
 //

pkg/analysis_server/test/lsp/definition_test.dart

@@ -55,6 +55,26 @@ class DefinitionTest extends AbstractLspAnalysisServerTest {
     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 @@ class DefinitionTest extends AbstractLspAnalysisServerTest {
     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;

pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart

@@ -19,7 +19,7 @@ NavigationCollector computeDartNavigation(
     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 @@ AstNode _getNodeForRange(CompilationUnit unit, int offset, int length) {
 /// 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 @@ class _DartNavigationCollector {
     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 @@ class _DartNavigationCollector {
     }
 
     // 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;