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: Make sure virtual configs do not appear as source files #5048

Merged
merged 2 commits into from
Dec 7, 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
27 changes: 26 additions & 1 deletion packages/cspell-config-lib/src/CSpellConfigFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export class CSpellConfigFileInMemory extends ImplCSpellConfigFile {
super(url, settings);
}

get readonly(): boolean {
get virtual(): boolean {
return true;
}
}
Original file line number Diff line number Diff line change
@@ -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.');
});
});
Original file line number Diff line number Diff line change
@@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
4 changes: 4 additions & 0 deletions packages/cspell-config-lib/src/TextFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ export interface TextFile {
url: URL;
content: string;
}

export function createTextFile(url: URL, content: string): TextFile {
return { url, content };
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions packages/cspell-lib/api/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/**
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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', () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/cspell-lib/src/lib/Settings/CSpellSettingsServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -419,7 +421,6 @@ export class ConfigLoader implements IConfigLoader {
const fileRef = rawSettings.__importRef;
const source = rawSettings.source;

assert(fileRef);
assert(source);

// Fix up dictionaryDefinitions
Expand Down Expand Up @@ -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;
}

Expand All @@ -472,6 +475,10 @@ export class ConfigLoader implements IConfigLoader {
}
}
}

getStats() {
return { ...getMergeStats() };
}
}

class ConfigLoaderInternal extends ConfigLoader {
Expand Down
Loading