Unexpected indenting enforcement on multiline rules

[pdehaan]

In one of our projects we have the following Sass for hi-dpi images:

//Image Manangement
@mixin hidpi-background-image($filename, $background-size: 'mixed', $extension: png) {
  background-image: url('../images/#{$filename}.#{$extension}');
  @if ($background-size != 'mixed') {
    background-size: $background-size;
  }
  @media (min--moz-device-pixel-ratio: 1.3),
    (-o-min-device-pixel-ratio: 2.6/2),
    (-webkit-min-device-pixel-ratio: 1.3),
    (min-device-pixel-ratio: 1.3),
    (min-resolution: 1.3dppx) {
      background-image: url('../images/#{$filename}@2x.#{$extension}');
    }
}

sass-lint@1.3.3 (via gulp-sass-lint) throws the following warnings:

frontend/static-src/styles/_theme-helpers.scss
  61:11  warning  Vendor prefixes should not be used  no-vendor-prefixes
  62:3   warning  Indentation of 2, expected 0        indentation
  63:3   warning  Indentation of 2, expected -2       indentation
  63:4   warning  Vendor prefixes should not be used  no-vendor-prefixes
  64:3   warning  Indentation of 2, expected -4       indentation
  65:3   warning  Indentation of 2, expected -6       indentation
  66:5   warning  Indentation of 4, expected -4       indentation

✖ 7 problems (0 errors, 7 warnings)

Not sure why it's reporting an expected indentation with negative values, or how I could improve my indenting/formatting...

Is there a way to disable specific rules (like indentation and no-vendor-prefixes) for specific lines of a file while keeping it enabled elsewhere?

[BPScott]

Yep that's sounding hella funky and bug-like. Disabling rules on a per codeblock basis would be #70, but that's not merged just yet.

[DanPurdy]

Hi @pdehaan thanks for the report. As @BPScott said that is a hella funky bug going on there. Disabling linters on the fly is currently being worked on and hopefully won't be too far away, though it's obviously going to require a lot of testing still.

As for the indentation rule, I think I can probably take a guess at why it's doing what it's doing there. That whole rule is due for a rewrite as soon as we can update to the latest version of the AST we're using but it's currently not stable enough for us. I'll look into it though to see if there's anything we can do right now.

[pdehaan]

Awesome, thanks guys.

[jessepollak]

Did y'all have a temporary solution for this indentation issue? I'm facing the same thing and wondering if I just have to abandon the tool. I'll also take a look at #70 and see if I can help.

[DanPurdy]

Indentation is still in need of a little overhaul, I intend to get it sorted soon it's just time at the moment. The reason it's been left for so long is due to the big AST update we released in 1.6 which drastically changed things. Now that that is out of the way I've been working on updating this rule a bit but nothing concrete yet unfortunately, soon though.

Also #70 is already underway you can follow it / help out in #677

[IceCreamYou]

In case it helps reproduce this issue, I've found that it only occurs for me in @media queries that include a comma (sass-lint 1.7.0). So it happens here:

/* Produces:
  1010:5  warning  Indentation of 4, expected 0  indentation
  1011:9  warning  Indentation of 8, expected 4  indentation
 */
@media (max-width: 800px), (max-height: 600px) {
    #unit {
        margin: 16px;
    }
}

But not here:

@media (max-width: 800px) {
    #unit {
        margin: 16px;
    }
}
@media (max-height: 600px) {
    #unit {
        margin: 16px;
    }
}

I've found this to be consistent across several rules. Additional commas in the media query seem to further decrease the indentation that sass-lint expects.

[DanPurdy]

@IceCreamYou Thanks for this, can confirm that this will be fixed with the updates to the indentation rule scheduled to drop in our 1.8 release