From 0865bd45f2c8f412f832b29997c5f86d90b93f10 Mon Sep 17 00:00:00 2001 From: z0gSh1u Date: Tue, 13 Apr 2021 16:15:01 +0800 Subject: [PATCH 1/7] Complete `constructor` keyword after property declaration. --- src/services/completions.ts | 21 ++++++++++++++-- ...structorKeywordAfterPropertyDeclaration.ts | 24 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index 6dd3e51640a8e..5cafd9a9d861d 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2370,12 +2370,18 @@ namespace ts.Completions { return isPropertyDeclaration(contextToken.parent); } - return isDeclarationName(contextToken) + return (isDeclarationName(contextToken) || isTypeKeyword(contextToken.kind)) && !isShorthandPropertyAssignment(contextToken.parent) && !isJsxAttribute(contextToken.parent) // Don't block completions if we're in `class C /**/`, because we're *past* the end of the identifier and might want to complete `extends`. // If `contextToken !== previousToken`, this is `class C ex/**/`. - && !(isClassLike(contextToken.parent) && (contextToken !== previousToken || position > previousToken.end)); + && !(isClassLike(contextToken.parent) && (contextToken !== previousToken || position > previousToken.end)) + // Don't block completions for these 2: + // `class C { prop \n constructor/**/ }` + // `class C { prop: number \n constructor/**/ }` + && !(isClassLike(contextToken.parent.parent) && isPropertyDeclaration(contextToken.parent) && + (isIdentifier(contextToken) || isTypeKeyword(contextToken.kind)) + && previousToken.kind === SyntaxKind.ConstructorKeyword); } function isFunctionLikeButNotConstructor(kind: SyntaxKind) { @@ -2831,11 +2837,22 @@ namespace ts.Completions { if (!contextToken) return undefined; + // class c { prop: number \n constructor| } + if (isTypeKeyword(contextToken.kind) && location.kind === SyntaxKind.ConstructorKeyword) { + return contextToken.parent.parent as ObjectTypeDeclaration; + } + switch (contextToken.kind) { case SyntaxKind.EqualsToken: // class c { public prop = | /* global completions */ } return undefined; + case SyntaxKind.Identifier: // class c { prop \n constructor| } case SyntaxKind.SemicolonToken: // class c {getValue(): number; | } + // class c { prop: number; constructor| } + if (location.kind === SyntaxKind.ConstructorKeyword) { + return contextToken.parent.parent as ObjectTypeDeclaration; + } + // fall through case SyntaxKind.CloseBraceToken: // class c { method() { } | } // class c { method() { } b| } return isFromObjectTypeDeclaration(location) && (location.parent as ClassElement | TypeElement).name === location diff --git a/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts new file mode 100644 index 0000000000000..bdd94456a31c3 --- /dev/null +++ b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts @@ -0,0 +1,24 @@ +/// + +////class A { +//// bla +//// constructor/*1*/(props) {} +////} +////class B { +//// bla; +//// constructor/*2*/(props) {} +////} +////class C { +//// bla: number +//// constructor/*3*/(props) {} +////} +////class D { +//// bla: number; +//// constructor/*4*/(props) {} +////} + +verify.completions({ + marker: [1, 2, 3, 4].map(String), + includes: { name: "constructor", sortText: completion.SortText.GlobalsOrKeywords }, + isNewIdentifierLocation: true, +}); From 6edfc262825118bd26b324fe506b3fa88b77c04d Mon Sep 17 00:00:00 2001 From: z0gSh1u Date: Tue, 13 Apr 2021 18:11:49 +0800 Subject: [PATCH 2/7] Fix logical errors. --- src/services/completions.ts | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 5cafd9a9d861d..dd642529db996 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2370,18 +2370,21 @@ namespace ts.Completions { return isPropertyDeclaration(contextToken.parent); } - return (isDeclarationName(contextToken) || isTypeKeyword(contextToken.kind)) + // Don't block completions for these 2: + // `class C { prop \n constructor/**/ }` + // `class C { prop: number \n constructor/**/ }` + if ((isClassLike(contextToken.parent.parent) && isPropertyDeclaration(contextToken.parent) && + (isIdentifier(contextToken) || isTypeKeyword(contextToken.kind)) + && previousToken.kind === SyntaxKind.ConstructorKeyword)) { + return false; + } + + return isDeclarationName(contextToken) && !isShorthandPropertyAssignment(contextToken.parent) && !isJsxAttribute(contextToken.parent) // Don't block completions if we're in `class C /**/`, because we're *past* the end of the identifier and might want to complete `extends`. // If `contextToken !== previousToken`, this is `class C ex/**/`. - && !(isClassLike(contextToken.parent) && (contextToken !== previousToken || position > previousToken.end)) - // Don't block completions for these 2: - // `class C { prop \n constructor/**/ }` - // `class C { prop: number \n constructor/**/ }` - && !(isClassLike(contextToken.parent.parent) && isPropertyDeclaration(contextToken.parent) && - (isIdentifier(contextToken) || isTypeKeyword(contextToken.kind)) - && previousToken.kind === SyntaxKind.ConstructorKeyword); + && !(isClassLike(contextToken.parent) && (contextToken !== previousToken || position > previousToken.end)); } function isFunctionLikeButNotConstructor(kind: SyntaxKind) { @@ -2837,8 +2840,15 @@ namespace ts.Completions { if (!contextToken) return undefined; + // class c { prop \n constructor| } + // class c { prop; \n constructor| } // class c { prop: number \n constructor| } - if (isTypeKeyword(contextToken.kind) && location.kind === SyntaxKind.ConstructorKeyword) { + // class c { prop: number; constructor| } + if (location.kind === SyntaxKind.ConstructorKeyword && ( + isTypeKeyword(contextToken.kind) || + isIdentifier(contextToken) || + contextToken.kind === SyntaxKind.SemicolonToken + )) { return contextToken.parent.parent as ObjectTypeDeclaration; } @@ -2846,13 +2856,7 @@ namespace ts.Completions { case SyntaxKind.EqualsToken: // class c { public prop = | /* global completions */ } return undefined; - case SyntaxKind.Identifier: // class c { prop \n constructor| } case SyntaxKind.SemicolonToken: // class c {getValue(): number; | } - // class c { prop: number; constructor| } - if (location.kind === SyntaxKind.ConstructorKeyword) { - return contextToken.parent.parent as ObjectTypeDeclaration; - } - // fall through case SyntaxKind.CloseBraceToken: // class c { method() { } | } // class c { method() { } b| } return isFromObjectTypeDeclaration(location) && (location.parent as ClassElement | TypeElement).name === location From 59756d373e5425e5bf9e895072cd84d7f3aad6f5 Mon Sep 17 00:00:00 2001 From: z0gSh1u Date: Thu, 22 Apr 2021 17:20:18 +0800 Subject: [PATCH 3/7] Fix for more universal situations. --- src/services/completions.ts | 34 +++++--- ...structorKeywordAfterPropertyDeclaration.ts | 84 +++++++++++++++---- 2 files changed, 89 insertions(+), 29 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index dd642529db996..e347d295801cd 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2332,7 +2332,7 @@ namespace ts.Completions { return isFunctionLike(contextToken.parent) && !isMethodDeclaration(contextToken.parent); } - // If the previous token is keyword correspoding to class member completion keyword + // If the previous token is keyword corresponding to class member completion keyword // there will be completion available here if (isClassMemberCompletionKeyword(keywordForNode(contextToken)) && isFromObjectTypeDeclaration(contextToken)) { return false; @@ -2370,13 +2370,14 @@ namespace ts.Completions { return isPropertyDeclaration(contextToken.parent); } - // Don't block completions for these 2: - // `class C { prop \n constructor/**/ }` - // `class C { prop: number \n constructor/**/ }` - if ((isClassLike(contextToken.parent.parent) && isPropertyDeclaration(contextToken.parent) && - (isIdentifier(contextToken) || isTypeKeyword(contextToken.kind)) - && previousToken.kind === SyntaxKind.ConstructorKeyword)) { - return false; + // If we are inside a class declaration and typing `constructor`... + if (isClassLike(contextToken.parent.parent) + && isPropertyDeclaration(contextToken.parent) + && (isIdentifier(contextToken) || isTypeKeyword(contextToken.kind)) + // And the cursor is at the token... + && (position <= previousToken.end || contextToken.kind === SyntaxKind.SemicolonToken) + ) { + return false; // Don't block completions then. } return isDeclarationName(contextToken) @@ -2840,16 +2841,25 @@ namespace ts.Completions { if (!contextToken) return undefined; + function isPropertyBeforeConstructorInitalized(classDecl: ClassDeclaration, _constructor: ClassElement) { + const indexOfConstructor = classDecl.members.indexOf(_constructor); + if (indexOfConstructor <= 0) { + return true; + } + return isInitializedProperty(classDecl.members[indexOfConstructor - 1]); + } + // class c { prop \n constructor| } - // class c { prop; \n constructor| } // class c { prop: number \n constructor| } - // class c { prop: number; constructor| } + // class c { prop = somewhat \n constructor| } + // and also for `;` replacing `\n` if (location.kind === SyntaxKind.ConstructorKeyword && ( - isTypeKeyword(contextToken.kind) || isIdentifier(contextToken) || + isTypeKeyword(contextToken.kind) || + isPropertyBeforeConstructorInitalized(getAncestor(location, SyntaxKind.ClassDeclaration) as ClassDeclaration, location.parent as ClassElement) || contextToken.kind === SyntaxKind.SemicolonToken )) { - return contextToken.parent.parent as ObjectTypeDeclaration; + return getAncestor(contextToken, SyntaxKind.ClassDeclaration) as ObjectTypeDeclaration; } switch (contextToken.kind) { diff --git a/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts index bdd94456a31c3..be3c4d21400f2 100644 --- a/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts +++ b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts @@ -1,24 +1,74 @@ /// -////class A { -//// bla -//// constructor/*1*/(props) {} -////} -////class B { -//// bla; -//// constructor/*2*/(props) {} -////} -////class C { -//// bla: number -//// constructor/*3*/(props) {} -////} -////class D { -//// bla: number; -//// constructor/*4*/(props) {} -////} +//// // *** situations that `constructor` is partly present *** +//// class A { +//// blah; con/*1*/ +//// } +//// class B { +//// blah +//// con/*2*/ +//// } +//// class C { +//// blah; +//// con/*3*/ +//// } +//// class D { +//// blah: number +//// con/*4*/ +//// } +//// class E { +//// blah: number; con/*5*/ +//// } +//// class F { +//// blah = 123 +//// con/*6*/ +//// } +//// class G { +//// blah = 123; con/*7*/ +//// } +//// // *** situations that `constructor` is fully present *** +//// class H { +//// blah +//// constructor/*8*/(props) {} +//// } +//// class I { +//// blah; +//// constructor/*9*/(props) {} +//// } +//// class J { +//// blah: number +//// constructor/*10*/(props) {} +//// } +//// class K { +//// blah: number; +//// constructor/*11*/(props) {} +//// } +//// class L { +//// blah = 123 +//// constructor/*12*/(props) {} +//// } +//// class M { +//// blah = 123; +//// constructor/*13*/(props) {} +//// } +//// class N { +//// blah = [123] +//// constructor/*14*/(props) {} +//// } +//// class O { +//// blah = {key: 1} +//// constructor/*15*/(props) {} +//// } +//// function return1() { +//// return 1 +//// } +//// class P { +//// blah = return1() +//// constructor/*16*/(props) {} +//// } verify.completions({ - marker: [1, 2, 3, 4].map(String), + marker: Array.from(Array(16), (_, i) => String(i + 1)), includes: { name: "constructor", sortText: completion.SortText.GlobalsOrKeywords }, isNewIdentifierLocation: true, }); From d82462920ca8e709cb0709597960ef13e6f9dfc5 Mon Sep 17 00:00:00 2001 From: z0gSh1u Date: Fri, 23 Apr 2021 17:40:53 +0800 Subject: [PATCH 4/7] Only provide completions if property declaration is terminated. --- src/services/completions.ts | 24 ++++++--- ...structorKeywordAfterPropertyDeclaration.ts | 53 ++++++++++++++----- 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index e347d295801cd..977356673d5b1 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2370,14 +2370,22 @@ namespace ts.Completions { return isPropertyDeclaration(contextToken.parent); } - // If we are inside a class declaration and typing `constructor`... - if (isClassLike(contextToken.parent.parent) - && isPropertyDeclaration(contextToken.parent) - && (isIdentifier(contextToken) || isTypeKeyword(contextToken.kind)) - // And the cursor is at the token... - && (position <= previousToken.end || contextToken.kind === SyntaxKind.SemicolonToken) - ) { - return false; // Don't block completions then. + function isPreviousPropertyDeclarationTerminated(contextToken: Node, previousToken: Node) { + return getLinesBetweenPositions(sourceFile, contextToken.end, previousToken.end) > 0 || + contextToken.kind === SyntaxKind.SemicolonToken; + } + + // If we are inside a class declaration and typing `constructor` after property declaration... + if (contextToken !== previousToken + && isClassLike(previousToken.parent.parent) + && isPropertyDeclaration(contextToken.parent) + // After considering different contextToken... + && (isIdentifier(contextToken) || isTypeKeyword(contextToken.kind)) + // And the cursor is at the token... + && position <= previousToken.end) { + // If the previous property declaration is terminated according to newline or semicolon, don't block completions then. + // Otherwise, the completions should be blocked if we cant assert the previous property declaration is terminated. + return !isPreviousPropertyDeclarationTerminated(contextToken, previousToken); } return isDeclarationName(contextToken) diff --git a/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts index be3c4d21400f2..4a9b85189d071 100644 --- a/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts +++ b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts @@ -1,6 +1,6 @@ /// -//// // *** situations that `constructor` is partly present *** +//// // situations that `constructor` is partly present //// class A { //// blah; con/*1*/ //// } @@ -26,49 +26,78 @@ //// class G { //// blah = 123; con/*7*/ //// } -//// // *** situations that `constructor` is fully present *** +//// // situations that `constructor` is fully present //// class H { //// blah -//// constructor/*8*/(props) {} +//// constructor/*8*/ //// } //// class I { //// blah; -//// constructor/*9*/(props) {} +//// constructor/*9*/ //// } //// class J { //// blah: number -//// constructor/*10*/(props) {} +//// constructor/*10*/ //// } //// class K { //// blah: number; -//// constructor/*11*/(props) {} +//// constructor/*11*/ //// } //// class L { //// blah = 123 -//// constructor/*12*/(props) {} +//// constructor/*12*/ //// } //// class M { //// blah = 123; -//// constructor/*13*/(props) {} +//// constructor/*13*/ //// } //// class N { //// blah = [123] -//// constructor/*14*/(props) {} +//// constructor/*14*/ //// } //// class O { //// blah = {key: 1} -//// constructor/*15*/(props) {} +//// constructor/*15*/ //// } //// function return1() { //// return 1 //// } //// class P { //// blah = return1() -//// constructor/*16*/(props) {} +//// constructor/*16*/ +//// } +//// // situations that `constructor` isn't present +//// class Q { +//// blah; /*17*/ +//// } +//// class R { +//// blah +//// /*18*/ // dont complete since `blah \n = 123` is legal +//// } +//// class R { +//// blah /*19*/ +//// } +//// // situations that `constructor` should not be suggested +//// class S { +//// blah con/*20*/ +//// } +//// class T { +//// blah: number con/*21*/ //// } +const positiveCaseCount = 17; // 1~17 +const negativeCaseCount = 4; // 18~21 +const positiveCases = Array.from(Array(positiveCaseCount), (_, i) => String(i + 1)); +const negativeCases = Array.from(Array(negativeCaseCount), (_, i) => String(i + 1 + positiveCaseCount)); + verify.completions({ - marker: Array.from(Array(16), (_, i) => String(i + 1)), + marker: positiveCases, includes: { name: "constructor", sortText: completion.SortText.GlobalsOrKeywords }, isNewIdentifierLocation: true, }); + +verify.completions({ + marker: negativeCases, + exact: [], + isNewIdentifierLocation: true, +}); From dcadad37e8b343811ba2565cb8810ae7e0102e55 Mon Sep 17 00:00:00 2001 From: z0gSh1u Date: Sat, 24 Apr 2021 03:51:07 +0800 Subject: [PATCH 5/7] Simplify many logical conditions. --- src/services/completions.ts | 49 +++++++------------ ...structorKeywordAfterPropertyDeclaration.ts | 21 +++++--- 2 files changed, 31 insertions(+), 39 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 977356673d5b1..9bc4dd9aca8c9 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2370,22 +2370,19 @@ namespace ts.Completions { return isPropertyDeclaration(contextToken.parent); } - function isPreviousPropertyDeclarationTerminated(contextToken: Node, previousToken: Node) { - return getLinesBetweenPositions(sourceFile, contextToken.end, previousToken.end) > 0 || - contextToken.kind === SyntaxKind.SemicolonToken; - } - - // If we are inside a class declaration and typing `constructor` after property declaration... - if (contextToken !== previousToken + // If `constructor` is not present, but we request a completion manually... + if (contextToken === previousToken && isPreviousPropertyDeclarationTerminated(contextToken, position)) { + return false; // Don't block completions. + } + // If the previous property declaration is terminated according to newline or semicolon... + else if (isPreviousPropertyDeclarationTerminated(contextToken, previousToken.end) + // And we are inside a class declaration and typing `constructor` after property declaration... + && contextToken !== previousToken && isClassLike(previousToken.parent.parent) && isPropertyDeclaration(contextToken.parent) - // After considering different contextToken... - && (isIdentifier(contextToken) || isTypeKeyword(contextToken.kind)) // And the cursor is at the token... && position <= previousToken.end) { - // If the previous property declaration is terminated according to newline or semicolon, don't block completions then. - // Otherwise, the completions should be blocked if we cant assert the previous property declaration is terminated. - return !isPreviousPropertyDeclarationTerminated(contextToken, previousToken); + return false; // Don't block completions. } return isDeclarationName(contextToken) @@ -2396,6 +2393,12 @@ namespace ts.Completions { && !(isClassLike(contextToken.parent) && (contextToken !== previousToken || position > previousToken.end)); } + function isPreviousPropertyDeclarationTerminated(contextToken: Node, position: number) { + return contextToken.kind !== SyntaxKind.EqualsToken && + (contextToken.kind === SyntaxKind.SemicolonToken + || !positionsAreOnSameLine(contextToken.end, position, sourceFile)); + } + function isFunctionLikeButNotConstructor(kind: SyntaxKind) { return isFunctionLikeKind(kind) && kind !== SyntaxKind.Constructor; } @@ -2849,25 +2852,9 @@ namespace ts.Completions { if (!contextToken) return undefined; - function isPropertyBeforeConstructorInitalized(classDecl: ClassDeclaration, _constructor: ClassElement) { - const indexOfConstructor = classDecl.members.indexOf(_constructor); - if (indexOfConstructor <= 0) { - return true; - } - return isInitializedProperty(classDecl.members[indexOfConstructor - 1]); - } - - // class c { prop \n constructor| } - // class c { prop: number \n constructor| } - // class c { prop = somewhat \n constructor| } - // and also for `;` replacing `\n` - if (location.kind === SyntaxKind.ConstructorKeyword && ( - isIdentifier(contextToken) || - isTypeKeyword(contextToken.kind) || - isPropertyBeforeConstructorInitalized(getAncestor(location, SyntaxKind.ClassDeclaration) as ClassDeclaration, location.parent as ClassElement) || - contextToken.kind === SyntaxKind.SemicolonToken - )) { - return getAncestor(contextToken, SyntaxKind.ClassDeclaration) as ObjectTypeDeclaration; + if (location.kind === SyntaxKind.ConstructorKeyword + || (isIdentifier(contextToken) && isPropertyDeclaration(contextToken.parent) && isClassLike(location))) { + return findAncestor(contextToken, isClassLike) as ObjectTypeDeclaration; } switch (contextToken.kind) { diff --git a/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts index 4a9b85189d071..1ef5dcf8c7427 100644 --- a/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts +++ b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts @@ -1,5 +1,6 @@ /// +//// const GlobalWhat = 123 //// // situations that `constructor` is partly present //// class A { //// blah; con/*1*/ @@ -72,32 +73,36 @@ //// } //// class R { //// blah -//// /*18*/ // dont complete since `blah \n = 123` is legal +//// /*18*/ //// } +//// // situations that `constructor` should not be suggested //// class R { //// blah /*19*/ //// } -//// // situations that `constructor` should not be suggested //// class S { //// blah con/*20*/ //// } +//// // situations that `constructor` should not be suggested but //// class T { //// blah: number con/*21*/ //// } +//// const SomeValue = 123 +//// class U { +//// blah = SomeValue con/*22*/ +//// } -const positiveCaseCount = 17; // 1~17 -const negativeCaseCount = 4; // 18~21 -const positiveCases = Array.from(Array(positiveCaseCount), (_, i) => String(i + 1)); -const negativeCases = Array.from(Array(negativeCaseCount), (_, i) => String(i + 1 + positiveCaseCount)); +function generateRange(l: number, r: number) { + return Array.from(Array(r - l + 1), (_, i) => String(i + l)); // [l, r] +} verify.completions({ - marker: positiveCases, + marker: generateRange(1, 18), includes: { name: "constructor", sortText: completion.SortText.GlobalsOrKeywords }, isNewIdentifierLocation: true, }); verify.completions({ - marker: negativeCases, + marker: generateRange(19, 20), exact: [], isNewIdentifierLocation: true, }); From 70adf96ed0b9445ec0468ff038588247f87a7c81 Mon Sep 17 00:00:00 2001 From: z0gSh1u Date: Sun, 25 Apr 2021 00:39:00 +0800 Subject: [PATCH 6/7] Make the fix more reliable. --- src/services/completions.ts | 20 ++++-- ...structorKeywordAfterPropertyDeclaration.ts | 62 ++++++++----------- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 9bc4dd9aca8c9..a23f732da157a 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2370,19 +2370,29 @@ namespace ts.Completions { return isPropertyDeclaration(contextToken.parent); } - // If `constructor` is not present, but we request a completion manually... + // If `constructor` is totally not present, but we request a completion manually at a space... if (contextToken === previousToken && isPreviousPropertyDeclarationTerminated(contextToken, position)) { return false; // Don't block completions. } - // If the previous property declaration is terminated according to newline or semicolon... - else if (isPreviousPropertyDeclarationTerminated(contextToken, previousToken.end) - // And we are inside a class declaration and typing `constructor` after property declaration... + + const ancestorPropertyDeclaraion = getAncestor(contextToken.parent, SyntaxKind.PropertyDeclaration); + // If we are inside a class declaration and typing `constructor` after property declaration... + if (ancestorPropertyDeclaraion && contextToken !== previousToken && isClassLike(previousToken.parent.parent) - && isPropertyDeclaration(contextToken.parent) // And the cursor is at the token... && position <= previousToken.end) { + // If we are sure that the previous property declaration is terminated according to newline or semicolon... + if (isPreviousPropertyDeclarationTerminated(contextToken, previousToken.end)) { return false; // Don't block completions. + } + else if (contextToken.kind !== SyntaxKind.EqualsToken + // Should not block: `class C { blah = c/**/ }` + // But should block: `class C { blah = somewhat c/**/ }` and `class C { blah: SomeType c/**/ }` + && (isInitializedProperty(ancestorPropertyDeclaraion as PropertyDeclaration) + || hasType(ancestorPropertyDeclaraion))) { + return true; + } } return isDeclarationName(contextToken) diff --git a/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts index 1ef5dcf8c7427..922da3c3cefb3 100644 --- a/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts +++ b/tests/cases/fourslash/completionConstructorKeywordAfterPropertyDeclaration.ts @@ -1,6 +1,5 @@ /// -//// const GlobalWhat = 123 //// // situations that `constructor` is partly present //// class A { //// blah; con/*1*/ @@ -10,85 +9,76 @@ //// con/*2*/ //// } //// class C { -//// blah; +//// blah: number //// con/*3*/ //// } //// class D { -//// blah: number +//// blah = 123 //// con/*4*/ //// } //// class E { -//// blah: number; con/*5*/ +//// blah = [123] +//// con/*5*/ //// } //// class F { -//// blah = 123 +//// blah = {key: 123} //// con/*6*/ //// } +//// // situations that `constructor` is fully present //// class G { -//// blah = 123; con/*7*/ +//// blah; constructor/*7*/ //// } -//// // situations that `constructor` is fully present //// class H { //// blah //// constructor/*8*/ //// } //// class I { -//// blah; +//// blah: number //// constructor/*9*/ //// } //// class J { -//// blah: number +//// blah = 123 //// constructor/*10*/ //// } //// class K { -//// blah: number; +//// blah = [123] //// constructor/*11*/ //// } //// class L { -//// blah = 123 +//// blah = {key: 123} //// constructor/*12*/ //// } +//// // situations that `constructor` isn't present, but we should offer it //// class M { -//// blah = 123; -//// constructor/*13*/ +//// blah; /*13*/ //// } //// class N { -//// blah = [123] -//// constructor/*14*/ +//// blah +//// /*14*/ //// } +//// // situations that `constructor` should not be suggested //// class O { -//// blah = {key: 1} -//// constructor/*15*/ -//// } -//// function return1() { -//// return 1 +//// blah /*15*/ //// } //// class P { -//// blah = return1() -//// constructor/*16*/ +//// blah con/*16*/ //// } -//// // situations that `constructor` isn't present //// class Q { -//// blah; /*17*/ +//// blah: number con/*17*/ //// } //// class R { -//// blah -//// /*18*/ -//// } -//// // situations that `constructor` should not be suggested -//// class R { -//// blah /*19*/ +//// blah = 123 con/*18*/ //// } //// class S { -//// blah con/*20*/ +//// blah = {key: 123} con/*19*/ //// } -//// // situations that `constructor` should not be suggested but +//// type SomeType = number //// class T { -//// blah: number con/*21*/ +//// blah: SomeType con/*20*/ //// } //// const SomeValue = 123 //// class U { -//// blah = SomeValue con/*22*/ +//// blah = SomeValue con/*21*/ //// } function generateRange(l: number, r: number) { @@ -96,13 +86,13 @@ function generateRange(l: number, r: number) { } verify.completions({ - marker: generateRange(1, 18), + marker: generateRange(1, 14), includes: { name: "constructor", sortText: completion.SortText.GlobalsOrKeywords }, isNewIdentifierLocation: true, }); verify.completions({ - marker: generateRange(19, 20), + marker: generateRange(15, 21), exact: [], isNewIdentifierLocation: true, }); From 939285c8751fa39328b48ddb9ee807281790a629 Mon Sep 17 00:00:00 2001 From: z0gSh1u Date: Tue, 27 Apr 2021 13:08:15 +0800 Subject: [PATCH 7/7] Narrowing the fix. --- src/services/completions.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index a23f732da157a..6e3a60b4706fc 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2370,8 +2370,10 @@ namespace ts.Completions { return isPropertyDeclaration(contextToken.parent); } - // If `constructor` is totally not present, but we request a completion manually at a space... - if (contextToken === previousToken && isPreviousPropertyDeclarationTerminated(contextToken, position)) { + // If we are inside a class declaration, and `constructor` is totally not present, + // but we request a completion manually at a whitespace... + const ancestorClassLike = findAncestor(contextToken.parent, isClassLike); + if (ancestorClassLike && contextToken === previousToken && isPreviousPropertyDeclarationTerminated(contextToken, position)) { return false; // Don't block completions. } @@ -2862,7 +2864,9 @@ namespace ts.Completions { if (!contextToken) return undefined; + // class C { blah; constructor/**/ } and so on if (location.kind === SyntaxKind.ConstructorKeyword + // class C { blah \n constructor/**/ } || (isIdentifier(contextToken) && isPropertyDeclaration(contextToken.parent) && isClassLike(location))) { return findAncestor(contextToken, isClassLike) as ObjectTypeDeclaration; }