diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 09e79549d166f..f5676653553e3 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13414,18 +13414,25 @@ namespace ts { copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.EnumMember); break; case SyntaxKind.ClassExpression: - if ((location).name) { + let className = (location).name; + if (className) { copySymbol(location.symbol, meaning); } - // Fall through + // fall through; this fall-through is necessary because we would like to handle + // type parameter inside class expression similar to how we handle it in classDeclaration and interface Declaration case SyntaxKind.ClassDeclaration: case SyntaxKind.InterfaceDeclaration: + // If we didn't come from static member of class or interface, + // add the type parameters into the symbol table + // (type parameters of classDeclaration/classExpression and interface are in member property of the symbol. + // Note: that the memberFlags come from previous iteration. if (!(memberFlags & NodeFlags.Static)) { copySymbols(getSymbolOfNode(location).members, meaning & SymbolFlags.Type); } break; case SyntaxKind.FunctionExpression: - if ((location).name) { + let funcName = (location).name; + if (funcName) { copySymbol(location.symbol, meaning); } break; @@ -13438,11 +13445,20 @@ namespace ts { copySymbols(globals, meaning); } - // Returns 'true' if we should stop processing symbols. + /** + * Copy the given symbol into symbol tables if the symbol has the given meaning + * and it doesn't already existed in the symbol table + * @param key a key for storing in symbol table; if undefined, use symbol.name + * @param symbol the symbol to be added into symbol table + * @param meaning meaning of symbol to filter by before adding to symbol table + */ function copySymbol(symbol: Symbol, meaning: SymbolFlags): void { if (symbol.flags & meaning) { let id = symbol.name; - if (!isReservedMemberName(id) && !hasProperty(symbols, id)) { + // We will copy all symbol regardless of its reserved name because + // symbolsToArray will check whether the key is a reserved name and + // it will not copy symbol with reserved name to the array + if (!hasProperty(symbols, id)) { symbols[id] = symbol; } } @@ -13451,9 +13467,8 @@ namespace ts { function copySymbols(source: SymbolTable, meaning: SymbolFlags): void { if (meaning) { for (let id in source) { - if (hasProperty(source, id)) { - copySymbol(source[id], meaning); - } + let symbol = source[id]; + copySymbol(symbol, meaning); } } } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 1750b7a524528..108c771ddb80d 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -16,9 +16,11 @@ namespace ts { export function getDeclarationOfKind(symbol: Symbol, kind: SyntaxKind): Declaration { let declarations = symbol.declarations; - for (let declaration of declarations) { - if (declaration.kind === kind) { - return declaration; + if (declarations) { + for (let declaration of declarations) { + if (declaration.kind === kind) { + return declaration; + } } } diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 208237b8b6471..c374f8add60a6 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -704,13 +704,61 @@ module FourSlash { } } - public verifyCompletionListDoesNotContain(symbol: string) { + /** + * Verify that the completion list does NOT contain the given symbol. + * The symbol is considered matched with the symbol in the list if and only if all given parameters must matched. + * When any parameter is omitted, the parameter is ignored during comparison and assumed that the parameter with + * that property of the symbol in the list. + * @param symbol the name of symbol + * @param expectedText the text associated with the symbol + * @param expectedDocumentation the documentation text associated with the symbol + * @param expectedKind the kind of symbol (see ScriptElementKind) + */ + public verifyCompletionListDoesNotContain(symbol: string, expectedText?: string, expectedDocumentation?: string, expectedKind?: string) { + let that = this; + function filterByTextOrDocumentation(entry: ts.CompletionEntry) { + let details = that.getCompletionEntryDetails(entry.name); + let documentation = ts.displayPartsToString(details.documentation); + let text = ts.displayPartsToString(details.displayParts); + if (expectedText && expectedDocumentation) { + return (documentation === expectedDocumentation && text === expectedText) ? true : false; + } + else if (expectedText && !expectedDocumentation) { + return text === expectedText ? true : false; + } + else if (expectedDocumentation && !expectedText) { + return documentation === expectedDocumentation ? true : false; + } + // Because expectedText and expectedDocumentation are undefined, we assume that + // users don't care to compare them so we will treat that entry as if the entry has matching text and documentation + // and keep it in the list of filtered entry. + return true; + } this.scenarioActions.push(''); this.scenarioActions.push(''); var completions = this.getCompletionListAtCaret(); - if (completions && completions.entries.filter(e => e.name === symbol).length !== 0) { - this.raiseError('Completion list did contain ' + symbol); + if (completions) { + let filterCompletions = completions.entries.filter(e => e.name === symbol); + filterCompletions = expectedKind ? filterCompletions.filter(e => e.kind === expectedKind) : filterCompletions; + filterCompletions = filterCompletions.filter(filterByTextOrDocumentation); + if (filterCompletions.length !== 0) { + // After filtered using all present criterion, if there are still symbol left in the list + // then these symbols must meet the criterion for Not supposed to be in the list. So we + // raise an error + let error = "Completion list did contain \'" + symbol + "\'."; + let details = this.getCompletionEntryDetails(filterCompletions[0].name); + if (expectedText) { + error += "Expected text: " + expectedText + " to equal: " + ts.displayPartsToString(details.displayParts) + "."; + } + if (expectedDocumentation) { + error += "Expected documentation: " + expectedDocumentation + " to equal: " + ts.displayPartsToString(details.documentation) + "."; + } + if (expectedKind) { + error += "Expected kind: " + expectedKind + " to equal: " + filterCompletions[0].kind + "." + } + this.raiseError(error); + } } } @@ -2167,7 +2215,7 @@ module FourSlash { var itemsString = items.map((item) => JSON.stringify({ name: item.name, kind: item.kind })).join(",\n"); - this.raiseError('Expected "' + JSON.stringify({ name: name, text: text, documentation: documentation, kind: kind }) + '" to be in list [' + itemsString + ']'); + this.raiseError('Expected "' + JSON.stringify({ name, text, documentation, kind }) + '" to be in list [' + itemsString + ']'); } private findFile(indexOrName: any) { diff --git a/src/services/services.ts b/src/services/services.ts index 94a3345af1fe1..10e38f986130a 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1428,6 +1428,9 @@ namespace ts { // class X {} export const classElement = "class"; + // var x = class X {} + export const localClassElement = "local class"; + // interface Y {} export const interfaceElement = "interface"; @@ -2799,18 +2802,15 @@ namespace ts { program.getGlobalDiagnostics(cancellationToken)); } - /// Completion - function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean): string { - let displayName = symbol.getName(); - if (displayName) { - // If this is the default export, get the name of the declaration if it exists - if (displayName === "default") { - let localSymbol = getLocalSymbolForExportDefault(symbol); - if (localSymbol && localSymbol.name) { - displayName = symbol.valueDeclaration.localSymbol.name; - } - } + /** + * Get the name to be display in completion from a given symbol. + * + * @return undefined if the name is of external module otherwise a name with striped of any quote + */ + function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean, location: Node): string { + let displayName: string = getDeclaredName(program.getTypeChecker(), symbol, location); + if (displayName) { let firstCharCode = displayName.charCodeAt(0); // First check of the displayName is not external module; if it is an external module, it is not valid entry if ((symbol.flags & SymbolFlags.Namespace) && (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) { @@ -2823,37 +2823,38 @@ namespace ts { return getCompletionEntryDisplayName(displayName, target, performCharacterChecks); } - function getCompletionEntryDisplayName(displayName: string, target: ScriptTarget, performCharacterChecks: boolean): string { - if (!displayName) { + /** + * 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): string { + if (!name) { return undefined; } - let firstCharCode = displayName.charCodeAt(0); - if (displayName.length >= 2 && - firstCharCode === displayName.charCodeAt(displayName.length - 1) && - (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote)) { - // 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. - displayName = displayName.substring(1, displayName.length - 1); - } + name = stripQuotes(name); - if (!displayName) { + if (!name) { return undefined; } + // 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) { - if (!isIdentifierStart(displayName.charCodeAt(0), target)) { + if (!isIdentifierStart(name.charCodeAt(0), target)) { return undefined; } - for (let i = 1, n = displayName.length; i < n; i++) { - if (!isIdentifierPart(displayName.charCodeAt(i), target)) { + for (let i = 1, n = name.length; i < n; i++) { + if (!isIdentifierPart(name.charCodeAt(i), target)) { return undefined; } } } - return unescapeIdentifier(displayName); + return name; } function getCompletionData(fileName: string, position: number) { @@ -3607,7 +3608,7 @@ namespace ts { // 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) - let displayName = getCompletionEntryDisplayNameForSymbol(symbol, program.getCompilerOptions().target, /*performCharacterChecks:*/ true); + let displayName = getCompletionEntryDisplayNameForSymbol(symbol, program.getCompilerOptions().target, /*performCharacterChecks:*/ true, location); if (!displayName) { return undefined; } @@ -3664,7 +3665,7 @@ namespace ts { // 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. - let symbol = forEach(symbols, s => getCompletionEntryDisplayNameForSymbol(s, target, /*performCharacterChecks:*/ false) === entryName ? s : undefined); + let symbol = forEach(symbols, s => getCompletionEntryDisplayNameForSymbol(s, target, /*performCharacterChecks:*/ false, location) === entryName ? s : undefined); if (symbol) { let { displayParts, documentation, symbolKind } = getSymbolDisplayPartsDocumentationAndSymbolKind(symbol, getValidSourceFile(fileName), location, location, SemanticMeaning.All); @@ -3697,7 +3698,8 @@ namespace ts { function getSymbolKind(symbol: Symbol, location: Node): string { let flags = symbol.getFlags(); - if (flags & SymbolFlags.Class) return ScriptElementKind.classElement; + if (flags & SymbolFlags.Class) return getDeclarationOfKind(symbol, SyntaxKind.ClassExpression) ? + ScriptElementKind.localClassElement : ScriptElementKind.classElement; if (flags & SymbolFlags.Enum) return ScriptElementKind.enumElement; if (flags & SymbolFlags.TypeAlias) return ScriptElementKind.typeElement; if (flags & SymbolFlags.Interface) return ScriptElementKind.interfaceElement; @@ -3906,7 +3908,16 @@ namespace ts { } } if (symbolFlags & SymbolFlags.Class && !hasAddedSymbolInfo) { - displayParts.push(keywordPart(SyntaxKind.ClassKeyword)); + if (getDeclarationOfKind(symbol, SyntaxKind.ClassExpression)) { + // Special case for class expressions because we would like to indicate that + // the class name is local to the class body (similar to function expression) + // (local class) class + pushTypePart(ScriptElementKind.localClassElement); + } + else { + // Class declaration has name which is not local. + displayParts.push(keywordPart(SyntaxKind.ClassKeyword)); + } displayParts.push(spacePart()); addFullSymbolName(symbol); writeTypeParametersOfSymbol(symbol, sourceFile); @@ -5112,7 +5123,7 @@ namespace ts { // Get the text to search for. // Note: if this is an external module symbol, the name doesn't include quotes. - let declaredName = getDeclaredName(typeChecker, symbol, node); + let declaredName = stripQuotes(getDeclaredName(typeChecker, symbol, node)); // Try to get the smallest valid scope that we can limit our search to; // otherwise we'll need to search globally (i.e. include each file). @@ -5189,10 +5200,10 @@ namespace ts { * a reference to a symbol can occur anywhere. */ function getSymbolScope(symbol: Symbol): Node { - // If this is the symbol of a function expression, then named references - // are limited to its own scope. + // If this is the symbol of a named function expression or named class expression, + // then named references are limited to its own scope. let valueDeclaration = symbol.valueDeclaration; - if (valueDeclaration && valueDeclaration.kind === SyntaxKind.FunctionExpression) { + if (valueDeclaration && (valueDeclaration.kind === SyntaxKind.FunctionExpression || valueDeclaration.kind === SyntaxKind.ClassExpression)) { return valueDeclaration; } @@ -6863,7 +6874,7 @@ namespace ts { } } - let displayName = getDeclaredName(typeChecker, symbol, node); + let displayName = stripQuotes(getDeclaredName(typeChecker, symbol, node)); let kind = getSymbolKind(symbol, node); if (kind) { return { diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 517cb24541a17..5f8859c815e1f 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -667,7 +667,7 @@ namespace ts { let name = typeChecker.symbolToString(localExportDefaultSymbol || symbol); - return stripQuotes(name); + return name; } export function isImportOrExportSpecifierName(location: Node): boolean { @@ -676,9 +676,16 @@ namespace ts { (location.parent).propertyName === location; } + /** + * Strip off existed single quotes or double quotes from a given string + * + * @return non-quoted string + */ export function stripQuotes(name: string) { let length = name.length; - if (length >= 2 && name.charCodeAt(0) === CharacterCodes.doubleQuote && name.charCodeAt(length - 1) === CharacterCodes.doubleQuote) { + if (length >= 2 && + name.charCodeAt(0) === name.charCodeAt(length - 1) && + (name.charCodeAt(0) === CharacterCodes.doubleQuote || name.charCodeAt(0) === CharacterCodes.singleQuote)) { return name.substring(1, length - 1); }; return name; diff --git a/tests/cases/fourslash/completionListForUnicodeEscapeName.ts b/tests/cases/fourslash/completionListForUnicodeEscapeName.ts new file mode 100644 index 0000000000000..afafd18a58b76 --- /dev/null +++ b/tests/cases/fourslash/completionListForUnicodeEscapeName.ts @@ -0,0 +1,27 @@ +/// + +////function \u0042 () { /*0*/ } +////export default function \u0043 () { /*1*/ } +////class \u0041 { /*2*/ } +/////*3*/ + +goTo.marker("0"); +verify.not.completionListContains("B"); +verify.not.completionListContains("\u0042"); + +goTo.marker("2"); +verify.not.completionListContains("C"); +verify.not.completionListContains("\u0043"); + +goTo.marker("2"); +verify.not.completionListContains("A"); +verify.not.completionListContains("\u0041"); + +goTo.marker("3"); +verify.not.completionListContains("B"); +verify.not.completionListContains("\u0042"); +verify.not.completionListContains("A"); +verify.not.completionListContains("\u0041"); +verify.not.completionListContains("C"); +verify.not.completionListContains("\u0043"); + diff --git a/tests/cases/fourslash/completionListInClassExpressionWithTypeParameter.ts b/tests/cases/fourslash/completionListInClassExpressionWithTypeParameter.ts new file mode 100644 index 0000000000000..6f016b73a761a --- /dev/null +++ b/tests/cases/fourslash/completionListInClassExpressionWithTypeParameter.ts @@ -0,0 +1,14 @@ +/// + +//// var x = class myClass { +//// getClassName (){ +//// /*0*/ +//// } +//// prop: Ty/*1*/ +//// } + +goTo.marker("0"); +verify.completionListContains("TypeParam", "(type parameter) TypeParam in myClass", /*documentation*/ undefined, "type parameter"); + +goTo.marker("1"); +verify.completionListContains("TypeParam", "(type parameter) TypeParam in myClass", /*documentation*/ undefined, "type parameter"); \ No newline at end of file diff --git a/tests/cases/fourslash/completionListInNamedClassExpression.ts b/tests/cases/fourslash/completionListInNamedClassExpression.ts new file mode 100644 index 0000000000000..cf0d170f6a503 --- /dev/null +++ b/tests/cases/fourslash/completionListInNamedClassExpression.ts @@ -0,0 +1,14 @@ +/// + +//// var x = class myClass { +//// getClassName (){ +//// m/*0*/ +//// } +//// /*1*/ +//// } + +goTo.marker("0"); +verify.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); + +goTo.marker("1"); +verify.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); \ No newline at end of file diff --git a/tests/cases/fourslash/completionListInNamedClassExpressionWithShadowing.ts b/tests/cases/fourslash/completionListInNamedClassExpressionWithShadowing.ts new file mode 100644 index 0000000000000..e1274e6f59285 --- /dev/null +++ b/tests/cases/fourslash/completionListInNamedClassExpressionWithShadowing.ts @@ -0,0 +1,40 @@ +/// + +//// class myClass { /*0*/ } +//// /*1*/ +//// var x = class myClass { +//// getClassName (){ +//// m/*2*/ +//// } +//// /*3*/ +//// } +//// var y = class { +//// getSomeName() { +//// /*4*/ +//// } +//// /*5*/ +//// } + +goTo.marker("0"); +verify.completionListContains("myClass", "class myClass", /*documentation*/ undefined, "class"); +verify.not.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); + +goTo.marker("1"); +verify.completionListContains("myClass", "class myClass", /*documentation*/ undefined, "class"); +verify.not.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); + +goTo.marker("2"); +verify.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); +verify.not.completionListContains("myClass", "class myClass", /*documentation*/ undefined, "class"); + +goTo.marker("3"); +verify.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); +verify.not.completionListContains("myClass", "class myClass", /*documentation*/ undefined, "class"); + +goTo.marker("4"); +verify.completionListContains("myClass", "class myClass", /*documentation*/ undefined, "class"); +verify.not.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); + +goTo.marker("5"); +verify.completionListContains("myClass", "class myClass", /*documentation*/ undefined, "class"); +verify.not.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); diff --git a/tests/cases/fourslash/completionListInNamedFunctionExpression1.ts b/tests/cases/fourslash/completionListInNamedFunctionExpression1.ts new file mode 100644 index 0000000000000..bb64e04b93b09 --- /dev/null +++ b/tests/cases/fourslash/completionListInNamedFunctionExpression1.ts @@ -0,0 +1,8 @@ +/// + +//// var x = function foo() { +//// /*1*/ +//// } + +goTo.marker("1"); +verify.completionListContains("foo", "(local function) foo(): void", /*documentation*/ undefined, "local function"); \ No newline at end of file diff --git a/tests/cases/fourslash/completionListInNamedFunctionExpressionWithShadowing.ts b/tests/cases/fourslash/completionListInNamedFunctionExpressionWithShadowing.ts new file mode 100644 index 0000000000000..14965253e8439 --- /dev/null +++ b/tests/cases/fourslash/completionListInNamedFunctionExpressionWithShadowing.ts @@ -0,0 +1,22 @@ +/// + +//// function foo() {} +//// /*0*/ +//// var x = function foo() { +//// /*1*/ +//// } +//// var y = function () { +//// /*2*/ +//// } + +goTo.marker("0"); +verify.completionListContains("foo", "function foo(): void", /*documentation*/ undefined, "function"); +verify.not.completionListContains("foo", "(local function) foo(): void", /*documentation*/ undefined, "local function");; + +goTo.marker("1"); +verify.completionListContains("foo", "(local function) foo(): void", /*documentation*/ undefined, "local function"); +verify.not.completionListContains("foo", "function foo(): void", /*documentation*/ undefined, "function");; + +goTo.marker("2"); +verify.completionListContains("foo", "function foo(): void", /*documentation*/ undefined, "function") +verify.not.completionListContains("foo", "(local function) foo(): void", /*documentation*/ undefined, "local function");; \ No newline at end of file diff --git a/tests/cases/fourslash/completionListInvalidMemberNames.ts b/tests/cases/fourslash/completionListInvalidMemberNames.ts index 7fff46e2a815f..0928f8b9d8c24 100644 --- a/tests/cases/fourslash/completionListInvalidMemberNames.ts +++ b/tests/cases/fourslash/completionListInvalidMemberNames.ts @@ -19,7 +19,6 @@ verify.completionListContains("bar"); verify.completionListContains("break"); verify.completionListContains("any"); verify.completionListContains("$"); -verify.completionListContains("b"); // Nothing else should show up -verify.memberListCount(5); +verify.memberListCount(4); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 07a1fc4de4099..a0463073d8c84 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -169,7 +169,7 @@ module FourSlashInterface { // completion list is brought up if necessary public completionListContains(symbol: string, text?: string, documentation?: string, kind?: string) { if (this.negative) { - FourSlash.currentTestState.verifyCompletionListDoesNotContain(symbol); + FourSlash.currentTestState.verifyCompletionListDoesNotContain(symbol, text, documentation, kind); } else { FourSlash.currentTestState.verifyCompletionListContains(symbol, text, documentation, kind); } diff --git a/tests/cases/fourslash/renameLocationsForClassExpression01.ts b/tests/cases/fourslash/renameLocationsForClassExpression01.ts index e7247f8e4ee3d..b95040085c9a7 100644 --- a/tests/cases/fourslash/renameLocationsForClassExpression01.ts +++ b/tests/cases/fourslash/renameLocationsForClassExpression01.ts @@ -9,7 +9,7 @@ //// } //// //// static doItStatically() { -//// return [|Foo|]; +//// return [|Foo|].y; //// } ////} //// @@ -18,15 +18,10 @@ //// return Foo //// } ////} - - -// TODO (yuit): Fix up this test when class expressions are supported. -// Just uncomment the below, remove the marker, and add the -// appropriate ranges in the test itself. +////var z = class Foo {} let ranges = test.ranges() for (let range of ranges) { goTo.position(range.start); - verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); } \ No newline at end of file