Skip to content

Commit

Permalink
fix: Invalidate cache when config has changed (#2160)
Browse files Browse the repository at this point in the history
* Invalidate cache when config or dictionaries have changed
* Move the dependency calc for dictionaries
* improve code coverage
  • Loading branch information
Jason3S committed Jan 2, 2022
1 parent 87425b4 commit 705c638
Show file tree
Hide file tree
Showing 11 changed files with 191 additions and 99 deletions.
13 changes: 13 additions & 0 deletions packages/cspell-lib/src/Settings/CSpellSettingsServer.test.ts
Expand Up @@ -8,6 +8,7 @@ import {
checkFilenameMatchesGlob,
clearCachedSettingsFiles,
ENV_CSPELL_GLOB_ROOT,
extractDependencies,
extractImportErrors,
getCachedFileSize,
getGlobalSettings,
Expand All @@ -28,6 +29,7 @@ import { getDefaultSettings, _defaultSettings } from './DefaultSettings';
const { normalizeSettings, validateRawConfigVersion, validateRawConfigExports } = __testing__;

const rootCspellLib = path.resolve(path.join(__dirname, '../..'));
const root = path.resolve(rootCspellLib, '../..');
const samplesDir = path.resolve(rootCspellLib, 'samples');
const samplesSrc = path.join(samplesDir, 'src');
const testFixtures = path.join(rootCspellLib, '../../test-fixtures');
Expand Down Expand Up @@ -627,6 +629,17 @@ describe('Validate search/load config files', () => {
});
});

describe('Validate Dependencies', () => {
test.each`
filename | relativeTo | expected
${r('../../cspell.config.json')} | ${undefined} | ${{ configFiles: [r(root, 'cspell.json'), r('../../cspell.config.json')], dictionaryFiles: [r(root, 'cspell-dict.txt')] }}
`('tests readSettings $filename $relativeTo', ({ filename, relativeTo, expected }) => {
const settings = readSettings(filename, relativeTo);
const dependencies = extractDependencies(settings);
expect(dependencies).toEqual(expected);
});
});

function p(...parts: string[]): string {
return path.join(...parts);
}
Expand Down
21 changes: 18 additions & 3 deletions packages/cspell-lib/src/Settings/CSpellSettingsServer.ts
Expand Up @@ -17,7 +17,7 @@ import { URI } from 'vscode-uri';
import { logError, logWarning } from '../util/logger';
import { resolveFile } from '../util/resolveFile';
import * as util from '../util/util';
import { normalizePathForDictDefs } from './DictionarySettings';
import { normalizePathForDictDefs, calcDictionaryDefsToLoad } from './DictionarySettings';
import { getRawGlobalSettings } from './GlobalSettings';
import { ImportError } from './ImportError';
import { resolvePatterns } from './patterns';
Expand Down Expand Up @@ -652,7 +652,7 @@ export function getSources(settings: CSpellSettings): CSpellSettings[] {

type Imports = CSpellSettings['__imports'];

function mergeImportRefs(left: CSpellSettings, right: CSpellSettings): Imports {
function mergeImportRefs(left: CSpellSettings, right: CSpellSettings = {}): Imports {
const imports = new Map(left.__imports || []);
if (left.__importRef) {
imports.set(left.__importRef.filename, left.__importRef);
Expand All @@ -676,10 +676,25 @@ function isImportFileRefWithError(ref: ImportFileRef): ref is ImportFileRefWithE
}

export function extractImportErrors(settings: CSpellSettings): ImportFileRefWithError[] {
const imports = mergeImportRefs(settings, {});
const imports = mergeImportRefs(settings);
return !imports ? [] : [...imports.values()].filter(isImportFileRefWithError);
}

export interface ConfigurationDependencies {
configFiles: string[];
dictionaryFiles: string[];
}

export function extractDependencies(settings: CSpellSettings): ConfigurationDependencies {
const configFiles = [...(mergeImportRefs(settings) || [])].map(([filename]) => filename);
const dictionaryFiles = calcDictionaryDefsToLoad(settings).map((dict) => dict.path);

return {
configFiles,
dictionaryFiles,
};
}

function resolveGlobRoot(settings: CSpellSettings, pathToSettingsFile: string): string {
const settingsFileDirRaw = path.dirname(pathToSettingsFile);
const isVSCode = path.basename(settingsFileDirRaw) === '.vscode';
Expand Down
19 changes: 18 additions & 1 deletion packages/cspell-lib/src/Settings/DictionarySettings.ts
@@ -1,4 +1,9 @@
import type { DictionaryDefinition, DictionaryDefinitionPreferred, DictionaryReference } from '@cspell/cspell-types';
import type {
DictionaryDefinition,
DictionaryDefinitionPreferred,
DictionaryReference,
CSpellSettingsWithSourceTrace,
} from '@cspell/cspell-types';
import * as path from 'path';
import { resolveFile } from '../util/resolveFile';
import { createDictionaryReferenceCollection } from './DictionaryReferenceCollection';
Expand Down Expand Up @@ -98,3 +103,15 @@ export function isDictionaryDefinitionWithSource(
function determineName(filename: string, options: DictionaryDefinition): string {
return options.name || path.basename(filename);
}

export function calcDictionaryDefsToLoad(settings: CSpellSettingsWithSourceTrace): DictionaryDefinitionPreferred[] {
const { dictionaries = [], dictionaryDefinitions = [], noSuggestDictionaries = [] } = settings;
const colNoSug = createDictionaryReferenceCollection(noSuggestDictionaries);
const colDicts = createDictionaryReferenceCollection(dictionaries.concat(colNoSug.enabled()));
const modDefs = dictionaryDefinitions.map((def) => {
const enabled = colNoSug.isEnabled(def.name);
if (enabled === undefined) return def;
return { ...def, noSuggest: enabled };
});
return filterDictDefsToLoad(colDicts.enabled(), modDefs);
}
10 changes: 6 additions & 4 deletions packages/cspell-lib/src/SpellingDictionary/Dictionaries.test.ts
Expand Up @@ -2,6 +2,7 @@ import type { CSpellUserSettings } from '@cspell/cspell-types';
import * as fs from 'fs-extra';
import * as path from 'path';
import { getDefaultSettings, loadConfig } from '../Settings';
import { filterDictDefsToLoad } from '../Settings/DictionarySettings';
import * as Dictionaries from './Dictionaries';
import { isSpellingDictionaryLoadError } from './SpellingDictionaryError';

Expand Down Expand Up @@ -151,13 +152,14 @@ describe('Validate getDictionary', () => {
},
]);
const toLoad = ['node', 'html', 'css', 'not_found', 'temp'];
const dicts = await Promise.all(Dictionaries.loadDictionaries(toLoad, defs));
const defsToLoad = filterDictDefsToLoad(toLoad, defs);
const dicts = await Promise.all(Dictionaries.loadDictionaryDefs(defsToLoad));

expect(dicts[3].has('one')).toBe(true);
expect(dicts[3].has('four')).toBe(false);

await Dictionaries.refreshDictionaryCache(0);
const dicts2 = await Promise.all(Dictionaries.loadDictionaries(toLoad, defs));
const dicts2 = await Promise.all(Dictionaries.loadDictionaryDefs(defsToLoad));

// Since noting changed, expect them to be the same.
expect(dicts.length).toEqual(toLoad.length);
Expand All @@ -167,14 +169,14 @@ describe('Validate getDictionary', () => {
// Update one of the dictionaries to see if it loads.
await fs.writeFile(tempDictPath, 'one\ntwo\nthree\nfour\n');

const dicts3 = await Promise.all(Dictionaries.loadDictionaries(toLoad, defs));
const dicts3 = await Promise.all(Dictionaries.loadDictionaryDefs(defsToLoad));
// Should be using cache and will not contain the new words.
expect(dicts3[3].has('one')).toBe(true);
expect(dicts3[3].has('four')).toBe(false);

await Dictionaries.refreshDictionaryCache(0);

const dicts4 = await Promise.all(Dictionaries.loadDictionaries(toLoad, defs));
const dicts4 = await Promise.all(Dictionaries.loadDictionaryDefs(defsToLoad));
// Should be using the latest copy of the words.
expect(dicts4[3].has('one')).toBe(true);
expect(dicts4[3].has('four')).toBe(true);
Expand Down
31 changes: 5 additions & 26 deletions packages/cspell-lib/src/SpellingDictionary/Dictionaries.ts
@@ -1,18 +1,12 @@
import type { CSpellUserSettings, DictionaryDefinition, DictionaryReference } from '@cspell/cspell-types';
import { createDictionaryReferenceCollection } from '../Settings/DictionaryReferenceCollection';
import { filterDictDefsToLoad } from '../Settings/DictionarySettings';
import type { CSpellUserSettings, DictionaryDefinitionPreferred } from '@cspell/cspell-types';
import { calcDictionaryDefsToLoad } from '../Settings/DictionarySettings';
import { createForbiddenWordsDictionary, createSpellingDictionary } from './createSpellingDictionary';
import { loadDictionary, refreshCacheEntries } from './DictionaryLoader';
import { SpellingDictionaryCollection } from './index';
import { SpellingDictionary } from './SpellingDictionary';
import { createCollectionP } from './SpellingDictionaryCollection';

export function loadDictionaries(
dictIds: DictionaryReference[],
defs: DictionaryDefinition[]
): Promise<SpellingDictionary>[] {
const defsToLoad = filterDictDefsToLoad(dictIds, defs);

export function loadDictionaryDefs(defsToLoad: DictionaryDefinitionPreferred[]): Promise<SpellingDictionary>[] {
return defsToLoad.map((def) => loadDictionary(def.path, def));
}

Expand All @@ -21,23 +15,8 @@ export function refreshDictionaryCache(maxAge?: number): Promise<void> {
}

export function getDictionary(settings: CSpellUserSettings): Promise<SpellingDictionaryCollection> {
const {
words = [],
userWords = [],
dictionaries = [],
dictionaryDefinitions = [],
noSuggestDictionaries = [],
flagWords = [],
ignoreWords = [],
} = settings;
const colNoSug = createDictionaryReferenceCollection(noSuggestDictionaries);
const colDicts = createDictionaryReferenceCollection(dictionaries.concat(colNoSug.enabled()));
const modDefs = dictionaryDefinitions.map((def) => {
const enabled = colNoSug.isEnabled(def.name);
if (enabled === undefined) return def;
return { ...def, noSuggest: enabled };
});
const spellDictionaries = loadDictionaries(colDicts.enabled(), modDefs);
const { words = [], userWords = [], flagWords = [], ignoreWords = [] } = settings;
const spellDictionaries = loadDictionaryDefs(calcDictionaryDefsToLoad(settings));
const settingsDictionary = createSpellingDictionary(
words.concat(userWords),
'[words]',
Expand Down
5 changes: 3 additions & 2 deletions packages/cspell/.vscode/launch.json
Expand Up @@ -10,8 +10,9 @@
"name": "cspell: Run",
"program": "${workspaceRoot}/bin.js",
"args": [
"-c=../../cspell.json",
"**/*"
"--cache",
"--no-progress",
"**"
],
"cwd": "${workspaceRoot}",
"outFiles": [
Expand Down
6 changes: 2 additions & 4 deletions packages/cspell/cSpell.json
Expand Up @@ -4,9 +4,7 @@
"id": "cspell-project-config",
"name": "cspell Project Config",
"language": "en",
"words": [
"configstore"
],
"words": [],
"maxNumberOfProblems": 1000,
"ignorePaths": [
"*.snap",
Expand All @@ -21,7 +19,7 @@
"dictionaryDefinitions": [
{
"name": "workspace",
"file": "../../cspell-dict.txt",
"path": "../../cspell-dict.txt",
"description": "Custom Workspace Dictionary"
}
],
Expand Down
19 changes: 15 additions & 4 deletions packages/cspell/src/lint/lint.ts
Expand Up @@ -3,9 +3,8 @@ import { MessageTypes } from '@cspell/cspell-types';
import * as commentJson from 'comment-json';
import { findRepoRoot, GitIgnore } from 'cspell-gitignore';
import { GlobMatcher, type GlobMatchOptions, type GlobPatternNormalized, type GlobPatternWithRoot } from 'cspell-glob';
import type { ValidationIssue } from 'cspell-lib';
import type { Logger, ValidationIssue } from 'cspell-lib';
import * as cspell from 'cspell-lib';
import { Logger } from 'cspell-lib';
import * as path from 'path';
import { format } from 'util';
import { URI } from 'vscode-uri';
Expand Down Expand Up @@ -43,7 +42,7 @@ export async function runLint(cfg: LintRequest): Promise<RunResult> {
configInfo: ConfigInfo,
cache: CSpellLintResultCache
): Promise<FileResult> {
const cachedResult = await cache.getCachedLintResults(filename, configInfo);
const cachedResult = await cache.getCachedLintResults(filename);
if (cachedResult) {
reporter.debug(`Filename: ${filename}, using cache`);
return cachedResult;
Expand Down Expand Up @@ -95,7 +94,9 @@ export async function runLint(cfg: LintRequest): Promise<RunResult> {
reporter.info(`Config file Used: ${spellResult.localConfigFilepath || configInfo.source}`, MessageTypes.Info);
reporter.info(`Dictionaries Used: ${dictionaries.join(', ')}`, MessageTypes.Info);

cache.setCachedLintResults(result, configInfo);
const dep = calcDependencies(config);

cache.setCachedLintResults(result, dep.files);
return result;
}

Expand Down Expand Up @@ -154,6 +155,16 @@ export async function runLint(cfg: LintRequest): Promise<RunResult> {
return status;
}

interface ConfigDependencies {
files: string[];
}

function calcDependencies(config: CSpellSettings): ConfigDependencies {
const { configFiles, dictionaryFiles } = cspell.extractDependencies(config);

return { files: configFiles.concat(dictionaryFiles) };
}

async function reportConfigurationErrors(config: CSpellSettings): Promise<number> {
const errors = cspell.extractImportErrors(config);
let count = 0;
Expand Down
7 changes: 3 additions & 4 deletions packages/cspell/src/util/cache/CSpellLintResultCache.ts
@@ -1,14 +1,13 @@
import { ConfigInfo, FileResult } from '../../fileHelper';

import { FileResult } from '../../fileHelper';
export interface CSpellLintResultCache {
/**
* Retrieve cached lint results for a given file name, if present in the cache.
*/
getCachedLintResults(filename: string, configInfo: ConfigInfo): Promise<FileResult | undefined>;
getCachedLintResults(filename: string): Promise<FileResult | undefined>;
/**
* Set the cached lint results.
*/
setCachedLintResults(result: FileResult, configInfo: ConfigInfo): void;
setCachedLintResults(result: FileResult, dependsUponFiles: string[]): void;
/**
* Persists the in-memory cache to disk.
*/
Expand Down

0 comments on commit 705c638

Please sign in to comment.