chore: refactor check-config handler

.changeset/config-problems-output.md

@@ -0,0 +1,10 @@
+---
+'@redocly/cli': patch
+'@redocly/openapi-core': patch
+---
+
+Improved the consistency of how configuration file problems are reported.
+Commands with machine-readable output formats, such as `json`, `junit`, `checkstyle`, and `sarif` also report configuration file problems instead of hiding them.
+A configuration file that fails to load displays a specific error message instead of a generic one.
+
+**Note**: Configuration file problems are printed to `stderr` in the `codeframe` format for every command, regardless of the `--format` option.

docs/@v2/commands/lint.md

@@ -493,6 +493,8 @@ The `lint` command also validates the configuration file.
 You can set severity level by using the `--lint-config` option.
 This option accepts one of the following values: `warn`,`error`, or `off`.
 Default value is `warn`.
+Configuration file problems are printed to `stderr` in the `codeframe` format.
+The `--format` option does not apply to them, so machine-readable output such as `json` stays intact.
 
 ```bash
 redocly lint --lint-config=off

packages/cli/src/__tests__/commands/bundle.test.ts

@@ -10,12 +10,12 @@ import { type MockInstance } from 'vitest';
 import { type Arguments } from 'yargs';
 
 import { type BundleArgv, bundleTelemetry, handleBundle } from '../../commands/bundle.js';
+import { loadAndCheckConfig } from '../../commands/check-config.js';
 import {
   dumpBundle,
   getFallbackApisOrExit,
   getOutputFileName,
   handleError,
-  loadConfigAndHandleErrors,
   saveBundle,
 } from '../../utils/miscellaneous.js';
 import { commandWrapper } from '../../wrapper.js';
@@ -51,7 +51,8 @@ describe('bundle', () => {
     });
 
     vi.mock('../../utils/miscellaneous.js');
-    vi.mocked(loadConfigAndHandleErrors).mockResolvedValue(configFixture);
+    vi.mock('../../commands/check-config.js');
+    vi.mocked(loadAndCheckConfig).mockResolvedValue(configFixture);
     vi.mocked(getFallbackApisOrExit).mockImplementation(
       async (entrypoints) => entrypoints?.map((path: string) => ({ path })) ?? []
     );

packages/cli/src/__tests__/commands/lint.test.ts

@@ -1,6 +1,5 @@
 import {
   lint,
-  lintConfig,
   getTotals,
   formatProblems,
   logger,
@@ -14,14 +13,14 @@ import { performance } from 'perf_hooks';
 import { type MockInstance } from 'vitest';
 import { type Arguments } from 'yargs';
 
-import { handleLint, handleLintConfig, type LintArgv } from '../../commands/lint.js';
+import { loadAndCheckConfig } from '../../commands/check-config.js';
+import { handleLint, type LintArgv } from '../../commands/lint.js';
 import type { VerifyConfigOptions } from '../../types.js';
 import {
   getFallbackApisOrExit,
   getExecutionTime,
   printUnusedWarnings,
   handleError,
-  loadConfigAndHandleErrors,
   checkIfRulesetExist,
 } from '../../utils/miscellaneous.js';
 import { commandWrapper } from '../../wrapper.js';
@@ -67,7 +66,8 @@ describe('handleLint', () => {
     });
 
     vi.mock('../../utils/miscellaneous.js');
-    vi.mocked(loadConfigAndHandleErrors).mockResolvedValue(configFixture);
+    vi.mock('../../commands/check-config.js');
+    vi.mocked(loadAndCheckConfig).mockResolvedValue(configFixture);
     vi.mocked(getFallbackApisOrExit).mockImplementation(
       async (entrypoints) => entrypoints?.map((path: string) => ({ path })) ?? []
     );
@@ -85,9 +85,9 @@ describe('handleLint', () => {
       );
     });
 
-    it('should call loadConfigAndHandleErrors and getFallbackApisOrExit', async () => {
+    it('should call loadAndCheckConfig and getFallbackApisOrExit', async () => {
       await commandWrapper(handleLint)(argvMock);
-      expect(loadConfigAndHandleErrors).toHaveBeenCalledWith(
+      expect(loadAndCheckConfig).toHaveBeenCalledWith(
         {
           apis: ['openapi.yaml'],
           format: 'codeframe',
@@ -105,7 +105,7 @@ describe('handleLint', () => {
         config: 'redocly.yaml',
         extends: ['some/path'],
       });
-      expect(loadConfigAndHandleErrors).toHaveBeenCalledWith(
+      expect(loadAndCheckConfig).toHaveBeenCalledWith(
         {
           apis: ['openapi.yaml'],
           config: 'redocly.yaml',
@@ -267,7 +267,7 @@ describe('handleLint', () => {
     });
 
     it('should call exit with 0 if no errors', async () => {
-      vi.mocked(loadConfigAndHandleErrors).mockImplementation(async () => {
+      vi.mocked(loadAndCheckConfig).mockImplementation(async () => {
         return configFixture;
       });
       await commandWrapper(handleLint)(argvMock);
@@ -283,7 +283,7 @@ describe('handleLint', () => {
     });
 
     it('should suggest recommended fallback if there is no config', async () => {
-      vi.mocked(loadConfigAndHandleErrors).mockImplementation(async () => {
+      vi.mocked(loadAndCheckConfig).mockImplementation(async () => {
         return await loadConfig({});
       });
       await commandWrapper(handleLint)(argvMock);
@@ -295,7 +295,7 @@ describe('handleLint', () => {
     });
 
     it('should not suggest recommended fallback if --extends is provided', async () => {
-      vi.mocked(loadConfigAndHandleErrors).mockImplementation(async () => {
+      vi.mocked(loadAndCheckConfig).mockImplementation(async () => {
         return await loadConfig({});
       });
       await commandWrapper(handleLint)({ ...argvMock, extends: ['some/path'] });
@@ -307,36 +307,3 @@ describe('handleLint', () => {
     });
   });
 });
-
-describe('handleLintConfig', () => {
-  const configWithDocument = { ...configFixture, document: { parsed: {} } } as any;
-
-  it.each(['json', 'junit', 'checkstyle'] as const)(
-    'should not print config lint results for the single-document %s format',
-    async (format) => {
-      vi.mocked(lintConfig).mockResolvedValue(['config-problem'] as any);
-      vi.mocked(formatProblems).mockClear();
-
-      await handleLintConfig(
-        { ...argvMock, 'lint-config': 'warn', format } as any,
-        '2.0.0',
-        configWithDocument
-      );
-
-      expect(formatProblems).not.toHaveBeenCalled();
-    }
-  );
-
-  it('should print config lint results for text formats', async () => {
-    vi.mocked(lintConfig).mockResolvedValue([] as any);
-    vi.mocked(formatProblems).mockClear();
-
-    await handleLintConfig(
-      { ...argvMock, 'lint-config': 'warn', format: 'stylish' } as any,
-      '2.0.0',
-      configWithDocument
-    );
-
-    expect(formatProblems).toHaveBeenCalledWith([], expect.objectContaining({ format: 'stylish' }));
-  });
-});

packages/cli/src/__tests__/utils.test.ts

@@ -1,5 +1,4 @@
 import {
-  type Totals,
   ResolveError,
   YamlParseError,
   HandledError,
@@ -19,7 +18,6 @@ import { fileURLToPath } from 'node:url';
 import {
   getFallbackApisOrExit,
   pathToFilename,
-  printConfigLintTotals,
   langToExt,
   checkIfRulesetExist,
   handleError,
@@ -63,43 +61,6 @@ describe('pathToFilename', () => {
   });
 });
 
-describe('printConfigLintTotals', () => {
-  const totalProblemsMock: Totals = {
-    errors: 1,
-    warnings: 0,
-    ignored: 0,
-  };
-
-  const redColoretteMocks = vi.mocked(red);
-  const yellowColoretteMocks = vi.mocked(yellow);
-
-  beforeEach(() => {
-    yellowColoretteMocks.mockImplementation((text) => text as string);
-    redColoretteMocks.mockImplementation((text) => text as string);
-    vi.spyOn(process.stderr, 'write').mockImplementation(() => true);
-  });
-
-  it('should print errors if such exist', () => {
-    printConfigLintTotals(totalProblemsMock);
-    expect(process.stderr.write).toHaveBeenCalledWith('❌ Your config has 1 error.\n');
-    expect(redColoretteMocks).toHaveBeenCalledWith('❌ Your config has 1 error.\n');
-  });
-
-  it('should print warning if no error', () => {
-    printConfigLintTotals({ ...totalProblemsMock, errors: 0, warnings: 2 });
-    expect(process.stderr.write).toHaveBeenCalledWith('⚠️ Your config has 2 warnings.\n');
-    expect(yellowColoretteMocks).toHaveBeenCalledWith('⚠️ Your config has 2 warnings.\n');
-  });
-
-  it('should print nothing if no error and no warnings', () => {
-    const result = printConfigLintTotals({ ...totalProblemsMock, errors: 0 });
-    expect(result).toBeUndefined();
-    expect(process.stderr.write).toHaveBeenCalledTimes(0);
-    expect(yellowColoretteMocks).toHaveBeenCalledTimes(0);
-    expect(redColoretteMocks).toHaveBeenCalledTimes(0);
-  });
-});
-
 describe('getFallbackApisOrExit', async () => {
   const redColoretteMocks = vi.mocked(red);
   const yellowColoretteMocks = vi.mocked(yellow);

packages/cli/src/__tests__/wrapper.test.ts

@@ -6,8 +6,8 @@ import {
 } from '@redocly/openapi-core';
 import * as process from 'node:process';
 
+import { loadAndCheckConfig } from '../commands/check-config.js';
 import { handleLint } from '../commands/lint.js';
-import { loadConfigAndHandleErrors } from '../utils/miscellaneous.js';
 import { sendTelemetry } from '../utils/telemetry.js';
 import { commandWrapper } from '../wrapper.js';
 
@@ -26,14 +26,15 @@ describe('commandWrapper', () => {
     vi.mock('../utils/miscellaneous.js');
     vi.mock('../utils/telemetry.js');
     vi.mock('../commands/lint.js');
+    vi.mock('../commands/check-config.js');
   });
   afterEach(() => {
     global.fetch = originalFetch;
     process.env.REDOCLY_TELEMETRY = undefined;
   });
 
   it('should send telemetry if there is "telemetry: on" in the config', async () => {
-    vi.mocked(loadConfigAndHandleErrors).mockImplementation(async () => {
+    vi.mocked(loadAndCheckConfig).mockImplementation(async () => {
       return { resolvedConfig: { telemetry: 'on' } } as Config;
     });
     vi.mocked(detectSpec).mockImplementationOnce(() => {
@@ -69,7 +70,7 @@ describe('commandWrapper', () => {
   });
 
   it('should not collect spec version if the file is not parsed to json (except for graphql)', async () => {
-    vi.mocked(loadConfigAndHandleErrors).mockImplementation(async () => {
+    vi.mocked(loadAndCheckConfig).mockImplementation(async () => {
       return { resolvedConfig: { telemetry: 'on' } } as Config;
     });
     vi.mocked(handleLint).mockImplementation(async ({ collectSpecData }) => {
@@ -102,7 +103,7 @@ describe('commandWrapper', () => {
   });
 
   it('should collect the spec version of a GraphQL document', async () => {
-    vi.mocked(loadConfigAndHandleErrors).mockImplementation(async () => {
+    vi.mocked(loadAndCheckConfig).mockImplementation(async () => {
       return { resolvedConfig: { telemetry: 'on' } } as Config;
     });
     vi.mocked(handleLint).mockImplementation(async ({ collectSpecData }) => {
@@ -122,7 +123,7 @@ describe('commandWrapper', () => {
   });
 
   it('should not keep the spec keyword of a previously linted document', async () => {
-    vi.mocked(loadConfigAndHandleErrors).mockImplementation(async () => {
+    vi.mocked(loadAndCheckConfig).mockImplementation(async () => {
       return { resolvedConfig: { telemetry: 'on' } } as Config;
     });
     vi.mocked(detectSpec).mockImplementationOnce(() => {
@@ -146,7 +147,7 @@ describe('commandWrapper', () => {
   });
 
   it('should collect the names of lint rules that reported problems', async () => {
-    vi.mocked(loadConfigAndHandleErrors).mockImplementation(async () => {
+    vi.mocked(loadAndCheckConfig).mockImplementation(async () => {
       return { resolvedConfig: { telemetry: 'on' } } as Config;
     });
     vi.mocked(handleLint).mockImplementation(async ({ collectResults: collectLintResults }) => {
@@ -170,7 +171,7 @@ describe('commandWrapper', () => {
   });
 
   it('should NOT send telemetry if there is "telemetry: off" in the config', async () => {
-    vi.mocked(loadConfigAndHandleErrors).mockImplementation(async () => {
+    vi.mocked(loadAndCheckConfig).mockImplementation(async () => {
       return { resolvedConfig: { telemetry: 'off' } } as Config;
     });
     process.env.REDOCLY_TELEMETRY = 'on';

packages/cli/src/commands/__tests__/check-config.test.ts

@@ -0,0 +1,79 @@
+import {
+  formatProblems,
+  lintConfig,
+  loadConfig,
+  type Config,
+  type Exact,
+  type Totals,
+} from '@redocly/openapi-core';
+import { red, yellow } from 'colorette';
+
+import type { CommandArgv } from '../../types.js';
+import { loadAndCheckConfig, printConfigLintTotals } from '../check-config.js';
+
+vi.mock('colorette');
+vi.mock('@redocly/openapi-core', async () => {
+  const actual = await vi.importActual('@redocly/openapi-core');
+  return { ...actual, loadConfig: vi.fn(), lintConfig: vi.fn(), formatProblems: vi.fn() };
+});
+
+beforeEach(() => {
+  vi.spyOn(process.stderr, 'write').mockImplementation(() => true);
+});
+
+describe('loadAndCheckConfig', () => {
+  it('should print config problems in the check-config format regardless of --format', async () => {
+    const config = { document: { parsed: {} } } as Config;
+    vi.mocked(loadConfig).mockResolvedValue(config);
+    vi.mocked(lintConfig).mockResolvedValue([]);
+
+    const result = await loadAndCheckConfig(
+      { 'lint-config': 'warn', format: 'json' } as Exact<CommandArgv>,
+      '2.0.0'
+    );
+
+    expect(result).toBe(config);
+    expect(formatProblems).toHaveBeenCalledWith([], {
+      maxProblems: undefined,
+      totals: { errors: 0, warnings: 0, ignored: 0 },
+      version: '2.0.0',
+      command: 'check-config',
+    });
+  });
+});
+
+describe('printConfigLintTotals', () => {
+  const totalProblemsMock: Totals = {
+    errors: 1,
+    warnings: 0,
+    ignored: 0,
+  };
+
+  const redColoretteMocks = vi.mocked(red);
+  const yellowColoretteMocks = vi.mocked(yellow);
+
+  beforeEach(() => {
+    yellowColoretteMocks.mockImplementation((text) => text as string);
+    redColoretteMocks.mockImplementation((text) => text as string);
+  });
+
+  it('should print errors if such exist', () => {
+    printConfigLintTotals(totalProblemsMock);
+    expect(process.stderr.write).toHaveBeenCalledWith('❌ Your config has 1 error.\n');
+    expect(redColoretteMocks).toHaveBeenCalledWith('❌ Your config has 1 error.\n');
+  });
+
+  it('should print warning if no error', () => {
+    printConfigLintTotals({ ...totalProblemsMock, errors: 0, warnings: 2 });
+    expect(process.stderr.write).toHaveBeenCalledWith('⚠️ Your config has 2 warnings.\n');
+    expect(yellowColoretteMocks).toHaveBeenCalledWith('⚠️ Your config has 2 warnings.\n');
+  });
+
+  it('should print nothing if no error and no warnings', () => {
+    const result = printConfigLintTotals({ ...totalProblemsMock, errors: 0 });
+    expect(result).toBeUndefined();
+    expect(process.stderr.write).toHaveBeenCalledTimes(0);
+    expect(yellowColoretteMocks).toHaveBeenCalledTimes(0);
+    expect(redColoretteMocks).toHaveBeenCalledTimes(0);
+  });
+});