Replace code completion usage of token.previous with findPrevious

This CL is the first in a multi-part effort to remove the `previous`
field from the Token class. This first step introduces a new findPrevious
utility method that finds the the token before a given token in a token stream,
and then replaces usages of token.previous with calls to this
new findPrevious utility.

Change-Id: I57de4e542adb5950f1bddd951ff555dd98b19b3b
Reviewed-on: https://dart-review.googlesource.com/48461
Commit-Queue: Dan Rubel <danrubel@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

pkg/analyzer/lib/dart/ast/ast.dart

@@ -532,6 +532,11 @@ abstract class AstNode implements SyntacticEntity {
    */
   E getProperty<E>(String name);
 
+  /**
+   * Return the token before [target] or `null` if it cannot be found.
+   */
+  Token findPrevious(Token target);
+
   /**
    * Set the value of the property with the given [name] to the given [value].
    * If the value is `null`, the property will effectively be removed.

pkg/analyzer/lib/src/dart/ast/ast.dart

@@ -24,6 +24,7 @@ import 'package:analyzer/src/generated/parser.dart';
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/source.dart' show LineInfo, Source;
 import 'package:analyzer/src/generated/utilities_dart.dart';
+import 'package:analyzer/src/fasta/token_utils.dart' as util show findPrevious;
 
 /**
  * Two or more string literals that are implicitly concatenated because of being
@@ -988,6 +989,9 @@ abstract class AstNodeImpl implements AstNode {
     return _propertyMap[name] as E;
   }
 
+  Token findPrevious(Token target) =>
+      util.findPrevious(beginToken, target) ?? parent?.findPrevious(target);
+
   @override
   void setProperty(String name, Object value) {
     if (value == null) {

pkg/analyzer/lib/src/fasta/token_utils.dart

@@ -4,7 +4,7 @@
 
 library fasta.analyzer.token_utils;
 
-import 'package:front_end/src/scanner/token.dart' show Token;
+import 'package:front_end/src/scanner/token.dart' show CommentToken, Token;
 
 import 'package:front_end/src/fasta/scanner/token_constants.dart';
 
@@ -47,3 +47,21 @@ class ToAnalyzerTokenStreamConverter {
   void reportError(analyzer.ScannerErrorCode errorCode, int offset,
       List<Object> arguments) {}
 }
+
+/// Search for the token before [target] starting the search with [start].
+/// Return `null` if [target] is a comment token
+/// or the previous token cannot be found.
+Token findPrevious(Token start, Token target) {
+  if (start == target || target is CommentToken) {
+    return null;
+  }
+  Token token = start is CommentToken ? start.parent : start;
+  do {
+    Token next = token.next;
+    if (next == target) {
+      return token;
+    }
+    token = next;
+  } while (!token.isEof);
+  return null;
+}

pkg/analyzer/test/dart/ast/ast_test.dart

@@ -8,8 +8,11 @@ import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/standard_ast_factory.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/src/dart/ast/token.dart';
+import 'package:analyzer/src/dart/scanner/scanner.dart';
+import 'package:analyzer/src/generated/parser.dart';
 import 'package:analyzer/src/generated/testing/ast_test_factory.dart';
 import 'package:analyzer/src/generated/testing/token_factory.dart';
+import 'package:analyzer/src/string_source.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
@@ -25,6 +28,7 @@ main() {
     defineReflectiveTests(IndexExpressionTest);
     defineReflectiveTests(MethodDeclarationTest);
     defineReflectiveTests(NodeListTest);
+    defineReflectiveTests(PreviousTokenTest);
     defineReflectiveTests(SimpleIdentifierTest);
     defineReflectiveTests(SimpleStringLiteralTest);
     defineReflectiveTests(StringInterpolationTest);
@@ -603,6 +607,105 @@ class NodeListTest extends EngineTestCase {
   }
 }
 
+@reflectiveTest
+class PreviousTokenTest {
+  static final String contents = '''
+class A {
+  B foo(C c) {
+    return bar;
+  }
+  D get baz => null;
+}
+E f() => g;
+''';
+
+  CompilationUnit _unit;
+
+  CompilationUnit get unit {
+    if (_unit == null) {
+      GatheringErrorListener listener =
+          new GatheringErrorListener(checkRanges: true);
+      var source =
+          new StringSource(contents, 'PreviousTokenTest_findPrevious.dart');
+      Token tokens = new Scanner.fasta(source, listener).tokenize();
+      _unit = new Parser(source, listener, useFasta: true)
+          .parseCompilationUnit(tokens);
+    }
+    return _unit;
+  }
+
+  Token findToken(String lexeme) {
+    Token token = unit.beginToken;
+    while (!token.isEof) {
+      if (token.lexeme == lexeme) {
+        return token;
+      }
+      token = token.next;
+    }
+    fail('Failed to find $lexeme');
+  }
+
+  void test_findPrevious_basic_class() {
+    ClassDeclaration clazz = unit.declarations[0];
+    expect(clazz.findPrevious(findToken('A')).lexeme, 'class');
+  }
+
+  void test_findPrevious_basic_method() {
+    ClassDeclaration clazz = unit.declarations[0];
+    MethodDeclaration method = clazz.members[0];
+    expect(method.findPrevious(findToken('foo')).lexeme, 'B');
+  }
+
+  void test_findPrevious_basic_statement() {
+    ClassDeclaration clazz = unit.declarations[0];
+    MethodDeclaration method = clazz.members[0];
+    BlockFunctionBody body = method.body;
+    Statement statement = body.block.statements[0];
+    expect(statement.findPrevious(findToken('bar')).lexeme, 'return');
+    expect(statement.findPrevious(findToken(';')).lexeme, 'bar');
+  }
+
+  void test_findPrevious_missing() {
+    ClassDeclaration clazz = unit.declarations[0];
+    MethodDeclaration method = clazz.members[0];
+    BlockFunctionBody body = method.body;
+    Statement statement = body.block.statements[0];
+
+    GatheringErrorListener listener =
+        new GatheringErrorListener(checkRanges: true);
+    var source = new StringSource('missing', 'PreviousTokenTest_missing.dart');
+    Token missing = new Scanner.fasta(source, listener).tokenize();
+
+    expect(statement.findPrevious(missing), null);
+    expect(statement.findPrevious(null), null);
+  }
+
+  void test_findPrevious_parent_method() {
+    ClassDeclaration clazz = unit.declarations[0];
+    MethodDeclaration method = clazz.members[0];
+    expect(method.findPrevious(findToken('B')).lexeme, '{');
+  }
+
+  void test_findPrevious_parent_statement() {
+    ClassDeclaration clazz = unit.declarations[0];
+    MethodDeclaration method = clazz.members[0];
+    BlockFunctionBody body = method.body;
+    Statement statement = body.block.statements[0];
+    expect(statement.findPrevious(findToken('return')).lexeme, '{');
+  }
+
+  void test_findPrevious_sibling_method() {
+    ClassDeclaration clazz = unit.declarations[0];
+    MethodDeclaration method = clazz.members[1];
+    expect(method.findPrevious(findToken('D')).lexeme, '}');
+  }
+
+  void test_findPrevious_sibling_class() {
+    CompilationUnitMember declaration = unit.declarations[1];
+    expect(declaration.findPrevious(findToken('E')).lexeme, '}');
+  }
+}
+
 @reflectiveTest
 class SimpleIdentifierTest extends ParserTestCase {
   void test_inGetterContext() {

pkg/analyzer/test/src/fasta/token_utils_test.dart

@@ -0,0 +1,37 @@
+// Copyright (c) 2018, 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.
+
+import 'package:analyzer/src/fasta/token_utils.dart';
+import 'package:front_end/src/fasta/scanner.dart';
+import 'package:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(PreviousTokenTest);
+  });
+}
+
+@reflectiveTest
+class PreviousTokenTest {
+  void test_findPrevious() {
+    Token a =
+        scanString('a b c /* comment */ d e', includeComments: true).tokens;
+    Token b = a.next;
+    Token c = b.next;
+    Token d = c.next;
+    Token e = d.next;
+
+    expect(findPrevious(a, b), a);
+    expect(findPrevious(a, c), b);
+    expect(findPrevious(a, d), c);
+    expect(findPrevious(d.precedingComments, e), d);
+
+    Token x = scanString('x').tokens;
+    expect(findPrevious(a, x), null);
+    expect(findPrevious(b, b), null);
+    expect(findPrevious(d, b), null);
+    expect(findPrevious(a, null), null);
+  }
+}

