Skip to content
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.

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

Fix declaration-property-value-no-unknown false negatives for typed custom properties #7078

5 changes: 5 additions & 0 deletions .changeset/nice-toes-reflect.md
@@ -0,0 +1,5 @@
---
"stylelint": patch
---

Fixed: `declaration-property-value-no-unknown` false negatives for typed custom properties
103 changes: 102 additions & 1 deletion lib/rules/declaration-property-value-no-unknown/__tests__/index.mjs
@@ -1,3 +1,5 @@
import { stripIndent } from 'common-tags';

import rule from '../index.js';
const { messages, ruleName } = rule;

Expand Down Expand Up @@ -52,12 +54,81 @@ testRule({
code: 'a { font-size: max(1rem, 3rem); }',
},
{
code: `
code: stripIndent`
a { font-weight: bolder; }
@font-face { font-weight: 100 200; }
`,
description: 'at-rule descriptor and property with the same name but different syntaxes',
},
{
code: stripIndent`
a { --foo: red; }

@property --foo {
syntax: "<color>";
inherits: false;
initial-value: #c0ffee;
}
`,
},
{
code: stripIndent`
a { --foo: 10px; }
a { --foo: 10%; }

@property --foo {
syntax: "<length> | <percentage>";
inherits: false;
initial-value: 10px;
}
`,
},
{
code: stripIndent`
a { --foo: bigger; }
a { --foo: BIGGER; }

@property --foo {
syntax: "big | bigger | BIGGER";
inherits: false;
initial-value: big;
}
`,
},
{
code: stripIndent`
a { --foo: 10px; }
a { --foo: 10px 10vh; }

@property --foo {
syntax: "<length>+";
inherits: false;
initial-value: 10px;
}
`,
},
{
code: stripIndent`
a { --foo: red; }
a { --foo: 10px 10vh; }

@property --foo {
syntax: "*";
inherits: false;
initial-value: 10px;
}
`,
},
{
code: stripIndent`
a { --foo: var(--bar); }
@property --foo {
syntax: "<color>";
inherits: false;
initial-value: #c0ffee;
}
`,
},
],

reject: [
Expand Down Expand Up @@ -149,6 +220,36 @@ testRule({
endLine: 1,
endColumn: 30,
},
{
code: stripIndent`
a { --foo: 10px; }
@property --foo {
syntax: "<color>";
inherits: false;
initial-value: #c0ffee;
}
`,
message: messages.rejected('--foo', '10px'),
line: 1,
column: 12,
endLine: 1,
endColumn: 16,
},
{
code: stripIndent`
a { --foo: 10px; }
@property --foo {
syntax: not-a-string;
inherits: false;
initial-value: #c0ffee;
}
`,
message: messages.rejected('syntax', 'not-a-string'),
line: 3,
column: 10,
endLine: 3,
endColumn: 22,
},
],
});

Expand Down
69 changes: 60 additions & 9 deletions lib/rules/declaration-property-value-no-unknown/index.js
@@ -1,7 +1,7 @@
'use strict';

const { isPlainObject } = require('is-plain-object');
const { fork, parse, find } = require('css-tree');
const { fork, parse, find, string } = require('css-tree');

const declarationValueIndex = require('../../utils/declarationValueIndex');
const matchesStringOrRegExp = require('../../utils/matchesStringOrRegExp');
Expand All @@ -13,7 +13,7 @@ const isCustomProperty = require('../../utils/isCustomProperty');
const isStandardSyntaxValue = require('../../utils/isStandardSyntaxValue');
const isStandardSyntaxProperty = require('../../utils/isStandardSyntaxProperty');
const isStandardSyntaxDeclaration = require('../../utils/isStandardSyntaxDeclaration');
const { isAtRule } = require('../../utils/typeGuards');
const { isAtRule, isDeclaration } = require('../../utils/typeGuards');
const { isRegExp, isString } = require('../../utils/validateTypes');
const { nestingSupportedAtKeywords } = require('../../reference/atKeywords');

Expand All @@ -29,6 +29,8 @@ const meta = {
url: 'https://stylelint.io/user-guide/rules/declaration-property-value-no-unknown',
};

const SYNTAX_PROPERTY = /^syntax$/i;

/** @type {import('stylelint').Rule} */
const rule = (primary, secondaryOptions) => {
return (root, result) => {
Expand All @@ -51,9 +53,7 @@ const rule = (primary, secondaryOptions) => {
return;
}

const ignoreProperties = Array.from(
Object.entries((secondaryOptions && secondaryOptions.ignoreProperties) || {}),
);
const ignoreProperties = Array.from(Object.entries(secondaryOptions?.ignoreProperties ?? {}));

/** @type {(name: string, propValue: string) => boolean} */
const isPropIgnored = (name, value) => {
Expand All @@ -63,8 +63,47 @@ const rule = (primary, secondaryOptions) => {
return valuePattern && matchesStringOrRegExp(value, valuePattern);
};

const propertiesSyntax = (secondaryOptions && secondaryOptions.propertiesSyntax) || {};
const typesSyntax = (secondaryOptions && secondaryOptions.typesSyntax) || {};
const propertiesSyntax = {
// Take a shallow clone as this object will be appended to.
...(secondaryOptions?.propertiesSyntax ?? {}),
};
const typesSyntax = secondaryOptions?.typesSyntax ?? {};

/** @type {Map<string, string>} */
const typedCustomPropertyNames = new Map();
romainmenke marked this conversation as resolved.
Show resolved Hide resolved

root.walkAtRules(/^property$/i, (atRule) => {
const propName = atRule.params.trim();

if (!propName || !atRule.nodes || !isCustomProperty(propName)) return;

for (const node of atRule.nodes) {
if (isDeclaration(node) && SYNTAX_PROPERTY.test(node.prop)) {
const value = node.value.trim();
const unquoted = string.decode(value);

// Only string values are valid.
// We can not check the syntax of this property.
if (unquoted === value) continue;

// Any value is allowed in this custom property.
// We don't need to check this property.
if (unquoted === '*') continue;

// https://github.com/csstree/csstree/pull/256
// We can circumvent this issue by prefixing the property name,
// making it a vendor-prefixed property instead of a custom property.
// No one should be using `-stylelint--` as a property prefix.
//
// When this is resolved `typedCustomPropertyNames` can become a `Set<string>`
// and the prefix can be removed.
const prefixedPropName = `-stylelint${propName}`;

typedCustomPropertyNames.set(propName, prefixedPropName);
propertiesSyntax[prefixedPropName] = unquoted;
}
}
});

const forkedLexer = fork({
properties: propertiesSyntax,
Expand All @@ -84,10 +123,22 @@ const rule = (primary, secondaryOptions) => {

if (!isStandardSyntaxValue(value)) return;

if (isCustomProperty(prop)) return;
if (isCustomProperty(prop) && !typedCustomPropertyNames.has(prop)) return;

if (isPropIgnored(prop, value)) return;

// https://github.com/mdn/data/pull/674
// `initial-value` has an incorrect syntax definition.
// In reality everything is valid.
if (
/^initial-value$/i.test(prop) &&
decl.parent &&
isAtRule(decl.parent) &&
/^property$/i.test(decl.parent.name)
) {
return;
}

/** @type {import('css-tree').CssNode} */
let cssTreeValueNode;

Expand All @@ -114,7 +165,7 @@ const rule = (primary, secondaryOptions) => {
const { error } =
parent && isAtRule(parent) && !nestingSupportedAtKeywords.has(parent.name.toLowerCase())
? forkedLexer.matchAtruleDescriptor(parent.name, prop, cssTreeValueNode)
: forkedLexer.matchProperty(prop, cssTreeValueNode);
: forkedLexer.matchProperty(typedCustomPropertyNames.get(prop) ?? prop, cssTreeValueNode);

if (!error) return;

Expand Down