Merge pull request #3643 from Microsoft/completionListWithLocalName

Completion list show correct entry for function expression and class expression
This commit is contained in:
Yui 2015-07-10 13:02:47 -07:00
commit e72d0d896a
14 changed files with 264 additions and 62 deletions

View File

@ -13414,18 +13414,25 @@ namespace ts {
copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.EnumMember);
break;
case SyntaxKind.ClassExpression:
if ((<ClassExpression>location).name) {
let className = (<ClassExpression>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 ((<FunctionExpression>location).name) {
let funcName = (<FunctionExpression>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);
}
}
}

View File

@ -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;
}
}
}

View File

@ -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('<ShowCompletionList />');
this.scenarioActions.push('<VerifyCompletionDoesNotContainItem ItemName="' + escapeXmlAttributeValue(symbol) + '" />');
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) {

View File

@ -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 <className>
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 {

View File

@ -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 {
(<ImportOrExportSpecifier>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;

View File

@ -0,0 +1,27 @@
/// <reference path='fourslash.ts' />
////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");

View File

@ -0,0 +1,14 @@
///<reference path="fourslash.ts" />
//// var x = class myClass <TypeParam> {
//// getClassName (){
//// /*0*/
//// }
//// prop: Ty/*1*/
//// }
goTo.marker("0");
verify.completionListContains("TypeParam", "(type parameter) TypeParam in myClass<TypeParam>", /*documentation*/ undefined, "type parameter");
goTo.marker("1");
verify.completionListContains("TypeParam", "(type parameter) TypeParam in myClass<TypeParam>", /*documentation*/ undefined, "type parameter");

View File

@ -0,0 +1,14 @@
///<reference path="fourslash.ts" />
//// 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");

View File

@ -0,0 +1,40 @@
///<reference path="fourslash.ts" />
//// 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");

View File

@ -0,0 +1,8 @@
///<reference path="fourslash.ts" />
//// var x = function foo() {
//// /*1*/
//// }
goTo.marker("1");
verify.completionListContains("foo", "(local function) foo(): void", /*documentation*/ undefined, "local function");

View File

@ -0,0 +1,22 @@
///<reference path="fourslash.ts" />
//// 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");;

View File

@ -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);

View File

@ -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);
}

View File

@ -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);
}