pkg/analyzer_plugin/lib/src/utilities/completion/completion_target.dart

@@ -165,7 +165,7 @@ class CompletionTarget {
       }
       for (var entity in containingNode.childEntities) {
         if (entity is Token) {
-          if (_isCandidateToken(entity, offset)) {
+          if (_isCandidateToken(containingNode, entity, offset)) {
             // Try to replace with a comment token.
             Token commentToken = _getContainingCommentToken(entity, offset);
             if (commentToken != null) {
@@ -184,7 +184,7 @@ class CompletionTarget {
           // If the last token in the node isn't a candidate target, then
           // neither the node nor any of its descendants can possibly be the
           // completion target, so we can skip the node entirely.
-          if (!_isCandidateToken(entity.endToken, offset)) {
+          if (!_isCandidateToken(containingNode, entity.endToken, offset)) {
             continue;
           }
 
@@ -278,14 +278,14 @@ class CompletionTarget {
     Token token = droppedToken ??
         (entity is AstNode ? (entity as AstNode).beginToken : entity);
     if (token != null && requestOffset < token.offset) {
-      token = token.previous;
+      token = containingNode.findPrevious(token);
     }
     if (token != null) {
       if (requestOffset == token.offset && !isKeywordOrIdentifier(token)) {
         // If the insertion point is at the beginning of the current token
         // and the current token is not an identifier
         // then check the previous token to see if it should be replaced
-        token = token.previous;
+        token = containingNode.findPrevious(token);
       }
       if (token != null && isKeywordOrIdentifier(token)) {
         if (token.offset <= requestOffset && requestOffset <= token.end) {
@@ -296,7 +296,7 @@ class CompletionTarget {
       if (token is StringToken) {
         SimpleStringLiteral uri =
             astFactory.simpleStringLiteral(token, token.lexeme);
-        Keyword keyword = token.previous?.keyword;
+        Keyword keyword = containingNode.findPrevious(token)?.keyword;
         if (keyword == Keyword.IMPORT ||
             keyword == Keyword.EXPORT ||
             keyword == Keyword.PART) {
@@ -318,7 +318,7 @@ class CompletionTarget {
   bool isDoubleOrIntLiteral() {
     var entity = this.entity;
     if (entity is Token) {
-      TokenType previousTokenType = entity.previous?.type;
+      TokenType previousTokenType = containingNode.findPrevious(entity)?.type;
       return previousTokenType == TokenType.DOUBLE ||
           previousTokenType == TokenType.INT;
     }
@@ -405,7 +405,8 @@ class CompletionTarget {
       }
       if (entity == argList.rightParenthesis) {
         // Parser ignores trailing commas
-        if (argList.rightParenthesis.previous?.lexeme == ',') {
+        Token previous = containingNode.findPrevious(argList.rightParenthesis);
+        if (previous?.lexeme == ',') {
           return args.length;
         }
         return args.length - 1;
@@ -508,7 +509,7 @@ class CompletionTarget {
     // candidate entity if its first token is.
     Token beginToken = node.beginToken;
     if (beginToken.type.isKeyword || beginToken.type == TokenType.IDENTIFIER) {
-      return _isCandidateToken(beginToken, offset);
+      return _isCandidateToken(node, beginToken, offset);
     }
 
     // Otherwise, the node is a candidate entity only if the offset is before
@@ -522,7 +523,7 @@ class CompletionTarget {
    * Determine whether [token] could possibly be the [entity] for a
    * [CompletionTarget] associated with the given [offset].
    */
-  static bool _isCandidateToken(Token token, int offset) {
+  static bool _isCandidateToken(AstNode node, Token token, int offset) {
     if (token == null) {
       return false;
     }
@@ -541,7 +542,7 @@ class CompletionTarget {
     }
     // If the current token is synthetic, then check the previous token
     // because it may have been dropped from the parse tree
-    Token previous = token.previous;
+    Token previous = node.findPrevious(token);
     if (offset < previous.end) {
       return true;
     } else if (offset == previous.end) {

pkg/analyzer_plugin/lib/src/utilities/completion/optype.dart

@@ -335,7 +335,8 @@ class _OpTypeAstVisitor extends GeneralizingAstVisitor {
         index = 0;
       } else if (entity == node.rightParenthesis) {
         // Parser ignores trailing commas
-        if (node.rightParenthesis.previous?.lexeme == ',') {
+        Token previous = node.findPrevious(node.rightParenthesis);
+        if (previous?.lexeme == ',') {
           index = node.arguments.length;
         } else {
           index = node.arguments.length - 1;
@@ -598,10 +599,13 @@ class _OpTypeAstVisitor extends GeneralizingAstVisitor {
   @override
   void visitFormalParameterList(FormalParameterList node) {
     dynamic entity = this.entity;
-    if (entity is Token && entity.previous != null) {
-      TokenType type = entity.previous.type;
-      if (type == TokenType.OPEN_PAREN || type == TokenType.COMMA) {
-        optype.includeTypeNameSuggestions = true;
+    if (entity is Token) {
+      Token previous = node.findPrevious(entity);
+      if (previous != null) {
+        TokenType type = previous.type;
+        if (type == TokenType.OPEN_PAREN || type == TokenType.COMMA) {
+          optype.includeTypeNameSuggestions = true;
+        }
       }
     }
     // Handle default normal parameter just as a normal parameter.
@@ -844,7 +848,7 @@ class _OpTypeAstVisitor extends GeneralizingAstVisitor {
         // identifier to be a keyword and inserts a synthetic identifier
         (node.identifier != null &&
             node.identifier.isSynthetic &&
-            identical(entity, node.identifier.beginToken.previous))) {
+            identical(entity, node.findPrevious(node.identifier.beginToken)))) {
       optype.isPrefixed = true;
       if (node.parent is TypeName && node.parent.parent is ConstructorName) {
         optype.includeConstructorSuggestions = true;
@@ -1031,9 +1035,11 @@ class _OpTypeAstVisitor extends GeneralizingAstVisitor {
 
   bool _isEntityPrevTokenSynthetic() {
     Object entity = this.entity;
-    if (entity is AstNode &&
-        (entity.beginToken.previous?.isSynthetic ?? false)) {
-      return true;
+    if (entity is AstNode) {
+      Token previous = entity.findPrevious(entity.beginToken);
+      if (previous?.isSynthetic ?? false) {
+        return true;
+      }
     }
     return false;
   }