Skip to content

Add Quick Fix (code actions) for diagnostics#1237

Merged
kishikawakatsumi merged 2 commits into
masterfrom
feature/code-actions
Jun 11, 2026
Merged

Add Quick Fix (code actions) for diagnostics#1237
kishikawakatsumi merged 2 commits into
masterfrom
feature/code-actions

Conversation

@kishikawakatsumi

Copy link
Copy Markdown
Member

Surfaces sourcekit-lsp's fix-its as Monaco Quick Fixes (the 💡 lightbulb) on the red/yellow squiggles.

Why it's cheap

sourcekit-lsp embeds the fix-its inline on each diagnostic as a codeActions array (a sourcekit-lsp extension), and the backend already forwards them — they arrive in the diagnostics WebSocket message the client receives. The client simply discarded them when turning diagnostics into editor markers. So this is frontend-only, no backend change, no extra request.

Changes (editor.js, app.js)

  • Register a Monaco CodeActionProvider. For the markers under the cursor it looks up the stored fix-its and returns them as quickfix actions, mapping the LSP edit.changes TextEdits onto the local model (WorkspaceEdit).
  • Stash each diagnostics notification's fix-its in codeActionEntries (cleared when diagnostics clear). No round-trip — the data is already present.
  • Fix a pre-existing diagnostics bug: the marker's endColumn used the range start (zero-width squiggle) instead of the range end, so error underlines now span the actual range.

Example

Typing an incomplete call like greet( yields the sourcekit-lsp fix-it "Insert 'name: , times: '" — now offered as a Quick Fix that inserts the argument labels.

Verification

  • webpack build passes.
  • The codeActions payload shape was confirmed against the live backend (e.g. "Missing arguments for parameters…" ships a quickfix with an edit.changes TextEdit).

🤖 Generated with Claude Code

sourcekit-lsp ships fix-its inline on each diagnostic (a `codeActions` array),
and those already arrive with the diagnostics notification — the client just
dropped them when converting diagnostics to markers.

- Register a Monaco code action provider that matches the markers under the
  cursor to the stored fix-its and returns them as Quick Fix actions (the
  lightbulb), applying the LSP TextEdits to the local model. No backend change
  and no extra round-trip.
- Keep the fix-its from each diagnostics notification in `codeActionEntries`,
  cleared whenever diagnostics are cleared.

Also fix the diagnostic marker's end column, which used the range start (a
zero-width squiggle) instead of the range end.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 11, 2026 21:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR surfaces sourcekit-lsp diagnostic fix-its as Monaco Quick Fix (lightbulb) code actions by registering a CodeActionProvider and stashing fix-it payloads delivered inline with diagnostics. It also fixes a diagnostics marker bug where endColumn incorrectly used the start position, producing zero-width squiggles.

Changes:

  • Register a Monaco CodeActionProvider for Swift and route requests through Editor.oncodeaction.
  • Persist diagnostic.codeActions from diagnostics notifications and translate LSP WorkspaceEdit.changes into Monaco workspace edits for Quick Fixes.
  • Fix marker range rendering by using diagnostic.range.end.character for endColumn.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
Public/js/editor.js Registers the Swift code action provider and adds an oncodeaction hook.
Public/js/app.js Stores fix-its from diagnostics and converts them into Monaco Quick Fix actions; fixes diagnostics marker endColumn.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Public/js/app.js
Comment on lines +204 to +208
this.codeActionEntries = diagnostics
.filter((diagnostic) => diagnostic.codeActions?.length)
.map((diagnostic) => ({
startLineNumber: diagnostic.range.start.line + 1,
startColumn: diagnostic.range.start.character + 1,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Addressed in the latest commit: the lookup now also keys on the marker source (LSP diagnostics get a stable sourcekit-lsp fallback), so runner markers — which set no source — can't pick up a stale fix-it.

Comment thread Public/js/app.js
Comment on lines +342 to +346
(e) =>
e.startLineNumber === marker.startLineNumber &&
e.startColumn === marker.startColumn &&
e.message === marker.message
);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Addressed in the latest commit: the lookup now also keys on the marker source (LSP diagnostics get a stable sourcekit-lsp fallback), so runner markers — which set no source — can't pick up a stale fix-it.

Address review feedback: matching fix-its to markers by position+message alone
could attach a stale LSP fix-it to a runner marker (Runner.run also calls
updateMarkers) that happens to share the same position and message. Include the
marker source in the lookup, and give LSP markers a stable fallback source so
they never collide with runner markers (which set no source).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kishikawakatsumi kishikawakatsumi merged commit 563a22f into master Jun 11, 2026
1 check passed
@kishikawakatsumi kishikawakatsumi deleted the feature/code-actions branch June 11, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants