stylelint
Add end positions to warnings
Avoid errors:
- [x] at-rule-no-unknown
- [x] block-no-empty
- [x] color-no-invalid-hex
- [x] comment-no-empty
- [x] custom-property-no-missing-var-function
- [x] declaration-block-no-duplicate-custom-properties
- [x] declaration-block-no-duplicate-properties
- [x] declaration-block-no-shorthand-property-overrides
- [x] font-family-no-duplicate-names
- [x] font-family-no-missing-generic-family-keyword
- [x] function-calc-no-unspaced-operator
- [x] function-linear-gradient-no-nonstandard-direction
- [x] function-no-unknown
- [x] keyframe-declaration-no-important
- [x] media-feature-name-no-unknown
- [x] named-grid-areas-no-invalid
- [x] no-descending-specificity
- [x] no-duplicate-at-import-rules
- [x] no-duplicate-selectors
- [x] no-empty-source
- [x] no-invalid-double-slash-comments
- [x] no-invalid-position-at-import-rule
- [x] property-no-unknown
- [x] selector-pseudo-class-no-unknown
- [x] selector-pseudo-element-no-unknown
- [x] selector-type-no-unknown
- [x] string-no-newline
- [x] unit-no-unknown
Enforce conventions:
- [x] alpha-value-notation
- [x] at-rule-allowed-list
- [x] at-rule-disallowed-list
- [x] at-rule-no-vendor-prefix
- [x] at-rule-property-required-list
- [x] color-function-notation
- [x] color-hex-alpha
- [x] color-hex-length
- [x] color-named
- [x] color-no-hex
- [x] comment-pattern
- [x] comment-word-disallowed-list
- [x] custom-media-pattern
- [x] custom-property-pattern
- [x] declaration-block-no-redundant-longhand-properties
- [x] declaration-block-single-line-max-declarations
- [x] declaration-no-important
- [x] declaration-property-unit-allowed-list
- [x] declaration-property-unit-disallowed-list
- [x] declaration-property-value-allowed-list
- [x] declaration-property-value-disallowed-list
- [x] font-family-name-quotes
- [x] font-weight-notation
- [x] function-allowed-list
- [x] function-disallowed-list
- [x] function-url-no-scheme-relative
- [x] function-url-quotes
- [x] function-url-scheme-allowed-list
- [x] function-url-scheme-disallowed-list
- [x] hue-degree-notation
- [x] keyframes-name-pattern
- [x] length-zero-no-unit
- [x] max-nesting-depth
- [x] media-feature-name-allowed-list
- [x] media-feature-name-disallowed-list
- [x] media-feature-name-no-vendor-prefix
- [x] media-feature-name-value-allowed-list
- [x] no-unknown-animations
- [x] number-max-precision
- [x] property-allowed-list
- [x] property-disallowed-list
- [x] property-no-vendor-prefix
- [x] rule-selector-property-disallowed-list
- [x] selector-attribute-name-disallowed-list
- [x] selector-attribute-operator-allowed-list
- [x] selector-attribute-operator-disallowed-list
- [x] selector-attribute-quotes
- [x] selector-class-pattern
- [x] selector-combinator-allowed-list
- [x] selector-combinator-disallowed-list
- [x] selector-disallowed-list
- [x] selector-id-pattern
- [x] selector-max-attribute
- [x] selector-max-class
- [x] selector-max-combinators
- [x] selector-max-compound-selectors
- [x] selector-max-id
- [ ] selector-max-pseudo-class
- [ ] selector-max-specificity
- [ ] selector-max-type
- [ ] selector-max-universal
- [ ] selector-nested-pattern
- [ ] selector-no-qualifying-type
- [ ] selector-no-vendor-prefix
- [x] selector-not-notation
- [ ] selector-pseudo-class-allowed-list
- [ ] selector-pseudo-class-disallowed-list
- [ ] selector-pseudo-element-allowed-list
- [x] selector-pseudo-element-colon-notation
- [ ] selector-pseudo-element-disallowed-list
- [ ] shorthand-property-no-redundant-values
- [ ] time-min-milliseconds
- [x] unit-allowed-list
- [x] unit-disallowed-list
- [x] value-no-vendor-prefix
What steps are needed to reproduce the bug?
Run Stylelint! It doesn't provide enough information back to plugins in order for them to effectively show warnings in editors:
[Debug - 10:13:21 p.m.] [language-server] Lint run complete | uri: "file:///c%3A/Users/SOMEPLACEONMYMACHINE/Icon/icon.scss" results: {"diagnostics":[{"range":{"start":{"line":36,"character":18},"end":{"line":36,"character":18}},"message":"Expected list.index instead of index (scss/no-global-function-names)","severity":1,"code":"scss/no-global-function-names","source":"Stylelint"},{"range":{"start":{"line":19,"character":16},"end":{"line":19,"character":16}},"message":"Unexpected named color \"pink\" (color-named)","severity":1,"code":"color-named","source":"Stylelint","codeDescription":{"href":"https://stylelint.io/user-guide/rules/color-named"}},{"range":{"start":{"line":72,"character":0},"end":{"line":72,"character":0}},"message":"Expected selector \"svg\" to come before selector \".atom-icon svg\" (no-descending-specificity)","severity":1,"code":"no-descending-specificity","source":"Stylelint","codeDescription":{"href":"https://stylelint.io/user-guide/rules/no-descending-specificity"}},{"range":{"start":{"line":72,"character":0},"end":{"line":72,"character":0}},"message":"Expected selector \"svg\" to come before selector \".atom-icon.rounded svg\" (no-descending-specificity)","severity":1,"code":"no-descending-specificity","source":"Stylelint","codeDescription":{"href":"https://stylelint.io/user-guide/rules/no-descending-specificity"}}]}
Please note in the debug log above, the start and end line characters are the same, despite this being an entire word causing the error. This means errors in editors can only show a single character error underline:
Which in turn means an editor cannot show a dialog with error information. See discussion here from the dev over at vscode-stylelint.
What Stylelint configuration is needed to reproduce the bug?
{
"plugins": [
"stylelint-order",
"stylelint-scss",
"stylelint-prettier"
],
"rules": {
"prettier/prettier": true,
"order/properties-alphabetical-order": true,
"at-rule-no-unknown": null,
"scss/at-rule-no-unknown": null,
"declaration-bang-space-before": "always",
"declaration-bang-space-after": "never",
"color-named": "never",
"color-no-hex": [
true,
{
"message": "Please use a color variable"
}
],
"declaration-block-no-duplicate-properties": true,
"rule-empty-line-before": [
"always-multi-line",
{
"except": [
"after-single-line-comment",
"inside-block-and-after-rule",
"first-nested"
],
"ignore": [
"after-comment",
"inside-block"
]
}
],
"color-hex-length": "short",
"color-hex-case": "lower",
"no-missing-end-of-source-newline": true,
"block-no-empty": true,
"color-no-invalid-hex": true,
"declaration-no-important": true,
"number-leading-zero": "always",
"no-duplicate-selectors": true,
"max-nesting-depth": 6,
"unit-allowed-list": [
[
"%",
"em",
"vh",
"vw",
"s",
"ms",
"deg",
"units.px-to-rem"
],
{
"message": "Don't use px values (spacing() / px-to-rem()) function"
}
],
"selector-pseudo-element-colon-notation": "double",
"selector-no-qualifying-type": true,
"selector-class-pattern": "^([a-z]+\\-?[a-z]+?)*",
"shorthand-property-no-redundant-values": true,
"declaration-block-semicolon-newline-after": "always-multi-line",
"selector-list-comma-newline-after": "always",
"function-comma-space-after": "always",
"declaration-colon-space-after": "always",
"block-opening-brace-newline-before": "always-single-line",
"block-opening-brace-space-before": "always",
"function-parentheses-space-inside": "never",
"selector-attribute-quotes": "always",
"declaration-block-trailing-semicolon": "always",
"no-eol-whitespace": true,
"number-no-trailing-zeros": true,
"function-url-quotes": "always",
"property-no-vendor-prefix": true,
"selector-no-vendor-prefix": true,
"media-feature-name-no-vendor-prefix": true,
"at-rule-no-vendor-prefix": true,
"value-no-vendor-prefix": true,
"length-zero-no-unit": true,
"scss/at-import-no-partial-leading-underscore": true,
"scss/selector-no-redundant-nesting-selector": true,
"scss/dollar-variable-pattern": [
"^([a-z]+\\-?)+?$",
{
"message": "Please use lowercase, dashcase for var names"
}
],
"scss/at-mixin-pattern": [
"^([a-z]+\\-?[a-z]+?)*$",
{
"message": "Please use hyhenated_lowercase for mixin name"
}
],
"scss/at-function-pattern": [
"^([a-z]+\\-?[a-z]+?)*",
{
"message": "Please use hyhenated_lowercase for function name"
}
],
"scss/percent-placeholder-pattern": [
"^([a-z]+\\-?[a-z]+?)*",
{
"message": "Please use hyhenated_lowercase for placeholder name"
}
],
"comment-no-empty": true,
"number-max-precision": 3,
"function-comma-newline-after": "never-multi-line",
"function-comma-space-before": "never",
"function-linear-gradient-no-nonstandard-direction": true,
"function-calc-no-unspaced-operator": true,
"value-list-comma-newline-after": "always-multi-line",
"function-comma-newline-before": "never-multi-line",
"value-list-comma-newline-before": "never-multi-line",
"value-list-comma-space-after": "always-single-line",
"value-list-comma-space-before": "never-single-line",
"declaration-colon-newline-after": null,
"declaration-colon-space-before": "never",
"declaration-block-semicolon-newline-before": "never-multi-line",
"declaration-block-semicolon-space-after": "always-single-line",
"declaration-block-semicolon-space-before": "never",
"block-closing-brace-newline-after": "always",
"block-closing-brace-newline-before": "always-multi-line",
"block-opening-brace-newline-after": "always",
"block-opening-brace-space-after": "always-single-line",
"selector-combinator-space-after": "always",
"selector-combinator-space-before": "always",
"selector-list-comma-newline-before": "never-multi-line",
"selector-list-comma-space-after": "always-single-line",
"selector-list-comma-space-before": "never",
"media-feature-colon-space-after": "always",
"media-feature-colon-space-before": "never",
"media-feature-range-operator-space-after": "always",
"media-feature-range-operator-space-before": "never",
"media-query-list-comma-newline-after": "always-multi-line",
"media-query-list-comma-newline-before": "never-multi-line",
"media-query-list-comma-space-after": "always-single-line",
"media-query-list-comma-space-before": "never",
"comment-empty-line-before": [
"always",
{
"ignore": [
"stylelint-commands"
]
}
],
"declaration-block-no-redundant-longhand-properties": true,
"declaration-block-no-shorthand-property-overrides": true,
"font-family-no-duplicate-names": true,
"keyframe-declaration-no-important": true,
"media-feature-name-no-unknown": true,
"no-empty-source": true,
"no-extra-semicolons": true,
"no-invalid-double-slash-comments": true,
"property-no-unknown": true,
"selector-pseudo-class-no-unknown": true,
"selector-pseudo-element-no-unknown": true,
"selector-type-no-unknown": true,
"string-no-newline": true
}
}
How did you run Stylelint?
vscode-stylelint & CLI
Which version of Stylelint are you using?
13.12.0 & 14.0.1
What did you expect to happen?
Full start and end word & line information should be passed from stylelint AST parser and back to consuming resources
What actually happened?
Nothing, nada, no dice!
Does the bug relate to non-standard syntax?
SCSS (although I assume all supported syntax)
Proposal to fix the bug
No response
@ChillyBots Thanks for the report. I've edited the description to use 3 backticks for clarity.
@ybiquitous This isn't really a bug; I think this should be a feature request. The original discussion is in https://github.com/stylelint/vscode-stylelint/issues/315, but I'll summarize here.
Stylelint's API only returns a single position for warnings, unlike, for example, ESLint (see endLine and endColumn). This means that, in the extension, we're only sending diagnostics for a single character. This leads to issues such as difficulty trying to see error information by hovering over the problem, and also makes problems less clear.
Additionally, we can't supply fixes per problem since Stylelint doesn't provide fix edit information per warning, unlike ESLint.
So it's not really a bug per se, more a feature request to supply ranges instead of single positions for warnings when possible, and also to provide edits for individual fixes.
@adalinesimonian Thank you for the helpful summary! I understand it well. 😄
Since Stylelint relies on PostCSS, and PostCSS does not have range support, we won't be able to add end position information since the extra data would be ignored by the PostCSS result warn method and other methods. So, if we want to move forward on this, we'll need support for the additional information upstream.
I've opened a PR for PostCSS which adds support for ranges: https://github.com/postcss/postcss/pull/1669
The upstream PR has been merged and will most likely make it into PostCSS 8.3 per https://github.com/postcss/postcss/pull/1669#issuecomment-971051056. Once released, we'll be able to make the necessary changes to support ranges in Stylelint!
Range support in PostCSS has been released in 8.4. End indices are needed in postcss-value-parser, a PR has been opened at TrySound/postcss-value-parser#83
I've updated the task list in the first comment to reflect which rules have been updated.
Anyone is now welcome, if they like, to pick a rule and update it to use ranges.
As mentioned in https://github.com/stylelint/stylelint/pull/5725#pullrequestreview-937287164, you don't need to add end positions to every reject test. Add just enough to be confident the extra endIndex logic is working.
Looking to knock more of these out!
Just to confirm, I looked at custom-property-no-missing-var-function , font-family-no-duplicate-names, and font-family-no-missing-generic-family-keyword, and it seems like they support end positions even though they're not ticked - should I add a checkbox there? Or is there another preferred practice?
https://github.com/stylelint/stylelint/blob/6317202890fb8915e8fec661decd8252ce885776/lib/rules/custom-property-no-missing-var-function/index.js#L53-L60
https://github.com/stylelint/stylelint/blob/6317202890fb8915e8fec661decd8252ce885776/lib/rules/font-family-no-duplicate-names/index.js#L106-L114
https://github.com/stylelint/stylelint/blob/6317202890fb8915e8fec661decd8252ce885776/lib/rules/font-family-no-missing-generic-family-keyword/index.js#L105-L112
and it seems like they support end positions even though they're not ticked - should I add a checkbox there?
Yes, please. It's just an oversight that they're not already ticked.
Question for unit-no-unknown (let me know if I should move this somewhere else):
My understanding of the current behaviour is unit-no-unknown's start index is at the value, not the unit: for example,
// https://github.com/stylelint/stylelint/blob/ecc1ec25c73ed41d7bcd1e0c8950fe680ac43467/lib/rules/unit-no-unknown/__tests__/index.js#L258
{
code: 'a { font-size: .5remm; }',
/** ↑
* this is column 16 */
message: messages.rejected('remm'),
line: 1,
column: 16,
},
My perspective says we should just be highlighting the unit, not the value (i.e. the start, end indices of the unit). The description in the rule also says:
a { width: 100pixels; }
/** ↑
* These units */
Any thoughts on the above? Happy to change where the error is reported, though that will change all of the reported indices for the rule. Another suggestion could be to report the start index of the value and the end index of the unit.
Any thoughts on the above? Happy to change where the error is reported, though that will change all of the reported indices for the rule. Another suggestion could be to report the start index of the value and the end index of the unit.
As you suggested, I think it seems to make sense to highlight just a unit.
Looking to knock as many of these out as possible so I can enable the vscode-stylelint team to add better warning support!
As I go through the list of rules, I have some questions on exactly what the start and end should be. Off the top of my head:
-
comment-no-empty- should I include the comment delimiters (ex/*and*/), or not? if not, how would I handle an empty comment? -
no-empty-source- what exactly would I highlight here? - for
string-no-newline, would highlighting the entire string be reasonable? or should I "focus" on the newline somehow?
Looking to knock as many of these out as possible
Fantastic, thank you!
comment-no-empty- should I include the comment delimiters (ex /* and */), or not?
Let's include the markers.
no-empty-source- what exactly would I highlight here?
Perhaps we skip this one as a start and endIndex doesn't make sense.
for
string-no-newline, would highlighting the entire string be reasonable? or should I "focus" on the newline somehow?
Yes, I think highlighting the entire string would be reasonable.
Hi @mattxwang, I can take on parts of the remaining rules. If you've been starting some rules, could you tell me them?
Hi @mattxwang, I can take on parts of the remaining rules. If you've been starting some rules, could you tell me them?
I'm not in-progress on any of them - feel free to take anything you'd like!
I've been struggling with (and have not made any progress on) comment-no-empty, if you have any thoughts on that!
I've been struggling with (and have not made any progress on)
comment-no-empty, if you have any thoughts on that!
It seems comment-no-empty has supported correct end positions already, when I see this demo: 🤔
I've been struggling with (and have not made any progress on)
comment-no-empty, if you have any thoughts on that!It seems
comment-no-emptyhas supported correct end positions already, when I see this demo: 🤔
Ah! I feel ridiculous, I must've just not been writing the test cases properly 🥲 . Feel free to check it off.
I think this weekend is probably going to be busy for me, so feel free to take whatever end positions you want. In most settings I'll "finish" a rule in one sitting, so it's unlikely you'll interrupt any in-progress work on my end!
I've completed all the tasks! 🎉 If we miss something else, let's create another issue or pull request!
@jeddy3 @mattxwang @Mouvedia Thank you so much for writing or reviewing the code! 😊
Legendary effort y'all! I'm really looking forward to this, it's a huge improvement. Thanks a ton.
I've completed all the tasks! 🎉
Great work wrapping this up @ybiquitous! Apologies for not being more of a help - work really has picked up recently 😓
@adalinesimonian, what else can we do to help support vscode-stylelint here? I recall a couple of comments about other work to enable error reporting similar to what ESLint provides, but I can't seem to find it now - is it providing edit info per warning?
Edit: I see there's some activity in https://github.com/stylelint/vscode-stylelint/pull/358, apologies for the ping then!
@ybiquitous FYI here's a list of the rules that were missing from the OP:
- word
-
annotation-no-unknown -
keyframe-block-no-duplicate-selectors -
keyframe-selector-notation - [x]
no-irregular-whitespace
-
- [x]
import-notation - [x]
declaration-property-max-values - ~
unicode-bom~
@Mouvedia Thanks for finding what I missed! When I re-check them, the following rules need to be addressed:
-
no-irregular-whitespace -
declaration-property-max-values
I think the other rules have been fixed already. 🤔