From aab210096bad1da8c8d0311dd2307b4723b76ec0 Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 24 Jun 2015 14:10:02 -0700 Subject: [PATCH 01/20] Add name of function expression into completion list --- src/compiler/checker.ts | 8 ++++++-- src/services/services.ts | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index efee668a24b..7312dd137a2 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -12182,8 +12182,12 @@ namespace ts { } break; case SyntaxKind.FunctionExpression: - if ((location).name) { - copySymbol(location.symbol, meaning); + let name = (location).name; + if (name) { + let symbol = location.symbol; + if (symbol.flags & meaning && !hasProperty(symbols, name.text)) { + symbols[name.text] = symbol; + } } break; } diff --git a/src/services/services.ts b/src/services/services.ts index e11fb32c73b..6a7661e0dfd 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2807,6 +2807,22 @@ namespace ts { } } + // Special case for function expression because despite sometimes having a name, the binder + // binds them to a symbol with the name "__function". However, for completion entry, we want + // to display its declared name rather than "__function". + // var x = function foo () { + // fo$ <- completion list should contain local name "foo" + // } + // foo$ <- completion list should not contain "foo" + if (displayName === "__function") { + displayName = symbol.declarations[0].name.getText(); + + // At this point, we expect that all completion list entries have declared name including function expression + // because when we gather all relevant symbols, we check that the function expression must have declared name + // before adding the symbol into our symbols table. (see: getSymbolsInScope) + Debug.assert(displayName !== undefined,"Expected this function expression to have declared name"); + } + 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)) { From 9c9e29877d03691abe576d67b1b951aa31c6c71c Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 25 Jun 2015 18:21:52 -0700 Subject: [PATCH 02/20] Add test for completion in function expression --- ...ompletionListInNamedFunctionExpression1.ts | 8 ++++++++ ...tInNamedFunctionExpressionWithShadowing.ts | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 tests/cases/fourslash/completionListInNamedFunctionExpression1.ts create mode 100644 tests/cases/fourslash/completionListInNamedFunctionExpressionWithShadowing.ts diff --git a/tests/cases/fourslash/completionListInNamedFunctionExpression1.ts b/tests/cases/fourslash/completionListInNamedFunctionExpression1.ts new file mode 100644 index 00000000000..bb64e04b93b --- /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 00000000000..7d441acb7d7 --- /dev/null +++ b/tests/cases/fourslash/completionListInNamedFunctionExpressionWithShadowing.ts @@ -0,0 +1,19 @@ +/// + +//// function foo() {} +//// /*0*/ +//// var x = function foo() { +//// /*1*/ +//// } +//// var y = function () { +//// /*2*/ +//// } + +goTo.marker("0"); +verify.completionListContains("foo", "function foo(): void", /*documentation*/ undefined, "function"); + +goTo.marker("1"); +verify.completionListContains("foo", "(local function) foo(): void", /*documentation*/ undefined, "local function"); + +goTo.marker("2"); +verify.completionListContains("foo", "function foo(): void", /*documentation*/ undefined, "function") \ No newline at end of file From 34489fa0e092598ddfd64a13eb756cf218b0d0f5 Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 25 Jun 2015 18:22:33 -0700 Subject: [PATCH 03/20] Add test for completion in class expression --- .../completionListInNamedClassExpression.ts | 14 ++++++++ ...ListInNamedClassExpressionWithShadowing.ts | 34 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 tests/cases/fourslash/completionListInNamedClassExpression.ts create mode 100644 tests/cases/fourslash/completionListInNamedClassExpressionWithShadowing.ts diff --git a/tests/cases/fourslash/completionListInNamedClassExpression.ts b/tests/cases/fourslash/completionListInNamedClassExpression.ts new file mode 100644 index 00000000000..cf0d170f6a5 --- /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 00000000000..b1f7f1ca3f0 --- /dev/null +++ b/tests/cases/fourslash/completionListInNamedClassExpressionWithShadowing.ts @@ -0,0 +1,34 @@ +/// + +//// 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"); + +goTo.marker("1"); +verify.completionListContains("myClass", "class myClass", /*documentation*/ undefined, "class"); + +goTo.marker("2"); +verify.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); + +goTo.marker("3"); +verify.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class"); + +goTo.marker("4"); +verify.completionListContains("myClass", "class myClass", /*documentation*/ undefined, "class"); + +goTo.marker("5"); +verify.completionListContains("myClass", "class myClass", /*documentation*/ undefined, "class"); From 5467e1dfbd90d90f611fe7737a4a169673c7563b Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 25 Jun 2015 18:35:01 -0700 Subject: [PATCH 04/20] Support completion in named class expression and named function expression --- src/compiler/checker.ts | 8 ++------ src/services/services.ts | 25 +++++++++++++++++++------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7312dd137a2..ef7f2f7c29b 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -12170,11 +12170,6 @@ namespace ts { case SyntaxKind.EnumDeclaration: copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.EnumMember); break; - case SyntaxKind.ClassExpression: - if ((location).name) { - copySymbol(location.symbol, meaning); - } - // Fall through case SyntaxKind.ClassDeclaration: case SyntaxKind.InterfaceDeclaration: if (!(memberFlags & NodeFlags.Static)) { @@ -12182,7 +12177,8 @@ namespace ts { } break; case SyntaxKind.FunctionExpression: - let name = (location).name; + case SyntaxKind.ClassExpression: + let name = (location).name; if (name) { let symbol = location.symbol; if (symbol.flags & meaning && !hasProperty(symbols, name.text)) { diff --git a/src/services/services.ts b/src/services/services.ts index 6a7661e0dfd..aedf6bac35d 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1423,6 +1423,9 @@ namespace ts { // class X {} export const classElement = "class"; + // var x = class X {} + export const localClassElement = "local class"; + // interface Y {} export const interfaceElement = "interface"; @@ -2807,14 +2810,14 @@ namespace ts { } } - // Special case for function expression because despite sometimes having a name, the binder - // binds them to a symbol with the name "__function". However, for completion entry, we want - // to display its declared name rather than "__function". + // Special case for function expression and class expression because despite sometimes having a name, the binder + // binds them to a symbol with the name "__function" and "__class" respectively. However, for completion entry, we want + // to display its declared name rather than "__function" and "__class". // var x = function foo () { // fo$ <- completion list should contain local name "foo" // } // foo$ <- completion list should not contain "foo" - if (displayName === "__function") { + if (displayName === "__function" || displayName === "__class") { displayName = symbol.declarations[0].name.getText(); // At this point, we expect that all completion list entries have declared name including function expression @@ -3568,7 +3571,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 symbol.declarations[0].kind === 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; @@ -3777,7 +3781,16 @@ namespace ts { } } if (symbolFlags & SymbolFlags.Class && !hasAddedSymbolInfo) { - displayParts.push(keywordPart(SyntaxKind.ClassKeyword)); + // 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 + if (symbol.getName() === "__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); From 45182b88b0e9fcb14bf5a9d6cb9ff5cc2a4b402f Mon Sep 17 00:00:00 2001 From: Yui T Date: Fri, 26 Jun 2015 10:36:39 -0700 Subject: [PATCH 05/20] Add todo comment to use getDeclaredName --- src/services/services.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/services.ts b/src/services/services.ts index aedf6bac35d..0367786d610 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2817,6 +2817,7 @@ namespace ts { // fo$ <- completion list should contain local name "foo" // } // foo$ <- completion list should not contain "foo" + // TODO (yuisu): Use getDeclaredName instead once the functions is rewritten if (displayName === "__function" || displayName === "__class") { displayName = symbol.declarations[0].name.getText(); From 744f640f42cd8fc1411cf8556bcdf5e3d9b4c862 Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 1 Jul 2015 13:16:29 -0700 Subject: [PATCH 06/20] Add comment to clarify why we don't use copySymbol --- src/compiler/checker.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 8df1f8bf808..e1f99882079 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -12680,6 +12680,10 @@ namespace ts { break; case SyntaxKind.FunctionExpression: case SyntaxKind.ClassExpression: + // The reason we are not using copySymbol for function expression and class expression + // is that copySymbol will not copy a symbol into SymbolTable if the symbol has name prefix + // with "__". Therefore, if class expression or function expression have declared name, + // we will add the symbol into the table using its declared name let name = (location).name; if (name) { let symbol = location.symbol; From 514d054cac3955ef25f8e5c5e8153fdd54b05656 Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 1 Jul 2015 13:17:46 -0700 Subject: [PATCH 07/20] Address CR: Use getDeclaredName and getDeclarationOfKind --- src/compiler/utilities.ts | 8 ++++--- src/services/services.ts | 47 ++++++++++++++++----------------------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 750482bc8b0..cee87a95f07 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/services/services.ts b/src/services/services.ts index 099d83b7595..e1dfec82e12 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2801,34 +2801,25 @@ namespace ts { } /// 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; - } - } + function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean, location: Node): string { + let displayName: string; - // Special case for function expression and class expression because despite sometimes having a name, the binder - // binds them to a symbol with the name "__function" and "__class" respectively. However, for completion entry, we want - // to display its declared name rather than "__function" and "__class". - // var x = function foo () { - // fo$ <- completion list should contain local name "foo" - // } - // foo$ <- completion list should not contain "foo" - // TODO (yuisu): Use getDeclaredName instead once the functions is rewritten - if (displayName === "__function" || displayName === "__class") { - displayName = symbol.declarations[0].name.getText(); - - // At this point, we expect that all completion list entries have declared name including function expression - // because when we gather all relevant symbols, we check that the function expression must have declared name - // before adding the symbol into our symbols table. (see: getSymbolsInScope) - Debug.assert(displayName !== undefined,"Expected this function expression to have declared name"); - } + // In the case of default export, function expression and class expression, + // the binder bind them with "default", "__function", "__class" respectively. + // However, for completion entry, we want to display its declared name rather than binder name. + if (getLocalSymbolForExportDefault(symbol) || + getDeclarationOfKind(symbol, SyntaxKind.FunctionExpression) || + getDeclarationOfKind(symbol, SyntaxKind.ClassExpression)) { + let typeChecker = program.getTypeChecker(); + displayName = getDeclaredName(typeChecker, symbol, location); + // At this point, we expect that all completion list entries have declared name including function expression and class expression + // because when we gather all relevant symbols, we check that the function expression and class expression must have declared name + // before adding the symbol into our symbols table. (see: getSymbolsInScope) + Debug.assert(displayName !== undefined, "Expected displayed name from declaration to existed in this symbol: " + symbol.getName()); + } + else { + displayName = symbol.getName(); 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)) { @@ -3534,7 +3525,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; } @@ -3591,7 +3582,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); From 605ab0b1fce65dcf7bb87e6bbe3273e3ad34ddcc Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 1 Jul 2015 13:55:02 -0700 Subject: [PATCH 08/20] Fix rename for class expression --- src/services/services.ts | 6 ++--- .../renameLocationsForClassExpression01.ts | 26 +++++++------------ 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index e1dfec82e12..6e2698c9e6c 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5117,10 +5117,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; } diff --git a/tests/cases/fourslash/renameLocationsForClassExpression01.ts b/tests/cases/fourslash/renameLocationsForClassExpression01.ts index a1174d503ed..6bedc077d4d 100644 --- a/tests/cases/fourslash/renameLocationsForClassExpression01.ts +++ b/tests/cases/fourslash/renameLocationsForClassExpression01.ts @@ -3,13 +3,14 @@ ////class Foo { ////} //// -////var x = class /**/Foo { +//////The class expression Foo +////var x = class [|Foo|] { //// doIt() { -//// return Foo; +//// return [|Foo|]; //// } //// //// static doItStatically() { -//// return Foo; +//// return [|Foo|].y; //// } ////} //// @@ -18,17 +19,10 @@ //// return Foo //// } ////} +////var z = class 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. -goTo.marker(); -verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); - -////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 +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 From 7747ad4aa586e56d463da389448480c9ec89ecdb Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 1 Jul 2015 14:25:43 -0700 Subject: [PATCH 09/20] Clean up stripQuote and add comments --- src/services/services.ts | 36 ++++++++++++++++++++---------------- src/services/utilities.ts | 9 ++++++++- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 6e2698c9e6c..c1c0aee33af 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2800,7 +2800,11 @@ namespace ts { return program.getOptionsDiagnostics().concat(program.getGlobalDiagnostics()); } - /// Completion + /** + * 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; @@ -2832,37 +2836,37 @@ 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) { + // We can simply return name with strip quotes because the name could be an invalid 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 (!name) { return undefined; } 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 unescapeIdentifier(name); } function getCompletionData(fileName: string, position: number) { diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 0fd945caa7d..797cbee12cb 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -675,9 +675,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; From 30796073b408b0d6a5440127fa62cc8832a8da8a Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 1 Jul 2015 14:42:27 -0700 Subject: [PATCH 10/20] Update verification function to be able to test that the only symbol with certain kind, document, and text is the completion list --- src/harness/fourslash.ts | 48 +++++++++++++++++-- ...ListInNamedClassExpressionWithShadowing.ts | 6 +++ ...tInNamedFunctionExpressionWithShadowing.ts | 5 +- tests/cases/fourslash/fourslash.ts | 2 +- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 0717ed68e83..9a07482e60d 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -704,13 +704,55 @@ module FourSlash { } } - public verifyCompletionListDoesNotContain(symbol: string) { + /** + * Verfiy that the completion list does NOT contain the given symbol. If text, or documentation, or kind are provided, + * the list contains the symbol all given parameters must matched. When any parameter is omitted, the parameters is ignored during comparison. + */ + 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); + } } } diff --git a/tests/cases/fourslash/completionListInNamedClassExpressionWithShadowing.ts b/tests/cases/fourslash/completionListInNamedClassExpressionWithShadowing.ts index b1f7f1ca3f0..e1274e6f592 100644 --- a/tests/cases/fourslash/completionListInNamedClassExpressionWithShadowing.ts +++ b/tests/cases/fourslash/completionListInNamedClassExpressionWithShadowing.ts @@ -17,18 +17,24 @@ 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/completionListInNamedFunctionExpressionWithShadowing.ts b/tests/cases/fourslash/completionListInNamedFunctionExpressionWithShadowing.ts index 7d441acb7d7..14965253e84 100644 --- a/tests/cases/fourslash/completionListInNamedFunctionExpressionWithShadowing.ts +++ b/tests/cases/fourslash/completionListInNamedFunctionExpressionWithShadowing.ts @@ -11,9 +11,12 @@ 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") \ No newline at end of file +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/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 07a1fc4de40..a0463073d8c 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); } From 2e0c3908ea94e89204ecee6f1f7f852b5bfb538b Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 1 Jul 2015 14:59:07 -0700 Subject: [PATCH 11/20] Address code review --- src/services/services.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index c1c0aee33af..01500b23812 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -3619,7 +3619,7 @@ namespace ts { function getSymbolKind(symbol: Symbol, location: Node): string { let flags = symbol.getFlags(); - if (flags & SymbolFlags.Class) return symbol.declarations[0].kind === SyntaxKind.ClassExpression ? + 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; @@ -3832,7 +3832,7 @@ namespace ts { // 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 - if (symbol.getName() === "__class") { + if (getDeclarationOfKind(symbol, SyntaxKind.ClassExpression)) { pushTypePart(ScriptElementKind.localClassElement); } else { From 06c9876368a0466c47c322c7f8c153354844333f Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 1 Jul 2015 18:45:58 -0700 Subject: [PATCH 12/20] Address code review --- src/compiler/checker.ts | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e1f99882079..25841884f19 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -12686,10 +12686,7 @@ namespace ts { // we will add the symbol into the table using its declared name let name = (location).name; if (name) { - let symbol = location.symbol; - if (symbol.flags & meaning && !hasProperty(symbols, name.text)) { - symbols[name.text] = symbol; - } + copySymbol(location.symbol, meaning, name.text); } break; } @@ -12701,11 +12698,18 @@ namespace ts { copySymbols(globals, meaning); } - // Returns 'true' if we should stop processing symbols. - function copySymbol(symbol: Symbol, meaning: SymbolFlags): void { + /** + * 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 symbol the symbol to be added into symbol table + * @param meaning meaning of symbol to filter by before adding to symbol table + * @param key a key for storing in symbol table; if null, use symbol.name + */ + function copySymbol(symbol: Symbol, meaning: SymbolFlags, key?: string): void { if (symbol.flags & meaning) { - let id = symbol.name; - if (!isReservedMemberName(id) && !hasProperty(symbols, id)) { + let id = key || symbol.name; + if (!hasProperty(symbols, id)) { symbols[id] = symbol; } } @@ -12714,9 +12718,7 @@ namespace ts { function copySymbols(source: SymbolTable, meaning: SymbolFlags): void { if (meaning) { for (let id in source) { - if (hasProperty(source, id)) { - copySymbol(source[id], meaning); - } + copySymbol(source[id], meaning); } } } From 6da98ce897564ede1e83d7e29934035287887830 Mon Sep 17 00:00:00 2001 From: Yui T Date: Mon, 6 Jul 2015 10:51:43 -0700 Subject: [PATCH 13/20] Fix comments --- src/compiler/checker.ts | 4 ---- src/harness/fourslash.ts | 10 ++++++++-- src/services/services.ts | 13 +++++++------ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 25841884f19..14e2b889ede 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -12680,10 +12680,6 @@ namespace ts { break; case SyntaxKind.FunctionExpression: case SyntaxKind.ClassExpression: - // The reason we are not using copySymbol for function expression and class expression - // is that copySymbol will not copy a symbol into SymbolTable if the symbol has name prefix - // with "__". Therefore, if class expression or function expression have declared name, - // we will add the symbol into the table using its declared name let name = (location).name; if (name) { copySymbol(location.symbol, meaning, name.text); diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 9a07482e60d..2454ebb26c9 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -705,8 +705,14 @@ module FourSlash { } /** - * Verfiy that the completion list does NOT contain the given symbol. If text, or documentation, or kind are provided, - * the list contains the symbol all given parameters must matched. When any parameter is omitted, the parameters is ignored during comparison. + * 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; diff --git a/src/services/services.ts b/src/services/services.ts index 01500b23812..cfbe8f4ee29 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2847,13 +2847,14 @@ namespace ts { name = stripQuotes(name); - // We can simply return name with strip quotes because the name could be an invalid 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 (!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(name.charCodeAt(0), target)) { return undefined; @@ -3829,10 +3830,10 @@ namespace ts { } } if (symbolFlags & SymbolFlags.Class && !hasAddedSymbolInfo) { - // 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 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 { From b01e4d82c338a98077a71d1d2325af5f5ce228c5 Mon Sep 17 00:00:00 2001 From: Yui T Date: Mon, 6 Jul 2015 14:18:47 -0700 Subject: [PATCH 14/20] Address code review --- src/compiler/checker.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 45cd9a7f0c6..171b6eb1837 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13381,7 +13381,7 @@ namespace ts { case SyntaxKind.ClassExpression: let name = (location).name; if (name) { - copySymbol(location.symbol, meaning, name.text); + copySymbol(name.text, location.symbol, meaning); } break; } @@ -13396,12 +13396,11 @@ namespace ts { /** * 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 null, 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 - * @param key a key for storing in symbol table; if null, use symbol.name */ - function copySymbol(symbol: Symbol, meaning: SymbolFlags, key?: string): void { + function copySymbol(key: string, symbol: Symbol, meaning: SymbolFlags): void { if (symbol.flags & meaning) { let id = key || symbol.name; if (!hasProperty(symbols, id)) { @@ -13413,7 +13412,8 @@ namespace ts { function copySymbols(source: SymbolTable, meaning: SymbolFlags): void { if (meaning) { for (let id in source) { - copySymbol(source[id], meaning); + let symbol = source[id]; + copySymbol(symbol.name, symbol, meaning); } } } From f4cd1ac868ee76d4f7c6814be15a561b327a962c Mon Sep 17 00:00:00 2001 From: Yui T Date: Mon, 6 Jul 2015 17:03:33 -0700 Subject: [PATCH 15/20] Address code review, handle type parameter in completion list inside class expression --- src/compiler/checker.ts | 18 ++++++++++++++---- src/harness/fourslash.ts | 2 +- ...onListInClassExpressionWithTypeParameter.ts | 14 ++++++++++++++ 3 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 tests/cases/fourslash/completionListInClassExpressionWithTypeParameter.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 171b6eb1837..ca7df8d9810 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13371,17 +13371,27 @@ namespace ts { case SyntaxKind.EnumDeclaration: copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.EnumMember); break; + case SyntaxKind.ClassExpression: + let className = (location).name; + if (className) { + copySymbol(className.text, location.symbol, meaning); + } + // 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. + // jNote: that the memberFlags come from previous iteration. if (!(memberFlags & NodeFlags.Static)) { copySymbols(getSymbolOfNode(location).members, meaning & SymbolFlags.Type); } break; case SyntaxKind.FunctionExpression: case SyntaxKind.ClassExpression: - let name = (location).name; - if (name) { - copySymbol(name.text, location.symbol, meaning); + let funcName = (location).name; + if (funcName) { + copySymbol(funcName.text, location.symbol, meaning); } break; } @@ -13396,7 +13406,7 @@ namespace ts { /** * 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 null, use symbol.name + * @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 */ diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 2454ebb26c9..475a73ccd7c 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2215,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/tests/cases/fourslash/completionListInClassExpressionWithTypeParameter.ts b/tests/cases/fourslash/completionListInClassExpressionWithTypeParameter.ts new file mode 100644 index 00000000000..6f016b73a76 --- /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 From 0148915daf28d8a4117ffeb047198d72cb69b64b Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 7 Jul 2015 09:18:08 -0700 Subject: [PATCH 16/20] Fix comment --- src/compiler/checker.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index ca7df8d9810..df6796947fc 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13380,9 +13380,10 @@ namespace ts { // 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 + // 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. - // jNote: that the memberFlags come from previous iteration. + // Note: that the memberFlags come from previous iteration. if (!(memberFlags & NodeFlags.Static)) { copySymbols(getSymbolOfNode(location).members, meaning & SymbolFlags.Type); } From b5b1b7b5bb7155fe49e1de7f838258be52425fe0 Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 7 Jul 2015 15:26:01 -0700 Subject: [PATCH 17/20] Use symbol.getName for classExpression and functionExpression since it now correctly represent declared-name. --- src/compiler/checker.ts | 13 ++++++++----- src/services/services.ts | 39 ++++++++++++++++----------------------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 773f1e3f7d0..d18d0faba9b 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13414,7 +13414,7 @@ namespace ts { case SyntaxKind.ClassExpression: let className = (location).name; if (className) { - copySymbol(className.text, location.symbol, meaning); + copySymbol(location.symbol, meaning); } // 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 @@ -13432,7 +13432,7 @@ namespace ts { case SyntaxKind.ClassExpression: let funcName = (location).name; if (funcName) { - copySymbol(funcName.text, location.symbol, meaning); + copySymbol(location.symbol, meaning); } break; } @@ -13451,9 +13451,12 @@ namespace ts { * @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(key: string, symbol: Symbol, meaning: SymbolFlags): void { + function copySymbol(symbol: Symbol, meaning: SymbolFlags): void { if (symbol.flags & meaning) { - let id = key || symbol.name; + let id = symbol.name; + // 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; } @@ -13464,7 +13467,7 @@ namespace ts { if (meaning) { for (let id in source) { let symbol = source[id]; - copySymbol(symbol.name, symbol, meaning); + copySymbol(symbol, meaning); } } } diff --git a/src/services/services.ts b/src/services/services.ts index 8260d41c837..756bb200687 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2808,30 +2808,23 @@ namespace ts { * @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; + let displayName: string = symbol.getName(); - // In the case of default export, function expression and class expression, - // the binder bind them with "default", "__function", "__class" respectively. - // However, for completion entry, we want to display its declared name rather than binder name. - if (getLocalSymbolForExportDefault(symbol) || - getDeclarationOfKind(symbol, SyntaxKind.FunctionExpression) || - getDeclarationOfKind(symbol, SyntaxKind.ClassExpression)) { - let typeChecker = program.getTypeChecker(); - displayName = getDeclaredName(typeChecker, symbol, location); - - // At this point, we expect that all completion list entries have declared name including function expression and class expression - // because when we gather all relevant symbols, we check that the function expression and class expression must have declared name - // before adding the symbol into our symbols table. (see: getSymbolsInScope) - Debug.assert(displayName !== undefined, "Expected displayed name from declaration to existed in this symbol: " + symbol.getName()); - } - else { - displayName = symbol.getName(); - 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)) { - // If the symbol is external module, don't show it in the completion list - // (i.e declare module "http" { let x; } | // <= request completion here, "http" should not be there) - return undefined; + if (displayName) { + if (displayName === "default") { + // In the case of default export, the binder bind them with "default". + // However, for completion entry, we want to display its declared name rather than binder name. + let typeChecker = program.getTypeChecker(); + displayName = getDeclaredName(typeChecker, symbol, location); + } + else { + 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)) { + // If the symbol is external module, don't show it in the completion list + // (i.e declare module "http" { let x; } | // <= request completion here, "http" should not be there) + return undefined; + } } } From d0b8002701495f4f0fbfb4afc34d3f170f519fff Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 9 Jul 2015 16:34:30 -0700 Subject: [PATCH 18/20] Address code review --- src/compiler/checker.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d18d0faba9b..198827c32d4 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13429,7 +13429,6 @@ namespace ts { } break; case SyntaxKind.FunctionExpression: - case SyntaxKind.ClassExpression: let funcName = (location).name; if (funcName) { copySymbol(location.symbol, meaning); From 41bedd2768d1c47e901525b0a9c30911a375e70e Mon Sep 17 00:00:00 2001 From: Yui T Date: Thu, 9 Jul 2015 17:05:57 -0700 Subject: [PATCH 19/20] Use getDecalredName in service layer instead of symbol.getName --- src/services/services.ts | 28 +++++++------------ src/services/utilities.ts | 2 +- .../completionListInvalidMemberNames.ts | 3 +- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 756bb200687..5dc5f8a46db 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2808,23 +2808,15 @@ namespace ts { * @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 = symbol.getName(); + let displayName: string = getDeclaredName(program.getTypeChecker(), symbol, location); if (displayName) { - if (displayName === "default") { - // In the case of default export, the binder bind them with "default". - // However, for completion entry, we want to display its declared name rather than binder name. - let typeChecker = program.getTypeChecker(); - displayName = getDeclaredName(typeChecker, symbol, location); - } - else { - 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)) { - // If the symbol is external module, don't show it in the completion list - // (i.e declare module "http" { let x; } | // <= request completion here, "http" should not be there) - return undefined; - } + 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)) { + // If the symbol is external module, don't show it in the completion list + // (i.e declare module "http" { let x; } | // <= request completion here, "http" should not be there) + return undefined; } } @@ -2862,7 +2854,7 @@ namespace ts { } } - return unescapeIdentifier(name); + return name; } function getCompletionData(fileName: string, position: number) { @@ -5087,7 +5079,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). @@ -6838,7 +6830,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 5c3091000f7..5f8859c815e 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 { diff --git a/tests/cases/fourslash/completionListInvalidMemberNames.ts b/tests/cases/fourslash/completionListInvalidMemberNames.ts index 7fff46e2a81..0928f8b9d8c 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); From d0d1ee918ff4e5ba72c6b350741f57fa723fdbea Mon Sep 17 00:00:00 2001 From: Yui T Date: Fri, 10 Jul 2015 10:15:40 -0700 Subject: [PATCH 20/20] Add test for using unicode escape as function name --- .../completionListForUnicodeEscapeName.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/cases/fourslash/completionListForUnicodeEscapeName.ts diff --git a/tests/cases/fourslash/completionListForUnicodeEscapeName.ts b/tests/cases/fourslash/completionListForUnicodeEscapeName.ts new file mode 100644 index 00000000000..afafd18a58b --- /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"); +