Skip to content

Prioritize case and default keywords in switch statement completions #62126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 25, 2025

This PR implements smart prioritization of case and default keywords in TypeScript switch statement completions, addressing a longstanding usability issue where these essential keywords appeared buried in the completion list.

Problem

Previously, when requesting completions inside a switch statement, case and default keywords used the default sort order, causing them to appear after other completions:

switch (someValue) {
    /*completion here showed case/default after other items*/
}

This was problematic because these keywords are almost always what developers need in switch contexts, but they were sorted alphabetically after other suggestions.

Solution

The implementation adds intelligent context detection to prioritize case and default keywords when:

  1. Cursor is directly inside a switch block:
switch (thing) {
    /*cursor here - case/default now prioritized*/
}
  1. Cursor is aligned with case/default after proper statement termination:
switch (thing) {
  case 42:
    break;  // indented break statement
  /*cursor here - case/default prioritized*/
}

switch (thing) {
  case 42: break;  // same-line break
  /*cursor here - case/default prioritized*/
}
  1. No prioritization when inappropriate:
switch (thing) {
  case 42:
  break;  // break at same column as case
  /*cursor here - normal completions*/
}

Implementation Details

  • New function: shouldPrioritizeCaseAndDefaultKeywords() detects switch contexts
  • Smart detection: Analyzes cursor position, statement structure, and indentation
  • Sort text override: Uses SortText.LocalDeclarationPriority ("10") instead of SortText.GlobalsOrKeywords ("15")
  • Preserves existing behavior: Only affects switch statement contexts

Testing

Added comprehensive test coverage in switchCaseCompletionPriority.ts using baseline testing to demonstrate the relative ordering of case, default, and other completions:

  • ✅ Prioritized contexts show case/default with sortText "10"
  • ✅ Non-prioritized contexts show case/default with sortText "15"
  • ✅ User variables like thing remain available with consistent sortText "11"
  • ✅ All existing tests continue to pass

Fixes #52234.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jul 25, 2025
//// /*newPrio3*/
//// }

//// // NEW - prioritize case/default (after throw)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these NEW labels, don't claim what to expect. Instead, describe the positioning of these.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed all "NEW" labels and improved the descriptions to focus on positioning rather than claiming expectations. Comments now describe the context (e.g., "After break statement with proper indentation") instead of predicting behavior. Fixed in 7f7a30e.

//// /*newPrio4*/
//// }

verify.completions(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch this to baselining

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to baseline testing approach using verify.baselineCompletions() for all markers instead of explicit verify.completions() assertions. The baseline now clearly shows case/default prioritization (sortText "10") in appropriate contexts and normal ordering (sortText "15") elsewhere. Fixed in 7f7a30e.

@Copilot Copilot AI changed the title [WIP] Use context to prioritize or only show case and default in a switch statement Prioritize case and default keywords in switch statement completions Jul 25, 2025
@Copilot Copilot AI requested a review from DanielRosenwasser July 25, 2025 18:13
Copilot finished work on behalf of DanielRosenwasser July 25, 2025 18:13
Copilot finished work on behalf of DanielRosenwasser July 25, 2025 18:23
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff is huge, so maybe this needs to run with noLib or some similar thing to trim down the full completion list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added @noLib: true and switched from baseline testing to targeted verify.completions() assertions. This reduced the test from 256k+ lines to a focused test that validates the specific sortText values ("10" for prioritized, "15" for normal) without the massive global completion list. Fixed in c8b6d8c.

@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jul 25, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
pack this ✅ Started ✅ Results

Copy link
Contributor Author

Copilot AI commented Jul 25, 2025

@typescript-bot Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub.

You can ask me to try again later by mentioning me in a new comment.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 09f2c5c1-a820-42b4-9fa5-5821504f6219

Sorry for the inconvenience!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jul 25, 2025

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/165755/artifacts?artifactName=tgz&fileId=664952F17C8038F4FF83897BA9C11AF4A7008B746F072EB9307731263911F28802&fileName=/typescript-5.9.0-insiders.20250725.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

Copy link
Contributor Author

Copilot AI commented Jul 25, 2025

@typescript-bot Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub.

You can ask me to try again later by mentioning me in a new comment.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 1c6e3c4e-368f-4f9f-bf5a-7401acbbca3d

Sorry for the inconvenience!

@Copilot Copilot AI requested a review from DanielRosenwasser July 25, 2025 19:12
Copilot finished work on behalf of DanielRosenwasser July 25, 2025 19:12
verify.completions({
includes: [
{ name: "case", sortText: "10" },
{ name: "default", sortText: "10" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be tested that thing is still in the completion list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added explicit verification that thing is still in the completion list with proper sortText ("11") across all test scenarios. The test now uses both baseline testing to show relative ordering and targeted verify.completions() to confirm thing availability. Fixed in 75fcd1d.

});

goTo.marker("newPrio1");
verify.completions({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go back to baselining if the baselines aren't massive, and show some characteristics about the ordering. The key thing the tests need to witness is whether case, default, and thing appear in the list and what their relative order would be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched back to baseline testing with @nolib to keep baselines manageable (~548KB). The baseline now clearly shows the relative ordering of case, default, and thing in different contexts:

  • Prioritized contexts: case/default sortText "10", thing sortText "11"
  • Non-prioritized contexts: case/default sortText "15", thing sortText "11"

Fixed in 75fcd1d.

…ordering and verify thing availability

Co-authored-by: DanielRosenwasser <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use context to prioritize or only show case and default in a switch statement
3 participants