From 1231e90fee78537492eee27a5020d0b889472d1a Mon Sep 17 00:00:00 2001 From: Dan Rubel Date: Thu, 21 Dec 2017 21:11:17 -0500 Subject: [PATCH] Code complete identifiers not directly referenced by the analyzer AST The fasta parser sometimes "drops" unexpected tokens during recovery. When this happens, the dropped or skipped token is not reported to the listeners, and thus does not become part of the analyzer's AST. These tokens are still in the token stream, but will not be found by the existing CompletionTarget mechanism. For example, the fasta parser parses 'if (v i) {}' as 'if (v) {}' and drops the 'i' token. This CL introduces a new CompletionTarget.droppedToken field. If a keyword or identifier has been dropped by the parser, but overlaps the code completion offset, then that token is placed into this new field. In the example above, when code completing the 'i', the CompletionTarget will now be containingNode = '(v)' entity = ')' droppedToken = 'i' Change-Id: I15e43529ab4a72de9500c521be278fa13ae68d99 Reviewed-on: https://dart-review.googlesource.com/31285 Reviewed-by: Brian Wilkerson Commit-Queue: Dan Rubel --- .../completion/dart/keyword_contributor.dart | 19 ++++++ .../dart/keyword_contributor_test.dart | 10 +-- .../completion/completion_target.dart | 64 ++++++++++++++++++- .../completion/completion_target_test.dart | 23 ++++++- 4 files changed, 103 insertions(+), 13 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/completion/dart/keyword_contributor.dart b/pkg/analysis_server/lib/src/services/completion/dart/keyword_contributor.dart index 6819a623433..2e80f580d15 100644 --- a/pkg/analysis_server/lib/src/services/completion/dart/keyword_contributor.dart +++ b/pkg/analysis_server/lib/src/services/completion/dart/keyword_contributor.dart @@ -48,6 +48,8 @@ class _KeywordVisitor extends GeneralizingAstVisitor { : this.request = request, this.entity = request.target.entity; + Token get droppedToken => request.target.droppedToken; + bool isEmptyBody(FunctionBody body) => body is EmptyFunctionBody || (body is BlockFunctionBody && body.beginToken.isSynthetic); @@ -338,6 +340,23 @@ class _KeywordVisitor extends GeneralizingAstVisitor { } } + @override + visitParenthesizedExpression(ParenthesizedExpression node) { + Expression expression = node.expression; + if (expression is Identifier || expression is PropertyAccess) { + if (entity == node.rightParenthesis) { + var next = expression.endToken.next; + if (next == entity || next == droppedToken) { + // Fasta parses `if (x i^)` as `if (x ^) where the `i` is in the token + // stream but not part of the ParenthesizedExpression. + _addSuggestion(Keyword.IS, DART_RELEVANCE_HIGH); + return; + } + } + } + _addExpressionKeywords(node); + } + @override visitIfStatement(IfStatement node) { if (_isPreviousTokenSynthetic(entity, TokenType.CLOSE_PAREN)) { diff --git a/pkg/analysis_server/test/services/completion/dart/keyword_contributor_test.dart b/pkg/analysis_server/test/services/completion/dart/keyword_contributor_test.dart index 33ed658952e..4d859be44fa 100644 --- a/pkg/analysis_server/test/services/completion/dart/keyword_contributor_test.dart +++ b/pkg/analysis_server/test/services/completion/dart/keyword_contributor_test.dart @@ -1452,14 +1452,8 @@ class A { test_method_async() async { addTestSource('class A { foo() ^}'); await computeSuggestions(); - if (usingFastaParser) { - assertSuggestKeywords([], - pseudoKeywords: ['async', 'async*', 'sync*'], - relevance: DART_RELEVANCE_HIGH); - } else { - assertSuggestKeywords(CLASS_BODY_KEYWORDS, - pseudoKeywords: ['async', 'async*', 'sync*']); - } + assertSuggestKeywords(CLASS_BODY_KEYWORDS, + pseudoKeywords: ['async', 'async*', 'sync*']); } test_method_async2() async { diff --git a/pkg/analyzer_plugin/lib/src/utilities/completion/completion_target.dart b/pkg/analyzer_plugin/lib/src/utilities/completion/completion_target.dart index 431384112e3..b0dac1f4e52 100644 --- a/pkg/analyzer_plugin/lib/src/utilities/completion/completion_target.dart +++ b/pkg/analyzer_plugin/lib/src/utilities/completion/completion_target.dart @@ -87,6 +87,17 @@ class CompletionTarget { */ final AstNode containingNode; + /** + * The "dropped" identifier or keyword which the completed text will replace, + * or `null` if none. + * + * For the purposes of code completion, a "dropped" token is an identifier + * or keyword that is part of the token stream, but that the parser has + * skipped and not reported in to the parser listeners, meaning that it is + * not part of the AST. + */ + Token droppedToken; + /** * The entity which the completed text will replace (or which will be * displaced once the completed text is inserted). This may be an AstNode or @@ -235,7 +246,9 @@ class CompletionTarget { Object entity, this.isCommentText) : this.containingNode = containingNode, this.entity = entity, - this.argIndex = _computeArgIndex(containingNode, entity); + this.argIndex = _computeArgIndex(containingNode, entity), + this.droppedToken = + _computeDroppedToken(containingNode, entity, offset); /** * Return `true` if the [containingNode] is a cascade @@ -263,7 +276,8 @@ class CompletionTarget { bool isKeywordOrIdentifier(Token token) => token.type.isKeyword || token.type == TokenType.IDENTIFIER; - Token token = entity is AstNode ? (entity as AstNode).beginToken : entity; + Token token = droppedToken ?? + (entity is AstNode ? (entity as AstNode).beginToken : entity); if (token != null && requestOffset < token.offset) { token = token.previous; } @@ -388,6 +402,52 @@ class CompletionTarget { return null; } + static Token _computeDroppedToken( + AstNode containingNode, Object entity, int offset) { + // Find the last token of the member before the entity. + var previousMember; + for (var member in containingNode.childEntities) { + if (entity == member) { + break; + } + if (member is! Comment && member is! CommentToken) { + previousMember = member; + } + } + Token token; + if (previousMember is AstNode) { + token = previousMember.endToken; + } else if (previousMember is Token) { + token = previousMember; + } + if (token == null) { + return null; + } + + // Find the first token of the entity (which may be the entity itself). + Token endSearch; + if (entity is AstNode) { + endSearch = entity.beginToken; + } else if (entity is Token) { + endSearch = entity; + } + if (endSearch == null) { + return null; + } + + // Find a dropped token that overlaps the offset. + token = token.next; + while (token != endSearch && !token.isEof) { + if (token.isKeywordOrIdentifier && + token.offset <= offset && + offset <= token.end) { + return token; + } + token = token.next; + } + return null; + } + /** * Determine if the offset is contained in a preceding comment token * and return that token, otherwise return `null`. diff --git a/pkg/analyzer_plugin/test/src/utilities/completion/completion_target_test.dart b/pkg/analyzer_plugin/test/src/utilities/completion/completion_target_test.dart index a55537357ed..40c3fcd5937 100644 --- a/pkg/analyzer_plugin/test/src/utilities/completion/completion_target_test.dart +++ b/pkg/analyzer_plugin/test/src/utilities/completion/completion_target_test.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/src/dart/analysis/driver.dart'; +import 'package:analyzer/src/generated/parser.dart' as analyzer; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer_plugin/src/utilities/completion/completion_target.dart'; import 'package:test/test.dart'; @@ -25,6 +26,8 @@ class CompletionTargetTest extends AbstractContextTest { int completionOffset; CompletionTarget target; + bool get usingFastaParser => analyzer.Parser.useFasta; + Future addTestSource(String content) async { expect(completionOffset, isNull, reason: 'Call addTestSource exactly once'); completionOffset = content.indexOf('^'); @@ -39,12 +42,16 @@ class CompletionTargetTest extends AbstractContextTest { } Future assertTarget(entityText, nodeText, - {int argIndex: null, bool isFunctionalArgument: false}) async { + {int argIndex: null, + bool isFunctionalArgument: false, + String droppedToken}) async { void assertCommon() { expect(target.entity.toString(), entityText, reason: 'entity'); expect(target.containingNode.toString(), nodeText, reason: 'containingNode'); expect(target.argIndex, argIndex, reason: 'argIndex'); + expect(target.droppedToken?.toString(), droppedToken ?? isNull, + reason: 'droppedToken'); } // Assert with parsed unit @@ -403,6 +410,16 @@ class CompletionTargetTest extends AbstractContextTest { '// normal comment ', 'class C2 {zoo(z) {} String name;}'); } + test_IfStatement_droppedToken() async { + // Comment ClassDeclaration CompilationUnit + await addTestSource('main() { if (v i^) }'); + if (usingFastaParser) { + await assertTarget(')', '(v)', droppedToken: 'i'); + } else { + await assertTarget('i;', 'if (v) i;'); + } + } + test_MethodDeclaration_inLineComment2() async { // Comment ClassDeclaration CompilationUnit await addTestSource(''' @@ -555,13 +572,13 @@ class C2 { test_SwitchStatement_c() async { // Token('c') SwitchStatement await addTestSource('main() { switch(x) {c^} }'); - await assertTarget('}', 'switch (x) {}'); + await assertTarget('}', 'switch (x) {}', droppedToken: 'c'); } test_SwitchStatement_c2() async { // Token('c') SwitchStatement await addTestSource('main() { switch(x) { c^ } }'); - await assertTarget('}', 'switch (x) {}'); + await assertTarget('}', 'switch (x) {}', droppedToken: 'c'); } test_SwitchStatement_empty() async {