Skip to content

Commit

Permalink
Update: Added auto-fix to multiline-ternary
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Sawtaytoes committed Mar 3, 2020
1 parent 472025f commit 5638b2f
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 24 deletions.
56 changes: 56 additions & 0 deletions docs/rules/multiline-ternary.md
Expand Up @@ -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.
Expand Down
121 changes: 97 additions & 24 deletions lib/rules/multiline-ternary.js
Expand Up @@ -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
//--------------------------------------------------------------------------
Expand All @@ -70,23 +55,111 @@ 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) {
return;
}

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: "
)
);
}
});
}
}
}
Expand Down

0 comments on commit 5638b2f

Please sign in to comment.