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 dollar variables in no-invalid-position-at-import-rule #5263

Closed
XhmikosR opened this issue Apr 27, 2021 · 1 comment · Fixed by #5264
Closed

Fix false positives for dollar variables in no-invalid-position-at-import-rule #5263

XhmikosR opened this issue Apr 27, 2021 · 1 comment · Fixed by #5264
Labels
good first issue is good for newcomers status: wip is being worked on by someone type: bug a problem with a feature or rule

Comments

@XhmikosR
Copy link
Member

Clearly describe the bug

Should allow dollar variables

Which rule, if any, is the bug related to?

e.g. no-invalid-position-at-import-rule

What code is needed to reproduce the bug?

e.g.

/*!
 * Bootstrap Reboot v5.0.0-beta3 (https://getbootstrap.com/)
 * Copyright 2011-2021 The Bootstrap Authors
 * Copyright 2011-2021 Twitter, Inc.
 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/main/LICENSE)
 * Forked from Normalize.css, licensed MIT (https://github.com/necolas/normalize.css/blob/master/LICENSE.md)
 */

@import "functions";
@import "variables";
// Prevent the usage of custom properties since we don't add them to `:root` in reboot
$font-family-base: $font-family-sans-serif; // stylelint-disable-line scss/dollar-variable-default
$font-family-code: $font-family-monospace; // stylelint-disable-line scss/dollar-variable-default
@import "mixins";
@import "reboot";

What stylelint configuration is needed to reproduce the bug?

e.g.

{
  "rules": {
    "no-invalid-position-at-import-rule": true
  }
}

Which version of stylelint are you using?

e.g. 13.13.0

How are you running stylelint: CLI, PostCSS plugin, Node.js API?

e.g. "CLI with stylelint "**/*.css" --config myconfig.json"

What did you expect to happen?

e.g. "No warnings to be flagged."

What actually happened (e.g. what warnings or errors did you get)?

e.g. "The following warnings were flagged:"

bootstrap/scss/bootstrap-reboot.scss
 14:1  ✖  Unexpected invalid position @import rule   no-invalid-position-at-import-rule
 15:1  ✖  Unexpected invalid position @import rule   no-invalid-position-at-import-rule

Refs #5202 (comment)

@jeddy3 jeddy3 changed the title Improve no-invalid-position-at-import-rule Fix false positives for dollar varialbes in no-invalid-position-at-import-rule Apr 27, 2021
@jeddy3 jeddy3 added good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule labels Apr 27, 2021
@jeddy3
Copy link
Member

jeddy3 commented Apr 27, 2021

@XhmikosR Thanks for the report and for using the template.

Let's change the logic in the rule so that it only warns if @import is preceded by either a standard syntax:

  • rule, i.e. isStandardSyntaxRule(rule)
  • at-rule, i.e. isStandardSyntaxAtRule(atRule)

This will be true to the wording of the spec while allowing non-standard syntax, like dollar variables.

I've labelled the issue as ready to implement. Please consider contributing if you have time.

There are steps on how to fix a bug in a rule in the Developer guide.

@XhmikosR XhmikosR changed the title Fix false positives for dollar varialbes in no-invalid-position-at-import-rule Fix false positives for dollar variables in no-invalid-position-at-import-rule Apr 27, 2021
@jeddy3 jeddy3 added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Apr 27, 2021
yhatt added a commit to marp-team/marp-core that referenced this issue Apr 27, 2021
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: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

2 participants