Skip to content

Commit 9fb07fa

Browse files
nzakassnitin315
andauthored
feat: Validate property values containing variables (#148)
* feat: Validation property values containing variables * Remove unnecessary comments * Fix grammar in docs * Clean up edge cases * Better error message * Update docs/rules/no-invalid-properties.md Co-authored-by: Nitin Kumar <snitin315@gmail.com> * Account for whitespace inside of var() * Update tests and add comments * Remove unneeded file * Fix second variable reported location --------- Co-authored-by: Nitin Kumar <snitin315@gmail.com>
1 parent c397651 commit 9fb07fa

File tree

3 files changed

+333
-20
lines changed

3 files changed

+333
-20
lines changed

docs/rules/no-invalid-properties.md

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,21 @@ body {
4646

4747
### Limitations
4848

49-
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.
49+
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:
50+
51+
```css
52+
:root {
53+
--my-color: red;
54+
}
55+
56+
a {
57+
color: var(--my-color);
58+
}
59+
```
60+
61+
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.
62+
63+
If the custom property is defined in another file, it's recommended to create a dummy rule just to ensure proper validation.
5064

5165
## When Not to Use It
5266

src/rules/no-invalid-properties.js

Lines changed: 144 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,48 @@ import { isSyntaxMatchError } from "../util.js";
1515

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

23+
//-----------------------------------------------------------------------------
24+
// Helpers
25+
//-----------------------------------------------------------------------------
26+
27+
/**
28+
* Replaces all instances of a regex pattern with a replacement and tracks the offsets
29+
* @param {string} text The text to perform replacements on
30+
* @param {string} varName The regex pattern string to search for
31+
* @param {string} replaceValue The string to replace with
32+
* @returns {{text: string, offsets: Array<number>}} The updated text and array of offsets
33+
* where replacements occurred
34+
*/
35+
function replaceWithOffsets(text, varName, replaceValue) {
36+
const offsets = [];
37+
let result = "";
38+
let lastIndex = 0;
39+
40+
const regex = new RegExp(`var\\(\\s*${varName}\\s*\\)`, "gu");
41+
let match;
42+
43+
while ((match = regex.exec(text)) !== null) {
44+
result += text.slice(lastIndex, match.index);
45+
46+
/*
47+
* We need the offset of the replacement after other replacements have
48+
* been made, so we push the current length of the result before appending
49+
* the replacement value.
50+
*/
51+
offsets.push(result.length);
52+
result += replaceValue;
53+
lastIndex = match.index + match[0].length;
54+
}
55+
56+
result += text.slice(lastIndex);
57+
return { text: result, offsets };
58+
}
59+
2260
//-----------------------------------------------------------------------------
2361
// Rule Definition
2462
//-----------------------------------------------------------------------------
@@ -38,48 +76,135 @@ export default {
3876
invalidPropertyValue:
3977
"Invalid value '{{value}}' for property '{{property}}'. Expected {{expected}}.",
4078
unknownProperty: "Unknown property '{{property}}' found.",
79+
unknownVar: "Can't validate with unknown variable '{{var}}'.",
4180
},
4281
},
4382

4483
create(context) {
45-
const lexer = context.sourceCode.lexer;
84+
const sourceCode = context.sourceCode;
85+
const lexer = sourceCode.lexer;
86+
87+
/** @type {Map<string,ValuePlain>} */
88+
const vars = new Map();
89+
90+
/**
91+
* We need to track this as a stack because we can have nested
92+
* rules that use the `var()` function, and we need to
93+
* ensure that we validate the innermost rule first.
94+
* @type {Array<Map<string,FunctionNodePlain>>}
95+
*/
96+
const replacements = [];
4697

4798
return {
48-
"Rule > Block > Declaration"(node) {
49-
// don't validate custom properties
99+
"Rule > Block > Declaration"() {
100+
replacements.push(new Map());
101+
},
102+
103+
"Function[name=var]"(node) {
104+
const map = replacements.at(-1);
105+
if (!map) {
106+
return;
107+
}
108+
109+
/*
110+
* Store the custom property name and the function node
111+
* so can use these to validate the value later.
112+
*/
113+
const name = node.children[0].name;
114+
map.set(name, node);
115+
},
116+
117+
"Rule > Block > Declaration:exit"(node) {
50118
if (node.property.startsWith("--")) {
119+
// store the custom property name and value to validate later
120+
vars.set(node.property, node.value);
121+
122+
// don't validate custom properties
51123
return;
52124
}
53125

54-
const { error } = lexer.matchDeclaration(node);
126+
const varsFound = replacements.pop();
127+
128+
/** @type {Map<number,CssLocationRange>} */
129+
const varsFoundLocs = new Map();
130+
const usingVars = varsFound?.size > 0;
131+
let value = node.value;
132+
133+
if (usingVars) {
134+
// need to use a text version of the value here
135+
value = sourceCode.getText(node.value);
136+
let offsets;
137+
138+
// replace any custom properties with their values
139+
for (const [name, func] of varsFound) {
140+
const varValue = vars.get(name);
141+
142+
if (varValue) {
143+
({ text: value, offsets } = replaceWithOffsets(
144+
value,
145+
name,
146+
sourceCode.getText(varValue).trim(),
147+
));
148+
149+
/*
150+
* Store the offsets of the replacements so we can
151+
* report the correct location of any validation error.
152+
*/
153+
offsets.forEach(offset => {
154+
varsFoundLocs.set(offset, func.loc);
155+
});
156+
} else {
157+
context.report({
158+
loc: func.children[0].loc,
159+
messageId: "unknownVar",
160+
data: {
161+
var: name,
162+
},
163+
});
164+
165+
return;
166+
}
167+
}
168+
}
169+
170+
const { error } = lexer.matchProperty(node.property, value);
55171

56172
if (error) {
57173
// validation failure
58174
if (isSyntaxMatchError(error)) {
59175
context.report({
60-
loc: error.loc,
176+
/*
177+
* When using variables, check to see if the error
178+
* occurred at a location where a variable was replaced.
179+
* If so, use that location; otherwise, use the error's
180+
* reported location.
181+
*/
182+
loc: usingVars
183+
? (varsFoundLocs.get(error.mismatchOffset) ??
184+
node.value.loc)
185+
: error.loc,
61186
messageId: "invalidPropertyValue",
62187
data: {
63188
property: node.property,
64-
value: error.css,
189+
190+
/*
191+
* When using variables, slice the value to
192+
* only include the part that caused the error.
193+
* Otherwise, use the full value from the error.
194+
*/
195+
value: usingVars
196+
? value.slice(
197+
error.mismatchOffset,
198+
error.mismatchOffset +
199+
error.mismatchLength,
200+
)
201+
: error.css,
65202
expected: error.syntax,
66203
},
67204
});
68205
return;
69206
}
70207

71-
/*
72-
* There's no current way to get lexing to work when a
73-
* `var()` is present in a value. Rather than blowing up,
74-
* we'll just ignore it.
75-
*
76-
* https://github.com/csstree/csstree/issues/317
77-
*/
78-
79-
if (error.message.endsWith("var() is not supported")) {
80-
return;
81-
}
82-
83208
// unknown property
84209
context.report({
85210
loc: {

0 commit comments

Comments
 (0)