From c39f1bb4efbd990452464a82ddbbb207e747064c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Jun 2025 15:20:03 +0000 Subject: [PATCH 1/6] Initial plan From 06b5873e0ce9c0d6b3c38884fa556df6935b7a98 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Jun 2025 15:42:58 +0000 Subject: [PATCH 2/6] Fix Symbol completion priority and cursor positioning Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com> --- src/services/completions.ts | 24 +++++++++++++------ .../completionsUniqueSymbol2.baseline | 4 ++-- .../completionForComputedStringProperties.ts | 14 +++++------ .../fourslash/completionsSymbolMembers.ts | 4 ++-- .../fourslash/completionsUniqueSymbol1.ts | 2 +- .../completionsUniqueSymbol_import.ts | 2 +- .../symbolCompletionLowerPriority.ts | 20 ++++++++++++++++ 7 files changed, 50 insertions(+), 20 deletions(-) create mode 100644 tests/cases/fourslash/symbolCompletionLowerPriority.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index 6d0cbf7183ed3..35589937c218b 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1730,10 +1730,16 @@ function createCompletionEntry( } // We should only have needsConvertPropertyAccess if there's a property access to convert. But see #21790. // Somehow there was a global with a non-identifier name. Hopefully someone will complain about getting a "foo bar" global completion and provide a repro. - else if ((useBraces || insertQuestionDot) && propertyAccessToConvert) { - insertText = useBraces ? needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}]` : `[${name}]` : name; - if (insertQuestionDot || propertyAccessToConvert.questionDotToken) { - insertText = `?.${insertText}`; + else if ((useBraces || insertQuestionDot) && propertyAccessToConvert) { + if (useBraces && preferences.includeCompletionsWithSnippetText) { + // For symbol completions, position cursor inside brackets for better UX + insertText = needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}$0]` : `[${name}$0]`; + isSnippet = true; + } else { + insertText = useBraces ? needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}]` : `[${name}]` : name; + } + if (insertQuestionDot || propertyAccessToConvert.questionDotToken) { + insertText = `?.${insertText}`; } const dot = findChildOfKind(propertyAccessToConvert, SyntaxKind.DotToken, sourceFile) || @@ -3848,9 +3854,13 @@ function getCompletionData( // If this is nested like for `namespace N { export const sym = Symbol(); }`, we'll add the completion for `N`. const firstAccessibleSymbol = nameSymbol && getFirstSymbolInChain(nameSymbol, contextToken, typeChecker); const firstAccessibleSymbolId = firstAccessibleSymbol && getSymbolId(firstAccessibleSymbol); - if (firstAccessibleSymbolId && addToSeen(seenPropertySymbols, firstAccessibleSymbolId)) { - const index = symbols.length; - symbols.push(firstAccessibleSymbol); + if (firstAccessibleSymbolId && addToSeen(seenPropertySymbols, firstAccessibleSymbolId)) { + const index = symbols.length; + symbols.push(firstAccessibleSymbol); + + // Symbol completions should have lower priority since they represent computed property access + symbolToSortTextMap[getSymbolId(firstAccessibleSymbol)] = SortText.GlobalsOrKeywords; + const moduleSymbol = firstAccessibleSymbol.parent; if ( !moduleSymbol || diff --git a/tests/baselines/reference/completionsUniqueSymbol2.baseline b/tests/baselines/reference/completionsUniqueSymbol2.baseline index fc4ae791cdbda..2f8276a6b7efb 100644 --- a/tests/baselines/reference/completionsUniqueSymbol2.baseline +++ b/tests/baselines/reference/completionsUniqueSymbol2.baseline @@ -56,7 +56,7 @@ "name": "a", "kind": "const", "kindModifiers": "", - "sortText": "11", + "sortText": "15", "insertText": "[a]", "replacementSpan": { "start": 344, @@ -210,7 +210,7 @@ "name": "b", "kind": "const", "kindModifiers": "", - "sortText": "11", + "sortText": "15", "insertText": "[b]", "replacementSpan": { "start": 344, diff --git a/tests/cases/fourslash/completionForComputedStringProperties.ts b/tests/cases/fourslash/completionForComputedStringProperties.ts index e03ceb4abd0ee..d9ccf16f6aabb 100644 --- a/tests/cases/fourslash/completionForComputedStringProperties.ts +++ b/tests/cases/fourslash/completionForComputedStringProperties.ts @@ -8,11 +8,11 @@ //// declare const a: A; //// a[|./**/|] -verify.completions({ - marker: "", - exact: [ - { name: "p1" }, - { name: "p2", insertText: '[p2]', replacementSpan: test.ranges()[0] }, - ], - preferences: { includeInsertTextCompletions: true }, +verify.completions({ + marker: "", + exact: [ + { name: "p1" }, + { name: "p2", insertText: '[p2]', sortText: completion.SortText.GlobalsOrKeywords, replacementSpan: test.ranges()[0] }, + ], + preferences: { includeInsertTextCompletions: true }, }); \ No newline at end of file diff --git a/tests/cases/fourslash/completionsSymbolMembers.ts b/tests/cases/fourslash/completionsSymbolMembers.ts index 39b8c5c499ac9..e64f42d61819b 100644 --- a/tests/cases/fourslash/completionsSymbolMembers.ts +++ b/tests/cases/fourslash/completionsSymbolMembers.ts @@ -14,12 +14,12 @@ verify.completions( { marker: "i", - exact: { name: "s", insertText: "[s]", replacementSpan: test.ranges()[0] }, + exact: { name: "s", insertText: "[s]", sortText: completion.SortText.GlobalsOrKeywords, replacementSpan: test.ranges()[0] }, preferences: { includeInsertTextCompletions: true }, }, { marker: "j", - exact: { name: "N", insertText: "[N]", replacementSpan: test.ranges()[1] }, + exact: { name: "N", insertText: "[N]", sortText: completion.SortText.GlobalsOrKeywords, replacementSpan: test.ranges()[1] }, preferences: { includeInsertTextCompletions: true }, } ); diff --git a/tests/cases/fourslash/completionsUniqueSymbol1.ts b/tests/cases/fourslash/completionsUniqueSymbol1.ts index a00fe0d716751..3aef47d9f9b27 100644 --- a/tests/cases/fourslash/completionsUniqueSymbol1.ts +++ b/tests/cases/fourslash/completionsUniqueSymbol1.ts @@ -17,6 +17,6 @@ verify.completions({ marker: "", - exact: { name: "M", insertText: "[M]", replacementSpan: test.ranges()[0] }, + exact: { name: "M", insertText: "[M]", sortText: completion.SortText.GlobalsOrKeywords, replacementSpan: test.ranges()[0] }, preferences: { includeInsertTextCompletions: true }, }); diff --git a/tests/cases/fourslash/completionsUniqueSymbol_import.ts b/tests/cases/fourslash/completionsUniqueSymbol_import.ts index a12e4cae271f5..dbdaec369ac4e 100644 --- a/tests/cases/fourslash/completionsUniqueSymbol_import.ts +++ b/tests/cases/fourslash/completionsUniqueSymbol_import.ts @@ -24,7 +24,7 @@ verify.completions({ marker: "", exact: [ "n", - { name: "publicSym", source: "/a", insertText: "[publicSym]", replacementSpan: test.ranges()[0], hasAction: true }, + { name: "publicSym", source: "/a", insertText: "[publicSym]", sortText: completion.SortText.GlobalsOrKeywords, replacementSpan: test.ranges()[0], hasAction: true }, ], preferences: { includeInsertTextCompletions: true, diff --git a/tests/cases/fourslash/symbolCompletionLowerPriority.ts b/tests/cases/fourslash/symbolCompletionLowerPriority.ts new file mode 100644 index 0000000000000..eda43cfbdafce --- /dev/null +++ b/tests/cases/fourslash/symbolCompletionLowerPriority.ts @@ -0,0 +1,20 @@ +/// + +////declare const Symbol: (s: string) => symbol; +////const mySymbol = Symbol("test"); +////interface TestInterface { +//// [mySymbol]: string; +//// normalProperty: number; +////} +////const obj: TestInterface = {} as any; +////obj./*completions*/ + +// Test new behavior: Symbol completions should have lower priority and better cursor positioning +verify.completions({ + marker: "completions", + includes: [ + { name: "normalProperty", sortText: completion.SortText.LocationPriority }, + { name: "mySymbol", sortText: completion.SortText.GlobalsOrKeywords, insertText: "[mySymbol$0]", isSnippet: true } // Now with snippet cursor positioning + ], + preferences: { includeInsertTextCompletions: true, includeCompletionsWithSnippetText: true } +}); \ No newline at end of file From 6adb28184099fc53358c793bf2c1e6882eaa9f50 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Jun 2025 15:59:25 +0000 Subject: [PATCH 3/6] Fix all line endings in completions.ts to use consistent CRLF Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com> --- src/services/completions.ts | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 35589937c218b..c3f53c41c232c 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1730,16 +1730,16 @@ function createCompletionEntry( } // We should only have needsConvertPropertyAccess if there's a property access to convert. But see #21790. // Somehow there was a global with a non-identifier name. Hopefully someone will complain about getting a "foo bar" global completion and provide a repro. - else if ((useBraces || insertQuestionDot) && propertyAccessToConvert) { - if (useBraces && preferences.includeCompletionsWithSnippetText) { - // For symbol completions, position cursor inside brackets for better UX - insertText = needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}$0]` : `[${name}$0]`; - isSnippet = true; - } else { - insertText = useBraces ? needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}]` : `[${name}]` : name; - } - if (insertQuestionDot || propertyAccessToConvert.questionDotToken) { - insertText = `?.${insertText}`; + else if ((useBraces || insertQuestionDot) && propertyAccessToConvert) { + if (useBraces && preferences.includeCompletionsWithSnippetText) { + // For symbol completions, position cursor inside brackets for better UX + insertText = needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}$0]` : `[${name}$0]`; + isSnippet = true; + } else { + insertText = useBraces ? needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}]` : `[${name}]` : name; + } + if (insertQuestionDot || propertyAccessToConvert.questionDotToken) { + insertText = `?.${insertText}`; } const dot = findChildOfKind(propertyAccessToConvert, SyntaxKind.DotToken, sourceFile) || @@ -3854,13 +3854,13 @@ function getCompletionData( // If this is nested like for `namespace N { export const sym = Symbol(); }`, we'll add the completion for `N`. const firstAccessibleSymbol = nameSymbol && getFirstSymbolInChain(nameSymbol, contextToken, typeChecker); const firstAccessibleSymbolId = firstAccessibleSymbol && getSymbolId(firstAccessibleSymbol); - if (firstAccessibleSymbolId && addToSeen(seenPropertySymbols, firstAccessibleSymbolId)) { - const index = symbols.length; - symbols.push(firstAccessibleSymbol); - - // Symbol completions should have lower priority since they represent computed property access - symbolToSortTextMap[getSymbolId(firstAccessibleSymbol)] = SortText.GlobalsOrKeywords; - + if (firstAccessibleSymbolId && addToSeen(seenPropertySymbols, firstAccessibleSymbolId)) { + const index = symbols.length; + symbols.push(firstAccessibleSymbol); + + // Symbol completions should have lower priority since they represent computed property access + symbolToSortTextMap[getSymbolId(firstAccessibleSymbol)] = SortText.GlobalsOrKeywords; + const moduleSymbol = firstAccessibleSymbol.parent; if ( !moduleSymbol || From 1ed11222a56a7bcadeb4608b9786be72363bf4dd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Jun 2025 21:38:23 +0000 Subject: [PATCH 4/6] Format code using dprint formatter Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- src/services/completions.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index c3f53c41c232c..f0361b795633a 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1735,7 +1735,8 @@ function createCompletionEntry( // For symbol completions, position cursor inside brackets for better UX insertText = needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}$0]` : `[${name}$0]`; isSnippet = true; - } else { + } + else { insertText = useBraces ? needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}]` : `[${name}]` : name; } if (insertQuestionDot || propertyAccessToConvert.questionDotToken) { @@ -3857,10 +3858,10 @@ function getCompletionData( if (firstAccessibleSymbolId && addToSeen(seenPropertySymbols, firstAccessibleSymbolId)) { const index = symbols.length; symbols.push(firstAccessibleSymbol); - + // Symbol completions should have lower priority since they represent computed property access symbolToSortTextMap[getSymbolId(firstAccessibleSymbol)] = SortText.GlobalsOrKeywords; - + const moduleSymbol = firstAccessibleSymbol.parent; if ( !moduleSymbol || From ac1305802b06ffc27cb2ad1308f17a2a05c6b3b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Jun 2025 22:05:26 +0000 Subject: [PATCH 5/6] Revert snippet text cursor positioning but keep sort order changes Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com> --- src/services/completions.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index f0361b795633a..fb3dbe44db260 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1731,14 +1731,7 @@ function createCompletionEntry( // We should only have needsConvertPropertyAccess if there's a property access to convert. But see #21790. // Somehow there was a global with a non-identifier name. Hopefully someone will complain about getting a "foo bar" global completion and provide a repro. else if ((useBraces || insertQuestionDot) && propertyAccessToConvert) { - if (useBraces && preferences.includeCompletionsWithSnippetText) { - // For symbol completions, position cursor inside brackets for better UX - insertText = needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}$0]` : `[${name}$0]`; - isSnippet = true; - } - else { - insertText = useBraces ? needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}]` : `[${name}]` : name; - } + insertText = useBraces ? needsConvertPropertyAccess ? `[${quotePropertyName(sourceFile, preferences, name)}]` : `[${name}]` : name; if (insertQuestionDot || propertyAccessToConvert.questionDotToken) { insertText = `?.${insertText}`; } From 11c7743959a8d4b304ddf087892e5e20bd29b3e3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Jun 2025 22:41:17 +0000 Subject: [PATCH 6/6] Fix symbolCompletionLowerPriority test to match reverted snippet behavior Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com> --- tests/cases/fourslash/symbolCompletionLowerPriority.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/cases/fourslash/symbolCompletionLowerPriority.ts b/tests/cases/fourslash/symbolCompletionLowerPriority.ts index eda43cfbdafce..cb29c4e309215 100644 --- a/tests/cases/fourslash/symbolCompletionLowerPriority.ts +++ b/tests/cases/fourslash/symbolCompletionLowerPriority.ts @@ -9,12 +9,12 @@ ////const obj: TestInterface = {} as any; ////obj./*completions*/ -// Test new behavior: Symbol completions should have lower priority and better cursor positioning +// Test new behavior: Symbol completions should have lower priority verify.completions({ marker: "completions", includes: [ { name: "normalProperty", sortText: completion.SortText.LocationPriority }, - { name: "mySymbol", sortText: completion.SortText.GlobalsOrKeywords, insertText: "[mySymbol$0]", isSnippet: true } // Now with snippet cursor positioning + { name: "mySymbol", sortText: completion.SortText.GlobalsOrKeywords, insertText: "[mySymbol]" } ], - preferences: { includeInsertTextCompletions: true, includeCompletionsWithSnippetText: true } + preferences: { includeInsertTextCompletions: true } }); \ No newline at end of file