Fix code corruption when multiple code action providers are active on save#306045
Fix code corruption when multiple code action providers are active on save#306045ishaq2321 wants to merge 4 commits intomicrosoft:mainfrom
Conversation
… save (Fixes microsoft#174295) Previously, `CodeActionOnSaveParticipant` would retrieve code actions from all providers concurrently and then apply them sequentially. When multiple providers (e.g. `isort` and `ruff`) returned edits for the same kind of action (e.g. `source.organizeImports`), applying the second action would corrupt the document because its edits were calculated against the original, pre-first-action document state. This fix changes the behavior to re-evaluate remaining code action providers if the model version changes after applying an action, similar to how formatting on save operates but allowing multiple providers to cascade properly. A maximum limit of 10 iterations is introduced to prevent infinite loops from poorly behaving providers.
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @lszomoruMatched files:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes file corruption when multiple extensions provide code actions on save for the same kind by cascading code action execution so each subsequent action is computed against the latest document state.
Changes:
- Reworks editor on-save code action application to re-fetch code actions after a document-changing action is applied (iterating until stable).
- Applies the same cascading behavior to notebook on-save code actions.
- Improves Git error handling by downgrading
warning:-prefixed messages to non-modal warnings.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/vs/workbench/contrib/notebook/browser/contrib/saveParticipants/saveParticipants.ts | Cascade notebook on-save code actions by re-querying after document mutations, with an iteration cap. |
| src/vs/workbench/contrib/codeEditor/browser/saveParticipants.ts | Cascade editor on-save code actions by re-querying after document mutations, with an iteration cap. |
| extensions/git/src/commands.ts | Treat warning: output as a warning (non-modal) and normalize the displayed hint line. |
| const maxIterations = 10; | ||
| let currentIteration = 0; | ||
|
|
||
| if (token.isCancellationRequested) { | ||
| actionsToRun.dispose(); | ||
| return; | ||
| } | ||
| while (currentIteration < maxIterations) { | ||
| currentIteration++; |
There was a problem hiding this comment.
The while-loop stops after maxIterations without any signal to the user/logs. If the limit is hit, remaining providers for this kind won’t run and users will have no clue why. Consider logging/reporting when the iteration cap is reached (include the kind and iteration count) and/or making the cap a shared constant so the editor + notebook paths stay in sync.
| for (const codeActionKind of codeActionsOnSave) { | ||
| const actionsToRun = await CodeActionParticipantUtils.getActionsToRun(model, codeActionKind, excludes, languageFeaturesService, getActionProgress, token); | ||
| if (token.isCancellationRequested) { | ||
| actionsToRun.dispose(); | ||
| return; | ||
| } | ||
| const maxIterations = 10; | ||
| let currentIteration = 0; | ||
|
|
||
| try { | ||
| for (const action of actionsToRun.validActions) { | ||
| const codeActionEdits = action.action.edit?.edits; | ||
| let breakFlag = false; | ||
| if (!action.action.kind?.startsWith('notebook')) { | ||
| for (const edit of codeActionEdits ?? []) { | ||
| const workspaceTextEdit = edit as IWorkspaceTextEdit; | ||
| if (workspaceTextEdit.resource && isEqual(workspaceTextEdit.resource, model.uri)) { | ||
| continue; | ||
| } else { | ||
| // error -> applied to multiple resources | ||
| breakFlag = true; | ||
| break; | ||
| while (currentIteration < maxIterations) { | ||
| currentIteration++; |
There was a problem hiding this comment.
The loop enforces maxIterations, but if the cap is reached it will silently stop applying further code actions for this kind. Please add some warning/reporting when the iteration limit is hit (include kind + count) so users/extensions can diagnose "fighting" providers, and consider sharing the cap value with the editor implementation to avoid divergence.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@microsoft-github-policy-service agree |
Fixes #174295
When multiple extensions provide code actions on save for the same kind (e.g.,
isortandruffboth providingsource.organizeImports), VS Code previously fetched their code actions concurrently based on the initial state of the document and then applied them sequentially.Applying the second code action resulted in corrupted files, duplicated lines, and mangled imports because its
WorkspaceEditwas calculated based on the file before the first extension's edits were applied.The Fix:
This PR updates both the editor and notebook
CodeActionOnSaveParticipantlogic to properly cascade code actions.model.getVersionId()changes), we immediatelybreakthe batch execution and re-evaluategetCodeActions()for that kind against the fresh document state.This ensures extensions play nicely together on save without mangling code.