fix: allow theme-check-disable-next-line to suppress checks inside doc tags#1189
Open
SinhSinhAn wants to merge 1 commit intoShopify:mainfrom
Open
fix: allow theme-check-disable-next-line to suppress checks inside doc tags#1189SinhSinhAn wants to merge 1 commit intoShopify:mainfrom
SinhSinhAn wants to merge 1 commit intoShopify:mainfrom
Conversation
…c tags
When `{% # theme-check-disable-next-line %}` was placed before a
`{% doc %}` tag, it only covered the opening `{% doc %}` marker itself
(via `blockStartPosition`), not the body. Checks like
UniqueDocParamNames, ValidDocParamTypes, and ReservedDocParamNames
report offenses at positions inside the doc body, so they were never
suppressed.
The fix: when the next node is a `{% doc %}` LiquidRawTag, use the
full `position` span (covering the entire tag including its body)
instead of just `blockStartPosition`. This is safe because doc tags
contain a flat list of doc-specific nodes (params, descriptions),
not nested Liquid control flow that should remain independently
checkable.
Closes Shopify#945
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this PR?
This fixes a bug where
{% # theme-check-disable-next-line %}placed before a{% doc %}tag doesn't actually suppress any of the checks that run against the doc tag's contents.What was the issue?
Issue #945 reported that you can't use
theme-check-disable-next-lineto silence checks inside{% doc %}tags. For example, if you have a snippet with duplicate param names that you want to temporarily ignore:{% # theme-check-disable-next-line UniqueDocParamNames %} {% doc %} @param prods - products @param prods - intentional duplicate for some reason {% enddoc %}The
UniqueDocParamNamesoffense would still fire despite the disable comment. The workaround was wrapping the entire doc tag in a disable/enable pair, which is verbose.Why it was happening
The
disable-next-linelogic infindNextLinePositionhas a guard for block tags: when the next node has ablockStartPosition(like{% if %},{% for %}, or{% doc %}), it intentionally returns only theblockStartPositionspan (just the opening marker, e.g., the literal{% doc %}characters) rather than the full tag body. This preventsdisable-next-linefrom accidentally suppressing checks on all children of control flow tags.The problem is that
{% doc %}tags are different from{% if %}tags. Doc tag checks (likeUniqueDocParamNames,ValidDocParamTypes,ReservedDocParamNames) report their offenses at positions inside the doc body (where the@paramdeclarations live). SinceblockStartPositiononly covers the opening{% doc %}marker, those offense positions fall outside the disabled range and are never suppressed.How this PR fixes it
When
disable-next-lineresolves the next node and that node is aLiquidRawTagwithname === 'doc', it now returns the fullpositionspan (covering the entire tag from{% doc %}to{% enddoc %}) instead of justblockStartPosition.This is safe because doc tags don't contain nested Liquid control flow. Their body is parsed by a separate grammar (
LiquidDocGrammar) that only produces doc-specific nodes (@param,@description,@example). There's no risk of accidentally silencing unrelated checks on deeply nested children.The change is a 4-line addition to
findNextLinePositionindisabled-checks/index.ts.Tests
Added 3 test cases to
disabled-checks/index.spec.ts:All 18 tests pass (15 existing + 3 new).
Closes #945