Miscellaneous fixes to avoid duplicate completions (#20349)

* Miscellaneous fixes to avoid duplicate completions

* Move typeHasCallOrConstructSignatures to utility
This commit is contained in:
Andy 2017-11-30 09:36:17 -08:00 committed by GitHub
parent 192fabf89c
commit 43a35bad2e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 69 additions and 68 deletions

View File

@ -6245,12 +6245,12 @@ namespace ts {
function getAllPossiblePropertiesOfTypes(types: Type[]): Symbol[] {
const unionType = getUnionType(types);
if (!(unionType.flags & TypeFlags.Union)) {
return getPropertiesOfType(unionType);
return getAugmentedPropertiesOfType(unionType);
}
const props = createSymbolTable();
for (const memberType of types) {
for (const { escapedName } of getPropertiesOfType(memberType)) {
for (const { escapedName } of getAugmentedPropertiesOfType(memberType)) {
if (!props.has(escapedName)) {
props.set(escapedName, createUnionOrIntersectionProperty(unionType as UnionType, escapedName));
}
@ -9380,9 +9380,7 @@ namespace ts {
!(target.flags & TypeFlags.Union) &&
!isIntersectionConstituent &&
source !== globalObjectType &&
(getPropertiesOfType(source).length > 0 ||
getSignaturesOfType(source, SignatureKind.Call).length > 0 ||
getSignaturesOfType(source, SignatureKind.Construct).length > 0) &&
(getPropertiesOfType(source).length > 0 || typeHasCallOrConstructSignatures(source)) &&
isWeakType(target) &&
!hasCommonProperties(source, target)) {
if (reportErrors) {
@ -10770,8 +10768,7 @@ namespace ts {
*/
function isObjectTypeWithInferableIndex(type: Type) {
return type.symbol && (type.symbol.flags & (SymbolFlags.ObjectLiteral | SymbolFlags.TypeLiteral | SymbolFlags.ValueModule)) !== 0 &&
getSignaturesOfType(type, SignatureKind.Call).length === 0 &&
getSignaturesOfType(type, SignatureKind.Construct).length === 0;
!typeHasCallOrConstructSignatures(type);
}
function createSymbolWithType(source: Symbol, type: Type) {
@ -18351,10 +18348,7 @@ namespace ts {
error(left, Diagnostics.The_left_hand_side_of_an_instanceof_expression_must_be_of_type_any_an_object_type_or_a_type_parameter);
}
// NOTE: do not raise error if right is unknown as related error was already reported
if (!(isTypeAny(rightType) ||
getSignaturesOfType(rightType, SignatureKind.Call).length ||
getSignaturesOfType(rightType, SignatureKind.Construct).length ||
isTypeSubtypeOf(rightType, globalFunctionType))) {
if (!(isTypeAny(rightType) || typeHasCallOrConstructSignatures(rightType) || isTypeSubtypeOf(rightType, globalFunctionType))) {
error(right, Diagnostics.The_right_hand_side_of_an_instanceof_expression_must_be_of_type_any_or_of_a_type_assignable_to_the_Function_interface_type);
}
return booleanType;
@ -24431,7 +24425,7 @@ namespace ts {
function getAugmentedPropertiesOfType(type: Type): Symbol[] {
type = getApparentType(type);
const propsByName = createSymbolTable(getPropertiesOfType(type));
if (getSignaturesOfType(type, SignatureKind.Call).length || getSignaturesOfType(type, SignatureKind.Construct).length) {
if (typeHasCallOrConstructSignatures(type)) {
forEach(getPropertiesOfType(globalFunctionType), p => {
if (!propsByName.has(p.escapedName)) {
propsByName.set(p.escapedName, p);
@ -24441,6 +24435,10 @@ namespace ts {
return getNamedMembers(propsByName);
}
function typeHasCallOrConstructSignatures(type: Type): boolean {
return ts.typeHasCallOrConstructSignatures(type, checker);
}
function getRootSymbols(symbol: Symbol): Symbol[] {
if (getCheckFlags(symbol) & CheckFlags.Synthetic) {
const symbols: Symbol[] = [];

View File

@ -1969,6 +1969,11 @@ namespace ts {
return isKeyword(token) && !isContextualKeyword(token);
}
export function isStringANonContextualKeyword(name: string) {
const token = stringToToken(name);
return token !== undefined && isNonContextualKeyword(token);
}
export function isTrivia(token: SyntaxKind) {
return SyntaxKind.FirstTriviaToken <= token && token <= SyntaxKind.LastTriviaToken;
}
@ -3664,6 +3669,10 @@ namespace ts {
directory = parentPath;
}
}
export function typeHasCallOrConstructSignatures(type: Type, checker: TypeChecker) {
return checker.getSignaturesOfType(type, SignatureKind.Call).length !== 0 || checker.getSignaturesOfType(type, SignatureKind.Construct).length !== 0;
}
}
namespace ts {

View File

@ -3081,7 +3081,7 @@ Actual: ${stringify(fullActual)}`);
const itemsString = items.map(item => stringify({ name: item.name, source: item.source, kind: item.kind })).join(",\n");
this.raiseError(`Expected "${stringify({ entryId, text, documentation, kind })}" to be in list [${itemsString}]`);
}
else if (matchingItems.length > 1 && !(options && options.allowDuplicate)) {
else if (matchingItems.length > 1) {
this.raiseError(`Found duplicate completion items for ${stringify(entryId)}`);
}
const item = matchingItems[0];
@ -4558,7 +4558,6 @@ namespace FourSlashInterface {
export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {
sourceDisplay: string;
allowDuplicate: boolean; // TODO: GH#20042
}
export interface NewContentOptions {

View File

@ -814,7 +814,6 @@ namespace ts.codefix {
lastCharWasValid = isValid;
}
// Need `|| "_"` to ensure result isn't empty.
const token = stringToToken(res);
return token === undefined || !isNonContextualKeyword(token) ? res || "_" : `_${res}`;
return !isStringANonContextualKeyword(res) ? res || "_" : `_${res}`;
}
}

View File

@ -112,7 +112,7 @@ namespace ts.Completions {
}
const realName = unescapeLeadingUnderscores(name);
if (uniqueNames.has(realName)) {
if (uniqueNames.has(realName) || isStringANonContextualKeyword(realName)) {
return;
}
@ -552,8 +552,6 @@ namespace ts.Completions {
options: GetCompletionsAtPositionOptions,
target: ScriptTarget,
): CompletionData | undefined {
const isJavaScriptFile = isSourceFileJavaScript(sourceFile);
let request: Request | undefined;
let start = timestamp();
@ -832,23 +830,21 @@ namespace ts.Completions {
}
}
function addTypeProperties(type: Type) {
// Filter private properties
for (const symbol of type.getApparentProperties()) {
if (typeChecker.isValidPropertyAccess(<PropertyAccessExpression>(node.parent), symbol.name)) {
symbols.push(symbol);
}
}
if (isJavaScriptFile && type.flags & TypeFlags.Union) {
function addTypeProperties(type: Type): void {
if (isSourceFileJavaScript(sourceFile)) {
// 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
// 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 = <UnionType>type;
for (const elementType of unionType.types) {
addTypeProperties(elementType);
symbols.push(...getPropertiesForCompletion(type, typeChecker, /*isForAccess*/ true));
}
else {
// Filter private properties
for (const symbol of type.getApparentProperties()) {
if (typeChecker.isValidPropertyAccess(<PropertyAccessExpression>(node.parent), symbol.name)) {
symbols.push(symbol);
}
}
}
}
@ -1239,7 +1235,7 @@ namespace ts.Completions {
isNewIdentifierLocation = true;
const typeForObject = typeChecker.getContextualType(<ObjectLiteralExpression>objectLikeContainer);
if (!typeForObject) return false;
typeMembers = getPropertiesForCompletion(typeForObject, typeChecker);
typeMembers = getPropertiesForCompletion(typeForObject, typeChecker, /*isForAccess*/ false);
existingMembers = (<ObjectLiteralExpression>objectLikeContainer).properties;
}
else {
@ -1952,6 +1948,8 @@ namespace ts.Completions {
function getAllKeywordCompletions() {
const allKeywordsCompletions: CompletionEntry[] = [];
for (let i = SyntaxKind.FirstKeyword; i <= SyntaxKind.LastKeyword; i++) {
// "undefined" is a global variable, so don't need a keyword completion for it.
if (i === SyntaxKind.UndefinedKeyword) continue;
allKeywordsCompletions.push({
name: tokenToString(i),
kind: ScriptElementKind.keyword,
@ -2050,14 +2048,15 @@ namespace ts.Completions {
* tries to only include those types which declare properties, not methods.
* This ensures that we don't try providing completions for all the methods on e.g. Array.
*/
function getPropertiesForCompletion(type: Type, checker: TypeChecker): Symbol[] {
function getPropertiesForCompletion(type: Type, checker: TypeChecker, isForAccess: boolean): Symbol[] {
if (!(type.flags & TypeFlags.Union)) {
return checker.getPropertiesOfType(type);
return type.getApparentProperties();
}
const { types } = type as UnionType;
const filteredTypes = types.filter(memberType => !(memberType.flags & TypeFlags.Primitive || checker.isArrayLikeType(memberType)));
// If there are no property-only types, just provide completions for every type as usual.
// If we're providing completions for an object literal, skip primitive, array-like, or callable types since those shouldn't be implemented by object literals.
const filteredTypes = isForAccess ? types : types.filter(memberType =>
!(memberType.flags & TypeFlags.Primitive || checker.isArrayLikeType(memberType) || typeHasCallOrConstructSignatures(memberType, checker)));
return checker.getAllPossiblePropertiesOfTypes(filteredTypes);
}
}

View File

@ -138,39 +138,36 @@ namespace ts.Completions.PathCompletions {
function getCompletionEntriesForNonRelativeModules(fragment: string, scriptPath: string, span: TextSpan, compilerOptions: CompilerOptions, host: LanguageServiceHost, typeChecker: TypeChecker): CompletionEntry[] {
const { baseUrl, paths } = compilerOptions;
let result: CompletionEntry[];
const result: CompletionEntry[] = [];
const fileExtensions = getSupportedExtensions(compilerOptions);
if (baseUrl) {
const projectDir = compilerOptions.project || host.getCurrentDirectory();
const absolute = isRootedDiskPath(baseUrl) ? baseUrl : combinePaths(projectDir, baseUrl);
result = getCompletionEntriesForDirectoryFragment(fragment, normalizePath(absolute), fileExtensions, /*includeExtensions*/ false, span, host);
getCompletionEntriesForDirectoryFragment(fragment, normalizePath(absolute), fileExtensions, /*includeExtensions*/ false, span, host, /*exclude*/ undefined, result);
if (paths) {
for (const path in paths) {
if (paths.hasOwnProperty(path)) {
if (path === "*") {
if (paths[path]) {
for (const pattern of paths[path]) {
for (const match of getModulesForPathsPattern(fragment, baseUrl, pattern, fileExtensions, host)) {
result.push(createCompletionEntryForModule(match, ScriptElementKind.externalModuleName, span));
}
}
}
}
else if (startsWith(path, fragment)) {
const entry = paths[path] && paths[path].length === 1 && paths[path][0];
if (entry) {
result.push(createCompletionEntryForModule(path, ScriptElementKind.externalModuleName, span));
}
for (const path in paths) {
if (!paths.hasOwnProperty(path)) continue;
const patterns = paths[path];
if (!patterns) continue;
if (path === "*") {
for (const pattern of patterns) {
for (const match of getModulesForPathsPattern(fragment, baseUrl, pattern, fileExtensions, host)) {
// Path mappings may provide a duplicate way to get to something we've already added, so don't add again.
if (result.some(entry => entry.name === match)) continue;
result.push(createCompletionEntryForModule(match, ScriptElementKind.externalModuleName, span));
}
}
}
else if (startsWith(path, fragment)) {
if (patterns.length === 1) {
if (result.some(entry => entry.name === path)) continue;
result.push(createCompletionEntryForModule(path, ScriptElementKind.externalModuleName, span));
}
}
}
}
else {
result = [];
}
if (compilerOptions.moduleResolution === ts.ModuleResolutionKind.NodeJs) {
forEachAncestorDirectory(scriptPath, ancestor => {

View File

@ -51,5 +51,5 @@ for (const kind of kinds) {
verify.completionListContains("1test");
goTo.marker(kind + "2");
verify.completionListContains("2test", undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
verify.completionListContains("2test");
}

View File

@ -6,5 +6,5 @@
////var f = function () { return new/**/; }
goTo.marker();
verify.completionListCount(118);
verify.completionListContains('new', undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
verify.completionListCount(116);
verify.completionListContains('new');

View File

@ -5,5 +5,5 @@
////var f = function (s) { return this/**/; }
goTo.marker();
verify.completionListCount(119);
verify.completionListContains('this', undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
verify.completionListCount(117);
verify.completionListContains('this')

View File

@ -8,5 +8,5 @@ verify.completionListContains("boolean");
verify.completionListContains("null");
verify.completionListContains("number");
verify.completionListContains("string");
verify.completionListContains("undefined", undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
verify.completionListContains("undefined");
verify.completionListContains("void");

View File

@ -148,7 +148,7 @@ declare namespace FourSlashInterface {
kind?: string,
spanIndex?: number,
hasAction?: boolean,
options?: { includeExternalModuleExports?: boolean, sourceDisplay?: string, allowDuplicate?: boolean },
options?: { includeExternalModuleExports?: boolean, sourceDisplay?: string },
): void;
completionListItemsCountIsGreaterThan(count: number): void;
completionListIsEmpty(): void;

View File

@ -7,4 +7,4 @@
////v./**/
goTo.marker();
verify.completionListContains("valueOf", /*displayText:*/ undefined, /*documentation*/ undefined, "method", undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
verify.completionListContains("valueOf", /*displayText:*/ undefined, /*documentation*/ undefined, "method");