From a61ec94501ccabae192415067101c58f4d916c5c Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 2 Jul 2015 16:04:34 -0700 Subject: [PATCH 1/8] Remove unnecessary check. --- src/services/services.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/services/services.ts b/src/services/services.ts index 159b1b817b1..80fde22c73b 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -3359,7 +3359,6 @@ namespace ts { case SyntaxKind.DotDotDotToken: return containingNodeKind === SyntaxKind.Parameter || - containingNodeKind === SyntaxKind.Constructor || (previousToken.parent && previousToken.parent.parent && previousToken.parent.parent.kind === SyntaxKind.ArrayBindingPattern); // var [...z| From 4a53096171c9ad64b2517558718df9db72b002ed Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 2 Jul 2015 16:14:06 -0700 Subject: [PATCH 2/8] Added test. --- .../cases/fourslash/completionListAfterSpreadOperator01.ts | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 tests/cases/fourslash/completionListAfterSpreadOperator01.ts diff --git a/tests/cases/fourslash/completionListAfterSpreadOperator01.ts b/tests/cases/fourslash/completionListAfterSpreadOperator01.ts new file mode 100644 index 00000000000..48a50124daf --- /dev/null +++ b/tests/cases/fourslash/completionListAfterSpreadOperator01.ts @@ -0,0 +1,7 @@ +/// + +////let v = [1,2,3,4]; +////let x = [.../**/ + +goTo.marker(); +verify.completionListContains("v"); \ No newline at end of file From 935071f1c53bf042a62a7f9a5b4b0e6e853fc653 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 2 Jul 2015 16:21:09 -0700 Subject: [PATCH 3/8] Don't show completion with dots not part of property accesses and qualified names. --- src/services/services.ts | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 80fde22c73b..71f92d0c4c8 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2902,23 +2902,30 @@ namespace ts { let jsx = options.jsx !== JsxEmit.None; let target = options.target; - // Find the node where completion is requested on, in the case of a completion after - // a dot, it is the member access expression other wise, it is a request for all - // visible symbols in the scope, and the node is the current location. + // Find the node where completion is requested on. + // Also determine whether we are trying to complete with members of that node + // or attributes of a JSX tag. let node = currentToken; let isRightOfDot = false; let isRightOfOpenTag = false; let location = getTouchingPropertyName(sourceFile, position); - if(contextToken) { - let kind = contextToken.kind; - if (kind === SyntaxKind.DotToken && contextToken.parent.kind === SyntaxKind.PropertyAccessExpression) { - node = (contextToken.parent).expression; - isRightOfDot = true; - } - else if (kind === SyntaxKind.DotToken && contextToken.parent.kind === SyntaxKind.QualifiedName) { - node = (contextToken.parent).left; - isRightOfDot = true; + if (contextToken) { + let { parent, kind } = contextToken; + if (kind === SyntaxKind.DotToken) { + if (parent.kind === SyntaxKind.PropertyAccessExpression) { + node = (contextToken.parent).expression; + isRightOfDot = true; + } + else if (parent.kind === SyntaxKind.QualifiedName) { + node = (contextToken.parent).left; + isRightOfDot = true; + } + else { + // There is nothing that precedes the dot, so this likely just a stray character + // or leading into a '...' token. Just bail out instead. + return undefined; + } } else if (kind === SyntaxKind.LessThanToken && sourceFile.languageVariant === LanguageVariant.JSX) { isRightOfOpenTag = true; From d89c1d6586b79de7b0a8470b1b4f205cc4627e37 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 2 Jul 2015 17:08:03 -0700 Subject: [PATCH 4/8] Fixed/added test. --- .../fourslash/completionListBuilderLocations_Modules.ts | 3 +-- tests/cases/fourslash/memberListAfterDoubleDot.ts | 6 ++++++ tests/cases/fourslash/memberListAfterSingleDot.ts | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 tests/cases/fourslash/memberListAfterDoubleDot.ts diff --git a/tests/cases/fourslash/completionListBuilderLocations_Modules.ts b/tests/cases/fourslash/completionListBuilderLocations_Modules.ts index eac7085ab66..8a136d0125c 100644 --- a/tests/cases/fourslash/completionListBuilderLocations_Modules.ts +++ b/tests/cases/fourslash/completionListBuilderLocations_Modules.ts @@ -8,6 +8,5 @@ test.markers().forEach((m) => { goTo.position(m.position, m.fileName); - verify.not.completionListIsEmpty(); - verify.completionListAllowsNewIdentifier(); + verify.completionListIsEmpty(); }); \ No newline at end of file diff --git a/tests/cases/fourslash/memberListAfterDoubleDot.ts b/tests/cases/fourslash/memberListAfterDoubleDot.ts new file mode 100644 index 00000000000..21a1ad913ea --- /dev/null +++ b/tests/cases/fourslash/memberListAfterDoubleDot.ts @@ -0,0 +1,6 @@ +/// + +////../**/ + +goTo.marker(); +verify.memberListIsEmpty(); \ No newline at end of file diff --git a/tests/cases/fourslash/memberListAfterSingleDot.ts b/tests/cases/fourslash/memberListAfterSingleDot.ts index 0bc5107f2c1..62edf3b8c9e 100644 --- a/tests/cases/fourslash/memberListAfterSingleDot.ts +++ b/tests/cases/fourslash/memberListAfterSingleDot.ts @@ -3,4 +3,4 @@ ////./**/ goTo.marker(); -verify.not.memberListIsEmpty(); \ No newline at end of file +verify.memberListIsEmpty(); \ No newline at end of file From 3fe0d3defe974a72382f22917486247644dea20e Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Mon, 6 Jul 2015 11:30:43 -0700 Subject: [PATCH 5/8] Updated test. --- .../fourslash/completionListBuilderLocations_Modules.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/cases/fourslash/completionListBuilderLocations_Modules.ts b/tests/cases/fourslash/completionListBuilderLocations_Modules.ts index 8a136d0125c..0552e540a7e 100644 --- a/tests/cases/fourslash/completionListBuilderLocations_Modules.ts +++ b/tests/cases/fourslash/completionListBuilderLocations_Modules.ts @@ -5,8 +5,8 @@ ////module A./*moduleName2*/ +goTo.marker("moduleName1"); +verify.not.completionListIsEmpty(); -test.markers().forEach((m) => { - goTo.position(m.position, m.fileName); - verify.completionListIsEmpty(); -}); \ No newline at end of file +goTo.marker("moduleName2"); +verify.completionListIsEmpty(); From a7983a4f3e676cd75a971dc3d69e99f36f103b7c Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Mon, 6 Jul 2015 12:40:05 -0700 Subject: [PATCH 6/8] Added test for JSX spread properties. --- tests/cases/fourslash/tsxCompletion4.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/cases/fourslash/tsxCompletion4.ts diff --git a/tests/cases/fourslash/tsxCompletion4.ts b/tests/cases/fourslash/tsxCompletion4.ts new file mode 100644 index 00000000000..6a66c4a898f --- /dev/null +++ b/tests/cases/fourslash/tsxCompletion4.ts @@ -0,0 +1,14 @@ +/// + +//@Filename: file.tsx +//// declare namespace JSX { +//// interface Element { } +//// interface IntrinsicElements { +//// div: { one; two; } +//// } +//// } +//// let bag = { x: 100, y: 200 }; +////
Date: Mon, 6 Jul 2015 13:00:10 -0700 Subject: [PATCH 7/8] previousToken -> contextToken --- src/services/services.ts | 50 ++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 71f92d0c4c8..ac55e888c03 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -3142,11 +3142,11 @@ namespace ts { return scope; } - function isCompletionListBlocker(previousToken: Node): boolean { + function isCompletionListBlocker(contextToken: Node): boolean { let start = new Date().getTime(); - let result = isInStringOrRegularExpressionOrTemplateLiteral(previousToken) || - isIdentifierDefinitionLocation(previousToken) || - isRightOfIllegalDot(previousToken); + let result = isInStringOrRegularExpressionOrTemplateLiteral(contextToken) || + isIdentifierDefinitionLocation(contextToken) || + isRightOfIllegalDot(contextToken); log("getCompletionsAtPosition: isCompletionListBlocker: " + (new Date().getTime() - start)); return result; } @@ -3225,12 +3225,12 @@ namespace ts { return false; } - function isInStringOrRegularExpressionOrTemplateLiteral(previousToken: Node): boolean { - if (previousToken.kind === SyntaxKind.StringLiteral - || previousToken.kind === SyntaxKind.RegularExpressionLiteral - || isTemplateLiteralKind(previousToken.kind)) { - let start = previousToken.getStart(); - let end = previousToken.getEnd(); + function isInStringOrRegularExpressionOrTemplateLiteral(contextToken: Node): boolean { + if (contextToken.kind === SyntaxKind.StringLiteral + || contextToken.kind === SyntaxKind.RegularExpressionLiteral + || isTemplateLiteralKind(contextToken.kind)) { + let start = contextToken.getStart(); + let end = contextToken.getEnd(); // To be "in" one of these literals, the position has to be: // 1. entirely within the token text. @@ -3241,8 +3241,8 @@ namespace ts { } if (position === end) { - return !!(previousToken).isUnterminated || - previousToken.kind === SyntaxKind.RegularExpressionLiteral; + return !!(contextToken).isUnterminated || + contextToken.kind === SyntaxKind.RegularExpressionLiteral; } } @@ -3316,10 +3316,10 @@ namespace ts { return false; } - function isIdentifierDefinitionLocation(previousToken: Node): boolean { - if (previousToken) { - let containingNodeKind = previousToken.parent.kind; - switch (previousToken.kind) { + function isIdentifierDefinitionLocation(contextToken: Node): boolean { + if (contextToken) { + let containingNodeKind = contextToken.parent.kind; + switch (contextToken.kind) { case SyntaxKind.CommaToken: return containingNodeKind === SyntaxKind.VariableDeclaration || containingNodeKind === SyntaxKind.VariableDeclarationList || @@ -3351,9 +3351,9 @@ namespace ts { case SyntaxKind.SemicolonToken: return containingNodeKind === SyntaxKind.PropertySignature && - previousToken.parent && previousToken.parent.parent && - (previousToken.parent.parent.kind === SyntaxKind.InterfaceDeclaration || // interface a { f; | - previousToken.parent.parent.kind === SyntaxKind.TypeLiteral); // let x : { a; | + contextToken.parent && contextToken.parent.parent && + (contextToken.parent.parent.kind === SyntaxKind.InterfaceDeclaration || // interface a { f; | + contextToken.parent.parent.kind === SyntaxKind.TypeLiteral); // let x : { a; | case SyntaxKind.LessThanToken: return containingNodeKind === SyntaxKind.ClassDeclaration || // class A< | @@ -3366,8 +3366,8 @@ namespace ts { case SyntaxKind.DotDotDotToken: return containingNodeKind === SyntaxKind.Parameter || - (previousToken.parent && previousToken.parent.parent && - previousToken.parent.parent.kind === SyntaxKind.ArrayBindingPattern); // var [...z| + (contextToken.parent && contextToken.parent.parent && + contextToken.parent.parent.kind === SyntaxKind.ArrayBindingPattern); // var [...z| case SyntaxKind.PublicKeyword: case SyntaxKind.PrivateKeyword: @@ -3390,7 +3390,7 @@ namespace ts { } // Previous token may have been a keyword that was converted to an identifier. - switch (previousToken.getText()) { + switch (contextToken.getText()) { case "class": case "interface": case "enum": @@ -3407,9 +3407,9 @@ namespace ts { return false; } - function isRightOfIllegalDot(previousToken: Node): boolean { - if (previousToken && previousToken.kind === SyntaxKind.NumericLiteral) { - let text = previousToken.getFullText(); + function isRightOfIllegalDot(contextToken: Node): boolean { + if (contextToken && contextToken.kind === SyntaxKind.NumericLiteral) { + let text = contextToken.getFullText(); return text.charAt(text.length - 1) === "."; } From cacc366809294e6f6552688d9637c031944394b5 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Mon, 6 Jul 2015 13:27:00 -0700 Subject: [PATCH 8/8] Avoid unnecessary contextToken checking, addressed CR feedback. --- src/services/services.ts | 170 +++++++++++++++++++-------------------- 1 file changed, 82 insertions(+), 88 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index ac55e888c03..3d92bd5b476 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2892,16 +2892,6 @@ namespace ts { log("getCompletionData: Get previous token 2: " + (new Date().getTime() - start)); } - // Check if this is a valid completion location - if (contextToken && isCompletionListBlocker(contextToken)) { - log("Returning an empty list because completion was requested in an invalid position."); - return undefined; - } - - let options = program.getCompilerOptions(); - let jsx = options.jsx !== JsxEmit.None; - let target = options.target; - // Find the node where completion is requested on. // Also determine whether we are trying to complete with members of that node // or attributes of a JSX tag. @@ -2911,6 +2901,12 @@ namespace ts { let location = getTouchingPropertyName(sourceFile, position); if (contextToken) { + // Bail out if this is a known invalid completion location + if (isCompletionListBlocker(contextToken)) { + log("Returning an empty list because completion was requested in an invalid position."); + return undefined; + } + let { parent, kind } = contextToken; if (kind === SyntaxKind.DotToken) { if (parent.kind === SyntaxKind.PropertyAccessExpression) { @@ -3146,7 +3142,7 @@ namespace ts { let start = new Date().getTime(); let result = isInStringOrRegularExpressionOrTemplateLiteral(contextToken) || isIdentifierDefinitionLocation(contextToken) || - isRightOfIllegalDot(contextToken); + isDotOfNumericLiteral(contextToken); log("getCompletionsAtPosition: isCompletionListBlocker: " + (new Date().getTime() - start)); return result; } @@ -3241,8 +3237,8 @@ namespace ts { } if (position === end) { - return !!(contextToken).isUnterminated || - contextToken.kind === SyntaxKind.RegularExpressionLiteral; + return !!(contextToken).isUnterminated + || contextToken.kind === SyntaxKind.RegularExpressionLiteral; } } @@ -3317,98 +3313,96 @@ namespace ts { } function isIdentifierDefinitionLocation(contextToken: Node): boolean { - if (contextToken) { - let containingNodeKind = contextToken.parent.kind; - switch (contextToken.kind) { - case SyntaxKind.CommaToken: - return containingNodeKind === SyntaxKind.VariableDeclaration || - containingNodeKind === SyntaxKind.VariableDeclarationList || - containingNodeKind === SyntaxKind.VariableStatement || - containingNodeKind === SyntaxKind.EnumDeclaration || // enum a { foo, | - isFunction(containingNodeKind) || - containingNodeKind === SyntaxKind.ClassDeclaration || // class A