typescript-go icon indicating copy to clipboard operation
typescript-go copied to clipboard

Prevent renaming keywords by validating in handleRename

Open Copilot opened this issue 3 weeks ago • 0 comments

  • [x] Explore the codebase and understand the rename flow
  • [x] Review TypeScript implementation
  • [x] Implement simplified validation approach
  • [x] Localize error messages properly
  • [x] Fix format string bug in error creation
  • [ ] Resolve test regressions (awaiting feedback)

Current Status

Implemented validation in handleRename that returns a properly localized error when renaming invalid targets.

Changes:

  • Added nodeIsEligibleForRename() for basic node type validation
  • Updated ProvideSymbolsAndEntries() to check eligibility of adjusted nodes
  • Modified ProvideRenameFromSymbolAndEntries() to return localized error for ineligible renames
  • Fixed error message to use locale.FromContext(ctx) and .Localize() for proper localization
  • Fixed bug: Use errors.New() instead of fmt.Errorf() to avoid treating localized message as format string

Note:

Some fourslash tests are failing because they expect empty results (no changes) but now receive errors. Awaiting clarification on whether to:

  1. Keep error-based approach and update tests
  2. Revert to prepareRename for better UX (tooltip vs error notification)
Original prompt

This section details on the original issue you should resolve

<issue_title>Should not allow renaming keywords</issue_title> <issue_description>TS-go will allow triggering rename anywhere in the file, including on parens and keywords. When you press enter it will then either try doing a rename or say no result

Instead we should fail the rename early. Here's what happens in TS 6

Image

I believe this warning comes from the prepareRename call</issue_description>

<agent_instructions>Look at the submodule's rename code to guide this; this should already have been implemented there.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@mjbvz In 6.0 we do allow triggering renames on `class` and similar keywords but this shifts the rename to the class name instead
  • Fixes microsoft/typescript-go#2229

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot avatar Dec 04 '25 21:12 Copilot