Respect useAliasesForRenames in rename edits#4371
Conversation
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
js/ts.preferences.useAliasesForRenames not respectedThere was a problem hiding this comment.
⚠️ Not ready to approve
Rename edit generation now incorrectly gates numeric-literal index quoting on useAliasesForRenames, which can produce semantically wrong edits (e.g. obj[foo] vs obj["foo"]).
Pull request overview
This PR updates the native language service rename pipeline to respect the js/ts.preferences.useAliasesForRenames / typescript.preferences.useAliasesForRenames user preference, switching between alias-preserving import/export rewrites vs. renaming the underlying exported declaration and its references.
Changes:
- Threaded
UseAliasesForRenameinto rename reference collection souseAliasesForRenames: falsetargets the exported declaration rather than introducing import/export aliases. - Gated rename edit text generation for alias-style prefix/suffix rewrites on
UseAliasesForRename. - Added fourslash coverage for renaming an imported type with the preference enabled vs. disabled, and updated affected baselines.
File summaries
| File | Description |
|---|---|
| internal/ls/findallreferences.go | Uses the preference to choose rename reference-collection strategy. |
| internal/ls/rename.go | Threads the preference into rename edit text generation to enable/disable alias-style rewrites. |
| internal/fourslash/tests/renameNamedImportUseAliasesForRenames_test.go | New fourslash test covering named-import rename behavior for useAliasesForRenames true vs false. |
| testdata/baselines/reference/fourslash/findRenameLocations/renameNamedImportUseAliasesForRenames.baseline.jsonc | New reference baseline for the added fourslash test. |
| testdata/baselines/reference/submodule/fourslash/findRenameLocations/renameExportSpecifier2.baseline.jsonc | Updated submodule baseline reflecting preference-driven rename behavior changes. |
| testdata/baselines/reference/submodule/fourslash/findRenameLocations/renameExportSpecifier2.baseline.jsonc.diff | Removed accepted-diff baseline file (no longer needed after baseline updates). |
| testdata/baselines/reference/submodule/fourslash/findRenameLocations/findAllRefsReExportsUseInImportType.baseline.jsonc | Updated submodule baseline reflecting preference-driven rename behavior changes. |
| testdata/baselines/reference/submodule/fourslash/findRenameLocations/findAllRefsReExportsUseInImportType.baseline.jsonc.diff | Updated diff baseline for the above submodule baseline change. |
| testdata/baselines/reference/submodule/fourslash/findRenameLocations/findAllRefsPrefixSuffixPreference.baseline.jsonc | Updated submodule baseline reflecting preference-driven rename behavior changes. |
| testdata/baselines/reference/submodule/fourslash/findRenameLocations/findAllRefsPrefixSuffixPreference.baseline.jsonc.diff | Updated diff baseline for the above submodule baseline change. |
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 1
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
| // If the node is a numerical indexing literal, then add quotes around the property access. | ||
| if entry.kind != entryKindRange && ast.IsNumericLiteral(entry.node) && ast.IsAccessExpression(entry.node.Parent) { | ||
| if useAliasesForRename && entry.kind != entryKindRange && ast.IsNumericLiteral(entry.node) && ast.IsAccessExpression(entry.node.Parent) { | ||
| quote := getQuoteFromPreference(quotePreference) | ||
| return quote + newText + quote |
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
There was a problem hiding this comment.
⚠️ Not ready to approve
Rename preference default handling appears inconsistent across rename edit generation vs rename validation, which could cause incorrect behavior when the preference is unset.
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 1
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
| quotePreference := lsutil.GetQuotePreference(sourceFile, l.UserPreferences()) | ||
| useAliasesForRename := l.UserPreferences().UseAliasesForRename != core.TSFalse | ||
|
|
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
| } else { | ||
| options.use = referenceUseRename | ||
| options.useAliasesForRename = true | ||
| options.useAliasesForRename = l.UserPreferences().UseAliasesForRename != core.TSFalse |
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
The native language service parsed
js/ts.preferences.useAliasesForRenamesandtypescript.preferences.useAliasesForRenames, but rename edits still always used alias-preserving import rewrites.Rename behavior
UseAliasesForRenameinto rename reference collection.Edit generation
UseAliasesForRename.Coverage
Example:
With
useAliasesForRenames: false, renamingMyTypeAfrom the import now also renames the exportedinterface MyTypeAinstead of producing: