From 1c372f4b511743be4d1190b23e8509d6d2f22d37 Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Mon, 4 Mar 2024 13:42:06 +0100 Subject: [PATCH 1/6] fix: Improve the Configuration Loader Error Message --- .../configLoader/configLoader.test.ts | 13 +++-- .../Controller/configLoader/configLoader.ts | 51 ++++++++++++++++++- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts index 334621efa48..fc571992254 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts @@ -16,12 +16,17 @@ import { pathRepoTestFixturesURL, } from '../../../../test-util/test.locations.cjs'; import { logError, logWarning } from '../../../util/logger.js'; -import { resolveFileWithURL, toFilePathOrHref, toFileUrl } from '../../../util/url.js'; +import { cwdURL, resolveFileWithURL, toFilePathOrHref, toFileUrl } from '../../../util/url.js'; import { currentSettingsFileVersion, defaultConfigFileModuleRef, ENV_CSPELL_GLOB_ROOT } from '../../constants.js'; import type { ImportFileRefWithError } from '../../CSpellSettingsServer.js'; import { extractDependencies, getSources, mergeSettings } from '../../CSpellSettingsServer.js'; import { _defaultSettings, getDefaultBundledSettingsAsync } from '../../DefaultSettings.js'; -import { __testing__ as __configLoader_testing__, createConfigLoader, loadPnP } from './configLoader.js'; +import { + __testing__ as __configLoader_testing__, + ConfigurationLoaderFailedToResolveError, + createConfigLoader, + loadPnP, +} from './configLoader.js'; import { configToRawSettings } from './configToRawSettings.js'; import { clearCachedSettingsFiles, @@ -337,10 +342,10 @@ describe('Validate search/load config files', () => { clearCachedSettingsFiles(); }); - function resolveError(filename: string): ImportFileRefWithError { + function resolveError(filename: string, relativeTo = cwdURL()): ImportFileRefWithError { return { filename, - error: new Error(`Failed to resolve file: "${filename}"`), + error: new ConfigurationLoaderFailedToResolveError(filename, relativeTo), }; } diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts index 258186efd37..0a56a7f2293 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts @@ -21,6 +21,8 @@ import { cwdURL, resolveFileWithURL, toFilePathOrHref, + toFileUrl, + toURL, windowsDriveLetterToUpper, } from '../../../util/url.js'; import { @@ -550,7 +552,7 @@ export class ConfigLoader implements IConfigLoader { return { filename: r.filename.startsWith('file:/') ? fileURLToPath(r.filename) : r.filename, - error: r.found ? undefined : new Error(`Failed to resolve file: "${filename}"`), + error: r.found ? undefined : new ConfigurationLoaderFailedToResolveError(filename, relativeTo), }; } @@ -677,6 +679,53 @@ async function isDirectory(fs: VFileSystem, path: URL): Promise { } } +export class ConfigurationLoaderError extends Error { + constructor( + message: string, + public readonly configurationFile?: string, + public readonly relativeTo?: string | URL, + cause?: unknown, + ) { + super(message); + this.name = 'Configuration Loader Error'; + if (cause) { + this.cause = cause; + } + } +} + +export class ConfigurationLoaderFailedToResolveError extends ConfigurationLoaderError { + constructor( + public readonly configurationFile: string, + public readonly relativeTo: string | URL, + cause?: unknown, + ) { + const filename = configurationFile.startsWith('file:/') ? fileURLToPath(configurationFile) : configurationFile; + const relSource = relativeToCwd(relativeTo); + + const message = `Failed to resolve configuration file: "${filename}" referenced from "${relSource}"`; + super(message, configurationFile, relativeTo, cause); + // this.name = 'Configuration Loader Error'; + } +} + +function relativeToCwd(file: string | URL): string { + const url = toFileUrl(file); + const cwdPath = cwdURL().pathname.split('/').slice(0, -1); + const urlPath = url.pathname.split('/'); + if (urlPath[0] !== cwdPath[0]) return file.toString(); + let i = 0; + for (; i < cwdPath.length; ++i) { + if (cwdPath[i] !== urlPath[i]) break; + } + const prefix = '.' + .repeat(cwdPath.length - i) + .split('') + .map(() => '..') + .join('/'); + return [prefix || '.', ...urlPath.slice(i)].join('/'); +} + export const __testing__ = { getDefaultConfigLoaderInternal, normalizeCacheSettings, From 65ae4fd605e735796f720234f0ede1a125d1ff3b Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Mon, 4 Mar 2024 13:56:38 +0100 Subject: [PATCH 2/6] add some more tests --- .../configLoader/configLoader.test.ts | 18 +++++++++++++++++- .../Controller/configLoader/configLoader.ts | 7 +++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts index fc571992254..e955ff69cb4 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts @@ -43,7 +43,7 @@ import { extractImportErrors, extractImports } from './extractImportErrors.js'; import { readSettings } from './readSettings.js'; import { readSettingsFiles } from './readSettingsFiles.js'; -const { validateRawConfigVersion, resolveGlobRoot } = __configLoader_testing__; +const { validateRawConfigVersion, resolveGlobRoot, relativeToCwd } = __configLoader_testing__; const rootCspellLib = path.join(pathPackageRoot, '.'); const root = pathRepoRoot; @@ -808,6 +808,22 @@ describe('Validate Dependencies', () => { }); }); +describe.only('relativeToCwd', () => { + test.each` + filename | expected + ${cwdURL()} | ${'./'} + ${'../../cspell.json'} | ${'../../cspell.json'} + ${'../../cspell.json'} | ${'../../cspell.json'} + ${'../../../cspell.json'} | ${'../../../cspell.json'} + ${pathToFileURL('../../../../cspell.json')} | ${path.resolve('../../../../cspell.json')} + ${pathToFileURL('./samples/cspell.json')} | ${'./samples/cspell.json'} + ${pathToFileURL('../samples/cspell.json')} | ${'../samples/cspell.json'} + ${new URL('https://example.com/cspell.json')} | ${'https://example.com/cspell.json'} + `('relativeToCwd', ({ filename, expected }) => { + expect(relativeToCwd(filename)).toEqual(expected); + }); +}); + /** * Resolve relative to src/lib */ diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts index 0a56a7f2293..7a47a8996aa 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts @@ -713,13 +713,15 @@ function relativeToCwd(file: string | URL): string { const url = toFileUrl(file); const cwdPath = cwdURL().pathname.split('/').slice(0, -1); const urlPath = url.pathname.split('/'); - if (urlPath[0] !== cwdPath[0]) return file.toString(); + if (urlPath[0] !== cwdPath[0]) return toFilePathOrHref(file); let i = 0; for (; i < cwdPath.length; ++i) { if (cwdPath[i] !== urlPath[i]) break; } + const segments = cwdPath.length - i; + if (segments > 3) return toFilePathOrHref(file); const prefix = '.' - .repeat(cwdPath.length - i) + .repeat(segments) .split('') .map(() => '..') .join('/'); @@ -731,4 +733,5 @@ export const __testing__ = { normalizeCacheSettings, validateRawConfigVersion, resolveGlobRoot, + relativeToCwd, }; From 7eff2d2339eb9a6c758cd7b5900ca77dfe3c9551 Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Mon, 4 Mar 2024 13:56:51 +0100 Subject: [PATCH 3/6] Update configLoader.test.ts --- .../lib/Settings/Controller/configLoader/configLoader.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts index e955ff69cb4..adee1efe3dc 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts @@ -808,7 +808,7 @@ describe('Validate Dependencies', () => { }); }); -describe.only('relativeToCwd', () => { +describe('relativeToCwd', () => { test.each` filename | expected ${cwdURL()} | ${'./'} From e19a81146cd230655c3379ba4b56697584913499 Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Mon, 4 Mar 2024 13:58:18 +0100 Subject: [PATCH 4/6] Update configLoader.ts --- .../src/lib/Settings/Controller/configLoader/configLoader.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts index 7a47a8996aa..b2759bfef60 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts @@ -22,7 +22,6 @@ import { resolveFileWithURL, toFilePathOrHref, toFileUrl, - toURL, windowsDriveLetterToUpper, } from '../../../util/url.js'; import { From cdd566eb7af4b58d4a3db3b4d88f268ac5aaeda8 Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Mon, 4 Mar 2024 15:02:47 +0100 Subject: [PATCH 5/6] Update spellCheckFile.test.ts --- packages/cspell-lib/src/lib/spellCheckFile.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/cspell-lib/src/lib/spellCheckFile.test.ts b/packages/cspell-lib/src/lib/spellCheckFile.test.ts index d09ca585e57..e67742de169 100644 --- a/packages/cspell-lib/src/lib/spellCheckFile.test.ts +++ b/packages/cspell-lib/src/lib/spellCheckFile.test.ts @@ -276,7 +276,9 @@ function errNoEnt(file: string): Error { function eFailed(file: string): Error { return oc({ - message: expectToEqualCaseInsensitive(`Failed to resolve file: "${rpS(file)}"`), + message: expectToEqualCaseInsensitive( + `Failed to resolve configuration file: "${rpS(file)}" referenced from "./"`, + ), }); } From a12063b6cbe111fb1a0fe55292371f3979b2b36d Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Mon, 4 Mar 2024 15:24:34 +0100 Subject: [PATCH 6/6] Update app.test.ts.snap --- packages/cspell/src/app/__snapshots__/app.test.ts.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cspell/src/app/__snapshots__/app.test.ts.snap b/packages/cspell/src/app/__snapshots__/app.test.ts.snap index f486fe65be3..14f6c8a4be7 100644 --- a/packages/cspell/src/app/__snapshots__/app.test.ts.snap +++ b/packages/cspell/src/app/__snapshots__/app.test.ts.snap @@ -328,7 +328,7 @@ exports[`Validate cli > app 'cspell-bad.json' Expect Error: undefined 3`] = ` exports[`Validate cli > app 'cspell-import-missing.json' Expect Error: [Function CheckFailed] 1`] = `[]`; exports[`Validate cli > app 'cspell-import-missing.json' Expect Error: [Function CheckFailed] 2`] = ` -"error Configuration Error: Failed to resolve file: "../intentionally-missing-file.json" +"error Configuration Configuration Loader Error: Failed to resolve configuration file: "../intentionally-missing-file.json" referenced from "./samples/linked/cspell-import-missing.json" error CSpell: Files checked: 0, Issues found: 0 in 0 files" `;