From 53bc01840a3e648bfa123707f1bef2a90e68141d Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 7 Dec 2017 08:31:00 -0800 Subject: [PATCH 1/4] Support completions that require changing from dot to bracket access --- src/compiler/diagnosticMessages.json | 4 + src/services/completions.ts | 369 ++++++++++-------- .../completionListInvalidMemberNames.ts | 12 +- .../completionListInvalidMemberNames2.ts | 21 +- .../completionListInvalidMemberNames3.ts | 71 ---- ...validMemberNames_withExistingIdentifier.ts | 11 + ...entifiers-should-not-show-in-completion.ts | 13 - ...jsdocTypedefTagTypeExpressionCompletion.ts | 2 +- 8 files changed, 249 insertions(+), 254 deletions(-) delete mode 100644 tests/cases/fourslash/completionListInvalidMemberNames3.ts create mode 100644 tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts delete mode 100644 tests/cases/fourslash/completion_enum-members-with-invalid-identifiers-should-not-show-in-completion.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index fab7583d90cb3..d7c12fd6395d3 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3890,5 +3890,9 @@ "Install '{0}'": { "category": "Message", "code": 95014 + }, + "Use bracket notation instead of dot notation": { + "category": "Message", + "code": 95015 } } diff --git a/src/services/completions.ts b/src/services/completions.ts index d893f4852e4f8..dc303be564729 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -50,39 +50,48 @@ namespace ts.Completions { return undefined; } - const { symbols, isGlobalCompletion, isMemberCompletion, allowStringLiteral, isNewIdentifierLocation, location, request, keywordFilters, symbolToOriginInfoMap, recommendedCompletion } = completionData; + switch (completionData.kind) { + case CompletionDataKind.Data: + return completionInfoFromData(sourceFile, typeChecker, compilerOptions, log, completionData); + case CompletionDataKind.JsDocTagName: + // If the current position is a jsDoc tag name, only tag names should be provided for completion + return jsdocCompletionInfo(JsDoc.getJSDocTagNameCompletions()); + case CompletionDataKind.JsDocTag: + // If the current position is a jsDoc tag, only tags should be provided for completion + return jsdocCompletionInfo(JsDoc.getJSDocTagCompletions()); + case CompletionDataKind.JsDocParameterName: + return jsdocCompletionInfo(JsDoc.getJSDocParameterNameCompletions(completionData.tag)); + default: + throw Debug.assertNever(completionData); + } + } - if (sourceFile.languageVariant === LanguageVariant.JSX && - location && location.parent && location.parent.kind === SyntaxKind.JsxClosingElement) { + function jsdocCompletionInfo(entries: CompletionEntry[]): CompletionInfo { + return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries }; + } + + function completionInfoFromData(sourceFile: SourceFile, typeChecker: TypeChecker, compilerOptions: CompilerOptions, log: Log, completionData: CompletionData): CompletionInfo { + const { symbols, completionKind, isNewIdentifierLocation, location, keywordFilters, symbolToOriginInfoMap, recommendedCompletion } = completionData; + + if (sourceFile.languageVariant === LanguageVariant.JSX && location && location.parent && isJsxClosingElement(location.parent)) { // In the TypeScript JSX element, if such element is not defined. When users query for completion at closing tag, // instead of simply giving unknown value, the completion will return the tag-name of an associated opening-element. // For example: // var x =
completion list at "1" will contain "div" with type any - const tagName = (location.parent.parent).openingElement.tagName; + const tagName = location.parent.parent.openingElement.tagName; return { isGlobalCompletion: false, isMemberCompletion: true, isNewIdentifierLocation: false, entries: [{ - name: (tagName).getFullText(), + name: tagName.getFullText(), kind: ScriptElementKind.classElement, kindModifiers: undefined, sortText: "0", }]}; } - if (request) { - const entries = request.kind === "JsDocTagName" - // If the current position is a jsDoc tag name, only tag names should be provided for completion - ? JsDoc.getJSDocTagNameCompletions() - : request.kind === "JsDocTag" - // If the current position is a jsDoc tag, only tags should be provided for completion - ? JsDoc.getJSDocTagCompletions() - : JsDoc.getJSDocParameterNameCompletions(request.tag); - return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries }; - } - const entries: CompletionEntry[] = []; if (isSourceFileJavaScript(sourceFile)) { - const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true, typeChecker, compilerOptions.target, log, allowStringLiteral, recommendedCompletion, symbolToOriginInfoMap); + const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries, location, typeChecker, compilerOptions.target, log, completionKind, recommendedCompletion, symbolToOriginInfoMap); getJavaScriptCompletionEntries(sourceFile, location.pos, uniqueNames, compilerOptions.target, entries); } else { @@ -90,7 +99,7 @@ namespace ts.Completions { return undefined; } - getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true, typeChecker, compilerOptions.target, log, allowStringLiteral, recommendedCompletion, symbolToOriginInfoMap); + getCompletionEntriesFromSymbols(symbols, entries, location, typeChecker, compilerOptions.target, log, completionKind, recommendedCompletion, symbolToOriginInfoMap); } // TODO add filter for keyword based on type/value/namespace and also location @@ -98,17 +107,29 @@ namespace ts.Completions { // 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 + const isMemberCompletion = isMemberCompletionKind(completionKind); if (keywordFilters !== KeywordCompletionFilters.None || !isMemberCompletion) { addRange(entries, getKeywordCompletions(keywordFilters)); } - return { isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, entries }; + return { isGlobalCompletion: completionKind === CompletionKind.Global, isMemberCompletion, isNewIdentifierLocation, entries }; + } + + function isMemberCompletionKind(kind: CompletionKind): boolean { + switch (kind) { + case CompletionKind.ObjectPropertyDeclaration: + case CompletionKind.MemberLike: + case CompletionKind.PropertyAccess: + return true; + default: + return false; + } } function getJavaScriptCompletionEntries( sourceFile: SourceFile, position: number, - uniqueNames: Map<{}>, + uniqueNames: Map, target: ScriptTarget, entries: Push): void { getNameTable(sourceFile).forEach((pos, name) => { @@ -117,16 +138,9 @@ namespace ts.Completions { return; } const realName = unescapeLeadingUnderscores(name); - - if (uniqueNames.has(realName) || isStringANonContextualKeyword(realName)) { - return; - } - - uniqueNames.set(realName, true); - const displayName = getCompletionEntryDisplayName(realName, target, /*performCharacterChecks*/ true, /*allowStringLiteral*/ false); - if (displayName) { + if (addToSeen(uniqueNames, realName) && isIdentifierText(realName, target) && !isStringANonContextualKeyword(realName)) { entries.push({ - name: displayName, + name: realName, kind: ScriptElementKind.warning, kindModifiers: "", sortText: "1" @@ -138,20 +152,17 @@ namespace ts.Completions { function createCompletionEntry( symbol: Symbol, location: Node, - performCharacterChecks: boolean, typeChecker: TypeChecker, target: ScriptTarget, - allowStringLiteral: boolean, + kind: CompletionKind, origin: SymbolOriginInfo | undefined, recommendedCompletion: Symbol | undefined, ): CompletionEntry | undefined { - // Try to get a valid display name for this symbol, if we could not find one, then ignore it. - // We would like to only show things that can be added after a dot, so for instance numeric properties can - // not be accessed with a dot (a.1 <- invalid) - const displayName = getCompletionEntryDisplayNameForSymbol(symbol, target, performCharacterChecks, allowStringLiteral, origin); - if (!displayName) { + const info = getCompletionEntryDisplayNameForSymbol(symbol, target, origin, kind); + if (!info) { return undefined; } + const { name, needsConvertPropertyAccess } = info; // TODO(drosen): Right now we just permit *all* semantic meanings when calling // 'getSymbolKind' which is permissible given that it is backwards compatible; but @@ -162,12 +173,12 @@ namespace ts.Completions { // Use a 'sortText' of 0' so that all symbol completion entries come before any other // entries (like JavaScript identifier entries). return { - name: displayName, + name, kind: SymbolDisplay.getSymbolKind(typeChecker, symbol, location), kindModifiers: SymbolDisplay.getSymbolModifiers(symbol), sortText: "0", source: getSourceFromOrigin(origin), - hasAction: trueOrUndefined(origin !== undefined), + hasAction: trueOrUndefined(needsConvertPropertyAccess || origin !== undefined), isRecommended: trueOrUndefined(isRecommendedCompletionMatch(symbol, recommendedCompletion, typeChecker)), }; } @@ -189,11 +200,10 @@ namespace ts.Completions { symbols: ReadonlyArray, entries: Push, location: Node, - performCharacterChecks: boolean, typeChecker: TypeChecker, target: ScriptTarget, log: Log, - allowStringLiteral: boolean, + kind: CompletionKind, recommendedCompletion?: Symbol, symbolToOriginInfoMap?: SymbolOriginInfoMap, ): Map { @@ -206,7 +216,7 @@ namespace ts.Completions { if (symbols) { for (const symbol of symbols) { const origin = symbolToOriginInfoMap ? symbolToOriginInfoMap[getSymbolId(symbol)] : undefined; - const entry = createCompletionEntry(symbol, location, performCharacterChecks, typeChecker, target, allowStringLiteral, origin, recommendedCompletion); + const entry = createCompletionEntry(symbol, location, typeChecker, target, kind, origin, recommendedCompletion); if (!entry) { continue; } @@ -323,7 +333,7 @@ namespace ts.Completions { const type = typeChecker.getContextualType((element.parent)); const entries: CompletionEntry[] = []; if (type) { - getCompletionEntriesFromSymbols(type.getApparentProperties(), entries, element, /*performCharacterChecks*/ false, typeChecker, target, log, /*allowStringLiteral*/ true); + getCompletionEntriesFromSymbols(type.getApparentProperties(), entries, element, typeChecker, target, log, CompletionKind.String); if (entries.length) { return { isGlobalCompletion: false, isMemberCompletion: true, isNewIdentifierLocation: true, entries }; } @@ -352,7 +362,7 @@ namespace ts.Completions { const type = typeChecker.getTypeAtLocation(node.expression); const entries: CompletionEntry[] = []; if (type) { - getCompletionEntriesFromSymbols(type.getApparentProperties(), entries, node, /*performCharacterChecks*/ false, typeChecker, target, log, /*allowStringLiteral*/ true); + getCompletionEntriesFromSymbols(type.getApparentProperties(), entries, node, typeChecker, target, log, CompletionKind.String); if (entries.length) { return { isGlobalCompletion: false, isMemberCompletion: true, isNewIdentifierLocation: true, entries }; } @@ -423,6 +433,13 @@ namespace ts.Completions { } } + interface SymbolCompletion { + type: "symbol"; + symbol: Symbol; + propertyAccessToConvert: PropertyAccessExpression | undefined; + location: Node; + symbolToOriginInfoMap: SymbolOriginInfoMap; + } function getSymbolCompletionFromEntryId( typeChecker: TypeChecker, log: (message: string) => void, @@ -431,27 +448,26 @@ namespace ts.Completions { position: number, { name, source }: CompletionEntryIdentifier, allSourceFiles: ReadonlyArray, - ): { type: "symbol", symbol: Symbol, location: Node, symbolToOriginInfoMap: SymbolOriginInfoMap } | { type: "request", request: Request } | { type: "none" } { + ): SymbolCompletion | { type: "request", request: Request } | { type: "none" } { const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true }, compilerOptions.target); if (!completionData) { return { type: "none" }; } - - const { symbols, location, allowStringLiteral, symbolToOriginInfoMap, request } = completionData; - if (request) { - return { type: "request", request }; + if (completionData.kind !== CompletionDataKind.Data) { + return { type: "request", request: completionData }; } + const { symbols, location, completionKind, propertyAccessToConvert, symbolToOriginInfoMap } = completionData; + // Find the symbol with the matching entry name. // We don't need to perform character checks here because we're only comparing the // name against 'entryName' (which is known to be good), not building a new // completion entry. - const symbol = find(symbols, s => { - const origin = symbolToOriginInfoMap[getSymbolId(s)]; - return getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral, origin) === name - && getSourceFromOrigin(origin) === source; - }); - return symbol ? { type: "symbol", symbol, location, symbolToOriginInfoMap } : { type: "none" }; + return firstDefined(symbols, (symbol): SymbolCompletion => { // TODO: Shouldn't need return type annotation (GH#12632) + const origin = symbolToOriginInfoMap[getSymbolId(symbol)]; + const info = getCompletionEntryDisplayNameForSymbol(symbol, compilerOptions.target, origin, completionKind); + return info && info.name === name && getSourceFromOrigin(origin) === source ? { type: "symbol" as "symbol", symbol, propertyAccessToConvert: info.needsConvertPropertyAccess ? propertyAccessToConvert : undefined, location, symbolToOriginInfoMap } : undefined; + }) || { type: "none" }; } function getSymbolName(symbol: Symbol, origin: SymbolOriginInfo | undefined, target: ScriptTarget): string { @@ -485,19 +501,19 @@ namespace ts.Completions { case "request": { const { request } = symbolCompletion; switch (request.kind) { - case "JsDocTagName": + case CompletionDataKind.JsDocTagName: return JsDoc.getJSDocTagNameCompletionDetails(name); - case "JsDocTag": + case CompletionDataKind.JsDocTag: return JsDoc.getJSDocTagCompletionDetails(name); - case "JsDocParameterName": + case CompletionDataKind.JsDocParameterName: return JsDoc.getJSDocParameterNameCompletionDetails(name); default: return Debug.assertNever(request); } } case "symbol": { - const { symbol, location, symbolToOriginInfoMap } = symbolCompletion; - const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles); + const { symbol, location, propertyAccessToConvert, symbolToOriginInfoMap } = symbolCompletion; + const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles, propertyAccessToConvert); const kindModifiers = SymbolDisplay.getSymbolModifiers(symbol); const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); return { name, kindModifiers, kind: symbolKind, displayParts, documentation, tags, codeActions, source: sourceDisplay }; @@ -521,6 +537,10 @@ namespace ts.Completions { } } + interface CodeActionsAndSourceDisplay { + readonly codeActions: CodeAction[] | undefined; + readonly sourceDisplay: SymbolDisplayPart[] | undefined; + } function getCompletionEntryCodeActionsAndSourceDisplay( symbolToOriginInfoMap: SymbolOriginInfoMap, symbol: Symbol, @@ -532,12 +552,44 @@ namespace ts.Completions { formatContext: formatting.FormatContext, getCanonicalFileName: GetCanonicalFileName, allSourceFiles: ReadonlyArray, - ): { codeActions: CodeAction[] | undefined, sourceDisplay: SymbolDisplayPart[] | undefined } { + propertyAccessToConvert: PropertyAccessExpression | undefined, + ): CodeActionsAndSourceDisplay { const symbolOriginInfo = symbolToOriginInfoMap[getSymbolId(symbol)]; - if (!symbolOriginInfo) { - return { codeActions: undefined, sourceDisplay: undefined }; - } + Debug.assert(!(symbolOriginInfo && propertyAccessToConvert)); + return propertyAccessToConvert + ? { codeActions: convertDotToBracketAccess(propertyAccessToConvert), sourceDisplay: undefined } + : symbolOriginInfo + ? getCodeActionsAndSourceDisplayForImport(symbolOriginInfo, symbol, program, checker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles) + : { codeActions: undefined, sourceDisplay: undefined }; + } + function convertDotToBracketAccess(prop: PropertyAccessExpression): CodeAction[] { + const sourceFile = prop.getSourceFile(); + const changes: ts.FileTextChanges[] = [ + { + fileName: sourceFile.fileName, + textChanges: [ + { span: { start: ts.getFirstChildOfKind(prop, sourceFile, SyntaxKind.DotToken)!.getStart(), length: 1 }, newText: '["', }, + { span: { start: prop.name.getEnd(), length: 0 }, newText: '"]', }, + ], + }, + ]; + const description = formatMessage(/*dummy*/ undefined, Diagnostics.Use_bracket_notation_instead_of_dot_notation); + return [{ changes, description }]; + } + + function getCodeActionsAndSourceDisplayForImport( + symbolOriginInfo: SymbolOriginInfo, + symbol: Symbol, + program: Program, + checker: TypeChecker, + host: LanguageServiceHost, + compilerOptions: CompilerOptions, + sourceFile: SourceFile, + formatContext: formatting.FormatContext, + getCanonicalFileName: GetCanonicalFileName, + allSourceFiles: ReadonlyArray + ): CodeActionsAndSourceDisplay { const { moduleSymbol, isDefaultExport } = symbolOriginInfo; const exportedSymbol = skipAlias(symbol.exportSymbol || symbol, checker); const moduleSymbols = getAllReExportingModules(exportedSymbol, checker, allSourceFiles); @@ -585,20 +637,28 @@ namespace ts.Completions { return completion.type === "symbol" ? completion.symbol : undefined; } + const enum CompletionDataKind { Data, JsDocTagName, JsDocTag, JsDocParameterName } interface CompletionData { - symbols: ReadonlyArray; - isGlobalCompletion: boolean; - isMemberCompletion: boolean; - allowStringLiteral: boolean; - isNewIdentifierLocation: boolean; - location: Node | undefined; - isRightOfDot: boolean; - request?: Request; - keywordFilters: KeywordCompletionFilters; - symbolToOriginInfoMap: SymbolOriginInfoMap; - recommendedCompletion: Symbol | undefined; + readonly kind: CompletionDataKind.Data; + readonly symbols: ReadonlyArray; + readonly completionKind: CompletionKind; + readonly propertyAccessToConvert: PropertyAccessExpression | undefined; + readonly isNewIdentifierLocation: boolean; + readonly location: Node | undefined; + readonly keywordFilters: KeywordCompletionFilters; + readonly symbolToOriginInfoMap: SymbolOriginInfoMap; + readonly recommendedCompletion: Symbol | undefined; + } + type Request = { readonly kind: CompletionDataKind.JsDocTagName | CompletionDataKind.JsDocTag } | { readonly kind: CompletionDataKind.JsDocParameterName, tag: JSDocParameterTag }; + + const enum CompletionKind { + ObjectPropertyDeclaration, + Global, + PropertyAccess, + MemberLike, + String, + None, } - type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag }; function getRecommendedCompletion(currentToken: Node, checker: TypeChecker/*, symbolToOriginInfoMap: SymbolOriginInfoMap*/): Symbol | undefined { const ty = checker.getContextualType(currentToken as Expression); @@ -627,9 +687,7 @@ namespace ts.Completions { allSourceFiles: ReadonlyArray, options: GetCompletionsAtPositionOptions, target: ScriptTarget, - ): CompletionData | undefined { - let request: Request | undefined; - + ): CompletionData | Request | undefined { let start = timestamp(); let currentToken = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); // TODO: GH#15853 // We will check for jsdoc comments with insideComment and getJsDocTagAtPosition. (TODO: that seems rather inefficient to check the same thing so many times.) @@ -647,7 +705,7 @@ namespace ts.Completions { if (sourceFile.text.charCodeAt(position - 1) === CharacterCodes.at) { // The current position is next to the '@' sign, when no tag name being provided yet. // Provide a full list of tag names - request = { kind: "JsDocTagName" }; + return { kind: CompletionDataKind.JsDocTagName }; } else { // When completion is requested without "@", we will have check to make sure that @@ -668,7 +726,7 @@ namespace ts.Completions { // */ const lineStart = getLineStartPositionForPosition(position, sourceFile); if (!(sourceFile.text.substring(lineStart, position).match(/[^\*|\s|(/\*\*)]/))) { - request = { kind: "JsDocTag" }; + return { kind: CompletionDataKind.JsDocTag }; } } } @@ -679,7 +737,7 @@ namespace ts.Completions { const tag = getJsDocTagAtPosition(currentToken, position); if (tag) { if (tag.tagName.pos <= position && position <= tag.tagName.end) { - request = { kind: "JsDocTagName" }; + return { kind: CompletionDataKind.JsDocTagName }; } if (isTagWithTypeExpression(tag) && tag.typeExpression && tag.typeExpression.kind === SyntaxKind.JSDocTypeExpression) { currentToken = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ true); @@ -692,26 +750,10 @@ namespace ts.Completions { } } if (isJSDocParameterTag(tag) && (nodeIsMissing(tag.name) || tag.name.pos <= position && position <= tag.name.end)) { - request = { kind: "JsDocParameterName", tag }; + return { kind: CompletionDataKind.JsDocParameterName, tag }; } } - if (request) { - return { - symbols: emptyArray, - isGlobalCompletion: false, - isMemberCompletion: false, - allowStringLiteral: false, - isNewIdentifierLocation: false, - location: undefined, - isRightOfDot: false, - request, - keywordFilters: KeywordCompletionFilters.None, - symbolToOriginInfoMap: undefined, - recommendedCompletion: undefined, - }; - } - if (!insideJsDocTagTypeExpression) { // Proceed if the current position is in jsDoc tag expression; otherwise it is a normal // comment or the plain text part of a jsDoc comment, so no completion should be available @@ -740,6 +782,7 @@ namespace ts.Completions { // Also determine whether we are trying to complete with members of that node // or attributes of a JSX tag. let node = currentToken; + let propertyAccessToConvert: PropertyAccessExpression | undefined; let isRightOfDot = false; let isRightOfOpenTag = false; let isStartingCloseTag = false; @@ -754,18 +797,19 @@ namespace ts.Completions { let parent = contextToken.parent; if (contextToken.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; + isRightOfDot = true; + switch (parent.kind) { + case SyntaxKind.PropertyAccessExpression: + propertyAccessToConvert = parent as PropertyAccessExpression; + node = propertyAccessToConvert.expression; + break; + case SyntaxKind.QualifiedName: + node = (parent as QualifiedName).left; + break; + default: + // 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 (sourceFile.languageVariant === LanguageVariant.JSX) { @@ -805,10 +849,8 @@ namespace ts.Completions { } const semanticStart = timestamp(); - let isGlobalCompletion = false; - let isMemberCompletion: boolean; - let allowStringLiteral = false; - let isNewIdentifierLocation: boolean; + let completionKind = CompletionKind.None; + let isNewIdentifierLocation = false; let keywordFilters = KeywordCompletionFilters.None; let symbols: Symbol[] = []; const symbolToOriginInfoMap: SymbolOriginInfoMap = []; @@ -824,8 +866,7 @@ namespace ts.Completions { else { symbols = tagSymbols; } - isMemberCompletion = true; - isNewIdentifierLocation = false; + completionKind = CompletionKind.MemberLike; } else if (isStartingCloseTag) { const tagName = (contextToken.parent.parent).openingElement.tagName; @@ -834,8 +875,7 @@ namespace ts.Completions { if (!typeChecker.isUnknownSymbol(tagSymbol)) { symbols = [tagSymbol]; } - isMemberCompletion = true; - isNewIdentifierLocation = false; + completionKind = CompletionKind.MemberLike; } else { // For JavaScript or TypeScript, if we're not after a dot, then just try to get the @@ -849,7 +889,7 @@ namespace ts.Completions { log("getCompletionData: Semantic work: " + (timestamp() - semanticStart)); const recommendedCompletion = getRecommendedCompletion(previousToken, typeChecker); - return { symbols, isGlobalCompletion, isMemberCompletion, allowStringLiteral, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, keywordFilters, symbolToOriginInfoMap, recommendedCompletion }; + return { kind: CompletionDataKind.Data, symbols, completionKind, propertyAccessToConvert, isNewIdentifierLocation, location, keywordFilters, symbolToOriginInfoMap, recommendedCompletion }; type JSDocTagWithTypeExpression = JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag; @@ -866,9 +906,7 @@ namespace ts.Completions { function getTypeScriptMemberSymbols(): void { // Right of dot member completion list - isGlobalCompletion = false; - isMemberCompletion = true; - isNewIdentifierLocation = false; + completionKind = CompletionKind.PropertyAccess; // Since this is qualified name check its a type node location const isTypeLocation = insideJsDocTagTypeExpression || isPartOfTypeNode(node.parent); @@ -945,7 +983,7 @@ namespace ts.Completions { if (tryGetConstructorLikeCompletionContainer(contextToken)) { // no members, only keywords - isMemberCompletion = false; + completionKind = CompletionKind.None; // Declaring new property/method/accessor isNewIdentifierLocation = true; // Has keywords for constructor parameter @@ -967,7 +1005,7 @@ namespace ts.Completions { if (attrsType) { symbols = filterJsxAttributes(typeChecker.getPropertiesOfType(attrsType), (jsxContainer).attributes.properties); - isMemberCompletion = true; + completionKind = CompletionKind.MemberLike; isNewIdentifierLocation = false; return true; } @@ -975,7 +1013,7 @@ namespace ts.Completions { } // Get all entities in the current scope. - isMemberCompletion = false; + completionKind = CompletionKind.None; isNewIdentifierLocation = isNewIdentifierDefinitionLocation(contextToken); if (previousToken !== contextToken) { @@ -1011,13 +1049,8 @@ namespace ts.Completions { position; const scopeNode = getScopeNode(contextToken, adjustedPosition, sourceFile) || sourceFile; - if (scopeNode) { - isGlobalCompletion = - scopeNode.kind === SyntaxKind.SourceFile || - scopeNode.kind === SyntaxKind.TemplateExpression || - scopeNode.kind === SyntaxKind.JsxExpression || - scopeNode.kind === SyntaxKind.Block || // Some blocks aren't statements, but all get global completions - isStatement(scopeNode); + if (isGlobalCompletionScope(scopeNode)) { + completionKind = CompletionKind.Global; } const symbolMeanings = SymbolFlags.Type | SymbolFlags.Value | SymbolFlags.Namespace | SymbolFlags.Alias; @@ -1031,6 +1064,18 @@ namespace ts.Completions { return true; } + function isGlobalCompletionScope(scopeNode: Node): boolean { + switch (scopeNode.kind) { + case SyntaxKind.SourceFile: + case SyntaxKind.TemplateExpression: + case SyntaxKind.JsxExpression: + case SyntaxKind.Block: + return true; + default: + return isStatement(scopeNode); + } + } + function filterGlobalCompletion(symbols: Symbol[]): void { filterMutate(symbols, symbol => { if (!isSourceFile(location)) { @@ -1297,8 +1342,7 @@ namespace ts.Completions { */ function tryGetObjectLikeCompletionSymbols(objectLikeContainer: ObjectLiteralExpression | ObjectBindingPattern): boolean { // We're looking up possible property names from contextual/inferred/declared type. - isMemberCompletion = true; - allowStringLiteral = true; + completionKind = CompletionKind.ObjectPropertyDeclaration; let typeMembers: Symbol[]; let existingMembers: ReadonlyArray; @@ -1376,7 +1420,7 @@ namespace ts.Completions { return false; } - isMemberCompletion = true; + completionKind = CompletionKind.MemberLike; isNewIdentifierLocation = false; const moduleSpecifierSymbol = typeChecker.getSymbolAtLocation(moduleSpecifier); @@ -1396,7 +1440,7 @@ namespace ts.Completions { */ function getGetClassLikeCompletionSymbols(classLikeDeclaration: ClassLikeDeclaration) { // We're looking up possible property names from parent type. - isMemberCompletion = true; + completionKind = CompletionKind.MemberLike; // Declaring new property/method/accessor isNewIdentifierLocation = true; // Has keywords for class elements @@ -1947,12 +1991,16 @@ namespace ts.Completions { } } - /** - * Get the name to be display in completion from a given symbol. - * - * @return undefined if the name is of external module - */ - function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean, allowStringLiteral: boolean, origin: SymbolOriginInfo | undefined): string | undefined { + interface CompletionEntryDisplayNameForSymbol { + readonly name: string; + readonly needsConvertPropertyAccess: boolean; + } + function getCompletionEntryDisplayNameForSymbol( + symbol: Symbol, + target: ScriptTarget, + origin: SymbolOriginInfo | undefined, + kind: CompletionKind, + ): CompletionEntryDisplayNameForSymbol | undefined { const name = getSymbolName(symbol, origin, target); if (!name) return undefined; @@ -1978,24 +2026,23 @@ namespace ts.Completions { } } - return getCompletionEntryDisplayName(name, target, performCharacterChecks, allowStringLiteral); - } - - /** - * Get a displayName from a given for completion list, performing any necessary quotes stripping - * and checking whether the name is valid identifier name. - */ - function getCompletionEntryDisplayName(name: string, target: ScriptTarget, performCharacterChecks: boolean, allowStringLiteral: boolean): string { - // If the user entered name for the symbol was quoted, removing the quotes is not enough, as the name could be an - // invalid identifier name. We need to check if whatever was inside the quotes is actually a valid identifier name. - // e.g "b a" is valid quoted name but when we strip off the quotes, it is invalid. - // We, thus, need to check if whatever was inside the quotes is actually a valid identifier name. - if (performCharacterChecks && !isIdentifierText(name, target)) { - // TODO: GH#18169 - return allowStringLiteral ? JSON.stringify(name) : undefined; + const validIdentiferResult: CompletionEntryDisplayNameForSymbol = { name, needsConvertPropertyAccess: false }; + if (isIdentifierText(name, target)) return validIdentiferResult; + switch (kind) { + case CompletionKind.None: + case CompletionKind.Global: + case CompletionKind.MemberLike: + return undefined; + case CompletionKind.ObjectPropertyDeclaration: + // TODO: GH#18169 + return { name: JSON.stringify(name), needsConvertPropertyAccess: false }; + case CompletionKind.PropertyAccess: + return { name, needsConvertPropertyAccess: true }; + case CompletionKind.String: + return validIdentiferResult; + default: + Debug.assertNever(kind); } - - return name; } // A cache of completion entries for keywords, these do not change between sessions @@ -2008,7 +2055,7 @@ namespace ts.Completions { return _keywordCompletions[keywordFilter] = generateKeywordCompletions(keywordFilter); type FilterKeywordCompletions = (entryName: string) => boolean; - function generateKeywordCompletions(keywordFilter: KeywordCompletionFilters) { + function generateKeywordCompletions(keywordFilter: KeywordCompletionFilters): CompletionEntry[] { switch (keywordFilter) { case KeywordCompletionFilters.None: return getAllKeywordCompletions(); @@ -2016,6 +2063,8 @@ namespace ts.Completions { return getFilteredKeywordCompletions(isClassMemberCompletionKeywordText); case KeywordCompletionFilters.ConstructorParameterKeywords: return getFilteredKeywordCompletions(isConstructorParameterCompletionKeywordText); + default: + Debug.assertNever(keywordFilter); } } diff --git a/tests/cases/fourslash/completionListInvalidMemberNames.ts b/tests/cases/fourslash/completionListInvalidMemberNames.ts index 8e62ba2fb67b9..82fadcaa36577 100644 --- a/tests/cases/fourslash/completionListInvalidMemberNames.ts +++ b/tests/cases/fourslash/completionListInvalidMemberNames.ts @@ -11,8 +11,14 @@ //// "\u0031\u0062": "invalid unicode identifer name (1b)" ////}; //// -////x./*a*/; +////[|x./*a*/;|] ////x["/*b*/"]; -verify.completionsAt("a", ["bar", "break", "any", "$", "b"]); -verify.completionsAt("b", ["foo ", "bar", "break", "any", "#", "$", "b", "\u0031\u0062"]); +verify.completionsAt("b", ["foo ", "bar", "break", "any", "#", "$", "b", "1b"]); + +verify.completionsAt("a", ["foo ", "bar", "break", "any", "#", "$", "b", "1b"]); +verify.applyCodeActionFromCompletion("a", { + name: "foo ", + description: "Use bracket notation instead of dot notation", + newRangeContent: 'x[""];', +}); diff --git a/tests/cases/fourslash/completionListInvalidMemberNames2.ts b/tests/cases/fourslash/completionListInvalidMemberNames2.ts index 753f9bbcb3064..153ed3bdb5e32 100644 --- a/tests/cases/fourslash/completionListInvalidMemberNames2.ts +++ b/tests/cases/fourslash/completionListInvalidMemberNames2.ts @@ -1,10 +1,19 @@ /// -////enum Foo { -//// X, Y, '☆' +// TODO: we should probably support this like we do in completionListInvalidMemberNames.ts + +////declare var Symbol: SymbolConstructor; +////interface SymbolConstructor { +//// readonly hasInstance: symbol; +////} +////interface Function { +//// [Symbol.hasInstance](value: any): boolean; +////} +////interface SomeInterface { +//// (value: number): any; ////} -////Foo./*a*/; -////Foo["/*b*/"]; +////var _ : SomeInterface; +////_./**/ -verify.completionsAt("a", ["X", "Y"]); -verify.completionsAt("b", ["X", "Y", "☆"]); +goTo.marker(); +verify.not.completionListContains("[Symbol.hasInstance]"); diff --git a/tests/cases/fourslash/completionListInvalidMemberNames3.ts b/tests/cases/fourslash/completionListInvalidMemberNames3.ts deleted file mode 100644 index cf1141b409409..0000000000000 --- a/tests/cases/fourslash/completionListInvalidMemberNames3.ts +++ /dev/null @@ -1,71 +0,0 @@ -/// - -// @allowjs: true - -// @Filename: test.js -////interface Symbol { -//// /** Returns a string representation of an object. */ -//// toString(): string; - -//// /** Returns the primitive value of the specified object. */ -//// valueOf(): Object; -////} - -////interface SymbolConstructor { -//// /** -//// * A reference to the prototype. -//// */ -//// readonly prototype: Symbol; - -//// /** -//// * Returns a new unique Symbol value. -//// * @param description Description of the new Symbol object. -//// */ -//// (description?: string | number): symbol; - -//// /** -//// * Returns a Symbol object from the global symbol registry matching the given key if found. -//// * Otherwise, returns a new symbol with this key. -//// * @param key key to search for. -//// */ -//// for(key: string): symbol; - -//// /** -//// * Returns a key from the global symbol registry matching the given Symbol if found. -//// * Otherwise, returns a undefined. -//// * @param sym Symbol to find the key for. -//// */ -//// keyFor(sym: symbol): string | undefined; -////} - -////declare var Symbol: SymbolConstructor;/// - -////interface SymbolConstructor { -//// /** -//// * A method that determines if a constructor object recognizes an object as one of the -//// * constructor’s instances. Called by the semantics of the instanceof operator. -//// */ -//// readonly hasInstance: symbol; -////} - -////interface Function { -//// /** -//// * Determines whether the given value inherits from this function if this function was used -//// * as a constructor function. -//// * -//// * A constructor function can control which objects are recognized as its instances by -//// * 'instanceof' by overriding this method. -//// */ -//// [Symbol.hasInstance](value: any): boolean; -////} - -////interface SomeInterface { -//// (value: number): any; -////} - -////var _ : SomeInterface; -////_./**/ - -goTo.marker(); - -verify.not.completionListContains("[Symbol.hasInstance]"); \ No newline at end of file diff --git a/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts b/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts new file mode 100644 index 0000000000000..a3f72a4b59486 --- /dev/null +++ b/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts @@ -0,0 +1,11 @@ +/// + +////declare const x: { "foo ": "space in the name", }; +////[|x.fo/**/;|] + +verify.completionsAt("", ["foo "]); +verify.applyCodeActionFromCompletion("", { + name: "foo ", + description: "Use bracket notation instead of dot notation", + newRangeContent: 'x["fo"];', +}); diff --git a/tests/cases/fourslash/completion_enum-members-with-invalid-identifiers-should-not-show-in-completion.ts b/tests/cases/fourslash/completion_enum-members-with-invalid-identifiers-should-not-show-in-completion.ts deleted file mode 100644 index 6d5b316719838..0000000000000 --- a/tests/cases/fourslash/completion_enum-members-with-invalid-identifiers-should-not-show-in-completion.ts +++ /dev/null @@ -1,13 +0,0 @@ -/// - -//// enum e { -//// "1", -//// 2 = 3, -//// 3, -//// a, -//// b -//// } -//// -//// e./**/ - -verify.completionsAt("", ["a", "b"]); diff --git a/tests/cases/fourslash/jsdocTypedefTagTypeExpressionCompletion.ts b/tests/cases/fourslash/jsdocTypedefTagTypeExpressionCompletion.ts index 379cc60e0587b..b7a2b7d899397 100644 --- a/tests/cases/fourslash/jsdocTypedefTagTypeExpressionCompletion.ts +++ b/tests/cases/fourslash/jsdocTypedefTagTypeExpressionCompletion.ts @@ -11,7 +11,7 @@ //// /** //// * @param {string} foo A value. //// * @returns {number} Another value -//// * @mytag +//// * @mytag //// */ //// method4(foo: string) { return 3; } //// } From 65552833b3109727d96b5441aeda61dcd799088e Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 18 Dec 2017 09:29:17 -0800 Subject: [PATCH 2/4] Use insertText and replacementSpan --- src/harness/fourslash.ts | 34 ++++++++--- .../unittests/tsserverProjectSystem.ts | 12 ++-- src/server/protocol.ts | 11 ++++ src/server/session.ts | 4 +- src/services/completions.ts | 59 +++++++++---------- src/services/services.ts | 2 +- src/services/types.ts | 2 + .../reference/api/tsserverlibrary.d.ts | 13 ++++ tests/baselines/reference/api/typescript.d.ts | 2 + .../completionListInvalidMemberNames.ts | 19 ++++-- ...validMemberNames_withExistingIdentifier.ts | 10 +--- tests/cases/fourslash/completionsPaths.ts | 9 +-- tests/cases/fourslash/fourslash.ts | 5 +- 13 files changed, 114 insertions(+), 68 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 0d5216d62b503..4fe12f9238655 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -511,7 +511,7 @@ namespace FourSlash { } } - private raiseError(message: string) { + private raiseError(message: string): never { throw new Error(this.messageAtLastKnownMarker(message)); } @@ -848,10 +848,10 @@ namespace FourSlash { } } - public verifyCompletionsAt(markerName: string, expected: string[], options?: FourSlashInterface.CompletionsAtOptions) { + public verifyCompletionsAt(markerName: string, expected: ReadonlyArray, options?: FourSlashInterface.CompletionsAtOptions) { this.goToMarker(markerName); - const actualCompletions = this.getCompletionListAtCaret(); + const actualCompletions = this.getCompletionListAtCaret(options); if (!actualCompletions) { this.raiseError(`No completions at position '${this.currentCaretPosition}'.`); } @@ -867,9 +867,20 @@ namespace FourSlash { } ts.zipWith(actual, expected, (completion, expectedCompletion, index) => { - if (completion.name !== expectedCompletion) { + const { name, insertText, replacementSpan } = typeof expectedCompletion === "string" ? { name: expectedCompletion, insertText: undefined, replacementSpan: undefined } : expectedCompletion; + if (completion.name !== name) { this.raiseError(`Expected completion at index ${index} to be ${expectedCompletion}, got ${completion.name}`); } + if (completion.insertText !== insertText) { + this.raiseError(`Expected completion insert text at index ${index} to be ${insertText}, got ${completion.insertText}`); + } + const convertedReplacementSpan = replacementSpan && textSpanFromRange(replacementSpan); + try { + assert.deepEqual(completion.replacementSpan, convertedReplacementSpan); + } + catch { + this.raiseError(`Expected completion replacementSpan at index ${index} to be ${stringify(convertedReplacementSpan)}, got ${stringify(completion.replacementSpan)}`); + } }); } @@ -1807,7 +1818,7 @@ Actual: ${stringify(fullActual)}`); } else if (prevChar === " " && /A-Za-z_/.test(ch)) { /* Completions */ - this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, { includeExternalModuleExports: false }); + this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, { includeExternalModuleExports: false, includeBracketCompletions: false }); } if (i % checkCadence === 0) { @@ -2382,7 +2393,8 @@ Actual: ${stringify(fullActual)}`); public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) { this.goToMarker(markerName); - const actualCompletion = this.getCompletionListAtCaret({ includeExternalModuleExports: true }).entries.find(e => e.name === options.name && e.source === options.source); + const actualCompletion = this.getCompletionListAtCaret({ includeExternalModuleExports: true, includeBracketCompletions: false }).entries.find(e => + e.name === options.name && e.source === options.source); if (!actualCompletion.hasAction) { this.raiseError(`Completion for ${options.name} does not have an associated action.`); @@ -3182,8 +3194,7 @@ Actual: ${stringify(fullActual)}`); private getTextSpanForRangeAtIndex(index: number): ts.TextSpan { const ranges = this.getRanges(); if (ranges && ranges.length > index) { - const range = ranges[index]; - return { start: range.start, length: range.end - range.start }; + return textSpanFromRange(ranges[index]); } else { this.raiseError("Supplied span index: " + index + " does not exist in range list of size: " + (ranges ? 0 : ranges.length)); @@ -3213,6 +3224,10 @@ Actual: ${stringify(fullActual)}`); } } + function textSpanFromRange(range: FourSlash.Range): ts.TextSpan { + return ts.createTextSpanFromBounds(range.start, range.end); + } + export function runFourSlashTest(basePath: string, testType: FourSlashTestType, fileName: string) { const content = Harness.IO.readFile(fileName); runFourSlashTestContent(basePath, testType, content, fileName); @@ -3951,7 +3966,7 @@ namespace FourSlashInterface { super(state); } - public completionsAt(markerName: string, completions: string[], options?: CompletionsAtOptions) { + public completionsAt(markerName: string, completions: ReadonlyArray, options?: CompletionsAtOptions) { this.state.verifyCompletionsAt(markerName, completions, options); } @@ -4575,6 +4590,7 @@ namespace FourSlashInterface { newContent: string; } + export type ExpectedCompletionEntry = string | { name: string, insertText?: string, replacementSpan?: FourSlash.Range }; export interface CompletionsAtOptions extends ts.GetCompletionsAtPositionOptions { isNewIdentifierLocation?: boolean; } diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 39dde23b28a55..c74fadbaecb87 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -1284,13 +1284,13 @@ namespace ts.projectSystem { service.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]); - const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false }); + const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeBracketCompletions: false }); // should contain completions for string assert(completions1.entries.some(e => e.name === "charAt"), "should contain 'charAt'"); assert.isFalse(completions1.entries.some(e => e.name === "toExponential"), "should not contain 'toExponential'"); service.closeClientFile(f2.path); - const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false }); + const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeBracketCompletions: false }); // should contain completions for string assert.isFalse(completions2.entries.some(e => e.name === "charAt"), "should not contain 'charAt'"); assert(completions2.entries.some(e => e.name === "toExponential"), "should contain 'toExponential'"); @@ -1316,11 +1316,11 @@ namespace ts.projectSystem { service.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]); - const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false }); + const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeBracketCompletions: false }); assert(completions1.entries.some(e => e.name === "somelongname"), "should contain 'somelongname'"); service.closeClientFile(f2.path); - const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false }); + const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeBracketCompletions: false }); assert.isFalse(completions2.entries.some(e => e.name === "somelongname"), "should not contain 'somelongname'"); const sf2 = service.externalProjects[0].getLanguageService().getProgram().getSourceFile(f2.path); assert.equal(sf2.text, ""); @@ -1925,7 +1925,7 @@ namespace ts.projectSystem { // Check identifiers defined in HTML content are available in .ts file const project = configuredProjectAt(projectService, 0); - let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, { includeExternalModuleExports: false }); + let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, { includeExternalModuleExports: false, includeBracketCompletions: false }); assert(completions && completions.entries[0].name === "hello", `expected entry hello to be in completion list`); // Close HTML file @@ -1939,7 +1939,7 @@ namespace ts.projectSystem { checkProjectActualFiles(configuredProjectAt(projectService, 0), [file1.path, file2.path, config.path]); // Check identifiers defined in HTML content are not available in .ts file - completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, { includeExternalModuleExports: false }); + completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, { includeExternalModuleExports: false, includeBracketCompletions: false }); assert(completions && completions.entries[0].name !== "hello", `unexpected hello entry in completion list`); }); diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 4bd7a0a0967d7..4472dbae04651 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -1693,6 +1693,11 @@ namespace ts.server.protocol { * This affects lone identifier completions but not completions on the right hand side of `obj.`. */ includeExternalModuleExports: boolean; + /** + * If enabled, the completion list will include completions with invalid identifier names. + * For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`. + */ + includeBracketCompletions: boolean; } /** @@ -1768,6 +1773,12 @@ namespace ts.server.protocol { * is often the same as the name but may be different in certain circumstances. */ sortText: string; + /** + * Text to insert instead of `name`. + * This is used to support bracketed completions; If `name` might be "a-b" but `insertText` would be `["a-b"]`, + * coupled with `replacementSpan` to replace a dotted access with a bracket access. + */ + insertText?: string; /** * An optional span that indicates the text to be replaced by this completion item. * If present, this span should be used instead of the default one. diff --git a/src/server/session.ts b/src/server/session.ts index 17c5777132f22..cec558c56a883 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1207,10 +1207,10 @@ namespace ts.server { if (simplifiedResult) { return mapDefined(completions && completions.entries, entry => { if (completions.isMemberCompletion || startsWith(entry.name.toLowerCase(), prefix.toLowerCase())) { - const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source, isRecommended } = entry; + const { name, kind, kindModifiers, sortText, insertText, replacementSpan, hasAction, source, isRecommended } = entry; const convertedSpan = replacementSpan ? this.toLocationTextSpan(replacementSpan, scriptInfo) : undefined; // Use `hasAction || undefined` to avoid serializing `false`. - return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source, isRecommended }; + return { name, kind, kindModifiers, sortText, insertText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source, isRecommended }; } }).sort((a, b) => compareStringsCaseSensitiveUI(a.name, b.name)); } diff --git a/src/services/completions.ts b/src/services/completions.ts index dc303be564729..d6966805c6408 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -52,7 +52,7 @@ namespace ts.Completions { switch (completionData.kind) { case CompletionDataKind.Data: - return completionInfoFromData(sourceFile, typeChecker, compilerOptions, log, completionData); + return completionInfoFromData(sourceFile, typeChecker, compilerOptions, log, completionData, options.includeBracketCompletions); case CompletionDataKind.JsDocTagName: // If the current position is a jsDoc tag name, only tag names should be provided for completion return jsdocCompletionInfo(JsDoc.getJSDocTagNameCompletions()); @@ -70,8 +70,8 @@ namespace ts.Completions { return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries }; } - function completionInfoFromData(sourceFile: SourceFile, typeChecker: TypeChecker, compilerOptions: CompilerOptions, log: Log, completionData: CompletionData): CompletionInfo { - const { symbols, completionKind, isNewIdentifierLocation, location, keywordFilters, symbolToOriginInfoMap, recommendedCompletion } = completionData; + function completionInfoFromData(sourceFile: SourceFile, typeChecker: TypeChecker, compilerOptions: CompilerOptions, log: Log, completionData: CompletionData, includeBracketCompletions: boolean): CompletionInfo { + const { symbols, completionKind, isNewIdentifierLocation, location, propertyAccessToConvert, keywordFilters, symbolToOriginInfoMap, recommendedCompletion } = completionData; if (sourceFile.languageVariant === LanguageVariant.JSX && location && location.parent && isJsxClosingElement(location.parent)) { // In the TypeScript JSX element, if such element is not defined. When users query for completion at closing tag, @@ -91,7 +91,7 @@ namespace ts.Completions { const entries: CompletionEntry[] = []; if (isSourceFileJavaScript(sourceFile)) { - const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries, location, typeChecker, compilerOptions.target, log, completionKind, recommendedCompletion, symbolToOriginInfoMap); + const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries, location, typeChecker, compilerOptions.target, log, completionKind, includeBracketCompletions, propertyAccessToConvert, recommendedCompletion, symbolToOriginInfoMap); getJavaScriptCompletionEntries(sourceFile, location.pos, uniqueNames, compilerOptions.target, entries); } else { @@ -99,7 +99,7 @@ namespace ts.Completions { return undefined; } - getCompletionEntriesFromSymbols(symbols, entries, location, typeChecker, compilerOptions.target, log, completionKind, recommendedCompletion, symbolToOriginInfoMap); + getCompletionEntriesFromSymbols(symbols, entries, location, typeChecker, compilerOptions.target, log, completionKind, includeBracketCompletions, propertyAccessToConvert, recommendedCompletion, symbolToOriginInfoMap); } // TODO add filter for keyword based on type/value/namespace and also location @@ -157,12 +157,18 @@ namespace ts.Completions { kind: CompletionKind, origin: SymbolOriginInfo | undefined, recommendedCompletion: Symbol | undefined, + propertyAccessToConvert: PropertyAccessExpression | undefined, + includeBracketCompletions: boolean, ): CompletionEntry | undefined { const info = getCompletionEntryDisplayNameForSymbol(symbol, target, origin, kind); if (!info) { return undefined; } const { name, needsConvertPropertyAccess } = info; + Debug.assert(!(needsConvertPropertyAccess && !propertyAccessToConvert)); + if (needsConvertPropertyAccess && !includeBracketCompletions) { + return undefined; + } // TODO(drosen): Right now we just permit *all* semantic meanings when calling // 'getSymbolKind' which is permissible given that it is backwards compatible; but @@ -178,11 +184,17 @@ namespace ts.Completions { kindModifiers: SymbolDisplay.getSymbolModifiers(symbol), sortText: "0", source: getSourceFromOrigin(origin), + // TODO: GH#20619 Use configured quote style + insertText: needsConvertPropertyAccess ? `["${name}"]` : undefined, + replacementSpan: needsConvertPropertyAccess + ? createTextSpanFromBounds(findChildOfKind(propertyAccessToConvert, SyntaxKind.DotToken)!.getStart(), propertyAccessToConvert.name.end) + : undefined, hasAction: trueOrUndefined(needsConvertPropertyAccess || origin !== undefined), isRecommended: trueOrUndefined(isRecommendedCompletionMatch(symbol, recommendedCompletion, typeChecker)), }; } + function isRecommendedCompletionMatch(localSymbol: Symbol, recommendedCompletion: Symbol, checker: TypeChecker): boolean { return localSymbol === recommendedCompletion || !!(localSymbol.flags & SymbolFlags.ExportValue) && checker.getExportSymbolOfSymbol(localSymbol) === recommendedCompletion; @@ -204,6 +216,8 @@ namespace ts.Completions { target: ScriptTarget, log: Log, kind: CompletionKind, + includeBracketCompletions?: boolean, + propertyAccessToConvert?: PropertyAccessExpression | undefined, recommendedCompletion?: Symbol, symbolToOriginInfoMap?: SymbolOriginInfoMap, ): Map { @@ -216,7 +230,7 @@ namespace ts.Completions { if (symbols) { for (const symbol of symbols) { const origin = symbolToOriginInfoMap ? symbolToOriginInfoMap[getSymbolId(symbol)] : undefined; - const entry = createCompletionEntry(symbol, location, typeChecker, target, kind, origin, recommendedCompletion); + const entry = createCompletionEntry(symbol, location, typeChecker, target, kind, origin, recommendedCompletion, propertyAccessToConvert, includeBracketCompletions); if (!entry) { continue; } @@ -436,7 +450,6 @@ namespace ts.Completions { interface SymbolCompletion { type: "symbol"; symbol: Symbol; - propertyAccessToConvert: PropertyAccessExpression | undefined; location: Node; symbolToOriginInfoMap: SymbolOriginInfoMap; } @@ -449,7 +462,7 @@ namespace ts.Completions { { name, source }: CompletionEntryIdentifier, allSourceFiles: ReadonlyArray, ): SymbolCompletion | { type: "request", request: Request } | { type: "none" } { - const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true }, compilerOptions.target); + const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true, includeBracketCompletions: true }, compilerOptions.target); if (!completionData) { return { type: "none" }; } @@ -457,7 +470,7 @@ namespace ts.Completions { return { type: "request", request: completionData }; } - const { symbols, location, completionKind, propertyAccessToConvert, symbolToOriginInfoMap } = completionData; + const { symbols, location, completionKind, symbolToOriginInfoMap } = completionData; // Find the symbol with the matching entry name. // We don't need to perform character checks here because we're only comparing the @@ -466,7 +479,7 @@ namespace ts.Completions { return firstDefined(symbols, (symbol): SymbolCompletion => { // TODO: Shouldn't need return type annotation (GH#12632) const origin = symbolToOriginInfoMap[getSymbolId(symbol)]; const info = getCompletionEntryDisplayNameForSymbol(symbol, compilerOptions.target, origin, completionKind); - return info && info.name === name && getSourceFromOrigin(origin) === source ? { type: "symbol" as "symbol", symbol, propertyAccessToConvert: info.needsConvertPropertyAccess ? propertyAccessToConvert : undefined, location, symbolToOriginInfoMap } : undefined; + return info && info.name === name && getSourceFromOrigin(origin) === source ? { type: "symbol" as "symbol", symbol, location, symbolToOriginInfoMap } : undefined; }) || { type: "none" }; } @@ -512,8 +525,8 @@ namespace ts.Completions { } } case "symbol": { - const { symbol, location, propertyAccessToConvert, symbolToOriginInfoMap } = symbolCompletion; - const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles, propertyAccessToConvert); + const { symbol, location, symbolToOriginInfoMap } = symbolCompletion; + const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles); const kindModifiers = SymbolDisplay.getSymbolModifiers(symbol); const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); return { name, kindModifiers, kind: symbolKind, displayParts, documentation, tags, codeActions, source: sourceDisplay }; @@ -552,32 +565,13 @@ namespace ts.Completions { formatContext: formatting.FormatContext, getCanonicalFileName: GetCanonicalFileName, allSourceFiles: ReadonlyArray, - propertyAccessToConvert: PropertyAccessExpression | undefined, ): CodeActionsAndSourceDisplay { const symbolOriginInfo = symbolToOriginInfoMap[getSymbolId(symbol)]; - Debug.assert(!(symbolOriginInfo && propertyAccessToConvert)); - return propertyAccessToConvert - ? { codeActions: convertDotToBracketAccess(propertyAccessToConvert), sourceDisplay: undefined } - : symbolOriginInfo + return symbolOriginInfo ? getCodeActionsAndSourceDisplayForImport(symbolOriginInfo, symbol, program, checker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles) : { codeActions: undefined, sourceDisplay: undefined }; } - function convertDotToBracketAccess(prop: PropertyAccessExpression): CodeAction[] { - const sourceFile = prop.getSourceFile(); - const changes: ts.FileTextChanges[] = [ - { - fileName: sourceFile.fileName, - textChanges: [ - { span: { start: ts.getFirstChildOfKind(prop, sourceFile, SyntaxKind.DotToken)!.getStart(), length: 1 }, newText: '["', }, - { span: { start: prop.name.getEnd(), length: 0 }, newText: '"]', }, - ], - }, - ]; - const description = formatMessage(/*dummy*/ undefined, Diagnostics.Use_bracket_notation_instead_of_dot_notation); - return [{ changes, description }]; - } - function getCodeActionsAndSourceDisplayForImport( symbolOriginInfo: SymbolOriginInfo, symbol: Symbol, @@ -642,6 +636,7 @@ namespace ts.Completions { readonly kind: CompletionDataKind.Data; readonly symbols: ReadonlyArray; readonly completionKind: CompletionKind; + /** Note that the presence of this alone doesn't mean that we need a conversion. Only do that if the completion is not an ordinary identifier. */ readonly propertyAccessToConvert: PropertyAccessExpression | undefined; readonly isNewIdentifierLocation: boolean; readonly location: Node | undefined; diff --git a/src/services/services.ts b/src/services/services.ts index 1660ca10e4347..263fd1b0653db 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1432,7 +1432,7 @@ namespace ts { return [...program.getOptionsDiagnostics(cancellationToken), ...program.getGlobalDiagnostics(cancellationToken)]; } - function getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions = { includeExternalModuleExports: false }): CompletionInfo { + function getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions = { includeExternalModuleExports: false, includeBracketCompletions: false }): CompletionInfo { synchronizeHostData(); return Completions.getCompletionsAtPosition( host, diff --git a/src/services/types.ts b/src/services/types.ts index 471ae3b345977..c2f28fde09f05 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -333,6 +333,7 @@ namespace ts { export interface GetCompletionsAtPositionOptions { includeExternalModuleExports: boolean; + includeBracketCompletions: boolean; } export interface ApplyCodeActionCommandResult { @@ -746,6 +747,7 @@ namespace ts { kind: ScriptElementKind; kindModifiers: string; // see ScriptElementKindModifier, comma separated sortText: string; + insertText?: string; /** * An optional span that indicates the text to be replaced by this completion item. * If present, this span should be used instead of the default one. diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 464e70671f4bd..ae0dc78846a64 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -4002,6 +4002,7 @@ declare namespace ts { } interface GetCompletionsAtPositionOptions { includeExternalModuleExports: boolean; + includeBracketCompletions: boolean; } interface ApplyCodeActionCommandResult { successMessage: string; @@ -4344,6 +4345,7 @@ declare namespace ts { kind: ScriptElementKind; kindModifiers: string; sortText: string; + insertText?: string; /** * An optional span that indicates the text to be replaced by this completion item. * If present, this span should be used instead of the default one. @@ -6106,6 +6108,11 @@ declare namespace ts.server.protocol { * This affects lone identifier completions but not completions on the right hand side of `obj.`. */ includeExternalModuleExports: boolean; + /** + * If enabled, the completion list will include completions with invalid identifier names. + * For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`. + */ + includeBracketCompletions: boolean; } /** * Completions request; value of command field is "completions". @@ -6174,6 +6181,12 @@ declare namespace ts.server.protocol { * is often the same as the name but may be different in certain circumstances. */ sortText: string; + /** + * Text to insert instead of `name`. + * This is used to support bracketed completions; If `name` might be "a-b" but `insertText` would be `["a-b"]`, + * coupled with `replacementSpan` to replace a dotted access with a bracket access. + */ + insertText?: string; /** * An optional span that indicates the text to be replaced by this completion item. * If present, this span should be used instead of the default one. diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index bc7115a089a89..f9d1e017d8a29 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4002,6 +4002,7 @@ declare namespace ts { } interface GetCompletionsAtPositionOptions { includeExternalModuleExports: boolean; + includeBracketCompletions: boolean; } interface ApplyCodeActionCommandResult { successMessage: string; @@ -4344,6 +4345,7 @@ declare namespace ts { kind: ScriptElementKind; kindModifiers: string; sortText: string; + insertText?: string; /** * An optional span that indicates the text to be replaced by this completion item. * If present, this span should be used instead of the default one. diff --git a/tests/cases/fourslash/completionListInvalidMemberNames.ts b/tests/cases/fourslash/completionListInvalidMemberNames.ts index 82fadcaa36577..8fd2a195d0f8b 100644 --- a/tests/cases/fourslash/completionListInvalidMemberNames.ts +++ b/tests/cases/fourslash/completionListInvalidMemberNames.ts @@ -11,14 +11,21 @@ //// "\u0031\u0062": "invalid unicode identifer name (1b)" ////}; //// -////[|x./*a*/;|] +////x[|./*a*/|]; ////x["/*b*/"]; verify.completionsAt("b", ["foo ", "bar", "break", "any", "#", "$", "b", "1b"]); -verify.completionsAt("a", ["foo ", "bar", "break", "any", "#", "$", "b", "1b"]); -verify.applyCodeActionFromCompletion("a", { - name: "foo ", - description: "Use bracket notation instead of dot notation", - newRangeContent: 'x[""];', +const replacementSpan = test.ranges()[0]; +verify.completionsAt("a", [ + { name: "foo ", insertText: '["foo "]', replacementSpan }, + "bar", + "break", + "any", + { name: "#", insertText: '["#"]', replacementSpan }, + "$", + "b", + { name: "1b", insertText: '["1b"]', replacementSpan }, +], { + includeBracketCompletions: true, }); diff --git a/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts b/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts index a3f72a4b59486..c67e0bbf445a4 100644 --- a/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts +++ b/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts @@ -1,11 +1,7 @@ /// ////declare const x: { "foo ": "space in the name", }; -////[|x.fo/**/;|] +////x[|.fo/**/|]; -verify.completionsAt("", ["foo "]); -verify.applyCodeActionFromCompletion("", { - name: "foo ", - description: "Use bracket notation instead of dot notation", - newRangeContent: 'x["fo"];', -}); +const replacementSpan = test.ranges()[0]; +verify.completionsAt("", [{ name: "foo ", insertText: '["foo "]', replacementSpan }], { includeBracketCompletions: true }); diff --git a/tests/cases/fourslash/completionsPaths.ts b/tests/cases/fourslash/completionsPaths.ts index 9473ae54b257c..91a2f81054669 100644 --- a/tests/cases/fourslash/completionsPaths.ts +++ b/tests/cases/fourslash/completionsPaths.ts @@ -12,10 +12,11 @@ ////not read // @Filename: /src/a.ts -////import {} from "/*1*/"; +////import {} from "[|/*1*/|]"; // @Filename: /src/folder/b.ts -////import {} from "x//*2*/"; +////import {} from "x/[|/*2*/|]"; -verify.completionsAt("1", ["y", "x"]); -verify.completionsAt("2", ["bar", "foo"]); +const [r0, r1] = test.ranges(); +verify.completionsAt("1", [{ name: "y", replacementSpan: r0 }, { name: "x", replacementSpan: r0 }]); +verify.completionsAt("2", [{ name: "bar", replacementSpan: r1 }, { name: "foo", replacementSpan: r1 }]); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index acebb9dfc0545..7f4b47194cfd1 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -186,7 +186,10 @@ declare namespace FourSlashInterface { class verify extends verifyNegatable { assertHasRanges(ranges: Range[]): void; caretAtMarker(markerName?: string): void; - completionsAt(markerName: string, completions: string[], options?: { isNewIdentifierLocation?: boolean }): void; + completionsAt(markerName: string, completions: ReadonlyArray, options?: { + isNewIdentifierLocation?: boolean; + includeBracketCompletions?: boolean; + }): void; completionsAndDetailsAt( markerName: string, completions: { From fd9ea5b393f116625f664c3304e402669e5feb8e Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 19 Dec 2017 14:24:31 -0800 Subject: [PATCH 3/4] Rename includeBracketCompletions to includeInsertTextCompletions --- src/harness/fourslash.ts | 4 ++-- src/harness/unittests/tsserverProjectSystem.ts | 12 ++++++------ src/server/protocol.ts | 2 +- src/services/completions.ts | 18 +++++++++--------- src/services/services.ts | 2 +- src/services/types.ts | 2 +- .../reference/api/tsserverlibrary.d.ts | 4 ++-- tests/baselines/reference/api/typescript.d.ts | 2 +- .../completionListInvalidMemberNames.ts | 2 +- ...nvalidMemberNames_withExistingIdentifier.ts | 2 +- tests/cases/fourslash/fourslash.ts | 2 +- 11 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index f3877f6682a2f..ec62c3ad3f910 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1819,7 +1819,7 @@ Actual: ${stringify(fullActual)}`); } else if (prevChar === " " && /A-Za-z_/.test(ch)) { /* Completions */ - this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, { includeExternalModuleExports: false, includeBracketCompletions: false }); + this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, { includeExternalModuleExports: false, includeInsertTextCompletions: false }); } if (i % checkCadence === 0) { @@ -2394,7 +2394,7 @@ Actual: ${stringify(fullActual)}`); public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) { this.goToMarker(markerName); - const actualCompletion = this.getCompletionListAtCaret({ includeExternalModuleExports: true, includeBracketCompletions: false }).entries.find(e => + const actualCompletion = this.getCompletionListAtCaret({ includeExternalModuleExports: true, includeInsertTextCompletions: false }).entries.find(e => e.name === options.name && e.source === options.source); if (!actualCompletion.hasAction) { diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 058e4b50abda5..5606396ed24fa 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -1284,13 +1284,13 @@ namespace ts.projectSystem { service.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]); - const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeBracketCompletions: false }); + const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeInsertTextCompletions: false }); // should contain completions for string assert.isTrue(completions1.entries.some(e => e.name === "charAt"), "should contain 'charAt'"); assert.isFalse(completions1.entries.some(e => e.name === "toExponential"), "should not contain 'toExponential'"); service.closeClientFile(f2.path); - const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeBracketCompletions: false }); + const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeInsertTextCompletions: false }); // should contain completions for string assert.isFalse(completions2.entries.some(e => e.name === "charAt"), "should not contain 'charAt'"); assert.isTrue(completions2.entries.some(e => e.name === "toExponential"), "should contain 'toExponential'"); @@ -1316,11 +1316,11 @@ namespace ts.projectSystem { service.checkNumberOfProjects({ externalProjects: 1 }); checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]); - const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeBracketCompletions: false }); + const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeInsertTextCompletions: false }); assert.isTrue(completions1.entries.some(e => e.name === "somelongname"), "should contain 'somelongname'"); service.closeClientFile(f2.path); - const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeBracketCompletions: false }); + const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeInsertTextCompletions: false }); assert.isFalse(completions2.entries.some(e => e.name === "somelongname"), "should not contain 'somelongname'"); const sf2 = service.externalProjects[0].getLanguageService().getProgram().getSourceFile(f2.path); assert.equal(sf2.text, ""); @@ -1925,7 +1925,7 @@ namespace ts.projectSystem { // Check identifiers defined in HTML content are available in .ts file const project = configuredProjectAt(projectService, 0); - let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, { includeExternalModuleExports: false, includeBracketCompletions: false }); + let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, { includeExternalModuleExports: false, includeInsertTextCompletions: false }); assert(completions && completions.entries[0].name === "hello", `expected entry hello to be in completion list`); // Close HTML file @@ -1939,7 +1939,7 @@ namespace ts.projectSystem { checkProjectActualFiles(configuredProjectAt(projectService, 0), [file1.path, file2.path, config.path]); // Check identifiers defined in HTML content are not available in .ts file - completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, { includeExternalModuleExports: false, includeBracketCompletions: false }); + completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, { includeExternalModuleExports: false, includeInsertTextCompletions: false }); assert(completions && completions.entries[0].name !== "hello", `unexpected hello entry in completion list`); }); diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 4472dbae04651..cbe06872eb005 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -1697,7 +1697,7 @@ namespace ts.server.protocol { * If enabled, the completion list will include completions with invalid identifier names. * For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`. */ - includeBracketCompletions: boolean; + includeInsertTextCompletions: boolean; } /** diff --git a/src/services/completions.ts b/src/services/completions.ts index 07a1c92c247cf..e49489f03c371 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -52,7 +52,7 @@ namespace ts.Completions { switch (completionData.kind) { case CompletionDataKind.Data: - return completionInfoFromData(sourceFile, typeChecker, compilerOptions, log, completionData, options.includeBracketCompletions); + return completionInfoFromData(sourceFile, typeChecker, compilerOptions, log, completionData, options.includeInsertTextCompletions); case CompletionDataKind.JsDocTagName: // If the current position is a jsDoc tag name, only tag names should be provided for completion return jsdocCompletionInfo(JsDoc.getJSDocTagNameCompletions()); @@ -70,7 +70,7 @@ namespace ts.Completions { return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries }; } - function completionInfoFromData(sourceFile: SourceFile, typeChecker: TypeChecker, compilerOptions: CompilerOptions, log: Log, completionData: CompletionData, includeBracketCompletions: boolean): CompletionInfo { + function completionInfoFromData(sourceFile: SourceFile, typeChecker: TypeChecker, compilerOptions: CompilerOptions, log: Log, completionData: CompletionData, includeInsertTextCompletions: boolean): CompletionInfo { const { symbols, completionKind, isNewIdentifierLocation, location, propertyAccessToConvert, keywordFilters, symbolToOriginInfoMap, recommendedCompletion } = completionData; if (sourceFile.languageVariant === LanguageVariant.JSX && location && location.parent && isJsxClosingElement(location.parent)) { @@ -91,7 +91,7 @@ namespace ts.Completions { const entries: CompletionEntry[] = []; if (isSourceFileJavaScript(sourceFile)) { - const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries, location, sourceFile, typeChecker, compilerOptions.target, log, completionKind, includeBracketCompletions, propertyAccessToConvert, recommendedCompletion, symbolToOriginInfoMap); + const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries, location, sourceFile, typeChecker, compilerOptions.target, log, completionKind, includeInsertTextCompletions, propertyAccessToConvert, recommendedCompletion, symbolToOriginInfoMap); getJavaScriptCompletionEntries(sourceFile, location.pos, uniqueNames, compilerOptions.target, entries); } else { @@ -99,7 +99,7 @@ namespace ts.Completions { return undefined; } - getCompletionEntriesFromSymbols(symbols, entries, location, sourceFile, typeChecker, compilerOptions.target, log, completionKind, includeBracketCompletions, propertyAccessToConvert, recommendedCompletion, symbolToOriginInfoMap); + getCompletionEntriesFromSymbols(symbols, entries, location, sourceFile, typeChecker, compilerOptions.target, log, completionKind, includeInsertTextCompletions, propertyAccessToConvert, recommendedCompletion, symbolToOriginInfoMap); } // TODO add filter for keyword based on type/value/namespace and also location @@ -159,7 +159,7 @@ namespace ts.Completions { origin: SymbolOriginInfo | undefined, recommendedCompletion: Symbol | undefined, propertyAccessToConvert: PropertyAccessExpression | undefined, - includeBracketCompletions: boolean, + includeInsertTextCompletions: boolean, ): CompletionEntry | undefined { const info = getCompletionEntryDisplayNameForSymbol(symbol, target, origin, kind); if (!info) { @@ -167,7 +167,7 @@ namespace ts.Completions { } const { name, needsConvertPropertyAccess } = info; Debug.assert(!(needsConvertPropertyAccess && !propertyAccessToConvert)); - if (needsConvertPropertyAccess && !includeBracketCompletions) { + if (needsConvertPropertyAccess && !includeInsertTextCompletions) { return undefined; } @@ -218,7 +218,7 @@ namespace ts.Completions { target: ScriptTarget, log: Log, kind: CompletionKind, - includeBracketCompletions?: boolean, + includeInsertTextCompletions?: boolean, propertyAccessToConvert?: PropertyAccessExpression | undefined, recommendedCompletion?: Symbol, symbolToOriginInfoMap?: SymbolOriginInfoMap, @@ -232,7 +232,7 @@ namespace ts.Completions { if (symbols) { for (const symbol of symbols) { const origin = symbolToOriginInfoMap ? symbolToOriginInfoMap[getSymbolId(symbol)] : undefined; - const entry = createCompletionEntry(symbol, location, sourceFile, typeChecker, target, kind, origin, recommendedCompletion, propertyAccessToConvert, includeBracketCompletions); + const entry = createCompletionEntry(symbol, location, sourceFile, typeChecker, target, kind, origin, recommendedCompletion, propertyAccessToConvert, includeInsertTextCompletions); if (!entry) { continue; } @@ -464,7 +464,7 @@ namespace ts.Completions { { name, source }: CompletionEntryIdentifier, allSourceFiles: ReadonlyArray, ): SymbolCompletion | { type: "request", request: Request } | { type: "none" } { - const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true, includeBracketCompletions: true }, compilerOptions.target); + const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true, includeInsertTextCompletions: true }, compilerOptions.target); if (!completionData) { return { type: "none" }; } diff --git a/src/services/services.ts b/src/services/services.ts index 263fd1b0653db..f880660aa08c4 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1432,7 +1432,7 @@ namespace ts { return [...program.getOptionsDiagnostics(cancellationToken), ...program.getGlobalDiagnostics(cancellationToken)]; } - function getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions = { includeExternalModuleExports: false, includeBracketCompletions: false }): CompletionInfo { + function getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions = { includeExternalModuleExports: false, includeInsertTextCompletions: false }): CompletionInfo { synchronizeHostData(); return Completions.getCompletionsAtPosition( host, diff --git a/src/services/types.ts b/src/services/types.ts index c2f28fde09f05..ee206b347c511 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -333,7 +333,7 @@ namespace ts { export interface GetCompletionsAtPositionOptions { includeExternalModuleExports: boolean; - includeBracketCompletions: boolean; + includeInsertTextCompletions: boolean; } export interface ApplyCodeActionCommandResult { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 191530141d7f7..79f574b85cf01 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -3997,7 +3997,7 @@ declare namespace ts { } interface GetCompletionsAtPositionOptions { includeExternalModuleExports: boolean; - includeBracketCompletions: boolean; + includeInsertTextCompletions: boolean; } interface ApplyCodeActionCommandResult { successMessage: string; @@ -6107,7 +6107,7 @@ declare namespace ts.server.protocol { * If enabled, the completion list will include completions with invalid identifier names. * For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`. */ - includeBracketCompletions: boolean; + includeInsertTextCompletions: boolean; } /** * Completions request; value of command field is "completions". diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index e7b2b78516600..b689ce24f060f 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -3997,7 +3997,7 @@ declare namespace ts { } interface GetCompletionsAtPositionOptions { includeExternalModuleExports: boolean; - includeBracketCompletions: boolean; + includeInsertTextCompletions: boolean; } interface ApplyCodeActionCommandResult { successMessage: string; diff --git a/tests/cases/fourslash/completionListInvalidMemberNames.ts b/tests/cases/fourslash/completionListInvalidMemberNames.ts index 8fd2a195d0f8b..3dc54274e838e 100644 --- a/tests/cases/fourslash/completionListInvalidMemberNames.ts +++ b/tests/cases/fourslash/completionListInvalidMemberNames.ts @@ -27,5 +27,5 @@ verify.completionsAt("a", [ "b", { name: "1b", insertText: '["1b"]', replacementSpan }, ], { - includeBracketCompletions: true, + includeInsertTextCompletions: true, }); diff --git a/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts b/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts index c67e0bbf445a4..8463a710b01ae 100644 --- a/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts +++ b/tests/cases/fourslash/completionListInvalidMemberNames_withExistingIdentifier.ts @@ -4,4 +4,4 @@ ////x[|.fo/**/|]; const replacementSpan = test.ranges()[0]; -verify.completionsAt("", [{ name: "foo ", insertText: '["foo "]', replacementSpan }], { includeBracketCompletions: true }); +verify.completionsAt("", [{ name: "foo ", insertText: '["foo "]', replacementSpan }], { includeInsertTextCompletions: true }); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 7f4b47194cfd1..f0ead846ca636 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -188,7 +188,7 @@ declare namespace FourSlashInterface { caretAtMarker(markerName?: string): void; completionsAt(markerName: string, completions: ReadonlyArray, options?: { isNewIdentifierLocation?: boolean; - includeBracketCompletions?: boolean; + includeInsertTextCompletions?: boolean; }): void; completionsAndDetailsAt( markerName: string, From 8a504d54ad78d733fe4bb19f4f8f87f2513b4b84 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 4 Jan 2018 15:18:03 -0800 Subject: [PATCH 4/4] Don't add completions that start with space --- src/compiler/diagnosticMessages.json | 4 ---- src/services/completions.ts | 3 ++- .../completionListInvalidMemberNames_startWithSpace.ts | 8 ++++++++ 3 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 tests/cases/fourslash/completionListInvalidMemberNames_startWithSpace.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 7cd9a7b0214e9..826efa6994b00 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3898,9 +3898,5 @@ "Install '{0}'": { "category": "Message", "code": 95014 - }, - "Use bracket notation instead of dot notation": { - "category": "Message", - "code": 95015 } } diff --git a/src/services/completions.ts b/src/services/completions.ts index 99fa169f58b74..9fb4e46d28d5b 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2062,7 +2062,8 @@ namespace ts.Completions { // TODO: GH#18169 return { name: JSON.stringify(name), needsConvertPropertyAccess: false }; case CompletionKind.PropertyAccess: - return { name, needsConvertPropertyAccess: true }; + // Don't add a completion for a name starting with a space. See https://github.com/Microsoft/TypeScript/pull/20547 + return name.charCodeAt(0) === CharacterCodes.space ? undefined : { name, needsConvertPropertyAccess: true }; case CompletionKind.String: return validIdentiferResult; default: diff --git a/tests/cases/fourslash/completionListInvalidMemberNames_startWithSpace.ts b/tests/cases/fourslash/completionListInvalidMemberNames_startWithSpace.ts new file mode 100644 index 0000000000000..f86c8a5a8132b --- /dev/null +++ b/tests/cases/fourslash/completionListInvalidMemberNames_startWithSpace.ts @@ -0,0 +1,8 @@ +/// + +////declare const x: { " foo": 0, "foo ": 1 }; +////x[|./**/|]; + +const replacementSpan = test.ranges()[0]; +// No completion for " foo" because it starts with a space. See https://github.com/Microsoft/TypeScript/pull/20547 +verify.completionsAt("", [{ name: "foo ", insertText: '["foo "]', replacementSpan }], { includeInsertTextCompletions: true });