Skip to content

Commit

Permalink
Fix turning off autofix for sources containing scoped disable comments (
Browse files Browse the repository at this point in the history
  • Loading branch information
jeddy3 committed Apr 21, 2020
1 parent 6995182 commit 1e38d04
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 9 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ It's mighty as it:
- parses **CSS-like syntaxes** like SCSS, Sass, Less and SugarSS
- has over **170 built-in rules** to catch errors, apply limits and enforce stylistic conventions
- supports **plugins** so you can create your own rules or make use of plugins written by the community
- automatically **fixes** the majority of stylistic violations (_experimental feature_)
- automatically **fixes** the majority of stylistic violations
- is **well tested** with over 15000 unit tests
- supports **shareable configs** that you can extend or create
- is **unopinionated** so that you can customize it to your exact needs
Expand Down
7 changes: 6 additions & 1 deletion docs/user-guide/usage/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ Automatically fix, where possible, violations reported by rules.

For CSS with standard syntax, stylelint uses [postcss-safe-parser](https://github.com/postcss/postcss-safe-parser) to fix syntax errors.

_`fix` is an experimental feature that currently ignores sources with [`/* stylelint-disable */` comments](../ignore-code.md)._
If a source contains a:

- scoped disable comment, e.g. `/* stylelint-disable indentation */`, any violations reported by the scoped rules will not be automatically fixed anywhere in the source
- unscoped disable comment, i.e. `/* stylelint-disable */`, the entirety of source will not be automatically fixed

This limitation in being tracked in [issue #2643](https://github.com/stylelint/stylelint/issues/2643).

## `formatter`

Expand Down
8 changes: 5 additions & 3 deletions lib/lintSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,11 @@ function lintPostcssResult(stylelint, postcssResult, config) {
Promise.all(
postcssRoots.map((postcssRoot) =>
ruleFunction(primaryOption, secondaryOptions, {
fix: stylelint._options.fix && isFileFixCompatible,
fix:
stylelint._options.fix &&
// Next two conditionals are temporary measures until #2643 is resolved
isFileFixCompatible &&
!postcssResult.stylelint.disabledRanges[ruleName],
newline,
})(postcssRoot, postcssResult),
),
Expand Down Expand Up @@ -251,8 +255,6 @@ function createEmptyPostcssResult(filePath) {
* There are currently some bugs in the autofixer of Stylelint.
* The autofixer does not yet adhere to stylelint-disable comments, so if there are disabled
* ranges we can not autofix this document. More info in issue #2643.
* Also, if this document is parsed with postcss-jsx and there are nested template
* literals, it will duplicate some code. More info in issue #4119.
*
* @param {PostcssResult} postcssResult
* @returns {boolean}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ testRule(rule, {
code: '@media print { a { color: pink; }}\n@media screen { b { color: red; }}',
},
{
code: '.a {} /* stylelint-disable-line block-no-empty */',
code: '.a {} /* comment */',
},
{
code: '.a {} /* stylelint-disable-line block-no-empty */\n b {}',
code: '.a {} /* comment */\n b {}',
},
{
code: ':root {\n --x { color: pink; };\n --y { color: red; };\n }',
Expand Down Expand Up @@ -109,8 +109,8 @@ testRule(rule, {
column: 35,
},
{
code: '.a {} /* stylelint-disable-line block-no-empty */ b {}',
fixed: '.a {} /* stylelint-disable-line block-no-empty */\n b {}',
code: '.a {} /* comment */ b {}',
fixed: '.a {} /* comment */\n b {}',
message: messages.expectedAfter(),
line: 1,
column: 6,
Expand Down
102 changes: 102 additions & 0 deletions system-tests/fix/fix.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const del = require('del');
const fs = require('fs');
const os = require('os');
const path = require('path');
const stripIndent = require('common-tags').stripIndent;
const stylelint = require('../../lib');
const systemTestUtils = require('../systemTestUtils');
const { promisify } = require('util');
Expand Down Expand Up @@ -173,6 +174,107 @@ describe('fix', () => {
expect(result.output).toBe(code);
});
});

it("doesn't fix with scoped stylelint-disable commands", () => {
const code = `
/* stylelint-disable indentation */
a {
color: red;
}
`;

return stylelint
.lint({
code,
config: {
rules: {
indentation: 2,
},
},
fix: true,
})
.then((result) => {
expect(result.output).toBe(code);
});
});

it("doesn't fix with multiple scoped stylelint-disable commands", () => {
const code = `
/* stylelint-disable indentation, color-hex-length */
a {
color: #ffffff;
}
`;

return stylelint
.lint({
code,
config: {
rules: {
indentation: 2,
'color-hex-length': 'short',
},
},
fix: true,
})
.then((result) => {
expect(result.output).toBe(code);
});
});

it("the color-hex-length rule doesn't fix with scoped stylelint-disable commands", () => {
return stylelint
.lint({
code: stripIndent`
/* stylelint-disable color-hex-length */
a {
color: #ffffff;
}
`,
config: {
rules: {
indentation: 2,
'color-hex-length': 'short',
},
},
fix: true,
})
.then((result) => {
expect(result.output).toBe(stripIndent`
/* stylelint-disable color-hex-length */
a {
color: #ffffff;
}
`);
});
});

it("the indentation rule doesn't fix with scoped stylelint-disable commands", () => {
return stylelint
.lint({
code: stripIndent`
/* stylelint-disable indentation */
a {
color: #ffffff;
}
`,
config: {
rules: {
indentation: 2,
'color-hex-length': 'short',
},
},
fix: true,
})
.then((result) => {
expect(result.output).toBe(stripIndent`
/* stylelint-disable indentation */
a {
color: #fff;
}
`);
});
});
});

describe('fix with BOM', () => {
Expand Down

0 comments on commit 1e38d04

Please sign in to comment.