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

dev: eslint-plugin improve suggestion speed #2590

Merged
merged 2 commits into from Mar 15, 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
21 changes: 16 additions & 5 deletions packages/cspell-lib/src/Settings/DictionarySettings.test.ts
Expand Up @@ -7,6 +7,7 @@ import { getDefaultSettings } from './DefaultSettings';
import * as DictSettings from './DictionarySettings';

const defaultSettings = getDefaultSettings();
const oc = expect.objectContaining;

describe('Validate DictionarySettings', () => {
test('expects default to not be empty', () => {
Expand Down Expand Up @@ -79,11 +80,13 @@ describe('Validate DictionarySettings', () => {
};

const nDef = DictSettings.mapDictDefToInternal(def, pathToConfig);
expect(nDef).toEqual({
name: 'words',
path: absolutePath,
__source: pathToConfig,
});
expect(nDef).toEqual(
oc({
name: 'words',
path: absolutePath,
__source: pathToConfig,
})
);

const legacyDef: DictionaryDefinitionLegacy = {
name: 'words',
Expand Down Expand Up @@ -114,4 +117,12 @@ describe('Validate DictionarySettings', () => {
'Trying to normalize a dictionary definition with a different source.'
);
});

test.each`
def | expected
${{}} | ${false}
${DictSettings.mapDictDefToInternal({ name: 'def', path: './dict.txt' }, __filename)} | ${true}
`('isDictionaryDefinitionInternal', ({ def, expected }) => {
expect(DictSettings.isDictionaryDefinitionInternal(def)).toBe(expected);
});
});
32 changes: 17 additions & 15 deletions packages/cspell-lib/src/Settings/DictionarySettings.ts
@@ -1,7 +1,6 @@
import type {
CustomDictionaryScope,
DictionaryDefinition,
DictionaryDefinitionPreferred,
DictionaryDefinitionAugmented,
DictionaryDefinitionCustom,
DictionaryFileTypes,
Expand Down Expand Up @@ -39,26 +38,23 @@ export function filterDictDefsToLoad(
dictRefIds: DictionaryReference[],
defs: DictionaryDefinitionInternal[]
): DictionaryDefinitionInternal[] {
function isDefP(def: DictionaryDefinition): def is DictionaryDefinitionPreferred {
return !!def.path;
}

const col = createDictionaryReferenceCollection(dictRefIds);
const dictIdSet = new Set(col.enabled());
const allActiveDefs = defs
.filter(({ name }) => dictIdSet.has(name))
.map((def) => ({ ...def, path: getFullPathName(def) }))
// Remove any empty paths.
.filter(isDefP);
const allActiveDefs = defs.filter(({ name }) => dictIdSet.has(name)).map(fixPath);
return [...new Map(allActiveDefs.map((d) => [d.name, d])).values()];
}

function getFullPathName(def: DictionaryDefinition) {
const { path: filePath = '', file = '' } = def;
if (!filePath && !file) {
return '';
function fixPath(def: DictionaryDefinitionInternal): DictionaryDefinitionInternal {
if (def instanceof _DictionaryDefinitionInternalWithSource) {
return def;
}
return path.join(filePath, file);
const { path: filePath = '', file = '' } = def;
const newPath = !filePath && !file ? '' : path.join(filePath, file);
return {
...def,
file: undefined,
path: newPath,
};
}

export function mapDictDefsToInternal(defs: undefined, pathToSettingsFile: string): undefined;
Expand Down Expand Up @@ -117,6 +113,12 @@ type DictDef = Partial<
UnionFields<UnionFields<DictionaryDefinition, DictionaryDefinitionAugmented>, DictionaryDefinitionCustom>
>;

export function isDictionaryDefinitionInternal(
def: DictionaryDefinition | DictionaryDefinitionInternal
): def is DictionaryDefinitionInternal {
return def instanceof _DictionaryDefinitionInternalWithSource;
}

class _DictionaryDefinitionInternalWithSource implements DictionaryDefinitionInternalWithSource {
private _weightMap: WeightMap | undefined;
readonly name: string;
Expand Down
30 changes: 7 additions & 23 deletions packages/cspell-lib/src/SpellingDictionary/Dictionaries.test.ts
Expand Up @@ -3,7 +3,7 @@ import * as fs from 'fs-extra';
import * as path from 'path';
import { createCSpellSettingsInternal as csi } from '../Models/CSpellSettingsInternalDef';
import { getDefaultSettings, loadConfig } from '../Settings';
import { filterDictDefsToLoad } from '../Settings/DictionarySettings';
import { filterDictDefsToLoad, mapDictDefToInternal } from '../Settings/DictionarySettings';
import * as Dictionaries from './Dictionaries';
import { __testing__ } from './DictionaryLoader';
import { isSpellingDictionaryLoadError } from './SpellingDictionaryError';
Expand All @@ -22,6 +22,8 @@ function log(msg: string): void {
}
}

const di = mapDictDefToInternal;

describe('Validate getDictionary', () => {
const ignoreCaseFalse = { ignoreCase: false };
const ignoreCaseTrue = { ignoreCase: true };
Expand Down Expand Up @@ -136,9 +138,8 @@ describe('Validate getDictionary', () => {
});

test('Dictionary NOT Found', async () => {
const weightMap = undefined;
const settings = csi({
dictionaryDefinitions: [{ name: 'my-words', path: './not-found.txt', weightMap, __source: undefined }],
dictionaryDefinitions: [di({ name: 'my-words', path: './not-found.txt' }, __filename)],
dictionaries: ['my-words'],
});

Expand Down Expand Up @@ -184,21 +185,10 @@ describe('Validate Refresh', () => {
const tempDictPathNotFound = tempPath('not-found.txt');
await fs.mkdirp(path.dirname(tempDictPath));
await fs.writeFile(tempDictPath, 'one\ntwo\nthree\n');
const weightMap = undefined;
const settings = getDefaultSettings();
const defs = (settings.dictionaryDefinitions || []).concat([
{
name: 'temp',
path: tempDictPath,
weightMap,
__source: undefined,
},
{
name: 'not_found',
path: tempDictPathNotFound,
weightMap,
__source: undefined,
},
di({ name: 'temp', path: tempDictPath }, __filename),
di({ name: 'not_found', path: tempDictPathNotFound }, __filename),
]);
const toLoad = ['node', 'html', 'css', 'not_found', 'temp'];
const defsToLoad = filterDictDefsToLoad(toLoad, defs);
Expand Down Expand Up @@ -242,15 +232,9 @@ describe('Validate Refresh', () => {
const tempDictPath = tempPath('words_sync.txt');
await fs.mkdirp(path.dirname(tempDictPath));
await fs.writeFile(tempDictPath, 'one\ntwo\nthree\n');
const weightMap = undefined;
const settings = getDefaultSettings();
const defs = (settings.dictionaryDefinitions || []).concat([
{
name: 'temp',
path: tempDictPath,
weightMap,
__source: undefined,
},
di({ name: 'temp', path: tempDictPath }, __filename),
]);
const toLoad = ['node', 'html', 'css', 'temp'];
const defsToLoad = filterDictDefsToLoad(toLoad, defs);
Expand Down
4 changes: 2 additions & 2 deletions packages/cspell-lib/src/SpellingDictionary/Dictionaries.ts
Expand Up @@ -7,11 +7,11 @@ import { SpellingDictionary } from './SpellingDictionary';
import { createCollection, SpellingDictionaryCollection } from './SpellingDictionaryCollection';

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

export function loadDictionaryDefsSync(defsToLoad: DictionaryDefinitionInternal[]): SpellingDictionary[] {
return defsToLoad.map((def) => loadDictionarySync(def.path, def));
return defsToLoad.map(loadDictionarySync);
}

export function refreshDictionaryCache(maxAge?: number): Promise<void> {
Expand Down
20 changes: 10 additions & 10 deletions packages/cspell-lib/src/SpellingDictionary/DictionaryLoader.test.ts
@@ -1,5 +1,6 @@
import * as path from 'path';
import { DictionaryDefinitionInternal } from '../Models/CSpellSettingsInternalDef';
import { mapDictDefToInternal } from '../Settings/DictionarySettings';
import { clean } from '../util/util';
import { loadDictionary, loadDictionarySync, LoadOptions, refreshCacheEntries, testing } from './DictionaryLoader';
jest.mock('../util/logger');
Expand All @@ -9,6 +10,8 @@ const samples = path.join(root, 'samples');

type ErrorResults = Record<string, unknown> | Error;

const di = mapDictDefToInternal;

describe('Validate DictionaryLoader', () => {
const errorENOENT = { code: 'ENOENT' };
const unknownFormatError = new Error('Unknown file format');
Expand Down Expand Up @@ -63,7 +66,7 @@ describe('Validate DictionaryLoader', () => {
path: filename,
name: filename,
});
const dict = await loadDictionary(filename, def);
const dict = await loadDictionary(def);
expect(dict.getErrors?.()).toHaveLength(1);
});

Expand Down Expand Up @@ -130,7 +133,8 @@ describe('Validate DictionaryLoader', () => {
hasErrors: boolean;
}) => {
await refreshCacheEntries(maxAge, Date.now());
const d = await loadDictionary(file, options);
const def = { ...options, path: file };
const d = await loadDictionary(def);
expect(d.has(word)).toBe(hasWord);
expect(!!d.getErrors?.().length).toBe(hasErrors);
}
Expand All @@ -155,7 +159,7 @@ describe('Validate DictionaryLoader', () => {
'dict has word $testCase $word',
async ({ word, hasWord, ignoreCase }: { word: string; hasWord: boolean; ignoreCase?: boolean }) => {
const file = sample('words.txt');
const d = await loadDictionary(file, dDef({ name: 'words', path: file }));
const d = await loadDictionary(dDef({ name: 'words', path: file }));
expect(d.has(word, clean({ ignoreCase }))).toBe(hasWord);
}
);
Expand All @@ -173,7 +177,7 @@ describe('Validate DictionaryLoader', () => {
${dDef({ name: 'words', path: dict('cities.txt'), type: 'W' })} | ${'New York'} | ${false}
${dDef({ name: 'words', path: dict('cities.txt'), type: 'W' })} | ${'York'} | ${true}
`('sync load dict has word $def $word', ({ def, word, hasWord }) => {
const d = loadDictionarySync(def.path, def);
const d = loadDictionarySync(def);
expect(d.has(word)).toBe(hasWord);
});

Expand All @@ -185,7 +189,7 @@ describe('Validate DictionaryLoader', () => {
${dDef({ name: 'words', path: dict('cities.missing.txt'), type: 'S' })} | ${[expect.any(Error)]}
${dDef({ name: 'words', path: dict('cities.missing.txt'), type: 'W' })} | ${[expect.any(Error)]}
`('sync load dict with error $def', ({ def, expected }) => {
const d = loadDictionarySync(def.path, def);
const d = loadDictionarySync(def);
expect(d.getErrors?.()).toEqual(expected);
});

Expand All @@ -207,9 +211,5 @@ interface DDef extends Partial<DictionaryDefinitionInternal> {
}

function dDef(opts: DDef): DictionaryDefinitionInternal {
return {
weightMap: undefined,
__source: '',
...opts,
};
return di(opts, __filename);
}
66 changes: 36 additions & 30 deletions packages/cspell-lib/src/SpellingDictionary/DictionaryLoader.ts
Expand Up @@ -79,51 +79,54 @@ export interface SyncLoaders {
}

const dictionaryCache = new Map<string, CacheEntry>();
const dictionaryCacheByDef = new Map<DictionaryDefinitionInternal, { key: string; entry: CacheEntry }>();

export function loadDictionary(uri: string, options: DictionaryDefinitionInternal): Promise<SpellingDictionary> {
const key = calcKey(uri, options);
function getCacheEntry(def: DictionaryDefinitionInternal): { key: string; entry: CacheEntry | undefined } {
const defEntry = dictionaryCacheByDef.get(def);
if (defEntry) {
return defEntry;
}
const key = calcKey(def);
const entry = dictionaryCache.get(key);
if (entry) {
// replace old entry so it can be released.
entry.options = def;
}
return { key, entry };
}

function setCacheEntry(key: string, entry: CacheEntry, def: DictionaryDefinitionInternal) {
dictionaryCache.set(key, entry);
dictionaryCacheByDef.set(def, { key, entry });
}

export function loadDictionary(def: DictionaryDefinitionInternal): Promise<SpellingDictionary> {
const { key, entry } = getCacheEntry(def);
if (entry) {
return entry.pending.then(([dictionary]) => dictionary);
}
const loadedEntry = loadEntry(uri, options);
dictionaryCache.set(key, loadedEntry);
const loadedEntry = loadEntry(def.path, def);
setCacheEntry(key, loadedEntry, def);
return loadedEntry.pending.then(([dictionary]) => dictionary);
}

export function loadDictionarySync(uri: string, options: DictionaryDefinitionInternal): SpellingDictionary {
const key = calcKey(uri, options);
const entry = dictionaryCache.get(key);
export function loadDictionarySync(def: DictionaryDefinitionInternal): SpellingDictionary {
const { key, entry } = getCacheEntry(def);
if (entry?.dictionary && entry.loadingState === LoadingState.Loaded) {
// if (entry.options.name === 'temp') {
// __log(
// `Cache Found ${entry.options.name}; ts: ${entry.sig.toFixed(2)}; file: ${path.relative(
// process.cwd(),
// entry.uri
// )}`
// );
// }
return entry.dictionary;
}
// if (options.name === 'temp') {
// __log(
// `Cache Miss ${options.name}; ts: ${entry?.sig.toFixed(2) || Date.now()}; file: ${path.relative(
// process.cwd(),
// uri
// )}`
// );
// }
const loadedEntry = loadEntrySync(uri, options);
dictionaryCache.set(key, loadedEntry);
const loadedEntry = loadEntrySync(def.path, def);
setCacheEntry(key, loadedEntry, def);
return loadedEntry.dictionary;
}

const importantOptionKeys: (keyof DictionaryDefinitionInternal)[] = ['name', 'noSuggest', 'useCompounds', 'type'];

function calcKey(uri: string, options: DictionaryDefinitionInternal) {
const loaderType = determineType(uri, options);
const optValues = importantOptionKeys.map((k) => options[k]?.toString() || '');
const parts = [uri, loaderType].concat(optValues);
function calcKey(def: DictionaryDefinitionInternal) {
const path = def.path;
const loaderType = determineType(path, def);
const optValues = importantOptionKeys.map((k) => def[k]?.toString() || '');
const parts = [path, loaderType].concat(optValues);

return parts.join('|');
}
Expand Down Expand Up @@ -160,7 +163,10 @@ async function refreshEntry(entry: CacheEntry, maxAge: number, now: number): Pro
// }
if (sigMatches && hasChanged) {
entry.loadingState = LoadingState.Loading;
dictionaryCache.set(calcKey(entry.uri, entry.options), loadEntry(entry.uri, entry.options));
const key = calcKey(entry.options);
const newEntry = loadEntry(entry.uri, entry.options);
dictionaryCache.set(key, newEntry);
dictionaryCacheByDef.set(entry.options, { key, entry: newEntry });
}
}
}
Expand Down
@@ -1,5 +1,6 @@
import * as Trie from 'cspell-trie-lib';
import { SpellingDictionaryOptions } from '.';
import { mapDictDefToInternal } from '../Settings/DictionarySettings';
import {
createFailedToLoadDictionary,
createForbiddenWordsDictionary,
Expand All @@ -10,6 +11,8 @@ import { createCollection, SpellingDictionaryCollection } from './SpellingDictio
import { SpellingDictionaryLoadError } from './SpellingDictionaryError';
import { SpellingDictionaryFromTrie } from './SpellingDictionaryFromTrie';

const di = mapDictDefToInternal;

describe('Verify using multiple dictionaries', () => {
const wordsA = [
'',
Expand Down Expand Up @@ -75,7 +78,7 @@ describe('Verify using multiple dictionaries', () => {
createFailedToLoadDictionary(
new SpellingDictionaryLoadError(
'./missing.txt',
{ name: 'error', path: './missing.txt', weightMap: undefined, __source: '' },
di({ name: 'error', path: './missing.txt' }, __filename),
new Error('error'),
'failed to load'
)
Expand Down
@@ -1,14 +1,17 @@
import { SpellingDictionaryOptions } from '.';
import { DictionaryInformation } from '..';
import { mapDictDefToInternal } from '../Settings/DictionarySettings';
import { createFailedToLoadDictionary, createSpellingDictionary } from './createSpellingDictionary';
import { SpellingDictionaryLoadError } from './SpellingDictionaryError';

const di = mapDictDefToInternal;

describe('Validate createSpellingDictionary', () => {
test('createFailedToLoadDictionary', () => {
const error = new Error('error');
const loaderError = new SpellingDictionaryLoadError(
'./missing.txt',
{ name: 'failed dict', path: './missing.txt', weightMap: undefined, __source: undefined },
di({ name: 'failed dict', path: './missing.txt' }, __filename),
error,
'Failed to load'
);
Expand Down