From 5638b2fee1e19b22f04c7f53765050dfe540470b Mon Sep 17 00:00:00 2001 From: Kevin Ghadyani <3948069+Sawtaytoes@users.noreply.github.com> Date: Fri, 28 Feb 2020 10:59:16 -0600 Subject: [PATCH] Update: Added auto-fix to multiline-ternary I noticed unfixed warnings from this ESLint rule and wanted to auto-fix them. While it may seem like doing `'\n? '` is opinionated, I did have changed to make this take a new option and either put the `?` on the previous line or next line. This is actually unnecessary because `operator-linebreak` handles it for you. --- docs/rules/multiline-ternary.md | 56 +++++++++++++ lib/rules/multiline-ternary.js | 121 +++++++++++++++++++++------ tests/lib/rules/multiline-ternary.js | 38 +++++++++ 3 files changed, 191 insertions(+), 24 deletions(-) diff --git a/docs/rules/multiline-ternary.md b/docs/rules/multiline-ternary.md index e9461a1ca08..c1d08be3778 100644 --- a/docs/rules/multiline-ternary.md +++ b/docs/rules/multiline-ternary.md @@ -136,6 +136,62 @@ foo > bar ? value1 : value2; foo > bar ? (baz > qux ? value1 : value2) : value3; ``` +### --fix + +If this rule is invoked with the command-line `--fix` option, it's recommended to define both `indent` and `operator-linebreak` if you want to have sensible results when using the `always` and `always-multiline` options. + +For instance, this code: + +```js +const func = () => { + const items = hasStuff ? [ + ...stuff.items, + ...previousStuff.items, + ] : previousStuff.items +} +``` + +Is converted to: + +```js +const func = () => { + const items = hasStuff +? [ + ...stuff.items, + ...previousStuff.items, + ] +: previousStuff.items +} +``` + +But can be converted to depending on your `indent` value: + +```js +const func = () => { + const items = hasStuff + ? [ + ...stuff.items, + ...previousStuff.items, + ] + : previousStuff.items +} +``` + +Or even this way depending on your `operator-linebreak` value: + +```js +const func = () => { + const items = hasStuff ? + [ + ...stuff.items, + ...previousStuff.items, + ] : + previousStuff.items +} +``` + +The way it choses how to automatically fix depends on how your ternaries were formatted prior to running `--fix`, but with `indent` and `operator-linebreak`, you'll achieve consistent results. + ## When Not To Use It You can safely disable this rule if you do not have any strict conventions about whether the operands of a ternary expression should be separated by newlines. diff --git a/lib/rules/multiline-ternary.js b/lib/rules/multiline-ternary.js index 1df90b6feb8..ef29c22f01a 100644 --- a/lib/rules/multiline-ternary.js +++ b/lib/rules/multiline-ternary.js @@ -27,38 +27,23 @@ module.exports = { enum: ["always", "always-multiline", "never"] } ], + messages: { expectedTestCons: "Expected newline between test and consequent of ternary expression.", expectedConsAlt: "Expected newline between consequent and alternate of ternary expression.", unexpectedTestCons: "Unexpected newline between test and consequent of ternary expression.", unexpectedConsAlt: "Unexpected newline between consequent and alternate of ternary expression." - } + }, + + fixable: "whitespace" }, create(context) { + const sourceCode = context.getSourceCode(); const option = context.options[0]; const multiline = option !== "never"; const allowSingleLine = option === "always-multiline"; - //-------------------------------------------------------------------------- - // Helpers - //-------------------------------------------------------------------------- - - /** - * Tests whether node is preceded by supplied tokens - * @param {ASTNode} node node to check - * @param {ASTNode} parentNode parent of node to report - * @param {boolean} expected whether newline was expected or not - * @returns {void} - * @private - */ - function reportError(node, parentNode, expected) { - context.report({ - node, - messageId: `${expected ? "expected" : "unexpected"}${node === parentNode.test ? "TestCons" : "ConsAlt"}` - }); - } - //-------------------------------------------------------------------------- // Public //-------------------------------------------------------------------------- @@ -70,11 +55,55 @@ module.exports = { if (!multiline) { if (!areTestAndConsequentOnSameLine) { - reportError(node.test, node, false); + context.report({ + node: node.test, + messageId: "unexpectedTestCons", + fix: fixer => { + const testToken = astUtils.isParenthesised(sourceCode, node.test) + ? sourceCode.getTokenAfter(node.test) + : node.test; + + const consequentToken = astUtils.isParenthesised(sourceCode, node.consequent) + ? sourceCode.getTokenBefore(node.consequent) + : node.consequent; + + return ( + fixer.replaceTextRange( + [ + testToken.range[1], + consequentToken.range[0] + ], + " ? " + ) + ); + } + }); } if (!areConsequentAndAlternateOnSameLine) { - reportError(node.consequent, node, false); + context.report({ + node: node.consequent, + messageId: "unexpectedConsAlt", + fix: fixer => { + const consequentToken = astUtils.isParenthesised(sourceCode, node.consequent) + ? sourceCode.getTokenAfter(node.consequent) + : node.consequent; + + const alternateToken = astUtils.isParenthesised(sourceCode, node.alternate) + ? sourceCode.getTokenBefore(node.alternate) + : node.alternate; + + return ( + fixer.replaceTextRange( + [ + consequentToken.range[1], + alternateToken.range[0] + ], + " : " + ) + ); + } + }); } } else { if (allowSingleLine && node.loc.start.line === node.loc.end.line) { @@ -82,11 +111,55 @@ module.exports = { } if (areTestAndConsequentOnSameLine) { - reportError(node.test, node, true); + context.report({ + node: node.test, + messageId: "expectedTestCons", + fix: fixer => { + const testToken = astUtils.isParenthesised(sourceCode, node.test) + ? sourceCode.getTokenAfter(node.test) + : node.test; + + const consequentToken = astUtils.isParenthesised(sourceCode, node.consequent) + ? sourceCode.getTokenBefore(node.consequent) + : node.consequent; + + return ( + fixer.replaceTextRange( + [ + testToken.range[1], + consequentToken.range[0] + ], + "\n? " + ) + ); + } + }); } if (areConsequentAndAlternateOnSameLine) { - reportError(node.consequent, node, true); + context.report({ + node: node.consequent, + messageId: "expectedConsAlt", + fix: fixer => { + const consequentToken = astUtils.isParenthesised(sourceCode, node.consequent) + ? sourceCode.getTokenAfter(node.consequent) + : node.consequent; + + const alternateToken = astUtils.isParenthesised(sourceCode, node.alternate) + ? sourceCode.getTokenBefore(node.alternate) + : node.alternate; + + return ( + fixer.replaceTextRange( + [ + consequentToken.range[1], + alternateToken.range[0] + ], + "\n: " + ) + ); + } + }); } } } diff --git a/tests/lib/rules/multiline-ternary.js b/tests/lib/rules/multiline-ternary.js index a8d2ef6eaa6..d5fde0974f8 100644 --- a/tests/lib/rules/multiline-ternary.js +++ b/tests/lib/rules/multiline-ternary.js @@ -55,6 +55,7 @@ ruleTester.run("multiline-ternary", rule, { // default "always" { code: "a ? b : c", + output: "a\n? b\n: c", errors: [{ messageId: "expectedTestCons", line: 1, @@ -68,6 +69,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a\n? b : c", + output: "a\n? b\n: c", errors: [{ messageId: "expectedConsAlt", line: 2, @@ -76,6 +78,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? b\n: c", + output: "a\n? b\n: c", errors: [{ messageId: "expectedTestCons", line: 1, @@ -84,6 +87,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? (b ? c : d) : e", + output: "a\n? (b\n? c\n: d)\n: e", errors: [{ messageId: "expectedTestCons", line: 1, @@ -107,6 +111,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b ? c : d) :\ne", + output: "a ?\n(b\n? c\n: d) :\ne", errors: [{ messageId: "expectedTestCons", line: 2, @@ -120,6 +125,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? (b\n? c\n: d) : e", + output: "a\n? (b\n? c\n: d)\n: e", errors: [{ messageId: "expectedTestCons", line: 1, @@ -133,6 +139,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b? c\n: d) : e", + output: "a ?\n(b\n? c\n: d)\n: e", errors: [{ messageId: "expectedConsAlt", line: 2, @@ -146,6 +153,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b\n? c : d) : e", + output: "a ?\n(b\n? c\n: d)\n: e", errors: [{ messageId: "expectedConsAlt", line: 2, @@ -159,6 +167,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b\n? c\n : d) : e", + output: "a ?\n(b\n? c\n : d)\n: e", errors: [{ messageId: "expectedConsAlt", line: 2, @@ -169,6 +178,7 @@ ruleTester.run("multiline-ternary", rule, { // "always" { code: "a ? b : c", + output: "a\n? b\n: c", options: ["always"], errors: [{ messageId: "expectedTestCons", @@ -183,6 +193,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a\n? b : c", + output: "a\n? b\n: c", options: ["always"], errors: [{ messageId: "expectedConsAlt", @@ -192,6 +203,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? b\n: c", + output: "a\n? b\n: c", options: ["always"], errors: [{ messageId: "expectedTestCons", @@ -201,6 +213,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? (b ? c : d) : e", + output: "a\n? (b\n? c\n: d)\n: e", options: ["always"], errors: [{ messageId: "expectedTestCons", @@ -225,6 +238,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b ? c : d) :\ne", + output: "a ?\n(b\n? c\n: d) :\ne", options: ["always"], errors: [{ messageId: "expectedTestCons", @@ -239,6 +253,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? (b\n? c\n: d) : e", + output: "a\n? (b\n? c\n: d)\n: e", options: ["always"], errors: [{ messageId: "expectedTestCons", @@ -253,6 +268,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b? c\n: d) : e", + output: "a ?\n(b\n? c\n: d)\n: e", options: ["always"], errors: [{ messageId: "expectedConsAlt", @@ -267,6 +283,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b\n? c : d) : e", + output: "a ?\n(b\n? c\n: d)\n: e", options: ["always"], errors: [{ messageId: "expectedConsAlt", @@ -281,6 +298,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b\n? c\n : d) : e", + output: "a ?\n(b\n? c\n : d)\n: e", options: ["always"], errors: [{ messageId: "expectedConsAlt", @@ -292,6 +310,7 @@ ruleTester.run("multiline-ternary", rule, { // "always-multiline" { code: "a\n? b : c", + output: "a\n? b\n: c", options: ["always-multiline"], errors: [{ messageId: "expectedConsAlt", @@ -301,6 +320,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? b\n: c", + output: "a\n? b\n: c", options: ["always-multiline"], errors: [{ messageId: "expectedTestCons", @@ -310,6 +330,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a &&\nb ? c : d", + output: "a &&\nb\n? c\n: d", options: ["always-multiline"], errors: [{ messageId: "expectedTestCons", @@ -324,6 +345,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? b +\nc : d", + output: "a\n? b +\nc\n: d", options: ["always-multiline"], errors: [{ messageId: "expectedTestCons", @@ -338,6 +360,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? b : c +\nd", + output: "a\n? b\n: c +\nd", options: ["always-multiline"], errors: [{ messageId: "expectedTestCons", @@ -352,6 +375,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b ? c : d) : e", + output: "a ?\n(b ? c : d)\n: e", options: ["always-multiline"], errors: [{ messageId: "expectedConsAlt", @@ -361,6 +385,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? (b ? c : d) :\ne", + output: "a\n? (b ? c : d) :\ne", options: ["always-multiline"], errors: [{ messageId: "expectedTestCons", @@ -370,6 +395,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? (b\n? c\n: d) : e", + output: "a\n? (b\n? c\n: d)\n: e", options: ["always-multiline"], errors: [{ messageId: "expectedTestCons", @@ -384,6 +410,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b ? c\n: d) : e", + output: "a ?\n(b\n? c\n: d)\n: e", options: ["always-multiline"], errors: [{ messageId: "expectedConsAlt", @@ -398,6 +425,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b\n? c : d) : e", + output: "a ?\n(b\n? c\n: d)\n: e", options: ["always-multiline"], errors: [{ messageId: "expectedConsAlt", @@ -412,6 +440,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b\n? c\n : d) : e", + output: "a ?\n(b\n? c\n : d)\n: e", options: ["always-multiline"], errors: [{ messageId: "expectedConsAlt", @@ -423,6 +452,7 @@ ruleTester.run("multiline-ternary", rule, { // "never" { code: "a\n? b : c", + output: "a ? b : c", options: ["never"], errors: [{ messageId: "unexpectedTestCons", @@ -432,6 +462,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? b\n: c", + output: "a ? b : c", options: ["never"], errors: [{ messageId: "unexpectedConsAlt", @@ -441,6 +472,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b ? c : d) :\ne", + output: "a ? (b ? c : d) : e", options: ["never"], errors: [{ messageId: "unexpectedTestCons", @@ -455,6 +487,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? (b\n? c\n: d) : e", + output: "a ? (b ? c : d) : e", options: ["never"], errors: [{ messageId: "unexpectedTestCons", @@ -469,6 +502,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b? c\n: d) : e", + output: "a ? (b? c : d) : e", options: ["never"], errors: [{ messageId: "unexpectedTestCons", @@ -483,6 +517,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b\n? c : d) : e", + output: "a ? (b ? c : d) : e", options: ["never"], errors: [{ messageId: "unexpectedTestCons", @@ -497,6 +532,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ?\n(b\n? c\n : d) : e", + output: "a ? (b ? c : d) : e", options: ["never"], errors: [{ messageId: "unexpectedTestCons", @@ -516,6 +552,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a ? (b\n? c\n: d)\n: e", + output: "a ? (b ? c : d) : e", options: ["never"], errors: [{ messageId: "unexpectedConsAlt", @@ -535,6 +572,7 @@ ruleTester.run("multiline-ternary", rule, { }, { code: "a\n?\n(b\n?\nc\n:\nd)\n:\ne", + output: "a ? (b ? c : d) : e", options: ["never"], errors: [{ messageId: "unexpectedTestCons",