Skip to content

Commit

Permalink
fix: Handle missing files when spell checking from a file list. (#2286)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@Jason3S
Jason3S committed Jan 20, 2022
1 parent 879a0c8 commit fd1e7e2
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 39 deletions.
24 changes: 23 additions & 1 deletion packages/cspell/src/fileHelper.test.ts
View file
@@ -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 () => {
Expand Down Expand Up @@ -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);
});
});
24 changes: 18 additions & 6 deletions packages/cspell/src/fileHelper.ts
View file
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
);
}
Expand Down
37 changes: 20 additions & 17 deletions packages/cspell/src/lint/lint.test.ts
View file
Expand Up @@ -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;
Expand All @@ -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));
Expand Down
25 changes: 19 additions & 6 deletions packages/cspell/src/lint/lint.ts
View file
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
});

Expand Down
5 changes: 5 additions & 0 deletions packages/cspell/src/options.ts
View file
Expand Up @@ -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 {
Expand Down
14 changes: 8 additions & 6 deletions packages/cspell/src/util/errors.test.ts
View file
@@ -1,5 +1,7 @@
import { CheckFailed, ApplicationError, toError, isError, toApplicationError } from './errors';

const oc = expect.objectContaining;

describe('errors', () => {
test.each`
ErrorClass | params
Expand Down Expand Up @@ -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);
});
Expand Down
23 changes: 20 additions & 3 deletions packages/cspell/src/util/errors.ts
View file
Expand Up @@ -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,
};
}

Expand All @@ -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;
}

0 comments on commit fd1e7e2

Please sign in to comment.