Skip to content

Add code actions to disable a check for a line or the whole file#1217

Open
ryandiginomad wants to merge 7 commits into
Shopify:mainfrom
ryandiginomad:feat/432-disable-check-code-action
Open

Add code actions to disable a check for a line or the whole file#1217
ryandiginomad wants to merge 7 commits into
Shopify:mainfrom
ryandiginomad:feat/432-disable-check-code-action

Conversation

@ryandiginomad

@ryandiginomad ryandiginomad commented May 29, 2026

Copy link
Copy Markdown
Contributor

What are you adding in this PR?

ESLint-style quick-fixes for theme-check diagnostics, as requested in #432. When a theme-check offense is under the cursor, two code actions are offered:

  • Disable <Check> for this line — inserts {% # theme-check-disable-next-line <Check> %} on the line above, indentation-matched
  • Disable <Check> for entire file — inserts {% # theme-check-disable <Check> %} at the top of the file

Actions are grouped by check name, so selecting a range containing several offenses of the same check yields a single pair (not one per offense).

The disable-comment consumer already lives in theme-check-common/disabled-checks; this PR only adds the producer — the code action that writes the comment for you — matching the scope @charlespwd clarified when reopening #432 ("the right click code transformation vs supporting it in code").

Implemented as a new DisableCheckProvider in theme-language-server-common, mirroring SuggestionProvider. Unlike FixProvider/SuggestionProvider (which dispatch a server command to run an offense's fix function), this provider carries the WorkspaceEdit directly, since the inserted text is known and needs no server-side computation. A small toEditCodeAction helper was added next to toCodeAction for that.

fixes #432

{% liquid %} blocks

Inside a {% liquid %} tag, comments are bare # lines rather than {% # %}, so a {% # %} insertion would be invalid there. For now the for this line action is suppressed when the offense is inside a {% liquid %} block (detected via findCurrentNode); for entire file is still offered (it inserts at the top of the file, always valid).

What's next? Any followup issues?

  • Emit the bare-# theme-check-disable-next-line form for offenses inside {% liquid %} blocks (currently the line-level action is skipped there).
  • When a single check has offenses both inside and outside a {% liquid %} block within one selection, the first-seen offense decides whether the line-level action is offered and where the comment lands. Happy to refine to per-offense handling if you prefer.

What did you learn?

theme-check-disable (no -next-line) disables from the comment to EOF, so the file-level action just inserts at the top — no need to track an end marker.

Tophatting

Manually tophatted in a real extension host (vscode-test-web against a Dawn clone, with planted UnusedAssign offenses), plus unit tests.

To reproduce:

cd packages/vscode-extension
pnpm build
git clone --depth 1 https://github.com/Shopify/dawn.git .tmp/dawn
# plant an offense, e.g. an unused {% assign %} in a snippet
pnpm exec vscode-test-web --quality=stable --extensionDevelopmentPath=. .tmp/dawn
  1. Cursor on an offense → quick fix menu offers both new actions alongside the existing suggestion:

    Code action menu showing Disable UnusedAssign for this line / entire file

  2. Disable for this line → inserts {% # theme-check-disable-next-line UnusedAssign %} above the offense, indentation-matched; the diagnostic clears:

    Disable-next-line comment inserted, diagnostic gone

  3. Disable for entire file → inserts {% # theme-check-disable UnusedAssign %} at the top of the file:

    File-level disable comment inserted at top of file

  4. Offense inside a {% liquid %} block → only the file-level action is offered (line-level correctly suppressed, as described above):

    Inside liquid block only entire-file action offered

Unit tests (mirroring SuggestionProvider.spec.ts) assert the exact WorkspaceEdit for both variants, grouping/dedup, multiple distinct checks, the no-offense case, and the {% liquid %} skip + outside-{% liquid %} cases. pnpm test for the package: 440 passed, type-check and prettier clean.

  • I added screenshots of the changes (no actions existed before this change; screenshots show the new flow)

Before you deploy

  • I included a minor bump changeset
  • My feature is backward compatible

🤖 AI assistance disclosure — this PR was written with AI assistance (Claude Code). I directed the design (edit-based approach, the {% liquid %} v1 skip), reviewed every change, and it follows TDD with unit tests verified locally. I understand the code and can explain why it's correct.

Open to feedback on: (1) edit-based vs command-based (chose edit-based as there's nothing to compute server-side); (2) the {% liquid %} v1 skip vs emitting bare #; (3) the representative-offense behavior noted above.

@ryandiginomad ryandiginomad marked this pull request as ready for review May 29, 2026 02:39
@ryandiginomad ryandiginomad requested a review from a team as a code owner May 29, 2026 02:39
@ryandiginomad

Copy link
Copy Markdown
Contributor Author

Quick check-in — this PR has been ready for review for a week. CLA signed, CI green. Ready whenever the theme-tools team has a window, and happy to address anything that comes up.

@ryandiginomad

Copy link
Copy Markdown
Contributor Author

Quick check-in — this PR has been quiet on the review queue for a couple weeks. Since the last update I've also reworked the description to follow the PR template, including a Tophatting section with screenshots from a real extension host (per the feedback on #1218). Rebased, CI green, ready whenever the theme-tools team has a window.

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.

1 participant