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

Add end positions to warnings #5694

Closed
ChillyBots opened this issue Nov 5, 2021 · 26 comments
Closed

Add end positions to warnings #5694

ChillyBots opened this issue Nov 5, 2021 · 26 comments
Assignees
Labels
good first issue is good for newcomers status: wip is being worked on by someone type: umbrella a group of related issues

Comments

@ChillyBots
Copy link

ChillyBots commented Nov 5, 2021

Avoid errors:

  • at-rule-no-unknown
  • block-no-empty
  • color-no-invalid-hex
  • comment-no-empty
  • custom-property-no-missing-var-function
  • declaration-block-no-duplicate-custom-properties
  • declaration-block-no-duplicate-properties
  • declaration-block-no-shorthand-property-overrides
  • font-family-no-duplicate-names
  • font-family-no-missing-generic-family-keyword
  • function-calc-no-unspaced-operator
  • function-linear-gradient-no-nonstandard-direction
  • function-no-unknown
  • keyframe-declaration-no-important
  • media-feature-name-no-unknown
  • named-grid-areas-no-invalid
  • no-descending-specificity
  • no-duplicate-at-import-rules
  • no-duplicate-selectors
  • no-empty-source
  • no-invalid-double-slash-comments
  • no-invalid-position-at-import-rule
  • property-no-unknown
  • selector-pseudo-class-no-unknown
  • selector-pseudo-element-no-unknown
  • selector-type-no-unknown
  • string-no-newline
  • unit-no-unknown

Enforce conventions:

  • alpha-value-notation
  • at-rule-allowed-list
  • at-rule-disallowed-list
  • at-rule-no-vendor-prefix
  • at-rule-property-required-list
  • color-function-notation
  • color-hex-alpha
  • color-hex-length
  • color-named
  • color-no-hex
  • comment-pattern
  • comment-word-disallowed-list
  • custom-media-pattern
  • custom-property-pattern
  • declaration-block-no-redundant-longhand-properties
  • declaration-block-single-line-max-declarations
  • declaration-no-important
  • declaration-property-unit-allowed-list
  • declaration-property-unit-disallowed-list
  • declaration-property-value-allowed-list
  • declaration-property-value-disallowed-list
  • font-family-name-quotes
  • font-weight-notation
  • function-allowed-list
  • function-disallowed-list
  • function-url-no-scheme-relative
  • function-url-quotes
  • function-url-scheme-allowed-list
  • function-url-scheme-disallowed-list
  • hue-degree-notation
  • keyframes-name-pattern
  • length-zero-no-unit
  • max-nesting-depth
  • media-feature-name-allowed-list
  • media-feature-name-disallowed-list
  • media-feature-name-no-vendor-prefix
  • media-feature-name-value-allowed-list
  • no-unknown-animations
  • number-max-precision
  • property-allowed-list
  • property-disallowed-list
  • property-no-vendor-prefix
  • rule-selector-property-disallowed-list
  • selector-attribute-name-disallowed-list
  • selector-attribute-operator-allowed-list
  • selector-attribute-operator-disallowed-list
  • selector-attribute-quotes
  • selector-class-pattern
  • selector-combinator-allowed-list
  • selector-combinator-disallowed-list
  • selector-disallowed-list
  • selector-id-pattern
  • selector-max-attribute
  • selector-max-class
  • selector-max-combinators
  • selector-max-compound-selectors
  • 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
  • selector-not-notation
  • selector-pseudo-class-allowed-list
  • selector-pseudo-class-disallowed-list
  • selector-pseudo-element-allowed-list
  • selector-pseudo-element-colon-notation
  • selector-pseudo-element-disallowed-list
  • shorthand-property-no-redundant-values
  • time-min-milliseconds
  • unit-allowed-list
  • unit-disallowed-list
  • 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:

image

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 ChillyBots changed the title Provider full error and warning line formation to plugins Provider full error and warning line information to plugins Nov 5, 2021
@ybiquitous
Copy link
Member

@ChillyBots Thanks for the report. I've edited the description to use 3 backticks for clarity.

@ybiquitous
Copy link
Member

@ChillyBots Could you please provide a reproduction code with CLI?

@ybiquitous ybiquitous added the status: needs clarification triage needs clarification from author label Nov 7, 2021
@adalinesimonian
Copy link
Member

@ybiquitous This isn't really a bug; I think this should be a feature request. The original discussion is in stylelint/vscode-stylelint#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.

@ybiquitous
Copy link
Member

@adalinesimonian Thank you for the helpful summary! I understand it well. 😄

@adalinesimonian
Copy link
Member

adalinesimonian commented Nov 17, 2021

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: postcss/postcss#1669

@adalinesimonian adalinesimonian changed the title Provider full error and warning line information to plugins Provide end positions and fix edits per warning Nov 17, 2021
@adalinesimonian
Copy link
Member

The upstream PR has been merged and will most likely make it into PostCSS 8.3 per postcss/postcss#1669 (comment). Once released, we'll be able to make the necessary changes to support ranges in Stylelint!

@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules and removed status: needs discussion triage needs further discussion labels Nov 17, 2021
@jeddy3 jeddy3 changed the title Provide end positions and fix edits per warning Add end positions and fix edits per problem Nov 17, 2021
@adalinesimonian
Copy link
Member

adalinesimonian commented Nov 19, 2021

WIP implementation for ranges in #5725

@adalinesimonian adalinesimonian added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Nov 20, 2021
@adalinesimonian adalinesimonian self-assigned this Nov 20, 2021
@adalinesimonian
Copy link
Member

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

@jeddy3 jeddy3 changed the title Add end positions and fix edits per problem Add end positions to warnings Feb 9, 2022
@jeddy3 jeddy3 pinned this issue Feb 9, 2022
@jeddy3 jeddy3 added the type: umbrella a group of related issues label Feb 10, 2022
@ybiquitous
Copy link
Member

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! 😊

@IanVS
Copy link

IanVS commented Aug 17, 2022

Legendary effort y'all! I'm really looking forward to this, it's a huge improvement. Thanks a ton.

@mattxwang
Copy link
Member

mattxwang commented Aug 17, 2022

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 stylelint/vscode-stylelint#358, apologies for the ping then!

@Mouvedia
Copy link
Contributor

Mouvedia commented Aug 17, 2022

@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
    • no-irregular-whitespace
  • import-notation
  • declaration-property-max-values
  • unicode-bom

@ybiquitous
Copy link
Member

@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. 🤔

@ybiquitous
Copy link
Member

I've opened the follow-up PR #6278.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue is good for newcomers status: wip is being worked on by someone type: umbrella a group of related issues
Development

No branches or pull requests

7 participants