[Atom] Issue 424. Fix for 'getNavigation' and index expressions.

pkg/analysis_server/lib/src/domains/analysis/navigation_dart.dart

Index: pkg/analysis_server/lib/src/domains/analysis/navigation_dart.dart
diff --git a/pkg/analysis_server/lib/src/domains/analysis/navigation_dart.dart b/pkg/analysis_server/lib/src/domains/analysis/navigation_dart.dart
index a4971ce51f03efcb9a1f2202dc668d0bf57c38f0..6850f7e5163e29914ca47de9de39b49104a238e0 100644
--- a/pkg/analysis_server/lib/src/domains/analysis/navigation_dart.dart
+++ b/pkg/analysis_server/lib/src/domains/analysis/navigation_dart.dart
@@ -198,7 +198,9 @@ class _DartNavigationComputerVisitor extends RecursiveAstVisitor {
   @override
   visitIndexExpression(IndexExpression node) {
     super.visitIndexExpression(node);
-    computer._addRegionForToken(node.rightBracket, node.bestElement);
+    MethodElement element = node.bestElement;
+    computer._addRegionForToken(node.leftBracket, element);
+    computer._addRegionForToken(node.rightBracket, element);
   }
 
   @override
@@ -310,10 +312,6 @@ class _DartRangeAstVisitor extends UnifyingAstVisitor {
 
   _DartRangeAstVisitor(this.start, this.end, this.visitor);
 
-  bool isInRange(int offset) {
-    return start <= offset && offset <= end;
-  }
-
   @override
   visitNode(AstNode node) {
     // The node ends before the range starts.
@@ -326,7 +324,14 @@ class _DartRangeAstVisitor extends UnifyingAstVisitor {
     }
     // The node starts or ends in the range.
     if (node is! CompilationUnit) {
-      if (isInRange(node.offset) || isInRange(node.end) || node is Directive) {
+      if (node is Directive) {
+        node.accept(visitor);
+        return;
+      }
+      if (_isNodeInRange(node)) {
+        while (_isNodeInRange(node)) {

[Brian Wilkerson, 2015/10/25 15:28:04]

I don't understand this while loop, so I'm probably misunderstanding something.

This `visitNode` method has already been invoked on all of the parents of the
`node`, and if any of them had been in range we would not have invoked this
method on any of their children. Therefore, `node` is the highest level node for
which `_isNodeInRange` will return `true`. Therefore, the body of this while
loop will always be executed exactly once, and the `visitor` will always visit a
node that is not in range. At the very least it seems like lines 332 - 335 could
be replaced by:

    node.parent.accept(visitor);

(And if the loop really is necessary, don't we need to guard against running off
the top of the AST structure and having `node` be set to `null`?)

[scheglov, 2015/10/25 18:29:44]

Thank you.
Reworked.

+          node = node.parent;
+        }
         node.accept(visitor);
         return;
       }
@@ -334,4 +339,12 @@ class _DartRangeAstVisitor extends UnifyingAstVisitor {
     // Go deeper.
     super.visitNode(node);
   }
+
+  bool _isNodeInRange(AstNode node) {
+    return _isOffsetInRange(node.offset) || _isOffsetInRange(node.end);
+  }
+
+  bool _isOffsetInRange(int offset) {
+    return start <= offset && offset <= end;
+  }
 }