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.

Sign up for GitHub

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

Jump to bottom

Fix property-no-unknown false negatives for newer custom syntaxes #6553

Merged
merged 3 commits into from Jan 5, 2023
Merged

Fix property-no-unknown false negatives for newer custom syntaxes #6553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ed733f
fix: consider ruleless declarations as standard
43081j Jan 2, 2023
8323ba8
Add changelog entry
ybiquitous Jan 5, 2023
54a5098
Simplify test case
ybiquitous Jan 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/empty-goats-count.md
View file
Open in desktop
@@ -0,0 +1,5 @@
---
"stylelint": patch
---

Fixed: `property-no-unknown` false negatives for newer custom syntaxes
4 changes: 4 additions & 0 deletions lib/utils/__tests__/isStandardSyntaxDeclaration.test.js
View file
Open in desktop
Expand Up @@ -166,6 +166,10 @@ describe('isStandardSyntaxDeclaration', () => {
true,
);
});

it('supports root-level declarations', () => {
expect(isStandardSyntaxDeclaration(decl('color: yellow;'))).toBe(true);
});
});

function decl(css, parser = postcss) {
Expand Down
21 changes: 1 addition & 20 deletions lib/utils/isStandardSyntaxDeclaration.js
View file
Open in desktop
@@ -1,14 +1,7 @@
'use strict';

const isScssVariable = require('./isScssVariable');
const { isRoot, isRule } = require('./typeGuards');

/**
* @param {string} [lang]
*/
function isStandardSyntaxLang(lang) {
return lang && (lang === 'css' || lang === 'custom-template' || lang === 'template-literal');
}
const { isRule } = require('./typeGuards');

/**
* Check whether a declaration is standard
Expand All @@ -19,18 +12,6 @@ module.exports = function isStandardSyntaxDeclaration(decl) {
const prop = decl.prop;
Copy link
Contributor

@Mouvedia Mouvedia Jan 4, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isStandardSyntaxDeclaration is used in at least 7 rules. Are we sure this is not gonna cause regressions?
Is being more lax already covered by tests?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, tests don't cover. Test using postcss-css-in-js are skipped:

stylelint/lib/rules/property-no-unknown/__tests__/index.js

Lines 287 to 291 in 5e6d46a

testRule({
skip: true,
ruleName,
config: [true],
customSyntax: 'postcss-css-in-js',

const parent = decl.parent;

// Declarations belong in a declaration block or standard CSS source
if (
parent &&
isRoot(parent) &&
parent.source &&
!isStandardSyntaxLang(
/** @type {import('postcss').Source & {lang?: string}} */ (parent.source).lang,
)
) {
return false;
}

// SCSS var; covers map and list declarations
if (isScssVariable(prop)) {
return false;
Expand Down