From c74850c854033e60de0d2a3ee92c84f6b32ffd8b Mon Sep 17 00:00:00 2001 From: CP Clermont Date: Thu, 16 Jan 2025 11:32:28 -0500 Subject: [PATCH] Add "On section rename" handling (#707) * Add "On section rename" handling When a `sections/*.liquid` file is renamed, we'll update automatically update references in the following locations: - `templates/*.json` files that were using the old name - `sections/*.json` files that were using the old name - Static section calls of the form `{% section 'old-name' %}` Fixes #546 * Remove stale comments --- .changeset/breezy-balloons-begin.md | 5 + .changeset/sixty-jobs-draw.md | 12 + .../src/types/schemas/index.ts | 1 + .../src/types/schemas/template.ts | 34 ++ .../src/renamed/RenameHandler.ts | 2 + .../renamed/handlers/BlockRenameHandler.ts | 57 +--- .../handlers/SectionRenameHandler.spec.ts | 321 ++++++++++++++++++ .../renamed/handlers/SectionRenameHandler.ts | 222 ++++++++++++ .../src/renamed/handlers/utils.ts | 22 ++ 9 files changed, 621 insertions(+), 55 deletions(-) create mode 100644 .changeset/breezy-balloons-begin.md create mode 100644 .changeset/sixty-jobs-draw.md create mode 100644 packages/theme-check-common/src/types/schemas/template.ts create mode 100644 packages/theme-language-server-common/src/renamed/handlers/SectionRenameHandler.spec.ts create mode 100644 packages/theme-language-server-common/src/renamed/handlers/SectionRenameHandler.ts create mode 100644 packages/theme-language-server-common/src/renamed/handlers/utils.ts diff --git a/.changeset/breezy-balloons-begin.md b/.changeset/breezy-balloons-begin.md new file mode 100644 index 000000000..c360636cb --- /dev/null +++ b/.changeset/breezy-balloons-begin.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme-check-common': patch +--- + +[Internal] Add template type interfaces diff --git a/.changeset/sixty-jobs-draw.md b/.changeset/sixty-jobs-draw.md new file mode 100644 index 000000000..b87186b70 --- /dev/null +++ b/.changeset/sixty-jobs-draw.md @@ -0,0 +1,12 @@ +--- +'@shopify/theme-language-server-common': minor +'theme-check-vscode': minor +--- + +Add "On section rename" handling + +When `sections/*.liquid` files are renamed, we will update all references to these files in their previous locations. This includes: + +- `templates/*.json` files that referenced the old file name +- `sections/*.json` files that referenced the old file name +- Static section calls formatted as `{% section 'old-name' %}` diff --git a/packages/theme-check-common/src/types/schemas/index.ts b/packages/theme-check-common/src/types/schemas/index.ts index ebcd616d8..d861e0d3e 100644 --- a/packages/theme-check-common/src/types/schemas/index.ts +++ b/packages/theme-check-common/src/types/schemas/index.ts @@ -2,3 +2,4 @@ export { ThemeBlock } from './theme-block'; export { Section } from './section'; export { Setting } from './setting'; export { Preset } from './preset'; +export { Template } from './template'; diff --git a/packages/theme-check-common/src/types/schemas/template.ts b/packages/theme-check-common/src/types/schemas/template.ts new file mode 100644 index 000000000..0fb44fafe --- /dev/null +++ b/packages/theme-check-common/src/types/schemas/template.ts @@ -0,0 +1,34 @@ +import { Setting } from './setting'; + +export namespace Template { + export interface Template { + layout?: string | false; + wrapper?: string; + sections: Record; + order: string[]; + } + + export interface Section { + type: string; + settings?: Setting.Values; + disabled?: boolean; + blocks?: Record; + block_order?: string[]; + } + + export interface Block { + type: string; + settings?: Setting.Values; + disabled?: boolean; + blocks?: Record; + block_order?: string[]; + static?: boolean; + } + + export interface SectionGroup { + type: string; + name: string; + sections: Record; + order: string[]; + } +} diff --git a/packages/theme-language-server-common/src/renamed/RenameHandler.ts b/packages/theme-language-server-common/src/renamed/RenameHandler.ts index a0a8dd23e..7cc86b65e 100644 --- a/packages/theme-language-server-common/src/renamed/RenameHandler.ts +++ b/packages/theme-language-server-common/src/renamed/RenameHandler.ts @@ -6,6 +6,7 @@ import { BaseRenameHandler } from './BaseRenameHandler'; import { AssetRenameHandler } from './handlers/AssetRenameHandler'; import { BlockRenameHandler } from './handlers/BlockRenameHandler'; import { SnippetRenameHandler } from './handlers/SnippetRenameHandler'; +import { SectionRenameHandler } from './handlers/SectionRenameHandler'; /** * The RenameHandler is responsible for handling workspace/didRenameFiles notifications. @@ -27,6 +28,7 @@ export class RenameHandler { new SnippetRenameHandler(documentManager, connection, capabilities, findThemeRootURI), new AssetRenameHandler(documentManager, connection, capabilities, findThemeRootURI), new BlockRenameHandler(documentManager, connection, capabilities, findThemeRootURI), + new SectionRenameHandler(documentManager, connection, capabilities, findThemeRootURI), ]; } diff --git a/packages/theme-language-server-common/src/renamed/handlers/BlockRenameHandler.ts b/packages/theme-language-server-common/src/renamed/handlers/BlockRenameHandler.ts index 3f9effedd..8b051c86d 100644 --- a/packages/theme-language-server-common/src/renamed/handlers/BlockRenameHandler.ts +++ b/packages/theme-language-server-common/src/renamed/handlers/BlockRenameHandler.ts @@ -9,8 +9,8 @@ import { path, Preset, Section, - Setting, SourceCodeType, + Template, ThemeBlock, visit, } from '@shopify/theme-check-common'; @@ -35,42 +35,10 @@ import { } from '../../documents'; import { blockName, isBlock, isSection, isSectionGroup, isTemplate } from '../../utils/uri'; import { BaseRenameHandler } from '../BaseRenameHandler'; +import { isValidSectionGroup, isValidTemplate } from './utils'; type DocumentChange = TextDocumentEdit; -export namespace Template { - export interface Template { - layout?: string | false; - wrapper?: string; - sections: Record; - order: string[]; - } - - export interface Section { - type: string; - settings?: Setting.Values; - disabled?: boolean; - blocks?: Record; - block_order?: string[]; - } - - export interface Block { - type: string; - settings?: Setting.Values; - disabled?: boolean; - blocks?: Record; - block_order?: string[]; - static?: boolean; - } - - export interface SectionGroup { - type: string; - name: string; - sections: Record; - order: string[]; - } -} - const annotationId = 'renameBlock'; /** @@ -392,27 +360,6 @@ function isLocalBlock(blockDef: ThemeBlock.Block | Section.Block): blockDef is S return 'name' in blockDef && typeof blockDef.name === 'string'; } -// this is very very optimistic... -function isValidTemplate(parsed: unknown): parsed is Template.Template { - return ( - typeof parsed === 'object' && - parsed !== null && - 'sections' in parsed && - 'order' in parsed && - Array.isArray((parsed as Template.Template).order) - ); -} - -function isValidSectionGroup(parsed: unknown): parsed is Template.SectionGroup { - return ( - typeof parsed === 'object' && - parsed !== null && - 'sections' in parsed && - 'order' in parsed && - Array.isArray((parsed as Template.SectionGroup).order) - ); -} - function getBlocksEditsFactory( oldBlockName: string, newBlockName: string, diff --git a/packages/theme-language-server-common/src/renamed/handlers/SectionRenameHandler.spec.ts b/packages/theme-language-server-common/src/renamed/handlers/SectionRenameHandler.spec.ts new file mode 100644 index 000000000..2b8da2c2b --- /dev/null +++ b/packages/theme-language-server-common/src/renamed/handlers/SectionRenameHandler.spec.ts @@ -0,0 +1,321 @@ +import { MockFileSystem } from '@shopify/theme-check-common/src/test'; +import { assert, beforeEach, describe, expect, it } from 'vitest'; +import { TextDocumentEdit } from 'vscode-json-languageservice'; +import { ApplyWorkspaceEditParams } from 'vscode-languageserver-protocol'; +import { ClientCapabilities } from '../../ClientCapabilities'; +import { DocumentManager } from '../../documents'; +import { MockConnection, mockConnection } from '../../test/MockConnection'; +import { RenameHandler } from '../RenameHandler'; + +describe('Module: SectionRenameHandler', () => { + const mockRoot = 'mock-fs:'; + const findThemeRootURI = async () => mockRoot; + let capabilities: ClientCapabilities; + let documentManager: DocumentManager; + let handler: RenameHandler; + let connection: MockConnection; + let fs: MockFileSystem; + + beforeEach(() => { + connection = mockConnection(mockRoot); + connection.spies.sendRequest.mockReturnValue(Promise.resolve(true)); + capabilities = new ClientCapabilities(); + fs = new MockFileSystem( + { + 'sections/old-name.liquid': ` + {% schema %} + { + "name": "Old name" + } + {% endschema %}`, + + 'layout/theme.liquid': ` + {% section 'old-name' %} + `, + + 'sections/header.json': ` + { + "type": "header", + "name": "Header", + "sections": { + "section-id": { + "type": "old-name" + } + }, + "order": ["section-id"] + }`, + + 'templates/index.json': ` + { + "sections": { + "section-id": { + "type": "old-name" + } + }, + "order": ["section-id"] + }`, + }, + mockRoot, + ); + documentManager = new DocumentManager( + fs, // filesystem + undefined, // optional mode getter + undefined, // optional mode map + async () => 'theme', // getModeForURI + async () => true, // isValidSchema - assume all schemas are valid in tests + ); + handler = new RenameHandler(connection, capabilities, documentManager, findThemeRootURI); + }); + + describe('when the client does not support workspace/applyEdit', () => { + beforeEach(() => { + capabilities.setup({ + workspace: { + applyEdit: false, + }, + }); + }); + + it('does nothing', async () => { + await handler.onDidRenameFiles({ + files: [ + { + oldUri: 'mock-fs:/sections/old-name.liquid', + newUri: 'mock-fs:/sections/new-name.liquid', + }, + ], + }); + expect(connection.spies.sendRequest).not.toHaveBeenCalled(); + }); + }); + + describe('when the client supports workspace/applyEdit', () => { + beforeEach(() => { + capabilities.setup({ + workspace: { + applyEdit: true, + }, + }); + }); + + it('returns a needConfirmation: false workspace edit for renaming a section', async () => { + await handler.onDidRenameFiles({ + files: [ + { + oldUri: 'mock-fs:/sections/old-name.liquid', + newUri: 'mock-fs:/sections/new-name.liquid', + }, + ], + }); + + const replaceWithNewNameTextEditAtAnyLocation = { + annotationId: 'renameSection', + newText: 'new-name', + range: { + start: expect.any(Object), + end: expect.any(Object), + }, + }; + + const templateTextEdits = [ + // Section type is updated in the section sections array + replaceWithNewNameTextEditAtAnyLocation, + ]; + + const sectionGroupTextEdits = [ + // Section type is updated in the section sections array, + replaceWithNewNameTextEditAtAnyLocation, + ]; + + const sectionTagTextEdits = [ + // Over {% section 'old-name' %} + replaceWithNewNameTextEditAtAnyLocation, + ]; + + expect(connection.spies.sendRequest).toHaveBeenCalledWith('workspace/applyEdit', { + label: "Rename section 'old-name' to 'new-name'", + edit: { + changeAnnotations: { + renameSection: { + label: `Rename section 'old-name' to 'new-name'`, + needsConfirmation: false, + }, + }, + documentChanges: expect.arrayContaining([ + { + textDocument: { + uri: 'mock-fs:/layout/theme.liquid', + version: null, + }, + edits: sectionTagTextEdits, + }, + { + textDocument: { + uri: 'mock-fs:/templates/index.json', + version: null, + }, + edits: templateTextEdits, + }, + { + textDocument: { + uri: 'mock-fs:/sections/header.json', + version: null, + }, + edits: sectionGroupTextEdits, + }, + ]), + }, + }); + }); + + it('replaces the correct text in the documents', async () => { + await handler.onDidRenameFiles({ + files: [ + { + oldUri: 'mock-fs:/sections/old-name.liquid', + newUri: 'mock-fs:/sections/new-name.liquid', + }, + ], + }); + + const params: ApplyWorkspaceEditParams = connection.spies.sendRequest.mock.calls[0][1]; + const expectedFs = new MockFileSystem( + { + 'sections/new-name.liquid': ` + {% schema %} + { + "name": "Old name" + } + {% endschema %}`, + + 'layout/theme.liquid': ` + {% section 'new-name' %} + `, + + 'sections/header.json': ` + { + "type": "header", + "name": "Header", + "sections": { + "section-id": { + "type": "new-name" + } + }, + "order": ["section-id"] + }`, + + 'templates/index.json': ` + { + "sections": { + "section-id": { + "type": "new-name" + } + }, + "order": ["section-id"] + }`, + }, + mockRoot, + ); + + assert(params.edit); + assert(params.edit.documentChanges); + + for (const docChange of params.edit.documentChanges) { + assert(TextDocumentEdit.is(docChange)); + const uri = docChange.textDocument.uri; + const edits = docChange.edits; + const initialDoc = await fs.readFile(uri); + const expectedDoc = await expectedFs.readFile(uri); + expect(edits).to.applyEdits(initialDoc, expectedDoc); + } + }); + + it('preserves local section definitions', async () => { + fs = new MockFileSystem( + { + // a "theme" text section exists + 'sections/text.liquid': ` + {% schema %} + { "name": "text section" } + {% endschema %}`, + + // this section uses a local section of type "text" + 'sections/local.liquid': ` + {% schema %} + { + "name": "Section with local sections", + "sections": [ + { + "type": "text", + "name": "Local text section" + } + ], + "presets": [ + { + "name": "Because text is a local section definition, this preset won't get a rename", + "sections": [ + { "type": "text" } + ] + } + ] + } + {% endschema %}`, + + // This section group uses the local section that uses the local section, no rename needed + 'sections/header.json': JSON.stringify({ + type: 'header', + name: 'Header', + sections: { + local_id: { + type: 'local', + sections: { + text_tqQTNE: { + type: 'text', + }, + }, + section_order: ['text_tqQTNE'], + }, + }, + order: ['local_id'], + }), + + // This template uses the section that uses the local section, no rename needed + 'templates/index.json': JSON.stringify({ + sections: { + local: { + type: 'local', + sections: { + text_tqQTNE: { + type: 'text', + }, + }, + section_order: ['text_tqQTNE'], + }, + }, + }), + }, + mockRoot, + ); + documentManager = new DocumentManager( + fs, + undefined, + undefined, + async () => 'theme', + async () => true, + ); + handler = new RenameHandler(connection, capabilities, documentManager, findThemeRootURI); + + await handler.onDidRenameFiles({ + files: [ + { + oldUri: 'mock-fs:/sections/text.liquid', + newUri: 'mock-fs:/sections/poetry.liquid', + }, + ], + }); + + // Check if sendRequest was called at all + expect(connection.spies.sendRequest).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/theme-language-server-common/src/renamed/handlers/SectionRenameHandler.ts b/packages/theme-language-server-common/src/renamed/handlers/SectionRenameHandler.ts new file mode 100644 index 000000000..a5abf0d02 --- /dev/null +++ b/packages/theme-language-server-common/src/renamed/handlers/SectionRenameHandler.ts @@ -0,0 +1,222 @@ +import { LiquidString } from '@shopify/liquid-html-parser'; +import { + isError, + LiteralNode, + nodeAtPath, + parseJSON, + path, + SourceCodeType, + visit, +} from '@shopify/theme-check-common'; +import { Connection } from 'vscode-languageserver'; +import { + AnnotatedTextEdit, + ApplyWorkspaceEditRequest, + Range, + RenameFilesParams, + TextDocumentEdit, + AnnotatedTextEdit as TextEdit, + WorkspaceEdit, +} from 'vscode-languageserver-protocol'; +import { ClientCapabilities } from '../../ClientCapabilities'; +import { + AugmentedJsonSourceCode, + AugmentedLiquidSourceCode, + AugmentedSourceCode, + DocumentManager, + isJsonSourceCode, + isLiquidSourceCode, +} from '../../documents'; +import { isSection, isSectionGroup, isTemplate, sectionName } from '../../utils/uri'; +import { BaseRenameHandler } from '../BaseRenameHandler'; +import { isValidSectionGroup, isValidTemplate } from './utils'; + +type DocumentChange = TextDocumentEdit; + +const annotationId = 'renameSection'; + +/** + * The SectionRenameHandler will handle section renames + * + * Whenever a section gets renamed, a lot of things need to happen: + * 2. References in template files must be changed + * 3. References in section groups must be changed + * 4. References like {% section "oldName" %} must be changed + */ +export class SectionRenameHandler implements BaseRenameHandler { + constructor( + private documentManager: DocumentManager, + private connection: Connection, + private capabilities: ClientCapabilities, + private findThemeRootURI: (uri: string) => Promise, + ) {} + + async onDidRenameFiles(params: RenameFilesParams): Promise { + if (!this.capabilities.hasApplyEditSupport) return; + + const relevantRenames = params.files.filter( + (file) => isSection(file.oldUri) && isSection(file.newUri), + ); + + // Only preload if you have something to do (folder renames not supported yet). + if (relevantRenames.length !== 1) return; + const rename = relevantRenames[0]; + const rootUri = await this.findThemeRootURI(path.dirname(params.files[0].oldUri)); + await this.documentManager.preload(rootUri); + const theme = this.documentManager.theme(rootUri, true); + const liquidFiles = theme.filter(isLiquidSourceCode); + const templates = theme.filter(isJsonSourceCode).filter((file) => isTemplate(file.uri)); + const sectionGroups = theme.filter(isJsonSourceCode).filter((file) => isSectionGroup(file.uri)); + const oldSectionName = sectionName(rename.oldUri); + const newSectionName = sectionName(rename.newUri); + const editLabel = `Rename section '${oldSectionName}' to '${newSectionName}'`; + const workspaceEdit: WorkspaceEdit = { + documentChanges: [], + changeAnnotations: { + [annotationId]: { + label: editLabel, + needsConfirmation: false, + }, + }, + }; + + // All the templates/*.json files need to be updated with the new block name + // when the old block name wasn't a local block. + const [templateChanges, sectionGroupChanges, sectionTagChanges] = await Promise.all([ + Promise.all(templates.map(this.getTemplateChanges(oldSectionName, newSectionName))), + Promise.all(sectionGroups.map(this.getSectionGroupChanges(oldSectionName, newSectionName))), + Promise.all(liquidFiles.map(this.getSectionTagChanges(oldSectionName, newSectionName))), + ]); + + for (const docChange of [...templateChanges, ...sectionGroupChanges]) { + if (docChange !== null) { + workspaceEdit.documentChanges!.push(docChange); + } + } + + // Because section tag changes could make a change to an existing document, + // we need to group the edits together by document. Or else we might have + // index drifting issues. + for (const docChange of sectionTagChanges) { + if (docChange !== null) { + const existingDocChange = (workspaceEdit.documentChanges as DocumentChange[]).find( + (dc) => dc.textDocument.uri === docChange?.textDocument.uri, + ); + if (existingDocChange) { + existingDocChange.edits.push(...docChange.edits); + } else { + workspaceEdit.documentChanges!.push(docChange); + } + } + } + + if (workspaceEdit.documentChanges!.length === 0) { + console.error('Nothing to do!'); + return; + } + + await this.connection.sendRequest(ApplyWorkspaceEditRequest.type, { + label: editLabel, + edit: workspaceEdit, + }); + } + + private getTemplateChanges(oldSectionName: string, newSectionName: string) { + return async (sourceCode: AugmentedJsonSourceCode) => { + const { textDocument, ast, source } = sourceCode; + const parsed = parseJSON(source); + if (!parsed || isError(parsed) || isError(ast)) return null; + const edits: AnnotatedTextEdit[] = !isValidTemplate(parsed) + ? [] + : Object.entries(parsed.sections) + .filter(([_key, section]) => section.type === oldSectionName) + .map(([key]) => { + const node = nodeAtPath(ast, ['sections', key, 'type']) as LiteralNode; + return { + annotationId, + newText: newSectionName, + range: Range.create( + textDocument.positionAt(node.loc.start.offset + 1), + textDocument.positionAt(node.loc.end.offset - 1), + ), + } as AnnotatedTextEdit; + }); + + if (edits.length === 0) return null; + + return documentChanges(sourceCode, edits); + }; + } + + // Awfully similar except for the isValidSectionGroup check and the types of the objects. + // Feels like a coincidence that the types are so similar. I'm not sure this should be DRY'd up. + private getSectionGroupChanges(oldSectionName: string, newSectionName: string) { + return async (sourceCode: AugmentedJsonSourceCode) => { + const { textDocument, ast, source } = sourceCode; + const parsed = parseJSON(source); + if (!parsed || isError(parsed) || isError(ast)) return null; + const edits: TextEdit[] = !isValidSectionGroup(parsed) + ? [] + : Object.entries(parsed.sections) + .filter(([_key, section]) => section.type === oldSectionName) + .map(([key]) => { + const node = nodeAtPath(ast, ['sections', key, 'type']) as LiteralNode; + return { + annotationId, + newText: newSectionName, + range: Range.create( + textDocument.positionAt(node.loc.start.offset + 1), + textDocument.positionAt(node.loc.end.offset - 1), + ), + } as AnnotatedTextEdit; + }); + + if (edits.length === 0) return null; + + return documentChanges(sourceCode, edits); + }; + } + + private getSectionTagChanges(oldSectionName: string, newSectionName: string) { + return async (sourceCode: AugmentedLiquidSourceCode) => { + const { textDocument, ast } = sourceCode; + if (isError(ast)) return null; + + const edits = visit(ast, { + LiquidTag(node) { + if (node.name !== 'section') return; + if (typeof node.markup === 'string') return; + + // Note the type assertion to the LHS of the expression. + // The type assertions above are enough for this to be true. + // But I'm making the explicit annotation here to make it clear. + const typeNode: LiquidString = node.markup; + if (typeNode.value !== oldSectionName) return; + + return { + annotationId, + newText: newSectionName, + range: Range.create( + textDocument.positionAt(typeNode.position.start + 1), + textDocument.positionAt(typeNode.position.end - 1), + ), + }; + }, + }); + + if (edits.length === 0) return null; + + return documentChanges(sourceCode, edits); + }; + } +} + +function documentChanges(sourceCode: AugmentedSourceCode, edits: TextEdit[]): DocumentChange { + return { + textDocument: { + uri: sourceCode.uri, + version: sourceCode.version ?? null /* null means file from disk in this API */, + }, + edits, + }; +} diff --git a/packages/theme-language-server-common/src/renamed/handlers/utils.ts b/packages/theme-language-server-common/src/renamed/handlers/utils.ts new file mode 100644 index 000000000..df1c68087 --- /dev/null +++ b/packages/theme-language-server-common/src/renamed/handlers/utils.ts @@ -0,0 +1,22 @@ +import { Template } from '@shopify/theme-check-common'; + +// this is very very optimistic... +export function isValidTemplate(parsed: unknown): parsed is Template.Template { + return ( + typeof parsed === 'object' && + parsed !== null && + 'sections' in parsed && + 'order' in parsed && + Array.isArray((parsed as Template.Template).order) + ); +} + +export function isValidSectionGroup(parsed: unknown): parsed is Template.SectionGroup { + return ( + typeof parsed === 'object' && + parsed !== null && + 'sections' in parsed && + 'order' in parsed && + Array.isArray((parsed as Template.SectionGroup).order) + ); +}