sass / linter

An experimental Sass linter written using the Dart Sass AST
MIT License
39 stars 6 forks source link

Add a lint for named colors as map keys #17

Open nex3 opened 6 years ago

nex3 commented 6 years ago

A mistake we see occasionally is users using unquoted strings as map keys, having one of those keys be named something like blue, and getting confused later on when that key compiles to #00f instead. We should add a lint rule to help out with this.

The most conservative way to do this is probably to warn the user when a map:

Less conservatively, we could also warn about maps that have keys of mixed type in general or maps with any unquoted string keys, although this risks having more false positives.

mik01aj commented 6 years ago

I see you want to prevent https://github.com/sass/libsass/issues/652, but wouldn't https://github.com/sass/linter/issues/12 also solve it in a more generic way?

Also, do these examples describe what you expect here?

$theme1: (primary: #f00, secondary: #00f); // ok
map-get($theme1, primary); // ok
map-get($theme1, 'primary'); // ok

$theme2: (red: #f00, blue: #00f); // linter error
map-get($theme2, red); // linter error
map-get($theme2, 'red'); // ok (although this won't work, afaik)

$theme3: ('red': #f00, 'blue': #00f); // ok
map-get($theme3, red); // linter error
map-get($theme3, 'red'); // ok
nex3 commented 6 years ago

I see you want to prevent sass/libsass#652, but wouldn't #12 also solve it in a more generic way?

Yes, it is kind of redundant with #12. It may still be useful to have both, though, for users who find #12 to be too onerous and disable it.

Also, do these examples describe what you expect here?

$theme1: (primary: #f00, secondary: #00f); // ok
map-get($theme1, primary); // ok
map-get($theme1, 'primary'); // ok

$theme2: (red: #f00, blue: #00f); // linter error
map-get($theme2, red); // linter error
map-get($theme2, 'red'); // ok (although this won't work, afaik)

$theme3: ('red': #f00, 'blue': #00f); // ok
map-get($theme3, red); // linter error
map-get($theme3, 'red'); // ok

For this and other map key lints, I was just thinking that it would affect literal maps, not map-get() calls.