From f52da8106d7f53857dd108c73caf1a374f7eced2 Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Wed, 6 Dec 2023 11:18:46 +0100 Subject: [PATCH] Improve resolve reporting --- packages/cspell-lib/api/api.d.ts | 7 + .../Controller/configLoader/configLoader.ts | 4 + .../cspell-lib/src/lib/util/resolveFile.ts | 48 ++++++- pnpm-lock.yaml | 6 +- test-fixtures/issues/issue-5034/.cspell.json | 124 +----------------- .../frontend/cfg/cspell.config.yaml | 4 - .../issues/issue-5034/frontend/package.json | 2 +- .../issues/issue-5034/frontend/src/README.md | 9 ++ .../frontend/src/cspell.config.yaml | 5 + test-fixtures/issues/issue-5034/package.json | 6 +- 10 files changed, 81 insertions(+), 134 deletions(-) delete mode 100644 test-fixtures/issues/issue-5034/frontend/cfg/cspell.config.yaml create mode 100644 test-fixtures/issues/issue-5034/frontend/src/README.md create mode 100644 test-fixtures/issues/issue-5034/frontend/src/cspell.config.yaml diff --git a/packages/cspell-lib/api/api.d.ts b/packages/cspell-lib/api/api.d.ts index fbb61652d5b..20dd102ea1e 100644 --- a/packages/cspell-lib/api/api.d.ts +++ b/packages/cspell-lib/api/api.d.ts @@ -955,9 +955,16 @@ declare function setLogger(logger: Logger): Logger; declare function getLogger(): Logger; interface ResolveFileResult { + /** + * Absolute path or URL to the file. + */ filename: string; relativeTo: string | undefined; found: boolean; + /** + * A warning message if the file was found, but there was a problem. + */ + warning?: string; } /** * Resolve filename to absolute paths. 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 8a5edcb2323..f8df896e0d1 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts @@ -496,6 +496,10 @@ function resolveFilename(filename: string | URL, relativeTo: string | URL): Impo if (filename instanceof URL) return { filename: toFilePathOrHref(filename) }; const r = resolveFile(filename, relativeTo); + if (r.warning) { + logWarning(r.warning); + } + return { filename: r.filename.startsWith('file:/') ? fileURLToPath(r.filename) : r.filename, error: r.found ? undefined : new Error(`Failed to resolve file: "${filename}"`), diff --git a/packages/cspell-lib/src/lib/util/resolveFile.ts b/packages/cspell-lib/src/lib/util/resolveFile.ts index b173419867f..5b824a9e852 100644 --- a/packages/cspell-lib/src/lib/util/resolveFile.ts +++ b/packages/cspell-lib/src/lib/util/resolveFile.ts @@ -20,9 +20,16 @@ import { } from './url.js'; export interface ResolveFileResult { + /** + * Absolute path or URL to the file. + */ filename: string; relativeTo: string | undefined; found: boolean; + /** + * A warning message if the file was found, but there was a problem. + */ + warning?: string; } const testNodeModules = /^node_modules\//; @@ -45,6 +52,7 @@ export function resolveFile(filename: string, relativeTo: string | URL): Resolve { filename, fn: tryResolveFrom }, { filename: filename.replace(testNodeModules, ''), fn: tryResolveFrom }, { filename, fn: tryResolveGlobal }, + { filename, fn: tryLegacyResolve }, ]; for (const step of steps) { @@ -119,7 +127,7 @@ function tryNodeResolveDefaultPaths(filename: string): ResolveFileResult | undef function tryNodeRequireResolve(filenameOrURL: string, relativeTo: string | URL): ResolveFileResult | undefined { const filename = fileURLOrPathToPath(filenameOrURL); - const relativeToPath = fileURLOrPathToPath(relativeTo); + const relativeToPath = pathFromRelativeTo(relativeTo); const home = os.homedir(); function calcPaths(p: string) { const paths = [p]; @@ -157,9 +165,13 @@ function tryResolveGlobal(filename: string): ResolveFileResult | undefined { } function tryResolveExists(filename: string | URL, relativeTo: string | URL): ResolveFileResult | undefined { - if (filename instanceof URL || isURLLike(filename) || isURLLike(relativeTo)) return undefined; + if (filename instanceof URL || isURLLike(filename) || (isURLLike(relativeTo) && !isFileURL(relativeTo))) { + return undefined; + } + + relativeTo = pathFromRelativeTo(relativeTo); - const toTry = [{ filename }, { filename: path.resolve(relativeTo.toString(), filename), relativeTo }]; + const toTry = [{ filename }, { filename: path.resolve(relativeTo, filename), relativeTo }]; for (const { filename, relativeTo } of toTry) { const found = path.isAbsolute(filename) && fs.existsSync(filename); if (found) return { filename, relativeTo: relativeTo?.toString(), found }; @@ -175,13 +187,37 @@ function tryResolveExists(filename: string | URL, relativeTo: string | URL): Res function tryResolveFrom(filename: string, relativeTo: string | URL): ResolveFileResult | undefined { if (relativeTo instanceof URL) return undefined; try { - return { filename: resolveFrom(relativeTo, filename), relativeTo, found: true }; + return { filename: resolveFrom(pathFromRelativeTo(relativeTo), filename), relativeTo, found: true }; } catch (error) { // Failed to resolve a relative module request return undefined; } } +const regExpStartsWidthNodeModules = /^node_modules[/\\]/; + +function tryLegacyResolve(filename: string | URL, relativeTo: string | URL): ResolveFileResult | undefined { + if (filename instanceof URL || isURLLike(filename) || (isURLLike(relativeTo) && !isFileURL(relativeTo))) { + return undefined; + } + + const relativeToPath = isURLLike(relativeTo) ? fileURLToPath(new URL('./', relativeTo)) : relativeTo.toString(); + + const match = filename.match(regExpStartsWidthNodeModules); + + if (match) { + const found = tryImportResolve(filename.replace(regExpStartsWidthNodeModules, ''), relativeToPath); + if (found?.found) { + found.warning = `Import of '${filename}' should not start with '${match[0]}' in '${toFilePathOrHref( + relativeTo, + )}'`; + return found; + } + } + + return undefined; +} + function isRelative(filename: string | URL): boolean { if (filename instanceof URL) return false; if (filename.startsWith('./')) return true; @@ -197,6 +233,10 @@ function joinWith(filename: string, relativeTo: string | URL): string { : path.resolve(relativeTo, filename); } +function pathFromRelativeTo(relativeTo: string | URL): string { + return relativeTo instanceof URL || isURLLike(relativeTo) ? fileURLToPath(new URL('./', relativeTo)) : relativeTo; +} + export const __testing__ = { isRelative, isFileURL, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 250d3bd7447..44840d9151f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -800,7 +800,11 @@ importers: specifier: ^0.19.8 version: 0.19.8 - test-fixtures/issues/issue-5034: {} + test-fixtures/issues/issue-5034: + devDependencies: + cspell: + specifier: workspace:* + version: link:../../../packages/cspell test-fixtures/issues/issue-5034/frontend: devDependencies: diff --git a/test-fixtures/issues/issue-5034/.cspell.json b/test-fixtures/issues/issue-5034/.cspell.json index 1911ab32a16..8aca0011d73 100644 --- a/test-fixtures/issues/issue-5034/.cspell.json +++ b/test-fixtures/issues/issue-5034/.cspell.json @@ -1,126 +1,6 @@ { - "version": "0.2", - "ignorePaths": [ - ".neoconf.json", - ".vscode/settings.json", - ".vim/coc-settings.json", - "frontend/node_modules/**", - "frontend/coverage/**", - "frontend/build/**", - "frontend/db/dist/**", - "frontend/package.json", - "frontend/public/locales/ar/**", - "frontend/public/locales/zgh/**", - "frontend/public/supportedBrowsers.js", - "docker-compose*.yml", - "cloudformation.yml", - "vendor", - "web/src/composer.json", - "web/src/vendor", - "web/src/shared/scripts", - "web/src/css", - "web/src/locales/ar/translations.yml", - "node/src/locales/ar.json", - "web/src/locales/zgh/translations.yml", - "web/src/api/Validation/lang/ar/validation.php", - ".env", - "renovate.json", - "web/conf/**/*", - "*Dockerfile" - ], - "allowCompoundWords": true, - "ignoreWords": [ - "culturehq", - "yiisoft", - "lintstagedrc", - "rehype", - "noto", - "graphiql", - "webp" - ], - "words": [ - "AMZN", - "capce", - "cedata", - "crunz", - "cronitor", - "destructured", - "droppable", - "entityid", - "esetnik", - "esnext", - "falsey", - "Gravatar", - "hacky", - "Handtevy", - "hocs", - "iframe", - "imgix", - "immer", - "junit", - "jwplayer", - "JWPLAYER", - "Lngs", - "luxon", - "Maknz", - "Mebibytes", - "msodbcsql", - "newid", - "noopener", - "NREMT", - "nuka", - "oems", - "onstatechange", - "onupdatefound", - "phpcs", - "phpcbf", - "phuocng", - "recertification", - "reduxjs", - "resizer", - "signup", - "testid", - "tamazight", - "tifinagh", - "tsql", - "Tuupola", - "unmount", - "unregister", - "UPDLOCK", - "wdyr", - "xlarge" - ], + "language": "en,fr", "import": [ - "./frontend/node_modules/@cspell/dict-fr-fr/cspell-ext.json" - ], - "overrides": [ - { - "filename": "**/*.md", - "language": "en,fr" - }, - { - "filename": "**/locales/fr/**/{*.json,*.yml}", - "language": "en,fr" - }, - { - "filename": "**/__test__/**", - "language": "en,fr" - }, - { - "filename": "**/{*.test.ts,*.test.tsx}", - "language": "en,fr" - }, - { - "filename": "**/Validation/lang/fr/validation.php", - "language": "en,fr" - }, - { - "filename": "node/src/locales/fr.json", - "language": "en,fr" - }, - { - "filename": "frontend/src/themes/*.json", - "languageId": "css" - } + "frontend/node_modules/@cspell/dict-fr-fr/cspell-ext.json" ] } diff --git a/test-fixtures/issues/issue-5034/frontend/cfg/cspell.config.yaml b/test-fixtures/issues/issue-5034/frontend/cfg/cspell.config.yaml deleted file mode 100644 index 39cfcdf5e98..00000000000 --- a/test-fixtures/issues/issue-5034/frontend/cfg/cspell.config.yaml +++ /dev/null @@ -1,4 +0,0 @@ -name: Nested Config -version: "0.2" -words: - - cspell diff --git a/test-fixtures/issues/issue-5034/frontend/package.json b/test-fixtures/issues/issue-5034/frontend/package.json index 8bdcce873dc..2c447e69ddc 100644 --- a/test-fixtures/issues/issue-5034/frontend/package.json +++ b/test-fixtures/issues/issue-5034/frontend/package.json @@ -4,7 +4,7 @@ "private": true, "dependencies": {}, "scripts": { - "spellcheck": "cspell -c \"../.cspell.json\" -r \"..\" --no-progress --dot --gitignore \"**/*.{js,jsx,ts,tsx,json,yml,md}\"", + "spellcheck": "cspell -c \"../.cspell.json\" -r \"..\" --dot --gitignore \"**/*.{js,jsx,ts,tsx,json,yml,md}\"", "test": "pnpm run spellcheck" }, "devDependencies": { diff --git a/test-fixtures/issues/issue-5034/frontend/src/README.md b/test-fixtures/issues/issue-5034/frontend/src/README.md new file mode 100644 index 00000000000..9c137ecd83d --- /dev/null +++ b/test-fixtures/issues/issue-5034/frontend/src/README.md @@ -0,0 +1,9 @@ +# Read me + +Related to [#5034](https://github.com/streetsidesoftware/cspell/issues/5034) + +bon après-midi + + diff --git a/test-fixtures/issues/issue-5034/frontend/src/cspell.config.yaml b/test-fixtures/issues/issue-5034/frontend/src/cspell.config.yaml new file mode 100644 index 00000000000..2ae9b76d930 --- /dev/null +++ b/test-fixtures/issues/issue-5034/frontend/src/cspell.config.yaml @@ -0,0 +1,5 @@ +name: Nested Config +version: "0.2" +import: "node_modules/@cspell/dict-fr-fr/cspell-ext.json" +words: + - cspell diff --git a/test-fixtures/issues/issue-5034/package.json b/test-fixtures/issues/issue-5034/package.json index 01fbbd0e2a4..0c2da26de10 100644 --- a/test-fixtures/issues/issue-5034/package.json +++ b/test-fixtures/issues/issue-5034/package.json @@ -4,7 +4,9 @@ "private": true, "dependencies": {}, "scripts": { - "test": "yarn --cwd frontend spellcheck" + "test": "pnpm run -r spellcheck" }, - "devDependencies": {} + "devDependencies": { + "cspell": "workspace:*" + } }