Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
41913b7
skeleton of new feature
gabritto Feb 23, 2022
d5038ce
working prototype
gabritto Feb 25, 2022
fca74c1
refactor print and format code into its own function
gabritto Feb 25, 2022
249c3ba
minor changes; don't support overloads
gabritto Mar 1, 2022
ef15f81
have two completion entries
gabritto Mar 2, 2022
1f26a52
get rid of accessor support
gabritto Mar 4, 2022
1c28090
add snippet support
gabritto Mar 4, 2022
e628590
add formatting
gabritto Mar 4, 2022
515fc73
add trailing comma
gabritto Mar 4, 2022
0229a7d
add sourcedisplay
gabritto Mar 5, 2022
9892dc8
support auto-imports via completion details
gabritto Mar 5, 2022
8550bba
add user preference option and fix ordering of entries
gabritto Mar 7, 2022
78c7d9e
cleanup
gabritto Mar 7, 2022
1f67d97
don't return code actions for no import fixes
gabritto Mar 8, 2022
6c96290
Merge branch 'main' into gabritto/issue46590
gabritto Mar 8, 2022
814561f
make sortText lower priority for snippets
gabritto Mar 9, 2022
088904e
get rid of flag
gabritto Mar 9, 2022
4e63276
use optional member sort text
gabritto Mar 9, 2022
474d9a9
update baselines
gabritto Mar 9, 2022
b5d05c0
don't collect method symbols if insert text is not supported
gabritto Mar 9, 2022
6914576
remove comment
gabritto Mar 16, 2022
0e0ae05
return undefined if type is not function type
gabritto Mar 17, 2022
36eac73
only slice if needed
gabritto Mar 17, 2022
b9cb703
use union reduction; more test cases
gabritto Mar 17, 2022
7c9cbdb
WIP: modify sort text system
gabritto Mar 21, 2022
07c4fcf
Improve new sort text system
gabritto Mar 22, 2022
5b0d86d
add signature and union type check
gabritto Mar 22, 2022
b032aa5
Merge branch 'main' into gabritto/issue46590
gabritto Mar 23, 2022
08a55c3
re-add flag
gabritto Mar 23, 2022
6ebe361
fix tests
gabritto Mar 23, 2022
411bc18
rename sort text helper
gabritto Mar 23, 2022
1ebdf3d
fix test and code for union case
gabritto Mar 24, 2022
5477408
add new flag to protocol type
gabritto Mar 24, 2022
af7de76
fix spaces
gabritto Mar 24, 2022
1b884c4
CR: minor fixes
gabritto Mar 29, 2022
3820f32
CR: more fixes
gabritto Mar 29, 2022
e7a51e6
CR: restructure main flow
gabritto Mar 29, 2022
203aab0
minor fix
gabritto Mar 29, 2022
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
WIP: modify sort text system
  • Loading branch information
