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
Add autofix to selector-attribute-quotes #5248
Add autofix to selector-attribute-quotes #5248
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@doing-art Thank you for the pull request. It's almost there.
Can we add a test with a comment within a selector list please, e.g.:
{
code: 'a[target="_blank"], /* comment */ a { }',
fixed: 'a[target=_blank], /* comment */ a { }',
message: messages.rejected('_blank'),
line: 1,
column: 10,
},
It'll fail because comments are stripped for the selector
property of the AST. The full string is available in the raws
property, and we'll want to pass this full string to the selector parser. (Ref).
Let's create a getRuleSelector
util based off getDeclarationValue
and use it in the rule.
@@ -36,25 +38,41 @@ function rule(expectation) { | |||
} | |||
|
|||
parseSelector(ruleNode.selector, result, ruleNode, (selectorTree) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parseSelector(ruleNode.selector, result, ruleNode, (selectorTree) => { | |
parseSelector(getRuleSelector(ruleNode), result, ruleNode, (selectorTree) => { |
We'll need a new util here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@doing-art Thank you for the pull request. It's almost there.
Can we add a test with a comment within a selector list please, e.g.:
{ code: 'a[target="_blank"], /* comment */ a { }', fixed: 'a[target=_blank], /* comment */ a { }', message: messages.rejected('_blank'), line: 1, column: 10, },
It'll fail because comments are stripped for the
selector
property of the AST. The full string is available in theraws
property, and we'll want to pass this full string to the selector parser. (Ref).Let's create a
getRuleSelector
util based offgetDeclarationValue
and use it in the rule.
@jeddy3 Thanks for reviewing the PR. I made the requested change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, seems like a comprehensive set of test cases to me!
Changelog entry added:
|
Have added autofix to
selector-attribute-quotes
rule as dicussed in issue #5211Some of the unit tests can be a bit confusing. They cover pretty unusual but valid selectors: