From b9f8b5180be93889f865eb5d1d7cb95205606d45 Mon Sep 17 00:00:00 2001 From: thecalamiity Date: Mon, 4 Aug 2025 08:13:37 +0300 Subject: [PATCH 1/2] fix: no-invalid-properties false positives for var() in functions --- src/rules/no-invalid-properties.js | 243 ++++++++++++---------- tests/rules/no-invalid-properties.test.js | 124 +++++++++++ 2 files changed, 262 insertions(+), 105 deletions(-) diff --git a/src/rules/no-invalid-properties.js b/src/rules/no-invalid-properties.js index 104d236b..22456b60 100644 --- a/src/rules/no-invalid-properties.js +++ b/src/rules/no-invalid-properties.js @@ -124,6 +124,127 @@ export default { const [{ allowUnknownVariables }] = context.options; + /** + * Process a var function node and add its resolved value to the value list + * @param {Object} varNode The var function node + * @param {string[]} valueList Array to collect processed values + * @param {Map} valuesWithVarLocs Map of values to their locations + * @returns {boolean} Whether processing was successful + */ + function processVarFunction(varNode, valueList, valuesWithVarLocs) { + const varValue = vars.get(varNode.children[0].name); + + if (varValue) { + const varValueText = sourceCode.getText(varValue).trim(); + valueList.push(varValueText); + valuesWithVarLocs.set(varValueText, varNode.loc); + return true; + } + + // If the variable is not found and doesn't have a fallback value, report it + if (varNode.children.length === 1) { + if (!allowUnknownVariables) { + context.report({ + loc: varNode.children[0].loc, + messageId: "unknownVar", + data: { var: varNode.children[0].name }, + }); + return false; + } + return true; + } + + // Handle fallback values + if (varNode.children[2].type !== "Raw") { + return true; + } + + const fallbackVarList = getVarFallbackList( + varNode.children[2].value.trim(), + ); + + if (fallbackVarList.length === 0) { + const fallbackValue = varNode.children[2].value.trim(); + valueList.push(fallbackValue); + valuesWithVarLocs.set(fallbackValue, varNode.loc); + return true; + } + + // Process fallback variables + for (const fallbackVar of fallbackVarList) { + if (fallbackVar.startsWith("--")) { + const fallbackVarValue = vars.get(fallbackVar); + if (fallbackVarValue) { + const fallbackValue = sourceCode + .getText(fallbackVarValue) + .trim(); + valueList.push(fallbackValue); + valuesWithVarLocs.set(fallbackValue, varNode.loc); + return true; + } + } else { + valueList.push(fallbackVar.trim()); + valuesWithVarLocs.set(fallbackVar.trim(), varNode.loc); + return true; + } + } + + // No valid fallback found + if (!allowUnknownVariables) { + context.report({ + loc: varNode.children[0].loc, + messageId: "unknownVar", + data: { var: varNode.children[0].name }, + }); + return false; + } + + return true; + } + + /** + * Process a nested function by recursively handling its children + * @param {FunctionNodePlain} funcNode The function node + * @param {Map} valuesWithVarLocs Map of values to their locations + * @returns {string|null} The processed function string or null if processing failed + */ + function processNestedFunction(funcNode, valuesWithVarLocs) { + const nestedValueList = []; + + for (const nestedChild of funcNode.children) { + if ( + nestedChild.type === "Function" && + nestedChild.name === "var" + ) { + if ( + !processVarFunction( + nestedChild, + nestedValueList, + valuesWithVarLocs, + ) + ) { + return null; + } + } else if (nestedChild.type === "Function") { + // Recursively process nested functions + const processedNestedFunction = processNestedFunction( + nestedChild, + valuesWithVarLocs, + ); + if (!processedNestedFunction) { + return null; + } + nestedValueList.push(processedNestedFunction); + } else { + nestedValueList.push( + sourceCode.getText(nestedChild).trim(), + ); + } + } + + return `${funcNode.name}(${nestedValueList.join(" ")})`; + } + return { "Rule > Block > Declaration"() { replacements.push(new Map()); @@ -167,112 +288,24 @@ export default { for (const child of valueNodes) { // If value is a function starts with `var()` if (child.type === "Function" && child.name === "var") { - const varValue = vars.get(child.children[0].name); - - // If the variable is found, use its value, otherwise check for fallback values - if (varValue) { - const varValueText = sourceCode - .getText(varValue) - .trim(); - - valueList.push(varValueText); - valuesWithVarLocs.set(varValueText, child.loc); - } else { - // If the variable is not found and doesn't have a fallback value, report it - if (child.children.length === 1) { - if (!allowUnknownVariables) { - context.report({ - loc: child.children[0].loc, - messageId: "unknownVar", - data: { - var: child.children[0].name, - }, - }); - - return; - } - } else { - // If it has a fallback value, use that - if (child.children[2].type === "Raw") { - const fallbackVarList = - getVarFallbackList( - child.children[2].value.trim(), - ); - if (fallbackVarList.length > 0) { - let gotFallbackVarValue = false; - - for (const fallbackVar of fallbackVarList) { - if ( - fallbackVar.startsWith("--") - ) { - const fallbackVarValue = - vars.get(fallbackVar); - - if (!fallbackVarValue) { - continue; // Try the next fallback - } - - valueList.push( - sourceCode - .getText( - fallbackVarValue, - ) - .trim(), - ); - valuesWithVarLocs.set( - sourceCode - .getText( - fallbackVarValue, - ) - .trim(), - child.loc, - ); - gotFallbackVarValue = true; - break; // Stop after finding the first valid variable - } else { - const fallbackValue = - fallbackVar.trim(); - valueList.push( - fallbackValue, - ); - valuesWithVarLocs.set( - fallbackValue, - child.loc, - ); - gotFallbackVarValue = true; - break; // Stop after finding the first non-variable fallback - } - } - - // If none of the fallback value is defined then report an error - if ( - !allowUnknownVariables && - !gotFallbackVarValue - ) { - context.report({ - loc: child.children[0].loc, - messageId: "unknownVar", - data: { - var: child.children[0] - .name, - }, - }); - - return; - } - } else { - // if it has a fallback value, use that - const fallbackValue = - child.children[2].value.trim(); - valueList.push(fallbackValue); - valuesWithVarLocs.set( - fallbackValue, - child.loc, - ); - } - } - } + if ( + !processVarFunction( + child, + valueList, + valuesWithVarLocs, + ) + ) { + return; + } + } else if (child.type === "Function") { + const processedFunction = processNestedFunction( + child, + valuesWithVarLocs, + ); + if (!processedFunction) { + return; } + valueList.push(processedFunction); } else { // If the child is not a `var()` function, just add its text to the `valueList` const valueText = sourceCode.getText(child).trim(); diff --git a/tests/rules/no-invalid-properties.test.js b/tests/rules/no-invalid-properties.test.js index d8768749..101da59b 100644 --- a/tests/rules/no-invalid-properties.test.js +++ b/tests/rules/no-invalid-properties.test.js @@ -55,6 +55,36 @@ ruleTester.run("no-invalid-properties", rule, { ":root { --my-color: red; }\na { color: var(--my-color, var(--fallback-color, var(--foo, var(--bar)))) }", ":root { --my-color: red; }\na { color: var(--my-color, var(--fallback-color, var(--foo, var(--bar, blue)))) }", ":root { --color: red }\na { border-top: 1px var(--style, var(--fallback, solid)) var(--color, blue); }", + "a { width: calc(var(--my-width, 100px)) }", + ":root { --my-heading: 3rem; }\na { width: calc(var(--my-width, 100px)) }", + ":root { --my-heading: 3rem; --foo: 100px }\na { width: calc(var(--my-width, var(--foo, 200px))) }", + ":root { --my-heading: 3rem; }\na { width: calc(var(--my-width, var(--foo, 200px))) }", + "a { width: calc(var(--my-width, var(--foo, var(--bar, 200px)))) }", + ":root { --my-width: 100px; }\na { width: calc(var(--my-width, 200px)) }", + ":root { --my-fallback: 100px; }\na { width: calc(var(--my-width, var(--my-fallback))) }", + ":root { --my-fallback: 100px; }\na { width: calc(var(--my-width, var(--my-fallback, 200px))) }", + ":root { --foo: 100px; }\na { width: calc(var(--my-width, var(--my-fallback, var(--foo)))) }", + "a { width: calc(var(--my-width, var(--my-fallback, var(--foo, 200px)))) }", + ":root { --my-width: 100px; }\na { width: calc(var(--my-width, var(--fallback-width))) }", + ":root { --my-width: 100px; --fallback-width: 200px; }\na { width: calc(var(--my-width, var(--fallback-width))) }", + ":root { --my-width: 100px; }\na { width: calc(var(--my-width, var(--fallback-width, 200px))) }", + ":root { --my-width: 100px; }\na { width: calc(var(--my-width, var(--fallback-width, var(--foo)))) }", + ":root { --my-width: 100px; }\na { width: calc(var(--my-width, var(--fallback-width, var(--foo, 200px)))) }", + ":root { --my-width: 100px; }\na { width: calc(var(--my-width, var(--fallback-width, var(--foo, var(--bar))))) }", + ":root { --my-width: 100px; }\na { width: calc(var(--my-width, var(--fallback-width, var(--foo, var(--bar, 200px))))) }", + ":root { --width: 1px; }\na { border-top: calc(var(--width, 2px)) var(--style, var(--fallback, solid)) red; }", + ":root { --width: 100px; }\na { width: calc(calc(100% - var(--width))); }", + ":root { --width: 100px; }\na { width: calc(calc(var(--width) + 50px) - 25px); }", + ":root { --color: red; }\na { background: linear-gradient(to right, var(--color), blue); }", + ":root { --color: red; --offset: 10px; }\na { transform: translateX(calc(var(--offset) + 20px)); }", + ":root { --width: 100px; }\na { width: clamp(50px, var(--width), 200px); }", + ":root { --width: 100px; }\na { width: min(var(--width), 150px); }", + ":root { --width: 100px; }\na { width: max(var(--width), 50px); }", + ":root { --width: 100px; }\na { width: calc(min(var(--width), 150px) + 10px); }", + ":root { --width: 100px; }\na { width: calc(max(var(--width), 50px) - 5px); }", + ":root { --width: 100px; }\na { width: calc(clamp(50px, var(--width), 200px) / 2); }", + ":root { --color: red; }\na { filter: drop-shadow(0 0 10px var(--color)); }", + ":root { --color: red; }\na { background: radial-gradient(circle, var(--color), transparent); }", { code: "a { my-custom-color: red; }", languageOptions: { @@ -69,6 +99,10 @@ ruleTester.run("no-invalid-properties", rule, { code: "a { color: var(--my-color); }", options: [{ allowUnknownVariables: true }], }, + { + code: "a { width: calc(var(--width)); }", + options: [{ allowUnknownVariables: true }], + }, { code: "a { --my-color: red; color: var(--my-color); background-color: var(--unknown-var); }", options: [{ allowUnknownVariables: true }], @@ -592,5 +626,95 @@ ruleTester.run("no-invalid-properties", rule, { }, ], }, + { + code: "a { padding: calc(var(--padding-top, 1px) + 1px) 2px calc(var(--padding-bottom) + 1px); }", + errors: [ + { + messageId: "unknownVar", + data: { + var: "--padding-bottom", + }, + line: 1, + column: 63, + endLine: 1, + endColumn: 79, + }, + ], + }, + { + code: "a { padding: calc(var(--padding-top, var(--fallback))) 2px calc(var(--padding-bottom)); }", + errors: [ + { + messageId: "unknownVar", + data: { + var: "--padding-top", + }, + line: 1, + column: 23, + endLine: 1, + endColumn: 36, + }, + ], + }, + { + code: ":root { --width: 100px }\na { widthh: calc(var(--width, 200px)); }", + errors: [ + { + messageId: "unknownProperty", + data: { + property: "widthh", + }, + line: 2, + column: 5, + endLine: 2, + endColumn: 11, + }, + ], + }, + { + code: "a { padding: calc(max(var(--padding-top, var(--fallback))), 1px) 2px calc(var(--padding-bottom)); }", + errors: [ + { + messageId: "unknownVar", + data: { + var: "--padding-top", + }, + line: 1, + column: 27, + endLine: 1, + endColumn: 40, + }, + ], + }, + { + code: "a { color: rgba(calc(var(--red, 255) + var(--green)), 0, calc(var(--blue)), 1); }", + errors: [ + { + messageId: "unknownVar", + data: { + var: "--green", + }, + line: 1, + column: 44, + endLine: 1, + endColumn: 51, + }, + ], + }, + { + code: "a { transform: translateX(calc(var(--offset-x, min(var(--default-offset, 5px), 10px))) rotate(var(--rotation))); }", + errors: [ + { + messageId: "unknownVar", + data: { + var: "--rotation", + }, + line: 1, + column: 99, + endLine: 1, + endColumn: 109, + }, + ], + }, ], }); From a43ea8cc116480ff80adb2f1325ad87fc5d43d1b Mon Sep 17 00:00:00 2001 From: thecalamiity Date: Fri, 22 Aug 2025 11:03:54 +0300 Subject: [PATCH 2/2] use Function visitors to avoid double traversal --- src/rules/no-invalid-properties.js | 199 ++++++++++------------ tests/rules/no-invalid-properties.test.js | 18 ++ 2 files changed, 104 insertions(+), 113 deletions(-) diff --git a/src/rules/no-invalid-properties.js b/src/rules/no-invalid-properties.js index 22456b60..abe7ac14 100644 --- a/src/rules/no-invalid-properties.js +++ b/src/rules/no-invalid-properties.js @@ -115,29 +115,32 @@ export default { const vars = new Map(); /** - * We need to track this as a stack because we can have nested - * rules that use the `var()` function, and we need to - * ensure that we validate the innermost rule first. - * @type {Array>} + * @type {Array<{ + * valueParts: string[], + * functionPartsStack: string[][], + * valueSegmentLocs: Map, + * skipValidation: boolean, + * hadVarSubstitution: boolean, + * }>} */ - const replacements = []; + const declStack = []; const [{ allowUnknownVariables }] = context.options; /** * Process a var function node and add its resolved value to the value list - * @param {Object} varNode The var function node + * @param {Object} varNode The var() function node * @param {string[]} valueList Array to collect processed values - * @param {Map} valuesWithVarLocs Map of values to their locations + * @param {Map} valueSegmentLocs Map of rebuilt value segments to their locations * @returns {boolean} Whether processing was successful */ - function processVarFunction(varNode, valueList, valuesWithVarLocs) { + function processVarFunction(varNode, valueList, valueSegmentLocs) { const varValue = vars.get(varNode.children[0].name); if (varValue) { const varValueText = sourceCode.getText(varValue).trim(); valueList.push(varValueText); - valuesWithVarLocs.set(varValueText, varNode.loc); + valueSegmentLocs.set(varValueText, varNode.loc); return true; } @@ -166,7 +169,7 @@ export default { if (fallbackVarList.length === 0) { const fallbackValue = varNode.children[2].value.trim(); valueList.push(fallbackValue); - valuesWithVarLocs.set(fallbackValue, varNode.loc); + valueSegmentLocs.set(fallbackValue, varNode.loc); return true; } @@ -179,12 +182,12 @@ export default { .getText(fallbackVarValue) .trim(); valueList.push(fallbackValue); - valuesWithVarLocs.set(fallbackValue, varNode.loc); + valueSegmentLocs.set(fallbackValue, varNode.loc); return true; } } else { valueList.push(fallbackVar.trim()); - valuesWithVarLocs.set(fallbackVar.trim(), varNode.loc); + valueSegmentLocs.set(fallbackVar.trim(), varNode.loc); return true; } } @@ -202,69 +205,77 @@ export default { return true; } - /** - * Process a nested function by recursively handling its children - * @param {FunctionNodePlain} funcNode The function node - * @param {Map} valuesWithVarLocs Map of values to their locations - * @returns {string|null} The processed function string or null if processing failed - */ - function processNestedFunction(funcNode, valuesWithVarLocs) { - const nestedValueList = []; - - for (const nestedChild of funcNode.children) { - if ( - nestedChild.type === "Function" && - nestedChild.name === "var" - ) { - if ( - !processVarFunction( - nestedChild, - nestedValueList, - valuesWithVarLocs, - ) - ) { - return null; - } - } else if (nestedChild.type === "Function") { - // Recursively process nested functions - const processedNestedFunction = processNestedFunction( - nestedChild, - valuesWithVarLocs, - ); - if (!processedNestedFunction) { - return null; - } - nestedValueList.push(processedNestedFunction); - } else { - nestedValueList.push( - sourceCode.getText(nestedChild).trim(), - ); - } - } - - return `${funcNode.name}(${nestedValueList.join(" ")})`; - } - return { "Rule > Block > Declaration"() { - replacements.push(new Map()); + declStack.push({ + valueParts: [], + functionPartsStack: [], + valueSegmentLocs: new Map(), + skipValidation: false, + hadVarSubstitution: false, + }); + }, + + "Rule > Block > Declaration > Value > *:not(Function)"(node) { + const state = declStack.at(-1); + const text = sourceCode.getText(node).trim(); + state.valueParts.push(text); + state.valueSegmentLocs.set(text, node.loc); + }, + + Function() { + declStack.at(-1).functionPartsStack.push([]); + }, + + "Function > *:not(Function)"(node) { + const state = declStack.at(-1); + const parts = state.functionPartsStack.at(-1); + const text = sourceCode.getText(node).trim(); + parts.push(text); + state.valueSegmentLocs.set(text, node.loc); }, - "Function[name=var]"(node) { - const map = replacements.at(-1); - if (!map) { + "Function:exit"(node) { + const state = declStack.at(-1); + if (state.skipValidation) { return; } - /* - * Store the custom property name and the function node - * so can use these to validate the value later. - */ - const name = node.children[0].name; - map.set(name, node); + const parts = state.functionPartsStack.pop(); + let result; + if (node.name.toLowerCase() === "var") { + const resolvedParts = []; + const success = processVarFunction( + node, + resolvedParts, + state.valueSegmentLocs, + ); + + if (!success) { + state.skipValidation = true; + return; + } + + if (resolvedParts.length === 0) { + return; + } + + state.hadVarSubstitution = true; + result = resolvedParts[0]; + } else { + result = `${node.name}(${parts.join(" ")})`; + } + + const parentParts = state.functionPartsStack.at(-1); + if (parentParts) { + parentParts.push(result); + } else { + state.valueParts.push(result); + } }, "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); @@ -273,47 +284,13 @@ export default { return; } - const varsFound = replacements.pop(); + if (state.skipValidation) { + return; + } - /** @type {Map} */ - const valuesWithVarLocs = new Map(); - const usingVars = varsFound?.size > 0; let value = node.value; - - if (usingVars) { - const valueList = []; - const valueNodes = node.value.children; - - // When `var()` is used, we store all the values to `valueList` with the replacement of `var()` with there values or fallback values - for (const child of valueNodes) { - // If value is a function starts with `var()` - if (child.type === "Function" && child.name === "var") { - if ( - !processVarFunction( - child, - valueList, - valuesWithVarLocs, - ) - ) { - return; - } - } else if (child.type === "Function") { - const processedFunction = processNestedFunction( - child, - valuesWithVarLocs, - ); - if (!processedFunction) { - return; - } - valueList.push(processedFunction); - } else { - // If the child is not a `var()` function, just add its text to the `valueList` - const valueText = sourceCode.getText(child).trim(); - valueList.push(valueText); - valuesWithVarLocs.set(valueText, child.loc); - } - } - + if (state.hadVarSubstitution) { + const valueList = state.valueParts; value = valueList.length > 0 ? valueList.join(" ") @@ -326,7 +303,7 @@ export default { // validation failure if (isSyntaxMatchError(error)) { const errorValue = - usingVars && + state.hadVarSubstitution && value.slice( error.mismatchOffset, error.mismatchOffset + error.mismatchLength, @@ -339,8 +316,8 @@ export default { * If so, use that location; otherwise, use the error's * reported location. */ - loc: usingVars - ? (valuesWithVarLocs.get(errorValue) ?? + loc: state.hadVarSubstitution + ? (state.valueSegmentLocs.get(errorValue) ?? node.value.loc) : error.loc, messageId: "invalidPropertyValue", @@ -352,12 +329,8 @@ export default { * only include the part that caused the error. * Otherwise, use the full value from the error. */ - value: usingVars - ? value.slice( - error.mismatchOffset, - error.mismatchOffset + - error.mismatchLength, - ) + value: state.hadVarSubstitution + ? errorValue : error.css, expected: error.syntax, }, diff --git a/tests/rules/no-invalid-properties.test.js b/tests/rules/no-invalid-properties.test.js index 101da59b..a0e10f72 100644 --- a/tests/rules/no-invalid-properties.test.js +++ b/tests/rules/no-invalid-properties.test.js @@ -65,6 +65,7 @@ ruleTester.run("no-invalid-properties", rule, { ":root { --my-fallback: 100px; }\na { width: calc(var(--my-width, var(--my-fallback, 200px))) }", ":root { --foo: 100px; }\na { width: calc(var(--my-width, var(--my-fallback, var(--foo)))) }", "a { width: calc(var(--my-width, var(--my-fallback, var(--foo, 200px)))) }", + "a { background-image: linear-gradient(90deg, red, var(--c, blue)); }", ":root { --my-width: 100px; }\na { width: calc(var(--my-width, var(--fallback-width))) }", ":root { --my-width: 100px; --fallback-width: 200px; }\na { width: calc(var(--my-width, var(--fallback-width))) }", ":root { --my-width: 100px; }\na { width: calc(var(--my-width, var(--fallback-width, 200px))) }", @@ -626,6 +627,23 @@ ruleTester.run("no-invalid-properties", rule, { }, ], }, + { + code: "a { background-image: linear-gradient(90deg, 45deg, var(--c, blue)); }", + errors: [ + { + messageId: "invalidPropertyValue", + data: { + property: "background-image", + value: "45deg", + expected: "#", + }, + line: 1, + column: 46, + endLine: 1, + endColumn: 51, + }, + ], + }, { code: "a { padding: calc(var(--padding-top, 1px) + 1px) 2px calc(var(--padding-bottom) + 1px); }", errors: [