gabritto committed Mar 21, 2022
commit 7c9cbdbfc2b5faafdf1f31a9b3e324685ea291f9
4 changes: 2 additions & 2 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ namespace FourSlashInterface {
name,
kind: "function",
kindModifiers: "deprecated,declare",
sortText: SortText.DeprecatedGlobalsOrKeywords
sortText: "z15" as SortText,
});
const varEntry = (name: string): ExpectedCompletionEntryObject => ({
name,
Expand Down Expand Up @@ -992,7 +992,7 @@ namespace FourSlashInterface {
name,
kind: "method",
kindModifiers: "deprecated,declare",
sortText: SortText.DeprecatedLocationPriority
sortText: "z11" as SortText,
});
const propertyEntry = (name: string): ExpectedCompletionEntryObject => ({
name,
Expand Down
95 changes: 50 additions & 45 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,6 @@ namespace ts.Completions {
GlobalsOrKeywords = "15",
AutoImportSuggestions = "16",
JavascriptIdentifiers = "17",
DeprecatedLocalDeclarationPriority = "18",
DeprecatedLocationPriority = "19",
DeprecatedOptionalMember = "20",
DeprecatedMemberDeclaredBySpreadAssignment = "21",
DeprecatedSuggestedClassMembers = "22",
DeprecatedGlobalsOrKeywords = "23",
DeprecatedAutoImportSuggestions = "24"
}

const enum SortTextId {
LocalDeclarationPriority = 10,
LocationPriority = 11,
OptionalMember = 12,
MemberDeclaredBySpreadAssignment = 13,
SuggestedClassMembers = 14,
GlobalsOrKeywords = 15,
AutoImportSuggestions = 16,

// Don't use these directly.
_JavaScriptIdentifiers = 17,
_DeprecatedStart = 18,
_First = LocalDeclarationPriority,

DeprecatedOffset = _DeprecatedStart - _First,
}

/**
Expand Down Expand Up @@ -157,8 +133,8 @@ namespace ts.Completions {
*/
type SymbolOriginInfoMap = Record<number, SymbolOriginInfo>;

/** Map from symbol id -> SortTextId. */
type SymbolSortTextIdMap = (SortTextId | undefined)[];
/** Map from symbol id -> SortText. */
type SymbolSortTextIdMap = (SortText | undefined)[];

const enum KeywordCompletionFilters {
None, // No keywords
Expand Down Expand Up @@ -288,7 +264,7 @@ namespace ts.Completions {
return getLabelCompletionAtPosition(previousToken.parent);
}

const completionData = getCompletionData(program, log, sourceFile, isUncheckedFile(sourceFile, compilerOptions), position, preferences, /*detailsEntryId*/ undefined, host, cancellationToken);
const completionData = getCompletionData(program, log, sourceFile, compilerOptions, position, preferences, /*detailsEntryId*/ undefined, host, cancellationToken);
if (!completionData) {
return undefined;
}
Expand Down Expand Up @@ -791,7 +767,7 @@ namespace ts.Completions {
}
({ insertText, isSnippet, importAdder, sourceDisplay } = entry);
source = CompletionSource.ObjectLiteralMethodSnippet;
sortText = SortText.OptionalMember;
sortText = sortText + "1" as SortText;
if (importAdder.hasFixes()) {
hasAction = true;
}
Expand Down Expand Up @@ -1396,8 +1372,8 @@ namespace ts.Completions {
}

const { name, needsConvertPropertyAccess } = info;
const sortTextId = symbolToSortTextIdMap?.[getSymbolId(symbol)] ?? SortTextId.LocationPriority;
const sortText = (isDeprecated(symbol, typeChecker) ? SortTextId.DeprecatedOffset + sortTextId : sortTextId).toString() as SortText;
const originalSortText = symbolToSortTextIdMap?.[getSymbolId(symbol)] ?? SortText.LocationPriority;
const sortText = (isDeprecated(symbol, typeChecker) ? "z" + originalSortText : originalSortText) as SortText;
const entry = createCompletionEntry(
symbol,
sortText,
Expand Down Expand Up @@ -1465,9 +1441,9 @@ namespace ts.Completions {
// Auto Imports are not available for scripts so this conditional is always false
if (!!sourceFile.externalModuleIndicator
&& !compilerOptions.allowUmdGlobalAccess
&& symbolToSortTextIdMap[getSymbolId(symbol)] === SortTextId.GlobalsOrKeywords
&& (symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortTextId.AutoImportSuggestions
|| symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortTextId.LocationPriority)) {
&& symbolToSortTextIdMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords
&& (symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions
|| symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortText.LocationPriority)) {
return false;
}

Expand Down Expand Up @@ -1559,7 +1535,7 @@ namespace ts.Completions {
}

const compilerOptions = program.getCompilerOptions();
const completionData = getCompletionData(program, log, sourceFile, isUncheckedFile(sourceFile, compilerOptions), position, { includeCompletionsForModuleExports: true, includeCompletionsWithInsertText: true }, entryId, host);
const completionData = getCompletionData(program, log, sourceFile, compilerOptions, position, { includeCompletionsForModuleExports: true, includeCompletionsWithInsertText: true }, entryId, host);
if (!completionData) {
return { type: "none" };
}
Expand Down Expand Up @@ -1892,15 +1868,15 @@ namespace ts.Completions {
program: Program,
log: (message: string) => void,
sourceFile: SourceFile,
isUncheckedFile: boolean,
compilerOptions: CompilerOptions,
position: number,
preferences: UserPreferences,
detailsEntryId: CompletionEntryIdentifier | undefined,
host: LanguageServiceHost,
cancellationToken?: CancellationToken,
): CompletionData | Request | undefined {
const typeChecker = program.getTypeChecker();

const inUncheckedFile = isUncheckedFile(sourceFile, compilerOptions);
let start = timestamp();
let currentToken = getTokenAtPosition(sourceFile, position); // TODO: GH#15853
// We will check for jsdoc comments with insideComment and getJsDocTagAtPosition. (TODO: that seems rather inefficient to check the same thing so many times.)
Expand Down Expand Up @@ -2350,7 +2326,7 @@ namespace ts.Completions {
}

const propertyAccess = node.kind === SyntaxKind.ImportType ? node as ImportTypeNode : node.parent as PropertyAccessExpression | QualifiedName;
if (isUncheckedFile) {
if (inUncheckedFile) {
// 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
Expand Down Expand Up @@ -2437,7 +2413,7 @@ namespace ts.Completions {

function addSymbolSortInfo(symbol: Symbol) {
if (isStaticProperty(symbol)) {
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.LocalDeclarationPriority;
symbolToSortTextIdMap[getSymbolId(symbol)] = SortText.LocalDeclarationPriority;
}
}

Expand Down Expand Up @@ -2557,7 +2533,7 @@ namespace ts.Completions {
const symbol = symbols[i];
if (!typeChecker.isArgumentsSymbol(symbol) &&
!some(symbol.declarations, d => d.getSourceFile() === sourceFile)) {
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.GlobalsOrKeywords;
symbolToSortTextIdMap[getSymbolId(symbol)] = SortText.GlobalsOrKeywords;
}
if (typeOnlyAliasNeedsPromotion && !(symbol.flags & SymbolFlags.Value)) {
const typeOnlyAliasDeclaration = symbol.declarations && find(symbol.declarations, isTypeOnlyImportOrExportDeclaration);
Expand All @@ -2575,7 +2551,7 @@ namespace ts.Completions {
for (const symbol of getPropertiesForCompletion(thisType, typeChecker)) {
symbolToOriginInfoMap[symbols.length] = { kind: SymbolOriginInfoKind.ThisType };
symbols.push(symbol);
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.SuggestedClassMembers;
symbolToSortTextIdMap[getSymbolId(symbol)] = SortText.SuggestedClassMembers;
}
}
}
Expand Down Expand Up @@ -2762,12 +2738,12 @@ namespace ts.Completions {

function pushAutoImportSymbol(symbol: Symbol, origin: SymbolOriginInfoResolvedExport | SymbolOriginInfoExport) {
const symbolId = getSymbolId(symbol);
if (symbolToSortTextIdMap[symbolId] === SortTextId.GlobalsOrKeywords) {
if (symbolToSortTextIdMap[symbolId] === SortText.GlobalsOrKeywords) {
// If an auto-importable symbol is available as a global, don't add the auto import
return;
}
symbolToOriginInfoMap[symbols.length] = origin;
symbolToSortTextIdMap[symbolId] = importCompletionNode ? SortTextId.LocationPriority : SortTextId.AutoImportSuggestions;
symbolToSortTextIdMap[symbolId] = importCompletionNode ? SortText.LocationPriority : SortText.AutoImportSuggestions;
symbols.push(symbol);
}

Expand Down Expand Up @@ -3050,6 +3026,10 @@ namespace ts.Completions {
}
}
setSortTextToOptionalMember();
if (objectLikeContainer.kind === SyntaxKind.ObjectLiteralExpression && preferences.includeCompletionsWithInsertText) {
// >> TODO: specify `symbols` range?
transformObjectLiteralMembersSortText();
}

return GlobalsSearch.Success;
}
Expand Down Expand Up @@ -3131,7 +3111,7 @@ namespace ts.Completions {
localsContainer.locals?.forEach((symbol, name) => {
symbols.push(symbol);
if (localsContainer.symbol?.exports?.has(name)) {
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.OptionalMember;
symbolToSortTextIdMap[getSymbolId(symbol)] = SortText.OptionalMember;
}
});
return GlobalsSearch.Success;
Expand Down Expand Up @@ -3571,7 +3551,7 @@ namespace ts.Completions {
symbols.forEach(m => {
if (m.flags & SymbolFlags.Optional) {
const symbolId = getSymbolId(m);
symbolToSortTextIdMap[symbolId] = symbolToSortTextIdMap[symbolId] ?? SortTextId.OptionalMember;
symbolToSortTextIdMap[symbolId] = symbolToSortTextIdMap[symbolId] ?? SortText.OptionalMember;
}
});
}
Expand All @@ -3583,7 +3563,32 @@ namespace ts.Completions {
}
for (const contextualMemberSymbol of contextualMemberSymbols) {
if (membersDeclaredBySpreadAssignment.has(contextualMemberSymbol.name)) {
symbolToSortTextIdMap[getSymbolId(contextualMemberSymbol)] = SortTextId.MemberDeclaredBySpreadAssignment;
symbolToSortTextIdMap[getSymbolId(contextualMemberSymbol)] = SortText.MemberDeclaredBySpreadAssignment;
}
}
}

function transformObjectLiteralMembersSortText(): void {
const pastSymbolIds: Set<number> = new Set();
for (let i = 0; i < symbols.length; i++) {
const symbol = symbols[i];
const symbolId = getSymbolId(symbol);
if (pastSymbolIds.has(symbolId)) {
continue;
}
pastSymbolIds.add(symbolId);
const origin = symbolToOriginInfoMap?.[i];
const target = getEmitScriptTarget(compilerOptions);
const displayName = getCompletionEntryDisplayNameForSymbol(
symbol,
target,
origin,
CompletionKind.ObjectPropertyDeclaration,
/*jsxIdentifierExpected*/ false);
if (displayName) {
const originalSortText = symbolToSortTextIdMap[symbolId] ?? SortText.LocationPriority;
const { name } = displayName;
symbolToSortTextIdMap[symbolId] = `${originalSortText}\0${name}\0` as SortText;
}
}
}
Expand Down
22 changes: 11 additions & 11 deletions tests/cases/fourslash/completionsObjectLiteralMethod1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ verify.completions({
includes: [
{
name: "bar",
sortText: completion.SortText.LocationPriority,
sortText: `${completion.SortText.LocationPriority}\0bar\0` as completion.SortText,
insertText: undefined,
},
{
name: "bar",
sortText: completion.SortText.OptionalMember,
sortText: `${completion.SortText.LocationPriority}\0bar\0${1}` as completion.SortText,
source: completion.CompletionSource.ObjectLiteralMethodSnippet,
insertText: "bar(x: number): void {\n},",
},
Expand All @@ -61,23 +61,23 @@ verify.completions({
includes: [
{
name: "bar",
sortText: completion.SortText.LocationPriority,
sortText: `${completion.SortText.LocationPriority}\0bar\0` as completion.SortText,
insertText: undefined,
},
{
name: "bar",
sortText: completion.SortText.OptionalMember,
sortText: `${completion.SortText.LocationPriority}\0bar\0${1}` as completion.SortText,
source: completion.CompletionSource.ObjectLiteralMethodSnippet,
insertText: "bar(x: number): void {\n},",
},
{
name: "foo",
sortText: completion.SortText.LocationPriority,
sortText: `${completion.SortText.LocationPriority}\0foo\0` as completion.SortText,
insertText: undefined,
},
{
name: "foo",
sortText: completion.SortText.OptionalMember,
sortText: `${completion.SortText.LocationPriority}\0foo\0${1}` as completion.SortText,
source: completion.CompletionSource.ObjectLiteralMethodSnippet,
insertText: "foo(x: string): string {\n},",
},
Expand All @@ -92,7 +92,7 @@ verify.completions({
exact: [
{
name: "buzz",
sortText: completion.SortText.LocationPriority,
sortText: `${completion.SortText.LocationPriority}\0buzz\0` as completion.SortText,
// no declaration insert text, because this property has overloads
insertText: undefined,
},
Expand All @@ -107,12 +107,12 @@ verify.completions({
includes: [
{
name: "\"space bar\"",
sortText: completion.SortText.LocationPriority,
sortText: `${completion.SortText.LocationPriority}\0${"\"space bar\""}\0` as completion.SortText,
insertText: undefined,
},
{
name: "\"space bar\"",
sortText: completion.SortText.OptionalMember,
sortText: `${completion.SortText.LocationPriority}\0${"\"space bar\""}\0${1}` as completion.SortText,
source: completion.CompletionSource.ObjectLiteralMethodSnippet,
insertText: "\"space bar\"(): string {\n},",
},
Expand All @@ -127,12 +127,12 @@ verify.completions({
includes: [
{
name: "bar",
sortText: completion.SortText.LocationPriority,
sortText: `${completion.SortText.LocationPriority}\0bar\0` as completion.SortText,
insertText: undefined,
},
{
name: "bar",
sortText: completion.SortText.OptionalMember,
sortText: `${completion.SortText.LocationPriority}\0bar\0${1}` as completion.SortText,
source: completion.CompletionSource.ObjectLiteralMethodSnippet,
isSnippet: true,
insertText: "bar(x: number): void {\n $0\n},",
Expand Down