prefer-ternary: Skip fix if there are comments (#1763)

sindresorhus · Mar 29, 2022 · 3013565 · 3013565
1 parent 871ee95
commit 3013565
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 50 deletions.
diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js
@@ -194,61 +194,65 @@ const create = context => {
 				return;
 			}
 
-			const scope = context.getScope();
-
-			return {
-				node,
-				messageId,
-				* fix(fixer) {
-					const testText = getText(node.test);
-					const consequentText = typeof result.consequent === 'string'
-						? result.consequent
-						: getText(result.consequent);
-					const alternateText = typeof result.alternate === 'string'
-						? result.alternate
-						: getText(result.alternate);
-
-					let {type, before, after} = result;
-
-					let generateNewVariables = false;
-					if (type === 'ThrowStatement') {
-						const scopes = getScopes(scope);
-						const errorName = avoidCapture('error', scopes, isSafeName);
-
-						for (const scope of scopes) {
-							if (!scopeToNamesGeneratedByFixer.has(scope)) {
-								scopeToNamesGeneratedByFixer.set(scope, new Set());
-							}
-
-							const generatedNames = scopeToNamesGeneratedByFixer.get(scope);
-							generatedNames.add(errorName);
-						}
+			const problem = {node, messageId};
 
-						const indentString = getIndentString(node, sourceCode);
+			// Don't fix if there are comments
+			if (sourceCode.getCommentsInside(node).length > 0) {
+				return problem;
+			}
 
-						after = after
-							.replace('{{INDENT_STRING}}', indentString)
-							.replace('{{ERROR_NAME}}', errorName);
-						before = before
-							.replace('{{INDENT_STRING}}', indentString)
-							.replace('{{ERROR_NAME}}', errorName);
-						generateNewVariables = true;
-					}
+			const scope = context.getScope();
+			problem.fix = function * (fixer) {
+				const testText = getText(node.test);
+				const consequentText = typeof result.consequent === 'string'
+					? result.consequent
+					: getText(result.consequent);
+				const alternateText = typeof result.alternate === 'string'
+					? result.alternate
+					: getText(result.alternate);
+
+				let {type, before, after} = result;
+
+				let generateNewVariables = false;
+				if (type === 'ThrowStatement') {
+					const scopes = getScopes(scope);
+					const errorName = avoidCapture('error', scopes, isSafeName);
+
+					for (const scope of scopes) {
+						if (!scopeToNamesGeneratedByFixer.has(scope)) {
+							scopeToNamesGeneratedByFixer.set(scope, new Set());
+						}
 
-					let fixed = `${before}${testText} ? ${consequentText} : ${alternateText}${after}`;
-					const tokenBefore = sourceCode.getTokenBefore(node);
-					const shouldAddSemicolonBefore = needsSemicolon(tokenBefore, sourceCode, fixed);
-					if (shouldAddSemicolonBefore) {
-						fixed = `;${fixed}`;
+						const generatedNames = scopeToNamesGeneratedByFixer.get(scope);
+						generatedNames.add(errorName);
 					}
 
-					yield fixer.replaceText(node, fixed);
-
-					if (generateNewVariables) {
-						yield * extendFixRange(fixer, sourceCode.ast.range);
-					}
-				},
+					const indentString = getIndentString(node, sourceCode);
+
+					after = after
+						.replace('{{INDENT_STRING}}', indentString)
+						.replace('{{ERROR_NAME}}', errorName);
+					before = before
+						.replace('{{INDENT_STRING}}', indentString)
+						.replace('{{ERROR_NAME}}', errorName);
+					generateNewVariables = true;
+				}
+
+				let fixed = `${before}${testText} ? ${consequentText} : ${alternateText}${after}`;
+				const tokenBefore = sourceCode.getTokenBefore(node);
+				const shouldAddSemicolonBefore = needsSemicolon(tokenBefore, sourceCode, fixed);
+				if (shouldAddSemicolonBefore) {
+					fixed = `;${fixed}`;
+				}
+
+				yield fixer.replaceText(node, fixed);
+
+				if (generateNewVariables) {
+					yield * extendFixRange(fixer, sourceCode.ast.range);
+				}
 			};
+
+			return problem;
 		},
 	};
 };

diff --git a/test/prefer-ternary.mjs b/test/prefer-ternary.mjs
@@ -1166,8 +1166,9 @@ test({
 		{
 			code: outdent`
 				function unicorn() {
+					// There is an empty block inside consequent
 					if (test) {
-						; // Empty block
+						;
 						return a;
 					} else {
 						return b;
@@ -1176,6 +1177,7 @@ test({
 			`,
 			output: outdent`
 				function unicorn() {
+					// There is an empty block inside consequent
 					return test ? a : b;
 				}
 			`,
@@ -1319,5 +1321,9 @@ test({
 			`,
 			errors,
 		},
+		{
+			code: 'if (test) {foo = /* comment */1;} else {foo = 2;}',
+			errors,
+		},
 	],
 });