feat: no-invalid-property-values rule

[nzakas]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request?

Add a rule to validate property values. This uses the CSS spec to check every value of every property to ensure it's correct and expected.

What changes did you make? (Give an overview)

• Added no-invalid-property-values rule
• Added tests
• Added docs
• Updated README
• Added no-invalid-property-values rule to recommended config

Related Issues

Refs #6

Is there anything you'd like reviewers to focus on?

I'm a bit torn as to whether to combine this rule with no-unknown-properties, because lexer.matchDeclaration will also check for unknown properties. I'd like some opinions on this. Here's what that looks like:

#11

cc @akulsr0

[nzakas]

The types for the css-tree package are messed up and causing the build to fail even though the code runs fine. I'll address that next week.

[akulsr0]

@nzakas I would prefer it to be not combined with no-unknown-properties rule. Keeping both rules easily configurable independently.

[jeddy3]

I'm a bit torn as to whether to combine this rule with no-unknown-properties, because lexer.matchDeclaration will also check for unknown properties. I'd like some opinions on this.

It may be worth considering if the syntax can be customised and where the user can do that.

There'll be duplication in the configuration file if the rules are separate and the syntax can be customised via the rules' options. For example, if a user wants to use the non-spec size property, they'll need to configure "size" twice:

export default [
    {
        rules: {
            "no-unknown-properties": ["error", { "ignoreProperty": ["size"] }],
            "no-invalid-property-values": ["error", { "propertiesSyntax": { "size": "<length-percentage>" } }]
        }
    }
];

We have this problem in Stylelint. We added our declaration-property-value-no-unknown rule (also using CSSTree) after our property-no-unknown one (using known-css-properties) was already used by people.

To avoid the duplication, you could combine the rules. For example:

export default [
    {
        rules: {
            "no-invalid-declarations": ["error", { "propertiesSyntax": { "size": "<length-percentage>" } }]
        }
    }
];

Or you could keep the rules separate and allow customisation of the syntax in languageOptions. For example:

export default [
    {
        languageOptions: {
            "propertiesSyntax": { "size": "<length-percentage>" }
        },
        rules: {
            "no-unknown-properties": "error",
            "no-invalid-property-values": "error"
        }
    }
];

Whichever approach you take, you may want to use the same one for your no-unknown-at-rules rule as CSSTree can validate the preludes of some at-rules. For example, it'll flag foo because it expects a <custom-property-name> for the prelude of @property:

@property foo {}

It can also validate descriptors. For example, it'll flag margin as it's not a valid descriptor for @property:

@property --foo {
  margin: 10px;
}

We used the stylelint-csstree-validator custom rule as a blueprint when we added our built-in rule to check property values. The custom rule checks at-rule names, preludes and descriptors, and properties and their values while providing options for extending property and at-rule syntax. You may also find it a useful reference if you've not seen it already.

[nzakas]

@akulsr0 the problem is that we are duplicating effort with two rules. Even though I'm not reporting unknown properties in this rule, the check is still occurring (I'm just ignoring the result). We can certainly add options to allow people to configure whether they want to ignore unknown rules or not.

@jeddy3 thanks, that's very helpful. And yes, I agree that the same pattern should be followed for at rules.

[nzakas]

Closing in favor of #11.