diff --git a/.changeset/theme-check-config-directory.md b/.changeset/theme-check-config-directory.md new file mode 100644 index 00000000000..39272665c82 --- /dev/null +++ b/.changeset/theme-check-config-directory.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Report a directory passed to `theme check --config` as an expected user error instead of crashing with EISDIR. diff --git a/docs-shopify.dev/generated/generated_docs_data_v2.json b/docs-shopify.dev/generated/generated_docs_data_v2.json index 30839954c0a..b7b5385bdfa 100644 --- a/docs-shopify.dev/generated/generated_docs_data_v2.json +++ b/docs-shopify.dev/generated/generated_docs_data_v2.json @@ -8356,7 +8356,7 @@ "syntaxKind": "PropertySignature", "name": "-C, --config ", "value": "string", - "description": "Use the config provided, overriding .theme-check.yml if present Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported", + "description": "Path to a config file to use, overriding .theme-check.yml if present Use --path to set the theme directory\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported", "isOptional": true, "environmentValue": "SHOPIFY_FLAG_CONFIG" }, @@ -8397,7 +8397,7 @@ "environmentValue": "SHOPIFY_FLAG_VERSION" } ], - "value": "export interface themecheck {\n /**\n * Alias of the Shopify account to use for authentication.\n * @environment SHOPIFY_FLAG_AUTH_ALIAS\n */\n '--auth-alias '?: string\n\n /**\n * Automatically fix offenses\n * @environment SHOPIFY_FLAG_AUTO_CORRECT\n */\n '-a, --auto-correct'?: ''\n\n /**\n * Use the config provided, overriding .theme-check.yml if present\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported \n * @environment SHOPIFY_FLAG_CONFIG\n */\n '-C, --config '?: string\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment '?: string\n\n /**\n * Minimum severity for exit with error code\n * @environment SHOPIFY_FLAG_FAIL_LEVEL\n */\n '--fail-level '?: string\n\n /**\n * Generate a .theme-check.yml file\n * @environment SHOPIFY_FLAG_INIT\n */\n '--init'?: ''\n\n /**\n * Output the result as JSON. Automatically disables color output.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n\n /**\n * List enabled checks\n * @environment SHOPIFY_FLAG_LIST\n */\n '--list'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Disable interactive prompts and browser authentication.\n * @environment SHOPIFY_FLAG_NO_INPUT\n */\n '--no-input'?: ''\n\n /**\n * The output format to use\n * @environment SHOPIFY_FLAG_OUTPUT\n */\n '-o, --output '?: string\n\n /**\n * The path where you want to run the command. Defaults to the current working directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Output active config to STDOUT\n * @environment SHOPIFY_FLAG_PRINT\n */\n '--print'?: ''\n\n /**\n * Increase the verbosity of the output. May include sensitive data.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n\n /**\n * Print Theme Check version\n * @environment SHOPIFY_FLAG_VERSION\n */\n '-v, --version'?: ''\n}" + "value": "export interface themecheck {\n /**\n * Alias of the Shopify account to use for authentication.\n * @environment SHOPIFY_FLAG_AUTH_ALIAS\n */\n '--auth-alias '?: string\n\n /**\n * Automatically fix offenses\n * @environment SHOPIFY_FLAG_AUTO_CORRECT\n */\n '-a, --auto-correct'?: ''\n\n /**\n * Path to a config file to use, overriding .theme-check.yml if present\n Use --path to set the theme directory\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported \n * @environment SHOPIFY_FLAG_CONFIG\n */\n '-C, --config '?: string\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment '?: string\n\n /**\n * Minimum severity for exit with error code\n * @environment SHOPIFY_FLAG_FAIL_LEVEL\n */\n '--fail-level '?: string\n\n /**\n * Generate a .theme-check.yml file\n * @environment SHOPIFY_FLAG_INIT\n */\n '--init'?: ''\n\n /**\n * Output the result as JSON. Automatically disables color output.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n\n /**\n * List enabled checks\n * @environment SHOPIFY_FLAG_LIST\n */\n '--list'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Disable interactive prompts and browser authentication.\n * @environment SHOPIFY_FLAG_NO_INPUT\n */\n '--no-input'?: ''\n\n /**\n * The output format to use\n * @environment SHOPIFY_FLAG_OUTPUT\n */\n '-o, --output '?: string\n\n /**\n * The path where you want to run the command. Defaults to the current working directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Output active config to STDOUT\n * @environment SHOPIFY_FLAG_PRINT\n */\n '--print'?: ''\n\n /**\n * Increase the verbosity of the output. May include sensitive data.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n\n /**\n * Print Theme Check version\n * @environment SHOPIFY_FLAG_VERSION\n */\n '-v, --version'?: ''\n}" } }, "themeconsole": { diff --git a/packages/cli/README.md b/packages/cli/README.md index c59bf75e11d..131c9427024 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -12404,7 +12404,8 @@ USAGE FLAGS -C, --config= - Use the config provided, overriding .theme-check.yml if present + Path to a config file to use, overriding .theme-check.yml if present + Use --path to set the theme directory Supports all theme-check: config values, e.g., theme-check:theme-app-extension, theme-check:recommended, theme-check:all For backwards compatibility, :theme_app_extension is also supported diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index d9833c0a556..ba89278e3b5 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -10512,7 +10512,7 @@ }, "config": { "char": "C", - "description": "Use the config provided, overriding .theme-check.yml if present\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported ", + "description": "Path to a config file to use, overriding .theme-check.yml if present\n Use --path to set the theme directory\n Supports all theme-check: config values, e.g., theme-check:theme-app-extension,\n theme-check:recommended, theme-check:all\n For backwards compatibility, :theme_app_extension is also supported ", "env": "SHOPIFY_FLAG_CONFIG", "hasDynamicHelp": false, "multiple": false, diff --git a/packages/theme/src/cli/commands/theme/check.test.ts b/packages/theme/src/cli/commands/theme/check.test.ts index c86b109ca52..55d84eb5df6 100644 --- a/packages/theme/src/cli/commands/theme/check.test.ts +++ b/packages/theme/src/cli/commands/theme/check.test.ts @@ -3,7 +3,8 @@ import {describe, vi, expect, test, beforeEach} from 'vitest' import {Config} from '@oclif/core' import {themeCheckRun, Theme, Config as ThemeConfig, Offense} from '@shopify/theme-check-node' import {AbortError} from '@shopify/cli-kit/node/error' -import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' +import {inTemporaryDirectory, mkdir, symlink} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' vi.mock('@shopify/theme-check-node') const CommandConfig = new Config({root: __dirname}) @@ -96,6 +97,31 @@ describe('Check', () => { await expect(result).rejects.toThrowError(`Theme Check config file not found: ${config}`) }) + test('reports a directory passed as the config as an expected error', async () => { + await inTemporaryDirectory(async (directory) => { + const result = runThemeCheck(path, 'text', directory) + + await expect(result).rejects.toBeInstanceOf(AbortError) + await expect(result).rejects.toThrowError(`Theme Check config must be a file, but ${directory} is a directory.`) + expect(themeCheckRun).not.toHaveBeenCalled() + }) + }) + + test('reports a symlink to a directory passed as the config as an expected error', async () => { + await inTemporaryDirectory(async (directory) => { + const configDirectory = joinPath(directory, 'config') + const config = joinPath(directory, 'config-link') + await mkdir(configDirectory) + await symlink(configDirectory, config) + + const result = runThemeCheck(path, 'text', config) + + await expect(result).rejects.toBeInstanceOf(AbortError) + await expect(result).rejects.toThrowError(`Theme Check config must be a file, but ${config} is a directory.`) + expect(themeCheckRun).not.toHaveBeenCalled() + }) + }) + test('does not treat a missing extended config as a missing explicitly configured file', async () => { const config = '/my-theme/.theme-check.yml' const missingExtendedConfigError = Object.assign(new Error('ENOENT'), { diff --git a/packages/theme/src/cli/commands/theme/check.ts b/packages/theme/src/cli/commands/theme/check.ts index ccd3606842d..4e1871973fb 100644 --- a/packages/theme/src/cli/commands/theme/check.ts +++ b/packages/theme/src/cli/commands/theme/check.ts @@ -50,7 +50,8 @@ export default class Check extends ThemeCommand { config: Flags.string({ char: 'C', required: false, - description: `Use the config provided, overriding .theme-check.yml if present + description: `Path to a config file to use, overriding .theme-check.yml if present + Use --path to set the theme directory Supports all theme-check: config values, e.g., theme-check:theme-app-extension, theme-check:recommended, theme-check:all For backwards compatibility, :theme_app_extension is also supported `, diff --git a/packages/theme/src/cli/services/check.test.ts b/packages/theme/src/cli/services/check.test.ts index 2654de705a0..87d4f87eb2d 100644 --- a/packages/theme/src/cli/services/check.test.ts +++ b/packages/theme/src/cli/services/check.test.ts @@ -9,7 +9,7 @@ import { renderOffensesText, sortOffenses, } from './check.js' -import {fileExists, writeFile} from '@shopify/cli-kit/node/fs' +import {fileExists, isDirectory, writeFile} from '@shopify/cli-kit/node/fs' import {AbortError} from '@shopify/cli-kit/node/error' import {outputInfo, outputSuccess} from '@shopify/cli-kit/node/output' import {renderInfo} from '@shopify/cli-kit/node/ui' @@ -25,6 +25,8 @@ import {beforeEach, describe, expect, test, vi, type Mock} from 'vitest' vi.mock('@shopify/cli-kit/node/fs', async () => ({ fileExists: vi.fn(), + fileRealPath: vi.fn(async (path: string) => path), + isDirectory: vi.fn(), writeFile: vi.fn(), })) @@ -449,6 +451,28 @@ describe('outputActiveConfig', () => { await expect(result).rejects.toBeInstanceOf(AbortError) await expect(result).rejects.toThrowError(`Theme Check config file not found: ${configPath}`) }) + + test('reports a directory passed as the config as an expected error', async () => { + vi.mocked(fileExists).mockResolvedValue(true) + vi.mocked(isDirectory).mockResolvedValue(true) + + const result = outputActiveConfig('/my-theme', '.') + + await expect(result).rejects.toBeInstanceOf(AbortError) + await expect(result).rejects.toThrowError('Theme Check config must be a file, but . is a directory.') + expect(loadConfig).not.toHaveBeenCalled() + }) + + test('does not check whether modern identifiers are directories', async () => { + vi.mocked(fileExists).mockResolvedValue(true) + vi.mocked(isDirectory).mockResolvedValue(true) + vi.mocked(loadConfig).mockResolvedValue({settings: {}, ignore: []} as any) + + await outputActiveConfig('/my-theme', 'theme-check:recommended') + + expect(isDirectory).not.toHaveBeenCalled() + expect(loadConfig).toHaveBeenCalledWith('theme-check:recommended', '/my-theme') + }) }) describe('outputActiveChecks', () => { @@ -465,4 +489,26 @@ describe('outputActiveChecks', () => { await expect(result).rejects.toBeInstanceOf(AbortError) await expect(result).rejects.toThrowError(`Theme Check config file not found: ${configPath}`) }) + + test('reports a directory passed as the config as an expected error', async () => { + vi.mocked(fileExists).mockResolvedValue(true) + vi.mocked(isDirectory).mockResolvedValue(true) + + const result = outputActiveChecks('/my-theme', '.') + + await expect(result).rejects.toBeInstanceOf(AbortError) + await expect(result).rejects.toThrowError('Theme Check config must be a file, but . is a directory.') + expect(loadConfig).not.toHaveBeenCalled() + }) + + test('does not check whether modern identifiers are directories', async () => { + vi.mocked(fileExists).mockResolvedValue(true) + vi.mocked(isDirectory).mockResolvedValue(true) + vi.mocked(loadConfig).mockResolvedValue({settings: {}, ignore: [], checks: []} as any) + + await outputActiveChecks('/my-theme', 'theme-check:recommended') + + expect(isDirectory).not.toHaveBeenCalled() + expect(loadConfig).toHaveBeenCalledWith('theme-check:recommended', '/my-theme') + }) }) diff --git a/packages/theme/src/cli/services/check.ts b/packages/theme/src/cli/services/check.ts index 29e5edb8ad9..5a4345b1c5e 100644 --- a/packages/theme/src/cli/services/check.ts +++ b/packages/theme/src/cli/services/check.ts @@ -1,6 +1,6 @@ import {ThemeCheckResult} from './check/types.js' import {withThemeCheckDiagnostics} from './check/diagnostics.js' -import {fileExists, writeFile} from '@shopify/cli-kit/node/fs' +import {fileExists, fileRealPath, isDirectory, writeFile} from '@shopify/cli-kit/node/fs' import {outputResult, outputInfo, outputSuccess, outputDebug} from '@shopify/cli-kit/node/output' import {joinPath, resolvePath} from '@shopify/cli-kit/node/path' import {renderInfo} from '@shopify/cli-kit/node/ui' @@ -12,6 +12,7 @@ import { applyFixToString, autofix, loadConfig, + ModernIdentifiers, type FixApplicator, type Offense, type Theme, @@ -43,10 +44,28 @@ function isMissingThemeCheckConfigFile(error: unknown, configPath: string) { ) } +/** + * Theme Check reads the config with `readFile`, which fails with an EISDIR error that often has no `path`, + * so a directory is detected before loading instead of being matched after the failure. + */ +async function assertThemeCheckConfigIsNotDirectory(configPath: string | undefined) { + if (configPath === undefined || ModernIdentifiers.some((identifier) => identifier === configPath)) return + // isDirectory throws on a missing path, so missing files are left to the not-found handling. + if (!(await fileExists(configPath))) return + // isDirectory doesn't follow symlinks, so resolve them first. + if (!(await isDirectory(await fileRealPath(configPath)))) return + + throw new AbortError( + `Theme Check config must be a file, but ${configPath} is a directory.`, + 'Pass the path to a .theme-check.yml file to --config, or omit --config to use the .theme-check.yml in your theme root. To choose the theme directory, use --path.', + ) +} + async function withThemeCheckConfigErrorHandling( configPath: string | undefined, operation: () => Promise, ): Promise { + await assertThemeCheckConfigIsNotDirectory(configPath) try { return await operation() } catch (error) {