Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 23 additions & 1 deletion packages/cspell/src/fileHelper.test.ts
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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;
}