Verify fix and suggestion output (#1831)

\

rules/fix/switch-new-expression-to-call-expression.js

@@ -2,10 +2,10 @@
 const isNewExpressionWithParentheses = require('../utils/is-new-expression-with-parentheses.js');
 const {isParenthesized} = require('../utils/parentheses.js');
 
-function * fixReturnStatementArgument(newExpression, sourceCode, fixer) {
+function * fixReturnOrThrowStatementArgument(newExpression, sourceCode, fixer) {
 	const {parent} = newExpression;
 	if (
-		parent.type !== 'ReturnStatement'
+		(parent.type !== 'ReturnStatement' && parent.type !== 'ThrowStatement')
 		|| parent.argument !== newExpression
 		|| isParenthesized(newExpression, sourceCode)
 	) {
@@ -48,7 +48,7 @@ function * switchNewExpressionToCallExpression(node, sourceCode, fixer) {
 			}
 		```
 	*/
-	yield * fixReturnStatementArgument(node, sourceCode, fixer);
+	yield * fixReturnOrThrowStatementArgument(node, sourceCode, fixer);
 }
 
 module.exports = switchNewExpressionToCallExpression;

rules/utils/should-add-parentheses-to-logical-expression-child.js

@@ -24,7 +24,8 @@ function shouldAddParenthesesToLogicalExpressionChild(node, {operator, property}
 	// Lower precedence than `LogicalExpression`
 	// see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence#Table
 	if (
-		node.type === 'ConditionalExpression'
+		node.type === 'LogicalExpression'
+		|| node.type === 'ConditionalExpression'
 		|| node.type === 'AssignmentExpression'
 		|| node.type === 'AssignmentExpression'
 		|| node.type === 'YieldExpression'

test/prefer-module.mjs

@@ -236,8 +236,8 @@ test.snapshot({
 		'module.exports = foo;',
 		'(( ((exports)) = ((foo)) ));',
 		'(( ((module.exports)) = ((foo)) ));',
-		'exports.foo = foo;',
-		'module.exports.foo = foo;',
+		'const foo = 1;exports.foo = foo;',
+		'const foo = 1;module.exports.foo = foo;',
 		'exports["foo"] = foo;',
 		'module.exports["foo"] = foo;',
 		'const foo = exports;',
@@ -247,8 +247,8 @@ test.snapshot({
 		'module["exports"] = foo;',
 		'module[exports] = foo;',
 		'module.exports.foo.bar = foo;',
-		'exports.default = foo;',
-		'module.exports.default = foo;',
+		'const foo = 1;exports.default = foo;',
+		'const foo = 1;module.exports.default = foo;',
 		'exports.foo.bar = foo;',
 		'exports = 1;',
 		'exports.foo = [];',

test/prefer-spread.mjs

@@ -281,7 +281,7 @@ test.snapshot({
 		'do foo.concat(1); while (test)',
 		{
 			code: 'with (foo) foo.concat(1)',
-			parserOptions: {ecmaVersion: 5, sourceType: 'script'},
+			parserOptions: {ecmaVersion: 6, sourceType: 'script'},
 		},
 		// Code from example in docs
 		outdent`

test/run-rules-on-codebase/lint.mjs

@@ -26,6 +26,7 @@ const eslint = new ESLint({
 			'unicorn/consistent-destructuring': 'off',
 			// Buggy
 			'unicorn/custom-error-definition': 'off',
+			'unicorn/consistent-function-scoping': 'off',
 			// Annoying
 			'unicorn/no-keyword-prefix': 'off',
 			'unicorn/no-unsafe-regex': 'off',

test/snapshots/new-for-builtins.mjs.md

@@ -252,8 +252,8 @@ Generated by [AVA](https://avajs.dev).
 
     `␊
       1 | () => {␊
-      2 | 	throw // 1␊
-      3 | 		Symbol();␊
+      2 | 	throw ( // 1␊
+      3 | 		Symbol());␊
       4 | }␊
     `
 

test/snapshots/prefer-module.mjs.md

@@ -1061,31 +1061,31 @@ Generated by [AVA](https://avajs.dev).
     `
 
 ## Invalid #5
-      1 | exports.foo = foo;
+      1 | const foo = 1;exports.foo = foo;
 
 > Error 1/1
 
     `␊
-    > 1 | exports.foo = foo;␊
-        | ^^^^^^^ Do not use "exports".␊
+    > 1 | const foo = 1;exports.foo = foo;␊
+        |               ^^^^^^^ Do not use "exports".␊
     ␊
     --------------------------------------------------------------------------------␊
     Suggestion 1/1: Switch to \`export\`.␊
-      1 | export {foo as foo};␊
+      1 | const foo = 1;export {foo as foo};␊
     `
 
 ## Invalid #6
-      1 | module.exports.foo = foo;
+      1 | const foo = 1;module.exports.foo = foo;
 
 > Error 1/1
 
     `␊
-    > 1 | module.exports.foo = foo;␊
-        | ^^^^^^ Do not use "module".␊
+    > 1 | const foo = 1;module.exports.foo = foo;␊
+        |               ^^^^^^ Do not use "module".␊
     ␊
     --------------------------------------------------------------------------------␊
     Suggestion 1/1: Switch to \`export\`.␊
-      1 | export {foo as foo};␊
+      1 | const foo = 1;export {foo as foo};␊
     `
 
 ## Invalid #7
@@ -1186,31 +1186,31 @@ Generated by [AVA](https://avajs.dev).
     `
 
 ## Invalid #16
-      1 | exports.default = foo;
+      1 | const foo = 1;exports.default = foo;
 
 > Error 1/1
 
     `␊
-    > 1 | exports.default = foo;␊
-        | ^^^^^^^ Do not use "exports".␊
+    > 1 | const foo = 1;exports.default = foo;␊
+        |               ^^^^^^^ Do not use "exports".␊
     ␊
     --------------------------------------------------------------------------------␊
     Suggestion 1/1: Switch to \`export\`.␊
-      1 | export {foo as default};␊
+      1 | const foo = 1;export {foo as default};␊
     `
 
 ## Invalid #17
-      1 | module.exports.default = foo;
+      1 | const foo = 1;module.exports.default = foo;
 
 > Error 1/1
 
     `␊
-    > 1 | module.exports.default = foo;␊
-        | ^^^^^^ Do not use "module".␊
+    > 1 | const foo = 1;module.exports.default = foo;␊
+        |               ^^^^^^ Do not use "module".␊
     ␊
     --------------------------------------------------------------------------------␊
     Suggestion 1/1: Switch to \`export\`.␊
-      1 | export {foo as default};␊
+      1 | const foo = 1;export {foo as default};␊
     `
 
 ## Invalid #18

test/snapshots/prefer-string-starts-ends-with.mjs.md

@@ -117,7 +117,7 @@ Generated by [AVA](https://avajs.dev).
     ␊
     --------------------------------------------------------------------------------␊
     Suggestion 3/3: Use nullish coalescing \`(… ?? '').startsWith()\`.␊
-      1 | (foo || bar ?? '').startsWith('a')␊
+      1 | ((foo || bar) ?? '').startsWith('a')␊
     `
 
 ## Invalid #6
@@ -541,7 +541,7 @@ Generated by [AVA](https://avajs.dev).
     ␊
     --------------------------------------------------------------------------------␊
     Suggestion 3/3: Use nullish coalescing \`(… ?? '').startsWith()\`.␊
-      1 | (a || b ?? '').startsWith('a')␊
+      1 | ((a || b) ?? '').startsWith('a')␊
     `
 
 ## Invalid #22
@@ -569,7 +569,7 @@ Generated by [AVA](https://avajs.dev).
     ␊
     --------------------------------------------------------------------------------␊
     Suggestion 3/3: Use nullish coalescing \`(… ?? '').startsWith()\`.␊
-      1 | (a && b ?? '').startsWith('a')␊
+      1 | ((a && b) ?? '').startsWith('a')␊
     `
 
 ## Invalid #23

test/utils/snapshot-rule-tester.mjs

@@ -113,6 +113,18 @@ function defineParser(linter, parser) {
 	linter.defineParser(parser, require(parser));
 }
 
+function verify(linter, code, verifyConfig, {filename}) {
+	const messages = linter.verify(code, verifyConfig, {filename});
+
+	const fatalError = messages.find(({fatal}) => fatal);
+	if (fatalError) {
+		const {line, column, message} = fatalError;
+		throw new SyntaxError('\n' + codeFrameColumns(code, {start: {line, column}}, {message}));
+	}
+
+	return messages;
+}
+
 class SnapshotRuleTester {
 	constructor(test, config) {
 		this.test = test;
@@ -138,7 +150,7 @@ class SnapshotRuleTester {
 					${indentCode(printCode(code))}
 				`,
 				t => {
-					const messages = linter.verify(code, verifyConfig, {filename});
+					const messages = verify(linter, code, verifyConfig, {filename});
 					t.deepEqual(messages, [], 'Valid case should not have errors.');
 				},
 			);
@@ -148,21 +160,17 @@ class SnapshotRuleTester {
 			const {code, options, filename} = testCase;
 			const verifyConfig = getVerifyConfig(ruleId, config, testCase);
 			defineParser(linter, verifyConfig.parser);
+			const runVerify = code => verify(linter, code, verifyConfig, {filename});
 
 			test(
 				outdent`
 					Invalid #${index + 1}
 					${indentCode(printCode(code))}
 				`,
 				t => {
-					const messages = linter.verify(code, verifyConfig, {filename});
+					const messages = runVerify(code);
 					t.notDeepEqual(messages, [], 'Invalid case should have at least one error.');
 
-					const fatalError = messages.find(({fatal}) => fatal);
-					if (fatalError) {
-						throw fatalError;
-					}
-
 					const {fixed, output} = fixable ? linter.verifyAndFix(code, verifyConfig, {filename}) : {fixed: false};
 
 					if (filename) {
@@ -174,20 +182,18 @@ class SnapshotRuleTester {
 					}
 
 					if (fixable && fixed) {
+						runVerify(output);
 						t.snapshot(`\n${printCode(output)}\n`, 'Output');
 					}
 
 					for (const [index, message] of messages.entries()) {
 						let messageForSnapshot = visualizeEslintMessage(code, message);
 
 						const {suggestions = []} = message;
-						if (suggestions.length > 0 && rule.meta.hasSuggestions !== true) {
-							// This check will no longer be necessary if this change lands in ESLint 8: https://github.com/eslint/eslint/issues/14312
-							throw new Error('Rule with suggestion is missing `meta.hasSuggestions`.');
-						}
 
 						for (const [index, suggestion] of suggestions.entries()) {
 							const output = applyFix(code, suggestion);
+							runVerify(output);
 
 							messageForSnapshot += outdent`
 								\n