From 611fe66220556b1f0d4389295524a729ebafbf2f Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Thu, 7 Dec 2023 20:27:54 +0100 Subject: [PATCH 1/2] fix: Make sure virtual configs do not appear as source files --- .../cspell-config-lib/src/CSpellConfigFile.ts | 27 ++- .../CSpellConfigFileInMemory.test.ts | 10 ++ .../CSpellConfigFileInMemory.ts | 2 +- .../CSpellConfigFileJavaScript.test.ts | 49 ++++++ .../CSpellConfigFileJson.test.ts | 49 ++++++ .../CSpellConfigFile/CSpellConfigFileJson.ts | 48 ++++-- packages/cspell-config-lib/src/TextFile.ts | 4 + .../src/loaders/loaderJavaScript.test.ts | 2 +- .../src/serializers/cspellJson.test.ts | 2 +- packages/cspell-lib/api/api.d.ts | 2 + .../lib/Settings/CSpellSettingsServer.test.ts | 7 +- .../src/lib/Settings/CSpellSettingsServer.ts | 2 + .../configLoader/configLoader.test.ts | 12 +- .../Controller/configLoader/configLoader.ts | 13 +- .../configLoader/configToRawSettings.test.ts | 18 ++ .../configLoader/configToRawSettings.ts | 7 +- .../src/lib/Settings/GlobalSettings.test.ts | 23 ++- .../src/lib/Settings/GlobalSettings.ts | 12 +- .../src/lib/Settings/mergeCache.test.ts | 163 ++++++++++++++++++ .../cspell-lib/src/lib/Settings/mergeCache.ts | 10 +- .../cspell-lib/src/lib/Settings/mergeList.ts | 7 + packages/cspell-lib/src/lib/perf/perf.test.ts | 35 ++++ packages/cspell-lib/src/lib/perf/perf.ts | 42 +++-- .../src/lib/util/AutoResolve.test.ts | 64 +++++++ .../cspell-lib/src/lib/util/AutoResolve.ts | 68 +++++++- packages/cspell/src/app/lint/lint.ts | 7 +- 26 files changed, 638 insertions(+), 47 deletions(-) create mode 100644 packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJavaScript.test.ts create mode 100644 packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJson.test.ts create mode 100644 packages/cspell-lib/src/lib/Settings/Controller/configLoader/configToRawSettings.test.ts create mode 100644 packages/cspell-lib/src/lib/Settings/mergeCache.test.ts create mode 100644 packages/cspell-lib/src/lib/perf/perf.test.ts diff --git a/packages/cspell-config-lib/src/CSpellConfigFile.ts b/packages/cspell-config-lib/src/CSpellConfigFile.ts index e074c8d384b..4a7116bcb84 100644 --- a/packages/cspell-config-lib/src/CSpellConfigFile.ts +++ b/packages/cspell-config-lib/src/CSpellConfigFile.ts @@ -5,9 +5,26 @@ export interface CSpellConfigFileReference { } export interface ICSpellConfigFile { + /** + * The url of the config file, used to resolve imports. + */ readonly url: URL; + /** + * The settings from the config file. + */ readonly settings: CSpellSettings; - readonly readonly?: boolean; + /** + * Indicate that the config file is readonly. + */ + readonly?: boolean; + /** + * Indicate that the config file is virtual and not associated with a file on disk. + */ + virtual?: boolean; + /** + * Indicate that the config file is remote and not associated with a file on disk. + */ + remote?: boolean; } export abstract class CSpellConfigFile implements ICSpellConfigFile { @@ -19,6 +36,14 @@ export abstract class CSpellConfigFile implements ICSpellConfigFile { get readonly(): boolean { return this.settings.readonly || this.url.protocol !== 'file:'; } + + get virtual(): boolean { + return false; + } + + get remote(): boolean { + return this.url.protocol !== 'file:'; + } } export abstract class ImplCSpellConfigFile extends CSpellConfigFile { diff --git a/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileInMemory.test.ts b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileInMemory.test.ts index 0c1bcc2099e..675034daa92 100644 --- a/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileInMemory.test.ts +++ b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileInMemory.test.ts @@ -26,4 +26,14 @@ describe('CSpellConfigFileInMemory', () => { const configFile = new CSpellConfigFileInMemory(url, settings); expect(configFile.readonly).toBe(true); }); + + test('should be remote', () => { + const configFile = new CSpellConfigFileInMemory(url, settings); + expect(configFile.remote).toBe(true); + }); + + test('should be virtual', () => { + const configFile = new CSpellConfigFileInMemory(url, settings); + expect(configFile.virtual).toBe(true); + }); }); diff --git a/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileInMemory.ts b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileInMemory.ts index 21160bb44a5..23fde12a044 100644 --- a/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileInMemory.ts +++ b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileInMemory.ts @@ -11,7 +11,7 @@ export class CSpellConfigFileInMemory extends ImplCSpellConfigFile { super(url, settings); } - get readonly(): boolean { + get virtual(): boolean { return true; } } diff --git a/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJavaScript.test.ts b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJavaScript.test.ts new file mode 100644 index 00000000000..7f2cac26937 --- /dev/null +++ b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJavaScript.test.ts @@ -0,0 +1,49 @@ +import type { CSpellSettings } from '@cspell/cspell-types'; +import { describe, expect, test } from 'vitest'; + +import { CSpellConfigFileJavaScript } from './CSpellConfigFileJavaScript.js'; + +describe('CSpellConfigFileJavaScript', () => { + const url = new URL('https://example.com/config'); + const settings: CSpellSettings = {}; + + test('should create an instance of CSpellConfigFileInMemory', () => { + const configFile = new CSpellConfigFileJavaScript(url, settings); + expect(configFile).toBeInstanceOf(CSpellConfigFileJavaScript); + }); + + test('should have the correct URL', () => { + const configFile = new CSpellConfigFileJavaScript(url, settings); + expect(configFile.url).toEqual(url); + }); + + test('should have the correct settings', () => { + const configFile = new CSpellConfigFileJavaScript(url, settings); + expect(configFile.settings).toEqual(settings); + }); + + test('should be readonly', () => { + const configFile = new CSpellConfigFileJavaScript(url, settings); + expect(configFile.readonly).toBe(true); + }); + + test('should NOT be remote', () => { + const configFile = new CSpellConfigFileJavaScript(new URL(import.meta.url), settings); + expect(configFile.remote).toBe(false); + }); + + test('should be remote', () => { + const configFile = new CSpellConfigFileJavaScript(url, settings); + expect(configFile.remote).toBe(true); + }); + + test('should NOT be virtual', () => { + const configFile = new CSpellConfigFileJavaScript(url, settings); + expect(configFile.virtual).toBe(false); + }); + + test('should throw when adding words', () => { + const configFile = new CSpellConfigFileJavaScript(url, settings); + expect(() => configFile.addWords(['word'])).toThrowError('Unable to add words to a JavaScript config file.'); + }); +}); diff --git a/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJson.test.ts b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJson.test.ts new file mode 100644 index 00000000000..865dcafc367 --- /dev/null +++ b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJson.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, test } from 'vitest'; + +import { createTextFile } from '../TextFile.js'; +import { CSpellConfigFileJson } from './CSpellConfigFileJson.js'; + +describe('CSpellConfigFileJson', () => { + const url = new URL('https://example.com/config.json'); + const settings = { + language: 'en', + ignoreWords: ['foo', 'bar'], + }; + + test('should serialize the settings to JSON', () => { + const configFile = new CSpellConfigFileJson(url, settings); + const serialized = configFile.serialize(); + expect(JSON.parse(serialized)).toEqual(settings); + }); + + test('should serialize and preserve indent', () => { + const content = JSON.stringify(settings, undefined, '\t') + '\n'; + const configFile = CSpellConfigFileJson.parse({ url, content }); + const serialized = configFile.serialize(); + expect(serialized).toEqual(content); + }); + + test('should parse a JSON file into CSpellConfigFileJson object', () => { + const json = `{ + // This is a comment + "language": "en", + "ignoreWords": ["foo", "bar"] + }`; + const file = createTextFile(url, json); + const configFile = CSpellConfigFileJson.parse(file); + expect(configFile.url).toEqual(url); + expect(configFile.settings).toEqual(settings); + + const serialized = configFile.serialize(); + expect(serialized).toEqual(expect.stringContaining('// This is a comment')); + }); + + test('should throw an error when parsing an invalid JSON file', () => { + const json = `{ + "language": "en", + "ignoreWords": ["foo", "bar" + }`; + const file = createTextFile(url, json); + expect(() => CSpellConfigFileJson.parse(file)).toThrowError(); + }); +}); diff --git a/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJson.ts b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJson.ts index 978d93fa222..10ca2e17808 100644 --- a/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJson.ts +++ b/packages/cspell-config-lib/src/CSpellConfigFile/CSpellConfigFileJson.ts @@ -2,39 +2,57 @@ import type { CSpellSettings } from '@cspell/cspell-types'; import { parse, stringify } from 'comment-json'; import { ImplCSpellConfigFile } from '../CSpellConfigFile.js'; -import type { SerializeSettingsFn } from '../Serializer.js'; import { detectIndent } from '../serializers/util.js'; import type { TextFile } from '../TextFile.js'; export class CSpellConfigFileJson extends ImplCSpellConfigFile { + public indent: string | number = 2; + constructor( readonly url: URL, readonly settings: CSpellSettings, - readonly serializer: SerializeSettingsFn, ) { super(url, settings); } serialize() { - return this.serializer(this.settings); + return stringify(this.settings, null, this.indent) + '\n'; } -} -export function parseCSpellConfigFileJson(file: TextFile): CSpellConfigFileJson { - const cspell: CSpellSettings | unknown = parse(file.content); - if (!isCSpellSettings(cspell)) { - throw new Error(`Unable to parse ${file.url}`); - } - - const indent = detectIndent(file.content); - - function serialize(settings: CSpellSettings) { - return stringify(settings, null, indent) + '\n'; + public static parse(file: TextFile): CSpellConfigFileJson { + try { + const cspell: CSpellSettings | unknown = parse(file.content); + if (!isCSpellSettings(cspell)) { + throw new ParseError(file.url); + } + + const indent = detectIndent(file.content); + const cfg = new CSpellConfigFileJson(file.url, cspell); + cfg.indent = indent; + return cfg; + } catch (cause) { + if (cause instanceof ParseError) { + throw cause; + } + throw new ParseError(file.url, undefined, { cause }); + } } +} - return new CSpellConfigFileJson(file.url, cspell, serialize); +export function parseCSpellConfigFileJson(file: TextFile): CSpellConfigFileJson { + return CSpellConfigFileJson.parse(file); } function isCSpellSettings(cfg: unknown): cfg is CSpellSettings { return !(!cfg || typeof cfg !== 'object' || Array.isArray(cfg)); } + +class ParseError extends Error { + constructor( + readonly url: URL, + message?: string, + options?: ErrorOptions, + ) { + super(message || `Unable to parse ${url}`, options); + } +} diff --git a/packages/cspell-config-lib/src/TextFile.ts b/packages/cspell-config-lib/src/TextFile.ts index 9c71388483b..55a03a50c96 100644 --- a/packages/cspell-config-lib/src/TextFile.ts +++ b/packages/cspell-config-lib/src/TextFile.ts @@ -6,3 +6,7 @@ export interface TextFile { url: URL; content: string; } + +export function createTextFile(url: URL, content: string): TextFile { + return { url, content }; +} diff --git a/packages/cspell-config-lib/src/loaders/loaderJavaScript.test.ts b/packages/cspell-config-lib/src/loaders/loaderJavaScript.test.ts index c0ff9707466..f37bbf06491 100644 --- a/packages/cspell-config-lib/src/loaders/loaderJavaScript.test.ts +++ b/packages/cspell-config-lib/src/loaders/loaderJavaScript.test.ts @@ -78,5 +78,5 @@ describe('loaderJavaScript', () => { }); function deserialize(params: { url: URL; content: string }): CSpellConfigFileJson { - return new CSpellConfigFileJson(params.url, JSON.parse(params.content), vi.fn()); + return new CSpellConfigFileJson(params.url, JSON.parse(params.content)); } diff --git a/packages/cspell-config-lib/src/serializers/cspellJson.test.ts b/packages/cspell-config-lib/src/serializers/cspellJson.test.ts index cf1e9125b8d..d3356f6bd14 100644 --- a/packages/cspell-config-lib/src/serializers/cspellJson.test.ts +++ b/packages/cspell-config-lib/src/serializers/cspellJson.test.ts @@ -34,7 +34,7 @@ describe('cspellJson', () => { ${''} | ${''} | ${'Unable to parse config file: "file:///"'} ${'cspell.js'} | ${''} | ${'Unable to parse config file: "file:///cspell.js"'} ${'cspell.yaml'} | ${''} | ${'Unable to parse config file: "file:///cspell.yaml"'} - ${'cspell.json'} | ${''} | ${'Unexpected end of JSON input'} + ${'cspell.json'} | ${''} | ${'Unable to parse file:///cspell.json'} ${'cspell.json'} | ${'[]'} | ${'Unable to parse file:///cspell.json'} `('fail $uri', ({ uri, content, expected }) => { expect(() => serializerCSpellJson.deserialize({ url: new URL(uri, 'file:///'), content }, next)).toThrow( diff --git a/packages/cspell-lib/api/api.d.ts b/packages/cspell-lib/api/api.d.ts index eb719013201..46881cd62ad 100644 --- a/packages/cspell-lib/api/api.d.ts +++ b/packages/cspell-lib/api/api.d.ts @@ -362,6 +362,7 @@ interface DictionaryFileDefinitionInternal extends Readonly; type CSpellSettingsI$1 = CSpellSettingsInternal; + declare function mergeSettings(left: CSpellSettingsWSTO | CSpellSettingsI$1, ...settings: (CSpellSettingsWSTO | CSpellSettingsI$1 | undefined)[]): CSpellSettingsI$1; declare function mergeInDocSettings(left: CSpellSettingsWSTO, right: CSpellSettingsWSTO): CSpellSettingsWST$1; /** @@ -452,6 +453,7 @@ interface IConfigLoader { * Unsubscribe from any events and dispose of any resources including caches. */ dispose(): void; + getStats(): Readonly>>>; } declare function loadPnP(pnpSettings: PnPSettingsOptional, searchFrom: URL): Promise; declare function createConfigLoader(cspellIO?: CSpellIO): IConfigLoader; diff --git a/packages/cspell-lib/src/lib/Settings/CSpellSettingsServer.test.ts b/packages/cspell-lib/src/lib/Settings/CSpellSettingsServer.test.ts index 585d4bab075..cddf1674a42 100644 --- a/packages/cspell-lib/src/lib/Settings/CSpellSettingsServer.test.ts +++ b/packages/cspell-lib/src/lib/Settings/CSpellSettingsServer.test.ts @@ -21,7 +21,7 @@ import { readSettingsFiles, } from './Controller/configLoader/index.js'; import type { CSpellSettingsWST } from './Controller/configLoader/types.js'; -import { getSources, mergeSettings } from './CSpellSettingsServer.js'; +import { getMergeStats, getSources, mergeSettings } from './CSpellSettingsServer.js'; import { _defaultSettings, getDefaultBundledSettingsAsync } from './DefaultSettings.js'; const samplesDir = pathPackageSamples; @@ -302,6 +302,11 @@ describe('Validate CSpellSettingsServer', () => { const sources = getSources(config); expect(sources.length).toBe(3); }); + + test('getMergeStats', () => { + const stats = getMergeStats(); + expect(stats).toHaveProperty('cacheMergeListUnique'); + }); }); describe('Validate Overrides', () => { diff --git a/packages/cspell-lib/src/lib/Settings/CSpellSettingsServer.ts b/packages/cspell-lib/src/lib/Settings/CSpellSettingsServer.ts index 25253e8fb24..43324da28fb 100644 --- a/packages/cspell-lib/src/lib/Settings/CSpellSettingsServer.ts +++ b/packages/cspell-lib/src/lib/Settings/CSpellSettingsServer.ts @@ -26,6 +26,8 @@ type CSpellSettingsWST = AdvancedCSpellSettingsWithSourceTrace; export type CSpellSettingsWSTO = OptionalOrUndefined; export type CSpellSettingsI = CSpellSettingsInternal; +export { stats as getMergeStats } from './mergeList.js'; + const emptyWords: string[] = []; Object.freeze(emptyWords); diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts index cb8c01b2f3a..2893f028beb 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.test.ts @@ -1,5 +1,5 @@ import type { CSpellSettingsWithSourceTrace, CSpellUserSettings, ImportFileRef } from '@cspell/cspell-types'; -import type { CSpellConfigFile } from 'cspell-config-lib'; +import { CSpellConfigFile, CSpellConfigFileInMemory } from 'cspell-config-lib'; import * as path from 'path'; import { fileURLToPath, pathToFileURL } from 'url'; import { assert, beforeEach, describe, expect, test, vi } from 'vitest'; @@ -624,6 +624,16 @@ describe('Validate search/load config files', () => { validateRawConfigVersion(cf('filename', config)); expect(mocked).toHaveBeenCalledWith(expected); }); + + test('create', () => { + const loader = getDefaultConfigLoader(); + const cfgFile = loader.createCSpellConfigFile(import.meta.url, {}); + + expect(cfgFile.url.href).toBe(import.meta.url); + expect(cfgFile).toBeInstanceOf(CSpellConfigFile); + expect(cfgFile).toBeInstanceOf(CSpellConfigFileInMemory); + expect(cfgFile.virtual).toBe(true); + }); }); describe('Validate Dependencies', () => { diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts index f8df896e0d1..570684cb09a 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts @@ -19,7 +19,7 @@ import { currentSettingsFileVersion, ENV_CSPELL_GLOB_ROOT, } from '../../constants.js'; -import { mergeSettings } from '../../CSpellSettingsServer.js'; +import { getMergeStats, mergeSettings } from '../../CSpellSettingsServer.js'; import { getGlobalConfig } from '../../GlobalSettings.js'; import { ImportError } from '../ImportError.js'; import type { LoaderResult } from '../pnpLoader.js'; @@ -135,6 +135,8 @@ export interface IConfigLoader { * Unsubscribe from any events and dispose of any resources including caches. */ dispose(): void; + + getStats(): Readonly>>>; } export class ConfigLoader implements IConfigLoader { @@ -419,7 +421,6 @@ export class ConfigLoader implements IConfigLoader { const fileRef = rawSettings.__importRef; const source = rawSettings.source; - assert(fileRef); assert(source); // Fix up dictionaryDefinitions @@ -455,7 +456,9 @@ export class ConfigLoader implements IConfigLoader { const finalizeSettings = mergeSettings(mergedImportedSettings, fileSettings); finalizeSettings.name = settings.name || finalizeSettings.name || ''; finalizeSettings.id = settings.id || finalizeSettings.id || ''; - finalizeSettings.__importRef = fileRef; + if (fileRef) { + finalizeSettings.__importRef = fileRef; + } return finalizeSettings; } @@ -472,6 +475,10 @@ export class ConfigLoader implements IConfigLoader { } } } + + getStats() { + return { ...getMergeStats() }; + } } class ConfigLoaderInternal extends ConfigLoader { diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configToRawSettings.test.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configToRawSettings.test.ts new file mode 100644 index 00000000000..da327324c89 --- /dev/null +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configToRawSettings.test.ts @@ -0,0 +1,18 @@ +import { CSpellConfigFileJavaScript } from 'cspell-config-lib'; +import { fileURLToPath } from 'url'; +import { describe, expect, test } from 'vitest'; + +import { configToRawSettings } from './configToRawSettings.js'; + +describe('configToRawSettings', () => { + test('should return empty settings when config file is undefined', () => { + const result = configToRawSettings(undefined); + expect(result).toEqual({}); + }); + + test('should convert CSpellConfigFile to CSpellSettingsWST', () => { + const cfgFile = new CSpellConfigFileJavaScript(new URL(import.meta.url), {}); + const result = configToRawSettings(cfgFile); + expect(result.__importRef).toEqual(expect.objectContaining({ filename: fileURLToPath(import.meta.url) })); + }); +}); diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configToRawSettings.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configToRawSettings.ts index a19cba5b448..d2e850487c9 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configToRawSettings.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configToRawSettings.ts @@ -23,14 +23,17 @@ export function configToRawSettings(cfgFile: CSpellConfigFile | undefined): CSpe const source: Source = { name: cfgFile.settings.name || filename, - filename, + filename: cfgFile.virtual ? undefined : filename, }; const rawSettings: CSpellSettingsWST = { ...cfgFile.settings }; rawSettings.import = normalizeImport(rawSettings.import); normalizeRawConfig(rawSettings); rawSettings.source = source; - rawSettings.__importRef = fileRef; + // in virtual config files are ignored for the purposes of import history. + if (!cfgFile.virtual) { + rawSettings.__importRef = fileRef; + } const id = rawSettings.id || urlToSimpleId(url); const name = rawSettings.name || id; diff --git a/packages/cspell-lib/src/lib/Settings/GlobalSettings.test.ts b/packages/cspell-lib/src/lib/Settings/GlobalSettings.test.ts index 6d2cac1fb30..93cc465fc36 100644 --- a/packages/cspell-lib/src/lib/Settings/GlobalSettings.test.ts +++ b/packages/cspell-lib/src/lib/Settings/GlobalSettings.test.ts @@ -1,10 +1,16 @@ +import { CSpellConfigFileInMemory, CSpellConfigFileJson } from 'cspell-config-lib'; import path from 'path'; import type { Mock } from 'vitest'; import { afterEach, describe, expect, test, vi } from 'vitest'; import { getLogger } from '../util/logger.js'; import { ConfigStore } from './cfgStore.js'; -import { getGlobalConfigPath, getRawGlobalSettings, writeRawGlobalSettings } from './GlobalSettings.js'; +import { + getGlobalConfig, + getGlobalConfigPath, + getRawGlobalSettings, + writeRawGlobalSettings, +} from './GlobalSettings.js'; interface MockConfigStore extends ConfigStore { (name: string): MockConfigStore; @@ -136,6 +142,21 @@ describe('Validate GlobalSettings', () => { }); }); + test('writeRawGlobalSettings then read', async () => { + const mockImpl = createMockConfigStore(); + mockConfigstore.mockImplementation(mockImpl); + + const cfBefore = await getGlobalConfig(); + + expect(cfBefore).toBeInstanceOf(CSpellConfigFileInMemory); + + const updated = { import: ['hello'], extra: { name: 'extra', value: 'ok' } }; + await writeRawGlobalSettings(updated); + + const cfAfter = await getGlobalConfig(); + expect(cfAfter).toBeInstanceOf(CSpellConfigFileJson); + }); + test('writeRawGlobalSettings with Error', async () => { const mockImpl = createMockConfigStore(); mockConfigstore.mockImplementation(mockImpl); diff --git a/packages/cspell-lib/src/lib/Settings/GlobalSettings.ts b/packages/cspell-lib/src/lib/Settings/GlobalSettings.ts index cc46f1fceef..1da9f99dd93 100644 --- a/packages/cspell-lib/src/lib/Settings/GlobalSettings.ts +++ b/packages/cspell-lib/src/lib/Settings/GlobalSettings.ts @@ -1,6 +1,6 @@ import type { CSpellSettings, CSpellSettingsWithSourceTrace } from '@cspell/cspell-types'; import type { CSpellConfigFile } from 'cspell-config-lib'; -import { CSpellConfigFileInMemory } from 'cspell-config-lib'; +import { CSpellConfigFileInMemory, CSpellConfigFileJson } from 'cspell-config-lib'; import { pathToFileURL } from 'url'; import { isErrnoException } from '../util/errors.js'; @@ -34,12 +34,13 @@ export function getGlobalConfig(): Promise { const globalConf: GlobalSettingsWithSource = { source }; + let hasGlobalConfig = false; + try { - // console.warn('%o', ConfigStore); const cfgStore = new ConfigStore(packageName); - // console.warn('%o', cfgStore); const cfg = cfgStore.all; + // Only populate globalConf is there are values. if (cfg && Object.keys(cfg).length) { Object.assign(globalConf, cfg); @@ -47,6 +48,7 @@ export function getGlobalConfig(): Promise { name, filename: cfgStore.path, }; + hasGlobalConfig = Object.keys(cfg).length > 0; } } catch (error) { if ( @@ -60,7 +62,9 @@ export function getGlobalConfig(): Promise { const settings: CSpellSettingsWST = { ...globalConf, name, source }; - return Promise.resolve(new CSpellConfigFileInMemory(urlGlobal, settings)); + const ConfigFile = hasGlobalConfig ? CSpellConfigFileJson : CSpellConfigFileInMemory; + + return Promise.resolve(new ConfigFile(urlGlobal, settings)); } export async function writeRawGlobalSettings(settings: GlobalCSpellSettings): Promise { diff --git a/packages/cspell-lib/src/lib/Settings/mergeCache.test.ts b/packages/cspell-lib/src/lib/Settings/mergeCache.test.ts new file mode 100644 index 00000000000..4534d1e9db0 --- /dev/null +++ b/packages/cspell-lib/src/lib/Settings/mergeCache.test.ts @@ -0,0 +1,163 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +import { dispatchClearCache } from '../events/events.js'; +import { CalcLeftRightResultWeakCache } from './mergeCache.js'; + +const oc = expect.objectContaining; + +describe('CalcLeftRightResultWeakCache', () => { + let cache: CalcLeftRightResultWeakCache; + + beforeEach(() => { + cache = new CalcLeftRightResultWeakCache(); + }); + + afterEach(() => { + cache.dispose(); + }); + + test('should return the cached result if available', () => { + const left = {}; + const right = {}; + const expectedResult = 42; + + cache.get(left, right, () => expectedResult); + + const result = cache.get(left, right, () => { + throw new Error('This callback should not be called'); + }); + + expect(result).toBe(expectedResult); + }); + + test('should calculate and cache the result if not available', () => { + const left = {}; + const right = {}; + const expectedResult = 42; + + const result = cache.get(left, right, () => expectedResult); + + expect(result).toBe(expectedResult); + }); + + test('should clear the cache', () => { + const left = {}; + const right = {}; + const expectedResult = 42; + + const mockResolve = vi.fn(() => expectedResult); + + const result1 = cache.get(left, right, mockResolve); + expect(result1).toBe(expectedResult); + expect(mockResolve).toHaveBeenCalledTimes(1); + + const result2 = cache.get(left, right, mockResolve); + expect(result2).toBe(expectedResult); + expect(mockResolve).toHaveBeenCalledTimes(1); + + cache.clear(); + + const result3 = cache.get(left, right, mockResolve); + expect(result3).toBe(expectedResult); + expect(mockResolve).toHaveBeenCalledTimes(2); + + expect(result3).toBe(expectedResult); + }); + + test('should dispose the cache', () => { + const left = {}; + const right = {}; + const expectedResult = 42; + + const mockResolve = vi.fn(() => expectedResult); + + const result1 = cache.get(left, right, mockResolve); + expect(result1).toBe(expectedResult); + expect(mockResolve).toHaveBeenCalledTimes(1); + + const result2 = cache.get(left, right, mockResolve); + expect(result2).toBe(expectedResult); + expect(mockResolve).toHaveBeenCalledTimes(1); + + expect(cache.stats()).toEqual(oc({ hits: 1, misses: 1, deletes: 0, resolved: 1, sets: 0 })); + + dispatchClearCache(); + + expect(cache.stats()).toEqual({ + clears: 1, + deletes: 0, + disposals: 0, + hits: 0, + misses: 0, + resolved: 0, + sets: 0, + }); + + const result3 = cache.get(left, right, mockResolve); + expect(result3).toBe(expectedResult); + expect(mockResolve).toHaveBeenCalledTimes(2); + + expect(cache.stats()).toEqual({ + clears: 1, + deletes: 0, + disposals: 0, + hits: 0, + misses: 1, + resolved: 1, + sets: 0, + }); + + cache.dispose(); + + expect(cache.stats()).toEqual({ + clears: 2, + deletes: 0, + disposals: 1, + hits: 0, + misses: 0, + resolved: 0, + sets: 0, + }); + + const result4 = cache.get(left, right, mockResolve); + expect(result4).toBe(expectedResult); + expect(mockResolve).toHaveBeenCalledTimes(3); + + expect(cache.stats()).toEqual({ + clears: 2, + deletes: 0, + disposals: 1, + hits: 0, + misses: 1, + resolved: 1, + sets: 0, + }); + + const result5 = cache.get(left, right, mockResolve); + expect(result5).toBe(expectedResult); + expect(mockResolve).toHaveBeenCalledTimes(3); + + expect(cache.stats()).toEqual({ + clears: 2, + deletes: 0, + disposals: 1, + hits: 1, + misses: 1, + resolved: 1, + sets: 0, + }); + + // dispatchClearCache will be ignored because the cache has been disposed + dispatchClearCache(); + + expect(cache.stats()).toEqual({ + clears: 2, + deletes: 0, + disposals: 1, + hits: 1, + misses: 1, + resolved: 1, + sets: 0, + }); + }); +}); diff --git a/packages/cspell-lib/src/lib/Settings/mergeCache.ts b/packages/cspell-lib/src/lib/Settings/mergeCache.ts index cfc5974a282..98c71b9943e 100644 --- a/packages/cspell-lib/src/lib/Settings/mergeCache.ts +++ b/packages/cspell-lib/src/lib/Settings/mergeCache.ts @@ -8,7 +8,7 @@ interface IDisposable { export class CalcLeftRightResultWeakCache implements IDisposable { private map = new AutoResolveWeakCache>(); - private _toDispose: IDisposable; + private _toDispose: IDisposable | undefined; constructor() { this._toDispose = onClearCache(() => { @@ -26,6 +26,12 @@ export class CalcLeftRightResultWeakCache(left: T[] | undefined, right: T[] | undefined): T[] Object.freeze(result); return result; } + +export function stats() { + return { + cacheMergeListUnique: cacheMergeListUnique.stats(), + cacheMergeLists: cacheMergeLists.stats(), + }; +} diff --git a/packages/cspell-lib/src/lib/perf/perf.test.ts b/packages/cspell-lib/src/lib/perf/perf.test.ts new file mode 100644 index 00000000000..4138853bf0a --- /dev/null +++ b/packages/cspell-lib/src/lib/perf/perf.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, test } from 'vitest'; + +import { perf, PerfMonitor } from './perf.js'; + +describe('PerfMonitor', () => { + test('perf', () => { + expect(perf).toBeInstanceOf(PerfMonitor); + expect(perf.enabled).toBe(false); + }); + + test('should enable and disable performance monitoring', () => { + const perf = new PerfMonitor(); + + expect(perf.enabled).toBe(false); + + perf.enabled = true; + expect(perf.enabled).toBe(true); + + perf.enabled = false; + expect(perf.enabled).toBe(false); + }); + + test('should mark and measure performance', () => { + perf.enabled = true; + + const markStart = perf.mark('start'); + expect(markStart.name).toBe('start'); + // Perform some operations... + + const markEnd = perf.mark('end'); + expect(markEnd.name).toBe('end'); + + expect(markEnd.startTime).toBeGreaterThan(markStart.startTime); + }); +}); diff --git a/packages/cspell-lib/src/lib/perf/perf.ts b/packages/cspell-lib/src/lib/perf/perf.ts index 9b92ff8b027..a7fe6d3428e 100644 --- a/packages/cspell-lib/src/lib/perf/perf.ts +++ b/packages/cspell-lib/src/lib/perf/perf.ts @@ -1,23 +1,41 @@ -class PerfMonitor { +import assert from 'assert'; + +interface PerfEntry { + readonly entryType: string; + readonly name: string; + readonly startTime: number; + readonly duration?: number | undefined; +} + +interface PerfMark extends PerfEntry { + readonly entryType: 'mark'; +} + +export class PerfMonitor { private _enabled = false; constructor(private _performance: Performance = performance) {} - mark(name: string) { - this._enabled && this._performance.mark(name); + mark(name: string): PerfMark { + const mark = ((this._enabled && this._performance.mark(name)) || { + entryType: 'mark' as const, + name, + startTime: performance.now(), + }) as PerfMark; + return mark; } - measure(name: string, startMark: string, endMark: string) { - this._enabled && this._performance.measure(name, startMark, endMark); - } + // measure(name: string, startMark: string, endMark: string) { + // return this._enabled && this._performance.measure(name, startMark, endMark); + // } - clearMarks(name?: string) { - this._enabled && this._performance.clearMarks(name); - } + // clearMarks(name?: string) { + // this._enabled && this._performance.clearMarks(name); + // } - clearMeasures(name?: string) { - this._enabled && this._performance.clearMeasures(name); - } + // clearMeasures(name?: string) { + // this._enabled && this._performance.clearMeasures(name); + // } get enabled() { return this._enabled; diff --git a/packages/cspell-lib/src/lib/util/AutoResolve.test.ts b/packages/cspell-lib/src/lib/util/AutoResolve.test.ts index 3c1ad1bcf14..3e193a95a58 100644 --- a/packages/cspell-lib/src/lib/util/AutoResolve.test.ts +++ b/packages/cspell-lib/src/lib/util/AutoResolve.test.ts @@ -41,5 +41,69 @@ describe('AutoResolve', () => { expect(cache.get(tagHello2)).toBe(undefined); expect(cache.get(tagHello2, resolver)).toBe('HELLO'); expect(resolver).toHaveBeenCalledTimes(3); + expect(cache.stats()).toEqual({ + hits: 2, + misses: 5, + deletes: 0, + resolved: 3, + sets: 1, + disposals: 0, + clears: 0, + }); + expect(cache.get(tagHello2, resolver)).toBe('HELLO'); + expect(cache.stats()).toEqual({ + hits: 3, + misses: 5, + deletes: 0, + resolved: 3, + sets: 1, + disposals: 0, + clears: 0, + }); + cache.delete(tagHello); + expect(cache.stats()).toEqual({ + hits: 3, + misses: 5, + deletes: 1, + resolved: 3, + sets: 1, + disposals: 0, + clears: 0, + }); + expect(cache.get(tagHello, resolver)).toBe('HELLO'); + expect(cache.stats()).toEqual({ + hits: 3, + misses: 6, + deletes: 1, + resolved: 4, + sets: 1, + disposals: 0, + clears: 0, + }); + const weakMap = cache.map; + cache.clear(); + const weakMap2 = cache.map; + expect(weakMap2).toBe(cache.map); + expect(weakMap2).not.toBe(weakMap); + expect(cache.stats()).toEqual({ + hits: 0, + misses: 0, + deletes: 0, + resolved: 0, + sets: 0, + disposals: 0, + clears: 1, + }); + cache.dispose(); + expect(cache.map).not.toBe(weakMap2); + expect(cache.stats()).toEqual({ + hits: 0, + misses: 0, + deletes: 0, + resolved: 0, + sets: 0, + disposals: 1, + clears: 2, + }); }); }); diff --git a/packages/cspell-lib/src/lib/util/AutoResolve.ts b/packages/cspell-lib/src/lib/util/AutoResolve.ts index e355799781c..0eadf16df51 100644 --- a/packages/cspell-lib/src/lib/util/AutoResolve.ts +++ b/packages/cspell-lib/src/lib/util/AutoResolve.ts @@ -10,6 +10,49 @@ export function autoResolve(map: Map, key: K, resolve: (k: K) => V): return value; } +export interface CacheStats { + hits: number; + misses: number; + resolved: number; + deletes: number; + sets: number; + clears: number; + disposals: number; +} + +export type AutoResolveCacheStats = Readonly; + +class CacheStatsTracker implements CacheStats { + hits: number = 0; + misses: number = 0; + resolved: number = 0; + deletes: number = 0; + sets: number = 0; + clears: number = 0; + disposals: number = 0; + + stats(): AutoResolveCacheStats { + return { + hits: this.hits, + misses: this.misses, + resolved: this.resolved, + deletes: this.deletes, + sets: this.sets, + clears: this.clears, + disposals: this.disposals, + }; + } + + clear(): void { + this.hits = 0; + this.misses = 0; + this.resolved = 0; + this.deletes = 0; + this.sets = 0; + ++this.clears; + } +} + export class AutoResolveCache implements IDisposable { readonly map = new Map(); @@ -64,11 +107,26 @@ export function autoResolveWeak(map: IWeakMap, key: K export class AutoResolveWeakCache implements IWeakMap { private _map = new WeakMap(); + private _stats = new CacheStatsTracker(); + get(k: K): V | undefined; get(k: K, resolve: (k: K) => V): V; get(k: K, resolve?: (k: K) => V): V | undefined; get(k: K, resolve?: (k: K) => V): V | undefined { - return resolve ? autoResolveWeak(this._map, k, resolve) : this._map.get(k); + const map = this._map; + const found = map.get(k); + if (found !== undefined || map.has(k)) { + ++this._stats.hits; + return found as V; + } + ++this._stats.misses; + if (!resolve) { + return undefined; + } + ++this._stats.resolved; + const value = resolve(k); + map.set(k, value); + return value; } get map() { @@ -80,21 +138,29 @@ export class AutoResolveWeakCache implements IWeakMap } set(k: K, v: V): this { + ++this._stats.sets; this._map.set(k, v); return this; } clear(): void { + this._stats.clear(); this._map = new WeakMap(); } delete(k: K): boolean { + ++this._stats.deletes; return this._map.delete(k); } dispose(): void { + ++this._stats.disposals; this.clear(); } + + stats(): AutoResolveCacheStats { + return this._stats.stats(); + } } export function createAutoResolveWeakCache(): AutoResolveWeakCache { diff --git a/packages/cspell/src/app/lint/lint.ts b/packages/cspell/src/app/lint/lint.ts index 19dc1721545..bed31a8a00e 100644 --- a/packages/cspell/src/app/lint/lint.ts +++ b/packages/cspell/src/app/lint/lint.ts @@ -18,6 +18,7 @@ import { ENV_CSPELL_GLOB_ROOT, extractDependencies, extractImportErrors, + getDefaultConfigLoader, getDictionary, isBinaryFile as cspellIsBinaryFile, setLogger, @@ -64,6 +65,8 @@ const version = npmPackage.version; const BATCH_SIZE = 8; +const debugStats = false; + const { opFilterAsync } = operators; export async function runLint(cfg: LintRequest): Promise { @@ -445,7 +448,9 @@ export async function runLint(cfg: LintRequest): Promise { const cacheSettings = await calcCacheSettings(configInfo.config, { ...cfg.options, version }, root); const files = await determineFilesToCheck(configInfo, cfg, reporter, globInfo); - return await processFiles(files, configInfo, cacheSettings); + const result = await processFiles(files, configInfo, cacheSettings); + debugStats && console.error('stats: %o', getDefaultConfigLoader().getStats()); + return result; } catch (e) { const err = toApplicationError(e); reporter.error('Linter', err); From 9f1f655e5fb4ce9df94281c37de43685b3208e41 Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Thu, 7 Dec 2023 20:35:43 +0100 Subject: [PATCH 2/2] Update perf.ts --- packages/cspell-lib/src/lib/perf/perf.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/cspell-lib/src/lib/perf/perf.ts b/packages/cspell-lib/src/lib/perf/perf.ts index a7fe6d3428e..7fe76d4b0da 100644 --- a/packages/cspell-lib/src/lib/perf/perf.ts +++ b/packages/cspell-lib/src/lib/perf/perf.ts @@ -1,5 +1,3 @@ -import assert from 'assert'; - interface PerfEntry { readonly entryType: string; readonly name: string;