From 01f81ebaf914216b76f8e4a86a03de163e0c83c9 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 27 Jun 2017 08:22:07 -0700 Subject: [PATCH 1/2] When adding completions for a module, don't get the type of the module if not necessary. --- src/compiler/utilities.ts | 4 + src/services/codefixes/importFixes.ts | 5 +- src/services/completions.ts | 82 +++++++++---------- .../completionsNamespaceMergedWithClass.ts | 21 +++++ .../completionsNamespaceMergedWithObject.ts | 16 ++++ 5 files changed, 83 insertions(+), 45 deletions(-) create mode 100644 tests/cases/fourslash/completionsNamespaceMergedWithClass.ts create mode 100644 tests/cases/fourslash/completionsNamespaceMergedWithObject.ts diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 77d1a0c6496c6..fd9351e12d918 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3549,6 +3549,10 @@ namespace ts { } return previous[previous.length - 1]; } + + export function skipAlias(symbol: Symbol, checker: TypeChecker) { + return symbol.flags & SymbolFlags.Alias ? checker.getAliasedSymbol(symbol) : symbol; + } } namespace ts { diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index c2d26da4392d1..5ff9d76e15989 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -222,10 +222,7 @@ namespace ts.codefix { } function getUniqueSymbolId(symbol: Symbol) { - if (symbol.flags & SymbolFlags.Alias) { - return getSymbolId(checker.getAliasedSymbol(symbol)); - } - return getSymbolId(symbol); + return getSymbolId(skipAlias(symbol, checker)); } function checkSymbolHasMeaning(symbol: Symbol, meaning: SemanticMeaning) { diff --git a/src/services/completions.ts b/src/services/completions.ts index 8492c2d8f4091..4cc760c72edae 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -600,58 +600,60 @@ namespace ts.Completions { isNewIdentifierLocation = false; // Since this is qualified name check its a type node location - const isTypeLocation = isPartOfTypeNode(node.parent) || insideJsDocTagTypeExpression; + const isTypeLocation = insideJsDocTagTypeExpression || isPartOfTypeNode(node.parent); const isRhsOfImportDeclaration = isInRightSideOfInternalImportEqualsDeclaration(node); - if (node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.QualifiedName || node.kind === SyntaxKind.PropertyAccessExpression) { + if (isEntityName(node)) { let symbol = typeChecker.getSymbolAtLocation(node); + if (symbol) { + symbol = skipAlias(symbol, typeChecker); + + if (symbol.flags & (SymbolFlags.Module | SymbolFlags.Enum)) { + // Extract module or enum members + const exportedSymbols = typeChecker.getExportsOfModule(symbol); + const isValidValueAccess = (symbol: Symbol) => typeChecker.isValidPropertyAccess((node.parent), symbol.name); + const isValidTypeAccess = (symbol: Symbol) => symbolCanbeReferencedAtTypeLocation(symbol); + const isValidAccess = isRhsOfImportDeclaration ? + // Any kind is allowed when dotting off namespace in internal import equals declaration + (symbol: Symbol) => isValidTypeAccess(symbol) || isValidValueAccess(symbol) : + isTypeLocation ? isValidTypeAccess : isValidValueAccess; + for (const symbol of exportedSymbols) { + if (isValidAccess(symbol)) { + symbols.push(symbol); + } + } - // This is an alias, follow what it aliases - if (symbol && symbol.flags & SymbolFlags.Alias) { - symbol = typeChecker.getAliasedSymbol(symbol); - } - - if (symbol && symbol.flags & SymbolFlags.HasExports) { - // Extract module or enum members - const exportedSymbols = typeChecker.getExportsOfModule(symbol); - const isValidValueAccess = (symbol: Symbol) => typeChecker.isValidPropertyAccess((node.parent), symbol.name); - const isValidTypeAccess = (symbol: Symbol) => symbolCanbeReferencedAtTypeLocation(symbol); - const isValidAccess = isRhsOfImportDeclaration ? - // Any kind is allowed when dotting off namespace in internal import equals declaration - (symbol: Symbol) => isValidTypeAccess(symbol) || isValidValueAccess(symbol) : - isTypeLocation ? isValidTypeAccess : isValidValueAccess; - forEach(exportedSymbols, symbol => { - if (isValidAccess(symbol)) { - symbols.push(symbol); + // If the module is merged with a value, we must get the type of the class and add its propertes (for inherited static methods). + if (!isTypeLocation && symbol.declarations.some(d => d.kind !== SyntaxKind.SourceFile && d.kind !== SyntaxKind.ModuleDeclaration && d.kind !== SyntaxKind.EnumDeclaration)) { + addTypeProperties(typeChecker.getTypeOfSymbolAtLocation(symbol, node)); } - }); + + return; + } } } if (!isTypeLocation) { - const type = typeChecker.getTypeAtLocation(node); - addTypeProperties(type); + addTypeProperties(typeChecker.getTypeAtLocation(node)); } } function addTypeProperties(type: Type) { - if (type) { - // Filter private properties - for (const symbol of type.getApparentProperties()) { - if (typeChecker.isValidPropertyAccess((node.parent), symbol.name)) { - symbols.push(symbol); - } + // Filter private properties + for (const symbol of type.getApparentProperties()) { + if (typeChecker.isValidPropertyAccess((node.parent), symbol.name)) { + symbols.push(symbol); } + } - if (isJavaScriptFile && type.flags & TypeFlags.Union) { - // In javascript files, for union types, we don't just get the members that - // the individual types have in common, we also include all the members that - // each individual type has. This is because we're going to add all identifiers - // anyways. So we might as well elevate the members that were at least part - // of the individual types to a higher status since we know what they are. - const unionType = type; - for (const elementType of unionType.types) { - addTypeProperties(elementType); - } + if (isJavaScriptFile && type.flags & TypeFlags.Union) { + // In javascript files, for union types, we don't just get the members that + // the individual types have in common, we also include all the members that + // each individual type has. This is because we're going to add all identifiers + // anyways. So we might as well elevate the members that were at least part + // of the individual types to a higher status since we know what they are. + const unionType = type; + for (const elementType of unionType.types) { + addTypeProperties(elementType); } } } @@ -813,9 +815,7 @@ namespace ts.Completions { function symbolCanbeReferencedAtTypeLocation(symbol: Symbol): boolean { // This is an alias, follow what it aliases - if (symbol && symbol.flags & SymbolFlags.Alias) { - symbol = typeChecker.getAliasedSymbol(symbol); - } + symbol = skipAlias(symbol, typeChecker); if (symbol.flags & SymbolFlags.Type) { return true; diff --git a/tests/cases/fourslash/completionsNamespaceMergedWithClass.ts b/tests/cases/fourslash/completionsNamespaceMergedWithClass.ts new file mode 100644 index 0000000000000..414a233bdb154 --- /dev/null +++ b/tests/cases/fourslash/completionsNamespaceMergedWithClass.ts @@ -0,0 +1,21 @@ +/// + +////class C { +//// static m() { } +////} +//// +////class D extends C {} +////namespace D { +//// export type T = number; +////} +//// +////let x: D./*type*/; +////D./*value*/ + +goTo.marker("type"); +verify.completionListContains("T"); +verify.not.completionListContains("m"); + +goTo.marker("value"); +verify.not.completionListContains("T"); +verify.completionListContains("m"); diff --git a/tests/cases/fourslash/completionsNamespaceMergedWithObject.ts b/tests/cases/fourslash/completionsNamespaceMergedWithObject.ts new file mode 100644 index 0000000000000..cd799981ee768 --- /dev/null +++ b/tests/cases/fourslash/completionsNamespaceMergedWithObject.ts @@ -0,0 +1,16 @@ +/// + +////namespace N { +//// export type T = number; +////} +////const N = { m() {} }; +////let x: N./*type*/; +////N./*value*/; + +goTo.marker("type"); +verify.completionListContains("T"); +verify.not.completionListContains("m"); + +goTo.marker("value"); +verify.not.completionListContains("T"); +verify.completionListContains("m"); From 93ddecc2a024a29fe641191a5e4bf05c9f7e813d Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 6 Jul 2017 15:16:05 -0700 Subject: [PATCH 2/2] Use SymbolFlags.Module alias --- src/services/completions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 4cc760c72edae..47605d394d23c 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -821,7 +821,7 @@ namespace ts.Completions { return true; } - if (symbol.flags & (SymbolFlags.ValueModule | SymbolFlags.NamespaceModule)) { + if (symbol.flags & SymbolFlags.Module) { const exportedSymbols = typeChecker.getExportsOfModule(symbol); // If the exported symbols contains type, // symbol can be referenced at locations where type is allowed