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: Improve fetching/caching config files #5004

Merged
merged 9 commits into from
Nov 26, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion integration-tests/config/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,9 @@
"path": "eslint/eslint",
"url": "https://github.com/eslint/eslint",
"args": [
"**",
".",
"--config=../../../config/eslint/cspell.config.yaml",
"--issues-summary-report",
"--exclude=bin/**",
"--exclude=CHANGELOG.md",
"--exclude=_data",
Expand Down
78 changes: 78 additions & 0 deletions integration-tests/repositories/config/eslint/cspell.config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import:
- ../../cspell.yaml
words:
- autofix
- autofixes
- autofixer
- autofixed
- autofixable
- autofixing
- backreferences
- backreference
- builtins
- camelcase
- camelcased
- checkstyle
- classname
- codebase
- codebases
- codeframe
- dosomething
- doublecircle
- doublequote
- eqeqeq
- eslintconfig
- eslintplugin
- eslintignore
- espree
- escope
- esquery
- eslintrc
- eslintrcjson
- eslintrcyaml
- eslintrcjs
- eslintrcjson
- eslintrcyaml
- eslintrcjs
- fileoverview
- fillcolor
- fixedsize
- fixedmode
- gitignores
- globalstrict
- hashbang
- hashbangs
- iife
- iifes
- imurmurhash
- instanceof
- linebreak
- linebreaks
- autocrlf
- concat
- hiddenfolder
- hola
- lookaround
- lookbehinds
- multipass
- parenthesised
- pluggable
- postprocess
- postprocessed
- preprocess
- redeclaration
- redeclarations
- redeclared
- regexpp
- rulesdir
- rulesdirs
- singlequote
- testcase
- testcases
- testsuite
- testsuites
- typedefs
- unignore
- unignored
- unignores
- unparenthesized
758 changes: 654 additions & 104 deletions integration-tests/snapshots/eslint/eslint/report.yaml

Large diffs are not rendered by default.

100 changes: 5 additions & 95 deletions integration-tests/snapshots/eslint/eslint/snapshot.txt

Large diffs are not rendered by default.

49 changes: 46 additions & 3 deletions integration-tests/src/reporter/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { CSpellReporter, Issue, RunResult } from '@cspell/cspell-types';
import type { CSpellReporter, Issue, ReporterConfiguration, RunResult } from '@cspell/cspell-types';
import * as vscodeUri from 'vscode-uri';

import { readConfig } from '../config';
import type { Repository } from '../configDef';
import { writeSnapshotRaw } from '../snapshots';
import type { IssueSummary } from './reportGenerator';
import { generateReport } from './reportGenerator';
import { stringify } from './stringify';

Expand All @@ -13,10 +14,18 @@ const noopReporter = () => {
return;
};

export function getReporter(): CSpellReporter {
interface Config extends ReporterConfiguration {
issuesSummaryReport?: boolean;
}

export function getReporter(_settings: unknown, config?: Config): CSpellReporter {
const issueFilter = config?.unique ? uniqueFilter((i: Issue) => i.text) : () => true;
const issues: Issue[] = [];
const errors: string[] = [];
const files: string[] = [];
const issuesSummaryReport = !!config?.issuesSummaryReport;
const issuesSummary = new Map<string, IssueSummary>();
const summaryAccumulator = createIssuesSummaryAccumulator(issuesSummary);

async function processResult(result: RunResult): Promise<void> {
const root = URI.file(process.cwd());
Expand All @@ -27,14 +36,18 @@ export function getReporter(): CSpellReporter {
runResult: result,
root,
repository: fetchRepositoryInfo(root),
issuesSummary: issuesSummaryReport && issuesSummary.size ? [...issuesSummary.values()] : undefined,
});
const repPath = extractRepositoryPath(root);
writeSnapshotRaw(repPath, 'report.yaml', stringify(report));
}

const reporter: CSpellReporter = {
issue: (issue) => {
issues.push(issue);
summaryAccumulator(issue);
if (issueFilter(issue)) {
issues.push(issue);
}
},
info: noopReporter,
debug: noopReporter,
Expand All @@ -60,3 +73,33 @@ function fetchRepositoryInfo(root: vscodeUri.URI): Repository | undefined {
const path = extractRepositoryPath(root);
return reps.get(path);
}

function uniqueFilter<T, K>(keyFn: (v: T) => K): (v: T) => boolean {
const seen = new Set<K>();
return (v) => {
const k = keyFn(v);
if (seen.has(k)) return false;
seen.add(k);
return true;
};
}

function createIssuesSummaryAccumulator(issuesSummary: Map<string, IssueSummary>): (issue: Issue) => void {
function uniqueKey(issue: Issue): string {
return [issue.text, issue.uri || ''].join('::');
}

const isUnique = uniqueFilter(uniqueKey);

return (issue: Issue) => {
const { text } = issue;
const summary = issuesSummary.get(text) || { text, count: 0, files: 0 };
const unique = isUnique(issue);
summary.count += 1;
summary.files += unique ? 1 : 0;
if (issue.isFlagged) {
summary.isFlagged = true;
}
issuesSummary.set(text, summary);
};
}
11 changes: 11 additions & 0 deletions integration-tests/src/reporter/reportGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export type Report = {
errors: string[];
summary: Summary;
repository: Repository | undefined;
issuesSummary: IssueSummary[];
};

export interface Summary {
Expand All @@ -17,6 +18,13 @@ export interface Summary {
errors: number;
}

export interface IssueSummary {
text: string;
isFlagged?: boolean | undefined;
count: number;
files: number;
}

type SortedFileIssues = FileIssue[];
type FileIssue = string;

Expand All @@ -27,6 +35,7 @@ export interface ReportData {
root: Uri;
runResult: RunResult;
repository: Repository | undefined;
issuesSummary?: IssueSummary[] | undefined;
}

const compare = new Intl.Collator().compare;
Expand Down Expand Up @@ -61,6 +70,7 @@ export function generateReport(data: ReportData): Report {

const base: SortedFileIssues = [];
const fileIssues: SortedFileIssues = base.concat(...issuesByFile);
const issuesSummary = [...(data.issuesSummary || [])].sort((a, b) => compare(a.text, b.text));

return {
fileIssues,
Expand All @@ -72,6 +82,7 @@ export function generateReport(data: ReportData): Report {
errors: runResult.errors,
},
repository: data.repository,
issuesSummary,
};
}

Expand Down
14 changes: 13 additions & 1 deletion integration-tests/src/reporter/stringify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,17 @@ export function stringify(report: Report): string {
defaultKeyType: 'PLAIN',
doubleQuotedAsJSON: true,
});
return [header, issues].join('\n');

const issuesSummary = report.issuesSummary?.length
? stringifyYaml(
{
issuesSummary: report.issuesSummary.map((issue) =>
stringifyYaml(issue).replace(/\n/g, ', ').replace(/\s+/g, ' ').trim(),
),
},
{ lineWidth: 200 },
)
: '';

return [header, issues, issuesSummary].filter((a) => !!a).join('\n');
}
2 changes: 1 addition & 1 deletion packages/cspell-eslint-plugin/src/worker/spellCheck.mts
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ function getDocValidator(filename: string, text: string, options: WorkerOptions)
const cachedValidator = docValCache.get(doc);
if (cachedValidator && deepEqual(cachedValidator.settings, settings)) {
refreshDictionaryCache(0);
cachedValidator.updateDocumentText(text);
cachedValidator.updateDocumentText(text).catch(() => undefined);
return cachedValidator;
}

Expand Down
53 changes: 51 additions & 2 deletions packages/cspell-lib/api/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,17 @@ declare function searchForConfig(searchFrom: URL | string | undefined, pnpSettin
* @returns normalized CSpellSettings
*/
declare function loadConfig(file: string, pnpSettings?: PnPSettingsOptional): Promise<CSpellSettingsI$1>;
/**
* Might throw if the settings have not yet been loaded.
* @deprecated use {@link getGlobalSettingsAsync} instead.
*/
declare function getGlobalSettings(): CSpellSettingsI$1;
/**
* Loads and caches the global settings.
* @returns - global settings
*/
declare function getGlobalSettingsAsync(): Promise<CSpellSettingsI$1>;
declare function getCachedFileSize(): number;
declare function clearCachedSettingsFiles(): void;
declare function readRawSettings(filename: string | URL, relativeTo?: string | URL): Promise<CSpellSettingsWST$1>;

declare function extractImportErrors(settings: CSpellSettingsWST$1): ImportFileRefWithError$1[];
Expand Down Expand Up @@ -636,6 +644,11 @@ interface ValidateTextOptions {
* Verify that the in-document directives are correct.
*/
validateDirectives?: boolean;
/**
* Skips spell checking the document. Useful for testing and dry runs.
* It will read the configuration and parse the document.
*/
skipValidation?: boolean;
}

interface DocumentValidatorOptions extends ValidateTextOptions {
Expand All @@ -654,6 +667,7 @@ interface DocumentValidatorOptions extends ValidateTextOptions {
*/
noConfigSearch?: boolean;
}
type PerfTimings = Record<string, number>;
declare class DocumentValidator {
readonly settings: CSpellUserSettings;
private _document;
Expand All @@ -664,6 +678,8 @@ declare class DocumentValidator {
private _preparationTime;
private _suggestions;
readonly options: DocumentValidatorOptions;
readonly perfTiming: PerfTimings;
skipValidation: boolean;
/**
* @param doc - Document to validate
* @param config - configuration to use (not finalized).
Expand Down Expand Up @@ -717,6 +733,8 @@ declare class DocumentValidator {
* Internal `cspell-lib` use.
*/
_getPreparations(): Preparations | undefined;
private static getGlobMatcher;
private static _getGlobMatcher;
}
interface Preparations {
/** loaded config */
Expand Down Expand Up @@ -800,6 +818,12 @@ interface SpellCheckFileOptions extends ValidateTextOptions {
*/
noConfigSearch?: boolean;
}
interface SpellCheckFilePerf extends Record<string, number | undefined> {
loadTimeMs?: number;
prepareTimeMs?: number;
checkTimeMs?: number;
totalTimeMs?: number;
}
interface SpellCheckFileResult {
document: Document | DocumentWithText;
settingsUsed: CSpellSettingsWithSourceTrace;
Expand All @@ -808,6 +832,7 @@ interface SpellCheckFileResult {
issues: ValidationIssue[];
checked: boolean;
errors: Error[] | undefined;
perf?: SpellCheckFilePerf;
}
/**
* Spell Check a file
Expand Down Expand Up @@ -893,7 +918,21 @@ interface ResolveFileResult {
*/
declare function resolveFile(filename: string, relativeTo: string | URL): ResolveFileResult;

/**
* Clear the cached files and other cached data.
* Calling this function will cause the next spell check to take longer because it will need to reload configuration files and dictionaries.
* Call this function if configuration files have changed.
*
* It is safe to replace {@link clearCachedFiles} with {@link clearCaches}
*/
declare function clearCachedFiles(): Promise<void>;
/**
* Sends and event to clear the caches.
* It resets the configuration files and dictionaries.
*
* It is safe to replace {@link clearCaches} with {@link clearCachedFiles}
*/
declare function clearCaches(): void;

/**
* Load a dictionary collection defined by the settings.
Expand All @@ -902,4 +941,14 @@ declare function clearCachedFiles(): Promise<void>;
*/
declare function getDictionary(settings: CSpellUserSettings): Promise<SpellingDictionaryCollection>;

export { type CheckTextInfo, type ConfigurationDependencies, type CreateTextDocumentParams, type DetermineFinalDocumentSettingsResult, type Document, DocumentValidator, type DocumentValidatorOptions, ENV_CSPELL_GLOB_ROOT, type ExcludeFilesGlobMap, type ExclusionFunction, exclusionHelper_d as ExclusionHelper, type FeatureFlag, FeatureFlags, ImportError, type ImportFileRefWithError, IncludeExcludeFlag, type IncludeExcludeOptions, index_link_d as Link, type Logger, type SpellCheckFileOptions, type SpellCheckFileResult, SpellingDictionaryLoadError, type SuggestedWord, SuggestionError, type SuggestionOptions, type SuggestionsForWordResult, text_d as Text, type TextDocument, type TextDocumentLine, type TextDocumentRef, type TextInfoItem, type TraceOptions, type TraceResult, UnknownFeatureFlagError, type ValidationIssue, calcOverrideSettings, checkFilenameMatchesGlob, checkText, checkTextDocument, clearCachedFiles, clearCachedSettingsFiles, combineTextAndLanguageSettings, combineTextAndLanguageSettings as constructSettingsForText, createTextDocument, currentSettingsFileVersion, defaultConfigFilenames, defaultFileName, defaultFileName as defaultSettingsFilename, determineFinalDocumentSettings, extractDependencies, extractImportErrors, fileToDocument, fileToTextDocument, finalizeSettings, getCachedFileSize, getDefaultBundledSettingsAsync, getDefaultSettings, getDictionary, getGlobalSettings, getLanguagesForBasename as getLanguageIdsForBaseFilename, getLanguagesForExt, getLogger, getSources, getSystemFeatureFlags, isBinaryFile, isSpellingDictionaryLoadError, loadConfig, loadPnP, loadPnPSync, mergeInDocSettings, mergeSettings, readRawSettings, readSettings, readSettingsFiles, refreshDictionaryCache, resolveFile, searchForConfig, sectionCSpell, setLogger, shouldCheckDocument, spellCheckDocument, spellCheckFile, suggestionsForWord, suggestionsForWords, traceWords, traceWordsAsync, updateTextDocument, validateText };
interface PerfTimer {
readonly name: string;
readonly startTime: number;
readonly elapsed: number;
start(): void;
end(): void;
}
type TimeNowFn = () => number;
declare function createPerfTimer(name: string, onEnd?: (elapsed: number, name: string) => void, timeNowFn?: TimeNowFn): PerfTimer;

export { type CheckTextInfo, type ConfigurationDependencies, type CreateTextDocumentParams, type DetermineFinalDocumentSettingsResult, type Document, DocumentValidator, type DocumentValidatorOptions, ENV_CSPELL_GLOB_ROOT, type ExcludeFilesGlobMap, type ExclusionFunction, exclusionHelper_d as ExclusionHelper, type FeatureFlag, FeatureFlags, ImportError, type ImportFileRefWithError, IncludeExcludeFlag, type IncludeExcludeOptions, index_link_d as Link, type Logger, type PerfTimer, type SpellCheckFileOptions, type SpellCheckFileResult, SpellingDictionaryLoadError, type SuggestedWord, SuggestionError, type SuggestionOptions, type SuggestionsForWordResult, text_d as Text, type TextDocument, type TextDocumentLine, type TextDocumentRef, type TextInfoItem, type TraceOptions, type TraceResult, UnknownFeatureFlagError, type ValidationIssue, calcOverrideSettings, checkFilenameMatchesGlob, checkText, checkTextDocument, clearCachedFiles, clearCaches, combineTextAndLanguageSettings, combineTextAndLanguageSettings as constructSettingsForText, createPerfTimer, createTextDocument, currentSettingsFileVersion, defaultConfigFilenames, defaultFileName, defaultFileName as defaultSettingsFilename, determineFinalDocumentSettings, extractDependencies, extractImportErrors, fileToDocument, fileToTextDocument, finalizeSettings, getCachedFileSize, getDefaultBundledSettingsAsync, getDefaultSettings, getDictionary, getGlobalSettings, getGlobalSettingsAsync, getLanguagesForBasename as getLanguageIdsForBaseFilename, getLanguagesForExt, getLogger, getSources, getSystemFeatureFlags, isBinaryFile, isSpellingDictionaryLoadError, loadConfig, loadPnP, loadPnPSync, mergeInDocSettings, mergeSettings, readRawSettings, readSettings, readSettingsFiles, refreshDictionaryCache, resolveFile, searchForConfig, sectionCSpell, setLogger, shouldCheckDocument, spellCheckDocument, spellCheckFile, suggestionsForWord, suggestionsForWords, traceWords, traceWordsAsync, updateTextDocument, validateText };
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
getCachedFileSize,
getDefaultConfigLoader,
getGlobalSettings,
getGlobalSettingsAsync,
loadConfig,
readConfigFile,
readRawSettings,
Expand Down Expand Up @@ -109,13 +110,14 @@ describe('Validate CSpellSettingsServer', () => {

test('makes sure global settings is an object', async () => {
const settings = getGlobalSettings();
expect(await getGlobalSettingsAsync()).toBe(settings);
expect(Object.keys(settings)).not.toHaveLength(0);
const merged = mergeSettings(await getDefaultBundledSettingsAsync(), await getGlobalSettings());
const merged = mergeSettings(await getDefaultBundledSettingsAsync(), await getGlobalSettingsAsync());
expect(Object.keys(merged)).not.toHaveLength(0);
});

test('verify clearing the file cache works', async () => {
mergeSettings(await getDefaultBundledSettingsAsync(), await getGlobalSettings());
mergeSettings(await getDefaultBundledSettingsAsync(), await getGlobalSettingsAsync());
expect(getCachedFileSize()).toBeGreaterThan(0);
clearCachedSettingsFiles();
expect(getCachedFileSize()).toBe(0);
Expand Down
Loading