Disable fixing files if fixing can cause bugs (#4573)

* Disable fixing files if fixing can cause bugs

* Make isFixCompatible function more readable

lib/lintSource.js

@@ -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'
@@ -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),
 				),
@@ -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 eslint-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;
+}

lib/standalone.js

@@ -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;
@@ -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);

system-tests/fix/fix.test.js

@@ -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', () => {

types/postcss/extensions.d.ts

@@ -0,0 +1,12 @@
+import * as postcss from 'postcss';
+declare module 'postcss' {
+
+	interface NodeSource {
+		lang?: string;
+
+	}
+
+	interface Input {
+		css?: string;
+	}
+}

types/stylelint/index.d.ts

@@ -1,5 +1,5 @@
 declare module 'stylelint' {
-	import {Result, ResultMessage, WarningOptions, Warning} from 'postcss';
+	import {Result, ResultMessage, WarningOptions, Warning, Root as PostcssRoot, NodeSource} from 'postcss';
 
 	export type StylelintConfigExtends = string | Array<string>;
 	export type StylelintConfigPlugins = string | Array<string>;
@@ -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
 				}