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 false positives for maps in property-no-unknown #5690

Merged
merged 9 commits into from Nov 13, 2021

Conversation

strigefleur
Copy link
Contributor

Which issue, if any, is this issue related to?

Partial fix #5654

Is there anything in the PR that needs further explanation?

No, it's self-explanatory.

@strigefleur strigefleur changed the title Fix false positives for maps in property-no-unknown (#5654) Fix false positives for maps in property-no-unknown Nov 4, 2021
@strigefleur
Copy link
Contributor Author

Was going to split into separate PRs but then ended up pushing in the same branch.
Hope that's fine.
In it's current state should fix #5654 whole.

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

@strigefleur Thank you for creating this PR! 😄

I've suggested some refactoring ideas, but almost LGTM. 👍🏼

lib/utils/isStandardSyntaxDeclaration.js Outdated Show resolved Hide resolved
lib/utils/__tests__/isStandardSyntaxHexColor.test.js Outdated Show resolved Hide resolved
lib/utils/isStandardSyntaxHexColor.js Outdated Show resolved Hide resolved
@strigefleur
Copy link
Contributor Author

Agreed on every point :)
There's lot of function (property) syntax already so probably one day someone will refactor it as well.

strigefleur and others added 3 commits November 6, 2021 12:54
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

@strigefleur Thank you. LGTM! 😄

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request.

I've requested some changes.

lib/utils/isStandardSyntaxHexColor.js Outdated Show resolved Hide resolved
lib/utils/isStandardSyntaxHexColor.js Outdated Show resolved Hide resolved
lib/utils/isStandardSyntaxHexColor.js Outdated Show resolved Hide resolved
Comment on lines 64 to 67
{
code: 'a { color: #colors[somecolor]; }',
description: 'Less map usage',
},
Copy link
Member

@jeddy3 jeddy3 Nov 8, 2021

Choose a reason for hiding this comment

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

Let's move this into its own testRule case (towards the bottom of the file), which sets the custom syntax to postcss-less as testRule without syntax should only include vanilla CSS constructs.

testRule({
	ruleName,
	config: [true],
	customSyntax: 'postcss-less',
	accept: [
		{
			code: 'a { color: #colors[somecolor]; }',
			description: 'Less map usage',
		},
	],
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Sure, as soon as I have at least some time.

strigefleur and others added 3 commits November 10, 2021 14:00
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

(I move the test case myself).

@jeddy3 jeddy3 merged commit 172c92d into stylelint:main Nov 13, 2021
@jeddy3
Copy link
Member

jeddy3 commented Nov 13, 2021

  • Fixed: property-no-unknown false positives for maps (#5690).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix false positives for maps in color-no-invalid-hex and property-no-unknown
3 participants