Skip to content

Commit

Permalink
Fix declaration-property-value-no-unknown false negatives for typed…
Browse files Browse the repository at this point in the history
… custom properties (#7078)


Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
  • Loading branch information
romainmenke and ybiquitous committed Jul 20, 2023
1 parent f383a26 commit a3b8d32
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 10 deletions.
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();

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

0 comments on commit a3b8d32

Please sign in to comment.