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

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

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // 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.
 
 library domains.analysis.navigation_dart;
 
 import 'package:analysis_server/plugin/analysis/navigation/navigation_core.dart';
 import 'package:analysis_server/src/protocol_server.dart' as protocol;
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/element.dart';
@@ skipping 180 matching lines @@
     if (importElement != null) {
       Element libraryElement = importElement.importedLibrary;
       _addUriDirectiveRegion(node, libraryElement);
     }
     super.visitImportDirective(node);
   }
 
   @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
   visitLibraryDirective(LibraryDirective node) {
     computer._addRegionForNode(node.name, node.element);
   }
 
   @override
   visitPartDirective(PartDirective node) {
     _addUriDirectiveRegion(node, node.element);
@@ skipping 91 matching lines @@
  * An AST visitor that forwards nodes intersecting with the range from
  * [start] to [end] to the given [visitor].
  */
 class _DartRangeAstVisitor extends UnifyingAstVisitor {
   final int start;
   final int end;
   final AstVisitor visitor;
 
   _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.
     if (node.end < start) {
       return;
     }
     // The node starts after the range ends.
     if (node.offset > end) {
       return;
     }
     // 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;
+      }
     }
     // 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;
+  }
 }