From 9af6d261031e45fac6ffe6b36a68ac1a00587934 Mon Sep 17 00:00:00 2001 From: SinhSinh An Date: Tue, 21 Apr 2026 10:56:59 -0500 Subject: [PATCH] fix: allow theme-check-disable-next-line to suppress checks inside doc 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 #945 --- .changeset/fix-disable-doc-tag.md | 5 +++ .../src/disabled-checks/index.spec.ts | 38 +++++++++++++++++++ .../src/disabled-checks/index.ts | 8 ++++ 3 files changed, 51 insertions(+) create mode 100644 .changeset/fix-disable-doc-tag.md diff --git a/.changeset/fix-disable-doc-tag.md b/.changeset/fix-disable-doc-tag.md new file mode 100644 index 000000000..b1aed52d7 --- /dev/null +++ b/.changeset/fix-disable-doc-tag.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': patch +--- + +Fix theme-check-disable-next-line not working for checks inside doc tags diff --git a/packages/theme-check-common/src/disabled-checks/index.spec.ts b/packages/theme-check-common/src/disabled-checks/index.spec.ts index b8d4a2f55..7c7233dc2 100644 --- a/packages/theme-check-common/src/disabled-checks/index.spec.ts +++ b/packages/theme-check-common/src/disabled-checks/index.spec.ts @@ -3,6 +3,7 @@ import { Offense } from '..'; import { describe, it, expect } from 'vitest'; import { LiquidFilter, RenderMarkup } from './test-checks'; import { UndefinedObject } from '../checks/undefined-object'; +import { UniqueDocParamNames } from '../checks/unique-doc-param-names'; const commentTypes = [ (text: string) => `{% # ${text} %}`, @@ -257,6 +258,43 @@ ${buildComment('theme-check-enable')} }), ); }); + + it('should disable checks inside doc tags when disable-next-line is placed before the doc tag', async () => { + const file = `{% # theme-check-disable-next-line UniqueDocParamNames %} +{% doc %} + @param foo - first param + @param foo - duplicate param +{% enddoc %}`; + + const offenses = await check({ 'code.liquid': file }, [UniqueDocParamNames]); + expect(offenses).to.have.length(0); + }); + + it('should disable all checks inside doc tags when disable-next-line is used without specific check names', async () => { + const file = `{% # theme-check-disable-next-line %} +{% doc %} + @param bar - first param + @param bar - duplicate param +{% enddoc %}`; + + const offenses = await check({ 'code.liquid': file }, [UniqueDocParamNames]); + expect(offenses).to.have.length(0); + }); + + it('should still report offenses in doc tags when disable-next-line is not present', async () => { + const file = `{% doc %} + @param baz - first param + @param baz - duplicate param +{% enddoc %}`; + + const offenses = await check({ 'code.liquid': file }, [UniqueDocParamNames]); + expect(offenses).to.have.length(1); + expect(offenses).toContainEqual( + expect.objectContaining({ + message: "The parameter 'baz' is defined more than once.", + }), + ); + }); }); }); }); diff --git a/packages/theme-check-common/src/disabled-checks/index.ts b/packages/theme-check-common/src/disabled-checks/index.ts index 09b0f20a5..dd925df39 100644 --- a/packages/theme-check-common/src/disabled-checks/index.ts +++ b/packages/theme-check-common/src/disabled-checks/index.ts @@ -154,8 +154,16 @@ export function findNextLinePosition( /* * If the node contains children nodes, we don't want to disable checks for them. * We want to keep it exclusively to the tag itself. + * + * Exception: For `{% doc %}` tags, the checks we care about (ValidDocParamTypes, + * UniqueDocParamNames, ReservedDocParamNames, etc.) report offenses at positions + * inside the doc body. Using only `blockStartPosition` would miss those offenses, + * so we use the full `position` span to cover the entire tag including its body. */ if ('blockStartPosition' in nextNode) { + if (nextNode.type === LiquidHtmlNodeTypes.LiquidRawTag && nextNode.name === 'doc') { + return nextNode.position; + } return nextNode.blockStartPosition; }