Add dashed-ident-no-missing-reference-function

[nex3]

What is the problem you're trying to solve?

Developers sometimes accidentally forget to include the var() function call when trying to dereference custom properties. For example, they might write:

a {
  --my-color: --other-color;
  color: --my-color;
}

when they meant

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

What solution would you like to see?

A new rule that flags identifiers beginning with -- that appear outside of the first argument of a var() function.

[jeddy3]

Sounds good to me.

The following invalid syntax is caught by the csstree-validator plugin:

a {
  color: --my-color;
}

However, it's a simple validation case that we can add a built-in rule for.

I suggest the name function-var-no-invalid to align with our conventions, where function var is the thing being target (see similar rules) and no invalid what is being checked.

• Name: function-var-no-invalid
• Primary option: true
• Secondary options: None
• Autofixable: Yes, but limited ("The fix option can automatically fix most of the problems reported by this rule.")
• Message: Unexpected unreferenced custom property
• Description: "Disallow invalid var functions"
• Section: "Possible errors" -> "Functions"

We'll want to use the value parser in the implementation. It parses --x as a word node. We'll want to check that the parent nodes of word nodes beginning with -- are of type function and value var (case insensitive).

I think we'll be able to autofix most violations. We may want to not autofix if the next node is a comma, e.g.:

:root {
  --main-color: #c06;
  --accent-background: linear-gradient(to top, --main-color, white);
}

In this example, it is unclear if white should be a fallback for the --main-color variable or an argument of the linear-gradient function. I think we should report this violation but not attempt to autofix it.

(Spec for reference)

Any thoughts or can I label this ready to implement?

[hudochenkov]

Great summary, @jeddy3!

I think function-var-no-invalid is misleading, because there are no var to be invalid. What about function-var-no-missing? We have no-missing rules: font-family-no-missing-generic-family-keyword, no-missing-end-of-source-newline.

[alexander-akait]

Also we have env() with similar features

[jeddy3]

What about function-var-no-missing

I originally thought of *-no-missing, but the two no-missing rules we have are for user preferences. A font-family declaration without a generic family keyword and a source without a newline at the end are both valid CSS.

The examples in the original request are invalid CSS, more akin to color-no-invalid-hex. It's something a CSS validator would also flag.

Also we have env() with similar features

Good catch. There's more to unpack here.

We're talking about dashed-idents, which are a specialised form of custom-idents. They're used in at least the following places:

• custom properties
• environmental variables
• custom media queries
• custom selectors
• color profiles

And it'll likely be used in more as it's the perferred syntax for authored custom idents.

Example:

@color-profile --my-profile { src: url(https://example.com/foo.icc); }
@custom-media --narrow-window (max-width: 30em);
@custom-selector :--heading h1, h2, h3, h4, h5, h6;

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

@media (--narrow-window) {
  :--heading {
    color: color(--my-profile 1 0 .5 / .2);
    background: var(--my-color);
    margin: env(--my-margin);
  }
}

Both the environmental variables spec and the custom properties spec talk about defining, which is when a dashed-ident is created, and referencing, which is when a previously defined dashed-ident is used. We're looking at the latter here.

A dashed-ident can be referenced in either a declaration-value, at-rule or selector. We can either have separate rules for these or combine them into a single rule. My preference would be separate rules.

• declaration-value-dashed-ident-no-invalid-reference
• at-rule-dashed-ident-no-invalid-reference
• selector-dashed-ident-no-invalid-reference
• and so on

The former would catch:

.foo {
  color: --my-profile 1 0 .5 / .2; /* previously color(...) */
  background: --my-color; /* previously var(...) */
  margin: --my-margin; /* previously env(...) */
}

And also this because custom properties have declaration values:

:root {
  --my-prop: --my-color; /* previously var(...) */
}

And the latter two would catch:

@media --narrow-window {} /* previously @media (--narrow-window) {} */
@custom-selector --heading h1, h2, h3, h4, h5, h6; /* previously @custom-selector :--heading h1, h2, h3, h4, h5, h6; */

We can put the latter two on the backburner for now and focus on the former as it addresses the original request.

We can't add autofix to this rule because a dashed-ident can be referenced by var(), env(), color() and so on inside a declaration value.

Revised blueprint:

• Name: declaration-value-dashed-ident-no-invalid-reference
• Primary option: true
• Secondary options: None
• Autofixable: No
• Message: Unexpected invalid dashed-ident reference
• Description: "Disallow invalid dashed-ident references within declaration values"
• Section: "Possible errors" -> "Declaration value"

How does that look?

[jeddy3]

As there are no objections to the above blueprint, I'll label this as ready to implement.

[alexander-akait]

@jeddy3 I think better to name it identifier-pattern, because --test is valid identifier, but doing nothing, but you can use JS to get it 😄

[jeddy3]

Are you saying that the following is valid CSS because a dashed-ident doesn't have to be referenced?:

a { prop: --test }

If so, then we can loop back to @hudochenkov suggestion of using no-missing as it's a preference (that most users will want to enforce, like with font-family-no-missing-generic-family-keyword). We can, as we similarly do with that rule, add an ignoreDashedIdents: [] option if the need arises.

For example:

"rules": {
  "declaration-value-dashed-ident-no-missing-reference-function": [true, {
    "ignoreDashedIdents": ["my-js-ident"]
  }]
}

With:

a {
  color: --my-color;
  prop: --my-js-ident;
}

Would only produce one violation:

2:9 error Unexpected missing dashed-ident reference function for "--my-color"

Revised blueprint:

• Name: declaration-value-dashed-ident-no-missing-reference-function (where declaration-value-dashed-ident is the thing being targeted and no-missing-reference-function is what is being checked).
• Primary option: true
• Secondary options: None (for now, but ignoreDashedIdents: [] if needed in future)
• Autofixable: No
• Message: Unexpected missing dashed-ident reference function for "{dashed-ident}"
• Description: "Disallow missing dashed-ident reference functions within declaration values"
• Section: "Possible errors" -> "Declaration value"

These dashed-idents are a fascinating can of worms. As such I suggest we keep this rule narrowly scoped, and I believe the above blueprint does that.

[alexander-akait]

Still found declaration-value-dashed-ident-no-missing-reference-function is very misleading, maybe declaration-value-identifier-pattern, so you can ban --test here, it is sounds weird for DX, but that would be true from a spec point of view

[jeddy3]

Initial rule done in #5317