fix: Make sure virtual configs do not appear as source files (#5048)

streetsidesoftware · Dec 7, 2023 · f1798e6 · f1798e6
1 parent 02f56db
commit f1798e6
Show file tree

Hide file tree

Showing 26 changed files with 636 additions and 47 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -362,6 +362,7 @@ interface DictionaryFileDefinitionInternal extends Readonly<DictionaryDefinition
 type CSpellSettingsWST$1 = AdvancedCSpellSettingsWithSourceTrace;
 type CSpellSettingsWSTO = OptionalOrUndefined<AdvancedCSpellSettingsWithSourceTrace>;
 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<Record<string, Readonly<Record<string, number>>>>;
 }
 declare function loadPnP(pnpSettings: PnPSettingsOptional, searchFrom: URL): Promise<LoaderResult>;
 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
@@ -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
@@ -26,6 +26,8 @@ type CSpellSettingsWST = AdvancedCSpellSettingsWithSourceTrace;
 export type CSpellSettingsWSTO = OptionalOrUndefined<AdvancedCSpellSettingsWithSourceTrace>;
 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
@@ -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
@@ -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<Record<string, Readonly<Record<string, number>>>>;
 }
 
 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 {