Skip to content

fix: Ensure languageOptions.customSyntax is serializable #212

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
75 changes: 74 additions & 1 deletion src/languages/css-language.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,34 @@ const blockCloserTokenTypes = new Map([
[tokenTypes.RightSquareBracket, "["],
]);

/**
* Recursively replaces all function values in an object with boolean true.
* Used to make objects serializable for JSON output.
*
* @param {Record<string,any>} object The object to process.
* @returns {Record<string,any>} A copy of the object with all functions replaced by true.
Comment on lines +58 to +59
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this function also returns arrays and non-object values depending on the type of the argument?

*/
function replaceFunctions(object) {
if (typeof object !== "object" || object === null) {
return object;
}
if (Array.isArray(object)) {
return object.map(replaceFunctions);
}
const result = {};
for (const key of Object.keys(object)) {
const value = object[key];
if (typeof value === "function") {
result[key] = true;
} else if (typeof value === "object" && value !== null) {
result[key] = replaceFunctions(value);
} else {
result[key] = value;
}
}
return result;
}

//-----------------------------------------------------------------------------
// Exports
//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -101,7 +129,8 @@ export class CSSLanguage {
/**
* Validates the language options.
* @param {CSSLanguageOptions} languageOptions The language options to validate.
* @throws {Error} When the language options are invalid.
* @returns {void}
* @throws {TypeError} When the language options are invalid.
*/
validateLanguageOptions(languageOptions) {
if (
Expand All @@ -125,6 +154,50 @@ export class CSSLanguage {
}
}

/**
* Normalizes the language options so they can be serialized.
* @param {CSSLanguageOptions} languageOptions The language options to normalize.
* @returns {CSSLanguageOptions} The normalized language options.
*/
normalizeLanguageOptions(languageOptions) {
Copy link
Preview

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The method modifies the input object by adding a toJSON property. Consider returning a new object instead of mutating the input to avoid side effects and make the API more predictable.

Copilot uses AI. Check for mistakes.

// if there's no custom syntax then no changes are necessary
if (!languageOptions?.customSyntax) {
return languageOptions;
}

Object.defineProperty(languageOptions, "toJSON", {
value() {
// Shallow copy
const result = { ...languageOptions };
Comment on lines +168 to +171
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to set the toJSON method on a shallow clone of the argument (results) so that the original object isn't modified?

Suggested change
Object.defineProperty(languageOptions, "toJSON", {
value() {
// Shallow copy
const result = { ...languageOptions };
// Shallow copy
const result = { ...languageOptions };
Object.defineProperty(result, "toJSON", {
value() {

We'd need to change the return value of the function as well and update some unit tests.

result.customSyntax = { ...result.customSyntax };

if (result.customSyntax.node) {
result.customSyntax.node = replaceFunctions(
result.customSyntax.node,
);
}

if (result.customSyntax.scope) {
result.customSyntax.scope = replaceFunctions(
result.customSyntax.scope,
);
}

if (result.customSyntax.atrule) {
result.customSyntax.atrule = replaceFunctions(
result.customSyntax.atrule,
);
}

return result;
},
enumerable: false,
configurable: true,
});

return languageOptions;
}

/**
* Parses the given file into an AST.
* @param {File} file The virtual file to parse.
Expand Down
60 changes: 60 additions & 0 deletions tests/languages/css-language.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,4 +261,64 @@ describe("CSSLanguage", () => {
assert.strictEqual(sourceCode.comments.length, 1);
});
});

describe("normalizeLanguageOptions", () => {
it("should return the same object if no customSyntax is present", () => {
const language = new CSSLanguage();
const options = { tolerant: true };
const result = language.normalizeLanguageOptions(options);
assert.strictEqual(result, options);
assert.strictEqual(typeof result.toJSON, "undefined");
});

it("should add a toJSON method if customSyntax is present", () => {
const language = new CSSLanguage();
const options = { tolerant: true, customSyntax: { foo: "bar" } };
const result = language.normalizeLanguageOptions(options);
assert.strictEqual(result, options);
assert.strictEqual(typeof result.toJSON, "function");
});

it("should replace functions with true in toJSON output", () => {
const language = new CSSLanguage();
const options = {
tolerant: false,
customSyntax: {
node: {
foo() {},
bar: 42,
baz: {
qux() {},
},
},
scope: {
test() {},
},
atrule: {
other() {},
},
},
};
language.normalizeLanguageOptions(options);
const json = options.toJSON();
assert.deepStrictEqual(json, {
tolerant: false,
customSyntax: {
node: {
foo: true,
bar: 42,
baz: {
qux: true,
},
},
scope: {
test: true,
},
atrule: {
other: true,
},
},
});
});
});
});
83 changes: 83 additions & 0 deletions tests/plugin/eslint.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,89 @@ describe("Plugin", () => {
assert.strictEqual(results[0].messages[1].column, 18);
});
});

describe("serialization", () => {
it("should serialize the config to JSON", async () => {
const languageOptions = {
tolerant: true,
};

const configWithLanguageOptions = {
...config,
languageOptions,
};

const eslint = new ESLint({
overrideConfigFile: true,
overrideConfig: configWithLanguageOptions,
});

const resultConfig =
await eslint.calculateConfigForFile("test.css");
const serializedConfig = JSON.stringify(
resultConfig.languageOptions,
null,
2,
);
const expectedConfig = JSON.stringify(languageOptions, null, 2);
assert.strictEqual(serializedConfig, expectedConfig);
});

it("should serialize the config to JSON when it has functions", async () => {
const languageOptions = {
tolerant: true,
customSyntax: {
node: {
CustomNode: {
parse() {},
},
},
scope: {
Value: {
theme() {},
},
},
atrule: {
CustomAtRule: {
parse: {
prelude() {},
},
},
},
},
};

const configWithLanguageOptions = {
...config,
languageOptions,
};

const eslint = new ESLint({
overrideConfigFile: true,
overrideConfig: configWithLanguageOptions,
});

const resultConfig =
await eslint.calculateConfigForFile("test.css");
const serializedConfig = JSON.stringify(
resultConfig.languageOptions,
null,
2,
);
const expectedConfig = JSON.stringify(
languageOptions,
(key, value) => {
if (typeof value === "function") {
return true;
}

return value;
},
2,
);
assert.strictEqual(serializedConfig, expectedConfig);
});
});
});

describe("Configuration Comments", () => {
Expand Down
Loading