diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 7fdbef74ad7..3e01504b484 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -694,7 +694,7 @@ namespace FourSlash { public verifyCompletionListItemsCountIsGreaterThan(count: number, negative: boolean) { const completions = this.getCompletionListAtCaret(); - const itemsCount = completions.entries.length; + const itemsCount = completions ? completions.entries.length : 0; if (negative) { if (itemsCount > count) { @@ -3521,6 +3521,12 @@ namespace FourSlashInterface { "constructor", "async" ]; + public allowedConstructorParameterKeywords = [ + "public", + "private", + "protected", + "readonly", + ]; constructor(protected state: FourSlash.TestState, private negative = false) { if (!negative) { @@ -3563,6 +3569,12 @@ namespace FourSlashInterface { } } + public completionListContainsConstructorParameterKeywords() { + for (const keyword of this.allowedConstructorParameterKeywords) { + this.completionListContains(keyword, keyword, /*documentation*/ undefined, "keyword"); + } + } + public completionListIsGlobal(expected: boolean) { this.state.verifyCompletionListIsGlobal(expected); } diff --git a/src/services/completions.ts b/src/services/completions.ts index 6f1f3fae1ea..8492c2d8f40 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4,6 +4,12 @@ namespace ts.Completions { export type Log = (message: string) => void; + const enum KeywordCompletionFilters { + None, + ClassElementKeywords, // Keywords at class keyword + ConstructorParameterKeywords, // Keywords at constructor parameter + } + export function getCompletionsAtPosition(host: LanguageServiceHost, typeChecker: TypeChecker, log: Log, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number): CompletionInfo | undefined { if (isInReferenceComment(sourceFile, position)) { return PathCompletions.getTripleSlashReferenceCompletion(sourceFile, position, compilerOptions, host); @@ -18,7 +24,7 @@ namespace ts.Completions { return undefined; } - const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, request, hasFilteredClassMemberKeywords } = completionData; + const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, request, keywordFilters } = completionData; if (sourceFile.languageVariant === LanguageVariant.JSX && location && location.parent && location.parent.kind === SyntaxKind.JsxClosingElement) { @@ -54,21 +60,20 @@ namespace ts.Completions { addRange(entries, getJavaScriptCompletionEntries(sourceFile, location.pos, uniqueNames, compilerOptions.target)); } else { - if (!symbols || symbols.length === 0) { - if (!hasFilteredClassMemberKeywords) { + if ((!symbols || symbols.length === 0) && keywordFilters === KeywordCompletionFilters.None) { return undefined; - } } getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true, typeChecker, compilerOptions.target, log); } - if (hasFilteredClassMemberKeywords) { - addRange(entries, classMemberKeywordCompletions); - } - // Add keywords if this is not a member completion list - else if (!isMemberCompletion) { - addRange(entries, keywordCompletions); + // TODO add filter for keyword based on type/value/namespace and also location + + // Add all keywords if + // - this is not a member completion list (all the keywords) + // - other filters are enabled in required scenario so add those keywords + if (keywordFilters !== KeywordCompletionFilters.None || !isMemberCompletion) { + addRange(entries, getKeywordCompletions(keywordFilters)); } return { isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation: isNewIdentifierLocation, entries }; @@ -317,7 +322,10 @@ namespace ts.Completions { } // Didn't find a symbol with this name. See if we can find a keyword instead. - const keywordCompletion = forEach(keywordCompletions, c => c.name === entryName); + const keywordCompletion = forEach( + getKeywordCompletions(KeywordCompletionFilters.None), + c => c.name === entryName + ); if (keywordCompletion) { return { name: entryName, @@ -356,7 +364,7 @@ namespace ts.Completions { location: Node; isRightOfDot: boolean; request?: Request; - hasFilteredClassMemberKeywords: boolean; + keywordFilters: KeywordCompletionFilters; } type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag }; @@ -432,7 +440,7 @@ namespace ts.Completions { } if (request) { - return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, request, hasFilteredClassMemberKeywords: false }; + return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, request, keywordFilters: KeywordCompletionFilters.None }; } if (!insideJsDocTagTypeExpression) { @@ -531,7 +539,7 @@ namespace ts.Completions { let isGlobalCompletion = false; let isMemberCompletion: boolean; let isNewIdentifierLocation: boolean; - let hasFilteredClassMemberKeywords = false; + let keywordFilters = KeywordCompletionFilters.None; let symbols: Symbol[] = []; if (isRightOfDot) { @@ -569,7 +577,7 @@ namespace ts.Completions { log("getCompletionData: Semantic work: " + (timestamp() - semanticStart)); - return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, hasFilteredClassMemberKeywords }; + return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, keywordFilters }; type JSDocTagWithTypeExpression = JSDocAugmentsTag | JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag; @@ -664,6 +672,16 @@ namespace ts.Completions { return tryGetImportOrExportClauseCompletionSymbols(namedImportsOrExports); } + if (tryGetConstructorLikeCompletionContainer(contextToken)) { + // no members, only keywords + isMemberCompletion = false; + // Declaring new property/method/accessor + isNewIdentifierLocation = true; + // Has keywords for constructor parameter + keywordFilters = KeywordCompletionFilters.ConstructorParameterKeywords; + return true; + } + if (classLikeContainer = tryGetClassLikeCompletionContainer(contextToken)) { // cursor inside class declaration getGetClassLikeCompletionSymbols(classLikeContainer); @@ -1046,7 +1064,7 @@ namespace ts.Completions { // Declaring new property/method/accessor isNewIdentifierLocation = true; // Has keywords for class elements - hasFilteredClassMemberKeywords = true; + keywordFilters = KeywordCompletionFilters.ClassElementKeywords; const baseTypeNode = getClassExtendsHeritageClauseElement(classLikeDeclaration); const implementsTypeNodes = getClassImplementsHeritageClauseElements(classLikeDeclaration); @@ -1136,6 +1154,16 @@ namespace ts.Completions { return isClassElement(node.parent) && isClassLike(node.parent.parent); } + function isParameterOfConstructorDeclaration(node: Node) { + return isParameter(node) && isConstructorDeclaration(node.parent); + } + + function isConstructorParameterCompletion(node: Node) { + return node.parent && + isParameterOfConstructorDeclaration(node.parent) && + (isConstructorParameterCompletionKeyword(node.kind) || isDeclarationName(node)); + } + /** * Returns the immediate owning class declaration of a context token, * on the condition that one exists and that the context implies completion should be given. @@ -1149,8 +1177,14 @@ namespace ts.Completions { } break; - // class c {getValue(): number; | } + // class c {getValue(): number, | } case SyntaxKind.CommaToken: + if (isClassLike(contextToken.parent)) { + return contextToken.parent; + } + break; + + // class c {getValue(): number; | } case SyntaxKind.SemicolonToken: // class c { method() { } | } case SyntaxKind.CloseBraceToken: @@ -1175,6 +1209,26 @@ namespace ts.Completions { return undefined; } + /** + * Returns the immediate owning class declaration of a context token, + * on the condition that one exists and that the context implies completion should be given. + */ + function tryGetConstructorLikeCompletionContainer(contextToken: Node): ConstructorDeclaration { + if (contextToken) { + switch (contextToken.kind) { + case SyntaxKind.OpenParenToken: + case SyntaxKind.CommaToken: + return isConstructorDeclaration(contextToken.parent) && contextToken.parent; + + default: + if (isConstructorParameterCompletion(contextToken)) { + return contextToken.parent.parent as ConstructorDeclaration; + } + } + } + return undefined; + } + function tryGetContainingJsxElement(contextToken: Node): JsxOpeningLikeElement { if (contextToken) { const parent = contextToken.parent; @@ -1250,11 +1304,14 @@ namespace ts.Completions { containingNodeKind === SyntaxKind.VariableStatement || containingNodeKind === SyntaxKind.EnumDeclaration || // enum a { foo, | isFunctionLikeButNotConstructor(containingNodeKind) || - containingNodeKind === SyntaxKind.ClassDeclaration || // class A= contextToken.pos); case SyntaxKind.DotToken: return containingNodeKind === SyntaxKind.ArrayBindingPattern; // var [.| @@ -1298,7 +1355,7 @@ namespace ts.Completions { case SyntaxKind.PublicKeyword: case SyntaxKind.PrivateKeyword: case SyntaxKind.ProtectedKeyword: - return containingNodeKind === SyntaxKind.Parameter; + return containingNodeKind === SyntaxKind.Parameter && !isConstructorDeclaration(contextToken.parent.parent); case SyntaxKind.AsKeyword: return containingNodeKind === SyntaxKind.ImportSpecifier || @@ -1331,6 +1388,18 @@ namespace ts.Completions { return false; } + if (isConstructorParameterCompletion(contextToken)) { + // constructor parameter completion is available only if + // - its modifier of the constructor parameter or + // - its name of the parameter and not being edited + // eg. constructor(a |<- this shouldnt show completion + if (!isIdentifier(contextToken) || + isConstructorParameterCompletionKeywordText(contextToken.getText()) || + isCurrentlyEditingNode(contextToken)) { + return false; + } + } + // Previous token may have been a keyword that was converted to an identifier. switch (contextToken.getText()) { case "abstract": @@ -1351,7 +1420,7 @@ namespace ts.Completions { return true; } - return false; + return isDeclarationName(contextToken) && !isJsxAttribute(contextToken.parent); } function isFunctionLikeButNotConstructor(kind: SyntaxKind) { @@ -1574,14 +1643,45 @@ namespace ts.Completions { } // A cache of completion entries for keywords, these do not change between sessions - const keywordCompletions: CompletionEntry[] = []; - for (let i = SyntaxKind.FirstKeyword; i <= SyntaxKind.LastKeyword; i++) { - keywordCompletions.push({ - name: tokenToString(i), - kind: ScriptElementKind.keyword, - kindModifiers: ScriptElementKindModifier.none, - sortText: "0" - }); + const _keywordCompletions: CompletionEntry[][] = []; + function getKeywordCompletions(keywordFilter: KeywordCompletionFilters): CompletionEntry[] { + const completions = _keywordCompletions[keywordFilter]; + if (completions) { + return completions; + } + return _keywordCompletions[keywordFilter] = generateKeywordCompletions(keywordFilter); + + type FilterKeywordCompletions = (entryName: string) => boolean; + function generateKeywordCompletions(keywordFilter: KeywordCompletionFilters) { + switch (keywordFilter) { + case KeywordCompletionFilters.None: + return getAllKeywordCompletions(); + case KeywordCompletionFilters.ClassElementKeywords: + return getFilteredKeywordCompletions(isClassMemberCompletionKeywordText); + case KeywordCompletionFilters.ConstructorParameterKeywords: + return getFilteredKeywordCompletions(isConstructorParameterCompletionKeywordText); + } + } + + function getAllKeywordCompletions() { + const allKeywordsCompletions: CompletionEntry[] = []; + for (let i = SyntaxKind.FirstKeyword; i <= SyntaxKind.LastKeyword; i++) { + allKeywordsCompletions.push({ + name: tokenToString(i), + kind: ScriptElementKind.keyword, + kindModifiers: ScriptElementKindModifier.none, + sortText: "0" + }); + } + return allKeywordsCompletions; + } + + function getFilteredKeywordCompletions(filterFn: FilterKeywordCompletions) { + return filter( + getKeywordCompletions(KeywordCompletionFilters.None), + entry => filterFn(entry.name) + ); + } } function isClassMemberCompletionKeyword(kind: SyntaxKind) { @@ -1604,8 +1704,19 @@ namespace ts.Completions { return isClassMemberCompletionKeyword(stringToToken(text)); } - const classMemberKeywordCompletions = filter(keywordCompletions, entry => - isClassMemberCompletionKeywordText(entry.name)); + function isConstructorParameterCompletionKeyword(kind: SyntaxKind) { + switch (kind) { + case SyntaxKind.PublicKeyword: + case SyntaxKind.PrivateKeyword: + case SyntaxKind.ProtectedKeyword: + case SyntaxKind.ReadonlyKeyword: + return true; + } + } + + function isConstructorParameterCompletionKeywordText(text: string) { + return isConstructorParameterCompletionKeyword(stringToToken(text)); + } function isEqualityExpression(node: Node): node is BinaryExpression { return isBinaryExpression(node) && isEqualityOperatorKind(node.operatorToken.kind); diff --git a/tests/cases/fourslash/completionEntryForClassMembers.ts b/tests/cases/fourslash/completionEntryForClassMembers.ts index 527611b85bf..8945245a286 100644 --- a/tests/cases/fourslash/completionEntryForClassMembers.ts +++ b/tests/cases/fourslash/completionEntryForClassMembers.ts @@ -112,6 +112,11 @@ ////class N extends B { //// async /*classThatHasWrittenAsyncKeyword*/ ////} +////class O extends B { +//// constructor(public a) { +//// }, +//// /*classElementAfterConstructorSeparatedByComma*/ +////} const allowedKeywordCount = verify.allowedClassElementKeywords.length; type CompletionInfo = [string, string]; @@ -220,7 +225,8 @@ const classInstanceElementLocations = [ "classThatStartedWritingIdentifierOfGetAccessor", "classThatStartedWritingIdentifierOfSetAccessor", "classThatStartedWritingIdentifierAfterModifier", - "classThatHasWrittenAsyncKeyword" + "classThatHasWrittenAsyncKeyword", + "classElementAfterConstructorSeparatedByComma" ]; verifyClassElementLocations(instanceMemberInfo, classInstanceElementLocations); diff --git a/tests/cases/fourslash/completionListAfterPropertyName.ts b/tests/cases/fourslash/completionListAfterPropertyName.ts new file mode 100644 index 00000000000..c83b16c9741 --- /dev/null +++ b/tests/cases/fourslash/completionListAfterPropertyName.ts @@ -0,0 +1,88 @@ +/// + +// @Filename: a.ts +////class Test1 { +//// public some /*afterPropertyName*/ +////} + +// @Filename: b.ts +////class Test2 { +//// public some(/*inMethodParameter*/ +////} + +// @Filename: c.ts +////class Test3 { +//// public some(a/*atMethodParameter*/ +////} + +// @Filename: d.ts +////class Test4 { +//// public some(a /*afterMethodParameter*/ +////} + +// @Filename: e.ts +////class Test5 { +//// public some(a /*afterMethodParameterBeforeComma*/, +////} + +// @Filename: f.ts +////class Test6 { +//// public some(a, /*afterMethodParameterComma*/ +////} + +// @Filename: g.ts +////class Test7 { +//// constructor(/*inConstructorParameter*/ +////} + +// @Filename: h.ts +////class Test8 { +//// constructor(public /*inConstructorParameterAfterModifier*/ +////} + +// @Filename: i.ts +////class Test9 { +//// constructor(a/*atConstructorParameter*/ +////} + +// @Filename: j.ts +////class Test10 { +//// constructor(public/*atConstructorParameterModifier*/ +////} + +// @Filename: k.ts +////class Test11 { +//// constructor(public a/*atConstructorParameterAfterModifier*/ +////} + +// @Filename: l.ts +////class Test12 { +//// constructor(a /*afterConstructorParameter*/ +////} + +// @Filename: m.ts +////class Test13 { +//// constructor(a /*afterConstructorParameterBeforeComma*/, +////} + +// @Filename: n.ts +////class Test14 { +//// constructor(public a, /*afterConstructorParameterComma*/ +////} + +for (const marker of ["afterPropertyName", + "inMethodParameter", "atMethodParameter", "afterMethodParameter", + "afterMethodParameterBeforeComma", "afterMethodParameterComma", + "afterConstructorParameter", "afterConstructorParameterBeforeComma"]) { + + goTo.marker(marker); + verify.completionListIsEmpty(); +} + +for (const marker of ["inConstructorParameter", "inConstructorParameterAfterModifier", + "atConstructorParameter", "atConstructorParameterModifier", "atConstructorParameterAfterModifier", + "afterConstructorParameterComma"]) { + goTo.marker(marker); + verify.completionListContainsConstructorParameterKeywords(); + verify.completionListCount(verify.allowedConstructorParameterKeywords.length); +} \ No newline at end of file diff --git a/tests/cases/fourslash/completionListAtIdentifierDefinitionLocations_parameters.ts b/tests/cases/fourslash/completionListAtIdentifierDefinitionLocations_parameters.ts index 474859129db..c59b08dd70a 100644 --- a/tests/cases/fourslash/completionListAtIdentifierDefinitionLocations_parameters.ts +++ b/tests/cases/fourslash/completionListAtIdentifierDefinitionLocations_parameters.ts @@ -22,4 +22,18 @@ ////class bar10{ constructor(...a/*constructorParamter6*/ -goTo.eachMarker(() => verify.completionListIsEmpty()); +for (let i = 1; i <= 4; i++) { + goTo.marker("parameterName" + i.toString()); + verify.completionListIsEmpty(); +} + +for (let i = 1; i <= 4; i++) { + goTo.marker("constructorParamter" + i.toString()); + verify.completionListContainsConstructorParameterKeywords(); + verify.completionListCount(verify.allowedConstructorParameterKeywords.length); +} + +for (let i = 5; i <= 6; i++) { + goTo.marker("constructorParamter" + i.toString()); + verify.completionListIsEmpty(); +} diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 58f14353c66..767701a2af6 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -134,12 +134,14 @@ declare namespace FourSlashInterface { private negative; not: verifyNegatable; allowedClassElementKeywords: string[]; + allowedConstructorParameterKeywords: string[]; constructor(negative?: boolean); completionListCount(expectedCount: number): void; completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number): void; completionListItemsCountIsGreaterThan(count: number): void; completionListIsEmpty(): void; completionListContainsClassElementKeywords(): void; + completionListContainsConstructorParameterKeywords(): void; completionListAllowsNewIdentifier(): void; signatureHelpPresent(): void; errorExistsBetweenMarkers(startMarker: string, endMarker: string): void; diff --git a/tests/cases/fourslash/globalCompletionListInsideObjectLiterals.ts b/tests/cases/fourslash/globalCompletionListInsideObjectLiterals.ts index af7bc075f8c..33f8534344e 100644 --- a/tests/cases/fourslash/globalCompletionListInsideObjectLiterals.ts +++ b/tests/cases/fourslash/globalCompletionListInsideObjectLiterals.ts @@ -39,7 +39,7 @@ goTo.marker("3"); VerifyGlobalCompletionList(); goTo.marker("4"); -VerifyGlobalCompletionList(); +verify.completionListIsEmpty(); // Literal member completion after member name with empty member expression. goTo.marker("5");