Skip to content

fix: no-invalid-properties false positives for var() in functions #227

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.

Sign up for GitHub

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 1 commit into
base: main
Choose a base branch
from
Open

fix: no-invalid-properties false positives for var() in functions #227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
243 changes: 138 additions & 105 deletions src/rules/no-invalid-properties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string,CssLocationRange>} 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<string,CssLocationRange>} 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
Copy link
Member

Choose a reason for hiding this comment

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

Here we're doing a traversal on top of the traversal that is already happening in the rule, which is inefficient.

A more efficient approach would be to use the Function visitor method to do the traversal automatically. Can you take a look at that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into using the Function visitor method here, but I don’t see a straightforward way to apply it. The code at this point isn’t just traversing—it’s reconstructing the property value as a string, including nested function names and arguments. The visitor pattern alone doesn’t provide the context needed for this reconstruction, so a manual traversal seems necessary. If you know of a way to leverage the visitor pattern for this case, I’d appreciate your input.

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());
Expand Down Expand Up @@ -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();
Expand Down
Loading
Loading