diff --git a/src/languages/css-source-code.js b/src/languages/css-source-code.js index ec98a51b..d84c03f1 100644 --- a/src/languages/css-source-code.js +++ b/src/languages/css-source-code.js @@ -20,12 +20,19 @@ import { visitorKeys } from "./css-visitor-keys.js"; //----------------------------------------------------------------------------- /** - * @import { CssNode, CssNodePlain, CssLocationRange, Comment, Lexer, StyleSheetPlain } from "@eslint/css-tree" + * @import { CssNode, CssNodePlain, CssLocationRange, Comment, Lexer, StyleSheetPlain, DeclarationPlain, AtrulePlain, FunctionNodePlain, ValuePlain, Raw } from "@eslint/css-tree" * @import { SourceRange, FileProblem, DirectiveType, RulesConfig } from "@eslint/core" * @import { CSSSyntaxElement } from "../types.js" * @import { CSSLanguageOptions } from "./css-language.js" */ +/** + * @typedef {Object} CustomPropertyUses + * @property {Array} declarations Declaration nodes where the custom property value is declared. + * @property {Array} definitions Atrule nodes where the custom property is defined using `@property`. + * @property {Array} references Function nodes (`var()`) where the custom property is used. + */ + //----------------------------------------------------------------------------- // Helpers //----------------------------------------------------------------------------- @@ -86,6 +93,18 @@ export class CSSSourceCode extends TextSourceCodeBase { */ #inlineConfigComments; + /** + * Map of custom property names to their uses. + * @type {Map|undefined} + */ + #customProperties; + + /** + * Map of declarations to the var() functions they contain. + * @type {WeakMap>} + */ + #declarationVariables = new WeakMap(); + /** * The AST of the source code. * @type {StyleSheetPlain} @@ -254,6 +273,22 @@ export class CSSSourceCode extends TextSourceCodeBase { return this.#parents.get(node); } + /** + * Ensures the custom properties map entry exists for a given name. + * @param {string} name The custom property name. + * @returns {CustomPropertyUses} The uses object. + */ + #ensureCustomProperty(name) { + if (!this.#customProperties.has(name)) { + this.#customProperties.set(name, { + declarations: [], + definitions: [], + references: [], + }); + } + return this.#customProperties.get(name); + } + /** * Traverse the source code and return the steps that were taken. * @returns {Iterable} The steps that were taken while traversing the source code. @@ -266,13 +301,70 @@ export class CSSSourceCode extends TextSourceCodeBase { /** @type {Array} */ const steps = (this.#steps = []); + this.#customProperties = new Map(); // Note: We can't use `walk` from `css-tree` because it uses `CssNode` instead of `CssNodePlain` + /** + * Stack of declaration nodes currently being visited. + * Used to track which var() functions belong to which declaration. + * @type {Array} + */ + const declStack = []; + const visit = (node, parent) => { // first set the parent this.#parents.set(node, parent); + // Track custom property declarations + if (node.type === "Declaration" && node.property.startsWith("--")) { + this.#ensureCustomProperty(node.property).declarations.push( + node, + ); + } + + // Track @property definitions + if (node.type === "Atrule" && node.name === "property") { + const identNode = node.prelude?.children?.[0]; + if ( + identNode?.type === "Identifier" && + identNode.name.startsWith("--") + ) { + this.#ensureCustomProperty(identNode.name).definitions.push( + node, + ); + } + } + + // Track var() references + if (node.type === "Function" && node.name.toLowerCase() === "var") { + const identNode = node.children?.[0]; + if ( + identNode?.type === "Identifier" && + identNode.name.startsWith("--") + ) { + this.#ensureCustomProperty(identNode.name).references.push( + node, + ); + } + + // Associate this var() with the current declaration + if (declStack.length > 0) { + const currentDecl = declStack.at(-1); + const vars = this.#declarationVariables.get(currentDecl); + if (vars) { + vars.push(node); + } + } + } + + // Track declaration context for getDeclarationVariables + const isDeclaration = node.type === "Declaration"; + if (isDeclaration) { + declStack.push(node); + this.#declarationVariables.set(node, []); + } + // then add the step steps.push( new CSSTraversalStep({ @@ -297,6 +389,11 @@ export class CSSSourceCode extends TextSourceCodeBase { } } + // Pop declaration context + if (isDeclaration) { + declStack.pop(); + } + // then add the exit step steps.push( new CSSTraversalStep({ @@ -311,4 +408,223 @@ export class CSSSourceCode extends TextSourceCodeBase { return steps; } + + /** + * Returns an array of `var()` function nodes used in the given declaration's value. + * @param {DeclarationPlain} declaration The declaration node to inspect. + * @returns {Array} The `var()` function nodes found in the declaration. + */ + getDeclarationVariables(declaration) { + // Ensure traversal has happened + if (!this.#customProperties) { + this.traverse(); + } + + return this.#declarationVariables.get(declaration) ?? []; + } + + /** + * Returns the closest computed value for a `var()` function node or a + * custom property name. + * + * When called with a `var()` function node, the resolution order is: + * 1. If the current rule block has one or more custom property declarations + * for the variable, return the value of the last one. + * 2. If the `var()` has a fallback value, return the fallback. + * 3. If a previous rule had a custom property declaration, return the last value. + * 4. If there's a `@property` with an `initial-value`, return the initial value. + * 5. Otherwise, return `undefined`. + * + * When called with a custom property name string, returns the last declared + * value for that property, or the `@property` initial-value if no + * declarations exist. + * @param {FunctionNodePlain|string} funcOrName The `var()` function node or custom property name. + * @returns {ValuePlain|Raw|undefined} The closest value node, or `undefined`. + */ + getClosestVariableValue(funcOrName) { + // Ensure traversal has happened + if (!this.#customProperties) { + this.traverse(); + } + + // When called with a string name, return the last declaration value + if (typeof funcOrName === "string") { + const uses = this.#customProperties.get(funcOrName); + + if (uses && uses.declarations.length > 0) { + return uses.declarations.at(-1).value; + } + + // Fall back to @property initial-value + if (uses) { + for (const definition of uses.definitions) { + const block = definition.block; + if (block?.children) { + for (const child of block.children) { + if ( + child.type === "Declaration" && + child.property === "initial-value" + ) { + return child.value; + } + } + } + } + } + + return undefined; + } + + const func = funcOrName; + const identNode = func.children?.[0]; + if (!identNode || identNode.type !== "Identifier") { + return undefined; + } + + const varName = identNode.name; + const uses = this.#customProperties.get(varName); + + // Find the enclosing Rule node for this var() function + let ruleBlock = null; + let ancestor = this.#parents.get(func); + while (ancestor) { + if (ancestor.type === "Rule") { + ruleBlock = ancestor; + break; + } + ancestor = this.#parents.get(ancestor); + } + + // Step 1: Check current rule block for declarations + if (ruleBlock && uses) { + const blockDeclarations = uses.declarations.filter(decl => { + let parent = this.#parents.get(decl); + while (parent) { + if (parent === ruleBlock) { + return true; + } + if (parent.type === "Rule") { + return false; + } + parent = this.#parents.get(parent); + } + return false; + }); + + if (blockDeclarations.length > 0) { + return blockDeclarations.at(-1).value; + } + } + + // Step 2: Check fallback value + if (func.children.length >= 3) { + const fallback = func.children[2]; + if (fallback) { + return /** @type {Raw} */ (fallback); + } + } + + // Step 3: Check declarations in previous rules + if (uses) { + const funcOffset = func.loc?.start?.offset ?? Infinity; + const previousDeclarations = uses.declarations.filter(decl => { + // Must not be in the same rule block (already checked) + let parent = this.#parents.get(decl); + while (parent) { + if (parent === ruleBlock) { + return false; + } + if (parent.type === "Rule") { + break; + } + parent = this.#parents.get(parent); + } + return (decl.loc?.start?.offset ?? 0) < funcOffset; + }); + + if (previousDeclarations.length > 0) { + return previousDeclarations.at(-1).value; + } + } + + // Step 4: Check @property initial-value + if (uses) { + for (const definition of uses.definitions) { + const block = definition.block; + if (block?.children) { + for (const child of block.children) { + if ( + child.type === "Declaration" && + child.property === "initial-value" + ) { + return child.value; + } + } + } + } + } + + // Step 5: Return undefined + return undefined; + } + + /** + * Returns all possible values for a `var()` function node. + * + * The returned array is composed of: + * 1. If there's a `@property` with an `initial-value`, that value comes first. + * 2. The values from custom property declarations throughout the file, in source order. + * 3. The fallback value (if present) comes last. + * @param {FunctionNodePlain} func The `var()` function node. + * @returns {Array} Array of value nodes. + */ + getVariableValues(func) { + // Ensure traversal has happened + if (!this.#customProperties) { + this.traverse(); + } + + const identNode = func.children?.[0]; + if (!identNode || identNode.type !== "Identifier") { + return []; + } + + const varName = identNode.name; + const uses = this.#customProperties.get(varName); + + /** @type {Array} */ + const values = []; + + if (uses) { + // Step 1: @property initial-value first + for (const definition of uses.definitions) { + const block = definition.block; + if (block?.children) { + for (const child of block.children) { + if ( + child.type === "Declaration" && + child.property === "initial-value" + ) { + values.push(child.value); + } + } + } + } + + // Step 2: All declarations in source order + for (const decl of uses.declarations) { + values.push(decl.value); + } + } + + // Step 3: Fallback value last + if (func.children.length >= 3) { + const fallback = func.children[2]; + if (fallback) { + values.push(/** @type {Raw} */ (fallback)); + } + } + + return values; + } } diff --git a/src/rules/no-invalid-properties.js b/src/rules/no-invalid-properties.js index 8a6bd638..1bcd1ada 100644 --- a/src/rules/no-invalid-properties.js +++ b/src/rules/no-invalid-properties.js @@ -127,9 +127,6 @@ export default { const sourceCode = context.sourceCode; const lexer = sourceCode.lexer; - /** @type {Map} */ - const vars = new Map(); - /** * @type {Array<{ * valueParts: string[], @@ -146,6 +143,9 @@ export default { /** * Iteratively resolves CSS variable references until a value is found. + * Uses `sourceCode.getVariableValues()` to look up all possible values + * for a custom property (including declarations that appear after the + * reference), which naturally handles variable hoisting. * @param {string} variableName The variable name to resolve * @param {Map} cache Cache for memoization within a single resolution scope * @param {Set} [seen] Set of already seen variables to detect cycles @@ -182,12 +182,13 @@ export default { return cache.get(currentVarName); } - const valueNode = vars.get(currentVarName); - if (!valueNode) { + const closestValue = + sourceCode.getClosestVariableValue(currentVarName); + if (!closestValue) { break; } - const valueText = sourceCode.getText(valueNode).trim(); + const valueText = sourceCode.getText(closestValue).trim(); const parsed = parseVarFunction(valueText); if (!parsed) { @@ -250,7 +251,9 @@ export default { } /** - * Process a var function node and add its resolved value to the value list + * Process a var function node and add its resolved value to the value list. + * Uses `sourceCode.getVariableValues()` to check all possible values + * in the file, which enables support for variable hoisting. * @param {Object} varNode The var() function node * @param {string[]} valueList Array to collect processed values * @param {Map} valueSegmentLocs Map of rebuilt value segments to their locations @@ -263,13 +266,12 @@ export default { valueSegmentLocs, resolvedCache, ) { - const varValue = vars.get(varNode.children[0].name); + const varName = varNode.children[0].name; + const allValues = sourceCode.getVariableValues(varNode); + const hasKnownDeclaration = allValues.length > 0; - if (varValue) { - const resolvedValue = resolveVariable( - varNode.children[0].name, - resolvedCache, - ); + if (hasKnownDeclaration) { + const resolvedValue = resolveVariable(varName, resolvedCache); if (resolvedValue) { valueList.push(resolvedValue); valueSegmentLocs.set(resolvedValue, varNode.loc); @@ -283,7 +285,7 @@ export default { context.report({ loc: varNode.children[0].loc, messageId: "unknownVar", - data: { var: varNode.children[0].name }, + data: { var: varName }, }); return false; } @@ -311,7 +313,7 @@ export default { context.report({ loc: varNode.children[0].loc, messageId: "unknownVar", - data: { var: varNode.children[0].name }, + data: { var: varName }, }); return false; } @@ -404,9 +406,6 @@ export default { "Rule > Block Declaration:exit"(node) { const state = declStack.pop(); if (node.property.startsWith("--")) { - // store the custom property name and value to validate later - vars.set(node.property, node.value); - // don't validate custom properties return; } diff --git a/tests/languages/css-source-code.test.js b/tests/languages/css-source-code.test.js index f5a725c1..4f0fc63e 100644 --- a/tests/languages/css-source-code.test.js +++ b/tests/languages/css-source-code.test.js @@ -932,4 +932,282 @@ describe("CSSSourceCode", () => { ]); }); }); + + describe("Custom property tracking", () => { + /** + * Helper to create a CSSSourceCode instance from CSS text. + * @param {string} text The CSS source text. + * @returns {import("../../src/languages/css-source-code.js").CSSSourceCode} The CSSSourceCode instance. + */ + function createSourceCode(text) { + const sourceCode = new CSSSourceCode({ + text, + ast: toPlainObject(parse(text, { positions: true })), + }); + // trigger traversal to populate custom property data + sourceCode.traverse(); + return sourceCode; + } + + /** + * Helper to find all nodes of a given type in the AST. + * @param {Object} node The root node. + * @param {string} type The node type to search for. + * @param {Function} [filter] Optional filter function. + * @returns {Array} Matching nodes. + */ + function findNodes(node, type, filter) { + const results = []; + (function walk(n) { + if (n.type === type && (!filter || filter(n))) { + results.push(n); + } + for (const key of ["children", "prelude", "block", "value"]) { + const child = n[key]; + if (child) { + if (Array.isArray(child)) { + child.forEach(walk); + } else if (typeof child === "object" && child.type) { + walk(child); + } + } + } + })(node); + return results; + } + + describe("getDeclarationVariables()", () => { + it("should return var() functions used in a declaration", () => { + const sourceCode = createSourceCode( + ":root { --my-color: red; }\na { color: var(--my-color); }", + ); + const declarations = findNodes( + sourceCode.ast, + "Declaration", + n => n.property === "color", + ); + + const vars = sourceCode.getDeclarationVariables( + declarations[0], + ); + assert.strictEqual(vars.length, 1); + assert.strictEqual(vars[0].type, "Function"); + assert.strictEqual(vars[0].name, "var"); + }); + + it("should return multiple var() functions in one declaration", () => { + const sourceCode = createSourceCode( + ":root { --a: 1px; --b: solid; }\na { border: var(--a) var(--b) red; }", + ); + const declarations = findNodes( + sourceCode.ast, + "Declaration", + n => n.property === "border", + ); + + const vars = sourceCode.getDeclarationVariables( + declarations[0], + ); + assert.strictEqual(vars.length, 2); + }); + + it("should return empty array for declarations without var()", () => { + const sourceCode = createSourceCode("a { color: red; }"); + const declarations = findNodes( + sourceCode.ast, + "Declaration", + n => n.property === "color", + ); + + const vars = sourceCode.getDeclarationVariables( + declarations[0], + ); + assert.strictEqual(vars.length, 0); + }); + + it("should return empty array for unknown declarations", () => { + const sourceCode = createSourceCode("a { color: red; }"); + const vars = sourceCode.getDeclarationVariables({}); + assert.strictEqual(vars.length, 0); + }); + }); + + describe("getClosestVariableValue()", () => { + it("should return the value from the same rule block", () => { + const sourceCode = createSourceCode( + "a { --my-color: red; color: var(--my-color); }", + ); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const value = sourceCode.getClosestVariableValue(varFuncs[0]); + assert.ok(value); + assert.strictEqual(value.type, "Raw"); + assert.strictEqual(value.value.trim(), "red"); + }); + + it("should return the fallback when no same-block declaration exists", () => { + const sourceCode = createSourceCode( + "a { color: var(--my-color, blue); }", + ); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const value = sourceCode.getClosestVariableValue(varFuncs[0]); + assert.ok(value); + assert.strictEqual(value.type, "Raw"); + assert.ok(value.value.includes("blue")); + }); + + it("should return the value from a previous rule", () => { + const sourceCode = createSourceCode( + ":root { --my-color: red; }\na { color: var(--my-color); }", + ); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const value = sourceCode.getClosestVariableValue(varFuncs[0]); + assert.ok(value); + assert.strictEqual(value.type, "Raw"); + assert.strictEqual(value.value.trim(), "red"); + }); + + it("should return @property initial-value as last resort", () => { + const css = + '@property --my-color { syntax: "*"; inherits: false; initial-value: green; }\na { color: var(--my-color); }'; + const sourceCode = createSourceCode(css); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const value = sourceCode.getClosestVariableValue(varFuncs[0]); + assert.ok(value); + }); + + it("should return undefined when no value can be found", () => { + const sourceCode = createSourceCode( + "a { color: var(--unknown); }", + ); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const value = sourceCode.getClosestVariableValue(varFuncs[0]); + assert.strictEqual(value, undefined); + }); + + it("should accept a string name and return the last declaration value", () => { + const sourceCode = createSourceCode( + ":root { --my-color: red; }\n.foo { --my-color: blue; }", + ); + + const value = sourceCode.getClosestVariableValue("--my-color"); + assert.ok(value); + assert.strictEqual(value.type, "Raw"); + assert.strictEqual(value.value.trim(), "blue"); + }); + + it("should return undefined for unknown name string", () => { + const sourceCode = createSourceCode("a { color: red; }"); + const value = sourceCode.getClosestVariableValue("--unknown"); + assert.strictEqual(value, undefined); + }); + }); + + describe("getVariableValues()", () => { + it("should return all declaration values in source order", () => { + const sourceCode = createSourceCode( + ":root { --c: red; }\n.foo { --c: blue; }\na { color: var(--c); }", + ); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const values = sourceCode.getVariableValues(varFuncs[0]); + assert.strictEqual(values.length, 2); + assert.strictEqual(values[0].value.trim(), "red"); + assert.strictEqual(values[1].value.trim(), "blue"); + }); + + it("should include @property initial-value first", () => { + const css = + '@property --c { syntax: "*"; inherits: false; initial-value: green; }\n:root { --c: red; }\na { color: var(--c); }'; + const sourceCode = createSourceCode(css); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const values = sourceCode.getVariableValues(varFuncs[0]); + assert.strictEqual(values.length, 2); + + // First value is from @property initial-value (a Value node) + assert.strictEqual(values[0].type, "Value"); + + // Second is from :root declaration (a Raw node) + assert.strictEqual(values[1].type, "Raw"); + }); + + it("should include fallback value last", () => { + const sourceCode = createSourceCode( + ":root { --c: red; }\na { color: var(--c, blue); }", + ); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const values = sourceCode.getVariableValues(varFuncs[0]); + assert.strictEqual(values.length, 2); + assert.strictEqual(values[0].value.trim(), "red"); + assert.ok(values[1].value.includes("blue")); + }); + + it("should include declarations after the var() reference (hoisting)", () => { + const sourceCode = createSourceCode( + "a { color: var(--c); }\n:root { --c: red; }", + ); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const values = sourceCode.getVariableValues(varFuncs[0]); + assert.strictEqual(values.length, 1); + assert.strictEqual(values[0].value.trim(), "red"); + }); + + it("should return empty array for unknown variables without fallback", () => { + const sourceCode = createSourceCode( + "a { color: var(--unknown); }", + ); + const varFuncs = findNodes( + sourceCode.ast, + "Function", + n => n.name === "var", + ); + + const values = sourceCode.getVariableValues(varFuncs[0]); + assert.strictEqual(values.length, 0); + }); + }); + }); }); diff --git a/tests/rules/no-invalid-properties.test.js b/tests/rules/no-invalid-properties.test.js index 618605c2..e3f1b79f 100644 --- a/tests/rules/no-invalid-properties.test.js +++ b/tests/rules/no-invalid-properties.test.js @@ -35,6 +35,10 @@ ruleTester.run("no-invalid-properties", rule, { "a { --my-color: red; color: var(--my-color) }", ":root { --my-color: red; }\na { color: var(--my-color) }", ":root { --my-color: red; }\na { color: var( --my-color ) }", + + // variable hoisting: definition after usage (#199) + "a { color: var(--my-color) }\n:root { --my-color: red; }", + ".test { color: var(--myColor); }\n:root { --myColor: blue; }", ":root { --my-color: red;\n.foo { color: var(--my-color) }\n}", ".fluidHeading {font-size: clamp(2.1rem, calc(7.2vw - 0.2rem), 2.5rem);}", "a { color: var(--my-color, red) }",