diff --git a/.changeset/orphaned-files-startup-prompt.md b/.changeset/orphaned-files-startup-prompt.md new file mode 100644 index 000000000..40483bad2 --- /dev/null +++ b/.changeset/orphaned-files-startup-prompt.md @@ -0,0 +1,7 @@ +--- +'theme-check-vscode': minor +--- + +Surface orphaned (dead) files on startup with a notification + +When a theme has orphaned files, the VS Code extension now shows a single dismissable notification on startup with **Review** (opens the existing dead-code picker) and **Don't show again** actions, instead of requiring the user to run the dead-code command manually. Gated by the new `themeCheck.checkOrphanedFilesOnBoot` setting (default `true`). diff --git a/packages/vscode-extension/package.json b/packages/vscode-extension/package.json index 6a230bce9..adffc2dd7 100644 --- a/packages/vscode-extension/package.json +++ b/packages/vscode-extension/package.json @@ -165,6 +165,13 @@ ], "description": "When true, theme check preloads all the files from your theme for fast rename handling and theme graph generation.", "default": true + }, + "themeCheck.checkOrphanedFilesOnBoot": { + "type": [ + "boolean" + ], + "description": "When true, show a notification on startup when your theme has orphaned (dead) files.", + "default": true } } }, diff --git a/packages/vscode-extension/src/browser/extension.ts b/packages/vscode-extension/src/browser/extension.ts index 074c1447b..31a29214c 100644 --- a/packages/vscode-extension/src/browser/extension.ts +++ b/packages/vscode-extension/src/browser/extension.ts @@ -10,6 +10,7 @@ import LiquidFormatter from '../common/formatter'; import { vscodePrettierFormat } from './formatter'; import { documentSelectors } from '../common/constants'; import { makeDeadCode, openLocation } from '../common/commands'; +import { checkOrphanedFilesOnBoot } from '../common/orphanedFilesOnBoot'; import { createReferencesTreeView, setupContext, @@ -45,6 +46,9 @@ export async function activate(context: ExtensionContext) { createReferencesTreeView('shopify.themeGraph.dependencies', context, client, 'dependencies'), watchReferencesTreeViewConfig(), ); + + // Fire-and-forget: surfacing orphaned files must never block or fail activation. + checkOrphanedFilesOnBoot(client).catch((error) => console.error(error)); } } diff --git a/packages/vscode-extension/src/common/commands.spec.ts b/packages/vscode-extension/src/common/commands.spec.ts new file mode 100644 index 000000000..596ef8e83 --- /dev/null +++ b/packages/vscode-extension/src/common/commands.spec.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { BaseLanguageClient } from 'vscode-languageclient'; + +const mocks = vi.hoisted(() => ({ + showInformationMessage: vi.fn(), + showQuickPick: vi.fn(), + showTextDocument: vi.fn(), + openTextDocument: vi.fn(() => Promise.resolve({})), + executeCommand: vi.fn(), + activeTextEditor: { + document: { uri: { toString: () => 'file:///theme/layout/theme.liquid' } }, + } as any, +})); + +vi.mock('vscode', () => ({ + window: { + get activeTextEditor() { + return mocks.activeTextEditor; + }, + showInformationMessage: mocks.showInformationMessage, + showQuickPick: mocks.showQuickPick, + showTextDocument: mocks.showTextDocument, + }, + workspace: { openTextDocument: mocks.openTextDocument }, + commands: { executeCommand: mocks.executeCommand }, + Uri: { parse: (s: string) => ({ toString: () => s }) }, + Position: class { + constructor( + public line: number, + public character: number, + ) {} + }, + Range: class { + constructor( + public start: any, + public end: any, + ) {} + }, +})); + +import { makeDeadCode } from './commands'; + +function makeClient(rootUri: string, deadCode: string[]): BaseLanguageClient { + return { + sendRequest: vi + .fn() + .mockResolvedValueOnce(rootUri) // ThemeGraphRootRequest + .mockResolvedValueOnce(deadCode), // ThemeGraphDeadCodeRequest + } as unknown as BaseLanguageClient; +} + +describe('makeDeadCode (characterization — behavior must survive refactor)', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.activeTextEditor = { + document: { uri: { toString: () => 'file:///theme/layout/theme.liquid' } }, + }; + }); + + it('tells the user when there is no dead code', async () => { + const client = makeClient('file:///theme', []); + + await makeDeadCode(client)(); + + expect(mocks.showInformationMessage).toHaveBeenCalledWith('No dead code found.'); + expect(mocks.showQuickPick).not.toHaveBeenCalled(); + }); + + it('offers a quick pick of the orphaned files when dead code is found', async () => { + const client = makeClient('file:///theme', [ + 'file:///theme/snippets/unused-a.liquid', + 'file:///theme/snippets/unused-b.liquid', + ]); + + await makeDeadCode(client)(); + + expect(mocks.showInformationMessage).not.toHaveBeenCalled(); + expect(mocks.showQuickPick).toHaveBeenCalledTimes(1); + const [items, options] = mocks.showQuickPick.mock.calls[0]; + expect(items).toHaveLength(2); + expect(options).toMatchObject({ canPickMany: true }); + }); + + it('does nothing when there is no active editor', async () => { + mocks.activeTextEditor = undefined; + const client = makeClient('file:///theme', ['file:///theme/snippets/unused.liquid']); + + await makeDeadCode(client)(); + + expect(client.sendRequest).not.toHaveBeenCalled(); + expect(mocks.showInformationMessage).not.toHaveBeenCalled(); + expect(mocks.showQuickPick).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/vscode-extension/src/common/commands.ts b/packages/vscode-extension/src/common/commands.ts index 12c5f59e7..5318bd761 100644 --- a/packages/vscode-extension/src/common/commands.ts +++ b/packages/vscode-extension/src/common/commands.ts @@ -25,31 +25,53 @@ export function openLocation(ref: AugmentedLocation) { }); } +/** + * Fetches the theme root and the list of dead (orphaned) files for a given uri. + * The uri only needs to belong to the theme — the server resolves the root and + * computes dead code across the whole theme graph. + */ +export async function fetchDeadCode( + client: BaseLanguageClient, + uri: string, +): Promise<{ rootUri: string; deadCode: string[] }> { + const [rootUri, deadCode] = await Promise.all([ + client.sendRequest(ThemeGraphRootRequest.type, { uri }), + client.sendRequest(ThemeGraphDeadCodeRequest.type, { uri }), + ]); + return { rootUri, deadCode }; +} + +/** + * Presents the dead files as a multi-select quick pick and opens whatever the + * user selects. Does not depend on the active editor, so it can be driven from + * a startup check as well as the command. + */ +export async function showDeadCodePicker(rootUri: string, deadCode: string[]): Promise { + const relativePaths = deadCode.map((file) => path.relative(file, rootUri)); + const selectedFiles = await window.showQuickPick(relativePaths, { + canPickMany: true, + placeHolder: 'Select files to open', + }); + if (selectedFiles) { + selectedFiles.forEach((file) => { + const uri = path.join(rootUri, file); + workspace.openTextDocument(Uri.parse(uri)).then((doc) => { + window.showTextDocument(doc, { preview: false, preserveFocus: true, viewColumn: 2 }); + }); + }); + } +} + export function makeDeadCode(client: BaseLanguageClient) { return async function deadCode() { const uri = window.activeTextEditor?.document.uri.toString(); if (!uri) return; - const [rootUri, deadCode] = await Promise.all([ - client.sendRequest(ThemeGraphRootRequest.type, { uri }), - client.sendRequest(ThemeGraphDeadCodeRequest.type, { uri }), - ]); + const { rootUri, deadCode } = await fetchDeadCode(client, uri); if (deadCode.length === 0) { window.showInformationMessage('No dead code found.'); } else { - const relativePaths = deadCode.map((file) => path.relative(file, rootUri)); - const selectedFiles = await window.showQuickPick(relativePaths, { - canPickMany: true, - placeHolder: 'Select files to open', - }); - if (selectedFiles) { - selectedFiles.forEach((file) => { - const uri = path.join(rootUri, file); - workspace.openTextDocument(Uri.parse(uri)).then((doc) => { - window.showTextDocument(doc, { preview: false, preserveFocus: true, viewColumn: 2 }); - }); - }); - } + await showDeadCodePicker(rootUri, deadCode); } }; } diff --git a/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts b/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts new file mode 100644 index 000000000..8ea334b47 --- /dev/null +++ b/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts @@ -0,0 +1,235 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { BaseLanguageClient } from 'vscode-languageclient'; + +const mocks = vi.hoisted(() => ({ + showInformationMessage: vi.fn(), + showQuickPick: vi.fn(), + showTextDocument: vi.fn(), + openTextDocument: vi.fn(() => Promise.resolve({})), + findFiles: vi.fn(), + getConfig: vi.fn(), + updateConfig: vi.fn(), +})); + +vi.mock('vscode', () => ({ + window: { + showInformationMessage: mocks.showInformationMessage, + showQuickPick: mocks.showQuickPick, + showTextDocument: mocks.showTextDocument, + }, + workspace: { + getConfiguration: () => ({ get: mocks.getConfig, update: mocks.updateConfig }), + findFiles: mocks.findFiles, + openTextDocument: mocks.openTextDocument, + }, + commands: { executeCommand: vi.fn() }, + ConfigurationTarget: { Global: 1 }, + Uri: { parse: (s: string) => ({ toString: () => s }) }, + Position: class { + constructor( + public line: number, + public character: number, + ) {} + }, + Range: class { + constructor( + public start: any, + public end: any, + ) {} + }, +})); + +import { checkOrphanedFilesOnBoot } from './orphanedFilesOnBoot'; + +function uriLike(s: string) { + return { toString: () => s } as any; +} + +/** + * Builds a client whose ThemeGraphRootRequest/ThemeGraphDeadCodeRequest pair + * resolves per anchor uri. `byUri` maps an anchor uri to the root + dead code the + * server would return for it. Matches fetchDeadCode's Promise.all (root, deadCode). + */ +function clientWith( + byUri: Record, +): BaseLanguageClient { + return { + sendRequest: vi.fn((type: any, params: { uri: string }) => { + const hit = byUri[params.uri]; + // Disambiguate the two requests by the request type's `method` string. + const method: string = type?.method ?? ''; + if (method.toLowerCase().includes('deadcode')) { + return Promise.resolve(hit?.deadCode ?? []); + } + return Promise.resolve(hit?.rootUri ?? ''); + }), + } as unknown as BaseLanguageClient; +} + +describe('checkOrphanedFilesOnBoot', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.getConfig.mockReturnValue(true); + }); + + it('globs for every theme-root signal, not just .theme-check.yml', async () => { + mocks.findFiles.mockResolvedValue([]); + const client = clientWith({}); + + await checkOrphanedFilesOnBoot(client); + + const globPattern = mocks.findFiles.mock.calls[0][0] as string; + expect(globPattern).toContain('.theme-check.yml'); + expect(globPattern).toContain('shopify.extension.toml'); + expect(globPattern).toContain('snippets'); + }); + + it('notifies the user when the theme has orphaned files', async () => { + mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + }); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showInformationMessage).toHaveBeenCalledTimes(1); + expect(mocks.showInformationMessage.mock.calls[0][0]).toContain('1 orphaned file'); + }); + + it('does not notify when there are no orphaned files', async () => { + mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { rootUri: 'file:///theme', deadCode: [] }, + }); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showInformationMessage).not.toHaveBeenCalled(); + }); + + it('does nothing when the setting is disabled', async () => { + mocks.getConfig.mockReturnValue(false); + const client = clientWith({}); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.findFiles).not.toHaveBeenCalled(); + expect(client.sendRequest).not.toHaveBeenCalled(); + expect(mocks.showInformationMessage).not.toHaveBeenCalled(); + }); + + it('disables the setting when the user picks "Don\'t show again"', async () => { + mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); + mocks.showInformationMessage.mockResolvedValue("Don't show again"); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + }); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.updateConfig).toHaveBeenCalledWith( + 'themeCheck.checkOrphanedFilesOnBoot', + false, + 1, // ConfigurationTarget.Global + ); + }); + + it('opens the dead-code picker when the user picks "Review"', async () => { + mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); + mocks.showInformationMessage.mockResolvedValue('Review'); + mocks.showQuickPick.mockResolvedValue(undefined); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + }); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showQuickPick).toHaveBeenCalledTimes(1); + expect(mocks.updateConfig).not.toHaveBeenCalled(); + }); + + it('deduplicates anchors that resolve to the same root', async () => { + // A theme with both a .theme-check.yml and a snippets dir yields two anchors + // that the server resolves to the same root — it should only prompt once. + mocks.findFiles.mockResolvedValue([ + uriLike('file:///theme/.theme-check.yml'), + uriLike('file:///theme/snippets/a.liquid'), + ]); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + 'file:///theme/snippets/a.liquid': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + }); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showInformationMessage).toHaveBeenCalledTimes(1); + }); + + it('prompts per root in a multi-theme workspace, with each count matching its own picker', async () => { + // Two separate themes, each with its own orphaned files. The aggregated-count + // bug would say "Found 3 orphaned files" once and only open theme-a's subset. + mocks.findFiles.mockResolvedValue([ + uriLike('file:///theme-a/.theme-check.yml'), + uriLike('file:///theme-b/shopify.extension.toml'), + ]); + const client = clientWith({ + 'file:///theme-a/.theme-check.yml': { + rootUri: 'file:///theme-a', + deadCode: ['file:///theme-a/snippets/a1.liquid'], + }, + 'file:///theme-b/shopify.extension.toml': { + rootUri: 'file:///theme-b', + deadCode: ['file:///theme-b/snippets/b1.liquid', 'file:///theme-b/snippets/b2.liquid'], + }, + }); + // Pick "Review" on every notification. + mocks.showInformationMessage.mockResolvedValue('Review'); + mocks.showQuickPick.mockResolvedValue(undefined); + + await checkOrphanedFilesOnBoot(client); + + // One notification per theme with orphans. + expect(mocks.showInformationMessage).toHaveBeenCalledTimes(2); + const messages = mocks.showInformationMessage.mock.calls.map((c) => c[0]); + expect(messages.some((m: string) => m.includes('1 orphaned file'))).toBe(true); + expect(messages.some((m: string) => m.includes('2 orphaned files'))).toBe(true); + + // Review opened a picker for each theme — the picker for theme-b must show + // theme-b's two files, never theme-a's subset. + expect(mocks.showQuickPick).toHaveBeenCalledTimes(2); + }); + + it('only prompts for roots that actually have orphaned files', async () => { + mocks.findFiles.mockResolvedValue([ + uriLike('file:///theme-a/.theme-check.yml'), + uriLike('file:///theme-b/.theme-check.yml'), + ]); + const client = clientWith({ + 'file:///theme-a/.theme-check.yml': { + rootUri: 'file:///theme-a', + deadCode: ['file:///theme-a/snippets/a1.liquid'], + }, + 'file:///theme-b/.theme-check.yml': { rootUri: 'file:///theme-b', deadCode: [] }, + }); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showInformationMessage).toHaveBeenCalledTimes(1); + expect(mocks.showInformationMessage.mock.calls[0][0]).toContain('1 orphaned file'); + }); +}); diff --git a/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts b/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts new file mode 100644 index 000000000..def50db44 --- /dev/null +++ b/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts @@ -0,0 +1,71 @@ +import { window, workspace, ConfigurationTarget } from 'vscode'; +import { BaseLanguageClient } from 'vscode-languageclient'; +import { fetchDeadCode, showDeadCodePicker } from './commands'; + +export const CHECK_ORPHANED_ON_BOOT_SETTING = 'themeCheck.checkOrphanedFilesOnBoot'; + +const REVIEW = 'Review'; +const DONT_SHOW_AGAIN = "Don't show again"; + +/** + * Glob for any file that signals a theme root, mirroring `findRoot`'s notion of a + * root: a `.theme-check.yml`, a `shopify.extension.toml` (theme app extensions), + * or a `snippets` directory (configless themes / zipped themes / TAEs). Each match + * is just an anchor inside a theme — the server resolves it to the actual root. + */ +const ROOT_ANCHOR_GLOB = '**/{.theme-check.yml,shopify.extension.toml,snippets/*}'; + +/** + * On extension startup, surface orphaned (dead) files with a dismissable + * notification instead of requiring the user to run the dead-code command. Gated + * by the `themeCheck.checkOrphanedFilesOnBoot` setting. + * + * A workspace can contain more than one theme, so this prompts once per theme root + * that has orphaned files — each notification's count matches the files its own + * "Review" picker opens (no cross-theme aggregation). + */ +export async function checkOrphanedFilesOnBoot(client: BaseLanguageClient): Promise { + const enabled = workspace.getConfiguration().get(CHECK_ORPHANED_ON_BOOT_SETTING, true); + if (!enabled) return; + + const anchors = await workspace.findFiles(ROOT_ANCHOR_GLOB, '**/node_modules/**'); + + // Resolve each anchor to its theme root + dead code, then dedupe by root so a + // theme matched by several anchors is only prompted once. + const byRoot = new Map(); + for (const anchor of anchors) { + const { rootUri, deadCode } = await fetchDeadCode(client, anchor.toString()); + if (rootUri && deadCode.length > 0 && !byRoot.has(rootUri)) { + byRoot.set(rootUri, deadCode); + } + } + + for (const [rootUri, deadCode] of byRoot) { + const shouldStop = await promptForRoot(rootUri, deadCode); + if (shouldStop) return; + } +} + +/** + * Shows the notification for a single theme root. Returns `true` if the user opted + * out ("Don't show again"), so the caller can stop prompting the remaining roots. + */ +async function promptForRoot(rootUri: string, deadCode: string[]): Promise { + const count = deadCode.length; + const choice = await window.showInformationMessage( + `Found ${count} orphaned file${count === 1 ? '' : 's'} in your theme.`, + REVIEW, + DONT_SHOW_AGAIN, + ); + + if (choice === REVIEW) { + await showDeadCodePicker(rootUri, deadCode); + } else if (choice === DONT_SHOW_AGAIN) { + await workspace + .getConfiguration() + .update(CHECK_ORPHANED_ON_BOOT_SETTING, false, ConfigurationTarget.Global); + return true; + } + + return false; +} diff --git a/packages/vscode-extension/src/node/extension.ts b/packages/vscode-extension/src/node/extension.ts index 0bf9b99fc..f44908b06 100644 --- a/packages/vscode-extension/src/node/extension.ts +++ b/packages/vscode-extension/src/node/extension.ts @@ -17,6 +17,7 @@ import { watchReferencesTreeViewConfig, } from '../common/ReferencesProvider'; import { makeDeadCode, openLocation } from '../common/commands'; +import { checkOrphanedFilesOnBoot } from '../common/orphanedFilesOnBoot'; const sleep = (ms: number) => new Promise((res) => setTimeout(res, ms)); @@ -47,6 +48,9 @@ export async function activate(context: ExtensionContext) { createReferencesTreeView('shopify.themeGraph.dependencies', context, client, 'dependencies'), watchReferencesTreeViewConfig(), ); + + // Fire-and-forget: surfacing orphaned files must never block or fail activation. + checkOrphanedFilesOnBoot(client).catch((error) => console.error(error)); } }