fix: Handle missing files when spell checking from a file list. (#2286)

fix: #2285 - Continue processing the list of files even if some are missing. - Report on any missing files. - error code is based upon `--no-must-file-files` option.
streetsidesoftware · Jan 20, 2022 · fd1e7e2 · fd1e7e2
1 parent 879a0c8
commit fd1e7e2
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 39 deletions.
diff --git a/packages/cspell/src/fileHelper.test.ts b/packages/cspell/src/fileHelper.test.ts
@@ -1,11 +1,13 @@
-import { readFileListFile, readFileListFiles } from './fileHelper';
+import { readFileInfo, readFileListFile, readFileListFiles } from './fileHelper';
 import * as path from 'path';
+import { IOError } from './util/errors';
 
 const fixtures = path.join(__dirname, '../fixtures/fileHelper');
 const fileListFile = path.join(fixtures, 'file-list.txt');
 const fileListFile2 = path.join(fixtures, 'nested/file-list-2.txt');
 
 const oc = expect.objectContaining;
+const r = path.resolve;
 
 describe('fileHelper', () => {
     test('readFileListFile', async () => {
@@ -33,4 +35,24 @@ describe('fileHelper', () => {
         const r = readFileListFiles(['not-found.txt']);
         return expect(r).rejects.toEqual(oc({ message: 'Error reading file list from: "not-found.txt"' }));
     });
+
+    test.each`
+        filename       | handleNotFound | expected
+        ${__dirname}   | ${true}        | ${{ filename: __dirname, text: '', errorCode: 'EISDIR' }}
+        ${'not_found'} | ${true}        | ${{ filename: r(__dirname, 'not_found'), text: '', errorCode: 'ENOENT' }}
+        ${__filename}  | ${true}        | ${oc({ filename: __filename, text: expect.stringMatching(/.+\n/) })}
+        ${__filename}  | ${false}       | ${oc({ filename: __filename, text: expect.stringMatching(/.+\n/) })}
+    `('readFile handle $filename $handleNotFound', async ({ filename, handleNotFound, expected }) => {
+        filename = r(__dirname, filename);
+        await expect(readFileInfo(filename, undefined, handleNotFound)).resolves.toEqual(expected);
+    });
+
+    test.each`
+        filename       | expected
+        ${__dirname}   | ${IOError}
+        ${'not_found'} | ${IOError}
+    `('readFile errors $filename', async ({ filename, expected }) => {
+        filename = r(__dirname, filename);
+        await expect(readFileInfo(filename, undefined, false)).rejects.toThrow(expected);
+    });
 });
diff --git a/packages/cspell/src/fileHelper.ts b/packages/cspell/src/fileHelper.ts
@@ -4,7 +4,7 @@ import getStdin from 'get-stdin';
 import { GlobOptions, globP } from './util/glob';
 import * as path from 'path';
 import { CSpellUserSettings, Document, fileToDocument, Issue } from 'cspell-lib';
-import { toApplicationError } from './util/errors';
+import { IOError, toApplicationError, toError } from './util/errors';
 
 const UTF8: BufferEncoding = 'utf8';
 const STDIN = 'stdin';
@@ -33,6 +33,7 @@ export async function readConfig(configFile: string | undefined, root: string |
 export interface FileInfo {
     filename: string;
     text?: string;
+    errorCode?: string;
 }
 export interface FileResult {
     fileInfo: FileInfo;
@@ -64,14 +65,25 @@ export function fileInfoToDocument(
     return fileToDocument(filename, text, languageId, locale);
 }
 
-export function readFileInfo(filename: string, encoding: string = UTF8): Promise<Required<FileInfo>> {
+interface ReadFileInfoResult extends FileInfo {
+    text: string;
+}
+
+export function readFileInfo(
+    filename: string,
+    encoding: string = UTF8,
+    handleNotFound = false
+): Promise<ReadFileInfoResult> {
     const pText = filename === STDIN ? getStdin() : fsp.readFile(filename, encoding);
     return pText.then(
         (text) => ({ text, filename }),
-        (error) => {
-            return error.code === 'EISDIR'
-                ? Promise.resolve({ text: '', filename })
-                : Promise.reject(toApplicationError(error, `Error reading file: "${filename}"`));
+        (e) => {
+            const error = toError(e);
+            return handleNotFound && error.code === 'EISDIR'
+                ? Promise.resolve({ text: '', filename, errorCode: error.code })
+                : handleNotFound && error.code === 'ENOENT'
+                ? Promise.resolve({ text: '', filename, errorCode: error.code })
+                : Promise.reject(new IOError(`Error reading file: "${filename}"`, error));
         }
     );
 }

diff --git a/packages/cspell/src/lint/lint.test.ts b/packages/cspell/src/lint/lint.test.ts
@@ -8,6 +8,7 @@ const samples = path.resolve(root, 'samples');
 const latexSamples = path.resolve(samples, 'latex');
 const hiddenSamples = path.resolve(samples, 'hidden-test');
 const filesToCheck = path.resolve(root, 'fixtures/features/file-list/files-to-check.txt');
+const filesToCheckWithMissing = path.resolve(root, 'fixtures/features/file-list/files-to-check-missing.txt');
 
 const oc = expect.objectContaining;
 const j = path.join;
@@ -24,23 +25,25 @@ describe('Linter Validation Tests', () => {
 
     // cspell:ignore Tufte
     test.each`
-        files               | options                                                                           | expectedRunResult              | expectedReport
-        ${[]}               | ${{ root: latexSamples }}                                                         | ${oc({ errors: 0, files: 4 })} | ${oc({ errorCount: 0, issues: [oc({ text: 'Tufte' })] })}
-        ${['**/ebook.tex']} | ${{ root: latexSamples }}                                                         | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${['**/ebook.tex']} | ${{ root: latexSamples, gitignore: true }}                                        | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${['**/hidden.md']} | ${{ root: hiddenSamples }}                                                        | ${oc({ errors: 0, files: 0 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${['**/hidden.md']} | ${{ root: hiddenSamples, dot: true }}                                             | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${['**/*.md']}      | ${{ root: hiddenSamples, dot: false }}                                            | ${oc({ errors: 0, files: 0 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${['**/*.md']}      | ${{ root: hiddenSamples }}                                                        | ${oc({ errors: 0, files: 0 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${['**/*.md']}      | ${{ root: hiddenSamples, dot: true }}                                             | ${oc({ errors: 0, files: 2 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${['**']}           | ${{ root: samples, config: j(samples, 'cspell-not-found.json') }}                 | ${oc({ errors: 1, files: 0 })} | ${oc({ errorCount: 1, errors: [expect.any(Error)], issues: [] })}
-        ${['**']}           | ${{ root: samples, config: j(samples, 'linked/cspell-import-missing.json') }}     | ${oc({ errors: 1, files: 0 })} | ${oc({ errorCount: 1, errors: [expect.any(Error)], issues: [] })}
-        ${['**/ebook.tex']} | ${{ root: samples, config: j(samples, 'cspell-missing-dict.json') }}              | ${oc({ errors: 0, files: 0 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
-        ${['**/ebook.tex']} | ${{ root: samples, config: j(samples, 'linked/cspell-import.json') }}             | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${[]}               | ${{ root, config: j(root, 'cspell.json'), fileLists: [filesToCheck], dot: true }} | ${oc({ errors: 0, files: 2 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${['**/*.md']}      | ${{ root, config: j(root, 'cspell.json'), fileLists: [filesToCheck] }}            | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${['**/*.ts']}      | ${{ root, config: j(root, 'cspell.json'), fileLists: [filesToCheck] }}            | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, issues: [] })}
-        ${[]}               | ${{ root, config: j(root, 'cspell.json'), fileLists: ['missing-file.txt'] }}      | ${oc({ errors: 1, files: 0 })} | ${oc({ errorCount: 1, errors: [expect.any(Error)], issues: [] })}
+        files               | options                                                                                                | expectedRunResult              | expectedReport
+        ${[]}               | ${{ root: latexSamples }}                                                                              | ${oc({ errors: 0, files: 4 })} | ${oc({ errorCount: 0, errors: [], issues: [oc({ text: 'Tufte' })] })}
+        ${['**/ebook.tex']} | ${{ root: latexSamples }}                                                                              | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**/ebook.tex']} | ${{ root: latexSamples, gitignore: true }}                                                             | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**/hidden.md']} | ${{ root: hiddenSamples }}                                                                             | ${oc({ errors: 0, files: 0 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**/hidden.md']} | ${{ root: hiddenSamples, dot: true }}                                                                  | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**/*.md']}      | ${{ root: hiddenSamples, dot: false }}                                                                 | ${oc({ errors: 0, files: 0 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**/*.md']}      | ${{ root: hiddenSamples }}                                                                             | ${oc({ errors: 0, files: 0 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**/*.md']}      | ${{ root: hiddenSamples, dot: true }}                                                                  | ${oc({ errors: 0, files: 2 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**']}           | ${{ root: samples, config: j(samples, 'cspell-not-found.json') }}                                      | ${oc({ errors: 1, files: 0 })} | ${oc({ errorCount: 1, errors: [expect.any(Error)], issues: [] })}
+        ${['**']}           | ${{ root: samples, config: j(samples, 'linked/cspell-import-missing.json') }}                          | ${oc({ errors: 1, files: 0 })} | ${oc({ errorCount: 1, errors: [expect.any(Error)], issues: [] })}
+        ${['**/ebook.tex']} | ${{ root: samples, config: j(samples, 'cspell-missing-dict.json') }}                                   | ${oc({ errors: 0, files: 0 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**/ebook.tex']} | ${{ root: samples, config: j(samples, 'linked/cspell-import.json') }}                                  | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${[]}               | ${{ root, config: j(root, 'cspell.json'), fileLists: [filesToCheck], dot: true }}                      | ${oc({ errors: 0, files: 2 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**/*.md']}      | ${{ root, config: j(root, 'cspell.json'), fileLists: [filesToCheck] }}                                 | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**/*.ts']}      | ${{ root, config: j(root, 'cspell.json'), fileLists: [filesToCheck] }}                                 | ${oc({ errors: 0, files: 1 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${[]}               | ${{ root, config: j(root, 'cspell.json'), fileLists: ['missing-file.txt'] }}                           | ${oc({ errors: 1, files: 0 })} | ${oc({ errorCount: 1, errors: [expect.any(Error)], issues: [] })}
+        ${['**']}           | ${{ root, config: j(root, 'cspell.json'), fileLists: [filesToCheckWithMissing] }}                      | ${oc({ errors: 0, files: 3 })} | ${oc({ errorCount: 0, errors: [], issues: [] })}
+        ${['**']}           | ${{ root, config: j(root, 'cspell.json'), fileLists: [filesToCheckWithMissing], mustFindFiles: true }} | ${oc({ errors: 1, files: 3 })} | ${oc({ errorCount: 1, errors: [expect.anything()], issues: [] })}
     `('runLint $files $options', async ({ files, options, expectedRunResult, expectedReport }) => {
         const reporter = new InMemoryReporter();
         const runResult = await runLint(new LintRequest(files, options, reporter));

diff --git a/packages/cspell/src/lint/lint.ts b/packages/cspell/src/lint/lint.ts
@@ -48,19 +48,32 @@ export async function runLint(cfg: LintRequest): Promise<RunResult> {
             return cachedResult;
         }
 
-        const fileInfo = await readFileInfo(filename);
-        const doc = fileInfoToDocument(fileInfo, cfg.options.languageId, cfg.locale);
-        const { text } = fileInfo;
-        reporter.debug(`Filename: ${filename}, LanguageIds: ${doc.languageId ?? 'default'}`);
         const result: FileResult = {
-            fileInfo,
+            fileInfo: {
+                filename,
+            },
             issues: [],
             processed: false,
             errors: 0,
             configErrors: 0,
             elapsedTimeMs: 0,
         };
 
+        const fileInfo = await readFileInfo(filename, undefined, true);
+        if (fileInfo.errorCode) {
+            if (fileInfo.errorCode !== 'EISDIR' && cfg.options.mustFindFiles) {
+                const err = toError(`File not found: "${filename}"`);
+                reporter.error('Linter:', err);
+                result.errors += 1;
+            }
+            return result;
+        }
+
+        const doc = fileInfoToDocument(fileInfo, cfg.options.languageId, cfg.locale);
+        const { text } = fileInfo;
+        reporter.debug(`Filename: ${filename}, LanguageIds: ${doc.languageId ?? 'default'}`);
+        result.fileInfo = fileInfo;
+
         const getElapsedTimeMs = getTimeMeasurer();
         let spellResult: Partial<cspell.SpellCheckFileResult> = {};
         reporter.info(
@@ -124,7 +137,7 @@ export async function runLint(cfg: LintRequest): Promise<RunResult> {
                 filename,
                 elapsedTimeMs: result?.elapsedTimeMs,
                 processed: result?.processed,
-                numErrors: result?.issues.length,
+                numErrors: result?.issues.length || result?.errors,
                 cached: result?.cached,
             });
 

diff --git a/packages/cspell/src/options.ts b/packages/cspell/src/options.ts
@@ -57,6 +57,11 @@ export interface LinterOptions extends BaseOptions, CacheOptions {
      * - an entry of `stdin` means to read the file list from **`stdin`**
      */
     fileLists?: string[] | undefined;
+
+    /**
+     * Files must be found and processed otherwise it is considered an error.
+     */
+    mustFindFiles?: boolean;
 }
 
 export interface TraceOptions extends BaseOptions {

diff --git a/packages/cspell/src/util/errors.test.ts b/packages/cspell/src/util/errors.test.ts
@@ -1,5 +1,7 @@
 import { CheckFailed, ApplicationError, toError, isError, toApplicationError } from './errors';
 
+const oc = expect.objectContaining;
+
 describe('errors', () => {
     test.each`
         ErrorClass          | params
@@ -27,12 +29,12 @@ describe('errors', () => {
         error                                | expected
         ${new CheckFailed('CheckFailed')}    | ${new Error('CheckFailed')}
         ${new ApplicationError('App Error')} | ${new Error('App Error')}
-        ${{ message: 'msg', name: 'error' }} | ${{ message: 'msg', name: 'error' }}
-        ${'hello'}                           | ${{ name: 'error', message: 'hello' }}
-        ${null}                              | ${{ name: 'error', message: 'null' }}
-        ${undefined}                         | ${{ name: 'error', message: 'undefined' }}
-        ${42}                                | ${{ name: 'error', message: '42' }}
-        ${{}}                                | ${{ name: 'error', message: '{}' }}
+        ${{ message: 'msg', name: 'error' }} | ${oc({ message: 'msg', name: 'error' })}
+        ${'hello'}                           | ${oc({ name: 'error', message: 'hello' })}
+        ${null}                              | ${oc({ name: 'error', message: 'null' })}
+        ${undefined}                         | ${oc({ name: 'error', message: 'undefined' })}
+        ${42}                                | ${oc({ name: 'error', message: '42' })}
+        ${{}}                                | ${oc({ name: 'error', message: '{}' })}
     `('toError $error', ({ error, expected }) => {
         expect(toError(error)).toEqual(expected);
     });

diff --git a/packages/cspell/src/util/errors.ts b/packages/cspell/src/util/errors.ts
@@ -11,11 +11,27 @@ export class ApplicationError extends Error {
     }
 }
 
-export function toError(e: unknown): Error {
+export class IOError extends ApplicationError {
+    constructor(message: string, readonly cause: NodeError) {
+        super(message, undefined, cause);
+    }
+
+    get code(): string | undefined {
+        return this.cause.code;
+    }
+
+    isNotFound() {
+        return this.cause.code === 'ENOENT';
+    }
+}
+
+export function toError(e: unknown): NodeError {
     if (isError(e)) return e;
+    const message = format(e);
     return {
         name: 'error',
-        message: format(e),
+        message,
+        toString: () => message,
     };
 }
 
@@ -32,6 +48,7 @@ export function toApplicationError(e: unknown, message?: string): ApplicationErr
     return new ApplicationError(message ?? err.message, undefined, err);
 }
 
-interface NodeError extends Error {
+export interface NodeError extends Error {
     code?: string;
+    toString?: () => string;
 }