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.

Sign up for GitHub

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

Add disableFix secondary option #5460

Merged
merged 10 commits into from
Sep 16, 2021
Merged

Add disableFix secondary option #5460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4e79cd3
Add disableFix secondary option
renshu1 Aug 12, 2021
7e7ddd8
Update docs/user-guide/configure.md
kuangrs Aug 13, 2021
3433b46
Add test code for disableFix secondary option
renshu1 Aug 17, 2021
ec9e5cc
Fix the judgement of ruleDisableFix
renshu1 Aug 17, 2021
f9388c4
Fix configure.md conflict
renshu1 Aug 17, 2021
6be4b1d
Update lib/__tests__/standalone-fix.test.js
kuangrs Sep 16, 2021
56aea85
Update lib/__tests__/standalone-fix.test.js
kuangrs Sep 16, 2021
1bc2b37
Update lib/__tests__/standalone-fix.test.js
kuangrs Sep 16, 2021
97045d3
Update lib/__tests__/standalone-fix.test.js
kuangrs Sep 16, 2021
f774196
Fix format
renshu1 Sep 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions docs/user-guide/configure.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,26 @@ For example:

The report is considered to be a lint error.

### `disableFix`

You can set the `disableFix` secondary option to disable autofix _on a per rule basis_.

For example:

```json
{
"rules": {
"indentation": [
2,
{
"except": ["value"],
"disableFix": true
}
]
}
}
```

## Disable Errors

These configurations provide extra validation for `stylelint-disable` comments. This can be helpful for enforcing useful and well-documented disables.
Expand Down
93 changes: 93 additions & 0 deletions lib/__tests__/standalone-fix.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,96 @@ describe('writing fixes to files', () => {
expect(fileContent.startsWith('\uFEFF')).toBe(true);
});
});

it('one rule being disabled', async () => {
const code = `
a {
color: red;
}`;

const result = await standalone({
code,
config: {
rules: {
indentation: [
2,
{
disableFix: true,
},
],
},
},
fix: true,
});

expect(JSON.parse(result.output)[0].errored).toBe(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than check for errors, can we instead check if the output itself has been changed (like the tests at the beginning of the file), e.g.:

const code = "  a { color: red; }";

const result = await standalone({
    code,
    config: {
        rules: {
            indentation: [
                2,
                {
                    disableFix: true,
                },
            ],
        },
    },
    fix: true,
});

expect(result.output).toBe(code);

And:

const code = "  a { COLOR: red; }";

const result = await standalone({
    code,
    config: {
        rules: {
            indentation: [
                2,
                {
                    disableFix: true,
                },
            ],
            'property-case': ['lower'],
        },
    },
    fix: true,
});

expect(result.output).toBe("  a { color: red; }");

Copy link
Contributor Author

@kuangrs kuangrs Aug 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeddy3 When we use disableFix, what we expect is that if there is an error in the check, the output still contains the error message instead of the initial code, so that we can find it easily.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point.

});

it('two rules being disabled', async () => {
const code = `
a {
COLOR: red;
}`;

const result = await standalone({
code,
config: {
rules: {
indentation: [
2,
{
disableFix: true,
},
],
'property-case': [
'lower',
{
disableFix: true,
},
],
},
},
fix: true,
});

const warnings = JSON.parse(result.output)[0].warnings;

expect(warnings.some((w) => w.text === 'Expected indentation of 0 spaces (indentation)')).toBe(
true,
);
expect(warnings.some((w) => w.text === 'Expected "COLOR" to be "color" (property-case)')).toBe(
true,
);
});

it('one rule being disabled and another still autofixing', async () => {
const code = `
a {
COLOR: red;
}`;

const result = await standalone({
code,
config: {
rules: {
indentation: [
2,
{
disableFix: true,
},
],
'property-case': ['lower'],
},
},
fix: true,
});

const warnings = JSON.parse(result.output)[0].warnings;

expect(warnings.some((w) => w.text === 'Expected indentation of 0 spaces (indentation)')).toBe(
true,
);
expect(warnings.some((w) => w.text === 'Expected "COLOR" to be "color" (property-case)')).toBe(
false,
);
});
7 changes: 7 additions & 0 deletions lib/lintPostcssResult.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ function lintPostcssResult(stylelintOptions, postcssResult, config) {

// Log the rule's severity in the PostCSS result
const defaultSeverity = config.defaultSeverity || 'error';
// disableFix in secondary option
const disableFix = (secondaryOptions && secondaryOptions.disableFix === true) || false;

if (disableFix) {
postcssResult.stylelint.ruleDisableFix = true;
}

postcssResult.stylelint.ruleSeverities[ruleName] =
(secondaryOptions && secondaryOptions.severity) || defaultSeverity;
Expand All @@ -98,6 +104,7 @@ function lintPostcssResult(stylelintOptions, postcssResult, config) {
postcssRoots.map((postcssRoot) =>
ruleFunction(primaryOption, secondaryOptions, {
fix:
!disableFix &&
stylelintOptions.fix &&
// Next two conditionals are temporary measures until #2643 is resolved
isFileFixCompatible &&
Expand Down
8 changes: 7 additions & 1 deletion lib/standalone.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,12 @@ module.exports = function (options) {
const postcssResult = stylelintResult._postcssResult;
const returnValue = prepareReturnValue([stylelintResult], options, formatterFunction);

if (options.fix && postcssResult && !postcssResult.stylelint.ignored) {
if (
options.fix &&
postcssResult &&
!postcssResult.stylelint.ignored &&
!postcssResult.stylelint.ruleDisableFix
) {
if (!postcssResult.stylelint.disableWritingFix) {
// If we're fixing, the output should be the fixed code
returnValue.output = postcssResult.root.toString(postcssResult.opts.syntax);
Expand Down Expand Up @@ -251,6 +256,7 @@ module.exports = function (options) {
postcssResult.root &&
postcssResult.opts &&
!postcssResult.stylelint.ignored &&
!postcssResult.stylelint.ruleDisableFix &&
options.fix &&
!postcssResult.stylelint.disableWritingFix
) {
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/validateOptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const arrayEqual = require('./arrayEqual');
const { isPlainObject } = require('is-plain-object');

const IGNORED_OPTIONS = new Set(['severity', 'message', 'reportDisables']);
const IGNORED_OPTIONS = new Set(['severity', 'message', 'reportDisables', 'disableFix']);

/** @typedef {(value: unknown) => boolean} PossibleFunc */

Expand Down
1 change: 1 addition & 0 deletions types/stylelint/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ declare module 'stylelint' {
stylelintError?: boolean;
disableWritingFix?: boolean;
config?: StylelintConfig;
ruleDisableFix?: boolean;
};

type EmptyResult = {
Expand Down