From c9fe538566e3dbf79f8066f84ad617d191efd6e2 Mon Sep 17 00:00:00 2001 From: Orlando Qiu Date: Wed, 1 Oct 2025 15:39:41 -0400 Subject: [PATCH 1/2] add command to insert theme-check disable comments and tests --- .../src/commands/ExecuteCommandProvider.ts | 14 +- .../ApplyDisableCheckProvider.spec.ts | 326 ++++++++++++++++++ .../providers/ApplyDisableCheckProvider.ts | 244 +++++++++++++ .../src/commands/providers/index.ts | 1 + 4 files changed, 584 insertions(+), 1 deletion(-) create mode 100644 packages/theme-language-server-common/src/commands/providers/ApplyDisableCheckProvider.spec.ts create mode 100644 packages/theme-language-server-common/src/commands/providers/ApplyDisableCheckProvider.ts diff --git a/packages/theme-language-server-common/src/commands/ExecuteCommandProvider.ts b/packages/theme-language-server-common/src/commands/ExecuteCommandProvider.ts index 9439748cd..f6f3dfd79 100644 --- a/packages/theme-language-server-common/src/commands/ExecuteCommandProvider.ts +++ b/packages/theme-language-server-common/src/commands/ExecuteCommandProvider.ts @@ -3,9 +3,15 @@ import { ClientCapabilities } from '../ClientCapabilities'; import { DiagnosticsManager } from '../diagnostics'; import { DocumentManager } from '../documents'; import { BaseExecuteCommandProvider } from './BaseExecuteCommandProvider'; -import { ApplyFixesProvider, ApplySuggestionProvider, RunChecksProvider } from './providers'; +import { + ApplyDisableCheckProvider, + ApplyFixesProvider, + ApplySuggestionProvider, + RunChecksProvider, +} from './providers'; export const Commands = [ + ApplyDisableCheckProvider.command, ApplyFixesProvider.command, ApplySuggestionProvider.command, RunChecksProvider.command, @@ -28,6 +34,12 @@ export class ExecuteCommandProvider { connection: Connection, ) { this.commands = { + [ApplyDisableCheckProvider.command]: new ApplyDisableCheckProvider( + documentManager, + diagnosticsManager, + clientCapabilities, + connection, + ), [ApplyFixesProvider.command]: new ApplyFixesProvider( documentManager, diagnosticsManager, diff --git a/packages/theme-language-server-common/src/commands/providers/ApplyDisableCheckProvider.spec.ts b/packages/theme-language-server-common/src/commands/providers/ApplyDisableCheckProvider.spec.ts new file mode 100644 index 000000000..f305fcab3 --- /dev/null +++ b/packages/theme-language-server-common/src/commands/providers/ApplyDisableCheckProvider.spec.ts @@ -0,0 +1,326 @@ +import { path } from '@shopify/theme-check-common'; +import { beforeEach, describe, expect, it, Mock, vi } from 'vitest'; +import { URI } from 'vscode-uri'; +import { DiagnosticsManager } from '../../diagnostics'; +import { DocumentManager } from '../../documents'; +import { ApplyDisableCheckProvider } from './ApplyDisableCheckProvider'; + +describe('Unit: ApplyDisableCheckProvider', () => { + const liquidUri = path.normalize(URI.file('/path/to/file.liquid')); + const liquidContents = ` + {% assign x = 1 %} + + + `; + let connection: { sendRequest: Mock; sendDiagnostics: Mock }; + let documentManager: DocumentManager; + let diagnosticsManager: DiagnosticsManager; + let applyDisableCheckProvider: ApplyDisableCheckProvider; + + beforeEach(() => { + connection = { sendRequest: vi.fn(), sendDiagnostics: vi.fn() }; + documentManager = new DocumentManager(); + diagnosticsManager = new DiagnosticsManager(connection as any); + const capabilities = { hasApplyEditSupport: true }; + applyDisableCheckProvider = new ApplyDisableCheckProvider( + documentManager, + diagnosticsManager, + capabilities as any, + connection as any, + ); + }); + + describe('execute - Liquid files', () => { + beforeEach(() => { + documentManager.open(liquidUri, liquidContents, 1); + diagnosticsManager.set(liquidUri, 1, []); + }); + + it('adds disable-next-line comment with proper indentation', async () => { + await applyDisableCheckProvider.execute(liquidUri, 1, 'next-line', 'UnusedAssign', 1); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 1 }, + edits: [ + { + newText: ' {% # theme-check-disable-next-line UnusedAssign %}\n', + range: { + start: { line: 1, character: 0 }, + end: { line: 1, character: 0 }, + }, + }, + ], + }, + ], + }, + }); + }); + + it('adds disable comment at top of file for file-level disable', async () => { + await applyDisableCheckProvider.execute(liquidUri, 1, 'file', 'ParserBlockingScript', 0); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 1 }, + edits: [ + { + newText: '{% # theme-check-disable ParserBlockingScript %}\n', + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, + }, + }, + ], + }, + ], + }, + }); + }); + + it('handles lines with no indentation', async () => { + const noIndentContents = `{% assign x = 1 %} +`; + documentManager.open(liquidUri, noIndentContents, 2); + diagnosticsManager.set(liquidUri, 2, []); + + await applyDisableCheckProvider.execute(liquidUri, 2, 'next-line', 'UnusedAssign', 0); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 2 }, + edits: [ + { + newText: '{% # theme-check-disable-next-line UnusedAssign %}\n', + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, + }, + }, + ], + }, + ], + }, + }); + }); + + it('preserves tabs in indentation', async () => { + const tabContents = `\t\t{% assign x = 1 %}`; + documentManager.open(liquidUri, tabContents, 3); + diagnosticsManager.set(liquidUri, 3, []); + + await applyDisableCheckProvider.execute(liquidUri, 3, 'next-line', 'UnusedAssign', 0); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 3 }, + edits: [ + { + newText: '\t\t{% # theme-check-disable-next-line UnusedAssign %}\n', + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, + }, + }, + ], + }, + ], + }, + }); + }); + + it('uses plain comment format inside liquid tags', async () => { + const liquidTagContents = ` +{% liquid + assign x = 1 + echo x +%}`; + documentManager.open(liquidUri, liquidTagContents, 1); + diagnosticsManager.set(liquidUri, 1, []); + + await applyDisableCheckProvider.execute(liquidUri, 1, 'next-line', 'UnusedAssign', 2); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 1 }, + edits: [ + { + newText: ' # theme-check-disable-next-line UnusedAssign\n', + range: { + start: { line: 2, character: 0 }, + end: { line: 2, character: 0 }, + }, + }, + ], + }, + ], + }, + }); + }); + + it('uses regular comment format for file-level disable even inside liquid tags', async () => { + const liquidTagContents = ` +{% liquid + assign x = 1 + echo x +%}`; + documentManager.open(liquidUri, liquidTagContents, 1); + diagnosticsManager.set(liquidUri, 1, []); + + // File-level disable should always use regular format + await applyDisableCheckProvider.execute(liquidUri, 1, 'file', 'UnusedAssign', 2); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 1 }, + edits: [ + { + newText: '{% # theme-check-disable UnusedAssign %}\n', + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, + }, + }, + ], + }, + ], + }, + }); + }); + + it('appends to existing disable-next-line comment', async () => { + const contentsWithComment = ` + {% assign x = 1 %} + {% # theme-check-disable-next-line UnusedAssign %} + `; + documentManager.open(liquidUri, contentsWithComment, 1); + diagnosticsManager.set(liquidUri, 1, []); + + await applyDisableCheckProvider.execute(liquidUri, 1, 'next-line', 'ParserBlockingScript', 3); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 1 }, + edits: [ + { + newText: + '{% # theme-check-disable-next-line UnusedAssign, ParserBlockingScript %}', + range: { + start: { line: 2, character: 4 }, + end: { line: 2, character: 54 }, + }, + }, + ], + }, + ], + }, + }); + }); + + it('appends to existing file-level disable comment', async () => { + const contentsWithComment = `{% # theme-check-disable UnusedAssign %} + {% assign x = 1 %} + `; + documentManager.open(liquidUri, contentsWithComment, 1); + diagnosticsManager.set(liquidUri, 1, []); + + await applyDisableCheckProvider.execute(liquidUri, 1, 'file', 'ParserBlockingScript', 2); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 1 }, + edits: [ + { + newText: '{% # theme-check-disable UnusedAssign, ParserBlockingScript %}', + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 40 }, + }, + }, + ], + }, + ], + }, + }); + }); + + it('appends to existing liquid tag disable comment', async () => { + const liquidTagContents = ` +{% liquid + # theme-check-disable-next-line UnusedAssign + assign x = 1 + echo x +%}`; + documentManager.open(liquidUri, liquidTagContents, 1); + diagnosticsManager.set(liquidUri, 1, []); + + await applyDisableCheckProvider.execute(liquidUri, 1, 'next-line', 'LiquidFilter', 3); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 1 }, + edits: [ + { + newText: '# theme-check-disable-next-line UnusedAssign, LiquidFilter', + range: { + start: { line: 2, character: 2 }, + end: { line: 2, character: 46 }, + }, + }, + ], + }, + ], + }, + }); + }); + + it('appends to existing disable comment even with blank lines in between', async () => { + const contentsWithBlankLine = ` + {% # theme-check-disable-next-line MissingTemplate %} + + {% render 'test' %}`; + documentManager.open(liquidUri, contentsWithBlankLine, 1); + diagnosticsManager.set(liquidUri, 1, []); + + // Try to disable SyntaxError on line 3 + await applyDisableCheckProvider.execute(liquidUri, 1, 'next-line', 'SyntaxError', 3); + + expect(connection.sendRequest).toHaveBeenCalledWith(expect.anything(), { + edit: { + documentChanges: [ + { + textDocument: { uri: liquidUri, version: 1 }, + edits: [ + { + newText: '{% # theme-check-disable-next-line MissingTemplate, SyntaxError %}', + range: { + start: { line: 1, character: 4 }, + end: { line: 1, character: 57 }, + }, + }, + ], + }, + ], + }, + }); + }); + }); +}); diff --git a/packages/theme-language-server-common/src/commands/providers/ApplyDisableCheckProvider.ts b/packages/theme-language-server-common/src/commands/providers/ApplyDisableCheckProvider.ts new file mode 100644 index 000000000..fb9956705 --- /dev/null +++ b/packages/theme-language-server-common/src/commands/providers/ApplyDisableCheckProvider.ts @@ -0,0 +1,244 @@ +import { SourceCodeType, findCurrentNode, LiquidHtmlNode } from '@shopify/theme-check-common'; +import { NodeTypes, LiquidTag } from '@shopify/liquid-html-parser'; +import { + ApplyWorkspaceEditRequest, + Command, + TextDocumentEdit, + TextEdit, +} from 'vscode-languageserver'; +import { TextDocument } from 'vscode-languageserver-textdocument'; +import { BaseExecuteCommandProvider } from '../BaseExecuteCommandProvider'; +import { AugmentedLiquidSourceCode } from '../../documents/types'; + +export class ApplyDisableCheckProvider extends BaseExecuteCommandProvider { + static command = 'themeCheck/applyDisableCheck' as const; + + async execute( + uri: string, + version: number | undefined, + type: 'next-line' | 'file', + checkName: string, + lineNumber: number, + ) { + const document = this.documentManager.get(uri); + const diagnostics = this.diagnosticsManager.get(uri); + if (!document || !diagnostics) return; + if (document.version !== version || diagnostics.version !== version) return; + + if (!this.clientCapabilities.hasApplyEditSupport) return; + + if (document.type !== SourceCodeType.LiquidHtml) { + this.connection.console.warn('Disable check comments are only supported in Liquid files'); + return; + } + + const { textDocument } = document; + + const isInLiquidTag = this.isInLiquidTagContext(document, lineNumber); + const disableComment = this.getDisableComment(checkName, type, isInLiquidTag); + let textEdit: TextEdit; + + switch (type) { + case 'next-line': { + let existingCommentLine = -1; + + for (let i = lineNumber - 1; i >= 0; i--) { + const line = textDocument.getText({ + start: { line: i, character: 0 }, + end: { line: i + 1, character: 0 }, + }); + + if (this.hasDisableComment(line, 'next-line', isInLiquidTag)) { + existingCommentLine = i; + break; + } + + if (line.trim() !== '' && !line.includes('theme-check-disable')) { + break; + } + } + + if (existingCommentLine >= 0) { + textEdit = this.appendCheckToComment( + textDocument, + existingCommentLine, + checkName, + 'next-line', + isInLiquidTag, + ); + } else { + const lineText = textDocument.getText({ + start: { line: lineNumber, character: 0 }, + end: { line: lineNumber + 1, character: 0 }, + }); + const indent = lineText.match(/^\s*/)?.[0] || ''; + textEdit = TextEdit.insert( + { line: lineNumber, character: 0 }, + `${indent}${disableComment}\n`, + ); + } + break; + } + case 'file': { + let existingCommentLine = -1; + + for (let i = 0; i < textDocument.lineCount; i++) { + const line = textDocument.getText({ + start: { line: i, character: 0 }, + end: { line: i + 1, character: 0 }, + }); + if (this.hasDisableComment(line, 'file', false)) { + existingCommentLine = i; + break; + } + if (line.trim() !== '' && !line.includes('theme-check-disable')) { + break; + } + } + + if (existingCommentLine >= 0) { + textEdit = this.appendCheckToComment( + textDocument, + existingCommentLine, + checkName, + 'file', + false, + ); + } else { + const fileComment = this.getDisableComment(checkName, type, false); + textEdit = TextEdit.insert({ line: 0, character: 0 }, `${fileComment}\n`); + } + break; + } + } + + const textDocumentEdit = TextDocumentEdit.create( + { uri: textDocument.uri, version: textDocument.version }, + [textEdit], + ); + + await this.connection.sendRequest(ApplyWorkspaceEditRequest.type, { + edit: { + documentChanges: [textDocumentEdit], + }, + }); + } + + private getDisableComment( + checkName: string, + type: 'next-line' | 'file', + isInLiquidTag: boolean, + ): string { + const comment = this.getDisableCommentGeneral(checkName, type); + return isInLiquidTag ? `${comment}` : `{% ${comment} %}`; + } + + private getDisableCommentGeneral(checkName: string, type: 'next-line' | 'file'): string { + return type === 'next-line' + ? `# theme-check-disable-next-line ${checkName}` + : `# theme-check-disable ${checkName}`; + } + + private isInLiquidTagContext(document: AugmentedLiquidSourceCode, lineNumber: number): boolean { + if (!document.ast || document.ast instanceof Error) { + return false; + } + + const offset = document.textDocument.offsetAt({ line: lineNumber, character: 0 }); + const [currentNode, ancestors] = findCurrentNode(document.ast, offset); + + const allNodes = currentNode ? [currentNode, ...ancestors] : ancestors; + return allNodes.some(this.isLiquidTag); + } + + private isLiquidTag(node: LiquidHtmlNode | undefined): node is LiquidTag & { name: 'liquid' } { + if (!node) return false; + return node.type === NodeTypes.LiquidTag && node.name === 'liquid'; + } + + private hasDisableComment( + line: string, + type: 'next-line' | 'file', + isInLiquidTag: boolean, + ): boolean { + const trimmed = line.trim(); + if (isInLiquidTag) { + return type === 'next-line' + ? trimmed.startsWith('# theme-check-disable-next-line') + : trimmed.startsWith('# theme-check-disable') && !trimmed.includes('disable-next-line'); + } + return type === 'next-line' + ? trimmed.startsWith('{% # theme-check-disable-next-line') && trimmed.endsWith('%}') + : trimmed.startsWith('{% # theme-check-disable') && + !trimmed.includes('disable-next-line') && + trimmed.endsWith('%}'); + } + + private appendCheckToComment( + textDocument: TextDocument, + lineNumber: number, + checkName: string, + type: 'next-line' | 'file', + isInLiquidTag: boolean, + ): TextEdit { + const line = textDocument.getText({ + start: { line: lineNumber, character: 0 }, + end: { line: lineNumber + 1, character: 0 }, + }); + const trimmedLine = line.replace(/\n$/, ''); + + const indent = line.match(/^\s*/)?.[0] || ''; + const commentStartChar = indent.length; + + const checksPattern = isInLiquidTag + ? type === 'next-line' + ? /# theme-check-disable-next-line (.+)$/ + : /# theme-check-disable (.+)$/ + : type === 'next-line' + ? /{% # theme-check-disable-next-line (.+) %}$/ + : /{% # theme-check-disable (.+) %}$/; + + const match = trimmedLine.match(checksPattern); + if (!match) { + const comment = this.getDisableComment(checkName, type, isInLiquidTag); + return TextEdit.insert({ line: lineNumber, character: 0 }, `${indent}${comment}\n`); + } + + const checksList = match[1]; + const checks = checksList.split(',').map((c) => c.trim()); + + checks.push(checkName); + + const updatedChecks = checks.join(', '); + const commentContent = trimmedLine.substring(commentStartChar); + const updatedComment = commentContent.replace(checksList, updatedChecks); + + return TextEdit.replace( + { + start: { line: lineNumber, character: commentStartChar }, + end: { line: lineNumber, character: trimmedLine.length }, + }, + updatedComment, + ); + } +} + +export function applyDisableCheckCommand( + uri: string, + version: number | undefined, + type: 'next-line' | 'file', + checkName: string, + lineNumber: number, +): Command { + return Command.create( + type === 'next-line' + ? `Disable ${checkName} for this line` + : `Disable ${checkName} for entire file`, + ApplyDisableCheckProvider.command, + uri, + version, + type, + checkName, + lineNumber, + ); +} diff --git a/packages/theme-language-server-common/src/commands/providers/index.ts b/packages/theme-language-server-common/src/commands/providers/index.ts index 76f55af71..e866bce10 100644 --- a/packages/theme-language-server-common/src/commands/providers/index.ts +++ b/packages/theme-language-server-common/src/commands/providers/index.ts @@ -1,3 +1,4 @@ +export { applyDisableCheckCommand, ApplyDisableCheckProvider } from './ApplyDisableCheckProvider'; export { applyFixCommand, ApplyFixesProvider } from './ApplyFixesProvider'; export { applySuggestionCommand, ApplySuggestionProvider } from './ApplySuggestionProvider'; export { RunChecksProvider } from './RunChecksProvider'; From b0e517b6dd322830c6e1bc5b15664f103a377b89 Mon Sep 17 00:00:00 2001 From: Orlando Qiu Date: Wed, 1 Oct 2025 15:39:57 -0400 Subject: [PATCH 2/2] add code actions for disabling theme-check rules --- .changeset/odd-rice-return.md | 5 + .../src/codeActions/CodeActionsProvider.ts | 10 +- .../providers/DisableCheckProvider.spec.ts | 267 ++++++++++++++++++ .../providers/DisableCheckProvider.ts | 103 +++++++ .../src/codeActions/providers/index.ts | 1 + 5 files changed, 384 insertions(+), 2 deletions(-) create mode 100644 .changeset/odd-rice-return.md create mode 100644 packages/theme-language-server-common/src/codeActions/providers/DisableCheckProvider.spec.ts create mode 100644 packages/theme-language-server-common/src/codeActions/providers/DisableCheckProvider.ts diff --git a/.changeset/odd-rice-return.md b/.changeset/odd-rice-return.md new file mode 100644 index 000000000..b01631826 --- /dev/null +++ b/.changeset/odd-rice-return.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-language-server-common': minor +--- + +Added Disable check option in the quick fix menu to disable the theme-check warning/error either for just that line or for the entire file diff --git a/packages/theme-language-server-common/src/codeActions/CodeActionsProvider.ts b/packages/theme-language-server-common/src/codeActions/CodeActionsProvider.ts index fe05cf119..d8f725424 100644 --- a/packages/theme-language-server-common/src/codeActions/CodeActionsProvider.ts +++ b/packages/theme-language-server-common/src/codeActions/CodeActionsProvider.ts @@ -1,11 +1,16 @@ import { CodeAction, CodeActionParams, Command } from 'vscode-languageserver'; import { DiagnosticsManager } from '../diagnostics'; import { DocumentManager } from '../documents'; -import { FixAllProvider, FixProvider, SuggestionProvider } from './providers'; +import { FixAllProvider, FixProvider, SuggestionProvider, DisableCheckProvider } from './providers'; import { BaseCodeActionsProvider } from './BaseCodeActionsProvider'; export const CodeActionKinds = Array.from( - new Set([FixAllProvider.kind, FixProvider.kind, SuggestionProvider.kind]), + new Set([ + DisableCheckProvider.kind, + FixAllProvider.kind, + FixProvider.kind, + SuggestionProvider.kind, + ]), ); export class CodeActionsProvider { @@ -13,6 +18,7 @@ export class CodeActionsProvider { constructor(documentManager: DocumentManager, diagnosticsManager: DiagnosticsManager) { this.providers = [ + new DisableCheckProvider(documentManager, diagnosticsManager), new FixAllProvider(documentManager, diagnosticsManager), new FixProvider(documentManager, diagnosticsManager), new SuggestionProvider(documentManager, diagnosticsManager), diff --git a/packages/theme-language-server-common/src/codeActions/providers/DisableCheckProvider.spec.ts b/packages/theme-language-server-common/src/codeActions/providers/DisableCheckProvider.spec.ts new file mode 100644 index 000000000..de4ad51ab --- /dev/null +++ b/packages/theme-language-server-common/src/codeActions/providers/DisableCheckProvider.spec.ts @@ -0,0 +1,267 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { URI } from 'vscode-uri'; +import { Offense, SourceCodeType, Severity, path } from '@shopify/theme-check-common'; +import { DiagnosticsManager } from '../../diagnostics'; +import { DocumentManager } from '../../documents'; +import { DisableCheckProvider } from './DisableCheckProvider'; +import { TextDocument } from 'vscode-languageserver-textdocument'; + +describe('Unit: DisableCheckProvider', () => { + const liquidUri = path.normalize(URI.file('/path/to/file.liquid')); + const liquidContents = ` + {% assign x = 1 %} + + + `; + const liquidDocument = TextDocument.create(liquidUri, 'liquid', 0, liquidContents); + let documentManager: DocumentManager; + let diagnosticsManager: DiagnosticsManager; + let disableCheckProvider: DisableCheckProvider; + + function makeOffense(checkName: string, needle: string): Offense { + const start = liquidContents.indexOf(needle); + const end = start + needle.length; + + const messages: Record = { + UnusedAssign: `Variable 'x' is defined but not used`, + ParserBlockingScript: 'Parser blocking script detected', + }; + + return { + type: SourceCodeType.LiquidHtml, + check: checkName, + message: messages[checkName] || 'Offense detected', + uri: liquidUri, + severity: Severity.ERROR, + start: { ...liquidDocument.positionAt(start), index: start }, + end: { ...liquidDocument.positionAt(end), index: end }, + }; + } + + beforeEach(() => { + documentManager = new DocumentManager(); + diagnosticsManager = new DiagnosticsManager({ sendDiagnostics: vi.fn() } as any); + disableCheckProvider = new DisableCheckProvider(documentManager, diagnosticsManager); + }); + + describe('Liquid files', () => { + beforeEach(() => { + documentManager.open(liquidUri, liquidContents, 1); + }); + + describe('When single offense exists under cursor', () => { + beforeEach(() => { + diagnosticsManager.set(liquidUri, 1, [makeOffense('UnusedAssign', '{% assign x = 1 %}')]); + }); + + it('provides disable actions for the offense', () => { + const codeActions = disableCheckProvider.codeActions({ + textDocument: { uri: liquidUri }, + range: { + start: liquidDocument.positionAt(liquidContents.indexOf('assign')), + end: liquidDocument.positionAt(liquidContents.indexOf('assign')), + }, + context: { diagnostics: [] }, + }); + + expect(codeActions.length).toBe(2); + + const [disableNextLineAction, disableFileAction] = codeActions; + + expect(disableNextLineAction).toEqual({ + title: 'Disable UnusedAssign for this line', + kind: 'quickfix', + diagnostics: expect.any(Array), + isPreferred: false, + command: { + title: 'Disable UnusedAssign for this line', + command: 'themeCheck/applyDisableCheck', + arguments: [liquidUri, 1, 'next-line', 'UnusedAssign', 1], + }, + }); + + expect(disableFileAction).toEqual({ + title: 'Disable UnusedAssign for entire file', + kind: 'quickfix', + diagnostics: expect.any(Array), + isPreferred: false, + command: { + title: 'Disable UnusedAssign for entire file', + command: 'themeCheck/applyDisableCheck', + arguments: [liquidUri, 1, 'file', 'UnusedAssign', 0], + }, + }); + }); + }); + + describe('When multiple offenses of same type exist on same line', () => { + beforeEach(() => { + diagnosticsManager.set(liquidUri, 1, [ + makeOffense('ParserBlockingScript', ' { + const codeActions = disableCheckProvider.codeActions({ + textDocument: { uri: liquidUri }, + range: { + start: liquidDocument.positionAt(liquidContents.indexOf('') + ''.length, + ), + }, + context: { diagnostics: [] }, + }); + + expect(codeActions.length).toBe(2); + + const disableNextLineActions = codeActions.filter( + (action) => + typeof action.command === 'object' && action.command?.arguments?.[2] === 'next-line', + ); + expect(disableNextLineActions.length).toBe(1); + expect(disableNextLineActions[0].title).toBe('Disable ParserBlockingScript for this line'); + }); + }); + + describe('When multiple offenses of various types exist', () => { + beforeEach(() => { + diagnosticsManager.set(liquidUri, 1, [ + makeOffense('UnusedAssign', '{% assign x = 1 %}'), + makeOffense('ParserBlockingScript', ''), + makeOffense('ParserBlockingScript', ''), + ]); + }); + + it('provides disable actions for each unique check under cursor', () => { + const codeActions = disableCheckProvider.codeActions({ + textDocument: { uri: liquidUri }, + range: { + start: liquidDocument.positionAt(0), + end: liquidDocument.positionAt(liquidContents.length), + }, + context: { diagnostics: [] }, + }); + + // Should have 2 checks × 2 action types = 4 actions + // But duplicates on same line are filtered + const uniqueActions = new Set( + codeActions + .map((a) => { + if (typeof a.command === 'object' && a.command?.arguments) { + return `${a.command.arguments[3]}-${a.command.arguments[2]}`; + } + return ''; + }) + .filter(Boolean), + ); + + expect(uniqueActions.has('UnusedAssign-next-line')).toBe(true); + expect(uniqueActions.has('UnusedAssign-file')).toBe(true); + expect(uniqueActions.has('ParserBlockingScript-next-line')).toBe(true); + expect(uniqueActions.has('ParserBlockingScript-file')).toBe(true); + }); + }); + + describe('When cursor is not on any offense', () => { + beforeEach(() => { + diagnosticsManager.set(liquidUri, 1, [makeOffense('UnusedAssign', '{% assign x = 1 %}')]); + }); + + it('provides no code actions', () => { + const codeActions = disableCheckProvider.codeActions({ + textDocument: { uri: liquidUri }, + range: { + start: liquidDocument.positionAt(liquidContents.length - 1), + end: liquidDocument.positionAt(liquidContents.length), + }, + context: { diagnostics: [] }, + }); + + expect(codeActions).toEqual([]); + }); + }); + + describe('When inside stylesheet or javascript tags', () => { + it('provides no code actions inside stylesheet tags', () => { + const stylesheetContents = ` + {% stylesheet %} + @starting-style { + opacity: 0; + } + {% endstylesheet %} + `; + const stylesheetDocument = TextDocument.create(liquidUri, 'liquid', 1, stylesheetContents); + documentManager.open(liquidUri, stylesheetContents, 1); + + // Create a CSS-related offense inside the stylesheet tag + const errorText = '@starting-style'; + const errorStart = stylesheetContents.indexOf(errorText); + const errorEnd = errorStart + errorText.length; + + diagnosticsManager.set(liquidUri, 1, [ + { + type: SourceCodeType.LiquidHtml, + check: 'CSSCheck', + message: 'Unknown at rule @starting-style', + uri: liquidUri, + severity: Severity.ERROR, + start: { ...stylesheetDocument.positionAt(errorStart), index: errorStart }, + end: { ...stylesheetDocument.positionAt(errorEnd), index: errorEnd }, + }, + ]); + + const codeActions = disableCheckProvider.codeActions({ + textDocument: { uri: liquidUri }, + range: { + start: stylesheetDocument.positionAt(errorStart), + end: stylesheetDocument.positionAt(errorEnd), + }, + context: { diagnostics: [] }, + }); + + expect(codeActions).toEqual([]); + }); + + it('provides no code actions inside javascript tags', () => { + const javascriptContents = ` + {% javascript %} + console.log('error'); + const x = await fetch('/api'); + {% endjavascript %} + `; + const javascriptDocument = TextDocument.create(liquidUri, 'liquid', 1, javascriptContents); + documentManager.open(liquidUri, javascriptContents, 1); + + // Create a JavaScript-related offense inside the javascript tag + const errorText = 'await'; + const errorStart = javascriptContents.indexOf(errorText); + const errorEnd = errorStart + errorText.length; + + diagnosticsManager.set(liquidUri, 1, [ + { + type: SourceCodeType.LiquidHtml, + check: 'JSCheck', + message: 'await is only valid in async functions', + uri: liquidUri, + severity: Severity.ERROR, + start: { ...javascriptDocument.positionAt(errorStart), index: errorStart }, + end: { ...javascriptDocument.positionAt(errorEnd), index: errorEnd }, + }, + ]); + + const codeActions = disableCheckProvider.codeActions({ + textDocument: { uri: liquidUri }, + range: { + start: javascriptDocument.positionAt(errorStart), + end: javascriptDocument.positionAt(errorEnd), + }, + context: { diagnostics: [] }, + }); + + expect(codeActions).toEqual([]); + }); + }); + }); +}); diff --git a/packages/theme-language-server-common/src/codeActions/providers/DisableCheckProvider.ts b/packages/theme-language-server-common/src/codeActions/providers/DisableCheckProvider.ts new file mode 100644 index 000000000..58f5b548e --- /dev/null +++ b/packages/theme-language-server-common/src/codeActions/providers/DisableCheckProvider.ts @@ -0,0 +1,103 @@ +import { CodeAction, CodeActionKind, CodeActionParams, Command } from 'vscode-languageserver'; +import { SourceCodeType, findCurrentNode } from '@shopify/theme-check-common'; +import { LiquidRawTag, NodeTypes } from '@shopify/liquid-html-parser'; +import { applyDisableCheckCommand } from '../../commands/providers'; +import { BaseCodeActionsProvider } from '../BaseCodeActionsProvider'; +import { isInRange, toCodeAction } from './utils'; +import { Anomaly } from '../../diagnostics/DiagnosticsManager'; +import { TextDocument } from 'vscode-languageserver-textdocument'; + +export class DisableCheckProvider extends BaseCodeActionsProvider { + static kind = CodeActionKind.QuickFix; + + codeActions(params: CodeActionParams): (Command | CodeAction)[] { + const { uri } = params.textDocument; + const document = this.documentManager.get(uri); + const diagnostics = this.diagnosticsManager.get(uri); + if (!document || !diagnostics) return []; + + if (document.type !== SourceCodeType.LiquidHtml) return []; + + const { textDocument } = document; + const { anomalies, version } = diagnostics; + const start = textDocument.offsetAt(params.range.start); + const end = textDocument.offsetAt(params.range.end); + + // Don't show disable actions inside stylesheet/javascript tags + if (document.ast && !(document.ast instanceof Error)) { + const [, ancestors] = findCurrentNode(document.ast, start); + const isInsideRestrictedTag = ancestors.some( + (node): node is LiquidRawTag => + node.type === NodeTypes.LiquidRawTag && + (node.name === 'stylesheet' || node.name === 'javascript'), + ); + if (isInsideRestrictedTag) return []; + } + + const anomaliesUnderCursor = anomalies.filter((anomaly) => isInRange(anomaly, start, end)); + if (anomaliesUnderCursor.length === 0) return []; + + return [ + ...disableNextLineCursorActions(uri, version, anomaliesUnderCursor, textDocument), + ...disableEntireFileActions(uri, version, anomaliesUnderCursor), + ]; + } +} + +/** + * @returns code actions to disable only one of the offenses under the cursor + * @example Disable ParserBlockingScript for this line + */ +function disableNextLineCursorActions( + uri: string, + version: number | undefined, + anomaliesUnderCursor: Anomaly[], + textDocument: TextDocument, +): CodeAction[] { + // Group by check name to avoid duplicate disable actions for the same check on the same line + const checksByLine = new Map(); + + anomaliesUnderCursor.forEach((anomaly) => { + const key = `${anomaly.offense.check}-${ + textDocument.positionAt(anomaly.offense.start.index).line + }`; + if (!checksByLine.has(key)) { + checksByLine.set(key, anomaly); + } + }); + + return Array.from(checksByLine.values()).map(({ offense, diagnostic }) => { + const position = textDocument.positionAt(offense.start.index); + return toCodeAction( + `Disable ${offense.check} for this line`, + applyDisableCheckCommand(uri, version, 'next-line', offense.check, position.line), + [diagnostic], + DisableCheckProvider.kind, + ); + }); +} + +/** + * @returns code actions to disable all offenses of a particular type + * @example Disable ParserBlockingScript for entire file + */ +function disableEntireFileActions( + uri: string, + version: number | undefined, + anomaliesUnderCursor: Anomaly[], +): CodeAction[] { + // Group by check name to avoid duplicate disable actions for the same check + const checks = new Set(anomaliesUnderCursor.map((anomaly) => anomaly.offense.check)); + + return Array.from(checks).map((check) => { + const checkAnomalies = anomaliesUnderCursor.filter(({ offense }) => offense.check === check); + const diagnostics = checkAnomalies.map((a) => a.diagnostic); + + return toCodeAction( + `Disable ${check} for entire file`, + applyDisableCheckCommand(uri, version, 'file', check, 0), // lineNumber 0 for file-level + diagnostics, + DisableCheckProvider.kind, + ); + }); +} diff --git a/packages/theme-language-server-common/src/codeActions/providers/index.ts b/packages/theme-language-server-common/src/codeActions/providers/index.ts index acd48a4aa..e17180e44 100644 --- a/packages/theme-language-server-common/src/codeActions/providers/index.ts +++ b/packages/theme-language-server-common/src/codeActions/providers/index.ts @@ -1,3 +1,4 @@ export { FixProvider } from './FixProvider'; export { FixAllProvider } from './FixAllProvider'; export { SuggestionProvider } from './SuggestionProvider'; +export { DisableCheckProvider } from './DisableCheckProvider';