Skip to content
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
16 changes: 15 additions & 1 deletion docs/rules/no-invalid-properties.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,21 @@ body {

### Limitations

This rule uses the lexer from [CSSTree](https://github.com/csstree/csstree), which does not support validation of property values that contain variable references (i.e., `var(--bg-color)`). The lexer throws an error when it comes across a variable reference, and rather than displaying that error, this rule ignores it. This unfortunately means that this rule cannot properly validate properties values that contain variable references. We'll continue to work towards a solution for this.
When a variable is used in a property value, such as `var(--my-color)`, the rule can only properly be validated if the parser has already encountered the `--my-color` custom property. For example, this will validate correctly:

```css
:root {
--my-color: red;
}

a {
color: var(--my-color);
}
```

This code defines `--my-color` before it is used and therefore the rule can validate the `color` property. If `--my-color` was not defined before `var(--my-color)` was used, it results in a lint error because the validation cannot be completed.

If the custom property is defined in another file, it's recommended to create a dummy rule just to ensure proper validation.

## When Not to Use It

Expand Down
163 changes: 144 additions & 19 deletions src/rules/no-invalid-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,48 @@ import { isSyntaxMatchError } from "../util.js";

/**
* @import { CSSRuleDefinition } from "../types.js"
* @typedef {"invalidPropertyValue" | "unknownProperty"} NoInvalidPropertiesMessageIds
* @import { ValuePlain, FunctionNodePlain, CssLocationRange } from "@eslint/css-tree";
* @typedef {"invalidPropertyValue" | "unknownProperty" | "unknownVar"} NoInvalidPropertiesMessageIds
* @typedef {CSSRuleDefinition<{ RuleOptions: [], MessageIds: NoInvalidPropertiesMessageIds }>} NoInvalidPropertiesRuleDefinition
*/

//-----------------------------------------------------------------------------
// Helpers
//-----------------------------------------------------------------------------

/**
* Replaces all instances of a regex pattern with a replacement and tracks the offsets
* @param {string} text The text to perform replacements on
* @param {string} varName The regex pattern string to search for
* @param {string} replaceValue The string to replace with
* @returns {{text: string, offsets: Array<number>}} The updated text and array of offsets
* where replacements occurred
*/
function replaceWithOffsets(text, varName, replaceValue) {
const offsets = [];
let result = "";
let lastIndex = 0;

const regex = new RegExp(`var\\(\\s*${varName}\\s*\\)`, "gu");
let match;

while ((match = regex.exec(text)) !== null) {
result += text.slice(lastIndex, match.index);

/*
* We need the offset of the replacement after other replacements have
* been made, so we push the current length of the result before appending
* the replacement value.
*/
offsets.push(result.length);
result += replaceValue;
lastIndex = match.index + match[0].length;
}

result += text.slice(lastIndex);
return { text: result, offsets };
}

//-----------------------------------------------------------------------------
// Rule Definition
//-----------------------------------------------------------------------------
Expand All @@ -38,48 +76,135 @@ export default {
invalidPropertyValue:
"Invalid value '{{value}}' for property '{{property}}'. Expected {{expected}}.",
unknownProperty: "Unknown property '{{property}}' found.",
unknownVar: "Can't validate with unknown variable '{{var}}'.",
},
},

create(context) {
const lexer = context.sourceCode.lexer;
const sourceCode = context.sourceCode;
const lexer = sourceCode.lexer;

/** @type {Map<string,ValuePlain>} */
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<Map<string,FunctionNodePlain>>}
*/
const replacements = [];

return {
"Rule > Block > Declaration"(node) {
// don't validate custom properties
"Rule > Block > Declaration"() {
replacements.push(new Map());
},

"Function[name=var]"(node) {
const map = replacements.at(-1);
if (!map) {
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);
},

"Rule > Block > Declaration:exit"(node) {
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;
}

const { error } = lexer.matchDeclaration(node);
const varsFound = replacements.pop();

/** @type {Map<number,CssLocationRange>} */
const varsFoundLocs = new Map();
const usingVars = varsFound?.size > 0;
let value = node.value;

if (usingVars) {
// need to use a text version of the value here
value = sourceCode.getText(node.value);
let offsets;

// replace any custom properties with their values
for (const [name, func] of varsFound) {
const varValue = vars.get(name);

if (varValue) {
({ text: value, offsets } = replaceWithOffsets(
value,
name,
sourceCode.getText(varValue).trim(),
));

/*
* Store the offsets of the replacements so we can
* report the correct location of any validation error.
*/
offsets.forEach(offset => {
varsFoundLocs.set(offset, func.loc);
});
} else {
context.report({
loc: func.children[0].loc,
messageId: "unknownVar",
data: {
var: name,
},
});

return;
}
}
}

const { error } = lexer.matchProperty(node.property, value);

if (error) {
// validation failure
if (isSyntaxMatchError(error)) {
context.report({
loc: error.loc,
/*
* When using variables, check to see if the error
* occurred at a location where a variable was replaced.
* If so, use that location; otherwise, use the error's
* reported location.
*/
loc: usingVars
? (varsFoundLocs.get(error.mismatchOffset) ??
node.value.loc)
: error.loc,
Comment on lines +176 to +185
Copy link
Member

Choose a reason for hiding this comment

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

I think this doesn't quite work as expected when there are multiple replacements. For example:

a { --width: 1px; border-top: var(--width) solid var(--width); }

I can see that in this case the SyntaxError has a mismatchOffset of 10, i.e. the offset of the second var(--width) after the replacements were applied, but varsFoundLocs contains the offsets of the variables before the replacement. Thus there is no match and the rule reports the error's original location.

I think this can be fixed by ensuring that replaceWithOffsets returns the offsets of the replaced locations in the output text, rather than in the original string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to make sure I understand what you're saying -- you're saying that when there's a second var() that contains an incorrect value the reported location is not the location of that var()?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I got it. 👍

messageId: "invalidPropertyValue",
data: {
property: node.property,
value: error.css,

/*
* When using variables, slice the value to
* 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,
)
: error.css,
expected: error.syntax,
},
});
return;
}

/*
* There's no current way to get lexing to work when a
* `var()` is present in a value. Rather than blowing up,
* we'll just ignore it.
*
* https://github.com/csstree/csstree/issues/317
*/

if (error.message.endsWith("var() is not supported")) {
return;
}

// unknown property
context.report({
loc: {
Expand Down
Loading