From 103cb8d35f011c039fcd55f1c8150f4694f969b5 Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 2 Sep 2025 14:27:40 -0700 Subject: [PATCH] don't use locale-aware sorting in completions --- internal/fourslash/tests/util/util.go | 11 ++-- internal/ls/completions.go | 74 ++++++++------------------- internal/stringutil/compare.go | 8 +++ 3 files changed, 32 insertions(+), 61 deletions(-) diff --git a/internal/fourslash/tests/util/util.go b/internal/fourslash/tests/util/util.go index 0df83e295f..2a3a54c921 100644 --- a/internal/fourslash/tests/util/util.go +++ b/internal/fourslash/tests/util/util.go @@ -8,8 +8,7 @@ import ( "github.com/microsoft/typescript-go/internal/fourslash" "github.com/microsoft/typescript-go/internal/ls" "github.com/microsoft/typescript-go/internal/lsp/lsproto" - "golang.org/x/text/collate" - "golang.org/x/text/language" + "github.com/microsoft/typescript-go/internal/stringutil" ) func PtrTo[T any](v T) *T { @@ -13265,10 +13264,8 @@ var CompletionGlobals = sortCompletionItems(append( CompletionUndefinedVarItem, )) -var defaultLanguage = language.AmericanEnglish - func sortCompletionItems(items []fourslash.CompletionsExpectedItem) []fourslash.CompletionsExpectedItem { - compareStringsUI := collate.New(defaultLanguage).CompareString + compareStrings := stringutil.CompareStringsCaseInsensitiveThenSensitive items = slices.Clone(items) slices.SortStableFunc(items, func(a fourslash.CompletionsExpectedItem, b fourslash.CompletionsExpectedItem) int { defaultSortText := string(ls.SortTextLocationPriority) @@ -13287,7 +13284,7 @@ func sortCompletionItems(items []fourslash.CompletionsExpectedItem) []fourslash. } aSortText = core.OrElse(aSortText, defaultSortText) bSortText = core.OrElse(bSortText, defaultSortText) - bySortText := compareStringsUI(aSortText, bSortText) + bySortText := compareStrings(aSortText, bSortText) if bySortText != 0 { return bySortText } @@ -13308,7 +13305,7 @@ func sortCompletionItems(items []fourslash.CompletionsExpectedItem) []fourslash. default: panic(fmt.Sprintf("unexpected completion item type: %T", b)) } - return compareStringsUI(aLabel, bLabel) + return compareStrings(aLabel, bLabel) }) return items } diff --git a/internal/ls/completions.go b/internal/ls/completions.go index a7e249637d..e2b2f73c19 100644 --- a/internal/ls/completions.go +++ b/internal/ls/completions.go @@ -29,8 +29,6 @@ import ( "github.com/microsoft/typescript-go/internal/scanner" "github.com/microsoft/typescript-go/internal/stringutil" "github.com/microsoft/typescript-go/internal/tspath" - "golang.org/x/text/collate" - "golang.org/x/text/language" ) func (l *LanguageService) ProvideCompletion( @@ -1884,7 +1882,6 @@ func (l *LanguageService) completionInfoFromData( clientOptions, ) - compareCompletionEntries := getCompareCompletionEntries(ctx) if data.keywordFilters != KeywordCompletionFiltersNone { keywordCompletions := getKeywordCompletions(data.keywordFilters, !data.insideJSDocTagTypeExpression && ast.IsSourceFileJS(file)) for _, keywordEntry := range keywordCompletions { @@ -1958,7 +1955,6 @@ func (l *LanguageService) getCompletionEntriesFromSymbols( // true otherwise. Based on the order we add things we will always see locals first, then globals, then module exports. // So adding a completion for a local will prevent us from adding completions for external module exports sharing the same name. uniques := make(uniqueNamesMap) - compareCompletionEntries := getCompareCompletionEntries(ctx) for _, symbol := range data.symbols { symbolId := ast.GetSymbolId(symbol) origin := data.symbolToOriginInfoMap[symbolId] @@ -3295,25 +3291,6 @@ func getCompletionsSymbolKind(kind ScriptElementKind) lsproto.CompletionItemKind } } -var collatorCache collections.SyncMap[language.Tag, *sync.Pool] - -func getCollator(tag language.Tag) *collate.Collator { - pool, ok := collatorCache.Load(tag) - if !ok { - pool, _ = collatorCache.LoadOrStore(tag, &sync.Pool{ - New: func() any { - return collate.New(tag) - }, - }) - } - return pool.Get().(*collate.Collator) -} - -func putCollator(tag language.Tag, collator *collate.Collator) { - pool, _ := collatorCache.Load(tag) - pool.Put(collator) -} - // Editors will use the `sortText` and then fall back to `name` for sorting, but leave ties in response order. // So, it's important that we sort those ties in the order we want them displayed if it matters. We don't // strictly need to sort by name or SortText here since clients are going to do it anyway, but we have to @@ -3321,33 +3298,28 @@ func putCollator(tag language.Tag, collator *collate.Collator) { // by the language service consistent with what TS Server does and what editors typically do. This also makes // completions tests make more sense. We used to sort only alphabetically and only in the server layer, but // this made tests really weird, since most fourslash tests don't use the server. -func getCompareCompletionEntries(ctx context.Context) func(entryInSlice *lsproto.CompletionItem, entryToInsert *lsproto.CompletionItem) int { - return func(entryInSlice *lsproto.CompletionItem, entryToInsert *lsproto.CompletionItem) int { - locale := core.GetLocale(ctx) - collator := getCollator(locale) - defer putCollator(locale, collator) - compareStrings := collator.CompareString - result := compareStrings(*entryInSlice.SortText, *entryToInsert.SortText) - if result == stringutil.ComparisonEqual { - result = compareStrings(entryInSlice.Label, entryToInsert.Label) - } - if result == stringutil.ComparisonEqual && entryInSlice.Data != nil && entryToInsert.Data != nil { - sliceEntryData, ok1 := (*entryInSlice.Data).(*completionEntryData) - insertEntryData, ok2 := (*entryToInsert.Data).(*completionEntryData) - if ok1 && ok2 && sliceEntryData.ModuleSpecifier != "" && insertEntryData.ModuleSpecifier != "" { - // Sort same-named auto-imports by module specifier - result = compareNumberOfDirectorySeparators( - sliceEntryData.ModuleSpecifier, - insertEntryData.ModuleSpecifier, - ) - } - } - if result == stringutil.ComparisonEqual { - // Fall back to symbol order - if we return `EqualTo`, `insertSorted` will put later symbols first. - return stringutil.ComparisonLessThan +func compareCompletionEntries(entryInSlice *lsproto.CompletionItem, entryToInsert *lsproto.CompletionItem) int { + compareStrings := stringutil.CompareStringsCaseInsensitiveThenSensitive + result := compareStrings(*entryInSlice.SortText, *entryToInsert.SortText) + if result == stringutil.ComparisonEqual { + result = compareStrings(entryInSlice.Label, entryToInsert.Label) + } + if result == stringutil.ComparisonEqual && entryInSlice.Data != nil && entryToInsert.Data != nil { + sliceEntryData, ok1 := (*entryInSlice.Data).(*completionEntryData) + insertEntryData, ok2 := (*entryToInsert.Data).(*completionEntryData) + if ok1 && ok2 && sliceEntryData.ModuleSpecifier != "" && insertEntryData.ModuleSpecifier != "" { + // Sort same-named auto-imports by module specifier + result = compareNumberOfDirectorySeparators( + sliceEntryData.ModuleSpecifier, + insertEntryData.ModuleSpecifier, + ) } - return result } + if result == stringutil.ComparisonEqual { + // Fall back to symbol order - if we return `EqualTo`, `insertSorted` will put later symbols first. + return stringutil.ComparisonLessThan + } + return result } // True if the first character of `lowercaseCharacters` is the first character @@ -3582,7 +3554,6 @@ func (l *LanguageService) getJSCompletionEntries( uniqueNames *collections.Set[string], sortedEntries []*lsproto.CompletionItem, ) []*lsproto.CompletionItem { - compareCompletionEntries := getCompareCompletionEntries(ctx) nameTable := getNameTable(file) for name, pos := range nameTable { // Skip identifiers produced only from the current location @@ -4929,11 +4900,6 @@ func hasCompletionItem(clientOptions *lsproto.CompletionClientCapabilities) bool return clientOptions != nil && clientOptions.CompletionItem != nil } -// strada TODO: this function is, at best, poorly named. Use sites are pretty suspicious. -func compilerOptionsIndicateEsModules(options *core.CompilerOptions) bool { - return options.Module == core.ModuleKindNone || options.GetEmitScriptTarget() >= core.ScriptTargetES2015 || options.NoEmit.IsTrue() -} - func clientSupportsItemLabelDetails(clientOptions *lsproto.CompletionClientCapabilities) bool { return hasCompletionItem(clientOptions) && ptrIsTrue(clientOptions.CompletionItem.LabelDetailsSupport) } diff --git a/internal/stringutil/compare.go b/internal/stringutil/compare.go index 522f8fc88e..832e1dbcf1 100644 --- a/internal/stringutil/compare.go +++ b/internal/stringutil/compare.go @@ -90,3 +90,11 @@ func HasSuffix(s string, suffix string, caseSensitive bool) bool { } return strings.EqualFold(s[len(s)-len(suffix):], suffix) } + +func CompareStringsCaseInsensitiveThenSensitive(a, b string) Comparison { + cmp := CompareStringsCaseInsensitive(a, b) + if cmp != ComparisonEqual { + return cmp + } + return CompareStringsCaseSensitive(a, b) +}