Skip to content

Commit

Permalink
Reinstate and document workaround feature (#4592)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeddy3 committed Feb 14, 2020
1 parent 20d1a96 commit 36e2dbb
Show file tree
Hide file tree
Showing 9 changed files with 187 additions and 18 deletions.
23 changes: 11 additions & 12 deletions docs/about/syntaxes.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
# Syntaxes

There are many styling languages, ranging from CSS language extension like SCSS to entirely different notations like CSS-in-JS objects. These styling languages can be then be embedded within other languages, including HTML `<style>` tags, markdown fences and JavaScript variables.
There are many styling languages, ranging from CSS language extensions like SCSS to entirely different notations, e.g. CSS-in-JS objects.

We aim to support all these use cases, but it's a complicated task.
These styling languages can be embedded within other languages too. For example:

We use [PostCSS syntaxes](https://github.com/postcss/postcss#syntaxes) to transform these languages into something that resembles CSS, which is the language that:
- HTML `<style>` tags
- markdown fences
- JavaScript template literals

- underpins all the other styling languages
- is best understood by stylelint
We aim to support all these use cases in stylelint, but it's a complicated endeavor.

We lean on [PostCSS syntaxes](https://github.com/postcss/postcss#syntaxes) to help us with this task. We use them to transform these languages into something that resembles CSS, which is the language that:

We need your help to [support and improve](https://github.com/postcss/postcss/blob/master/docs/syntax.md) the following PostCSS syntaxes:
- underpins all the other styling languages
- is best understood by rules built into stylelint

- [postcss-css-in-js](https://github.com/stylelint/postcss-css-in-js)
- [postcss-html](https://github.com/gucong3000/postcss-html)
- [postcss-less](https://github.com/webschik/postcss-less)
- [postcss-markdown](https://github.com/stylelint/postcss-markdown)
- [postcss-sass](https://github.com/AleshaOleg/postcss-sass)
- [postcss-scss](https://github.com/postcss/postcss-scss)
If you write your styles in anything other than CSS, please consider [contributing to these syntaxes](../developer-guide/syntaxes.md) so that they can remain compatible with stylelint.
35 changes: 35 additions & 0 deletions docs/developer-guide/syntaxes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Working on syntaxes

Please help us enhance and debug the [syntaxes](../about/syntaxes.md) we use in stylelint:

- [postcss-css-in-js](https://github.com/stylelint/postcss-css-in-js)
- [postcss-html](https://github.com/gucong3000/postcss-html)
- [postcss-less](https://github.com/webschik/postcss-less)
- [postcss-markdown](https://github.com/stylelint/postcss-markdown)
- [postcss-sass](https://github.com/AleshaOleg/postcss-sass)
- [postcss-scss](https://github.com/postcss/postcss-scss)

To contribute to a syntax, you should:

1. Familiarize yourself with PostCSS's [how to write custom syntax](https://github.com/postcss/postcss/blob/master/docs/syntax.md) guide.
2. Use the [`syntax: *` labels](https://github.com/stylelint/stylelint/labels?utf8=%E2%9C%93&q=syntax%3A) to identify which syntax is behind an issue.
3. Go to the repository for that syntax.
4. Read their contributing guidelines.

## Workarounds

Fixing bugs in syntaxes can take time. stylelint can work around these bug by turning off autofix for incompatible sources. Autofix can then remain safe to use while contributors try to fix the underlying issue.

### Current workarounds

stylelint currently turns off autofix for sources that contain:

- nested tagged template literals ([issue #4119](https://github.com/stylelint/stylelint/issues/4119))

### Add a workaround

To add a new workaround, you should:

1. Add code to [`lib/lintSource.js`](https://github.com/stylelint/stylelint/blob/master/lib/lintSource.js) to detect the incompatible pattern.
2. Add a corresponding test to [`system-tests/fix/fix.test.js`](https://github.com/stylelint/stylelint/blob/master/system-tests/fix/fix.test.js).
3. Document the workaround in [`docs/developer-guides/syntaxes.md`](https://github.com/stylelint/stylelint/blob/master/docs/developer-guide/syntaxes.md).
1 change: 1 addition & 0 deletions docs/toc.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
- [Errors](user-guide/errors.md)
- Developer guide
- [Rules](developer-guide/rules.md)
- [Syntaxes](developer-guide/syntaxes.md)
- [Plugins](developer-guide/plugins.md)
- [Formatters](developer-guide/formatters.md)
- [System tests](developer-guide/system-tests.md)
Expand Down
2 changes: 1 addition & 1 deletion docs/user-guide/usage/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ 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.

**Note:** This is an _experimental_ feature. It currently does not respect special comments for disabling stylelint within sources (e. g. `/* stylelint-disable */`). Autofixing is applied regardless of these comments.
_`fix` is an experimental feature that currently ignores sources with [`/* stylelint-disable */` comments](../ignore-code.md)._

## `formatter`

Expand Down
63 changes: 62 additions & 1 deletion lib/lintSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ function lintPostcssResult(stylelint, postcssResult, config) {
postcssResult.stylelint.ignoreDisables = true;
}

const isFileFixCompatible = isFixCompatible(postcssResult);

if (!isFileFixCompatible) {
postcssResult.stylelint.disableWritingFix = true;
}

const postcssRoots =
/** @type {import('postcss').Root[]} */ (postcssDoc &&
postcssDoc.constructor.name === 'Document'
Expand Down Expand Up @@ -200,7 +206,7 @@ function lintPostcssResult(stylelint, postcssResult, config) {
Promise.all(
postcssRoots.map((postcssRoot) =>
ruleFunction(primaryOption, secondaryOptions, {
fix: stylelint._options.fix,
fix: stylelint._options.fix && isFileFixCompatible,
newline,
})(postcssRoot, postcssResult),
),
Expand Down Expand Up @@ -241,3 +247,58 @@ function createEmptyPostcssResult(filePath) {
warn: () => {},
};
}

/**
* 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}
*/
function isFixCompatible({ stylelint, root }) {
// Check for issue #2643
if (stylelint.disabledRanges.all.length) return false;

// Check for issue #4119
if (root && root.source && root.source.lang === 'jsx' && root.nodes) {
// Gather all locations of template literals
/**
* @typedef TemplateLiteralLocation
* @type {object}
* @property {number} startLine - Start of the template literal.
* @property {number} endLine - End of the template literal
*/
/** @type {Array<TemplateLiteralLocation>} */
const templateLiteralLocations = [];

root.nodes.forEach((n) => {
if (n.source && n.source.start && n.source.input.css !== undefined) {
templateLiteralLocations.push({
startLine: n.source.start.line,
endLine: n.source.start.line + n.source.input.css.split('\n').length,
});
}
});

// Compare all different template literal locations with eachother
for (const location1 of templateLiteralLocations) {
for (const location2 of templateLiteralLocations) {
// Make sure it's not the same template literal
if (location1 !== location2) {
// The first location should be before or after the second location.
// If not, it's not compatible.
if (
!(location1.endLine < location2.startLine || location2.endLine < location1.startLine)
) {
return false;
}
}
}
}
}

return true;
}
12 changes: 9 additions & 3 deletions lib/standalone.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,13 @@ module.exports = function(options) {
const returnValue = prepareReturnValue([stylelintResult]);

if (options.fix && postcssResult && !postcssResult.stylelint.ignored) {
// If we're fixing, the output should be the fixed code
returnValue.output = postcssResult.root.toString(postcssResult.opts.syntax);
if (!postcssResult.stylelint.disableWritingFix) {
// If we're fixing, the output should be the fixed code
returnValue.output = postcssResult.root.toString(postcssResult.opts.syntax);
} else {
// If the writing of the fix is disabled, the input code is returned as-is
returnValue.output = code;
}
}

return returnValue;
Expand Down Expand Up @@ -247,7 +252,8 @@ module.exports = function(options) {
postcssResult.root &&
postcssResult.opts &&
!postcssResult.stylelint.ignored &&
options.fix
options.fix &&
!postcssResult.stylelint.disableWritingFix
) {
// @ts-ignore TODO TYPES toString accepts 0 arguments
const fixedCss = postcssResult.root.toString(postcssResult.opts.syntax);
Expand Down
52 changes: 52 additions & 0 deletions system-tests/fix/fix.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,58 @@ describe('fix', () => {
);
});
});

it("doesn't fix with stylelint-disable commands", () => {
const code = `
/* stylelint-disable */
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 nested template literals", () => {
const code = `
import styled, { css } from 'styled-components';
const Component = styled.div\`
padding: 10px;
\${() => css\`
color: #b02d00;
\`}
\`;
`;

return stylelint
.lint({
code,
syntax: 'css-in-js',
config: {
rules: {
indentation: 2,
},
},
fix: true,
})
.then((result) => {
expect(result.errored).toBe(true);
expect(result.output).toBe(code);
});
});
});

describe('fix with BOM', () => {
Expand Down
12 changes: 12 additions & 0 deletions types/postcss/extensions.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as postcss from 'postcss';
declare module 'postcss' {

interface NodeSource {
lang?: string;

}

interface Input {
css?: string;
}
}
5 changes: 4 additions & 1 deletion types/stylelint/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,15 @@ declare module 'stylelint' {
disabledRanges: DisabledRangeObject,
ignored?: boolean,
ignoreDisables?: boolean,
stylelintError?: boolean
stylelintError?: boolean,
disableWritingFix?: boolean
};

type EmptyResult = {
root: {
nodes?: undefined,
source: {
lang?: undefined,
input: {
file?: string
}
Expand Down

0 comments on commit 36e2dbb

Please sign in to comment.