From 47090fda44185d5d8b7f7ba000426820c123e145 Mon Sep 17 00:00:00 2001 From: Sg Date: Tue, 18 Jul 2023 21:06:27 +0800 Subject: [PATCH 01/17] fix(54729): skip invalid completion location check in newline --- src/services/completions.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index c0e58f01b8e3d..e820158110b83 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -3196,8 +3196,14 @@ function getCompletionData( importStatementCompletion = importStatementCompletionInfo; isNewIdentifierLocation = importStatementCompletionInfo.isNewIdentifierLocation; } + // Bail out if this is a known invalid completion location - if (!importStatementCompletionInfo.replacementSpan && isCompletionListBlocker(contextToken)) { + + // GH#54729 skip invalid completion location check if in newline + const contentTokenLineEnd = sourceFile.getLineEndOfPosition(contextToken.getEnd()); + const isCurrentInNewLine = contentTokenLineEnd < position; + + if (!importStatementCompletionInfo.replacementSpan && !isCurrentInNewLine && isCompletionListBlocker(contextToken)) { log("Returning an empty list because completion was requested in an invalid position."); return keywordFilters ? keywordCompletionData(keywordFilters, isJsOnlyLocation, isNewIdentifierDefinitionLocation()) From a6b5427e1078027a7b6a70a66264e275e05eb0f5 Mon Sep 17 00:00:00 2001 From: Sg Date: Tue, 18 Jul 2023 22:20:33 +0800 Subject: [PATCH 02/17] fix: fix failed cases --- src/services/completions.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index e820158110b83..492605da9631e 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -3199,11 +3199,7 @@ function getCompletionData( // Bail out if this is a known invalid completion location - // GH#54729 skip invalid completion location check if in newline - const contentTokenLineEnd = sourceFile.getLineEndOfPosition(contextToken.getEnd()); - const isCurrentInNewLine = contentTokenLineEnd < position; - - if (!importStatementCompletionInfo.replacementSpan && !isCurrentInNewLine && isCompletionListBlocker(contextToken)) { + if (!importStatementCompletionInfo.replacementSpan && isCompletionListBlocker(contextToken)) { log("Returning an empty list because completion was requested in an invalid position."); return keywordFilters ? keywordCompletionData(keywordFilters, isJsOnlyLocation, isNewIdentifierDefinitionLocation()) @@ -4068,10 +4064,15 @@ function getCompletionData( return scope; } + function isInDifferentLineWithContextToken(contextToken: Node, position: number): boolean { + return sourceFile.getLineEndOfPosition(contextToken.getEnd()) < position; + } function isCompletionListBlocker(contextToken: Node): boolean { const start = timestamp(); + // const isCurrentInNewLine = ; const result = isInStringOrRegularExpressionOrTemplateLiteral(contextToken) || - isSolelyIdentifierDefinitionLocation(contextToken) || + // GH#54729 ignore this case when current position is in a newline + (isSolelyIdentifierDefinitionLocation(contextToken) && !isInDifferentLineWithContextToken(contextToken, position) ) || isDotOfNumericLiteral(contextToken) || isInJsxText(contextToken) || isBigIntLiteral(contextToken); From 2a0ff0617e426d967aa232e6d86d47073527fc4f Mon Sep 17 00:00:00 2001 From: Sg Date: Tue, 18 Jul 2023 22:26:45 +0800 Subject: [PATCH 03/17] test: add test case --- tests/cases/fourslash/server/completions04.ts | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 tests/cases/fourslash/server/completions04.ts diff --git a/tests/cases/fourslash/server/completions04.ts b/tests/cases/fourslash/server/completions04.ts new file mode 100644 index 0000000000000..947b350089ab3 --- /dev/null +++ b/tests/cases/fourslash/server/completions04.ts @@ -0,0 +1,8 @@ +/// + +// issue: https://github.com/microsoft/TypeScript/issues/54729 + +//// let foo +//// /**/ + +verify.completions({ marker: "", includes: "const" }); \ No newline at end of file From 212931910035e2e2835085cd561abfeb0c1791a8 Mon Sep 17 00:00:00 2001 From: Sg Date: Tue, 18 Jul 2023 23:24:01 +0800 Subject: [PATCH 04/17] fix: fix failed test --- .../cases/fourslash/completionAfterNewline.ts | 20 +++++++++++++++++++ tests/cases/fourslash/server/completions04.ts | 8 -------- 2 files changed, 20 insertions(+), 8 deletions(-) create mode 100644 tests/cases/fourslash/completionAfterNewline.ts delete mode 100644 tests/cases/fourslash/server/completions04.ts diff --git a/tests/cases/fourslash/completionAfterNewline.ts b/tests/cases/fourslash/completionAfterNewline.ts new file mode 100644 index 0000000000000..1dcc0a4e73848 --- /dev/null +++ b/tests/cases/fourslash/completionAfterNewline.ts @@ -0,0 +1,20 @@ +/// + +// issue: https://github.com/microsoft/TypeScript/issues/54729 +// Tests that `isCompletionListBlocker` is true at position 1, but false after a newline. + +////let foo /*1*/ +/////*2*/ +/////*3*/ + +verify.completions( + { marker: "1", exact: undefined }, + { + marker: ["2", "3"], + exact: completion.globalsPlus([ + { + name: "foo", + }, + ]), + } +); diff --git a/tests/cases/fourslash/server/completions04.ts b/tests/cases/fourslash/server/completions04.ts deleted file mode 100644 index 947b350089ab3..0000000000000 --- a/tests/cases/fourslash/server/completions04.ts +++ /dev/null @@ -1,8 +0,0 @@ -/// - -// issue: https://github.com/microsoft/TypeScript/issues/54729 - -//// let foo -//// /**/ - -verify.completions({ marker: "", includes: "const" }); \ No newline at end of file From ffdcfe4cf9f212e3bdddfe5c69458e54f401f103 Mon Sep 17 00:00:00 2001 From: Sg Date: Tue, 18 Jul 2023 23:35:16 +0800 Subject: [PATCH 05/17] test: add one more case --- .../fourslash/completionAfterNewline2.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests/cases/fourslash/completionAfterNewline2.ts diff --git a/tests/cases/fourslash/completionAfterNewline2.ts b/tests/cases/fourslash/completionAfterNewline2.ts new file mode 100644 index 0000000000000..52ca2c49945df --- /dev/null +++ b/tests/cases/fourslash/completionAfterNewline2.ts @@ -0,0 +1,21 @@ +/// + +// issue: https://github.com/microsoft/TypeScript/issues/54729 +// Tests that `isCompletionListBlocker` is true at position 1, but false after a newline. + +////const obj = { +//// name: '', +////} as const /*1*/ +/////*2*/ + +verify.completions( + { marker: "1", exact: undefined }, + { + marker: "2", + exact: completion.globalsPlus([ + { + name: "obj", + }, + ]), + } +); From 0588091706982d311af2151f89c666e89f9dfdc2 Mon Sep 17 00:00:00 2001 From: Sg Date: Tue, 18 Jul 2023 23:37:47 +0800 Subject: [PATCH 06/17] test: modify test case --- tests/cases/fourslash/completionAfterNewline2.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/cases/fourslash/completionAfterNewline2.ts b/tests/cases/fourslash/completionAfterNewline2.ts index 52ca2c49945df..746039063b110 100644 --- a/tests/cases/fourslash/completionAfterNewline2.ts +++ b/tests/cases/fourslash/completionAfterNewline2.ts @@ -3,18 +3,16 @@ // issue: https://github.com/microsoft/TypeScript/issues/54729 // Tests that `isCompletionListBlocker` is true at position 1, but false after a newline. -////const obj = { -//// name: '', -////} as const /*1*/ +////let foo = 5 as const /*1*/ /////*2*/ verify.completions( - { marker: "1", exact: undefined }, + { marker: ["1"], exact: undefined }, { marker: "2", exact: completion.globalsPlus([ { - name: "obj", + name: "foo", }, ]), } From deff25bc9964031b16c1c2c8511663218906d559 Mon Sep 17 00:00:00 2001 From: Sg Date: Tue, 18 Jul 2023 23:39:26 +0800 Subject: [PATCH 07/17] refactor: clean code --- src/services/completions.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 492605da9631e..85e3c5ec0791f 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -3196,9 +3196,7 @@ function getCompletionData( importStatementCompletion = importStatementCompletionInfo; isNewIdentifierLocation = importStatementCompletionInfo.isNewIdentifierLocation; } - // Bail out if this is a known invalid completion location - if (!importStatementCompletionInfo.replacementSpan && isCompletionListBlocker(contextToken)) { log("Returning an empty list because completion was requested in an invalid position."); return keywordFilters @@ -4069,7 +4067,6 @@ function getCompletionData( } function isCompletionListBlocker(contextToken: Node): boolean { const start = timestamp(); - // const isCurrentInNewLine = ; const result = isInStringOrRegularExpressionOrTemplateLiteral(contextToken) || // GH#54729 ignore this case when current position is in a newline (isSolelyIdentifierDefinitionLocation(contextToken) && !isInDifferentLineWithContextToken(contextToken, position) ) || From b91464d55eee462e6662bf97c4e5afd7a2151aa4 Mon Sep 17 00:00:00 2001 From: Sg Date: Tue, 18 Jul 2023 23:43:13 +0800 Subject: [PATCH 08/17] chore: fix lint error --- src/services/completions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 85e3c5ec0791f..7fa2fcafb8abf 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4069,7 +4069,7 @@ function getCompletionData( const start = timestamp(); const result = isInStringOrRegularExpressionOrTemplateLiteral(contextToken) || // GH#54729 ignore this case when current position is in a newline - (isSolelyIdentifierDefinitionLocation(contextToken) && !isInDifferentLineWithContextToken(contextToken, position) ) || + (isSolelyIdentifierDefinitionLocation(contextToken) && !isInDifferentLineWithContextToken(contextToken, position)) || isDotOfNumericLiteral(contextToken) || isInJsxText(contextToken) || isBigIntLiteral(contextToken); From 6eb4ff304e063c13daa86063a2cb57f9a5509e74 Mon Sep 17 00:00:00 2001 From: Sg Date: Tue, 18 Jul 2023 23:46:28 +0800 Subject: [PATCH 09/17] chore: clean test code --- tests/cases/fourslash/completionAfterNewline2.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cases/fourslash/completionAfterNewline2.ts b/tests/cases/fourslash/completionAfterNewline2.ts index 746039063b110..cd4b27ef9936b 100644 --- a/tests/cases/fourslash/completionAfterNewline2.ts +++ b/tests/cases/fourslash/completionAfterNewline2.ts @@ -7,7 +7,7 @@ /////*2*/ verify.completions( - { marker: ["1"], exact: undefined }, + { marker: "1", exact: undefined }, { marker: "2", exact: completion.globalsPlus([ From a2ae89926cc025ed7b49fcfdf095822f7ebb6f10 Mon Sep 17 00:00:00 2001 From: Sg Date: Thu, 16 Nov 2023 10:48:15 +0800 Subject: [PATCH 10/17] Update src/services/completions.ts Co-authored-by: Isabel Duan --- src/services/completions.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 7fa2fcafb8abf..56ce1e84460a4 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4062,7 +4062,8 @@ function getCompletionData( return scope; } - function isInDifferentLineWithContextToken(contextToken: Node, position: number): boolean { + function isInDifferentLineThanContextToken(contextToken: Node, position: number): boolean { + return sourceFile.getLineEndOfPosition(contextToken.getEnd()) < position; } function isCompletionListBlocker(contextToken: Node): boolean { From dbbdae5d3008df6c3cc08730c160334c91385f08 Mon Sep 17 00:00:00 2001 From: Sg Date: Thu, 16 Nov 2023 10:48:31 +0800 Subject: [PATCH 11/17] Update src/services/completions.ts Co-authored-by: Isabel Duan --- src/services/completions.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/completions.ts b/src/services/completions.ts index 56ce1e84460a4..4234e82716c20 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4066,6 +4066,7 @@ function getCompletionData( return sourceFile.getLineEndOfPosition(contextToken.getEnd()) < position; } + function isCompletionListBlocker(contextToken: Node): boolean { const start = timestamp(); const result = isInStringOrRegularExpressionOrTemplateLiteral(contextToken) || From 380dd4baec23fcf5722d3aed6ec70374365db8ad Mon Sep 17 00:00:00 2001 From: Sg Date: Thu, 16 Nov 2023 12:43:52 +0800 Subject: [PATCH 12/17] refactor --- src/services/completions.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index c0f2ec5e0fb57..8c606630980a3 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4162,16 +4162,10 @@ function getCompletionData( return scope; } - function isInDifferentLineThanContextToken(contextToken: Node, position: number): boolean { - - return sourceFile.getLineEndOfPosition(contextToken.getEnd()) < position; - } - function isCompletionListBlocker(contextToken: Node): boolean { const start = timestamp(); const result = isInStringOrRegularExpressionOrTemplateLiteral(contextToken) || - // GH#54729 ignore this case when current position is in a newline - (isSolelyIdentifierDefinitionLocation(contextToken) && !isInDifferentLineWithContextToken(contextToken, position)) || + isSolelyIdentifierDefinitionLocation(contextToken) || isDotOfNumericLiteral(contextToken) || isInJsxText(contextToken) || isBigIntLiteral(contextToken); @@ -4663,6 +4657,10 @@ function getCompletionData( return undefined; } + function isInDifferentLineThanContextToken(contextToken: Node, position: number): boolean { + return sourceFile.getLineEndOfPosition(contextToken.getEnd()) < position; + } + /** * @returns true if we are certain that the currently edited location must define a new location; false otherwise. */ @@ -4835,7 +4833,9 @@ function getCompletionData( && !isJsxAttribute(contextToken.parent) // Don't block completions if we're in `class C /**/`, `interface I /**/` or `` , because we're *past* the end of the identifier and might want to complete `extends`. // If `contextToken !== previousToken`, this is `class C ex/**/`, `interface I ex/**/` or ``. - && !((isClassLike(contextToken.parent) || isInterfaceDeclaration(contextToken.parent) || isTypeParameterDeclaration(contextToken.parent)) && (contextToken !== previousToken || position > previousToken.end)); + && !((isClassLike(contextToken.parent) || isInterfaceDeclaration(contextToken.parent) || isTypeParameterDeclaration(contextToken.parent)) && (contextToken !== previousToken || position > previousToken.end)) + // TODO: add comment + && !isInDifferentLineThanContextToken(contextToken, position); } function isPreviousPropertyDeclarationTerminated(contextToken: Node, position: number) { From 1de8bebe269ef63abaa31917579578001a3a6869 Mon Sep 17 00:00:00 2001 From: Sg Date: Thu, 16 Nov 2023 14:58:20 +0800 Subject: [PATCH 13/17] fix --- src/services/completions.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 8c606630980a3..c15ce3c297741 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4728,7 +4728,7 @@ function getCompletionData( case SyntaxKind.SetKeyword: return !isFromObjectTypeDeclaration(contextToken); - case SyntaxKind.Identifier: + case SyntaxKind.Identifier: { if (containingNodeKind === SyntaxKind.ImportSpecifier && contextToken === (parent as ImportSpecifier).name && (contextToken as Identifier).text === "type" @@ -4736,7 +4736,14 @@ function getCompletionData( // import { type | } return false; } + const ancestorVariableDeclarationLike = findAncestor(contextToken.parent, isVariableDeclaration); + if (ancestorVariableDeclarationLike && isInDifferentLineThanContextToken(contextToken, position)) { + // let a + // | + return false; + } break; + } case SyntaxKind.ClassKeyword: case SyntaxKind.EnumKeyword: @@ -4833,9 +4840,7 @@ function getCompletionData( && !isJsxAttribute(contextToken.parent) // Don't block completions if we're in `class C /**/`, `interface I /**/` or `` , because we're *past* the end of the identifier and might want to complete `extends`. // If `contextToken !== previousToken`, this is `class C ex/**/`, `interface I ex/**/` or ``. - && !((isClassLike(contextToken.parent) || isInterfaceDeclaration(contextToken.parent) || isTypeParameterDeclaration(contextToken.parent)) && (contextToken !== previousToken || position > previousToken.end)) - // TODO: add comment - && !isInDifferentLineThanContextToken(contextToken, position); + && !((isClassLike(contextToken.parent) || isInterfaceDeclaration(contextToken.parent) || isTypeParameterDeclaration(contextToken.parent)) && (contextToken !== previousToken || position > previousToken.end)); } function isPreviousPropertyDeclarationTerminated(contextToken: Node, position: number) { From c6accf5772b12e78803cfd8bbb11e504d08deeab Mon Sep 17 00:00:00 2001 From: Sg Date: Thu, 16 Nov 2023 14:59:36 +0800 Subject: [PATCH 14/17] rename variable --- src/services/completions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index c15ce3c297741..6f91ea01573f0 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4736,8 +4736,8 @@ function getCompletionData( // import { type | } return false; } - const ancestorVariableDeclarationLike = findAncestor(contextToken.parent, isVariableDeclaration); - if (ancestorVariableDeclarationLike && isInDifferentLineThanContextToken(contextToken, position)) { + const ancestorVariableDeclaration = findAncestor(contextToken.parent, isVariableDeclaration); + if (ancestorVariableDeclaration && isInDifferentLineThanContextToken(contextToken, position)) { // let a // | return false; From 6b6d53f6efe026b05cbedff88aa7fb2c1b7fa953 Mon Sep 17 00:00:00 2001 From: Sg Date: Thu, 16 Nov 2023 15:00:25 +0800 Subject: [PATCH 15/17] fmt --- src/services/completions.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 6f91ea01573f0..e9fc8c310ddee 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4736,8 +4736,10 @@ function getCompletionData( // import { type | } return false; } - const ancestorVariableDeclaration = findAncestor(contextToken.parent, isVariableDeclaration); - if (ancestorVariableDeclaration && isInDifferentLineThanContextToken(contextToken, position)) { + const ancestorVariableDeclaration = findAncestor( + contextToken.parent, isVariableDeclaration); + if (ancestorVariableDeclaration + && isInDifferentLineThanContextToken(contextToken, position)) { // let a // | return false; From e44eedde332798c52aae792db04d5468def88711 Mon Sep 17 00:00:00 2001 From: Sg Date: Thu, 16 Nov 2023 15:02:29 +0800 Subject: [PATCH 16/17] reword tests --- tests/cases/fourslash/completionAfterNewline.ts | 2 +- tests/cases/fourslash/completionAfterNewline2.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cases/fourslash/completionAfterNewline.ts b/tests/cases/fourslash/completionAfterNewline.ts index 1dcc0a4e73848..81efbb27eedcd 100644 --- a/tests/cases/fourslash/completionAfterNewline.ts +++ b/tests/cases/fourslash/completionAfterNewline.ts @@ -1,7 +1,7 @@ /// // issue: https://github.com/microsoft/TypeScript/issues/54729 -// Tests that `isCompletionListBlocker` is true at position 1, but false after a newline. +// Tests that `isCompletionListBlocker` returns true at position 1, ans returns false after a newline. ////let foo /*1*/ /////*2*/ diff --git a/tests/cases/fourslash/completionAfterNewline2.ts b/tests/cases/fourslash/completionAfterNewline2.ts index cd4b27ef9936b..495800ce17875 100644 --- a/tests/cases/fourslash/completionAfterNewline2.ts +++ b/tests/cases/fourslash/completionAfterNewline2.ts @@ -1,7 +1,7 @@ /// // issue: https://github.com/microsoft/TypeScript/issues/54729 -// Tests that `isCompletionListBlocker` is true at position 1, but false after a newline. +// Tests that `isCompletionListBlocker` returns true at position 1, and returns false after a newline. ////let foo = 5 as const /*1*/ /////*2*/ From 6359bc7e4d30cacacb45a15707e4414ff3c84d24 Mon Sep 17 00:00:00 2001 From: Sg Date: Fri, 17 Nov 2023 13:54:27 +0800 Subject: [PATCH 17/17] Update tests/cases/fourslash/completionAfterNewline.ts Co-authored-by: Isabel Duan --- tests/cases/fourslash/completionAfterNewline.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/cases/fourslash/completionAfterNewline.ts b/tests/cases/fourslash/completionAfterNewline.ts index 81efbb27eedcd..94637a9fd9b41 100644 --- a/tests/cases/fourslash/completionAfterNewline.ts +++ b/tests/cases/fourslash/completionAfterNewline.ts @@ -1,7 +1,8 @@ /// // issue: https://github.com/microsoft/TypeScript/issues/54729 -// Tests that `isCompletionListBlocker` returns true at position 1, ans returns false after a newline. +// Tests that `isCompletionListBlocker` returns true at position 1, and returns false after a newline. + ////let foo /*1*/ /////*2*/