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

False positives with SCSS maps in color-named #2009

Closed
ThePeach opened this issue Oct 24, 2016 · 7 comments
Closed

False positives with SCSS maps in color-named #2009

ThePeach opened this issue Oct 24, 2016 · 7 comments
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

@ThePeach
Copy link

Describe the issue. Is it a bug or a feature request (new rule, new option, etc.)?

This is a bug related to color-named and happens when a variable name has the same name of a key of a map

The needed SCSS snippet to trigger this is the following:

@function color($key) {
  @if map-has-key($colors, $key) {
    @return map-get($colors, $key);
  }
  return null;
}

// Color palette
$colors: (
  white: #FFFFFF,
  grey: #333333
);

$color-footer: color(white) !default;

What stylelint configuration is needed to reproduce this issue?

{
  "rules": {
    "color-named": "never"
  }
}

Which version of stylelint are you using?

7.5.0

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

via gulp-stylelint

Does your issue relate to non-standard syntax (e.g. SCSS, nesting, etc.)?

"Yes, it's related to SCSS maps."

What did you expect to happen?

"No warnings to be flagged."

What actually happened (e.g. what warnings or errors you are getting)?

"The following warnings were flagged:"

test.css
  10:3   ✖  Unexpected named color "white"    color-named
  11:3   ✖  Unexpected named color "grey"     color-named
  14:22  ✖  Unexpected named color "white"    color-named
@jeddy3
Copy link
Member

jeddy3 commented Oct 25, 2016

@ThePeach Thanks for using the issue template.

You can the "ignore": ["inside-function"] secondary option here e.g.

"color-named": ["never", { "ignore": ["inside-function"] }]

@stylelint/core Perhaps we should change the behaviour to ignore maps by default, and reserve "ignore": ["inside-function"] for just things like $color-footer: color(white) !default;?

Currently "color-named": ["never", { "ignore": ["inside-function"] }] causes maps to be, rather unintutively, ignored as well. This happens because PostCSS parses the map as a decl and value-parser parsers (white: #FFFFFF, grey: #333333) as a function.

@ThePeach
Copy link
Author

@jeddy3 thanks for the explanation. I don't have a use case on all the projects I'm using to do the automatic replacement with PostCSS and I'd rather write that once and forget about it.

I've used the suggested option and it seems to be working fine.
I would also agree that as a default behaviour it might be probably better, but -again- I don't have a use case for this instance.

@davidtheclark
Copy link
Contributor

@jeddy3 I'm fine with ignoring maps by default.

@jeddy3 jeddy3 changed the title color-named: Unexpected named color "white" on map keys False positives with SCSS maps in color-named Oct 27, 2016
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule and removed status: needs discussion triage needs further discussion labels Oct 27, 2016
@jeddy3
Copy link
Member

jeddy3 commented Oct 27, 2016

@ThePeach Would you like to contribute this fix? It'll just be the case of returning early if the "delc" isn't of standard syntax.

@ThePeach
Copy link
Author

@jeddy3 happy to help, I wonder if you linked to declaration-colon-newline-after for a specific reason or am I missing something?

@jeddy3
Copy link
Member

jeddy3 commented Oct 28, 2016

I wonder if you linked to declaration-colon-newline-after for a specific reason or am I missing something?

It contains an example of how to return early if the "decl" isn't of standard syntax :)

@davidtheclark davidtheclark added the good first issue is good for newcomers label Nov 21, 2016
@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 Dec 10, 2016
@davidtheclark
Copy link
Contributor

Closed by #2182.

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

No branches or pull requests

3 participants