From 0e75c80d7017cd8848740978ca92b8cf59bb9206 Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Sun, 26 Jan 2020 15:32:49 -0500 Subject: [PATCH 01/47] Added rule and tests --- rules/prefer-ternary.js | 152 +++++++++++++++++++++++++++ test/prefer-ternary.js | 220 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 372 insertions(+) create mode 100644 rules/prefer-ternary.js create mode 100644 test/prefer-ternary.js diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js new file mode 100644 index 0000000000..783fb006c5 --- /dev/null +++ b/rules/prefer-ternary.js @@ -0,0 +1,152 @@ +'use strict'; +const getDocumentationUrl = require('./utils/get-documentation-url'); + +const schema = [ + { + oneOf: [ + { + enum: ['always'] + }, + { + type: 'object', + properties: { + assignment: { + enum: ['never', 'same', 'any'] + }, + return: {type: 'boolean'}, + call: {type: 'boolean'} + }, + additionalProperties: false + } + ] + } +]; + +const create = context => { + function parseOptions(options) { + let assignmentExpSame = true; + let assignmentExpAny = false; + let returnExp = true; + let callExp = false; + + if (typeof options === 'string') { + assignmentExpAny = true; + callExp = true; + } else if (typeof options === 'object' && options !== null) { + assignmentExpSame = options.assignment !== 'never'; + assignmentExpAny = options.assignment === 'any'; + returnExp = options.return !== false; + callExp = options.call === true; + } + + return {assignmentExpSame, + assignmentExpAny, + returnExp, + callExp + }; + } + + const options = parseOptions(context.options[0]); + + function checkIfStatement(node) { + if (checkConsequentAndAlternateLength(node) && + checkConsequentAndAlternateType(node)) { + context.report({ + node, + message: 'This `if` statement can be replaced by a ternary operator.', + fix(fixer) { + return fixFunction(node, fixer); + } + }); + } + } + + function checkConsequentAndAlternateLength(node) { + return checkConsequentOrAlternateLength(node.consequent) && + checkConsequentOrAlternateLength(node.alternate); + } + + function checkConsequentOrAlternateLength(consequentOrAlternateNode) { + return consequentOrAlternateNode && + consequentOrAlternateNode.body.length === 1; + } + + function checkConsequentAndAlternateType(node) { + return node.consequent.body[0].type === node.alternate.body[0].type && + (checkConsequentAndAlternateAssignment(node) || + checkConsequentAndAlternateReturn(node) || + checkConsequentAndAlternateCall(node)); + } + + function checkConsequentAndAlternateAssignment(node) { + return options.assignmentExpSame && + checkConsequentOrAlternateAssignment(node.consequent) && + checkConsequentOrAlternateAssignment(node.alternate) && + (options.assignmentExpAny || + compareConsequentAndAlternateAssignments(node) + ); + } + + function checkConsequentOrAlternateAssignment(consequentOrAlternateNode) { + return consequentOrAlternateNode.body[0].type === 'ExpressionStatement' && + consequentOrAlternateNode.body[0].expression.type === 'AssignmentExpression'; + } + + function compareConsequentAndAlternateAssignments(node) { + return node.consequent.body[0].expression.left.name === node.alternate.body[0].expression.left.name; + } + + function checkConsequentAndAlternateReturn(node) { + return options.returnExp && node.consequent.body[0].type === 'ReturnStatement'; + } + + function checkConsequentAndAlternateCall(node) { + return options.callExp && + node.consequent.body[0].type === 'ExpressionStatement' && + node.consequent.body[0].expression.type === 'CallExpression'; + } + + function fixFunction(node, fixer) { + let prefix = ''; + const ifCondition = node.test.name; + let left = ''; + let right = ''; + const sourceCode = context.getSourceCode(); + if (checkConsequentOrAlternateAssignment(node.consequent)) { + if (compareConsequentAndAlternateAssignments(node)) { + prefix = sourceCode.getText(node.consequent.body[0].expression.left) + ' = '; + left = sourceCode.getText(node.consequent.body[0].expression.right); + right = sourceCode.getText(node.alternate.body[0].expression.right); + } else { + left = sourceCode.getText(node.consequent.body[0].expression); + right = sourceCode.getText(node.alternate.body[0].expression); + } + } else if (node.consequent.body[0].type === 'ReturnStatement') { + prefix = 'return '; + left = sourceCode.getText(node.consequent.body[0].argument); + right = sourceCode.getText(node.alternate.body[0].argument); + } else { + left = sourceCode.getText(node.consequent.body[0].expression); + right = sourceCode.getText(node.alternate.body[0].expression); + } + + const replacement = prefix + ifCondition + ' ? ' + left + ' : ' + right; + return fixer.replaceText(node, replacement); + } + + return { + IfStatement: checkIfStatement + }; +}; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocumentationUrl(__filename) + }, + schema, + fixable: 'code' + } +}; diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js new file mode 100644 index 0000000000..1ce82194db --- /dev/null +++ b/test/prefer-ternary.js @@ -0,0 +1,220 @@ +import test from 'ava'; +import avaRuleTester from 'eslint-ava-rule-tester'; +import rule from '../rules/prefer-ternary'; + +const ruleTester = avaRuleTester(test, { + env: { + es6: true + } +}); + +ruleTester.run('prefer-ternary', rule, { + valid: [ + 'if(a){b = 1;}', + { + code: `if(a){ + b = 1 + doSomeStuff() + } + else{ + b = 2 + }` + }, + { + code: `if(a){ + b = 1 + } + else{ + c = 2 + }` + }, + { + code: `if(a){ + b = 1; + } + else{ + b = 2; + }`, + options: [{assignment: 'never'}] + }, + { + code: `if(a){ + b = 1; + } + else{ + c = 2; + }`, + options: [{assignment: 'same'}] + }, + { + code: `if(a){ + b = 1; + } + else{ + c = 2; + }`, + options: [{assignment: 'never'}] + }, + { + code: `function foo(){ + if(a){ + return 1; + } + else{ + return 2; + } + }`, + options: [{return: false}] + }, + { + code: `if(a){ + foo(); + } + else{ + bar(); + }` + }, + { + code: `if(a){ + foo(); + } + else{ + bar(); + }`, + options: [{call: false}] + } + + ], + + invalid: [ + { + code: `if(a){ + b = 1; + } + else{ + b = 2; + }`, + output: 'b = a ? 1 : 2', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, + { + code: `if(a){ + b = 1; + } + else{ + b = 2; + }`, + options: ['always'], + output: 'b = a ? 1 : 2', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, + { + code: `if(a){ + b = 1; + } + else{ + b = 2; + }`, + options: [{assignment: 'same'}], + output: 'b = a ? 1 : 2', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, + { + code: `if(a){ + b = 1; + } + else{ + b = 2; + }`, + options: [{assignment: 'any'}], + output: 'b = a ? 1 : 2', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, + + { + code: `if(a){ + b = 1; + } + else{ + c = 2; + }`, + options: ['always'], + output: 'a ? b = 1 : c = 2', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, + { + code: `if(a){ + b = 1; + } + else{ + c = 2; + }`, + options: [{assignment: 'any'}], + output: 'a ? b = 1 : c = 2', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, + + { + code: 'function foo(){if(a){return 1;}else{return 2;}}', + output: 'function foo(){return a ? 1 : 2}', + errors: [ + {column: 16, line: 1, type: 'IfStatement'} + ] + }, + { + code: 'function foo(){if(a){return 1;}else{return 2;}}', + options: ['always'], + output: 'function foo(){return a ? 1 : 2}', + errors: [ + {column: 16, line: 1, type: 'IfStatement'} + ] + }, + { + code: 'function foo(){if(a){return 1;}else{return 2;}}', + options: [{return: true}], + output: 'function foo(){return a ? 1 : 2}', + errors: [ + {column: 16, line: 1, type: 'IfStatement'} + ] + }, + + { + code: `if(a){ + foo(); + } + else{ + bar(); + }`, + options: ['always'], + output: 'a ? foo() : bar()', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, + { + code: `if(a){ + foo(param1, [param2, param3]); + } + else{ + bar(); + }`, + options: [{call: true}], + output: 'a ? foo(param1, [param2, param3]) : bar()', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + } + ] +}); From efc968a50c7e06b6aeb09b2c1f61cedcb6b20cc3 Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Fri, 7 Feb 2020 09:04:12 -0500 Subject: [PATCH 02/47] Added documentation --- docs/rules/prefer-ternary.md | 168 +++++++++++++++++++++++++++++++++++ index.js | 1 + readme.md | 2 + 3 files changed, 171 insertions(+) create mode 100644 docs/rules/prefer-ternary.md diff --git a/docs/rules/prefer-ternary.md b/docs/rules/prefer-ternary.md new file mode 100644 index 0000000000..7fe090baa0 --- /dev/null +++ b/docs/rules/prefer-ternary.md @@ -0,0 +1,168 @@ +# Prefer ternary expressions over simple `if/then/else` statements + +This rule enforces the use of ternary expressions over 'simple' `if/then/else` statements where 'simple' means the consequent and alternate are each one line and have the same basic type and form. + +Using an `if/then/else` statement typically results in more lines of code than a single lined ternary expression, which leads to an unnecessarily larger codebase that is more difficult to maintain. + +Additionally, using an `if/then/else` statement can result in defining variables using `let` or `var` solely to be reassigned within the blocks. This leads to varaibles being unnecessarily mutable and prevents `prefer-const` from flagging the variable. + +This rule is fixable. + + +## Fail + +```js +let foo ='' +if(bar){ + foo = 3 +} +else{ + foo = 4 +} +``` + +```js +if(bar){ + return 3 +} +else{ + return 4 +} +``` + +## Pass + +```js +let foo = bar ? 3 : 4 +``` + +```js +return bar ? 3 : 4 +``` + + +```js +let foo = '' +if(bar){ + baz() + foo = 3 +} +else{ + foo = 4 +} +``` + +```js +if(bar){ + foo = 3 +} +else{ + return 4 +} +``` + +## Options + +This rule can take the following options: +* An object with the following keys: 'assignment', 'return', 'call' +* The string 'always' + +### assignment +The assignment option determines whether the rule will flag assignment expressions. It can take the following values: 'never', 'same', 'always'. Default value is 'same'. + +**never**: the rule will not flag any assignment statements. +With `{assigment: 'never'}` the following would both NOT be flagged: +```js +let foo ='' +if(bar){ + foo = 3 +} +else{ + foo = 4 +} +``` + +```js +let foo ='' +if(bar){ + foo = 3 +} +else{ + baz = 4 +} +``` + +**same**: the rule will flag assignment statements assigning to the same variable. +With `{assigment: 'same'}` the following would be flagged: +```js +let foo ='' +if(bar){ + foo = 3 +} +else{ + foo = 4 +} +``` +With `{assigment: 'same'}` the following would NOT be flagged: +```js +let foo ='' +if(bar){ + foo = 3 +} +else{ + baz = 4 +} +``` + + +**always**: the rule will flag all assignment statements. +With `{assigment: 'always'}` the following would both be flagged: +```js +let foo ='' +if(bar){ + foo = 3 +} +else{ + foo = 4 +} +``` + +```js +let foo ='' +if(bar){ + foo = 3 +} +else{ + baz = 4 +} +``` + +### return +The return option determines whether the rule will flag return expressions. It can take a boolean. Default value is true. +With `{return: false}` the following would NOT be flagged: +```js +let foo ='' +if(bar){ + return 3 +} +else{ + return 4 +} +``` + +### call +The call option determines whether the rule will flag call expressions. It can take a boolean. Default value is false. +With `{call: true}` the following would be flagged: +```js +if(bar){ + foo() +} +else{ + baz() +} +``` + + +### 'always' + +Always prefer ternary to simple `if/then/else` statements. This option is equivalent to ```{assignment: 'always', return: true, call:true}```. \ No newline at end of file diff --git a/index.js b/index.js index c83f2ea4cf..893f2e4783 100644 --- a/index.js +++ b/index.js @@ -57,6 +57,7 @@ module.exports = { 'unicorn/prefer-spread': 'error', 'unicorn/prefer-starts-ends-with': 'error', 'unicorn/prefer-string-slice': 'error', + 'unicorn/prefer-ternary': 'off', 'unicorn/prefer-text-content': 'error', 'unicorn/prefer-trim-start-end': 'error', 'unicorn/prefer-type-error': 'error', diff --git a/readme.md b/readme.md index 3f4eb2b39c..fea6a3fc23 100644 --- a/readme.md +++ b/readme.md @@ -75,6 +75,7 @@ Configure it in `package.json`. "unicorn/prefer-spread": "error", "unicorn/prefer-starts-ends-with": "error", "unicorn/prefer-string-slice": "error", + "unicorn/prefer-ternary": "off", "unicorn/prefer-text-content": "error", "unicorn/prefer-trim-start-end": "error", "unicorn/prefer-type-error": "error", @@ -128,6 +129,7 @@ Configure it in `package.json`. - [prefer-spread](docs/rules/prefer-spread.md) - Prefer the spread operator over `Array.from()`. *(fixable)* - [prefer-starts-ends-with](docs/rules/prefer-starts-ends-with.md) - Prefer `String#startsWith()` & `String#endsWith()` over more complex alternatives. - [prefer-string-slice](docs/rules/prefer-string-slice.md) - Prefer `String#slice()` over `String#substr()` and `String#substring()`. *(partly fixable)* +- [prefer-ternary](docs/rules/prefer-ternary.md) - Prefer ternary expressions over simple `if/then/else` statements. *(fixable)* - [prefer-text-content](docs/rules/prefer-text-content.md) - Prefer `.textContent` over `.innerText`. *(fixable)* - [prefer-trim-start-end](docs/rules/prefer-trim-start-end.md) - Prefer `String#trimStart()` / `String#trimEnd()` over `String#trimLeft()` / `String#trimRight()`. *(fixable)* - [prefer-type-error](docs/rules/prefer-type-error.md) - Enforce throwing `TypeError` in type checking conditions. *(fixable)* From 3fffe86c7ac9f7fb23c1e9ba3a3c90148156f8f3 Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Fri, 14 Feb 2020 14:44:48 -0500 Subject: [PATCH 03/47] Tweaked language; adding throw, new, yield, await --- docs/rules/prefer-ternary.md | 10 ++-- index.js | 2 +- readme.md | 2 +- rules/prefer-ternary.js | 109 ++++++++++++++++++----------------- 4 files changed, 64 insertions(+), 59 deletions(-) diff --git a/docs/rules/prefer-ternary.md b/docs/rules/prefer-ternary.md index 7fe090baa0..a8d35572d4 100644 --- a/docs/rules/prefer-ternary.md +++ b/docs/rules/prefer-ternary.md @@ -1,10 +1,10 @@ -# Prefer ternary expressions over simple `if/then/else` statements +# Prefer ternary expressions over simple `if-else` statements -This rule enforces the use of ternary expressions over 'simple' `if/then/else` statements where 'simple' means the consequent and alternate are each one line and have the same basic type and form. +This rule enforces the use of ternary expressions over 'simple' `if-else` statements where 'simple' means the consequent and alternate are each one line and have the same basic type and form. -Using an `if/then/else` statement typically results in more lines of code than a single lined ternary expression, which leads to an unnecessarily larger codebase that is more difficult to maintain. +Using an `if-else` statement typically results in more lines of code than a single lined ternary expression, which leads to an unnecessarily larger codebase that is more difficult to maintain. -Additionally, using an `if/then/else` statement can result in defining variables using `let` or `var` solely to be reassigned within the blocks. This leads to varaibles being unnecessarily mutable and prevents `prefer-const` from flagging the variable. +Additionally, using an `if-else` statement can result in defining variables using `let` or `var` solely to be reassigned within the blocks. This leads to varaibles being unnecessarily mutable and prevents `prefer-const` from flagging the variable. This rule is fixable. @@ -165,4 +165,4 @@ else{ ### 'always' -Always prefer ternary to simple `if/then/else` statements. This option is equivalent to ```{assignment: 'always', return: true, call:true}```. \ No newline at end of file +Always prefer ternary to simple `if-else` statements. This option is equivalent to ```{assignment: 'always', return: true, call:true}```. \ No newline at end of file diff --git a/index.js b/index.js index 893f2e4783..283caec8e7 100644 --- a/index.js +++ b/index.js @@ -57,7 +57,7 @@ module.exports = { 'unicorn/prefer-spread': 'error', 'unicorn/prefer-starts-ends-with': 'error', 'unicorn/prefer-string-slice': 'error', - 'unicorn/prefer-ternary': 'off', + 'unicorn/prefer-ternary': 'error', 'unicorn/prefer-text-content': 'error', 'unicorn/prefer-trim-start-end': 'error', 'unicorn/prefer-type-error': 'error', diff --git a/readme.md b/readme.md index fea6a3fc23..a0841db62d 100644 --- a/readme.md +++ b/readme.md @@ -129,7 +129,7 @@ Configure it in `package.json`. - [prefer-spread](docs/rules/prefer-spread.md) - Prefer the spread operator over `Array.from()`. *(fixable)* - [prefer-starts-ends-with](docs/rules/prefer-starts-ends-with.md) - Prefer `String#startsWith()` & `String#endsWith()` over more complex alternatives. - [prefer-string-slice](docs/rules/prefer-string-slice.md) - Prefer `String#slice()` over `String#substr()` and `String#substring()`. *(partly fixable)* -- [prefer-ternary](docs/rules/prefer-ternary.md) - Prefer ternary expressions over simple `if/then/else` statements. *(fixable)* +- [prefer-ternary](docs/rules/prefer-ternary.md) - Prefer ternary expressions over simple `if-else` statements. *(fixable)* - [prefer-text-content](docs/rules/prefer-text-content.md) - Prefer `.textContent` over `.innerText`. *(fixable)* - [prefer-trim-start-end](docs/rules/prefer-trim-start-end.md) - Prefer `String#trimStart()` / `String#trimEnd()` over `String#trimLeft()` / `String#trimRight()`. *(fixable)* - [prefer-type-error](docs/rules/prefer-type-error.md) - Enforce throwing `TypeError` in type checking conditions. *(fixable)* diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 783fb006c5..3cc042b9bd 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -14,7 +14,11 @@ const schema = [ enum: ['never', 'same', 'any'] }, return: {type: 'boolean'}, - call: {type: 'boolean'} + call: {type: 'boolean'}, + new: {type: 'boolean'}, + throw: {type: 'boolean'}, + yield: {type: 'boolean'}, + await: {type: 'boolean'} }, additionalProperties: false } @@ -24,27 +28,27 @@ const schema = [ const create = context => { function parseOptions(options) { - let assignmentExpSame = true; - let assignmentExpAny = false; - let returnExp = true; - let callExp = false; + const optionsDefined = options ? options : {} + const optionsObject = + { + AssignmentExpression: optionsDefined.assignment ? optionsDefined.assignment : 'same', + ReturnStatement: optionsDefined.return !== false, + CallExpression: optionsDefined.call === true, + NewExpression: optionsDefined.new === true, + ThrowStatement: optionsDefined.throw !== false, + YieldExpression: optionsDefined.yield !== false, + AwaitExpression: optionsDefined.await === true + } if (typeof options === 'string') { - assignmentExpAny = true; - callExp = true; - } else if (typeof options === 'object' && options !== null) { - assignmentExpSame = options.assignment !== 'never'; - assignmentExpAny = options.assignment === 'any'; - returnExp = options.return !== false; - callExp = options.call === true; - } - - return {assignmentExpSame, - assignmentExpAny, - returnExp, - callExp - }; + optionsObject.AssignmentExpression = 'any'; + optionsObject.CallExpression = true; + optionsObject.NewExpression = true; + optionsObject.AwaitExpression = true; + } + return optionsObject +} const options = parseOptions(context.options[0]); @@ -72,56 +76,57 @@ const create = context => { } function checkConsequentAndAlternateType(node) { - return node.consequent.body[0].type === node.alternate.body[0].type && - (checkConsequentAndAlternateAssignment(node) || - checkConsequentAndAlternateReturn(node) || - checkConsequentAndAlternateCall(node)); + const consequentType = node.consequent.body[0].type + return consequentType === node.alternate.body[0].type && + ((Object.keys(options).includes(consequentType) && options[consequentType]) || + consequentType === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)) } - function checkConsequentAndAlternateAssignment(node) { - return options.assignmentExpSame && - checkConsequentOrAlternateAssignment(node.consequent) && - checkConsequentOrAlternateAssignment(node.alternate) && - (options.assignmentExpAny || - compareConsequentAndAlternateAssignments(node) - ); + function checkConsequentAndAlternateExpressionStatement(node){ + const consequentType = node.consequent.body[0].expression.type + return consequentType === node.alternate.body[0].expression.type && + (consequentType === "AssignmentExpression" ? checkConsequentAndAlternateAssignment(node) : + (Object.keys(options).includes(consequentType) && options[consequentType])) } - function checkConsequentOrAlternateAssignment(consequentOrAlternateNode) { - return consequentOrAlternateNode.body[0].type === 'ExpressionStatement' && - consequentOrAlternateNode.body[0].expression.type === 'AssignmentExpression'; + function checkConsequentAndAlternateAssignment(node) { + return options.AssignmentExpression === 'any' || + (options.AssignmentExpression === 'same' && compareConsequentAndAlternateAssignments(node)) } function compareConsequentAndAlternateAssignments(node) { return node.consequent.body[0].expression.left.name === node.alternate.body[0].expression.left.name; } - function checkConsequentAndAlternateReturn(node) { - return options.returnExp && node.consequent.body[0].type === 'ReturnStatement'; - } - - function checkConsequentAndAlternateCall(node) { - return options.callExp && - node.consequent.body[0].type === 'ExpressionStatement' && - node.consequent.body[0].expression.type === 'CallExpression'; - } - + function fixFunction(node, fixer) { let prefix = ''; const ifCondition = node.test.name; let left = ''; let right = ''; const sourceCode = context.getSourceCode(); - if (checkConsequentOrAlternateAssignment(node.consequent)) { - if (compareConsequentAndAlternateAssignments(node)) { - prefix = sourceCode.getText(node.consequent.body[0].expression.left) + ' = '; - left = sourceCode.getText(node.consequent.body[0].expression.right); - right = sourceCode.getText(node.alternate.body[0].expression.right); - } else { - left = sourceCode.getText(node.consequent.body[0].expression); - right = sourceCode.getText(node.alternate.body[0].expression); + if (node.consequent.body[0].type==='ExpressionStatement'){ + if(node.consequent.body[0].expression.type==='AssignmentExpression') { + if (compareConsequentAndAlternateAssignments(node)) { + prefix = sourceCode.getText(node.consequent.body[0].expression.left) + ' = '; + left = sourceCode.getText(node.consequent.body[0].expression.right); + right = sourceCode.getText(node.alternate.body[0].expression.right); + } else { + left = sourceCode.getText(node.consequent.body[0].expression); + right = sourceCode.getText(node.alternate.body[0].expression); + } + } + else if(node.consequent.body[0].expression.type==='CallExpression'){ + left = sourceCode.getText(node.consequent.body[0].expression); + right = sourceCode.getText(node.alternate.body[0].expression); + } + else{ + prefix = node.consequent.body[0].expression.type.replace('Expression','').toLowerCase() + left = sourceCode.getText(node.consequent.body[0].expression.argument) + right = sourceCode.getText(node.alternate.body[0].expression.argument) } - } else if (node.consequent.body[0].type === 'ReturnStatement') { + } + else if (node.consequent.body[0].type === 'ReturnStatement') { prefix = 'return '; left = sourceCode.getText(node.consequent.body[0].argument); right = sourceCode.getText(node.alternate.body[0].argument); From 80a3a155523a2f231eb4d1823860569a7c9e4dec Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Mon, 17 Feb 2020 10:35:35 -0500 Subject: [PATCH 04/47] Added tests and changed fix function --- .vscode/launch.json | 46 ++++++++++++++++++++++ rules/prefer-ternary.js | 84 ++++++++++++++++++----------------------- test/prefer-ternary.js | 66 ++++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 48 deletions(-) create mode 100644 .vscode/launch.json diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 0000000000..7fc2812634 --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,46 @@ +{ + "version": "0.2.0", + "configurations": [ + { + "type": "node", + "request": "launch", + "name": "AVA", + "program": "${workspaceFolder}/node_modules/ava/profile.js", + "args": [ + "${file}" + ], + "console": "integratedTerminal", + "skipFiles": [ + "/**/*.js" + ] + }, + { + "type": "node", + "request": "launch", + "name": "Mocha All", + "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha", + "args": [ + "--timeout", + "999999", + "--colors", + "${workspaceFolder}/tests/" + ], + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen" + }, + { + "type": "node", + "request": "launch", + "name": "Mocha Current File", + "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha", + "args": [ + "--timeout", + "999999", + "--colors", + "${file}" + ], + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen" + } + ] +} \ No newline at end of file diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 3cc042b9bd..8988831510 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -28,27 +28,27 @@ const schema = [ const create = context => { function parseOptions(options) { - const optionsDefined = options ? options : {} - const optionsObject = - { - AssignmentExpression: optionsDefined.assignment ? optionsDefined.assignment : 'same', - ReturnStatement: optionsDefined.return !== false, - CallExpression: optionsDefined.call === true, - NewExpression: optionsDefined.new === true, - ThrowStatement: optionsDefined.throw !== false, - YieldExpression: optionsDefined.yield !== false, - AwaitExpression: optionsDefined.await === true - } + const optionsDefined = options ? options : {}; + const optionsObject = + { + AssignmentExpression: optionsDefined.assignment ? optionsDefined.assignment : 'same', + ReturnStatement: optionsDefined.return !== false, + CallExpression: optionsDefined.call === true, + NewExpression: optionsDefined.new === true, + ThrowStatement: optionsDefined.throw !== false, + YieldExpression: optionsDefined.yield !== false, + AwaitExpression: optionsDefined.await === true + }; if (typeof options === 'string') { optionsObject.AssignmentExpression = 'any'; optionsObject.CallExpression = true; optionsObject.NewExpression = true; optionsObject.AwaitExpression = true; - + } + + return optionsObject; } - return optionsObject -} const options = parseOptions(context.options[0]); @@ -76,60 +76,48 @@ const create = context => { } function checkConsequentAndAlternateType(node) { - const consequentType = node.consequent.body[0].type - return consequentType === node.alternate.body[0].type && + const consequentType = node.consequent.body[0].type; + return (consequentType === node.alternate.body[0].type && ((Object.keys(options).includes(consequentType) && options[consequentType]) || - consequentType === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)) + (consequentType === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)))); } - function checkConsequentAndAlternateExpressionStatement(node){ - const consequentType = node.consequent.body[0].expression.type - return consequentType === node.alternate.body[0].expression.type && - (consequentType === "AssignmentExpression" ? checkConsequentAndAlternateAssignment(node) : - (Object.keys(options).includes(consequentType) && options[consequentType])) + function checkConsequentAndAlternateExpressionStatement(node) { + const consequentType = node.consequent.body[0].expression.type; + return consequentType === node.alternate.body[0].expression.type && + (consequentType === 'AssignmentExpression' ? checkConsequentAndAlternateAssignment(node) : + (Object.keys(options).includes(consequentType) && options[consequentType])); } function checkConsequentAndAlternateAssignment(node) { return options.AssignmentExpression === 'any' || - (options.AssignmentExpression === 'same' && compareConsequentAndAlternateAssignments(node)) + (options.AssignmentExpression === 'same' && compareConsequentAndAlternateAssignments(node)); } function compareConsequentAndAlternateAssignments(node) { return node.consequent.body[0].expression.left.name === node.alternate.body[0].expression.left.name; } - function fixFunction(node, fixer) { + const sourceCode = context.getSourceCode(); let prefix = ''; - const ifCondition = node.test.name; + const ifCondition = sourceCode.getText(node.test); let left = ''; let right = ''; - const sourceCode = context.getSourceCode(); - if (node.consequent.body[0].type==='ExpressionStatement'){ - if(node.consequent.body[0].expression.type==='AssignmentExpression') { - if (compareConsequentAndAlternateAssignments(node)) { - prefix = sourceCode.getText(node.consequent.body[0].expression.left) + ' = '; - left = sourceCode.getText(node.consequent.body[0].expression.right); - right = sourceCode.getText(node.alternate.body[0].expression.right); - } else { - left = sourceCode.getText(node.consequent.body[0].expression); - right = sourceCode.getText(node.alternate.body[0].expression); - } - } - else if(node.consequent.body[0].expression.type==='CallExpression'){ - left = sourceCode.getText(node.consequent.body[0].expression); - right = sourceCode.getText(node.alternate.body[0].expression); - } - else{ - prefix = node.consequent.body[0].expression.type.replace('Expression','').toLowerCase() - left = sourceCode.getText(node.consequent.body[0].expression.argument) - right = sourceCode.getText(node.alternate.body[0].expression.argument) - } - } - else if (node.consequent.body[0].type === 'ReturnStatement') { + if (node.consequent.body[0].type === 'ExpressionStatement' && + node.consequent.body[0].expression.type === 'AssignmentExpression' && + compareConsequentAndAlternateAssignments(node)) { + prefix = sourceCode.getText(node.consequent.body[0].expression.left) + ' = '; + left = sourceCode.getText(node.consequent.body[0].expression.right); + right = sourceCode.getText(node.alternate.body[0].expression.right); + } else if (node.consequent.body[0].type === 'ReturnStatement') { prefix = 'return '; left = sourceCode.getText(node.consequent.body[0].argument); right = sourceCode.getText(node.alternate.body[0].argument); + } else if (node.consequent.body[0].type === 'ThrowStatement') { + prefix = 'throw '; + left = sourceCode.getText(node.consequent.body[0].argument); + right = sourceCode.getText(node.alternate.body[0].argument); } else { left = sourceCode.getText(node.consequent.body[0].expression); right = sourceCode.getText(node.alternate.body[0].expression); diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 1ce82194db..0cbeee49fe 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -215,6 +215,72 @@ ruleTester.run('prefer-ternary', rule, { errors: [ {column: 1, line: 1, type: 'IfStatement'} ] + }, + { + code: `async () => { + if(a){ + await foo(); + } + else{ + await bar(); + } + }`, + parserOptions: {ecmaVersion: 2018}, + options: [{await: true}], + output: `async () => { + a ? await foo() : await bar() + }`, + errors: [ + {column: 5, line: 2, type: 'IfStatement'} + ] + }, + { + code: `if(a){ + new foo(); + } + else{ + new bar(); + }`, + options: [{new: true}], + output: 'a ? new foo() : new bar()', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, + { + code: `if(a){ + throw Error(123); + } + else{ + throw Error(456); + }`, + options: [{throw: true}], + output: 'throw a ? Error(123) : Error(456)', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, + { + code: `function* foo(index) { + while (index < 10) { + if(index < 3){ + yield index++; + } + else{ + yield index * 2 + } + } + }`, + options: [{yield: true}], + output: `function* foo(index) { + while (index < 10) { + index < 3 ? yield index++ : yield index * 2 + } + }`, + errors: [ + {column: 7, line: 3, type: 'IfStatement'} + ] } + ] }); From 3b4d6ab4ecfed7b2f82ae40ddb50f3708fa06e2b Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Mon, 17 Feb 2020 10:47:18 -0500 Subject: [PATCH 05/47] Added new options to docs; made yield false by default --- docs/rules/prefer-ternary.md | 71 +++++++++++++++++++++++++++++++++++- rules/prefer-ternary.js | 2 +- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/docs/rules/prefer-ternary.md b/docs/rules/prefer-ternary.md index a8d35572d4..c4a882221e 100644 --- a/docs/rules/prefer-ternary.md +++ b/docs/rules/prefer-ternary.md @@ -30,6 +30,15 @@ else{ } ``` +```js +if(bar){ + throw Error(123) +} +else{ + throw Error(456) +} +``` + ## Pass ```js @@ -61,10 +70,14 @@ else{ } ``` +```js +throw bar ? Error(123) : Error(456) +``` + ## Options This rule can take the following options: -* An object with the following keys: 'assignment', 'return', 'call' +* An object with the following keys: 'assignment', 'return', 'call', 'throw', 'new', 'yield', 'await' * The string 'always' ### assignment @@ -162,7 +175,61 @@ else{ } ``` +### throw +The throw option determines whether the rule will flag throw statements. It can take a boolean. Default value is true. +With `{thow: false}` the following would NOT be flagged: +```js +if(bar){ + throw Error(123) +} +else{ + throw Error(456) +} +``` + +### new +The new option determines whether the rule will flag new constructors. It can take a boolean. Default value is false. +With `{new: true}` the following would be flagged: +```js +if(bar){ + new foo() +} +else{ + new baz() +} +``` + +### yield +The yield option determines whether the rule will flag yield expressions. It can take a boolean. Default value is false. +With `{yield: true}` the following would be flagged: +```js +function* foo(index) { + while (index < 10) { + if(index < 3){ + yield index++; + } + else{ + yield index * 2 + } + } +} +``` + +### await +The await option determines whether the rule will flag await expressions. It can take a boolean. Default value is false. +With `{await: true}` the following would be flagged: +```js +async () => { + if(a){ + await foo(); + } + else{ + await bar(); + } +} +``` + ### 'always' -Always prefer ternary to simple `if-else` statements. This option is equivalent to ```{assignment: 'always', return: true, call:true}```. \ No newline at end of file +Always prefer ternary to simple `if-else` statements. This option is equivalent to ```{assignment: 'always', return: true, call:true, throw: true, new: true, yield: true, await: true}```. \ No newline at end of file diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 8988831510..b05a9e44da 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -36,7 +36,7 @@ const create = context => { CallExpression: optionsDefined.call === true, NewExpression: optionsDefined.new === true, ThrowStatement: optionsDefined.throw !== false, - YieldExpression: optionsDefined.yield !== false, + YieldExpression: optionsDefined.yield === false, AwaitExpression: optionsDefined.await === true }; From b210f43f54fae1618363e9a341a4616998dd0eb4 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Mon, 17 Feb 2020 22:53:40 +0700 Subject: [PATCH 06/47] Delete launch.json --- .vscode/launch.json | 46 --------------------------------------------- 1 file changed, 46 deletions(-) delete mode 100644 .vscode/launch.json diff --git a/.vscode/launch.json b/.vscode/launch.json deleted file mode 100644 index 7fc2812634..0000000000 --- a/.vscode/launch.json +++ /dev/null @@ -1,46 +0,0 @@ -{ - "version": "0.2.0", - "configurations": [ - { - "type": "node", - "request": "launch", - "name": "AVA", - "program": "${workspaceFolder}/node_modules/ava/profile.js", - "args": [ - "${file}" - ], - "console": "integratedTerminal", - "skipFiles": [ - "/**/*.js" - ] - }, - { - "type": "node", - "request": "launch", - "name": "Mocha All", - "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha", - "args": [ - "--timeout", - "999999", - "--colors", - "${workspaceFolder}/tests/" - ], - "console": "integratedTerminal", - "internalConsoleOptions": "neverOpen" - }, - { - "type": "node", - "request": "launch", - "name": "Mocha Current File", - "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha", - "args": [ - "--timeout", - "999999", - "--colors", - "${file}" - ], - "console": "integratedTerminal", - "internalConsoleOptions": "neverOpen" - } - ] -} \ No newline at end of file From eb7aace54e4d24f1c1eee8f1bf815e677e57a571 Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Sat, 29 Feb 2020 16:12:08 -0500 Subject: [PATCH 07/47] Simplified rule --- docs/rules/prefer-ternary.md | 164 ------------------ rules/prefer-ternary.js | 100 +++-------- test/prefer-ternary.js | 323 +++++++++-------------------------- 3 files changed, 96 insertions(+), 491 deletions(-) diff --git a/docs/rules/prefer-ternary.md b/docs/rules/prefer-ternary.md index c4a882221e..52d8af0fa7 100644 --- a/docs/rules/prefer-ternary.md +++ b/docs/rules/prefer-ternary.md @@ -30,15 +30,6 @@ else{ } ``` -```js -if(bar){ - throw Error(123) -} -else{ - throw Error(456) -} -``` - ## Pass ```js @@ -71,54 +62,6 @@ else{ ``` ```js -throw bar ? Error(123) : Error(456) -``` - -## Options - -This rule can take the following options: -* An object with the following keys: 'assignment', 'return', 'call', 'throw', 'new', 'yield', 'await' -* The string 'always' - -### assignment -The assignment option determines whether the rule will flag assignment expressions. It can take the following values: 'never', 'same', 'always'. Default value is 'same'. - -**never**: the rule will not flag any assignment statements. -With `{assigment: 'never'}` the following would both NOT be flagged: -```js -let foo ='' -if(bar){ - foo = 3 -} -else{ - foo = 4 -} -``` - -```js -let foo ='' -if(bar){ - foo = 3 -} -else{ - baz = 4 -} -``` - -**same**: the rule will flag assignment statements assigning to the same variable. -With `{assigment: 'same'}` the following would be flagged: -```js -let foo ='' -if(bar){ - foo = 3 -} -else{ - foo = 4 -} -``` -With `{assigment: 'same'}` the following would NOT be flagged: -```js -let foo ='' if(bar){ foo = 3 } @@ -126,110 +69,3 @@ else{ baz = 4 } ``` - - -**always**: the rule will flag all assignment statements. -With `{assigment: 'always'}` the following would both be flagged: -```js -let foo ='' -if(bar){ - foo = 3 -} -else{ - foo = 4 -} -``` - -```js -let foo ='' -if(bar){ - foo = 3 -} -else{ - baz = 4 -} -``` - -### return -The return option determines whether the rule will flag return expressions. It can take a boolean. Default value is true. -With `{return: false}` the following would NOT be flagged: -```js -let foo ='' -if(bar){ - return 3 -} -else{ - return 4 -} -``` - -### call -The call option determines whether the rule will flag call expressions. It can take a boolean. Default value is false. -With `{call: true}` the following would be flagged: -```js -if(bar){ - foo() -} -else{ - baz() -} -``` - -### throw -The throw option determines whether the rule will flag throw statements. It can take a boolean. Default value is true. -With `{thow: false}` the following would NOT be flagged: -```js -if(bar){ - throw Error(123) -} -else{ - throw Error(456) -} -``` - -### new -The new option determines whether the rule will flag new constructors. It can take a boolean. Default value is false. -With `{new: true}` the following would be flagged: -```js -if(bar){ - new foo() -} -else{ - new baz() -} -``` - -### yield -The yield option determines whether the rule will flag yield expressions. It can take a boolean. Default value is false. -With `{yield: true}` the following would be flagged: -```js -function* foo(index) { - while (index < 10) { - if(index < 3){ - yield index++; - } - else{ - yield index * 2 - } - } -} -``` - -### await -The await option determines whether the rule will flag await expressions. It can take a boolean. Default value is false. -With `{await: true}` the following would be flagged: -```js -async () => { - if(a){ - await foo(); - } - else{ - await bar(); - } -} -``` - - -### 'always' - -Always prefer ternary to simple `if-else` statements. This option is equivalent to ```{assignment: 'always', return: true, call:true, throw: true, new: true, yield: true, await: true}```. \ No newline at end of file diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index b05a9e44da..6071acd649 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -1,60 +1,10 @@ 'use strict'; const getDocumentationUrl = require('./utils/get-documentation-url'); -const schema = [ - { - oneOf: [ - { - enum: ['always'] - }, - { - type: 'object', - properties: { - assignment: { - enum: ['never', 'same', 'any'] - }, - return: {type: 'boolean'}, - call: {type: 'boolean'}, - new: {type: 'boolean'}, - throw: {type: 'boolean'}, - yield: {type: 'boolean'}, - await: {type: 'boolean'} - }, - additionalProperties: false - } - ] - } -]; - const create = context => { - function parseOptions(options) { - const optionsDefined = options ? options : {}; - const optionsObject = - { - AssignmentExpression: optionsDefined.assignment ? optionsDefined.assignment : 'same', - ReturnStatement: optionsDefined.return !== false, - CallExpression: optionsDefined.call === true, - NewExpression: optionsDefined.new === true, - ThrowStatement: optionsDefined.throw !== false, - YieldExpression: optionsDefined.yield === false, - AwaitExpression: optionsDefined.await === true - }; - - if (typeof options === 'string') { - optionsObject.AssignmentExpression = 'any'; - optionsObject.CallExpression = true; - optionsObject.NewExpression = true; - optionsObject.AwaitExpression = true; - } - - return optionsObject; - } - - const options = parseOptions(context.options[0]); - function checkIfStatement(node) { if (checkConsequentAndAlternateLength(node) && - checkConsequentAndAlternateType(node)) { + checkConsequentAndAlternateType(node)) { context.report({ node, message: 'This `if` statement can be replaced by a ternary operator.', @@ -67,31 +17,25 @@ const create = context => { function checkConsequentAndAlternateLength(node) { return checkConsequentOrAlternateLength(node.consequent) && - checkConsequentOrAlternateLength(node.alternate); + checkConsequentOrAlternateLength(node.alternate); } - function checkConsequentOrAlternateLength(consequentOrAlternateNode) { - return consequentOrAlternateNode && - consequentOrAlternateNode.body.length === 1; + function checkConsequentOrAlternateLength(node) { + return node && node.body.length === 1; } function checkConsequentAndAlternateType(node) { const consequentType = node.consequent.body[0].type; return (consequentType === node.alternate.body[0].type && - ((Object.keys(options).includes(consequentType) && options[consequentType]) || - (consequentType === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)))); + (consequentType === 'ReturnStatement' || + (consequentType === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)))); } function checkConsequentAndAlternateExpressionStatement(node) { const consequentType = node.consequent.body[0].expression.type; return consequentType === node.alternate.body[0].expression.type && - (consequentType === 'AssignmentExpression' ? checkConsequentAndAlternateAssignment(node) : - (Object.keys(options).includes(consequentType) && options[consequentType])); - } - - function checkConsequentAndAlternateAssignment(node) { - return options.AssignmentExpression === 'any' || - (options.AssignmentExpression === 'same' && compareConsequentAndAlternateAssignments(node)); + (consequentType === 'AssignmentExpression' ? compareConsequentAndAlternateAssignments(node) : + consequentType === 'YieldExpression'); } function compareConsequentAndAlternateAssignments(node) { @@ -104,26 +48,23 @@ const create = context => { const ifCondition = sourceCode.getText(node.test); let left = ''; let right = ''; - if (node.consequent.body[0].type === 'ExpressionStatement' && - node.consequent.body[0].expression.type === 'AssignmentExpression' && - compareConsequentAndAlternateAssignments(node)) { - prefix = sourceCode.getText(node.consequent.body[0].expression.left) + ' = '; - left = sourceCode.getText(node.consequent.body[0].expression.right); - right = sourceCode.getText(node.alternate.body[0].expression.right); - } else if (node.consequent.body[0].type === 'ReturnStatement') { + if (node.consequent.body[0].type === 'ExpressionStatement') { + if (node.consequent.body[0].expression.type === 'AssignmentExpression') { + prefix = sourceCode.getText(node.consequent.body[0].expression.left) + ' = '; + left = sourceCode.getText(node.consequent.body[0].expression.right); + right = sourceCode.getText(node.alternate.body[0].expression.right); + } else { + prefix = 'yield '; + left = sourceCode.getText(node.consequent.body[0].expression.argument); + right = sourceCode.getText(node.alternate.body[0].expression.argument); + } + } else { prefix = 'return '; left = sourceCode.getText(node.consequent.body[0].argument); right = sourceCode.getText(node.alternate.body[0].argument); - } else if (node.consequent.body[0].type === 'ThrowStatement') { - prefix = 'throw '; - left = sourceCode.getText(node.consequent.body[0].argument); - right = sourceCode.getText(node.alternate.body[0].argument); - } else { - left = sourceCode.getText(node.consequent.body[0].expression); - right = sourceCode.getText(node.alternate.body[0].expression); } - const replacement = prefix + ifCondition + ' ? ' + left + ' : ' + right; + const replacement = prefix + '(' + ifCondition + ' ? ' + left + ' : ' + right + ')'; return fixer.replaceText(node, replacement); } @@ -139,7 +80,6 @@ module.exports = { docs: { url: getDocumentationUrl(__filename) }, - schema, fixable: 'code' } }; diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 0cbeee49fe..cf1c4315c1 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -1,5 +1,6 @@ import test from 'ava'; import avaRuleTester from 'eslint-ava-rule-tester'; +import {outdent} from 'outdent'; import rule from '../rules/prefer-ternary'; const ruleTester = avaRuleTester(test, { @@ -10,277 +11,105 @@ const ruleTester = avaRuleTester(test, { ruleTester.run('prefer-ternary', rule, { valid: [ - 'if(a){b = 1;}', { - code: `if(a){ - b = 1 - doSomeStuff() - } - else{ - b = 2 - }` - }, - { - code: `if(a){ - b = 1 - } - else{ - c = 2 - }` - }, - { - code: `if(a){ - b = 1; - } - else{ - b = 2; - }`, - options: [{assignment: 'never'}] - }, - { - code: `if(a){ - b = 1; - } - else{ - c = 2; - }`, - options: [{assignment: 'same'}] - }, - { - code: `if(a){ - b = 1; - } - else{ - c = 2; - }`, - options: [{assignment: 'never'}] - }, - { - code: `function foo(){ - if(a){ - return 1; - } - else{ - return 2; - } - }`, - options: [{return: false}] - }, - { - code: `if(a){ - foo(); - } - else{ - bar(); - }` - }, - { - code: `if(a){ - foo(); - } - else{ - bar(); - }`, - options: [{call: false}] + code: outdent` + if(a){ + b = 1; + }` + }, + { + code: outdent` + if(a){ + b = 1 + bar() + } + else{ + b = 2 + }` + }, + { + code: outdent` + if(a){ + b = 1 + } + else{ + c = 2 + }` + }, + { + code: outdent` + if(a){ + b = 1; + } + else{ + c = 2; + }` + }, + { + code: outdent` + if(a){ + foo(); + } + else{ + bar(); + }` } ], invalid: [ { - code: `if(a){ - b = 1; - } - else{ - b = 2; - }`, - output: 'b = a ? 1 : 2', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] - }, - { - code: `if(a){ - b = 1; - } - else{ - b = 2; - }`, - options: ['always'], - output: 'b = a ? 1 : 2', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] - }, - { - code: `if(a){ - b = 1; - } - else{ - b = 2; - }`, - options: [{assignment: 'same'}], - output: 'b = a ? 1 : 2', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] - }, - { - code: `if(a){ - b = 1; - } - else{ - b = 2; - }`, - options: [{assignment: 'any'}], - output: 'b = a ? 1 : 2', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] - }, - - { - code: `if(a){ - b = 1; - } - else{ - c = 2; - }`, - options: ['always'], - output: 'a ? b = 1 : c = 2', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] - }, - { - code: `if(a){ - b = 1; - } - else{ - c = 2; - }`, - options: [{assignment: 'any'}], - output: 'a ? b = 1 : c = 2', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] - }, - - { - code: 'function foo(){if(a){return 1;}else{return 2;}}', - output: 'function foo(){return a ? 1 : 2}', - errors: [ - {column: 16, line: 1, type: 'IfStatement'} - ] - }, - { - code: 'function foo(){if(a){return 1;}else{return 2;}}', - options: ['always'], - output: 'function foo(){return a ? 1 : 2}', - errors: [ - {column: 16, line: 1, type: 'IfStatement'} - ] - }, - { - code: 'function foo(){if(a){return 1;}else{return 2;}}', - options: [{return: true}], - output: 'function foo(){return a ? 1 : 2}', - errors: [ - {column: 16, line: 1, type: 'IfStatement'} - ] - }, - - { - code: `if(a){ - foo(); - } - else{ - bar(); - }`, - options: ['always'], - output: 'a ? foo() : bar()', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] - }, - { - code: `if(a){ - foo(param1, [param2, param3]); - } - else{ - bar(); - }`, - options: [{call: true}], - output: 'a ? foo(param1, [param2, param3]) : bar()', + code: outdent` + if(foo){ + bar = 1; + } + else{ + bar = 2; + }`, + output: 'bar = (foo ? 1 : 2)', errors: [ {column: 1, line: 1, type: 'IfStatement'} ] }, { - code: `async () => { - if(a){ - await foo(); - } + code: outdent` + function foo(){ + if(bar){ + return 1; + } else{ - await bar(); + return 2; } }`, - parserOptions: {ecmaVersion: 2018}, - options: [{await: true}], - output: `async () => { - a ? await foo() : await bar() + output: outdent` + function foo(){ + return (bar ? 1 : 2) }`, errors: [ - {column: 5, line: 2, type: 'IfStatement'} - ] - }, - { - code: `if(a){ - new foo(); - } - else{ - new bar(); - }`, - options: [{new: true}], - output: 'a ? new foo() : new bar()', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} + {column: 2, line: 2, type: 'IfStatement'} ] }, { - code: `if(a){ - throw Error(123); - } - else{ - throw Error(456); - }`, - options: [{throw: true}], - output: 'throw a ? Error(123) : Error(456)', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] - }, - { - code: `function* foo(index) { - while (index < 10) { - if(index < 3){ - yield index++; - } - else{ - yield index * 2 - } + code: outdent` + function* generator(){ + while(foo){ + if(bar){ + yield bat + } + else{ + yield baz + } } - }`, - options: [{yield: true}], - output: `function* foo(index) { - while (index < 10) { - index < 3 ? yield index++ : yield index * 2 + }`, + output: outdent` + function* generator(){ + while(foo){ + yield (bar ? bat : baz) } - }`, + }`, errors: [ - {column: 7, line: 3, type: 'IfStatement'} + {column: 3, line: 3, type: 'IfStatement'} ] } - ] }); From 52f53445f8538f1e01a535ea152d881c75b2057b Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Tue, 3 Mar 2020 15:36:54 -0500 Subject: [PATCH 08/47] Added await; check to make sure not nested ternary --- docs/rules/prefer-ternary.md | 25 +++++++++++++ rules/prefer-ternary.js | 20 ++++++++-- test/prefer-ternary.js | 71 ++++++++++++++++++++++++++++++++++-- 3 files changed, 108 insertions(+), 8 deletions(-) diff --git a/docs/rules/prefer-ternary.md b/docs/rules/prefer-ternary.md index 52d8af0fa7..ee1efe94c3 100644 --- a/docs/rules/prefer-ternary.md +++ b/docs/rules/prefer-ternary.md @@ -30,6 +30,24 @@ else{ } ``` +```js +if(bar){ + await firstPromise() +} +else{ + await secondPromise() +} +``` + +```js +if(bar){ + yield bat +} +else{ + yield baz +} +``` + ## Pass ```js @@ -40,6 +58,13 @@ let foo = bar ? 3 : 4 return bar ? 3 : 4 ``` +```js +await (bar ? 3 : 4) +``` + +```js +yield (bar ? 3 : 4) +``` ```js let foo = '' diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 6071acd649..580443c5e3 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -34,14 +34,25 @@ const create = context => { function checkConsequentAndAlternateExpressionStatement(node) { const consequentType = node.consequent.body[0].expression.type; return consequentType === node.alternate.body[0].expression.type && - (consequentType === 'AssignmentExpression' ? compareConsequentAndAlternateAssignments(node) : - consequentType === 'YieldExpression'); + (consequentType === 'YieldExpression' || + consequentType === 'AwaitExpression' || + (consequentType === 'AssignmentExpression' && compareConsequentAndAlternateAssignments(node)) + ) && + checkNotAlreadyTernary(node); } function compareConsequentAndAlternateAssignments(node) { return node.consequent.body[0].expression.left.name === node.alternate.body[0].expression.left.name; } + function checkNotAlreadyTernary(node) { + return node.consequent.body[0].expression.type === 'AssignmentExpression' ? + node.consequent.body[0].expression.right.type !== 'ConditionalExpression' && + node.alternate.body[0].expression.right.type !== 'ConditionalExpression' : + node.consequent.body[0].expression.argument.type !== 'ConditionalExpression' && + node.alternate.body[0].expression.argument.type !== 'ConditionalExpression'; + } + function fixFunction(node, fixer) { const sourceCode = context.getSourceCode(); let prefix = ''; @@ -49,12 +60,13 @@ const create = context => { let left = ''; let right = ''; if (node.consequent.body[0].type === 'ExpressionStatement') { - if (node.consequent.body[0].expression.type === 'AssignmentExpression') { + const expressionType = node.consequent.body[0].expression.type; + if (expressionType === 'AssignmentExpression') { prefix = sourceCode.getText(node.consequent.body[0].expression.left) + ' = '; left = sourceCode.getText(node.consequent.body[0].expression.right); right = sourceCode.getText(node.alternate.body[0].expression.right); } else { - prefix = 'yield '; + prefix = expressionType === 'AwaitExpression' ? 'await ' : 'yield '; left = sourceCode.getText(node.consequent.body[0].expression.argument); right = sourceCode.getText(node.alternate.body[0].expression.argument); } diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index cf1c4315c1..5e8fd9540c 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -13,9 +13,9 @@ ruleTester.run('prefer-ternary', rule, { valid: [ { code: outdent` - if(a){ - b = 1; - }` + if(a){ + b = 1; + }` }, { code: outdent` @@ -53,8 +53,50 @@ ruleTester.run('prefer-ternary', rule, { else{ bar(); }` + }, + { + code: outdent` + if (foo) { + bar = a ? b : c; + } else { + bar = 2; + }` + }, + { + code: outdent` + if (foo) { + bar = 2; + } else { + bar = a ? b : c; + }` + }, + { + code: outdent` + async function foo(){ + if(bar){ + await (bat ? firstPromise() : secondPromise()); + } + else{ + await thirdPromise(); + } + }`, + parserOptions: { + ecmaVersion: 2020 + } + }, + { + code: outdent` + function* generator(){ + while(foo){ + if(bar){ + yield (a ? b : c) + } + else{ + yield d + } + } + }` } - ], invalid: [ @@ -89,6 +131,27 @@ ruleTester.run('prefer-ternary', rule, { {column: 2, line: 2, type: 'IfStatement'} ] }, + { + code: outdent` + async function foo(){ + if(bar){ + await firstPromise(); + } + else{ + await secondPromise(); + } + }`, + parserOptions: { + ecmaVersion: 2020 + }, + output: outdent` + async function foo(){ + await (bar ? firstPromise() : secondPromise()) + }`, + errors: [ + {column: 2, line: 2, type: 'IfStatement'} + ] + }, { code: outdent` function* generator(){ From d59f2b4202f4093054e4120a765cfc96e5b7b6f3 Mon Sep 17 00:00:00 2001 From: jmoore914 <30698083+jmoore914@users.noreply.github.com> Date: Tue, 3 Mar 2020 15:40:54 -0500 Subject: [PATCH 09/47] Update rules/prefer-ternary.js Co-Authored-By: fisker Cheung --- rules/prefer-ternary.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 580443c5e3..11502713df 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -15,9 +15,8 @@ const create = context => { } } - function checkConsequentAndAlternateLength(node) { - return checkConsequentOrAlternateLength(node.consequent) && - checkConsequentOrAlternateLength(node.alternate); + function isSingleBlockStatement(node) { + return [node.consequent, node.alternate].every(node => node && node.body.length === 1); } function checkConsequentOrAlternateLength(node) { From bd9e21cff57c95cee4785c57e13d8d9d0abd4f2c Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Tue, 3 Mar 2020 17:00:54 -0500 Subject: [PATCH 10/47] Works without braces --- rules/prefer-ternary.js | 56 +++++++++++++++++++++++------------------ test/prefer-ternary.js | 12 +++++++++ 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 11502713df..673da16f93 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -3,7 +3,8 @@ const getDocumentationUrl = require('./utils/get-documentation-url'); const create = context => { function checkIfStatement(node) { - if (checkConsequentAndAlternateLength(node) && + if (isSingleBlockStatement(node) && + isSameType(node) && checkConsequentAndAlternateType(node)) { context.report({ node, @@ -16,23 +17,28 @@ const create = context => { } function isSingleBlockStatement(node) { - return [node.consequent, node.alternate].every(node => node && node.body.length === 1); + return [node.consequent, node.alternate].every(node => { + return node && (node.type !== 'BlockStatement' || node.body.length === 1); + }) } - function checkConsequentOrAlternateLength(node) { - return node && node.body.length === 1; + function getNodeBody(node){ + return node.type==='BlockStatement' ? node.body[0] : node + } + + function isSameType(node) { + return getNodeBody(node.consequent).type === getNodeBody(node.alternate).type; } function checkConsequentAndAlternateType(node) { - const consequentType = node.consequent.body[0].type; - return (consequentType === node.alternate.body[0].type && - (consequentType === 'ReturnStatement' || - (consequentType === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)))); + return ( + (getNodeBody(node.consequent).type === 'ReturnStatement' || + (getNodeBody(node.consequent).type === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)))); } function checkConsequentAndAlternateExpressionStatement(node) { - const consequentType = node.consequent.body[0].expression.type; - return consequentType === node.alternate.body[0].expression.type && + const consequentType = getNodeBody(node.consequent).expression.type; + return consequentType === getNodeBody(node.alternate).expression.type && (consequentType === 'YieldExpression' || consequentType === 'AwaitExpression' || (consequentType === 'AssignmentExpression' && compareConsequentAndAlternateAssignments(node)) @@ -41,15 +47,15 @@ const create = context => { } function compareConsequentAndAlternateAssignments(node) { - return node.consequent.body[0].expression.left.name === node.alternate.body[0].expression.left.name; + return getNodeBody(node.consequent).expression.left.name === getNodeBody(node.alternate).expression.left.name; } function checkNotAlreadyTernary(node) { - return node.consequent.body[0].expression.type === 'AssignmentExpression' ? - node.consequent.body[0].expression.right.type !== 'ConditionalExpression' && - node.alternate.body[0].expression.right.type !== 'ConditionalExpression' : - node.consequent.body[0].expression.argument.type !== 'ConditionalExpression' && - node.alternate.body[0].expression.argument.type !== 'ConditionalExpression'; + return getNodeBody(node.consequent).expression.type === 'AssignmentExpression' ? + getNodeBody(node.consequent).expression.right.type !== 'ConditionalExpression' && + getNodeBody(node.alternate).expression.right.type !== 'ConditionalExpression' : + getNodeBody(node.consequent).expression.argument.type !== 'ConditionalExpression' && + getNodeBody(node.alternate).expression.argument.type !== 'ConditionalExpression'; } function fixFunction(node, fixer) { @@ -58,21 +64,21 @@ const create = context => { const ifCondition = sourceCode.getText(node.test); let left = ''; let right = ''; - if (node.consequent.body[0].type === 'ExpressionStatement') { - const expressionType = node.consequent.body[0].expression.type; + if (getNodeBody(node.consequent).type === 'ExpressionStatement') { + const expressionType = getNodeBody(node.consequent).expression.type; if (expressionType === 'AssignmentExpression') { - prefix = sourceCode.getText(node.consequent.body[0].expression.left) + ' = '; - left = sourceCode.getText(node.consequent.body[0].expression.right); - right = sourceCode.getText(node.alternate.body[0].expression.right); + prefix = sourceCode.getText(getNodeBody(node.consequent).expression.left) + ' = '; + left = sourceCode.getText(getNodeBody(node.consequent).expression.right); + right = sourceCode.getText(getNodeBody(node.alternate).expression.right); } else { prefix = expressionType === 'AwaitExpression' ? 'await ' : 'yield '; - left = sourceCode.getText(node.consequent.body[0].expression.argument); - right = sourceCode.getText(node.alternate.body[0].expression.argument); + left = sourceCode.getText(getNodeBody(node.consequent).expression.argument); + right = sourceCode.getText(getNodeBody(node.alternate).expression.argument); } } else { prefix = 'return '; - left = sourceCode.getText(node.consequent.body[0].argument); - right = sourceCode.getText(node.alternate.body[0].argument); + left = sourceCode.getText(getNodeBody(node.consequent).argument); + right = sourceCode.getText(getNodeBody(node.alternate).argument); } const replacement = prefix + '(' + ifCondition + ' ? ' + left + ' : ' + right + ')'; diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 5e8fd9540c..063f792b64 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -113,6 +113,18 @@ ruleTester.run('prefer-ternary', rule, { {column: 1, line: 1, type: 'IfStatement'} ] }, + { + code: outdent` + if(foo) + bar = 1; + else + bar = 2; + `, + output: 'bar = (foo ? 1 : 2)', + errors: [ + {column: 1, line: 1, type: 'IfStatement'} + ] + }, { code: outdent` function foo(){ From f0bdc343b2ed3be96c9f7df9053441640bd6a92c Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Tue, 3 Mar 2020 17:03:03 -0500 Subject: [PATCH 11/47] Linting --- rules/prefer-ternary.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 673da16f93..fbb5c11251 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -19,11 +19,11 @@ const create = context => { function isSingleBlockStatement(node) { return [node.consequent, node.alternate].every(node => { return node && (node.type !== 'BlockStatement' || node.body.length === 1); - }) + }); } - function getNodeBody(node){ - return node.type==='BlockStatement' ? node.body[0] : node + function getNodeBody(node) { + return node.type === 'BlockStatement' ? node.body[0] : node; } function isSameType(node) { From 48a26aa5582a68093abace88c1589fa8385fe0f4 Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Tue, 3 Mar 2020 17:20:44 -0500 Subject: [PATCH 12/47] More linting --- rules/prefer-ternary.js | 87 +++++++++++++++++++++-------------------- test/prefer-ternary.js | 8 ++++ 2 files changed, 53 insertions(+), 42 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index fbb5c11251..9d2c8f2b7d 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -1,6 +1,5 @@ 'use strict'; const getDocumentationUrl = require('./utils/get-documentation-url'); - const create = context => { function checkIfStatement(node) { if (isSingleBlockStatement(node) && @@ -16,47 +15,7 @@ const create = context => { } } - function isSingleBlockStatement(node) { - return [node.consequent, node.alternate].every(node => { - return node && (node.type !== 'BlockStatement' || node.body.length === 1); - }); - } - - function getNodeBody(node) { - return node.type === 'BlockStatement' ? node.body[0] : node; - } - - function isSameType(node) { - return getNodeBody(node.consequent).type === getNodeBody(node.alternate).type; - } - - function checkConsequentAndAlternateType(node) { - return ( - (getNodeBody(node.consequent).type === 'ReturnStatement' || - (getNodeBody(node.consequent).type === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)))); - } - - function checkConsequentAndAlternateExpressionStatement(node) { - const consequentType = getNodeBody(node.consequent).expression.type; - return consequentType === getNodeBody(node.alternate).expression.type && - (consequentType === 'YieldExpression' || - consequentType === 'AwaitExpression' || - (consequentType === 'AssignmentExpression' && compareConsequentAndAlternateAssignments(node)) - ) && - checkNotAlreadyTernary(node); - } - - function compareConsequentAndAlternateAssignments(node) { - return getNodeBody(node.consequent).expression.left.name === getNodeBody(node.alternate).expression.left.name; - } - - function checkNotAlreadyTernary(node) { - return getNodeBody(node.consequent).expression.type === 'AssignmentExpression' ? - getNodeBody(node.consequent).expression.right.type !== 'ConditionalExpression' && - getNodeBody(node.alternate).expression.right.type !== 'ConditionalExpression' : - getNodeBody(node.consequent).expression.argument.type !== 'ConditionalExpression' && - getNodeBody(node.alternate).expression.argument.type !== 'ConditionalExpression'; - } + function fixFunction(node, fixer) { const sourceCode = context.getSourceCode(); @@ -90,6 +49,50 @@ const create = context => { }; }; +function isSingleBlockStatement(node) { + return [node.consequent, node.alternate].every(node => { + return node && (node.type !== 'BlockStatement' || node.body.length === 1); + }); +} + +function getNodeBody(node) { + return node.type === 'BlockStatement' ? node.body[0] : node; +} + +function isSameType(node) { + return getNodeBody(node.consequent).type === getNodeBody(node.alternate).type; +} + +function checkConsequentAndAlternateType(node) { + return ( + (getNodeBody(node.consequent).type === 'ReturnStatement' || + (getNodeBody(node.consequent).type === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)))); +} + +function checkConsequentAndAlternateExpressionStatement(node) { + const consequentType = getNodeBody(node.consequent).expression.type; + return consequentType === getNodeBody(node.alternate).expression.type && + (consequentType === 'YieldExpression' || + consequentType === 'AwaitExpression' || + (consequentType === 'AssignmentExpression' && compareConsequentAndAlternateAssignments(node)) + ) && + checkNotAlreadyTernary(node); +} + +function compareConsequentAndAlternateAssignments(node) { + return getNodeBody(node.consequent).expression.left.name === getNodeBody(node.alternate).expression.left.name; +} + +function checkNotAlreadyTernary(node) { + return getNodeBody(node.consequent).expression.type === 'AssignmentExpression' ? + getNodeBody(node.consequent).expression.right.type !== 'ConditionalExpression' && + getNodeBody(node.alternate).expression.right.type !== 'ConditionalExpression' : + getNodeBody(node.consequent).expression.argument.type !== 'ConditionalExpression' && + getNodeBody(node.alternate).expression.argument.type !== 'ConditionalExpression'; +} + + + module.exports = { create, meta: { diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 063f792b64..b41a8d173d 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -17,6 +17,14 @@ ruleTester.run('prefer-ternary', rule, { b = 1; }` }, + { + code: outdent` + if(a){ + b = 1; + } + else{ + }` + }, { code: outdent` if(a){ From 73a226323ac6dc1cf5b30f9b611ad91a8b5515cf Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Tue, 3 Mar 2020 17:31:52 -0500 Subject: [PATCH 13/47] More linting --- rules/prefer-string-slice.js | 8 +++----- rules/prefer-ternary.js | 4 ---- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/rules/prefer-string-slice.js b/rules/prefer-string-slice.js index b8ad529318..f10818e56c 100644 --- a/rules/prefer-string-slice.js +++ b/rules/prefer-string-slice.js @@ -95,12 +95,10 @@ const create = context => { if (argumentNodes.length === 0) { slice = []; } else if (argumentNodes.length === 1) { - if (firstNumber !== undefined) { - slice = [Math.max(0, firstNumber)]; - } else if (isLengthProperty(argumentNodes[0])) { - slice = [firstArgument]; + if (firstNumber === undefined) { + slice = (isLengthProperty(argumentNodes[0])) ? [firstArgument] : [`Math.max(0, ${firstArgument})`]; } else { - slice = [`Math.max(0, ${firstArgument})`]; + slice = [Math.max(0, firstNumber)]; } } else if (argumentNodes.length === 2) { const secondNumber = argumentNodes[1] ? getNumericValue(argumentNodes[1]) : undefined; diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 9d2c8f2b7d..6c50451b18 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -15,8 +15,6 @@ const create = context => { } } - - function fixFunction(node, fixer) { const sourceCode = context.getSourceCode(); let prefix = ''; @@ -91,8 +89,6 @@ function checkNotAlreadyTernary(node) { getNodeBody(node.alternate).expression.argument.type !== 'ConditionalExpression'; } - - module.exports = { create, meta: { From c6004756f5bf10b81f7e6a5fdb01f34b7845c9f6 Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Fri, 6 Mar 2020 14:15:59 -0500 Subject: [PATCH 14/47] Added additional tests --- test/prefer-ternary.js | 71 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index b41a8d173d..0a9f586e06 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -193,6 +193,77 @@ ruleTester.run('prefer-ternary', rule, { errors: [ {column: 3, line: 3, type: 'IfStatement'} ] + }, + { + code: outdent` + function foo(){ + if (a) { + return 1; + } else if (b) { + return 2; + } else { + return 3; + } + }`, + output: outdent` + function foo(){ + if (a) { + return 1; + } else return (b ? 2 : 3) + }`, + errors: [ + {column: 9, line: 4, type: 'IfStatement'} + ] + }, + { + code: outdent` + function foo(){ + if (a) { + return 1; + } else { + if (b) { + return 2; + } else { + return 3; + } + } + }`, + output: outdent` + function foo(){ + if (a) { + return 1; + } else { + return (b ? 2 : 3) + } + }`, + errors: [ + {column: 3, line: 5, type: 'IfStatement'} + ] + }, + { + code: outdent` + function foo(){ + if (a) { + if (b) { + return 1; + } else { + return 2; + } + } else { + return 3; + } + }`, + output: outdent` + function foo(){ + if (a) { + return (b ? 1 : 2) + } else { + return 3; + } + }`, + errors: [ + {column: 3, line: 3, type: 'IfStatement'} + ] } ] }); From f75101e8de8b711db66ddbb340b9a2a9e10eebd6 Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Fri, 13 Mar 2020 09:38:59 -0400 Subject: [PATCH 15/47] Added support for yield* --- rules/prefer-ternary.js | 14 ++++++++++++-- test/prefer-ternary.js | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 6c50451b18..ae75aa703a 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -28,7 +28,12 @@ const create = context => { left = sourceCode.getText(getNodeBody(node.consequent).expression.right); right = sourceCode.getText(getNodeBody(node.alternate).expression.right); } else { - prefix = expressionType === 'AwaitExpression' ? 'await ' : 'yield '; + if (expressionType === 'AwaitExpression') { + prefix = 'await '; + } else { + prefix = getNodeBody(node.consequent).expression.delegate ? 'yield* ' : 'yield '; + } + left = sourceCode.getText(getNodeBody(node.consequent).expression.argument); right = sourceCode.getText(getNodeBody(node.alternate).expression.argument); } @@ -70,13 +75,18 @@ function checkConsequentAndAlternateType(node) { function checkConsequentAndAlternateExpressionStatement(node) { const consequentType = getNodeBody(node.consequent).expression.type; return consequentType === getNodeBody(node.alternate).expression.type && - (consequentType === 'YieldExpression' || + ( consequentType === 'AwaitExpression' || + (consequentType === 'YieldExpression' && compareYieldExpressions(node)) || (consequentType === 'AssignmentExpression' && compareConsequentAndAlternateAssignments(node)) ) && checkNotAlreadyTernary(node); } +function compareYieldExpressions(node) { + return getNodeBody(node.consequent).expression.delegate === getNodeBody(node.alternate).expression.delegate; +} + function compareConsequentAndAlternateAssignments(node) { return getNodeBody(node.consequent).expression.left.name === getNodeBody(node.alternate).expression.left.name; } diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 0a9f586e06..a667103f25 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -104,6 +104,19 @@ ruleTester.run('prefer-ternary', rule, { } } }` + }, + { + code: outdent` + function* generator(){ + while(foo){ + if(bar){ + yield baz() + } + else{ + yield* bat() + } + } + }` } ], @@ -194,6 +207,28 @@ ruleTester.run('prefer-ternary', rule, { {column: 3, line: 3, type: 'IfStatement'} ] }, + { + code: outdent` + function* generator(){ + while(foo){ + if(bar){ + yield* bat() + } + else{ + yield* baz() + } + } + }`, + output: outdent` + function* generator(){ + while(foo){ + yield* (bar ? bat() : baz()) + } + }`, + errors: [ + {column: 3, line: 3, type: 'IfStatement'} + ] + }, { code: outdent` function foo(){ From 64e9a0b2f302a87f4ccaffa0852ec67c5eb40393 Mon Sep 17 00:00:00 2001 From: jmoore914 Date: Fri, 13 Mar 2020 10:00:45 -0400 Subject: [PATCH 16/47] Linting --- rules/string-content.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/rules/string-content.js b/rules/string-content.js index 64ea86ce4d..fe46a0a84e 100644 --- a/rules/string-content.js +++ b/rules/string-content.js @@ -117,19 +117,17 @@ const create = context => { } const fixed = string.replace(replacement.regex, suggest); - if (type === 'Literal') { - problem.fix = fixer => fixer.replaceText( + problem.fix = type === 'Literal' ? + fixer => fixer.replaceText( node, quoteString(fixed, node.raw[0]) - ); - } else { - problem.fix = fixer => replaceTemplateElement( + ) : + fixer => replaceTemplateElement( fixer, node, escapeTemplateElementRaw(fixed) ); - } - + context.report(problem); } }; From 5860a7364a1111a315686c5cc9d2cf1f8fdcb1f5 Mon Sep 17 00:00:00 2001 From: fisker Date: Mon, 16 Mar 2020 09:58:28 +0800 Subject: [PATCH 17/47] fix style --- rules/string-content.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rules/string-content.js b/rules/string-content.js index fe46a0a84e..bbed40d21f 100644 --- a/rules/string-content.js +++ b/rules/string-content.js @@ -118,16 +118,16 @@ const create = context => { const fixed = string.replace(replacement.regex, suggest); problem.fix = type === 'Literal' ? - fixer => fixer.replaceText( + fixer => fixer.replaceText( node, quoteString(fixed, node.raw[0]) - ) : + ) : fixer => replaceTemplateElement( fixer, node, escapeTemplateElementRaw(fixed) ); - + context.report(problem); } }; From a325e560702a28329a926469bd12bc0a7f2a3f79 Mon Sep 17 00:00:00 2001 From: fisker Date: Mon, 16 Mar 2020 10:06:46 +0800 Subject: [PATCH 18/47] fix integration test --- rules/no-nested-ternary.js | 16 ++-------------- rules/prefer-ternary.js | 26 +++++++++++++++++++------- rules/utils/is-parenthesized.js | 16 ++++++++++++++++ test/prefer-ternary.js | 32 ++++++++++++++++++++++++++++---- 4 files changed, 65 insertions(+), 25 deletions(-) create mode 100644 rules/utils/is-parenthesized.js diff --git a/rules/no-nested-ternary.js b/rules/no-nested-ternary.js index d8b922a7b8..75a84795ff 100644 --- a/rules/no-nested-ternary.js +++ b/rules/no-nested-ternary.js @@ -1,18 +1,6 @@ 'use strict'; const getDocumentationUrl = require('./utils/get-documentation-url'); - -const isParenthesized = (sourceCode, node) => { - const previousToken = sourceCode.getTokenBefore(node); - const nextToken = sourceCode.getTokenAfter(node); - - return ( - Boolean(previousToken && nextToken) && - previousToken.value === '(' && - previousToken.end <= node.range[0] && - nextToken.value === ')' && - nextToken.start >= node.range[1] - ); -}; +const isParenthesized = require('./utils/is-parenthesized'); const create = context => { const sourceCode = context.getSourceCode(); @@ -35,7 +23,7 @@ const create = context => { ) { context.report({node, message}); break; - } else if (!isParenthesized(sourceCode, childNode)) { + } else if (!isParenthesized(childNode, sourceCode)) { context.report({ node: childNode, message, diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index ae75aa703a..c297966b72 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -1,5 +1,7 @@ 'use strict'; const getDocumentationUrl = require('./utils/get-documentation-url'); +const isParenthesized = require('./utils/is-parenthesized'); + const create = context => { function checkIfStatement(node) { if (isSingleBlockStatement(node) && @@ -25,8 +27,8 @@ const create = context => { const expressionType = getNodeBody(node.consequent).expression.type; if (expressionType === 'AssignmentExpression') { prefix = sourceCode.getText(getNodeBody(node.consequent).expression.left) + ' = '; - left = sourceCode.getText(getNodeBody(node.consequent).expression.right); - right = sourceCode.getText(getNodeBody(node.alternate).expression.right); + left = getNodeBody(node.consequent).expression.right; + right = getNodeBody(node.alternate).expression.right; } else { if (expressionType === 'AwaitExpression') { prefix = 'await '; @@ -34,16 +36,26 @@ const create = context => { prefix = getNodeBody(node.consequent).expression.delegate ? 'yield* ' : 'yield '; } - left = sourceCode.getText(getNodeBody(node.consequent).expression.argument); - right = sourceCode.getText(getNodeBody(node.alternate).expression.argument); + left = getNodeBody(node.consequent).expression.argument; + right = getNodeBody(node.alternate).expression.argument; } } else { prefix = 'return '; - left = sourceCode.getText(getNodeBody(node.consequent).argument); - right = sourceCode.getText(getNodeBody(node.alternate).argument); + left = getNodeBody(node.consequent).argument; + right = getNodeBody(node.alternate).argument; + } + + let leftCode = sourceCode.getText(left); + if (isParenthesized(left, sourceCode)) { + leftCode = `(${leftCode})`; + } + + let rightCode = sourceCode.getText(right); + if (isParenthesized(right, sourceCode)) { + rightCode = `(${rightCode})`; } - const replacement = prefix + '(' + ifCondition + ' ? ' + left + ' : ' + right + ')'; + const replacement = prefix + '(' + ifCondition + ' ? ' + leftCode + ' : ' + rightCode + ')'; return fixer.replaceText(node, replacement); } diff --git a/rules/utils/is-parenthesized.js b/rules/utils/is-parenthesized.js new file mode 100644 index 0000000000..7c7f231ee6 --- /dev/null +++ b/rules/utils/is-parenthesized.js @@ -0,0 +1,16 @@ +'use strict'; + +const isParenthesized = (node, sourceCode) => { + const previousToken = sourceCode.getTokenBefore(node); + const nextToken = sourceCode.getTokenAfter(node); + + return ( + Boolean(previousToken && nextToken) && + previousToken.value === '(' && + previousToken.end <= node.range[0] && + nextToken.value === ')' && + nextToken.start >= node.range[1] + ); +}; + +module.exports = isParenthesized; diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index a667103f25..37918fc472 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -9,6 +9,10 @@ const ruleTester = avaRuleTester(test, { } }); +const babelRuleTester = avaRuleTester(test, { + parser: require.resolve('babel-eslint') +}); + ruleTester.run('prefer-ternary', rule, { valid: [ { @@ -48,7 +52,7 @@ ruleTester.run('prefer-ternary', rule, { code: outdent` if(a){ b = 1; - } + } else{ c = 2; }` @@ -57,7 +61,7 @@ ruleTester.run('prefer-ternary', rule, { code: outdent` if(a){ foo(); - } + } else{ bar(); }` @@ -125,7 +129,7 @@ ruleTester.run('prefer-ternary', rule, { code: outdent` if(foo){ bar = 1; - } + } else{ bar = 2; }`, @@ -137,7 +141,7 @@ ruleTester.run('prefer-ternary', rule, { { code: outdent` if(foo) - bar = 1; + bar = 1; else bar = 2; `, @@ -302,3 +306,23 @@ ruleTester.run('prefer-ternary', rule, { } ] }); + +babelRuleTester.run('prefer-ternary', rule, { + valid: [], + invalid: [ + // https://github.com/facebook/react/blob/7a1691cdff209249b49a4472ba87b542980a5f71/packages/react-dom/src/client/DOMPropertyOperations.js#L183 + { + code: outdent` + if (enableTrustedTypesIntegration) { + attributeValue = (value: any); + } else { + attributeValue = '' + (value: any); + } + `, + output: outdent` + attributeValue = (enableTrustedTypesIntegration ? (value: any) : '' + (value: any)) + `, + errors: [{}] + } + ] +}); From a99e7aed3a20d1aecddb99a8f244f3de4cb2f4e9 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 5 May 2020 12:11:21 +0800 Subject: [PATCH 19/47] Ignore `IfStatement.test=ConditionalExpression` --- rules/prefer-ternary.js | 9 +++++++-- rules/utils/is-parenthesized.js | 16 ---------------- test/prefer-ternary.js | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 18 deletions(-) delete mode 100644 rules/utils/is-parenthesized.js diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index c297966b72..c496a6a614 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -1,6 +1,11 @@ 'use strict'; +const {isParenthesized} = require('eslint-utils'); const getDocumentationUrl = require('./utils/get-documentation-url'); -const isParenthesized = require('./utils/is-parenthesized'); + +const selector = [ + 'IfStatement', + '[test.type!="ConditionalExpression"]' +]; const create = context => { function checkIfStatement(node) { @@ -60,7 +65,7 @@ const create = context => { } return { - IfStatement: checkIfStatement + [selector]: checkIfStatement }; }; diff --git a/rules/utils/is-parenthesized.js b/rules/utils/is-parenthesized.js deleted file mode 100644 index 7c7f231ee6..0000000000 --- a/rules/utils/is-parenthesized.js +++ /dev/null @@ -1,16 +0,0 @@ -'use strict'; - -const isParenthesized = (node, sourceCode) => { - const previousToken = sourceCode.getTokenBefore(node); - const nextToken = sourceCode.getTokenAfter(node); - - return ( - Boolean(previousToken && nextToken) && - previousToken.value === '(' && - previousToken.end <= node.range[0] && - nextToken.value === ')' && - nextToken.start >= node.range[1] - ); -}; - -module.exports = isParenthesized; diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 37918fc472..075a7f84ab 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -15,6 +15,29 @@ const babelRuleTester = avaRuleTester(test, { ruleTester.run('prefer-ternary', rule, { valid: [ + // IfStatement contains ternary + outdent` + if (a ? b : c) { + a(); + } else { + b(); + } + `, + outdent` + if (a) { + a ? b() : c(); + } else { + b(); + } + `, + outdent` + if (a) { + b(); + } else { + a ? b() : c(); + } + `, + { code: outdent` if(a){ From 6a3c79325582fd6fd261eec0df03d609d1f49d6d Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 5 May 2020 12:20:28 +0800 Subject: [PATCH 20/47] Improve `getNodeBody` --- rules/prefer-ternary.js | 16 +++++++++++++++- test/prefer-ternary.js | 18 +++++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index c496a6a614..f130bdbd61 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -76,9 +76,23 @@ function isSingleBlockStatement(node) { } function getNodeBody(node) { - return node.type === 'BlockStatement' ? node.body[0] : node; + // `if (a) b;` + if (node.type === 'ExpressionStatement') { + return node.expression; + } + + if (node.type === 'BlockStatement') { + const body = node.body.filter(({type}) => type === 'EmptyStatement'); + if (body.length === 1) { + return getNodeBody(body[0]); + } + } + + return node; } + + function isSameType(node) { return getNodeBody(node.consequent).type === getNodeBody(node.alternate).type; } diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 075a7f84ab..9c9658d328 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -13,6 +13,8 @@ const babelRuleTester = avaRuleTester(test, { parser: require.resolve('babel-eslint') }); +const errors = [{}] + ruleTester.run('prefer-ternary', rule, { valid: [ // IfStatement contains ternary @@ -150,16 +152,14 @@ ruleTester.run('prefer-ternary', rule, { invalid: [ { code: outdent` - if(foo){ - bar = 1; - } - else{ - bar = 2; - }`, + if(foo){ + bar = 1; + } else{ + bar = 2; + } + `, output: 'bar = (foo ? 1 : 2)', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] + errors }, { code: outdent` From ee7f8889b8749b6fd6f4a3f29973c7dde1c324ae Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 5 May 2020 12:51:11 +0800 Subject: [PATCH 21/47] Fix logic --- rules/prefer-ternary.js | 191 +++++++++++++++++++--------------------- test/prefer-ternary.js | 7 +- 2 files changed, 96 insertions(+), 102 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index f130bdbd61..f6a761873a 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -2,87 +2,29 @@ const {isParenthesized} = require('eslint-utils'); const getDocumentationUrl = require('./utils/get-documentation-url'); +const messageId = 'prefer-ternary'; + const selector = [ 'IfStatement', - '[test.type!="ConditionalExpression"]' -]; - -const create = context => { - function checkIfStatement(node) { - if (isSingleBlockStatement(node) && - isSameType(node) && - checkConsequentAndAlternateType(node)) { - context.report({ - node, - message: 'This `if` statement can be replaced by a ternary operator.', - fix(fixer) { - return fixFunction(node, fixer); - } - }); - } - } + '[test.type!="ConditionalExpression"]', + '[consequent]', + '[alternate]', +].join(''); - function fixFunction(node, fixer) { - const sourceCode = context.getSourceCode(); - let prefix = ''; - const ifCondition = sourceCode.getText(node.test); - let left = ''; - let right = ''; - if (getNodeBody(node.consequent).type === 'ExpressionStatement') { - const expressionType = getNodeBody(node.consequent).expression.type; - if (expressionType === 'AssignmentExpression') { - prefix = sourceCode.getText(getNodeBody(node.consequent).expression.left) + ' = '; - left = getNodeBody(node.consequent).expression.right; - right = getNodeBody(node.alternate).expression.right; - } else { - if (expressionType === 'AwaitExpression') { - prefix = 'await '; - } else { - prefix = getNodeBody(node.consequent).expression.delegate ? 'yield* ' : 'yield '; - } - - left = getNodeBody(node.consequent).expression.argument; - right = getNodeBody(node.alternate).expression.argument; - } - } else { - prefix = 'return '; - left = getNodeBody(node.consequent).argument; - right = getNodeBody(node.alternate).argument; - } +const isTernary = node => node && node.type === 'ConditionalExpression'; - let leftCode = sourceCode.getText(left); - if (isParenthesized(left, sourceCode)) { - leftCode = `(${leftCode})`; - } - - let rightCode = sourceCode.getText(right); - if (isParenthesized(right, sourceCode)) { - rightCode = `(${rightCode})`; - } - const replacement = prefix + '(' + ifCondition + ' ? ' + leftCode + ' : ' + rightCode + ')'; - return fixer.replaceText(node, replacement); +function getNodeBody(node) { + if (!node) { + return; } - return { - [selector]: checkIfStatement - }; -}; - -function isSingleBlockStatement(node) { - return [node.consequent, node.alternate].every(node => { - return node && (node.type !== 'BlockStatement' || node.body.length === 1); - }); -} - -function getNodeBody(node) { - // `if (a) b;` if (node.type === 'ExpressionStatement') { - return node.expression; + return getNodeBody(node.expression); } if (node.type === 'BlockStatement') { - const body = node.body.filter(({type}) => type === 'EmptyStatement'); + const body = node.body.filter(({type}) => type !== 'EmptyStatement'); if (body.length === 1) { return getNodeBody(body[0]); } @@ -91,44 +33,88 @@ function getNodeBody(node) { return node; } +function isSameNode(node1, node2) { + // [TBD]: compare more type + return node1.type == node2.type && node1.type === 'Identifier' && node1.name === node2.name; +} + +const create = context => { + const sourceCode = context.getSourceCode(); + return { + [selector](node) { + const consequent = getNodeBody(node.consequent); + const alternate = getNodeBody(node.alternate); -function isSameType(node) { - return getNodeBody(node.consequent).type === getNodeBody(node.alternate).type; -} + if (!consequent || !alternate || consequent.type !== alternate.type) { + return; + } -function checkConsequentAndAlternateType(node) { - return ( - (getNodeBody(node.consequent).type === 'ReturnStatement' || - (getNodeBody(node.consequent).type === 'ExpressionStatement' && checkConsequentAndAlternateExpressionStatement(node)))); -} + const {type} = consequent; + let prefix = ''; + let left; + let right; + + if ( + type === 'ReturnStatement' && + !isTernary(consequent.argument) && + !isTernary(alternate.argument) + ) { + prefix = 'return '; + left = consequent.argument; + right = alternate.argument; + } -function checkConsequentAndAlternateExpressionStatement(node) { - const consequentType = getNodeBody(node.consequent).expression.type; - return consequentType === getNodeBody(node.alternate).expression.type && - ( - consequentType === 'AwaitExpression' || - (consequentType === 'YieldExpression' && compareYieldExpressions(node)) || - (consequentType === 'AssignmentExpression' && compareConsequentAndAlternateAssignments(node)) - ) && - checkNotAlreadyTernary(node); -} + if ( + type === 'YieldStatement' && + consequent.delegate === alternate.delegate && + !isTernary(consequent.argument) && + !isTernary(alternate.argument) + ) { + prefix = `yield${consequent.delegate ? '*' : ''} `; + left = consequent.argument; + right = alternate.argument; + } -function compareYieldExpressions(node) { - return getNodeBody(node.consequent).expression.delegate === getNodeBody(node.alternate).expression.delegate; -} + if ( + type === 'AwaitExpression' && + !isTernary(consequent.argument) && + !isTernary(alternate.argument) + ) { + prefix = `await `; + left = consequent.argument; + right = alternate.argument; + } -function compareConsequentAndAlternateAssignments(node) { - return getNodeBody(node.consequent).expression.left.name === getNodeBody(node.alternate).expression.left.name; -} + if ( + type === 'AssignmentExpression' && + isSameNode(consequent.left, alternate.left) && + !isTernary(consequent.left) && + !isTernary(alternate.left) && + !isTernary(consequent.right) && + !isTernary(alternate.right) + ) { + prefix = sourceCode.getText(consequent.left) + ' = ' + left = consequent.right; + right = alternate.right; + } -function checkNotAlreadyTernary(node) { - return getNodeBody(node.consequent).expression.type === 'AssignmentExpression' ? - getNodeBody(node.consequent).expression.right.type !== 'ConditionalExpression' && - getNodeBody(node.alternate).expression.right.type !== 'ConditionalExpression' : - getNodeBody(node.consequent).expression.argument.type !== 'ConditionalExpression' && - getNodeBody(node.alternate).expression.argument.type !== 'ConditionalExpression'; -} + if (left && right) { + left = sourceCode.getText(left); + right = sourceCode.getText(right); + const test = sourceCode.getText(node.test); + context.report({ + node, + messageId, + fix: fixer => fixer.replaceText( + node, + `${prefix}(${test}) ? (${left}) : (${right})` + ) + }) + } + } + }; +}; module.exports = { create, @@ -137,6 +123,9 @@ module.exports = { docs: { url: getDocumentationUrl(__filename) }, + messages: { + [messageId]: 'This `if` statement can be replaced by a ternary expression.', + }, fixable: 'code' } }; diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 9c9658d328..13de301baf 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -3,6 +3,8 @@ import avaRuleTester from 'eslint-ava-rule-tester'; import {outdent} from 'outdent'; import rule from '../rules/prefer-ternary'; +const messageId = 'prefer-ternary'; + const ruleTester = avaRuleTester(test, { env: { es6: true @@ -13,7 +15,7 @@ const babelRuleTester = avaRuleTester(test, { parser: require.resolve('babel-eslint') }); -const errors = [{}] +const errors = [{messageId}] ruleTester.run('prefer-ternary', rule, { valid: [ @@ -39,6 +41,9 @@ ruleTester.run('prefer-ternary', rule, { a ? b() : c(); } `, + // No `consequent` / `alternate` + 'if (a) {b}', + 'if (a) {} else {}', { code: outdent` From b8dd909cb1b6c6782c82f1404b4b4e9187bc9f75 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 16:11:37 +0800 Subject: [PATCH 22/47] Nested merge --- rules/prefer-ternary.js | 182 ++++++---- test/prefer-ternary.js | 755 ++++++++++++++++++++++++++++------------ 2 files changed, 652 insertions(+), 285 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index f6a761873a..851535e8c7 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -8,12 +8,11 @@ const selector = [ 'IfStatement', '[test.type!="ConditionalExpression"]', '[consequent]', - '[alternate]', + '[alternate]' ].join(''); const isTernary = node => node && node.type === 'ConditionalExpression'; - function getNodeBody(node) { if (!node) { return; @@ -33,85 +32,132 @@ function getNodeBody(node) { return node; } -function isSameNode(node1, node2) { - // [TBD]: compare more type - return node1.type == node2.type && node1.type === 'Identifier' && node1.name === node2.name; +function isSameAssignmentLeft(node1, node2) { + // [TBD]: Allow more types of left + return node1.type === node2.type && node1.type === 'Identifier' && node1.name === node2.name; } const create = context => { const sourceCode = context.getSourceCode(); + const getParenthesizedText = node => { + const text = sourceCode.getText(node); + return (isParenthesized(node, sourceCode) || node.type === 'AwaitExpression') ? + `(${text})` : text; + }; + + function merge(options, returnFalseIfNotMergeable = false) { + const { + before = '', + after = ';', + consequent, + alternate + } = options; + + if (!consequent || !alternate || consequent.type !== alternate.type) { + return returnFalseIfNotMergeable ? false : options; + } + + const {type} = consequent; + + if ( + type === 'ReturnStatement' && + consequent.argument && + alternate.argument && + !isTernary(consequent.argument) && + !isTernary(alternate.argument) + ) { + return merge({ + before: `${before}return `, + after, + consequent: consequent.argument, + alternate: alternate.argument + }); + } + + if ( + type === 'YieldExpression' && + consequent.delegate === alternate.delegate && + consequent.argument && + alternate.argument && + !isTernary(consequent.argument) && + !isTernary(alternate.argument) + ) { + return merge({ + before: `${before}yield${consequent.delegate ? '*' : ''} `, + after, + consequent: consequent.argument, + alternate: alternate.argument + }); + } + + if ( + type === 'AwaitExpression' && + !isTernary(consequent.argument) && + !isTernary(alternate.argument) + ) { + return merge({ + before: `${before}await (`, + after: `)${after}`, + consequent: consequent.argument, + alternate: alternate.argument + }); + } + + if ( + type === 'ThrowStatement' && + !isTernary(consequent.argument) && + !isTernary(alternate.argument) + ) { + return merge({ + before: `${before}throw `, + after, + consequent: consequent.argument, + alternate: alternate.argument + }); + } + + if ( + type === 'AssignmentExpression' && + isSameAssignmentLeft(consequent.left, alternate.left) && + !isTernary(consequent.left) && + !isTernary(alternate.left) && + !isTernary(consequent.right) && + !isTernary(alternate.right) + ) { + return merge({ + before: `${before}${sourceCode.getText(consequent.left)} = `, + after, + consequent: consequent.right, + alternate: alternate.right + }); + } + + return returnFalseIfNotMergeable ? false : options; + } + return { [selector](node) { const consequent = getNodeBody(node.consequent); const alternate = getNodeBody(node.alternate); - if (!consequent || !alternate || consequent.type !== alternate.type) { - return; - } - - const {type} = consequent; - let prefix = ''; - let left; - let right; - - if ( - type === 'ReturnStatement' && - !isTernary(consequent.argument) && - !isTernary(alternate.argument) - ) { - prefix = 'return '; - left = consequent.argument; - right = alternate.argument; - } - - if ( - type === 'YieldStatement' && - consequent.delegate === alternate.delegate && - !isTernary(consequent.argument) && - !isTernary(alternate.argument) - ) { - prefix = `yield${consequent.delegate ? '*' : ''} `; - left = consequent.argument; - right = alternate.argument; - } + const result = merge({consequent, alternate}, true); - if ( - type === 'AwaitExpression' && - !isTernary(consequent.argument) && - !isTernary(alternate.argument) - ) { - prefix = `await `; - left = consequent.argument; - right = alternate.argument; - } - - if ( - type === 'AssignmentExpression' && - isSameNode(consequent.left, alternate.left) && - !isTernary(consequent.left) && - !isTernary(alternate.left) && - !isTernary(consequent.right) && - !isTernary(alternate.right) - ) { - prefix = sourceCode.getText(consequent.left) + ' = ' - left = consequent.right; - right = alternate.right; + if (!result) { + return; } - if (left && right) { - left = sourceCode.getText(left); - right = sourceCode.getText(right); - const test = sourceCode.getText(node.test); - context.report({ - node, - messageId, - fix: fixer => fixer.replaceText( + context.report({ + node, + messageId, + fix: fixer => { + const fixed = `${result.before}${getParenthesizedText(node.test)} ? ${getParenthesizedText(result.consequent)} : ${getParenthesizedText(result.alternate)}${result.after}`; + return fixer.replaceText( node, - `${prefix}(${test}) ? (${left}) : (${right})` - ) - }) - } + fixed + ); + } + }); } }; }; @@ -124,7 +170,7 @@ module.exports = { url: getDocumentationUrl(__filename) }, messages: { - [messageId]: 'This `if` statement can be replaced by a ternary expression.', + [messageId]: 'This `if` statement can be replaced by a ternary expression.' }, fixable: 'code' } diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 13de301baf..2540f53f46 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -6,8 +6,8 @@ import rule from '../rules/prefer-ternary'; const messageId = 'prefer-ternary'; const ruleTester = avaRuleTester(test, { - env: { - es6: true + parserOptions: { + ecmaVersion: 2020 } }); @@ -15,322 +15,643 @@ const babelRuleTester = avaRuleTester(test, { parser: require.resolve('babel-eslint') }); -const errors = [{messageId}] +const errors = [{messageId}]; +// ReturnStatement ruleTester.run('prefer-ternary', rule, { valid: [ - // IfStatement contains ternary + // When ReturnStatement has no argument, should fix to `test ? undefined : …`, not handled outdent` - if (a ? b : c) { - a(); - } else { - b(); + function unicorn() { + if(test){ + return; + } else{ + return b; + } } `, + // Test is Ternary outdent` - if (a) { - a ? b() : c(); - } else { - b(); + function unicorn() { + if(a ? b : c){ + return a; + } else{ + return b; + } } `, + // Consequent is Ternary outdent` - if (a) { - b(); - } else { - a ? b() : c(); + function unicorn() { + if(test){ + return a ? b : c; + } else{ + return b; + } } `, - // No `consequent` / `alternate` - 'if (a) {b}', - 'if (a) {} else {}', - + // Alternate is Ternary + outdent` + function unicorn() { + if(test){ + return a; + } else{ + return a ? b : c; + } + } + ` + ], + invalid: [ { code: outdent` - if(a){ - b = 1; - }` + function unicorn() { + if(test){ + return a; + } else{ + return b; + } + } + `, + output: outdent` + function unicorn() { + return test ? a : b; + } + `, + errors }, { code: outdent` - if(a){ - b = 1; - } - else{ - }` + async function unicorn() { + if(test){ + return await a; + } else{ + return b; + } + } + `, + output: outdent` + async function unicorn() { + return test ? (await a) : b; + } + `, + errors }, { code: outdent` - if(a){ - b = 1 - bar() + async function unicorn() { + if(test){ + return await a; + } else{ + return await b; + } + } + `, + output: outdent` + async function unicorn() { + return await (test ? a : b); + } + `, + errors + } + ] +}); + +// YieldExpression +ruleTester.run('prefer-ternary', rule, { + valid: [ + // When YieldExpression has no argument, should fix to `test ? undefined : …`, not handled + outdent` + function* unicorn() { + if(test){ + yield; + } else{ + yield b; + } } - else{ - b = 2 - }` - }, - { - code: outdent` - if(a){ - b = 1 + `, + // Different `delegate` + outdent` + function* unicorn() { + if(test){ + yield* a; + } else{ + yield b; + } } - else{ - c = 2 - }` - }, - { - code: outdent` - if(a){ - b = 1; + `, + // Test is Ternary + outdent` + function* unicorn() { + if(a ? b : c){ + yield a; + } else{ + yield b; + } } - else{ - c = 2; - }` - }, + `, + // Consequent is Ternary + outdent` + function* unicorn() { + if(test){ + yield a ? b : c; + } else{ + yield b; + } + } + `, + // Alternate is Ternary + outdent` + function* unicorn() { + if(test){ + yield a; + } else{ + yield a ? b : c; + } + } + ` + ], + invalid: [ { code: outdent` - if(a){ - foo(); - } - else{ - bar(); - }` + function* unicorn() { + if(test){ + yield a; + } else{ + yield b; + } + } + `, + output: outdent` + function* unicorn() { + yield test ? a : b; + } + `, + errors }, { code: outdent` - if (foo) { - bar = a ? b : c; - } else { - bar = 2; - }` + function* unicorn() { + if(test){ + yield* a; + } else{ + yield* b; + } + } + `, + output: outdent` + function* unicorn() { + yield* test ? a : b; + } + `, + errors }, { code: outdent` - if (foo) { - bar = 2; - } else { - bar = a ? b : c; - }` + async function* unicorn() { + if(test){ + yield await a; + } else{ + yield b; + } + } + `, + output: outdent` + async function* unicorn() { + yield test ? (await a) : b; + } + `, + errors }, { code: outdent` - async function foo(){ - if(bar){ - await (bat ? firstPromise() : secondPromise()); + async function* unicorn() { + if(test){ + yield await a; + } else{ + yield await b; + } + } + `, + output: outdent` + async function* unicorn() { + yield await (test ? a : b); + } + `, + errors + } + ] +}); + +// AwaitExpression +ruleTester.run('prefer-ternary', rule, { + valid: [ + // Test is Ternary + outdent` + async function unicorn() { + if(a ? b : c){ + await a; + } else{ + await b; } - else{ - await thirdPromise(); + } + `, + // Consequent is Ternary + outdent` + async function unicorn() { + if(test){ + await a ? b : c; + } else{ + await b; } - }`, - parserOptions: { - ecmaVersion: 2020 } - }, + `, + // Alternate is Ternary + outdent` + async function unicorn() { + if(test){ + await a; + } else{ + await a ? b : c; + } + } + ` + ], + invalid: [ { code: outdent` - function* generator(){ - while(foo){ - if(bar){ - yield (a ? b : c) + async function unicorn() { + if(test){ + await a; + } else{ + await b; } - else{ - yield d + } + `, + output: outdent` + async function unicorn() { + await (test ? a : b); + } + `, + errors + } + ] +}); + +// ThrowStatement +ruleTester.run('prefer-ternary', rule, { + valid: [ + // Test is Ternary + outdent` + function unicorn() { + if(a ? b : c){ + throw a; + } else{ + throw b; + } + } + `, + // Consequent is Ternary + outdent` + function unicorn() { + if(test){ + throw a ? b : c; + } else{ + throw b; + } + } + `, + // Alternate is Ternary + outdent` + function unicorn() { + if(test){ + throw a; + } else{ + throw a ? b : c; + } + } + ` + ], + invalid: [ + { + code: outdent` + function unicorn() { + if(test){ + throw a; + } else{ + throw b; } } - }` + `, + output: outdent` + function unicorn() { + throw test ? a : b; + } + `, + errors }, { code: outdent` - function* generator(){ - while(foo){ - if(bar){ - yield baz() + async function unicorn() { + if(test){ + throw await a; + } else{ + throw b; } - else{ - yield* bat() + } + `, + output: outdent` + async function unicorn() { + throw test ? (await a) : b; + } + `, + errors + }, + { + code: outdent` + async function unicorn() { + if(test){ + throw await a; + } else{ + throw await b; } } - }` + `, + output: outdent` + async function unicorn() { + throw await (test ? a : b); + } + `, + errors } - ], + ] +}); +// AssignmentExpression +ruleTester.run('prefer-ternary', rule, { + valid: [ + // Different `left` + outdent` + function unicorn() { + if(test){ + foo = a; + } else{ + bar = b; + } + } + `, + // Same `left`, but not handled + outdent` + function unicorn() { + if(test){ + foo.bar = a; + } else{ + foo.bar = b; + } + } + `, + // Test is Ternary + outdent` + function unicorn() { + if(a ? b : c){ + foo = a; + } else{ + foo = b; + } + } + `, + // Consequent is Ternary + outdent` + function unicorn() { + if(test){ + foo = a ? b : c; + } else{ + foo = b; + } + } + `, + // Alternate is Ternary + outdent` + function unicorn() { + if(test){ + foo = a; + } else{ + foo = a ? b : c; + } + } + ` + ], invalid: [ { code: outdent` - if(foo){ - bar = 1; - } else{ - bar = 2; + function unicorn() { + if(test){ + foo = a; + } else{ + foo = b; + } + } + `, + output: outdent` + function unicorn() { + foo = test ? a : b; } `, - output: 'bar = (foo ? 1 : 2)', errors }, { code: outdent` - if(foo) - bar = 1; - else - bar = 2; - `, - output: 'bar = (foo ? 1 : 2)', - errors: [ - {column: 1, line: 1, type: 'IfStatement'} - ] + async function unicorn() { + if(test){ + foo = await a; + } else{ + foo = b; + } + } + `, + output: outdent` + async function unicorn() { + foo = test ? (await a) : b; + } + `, + errors }, { code: outdent` - function foo(){ - if(bar){ - return 1; - } - else{ - return 2; + async function unicorn() { + if(test){ + foo = await a; + } else{ + foo = await b; + } } - }`, + `, output: outdent` - function foo(){ - return (bar ? 1 : 2) - }`, - errors: [ - {column: 2, line: 2, type: 'IfStatement'} - ] + async function unicorn() { + foo = await (test ? a : b); + } + `, + errors }, { code: outdent` - async function foo(){ - if(bar){ - await firstPromise(); - } - else{ - await secondPromise(); + async function* unicorn() { + if(test){ + foo = yield await a; + } else{ + foo = yield await b; + } } - }`, - parserOptions: { - ecmaVersion: 2020 - }, + `, output: outdent` - async function foo(){ - await (bar ? firstPromise() : secondPromise()) - }`, - errors: [ - {column: 2, line: 2, type: 'IfStatement'} - ] - }, + async function* unicorn() { + foo = yield await (test ? a : b); + } + `, + errors + } + ] +}); + +ruleTester.run('prefer-ternary', rule, { + valid: [ + // No `consequent` / `alternate` + 'if (a) {b}', + 'if (a) {} else {b}', + 'if (a) {} else {}', + + // Call is not allow to merge + outdent` + if (test) { + a(); + } else { + b(); + } + ` + ], + invalid: [ + // Empty block should not matters { code: outdent` - function* generator(){ - while(foo){ - if(bar){ - yield bat - } - else{ - yield baz + function unicorn() { + if (test) { + ; // Empty block + return a; + } else { + return b; } } - }`, + `, output: outdent` - function* generator(){ - while(foo){ - yield (bar ? bat : baz) - } - }`, - errors: [ - {column: 3, line: 3, type: 'IfStatement'} - ] + function unicorn() { + return test ? a : b; + } + `, + errors }, + // `ExpressionStatement` or `BlockStatement` should not matters { code: outdent` - function* generator(){ - while(foo){ - if(bar){ - yield* bat() - } - else{ - yield* baz() - } + function unicorn() { + if (test) { + foo = a + } else foo = b; } - }`, + `, output: outdent` - function* generator(){ - while(foo){ - yield* (bar ? bat() : baz()) - } - }`, - errors: [ - {column: 3, line: 3, type: 'IfStatement'} - ] + function unicorn() { + foo = test ? a : b; + } + `, + errors }, + // No `ExpressionStatement` or `BlockStatement` should not matters { code: outdent` - function foo(){ - if (a) { - return 1; - } else if (b) { - return 2; - } else { - return 3; + function unicorn() { + if (test) return a; + else return b; } - }`, + `, output: outdent` - function foo(){ - if (a) { - return 1; - } else return (b ? 2 : 3) - }`, - errors: [ - {column: 9, line: 4, type: 'IfStatement'} - ] + function unicorn() { + return test ? a : b; + } + `, + errors }, + + // Nested + // [TBD]: this need discuss { code: outdent` - function foo(){ - if (a) { - return 1; - } else { - if (b) { + function foo(){ + if (a) { + return 1; + } else if (b) { return 2; } else { return 3; } } - }`, + `, output: outdent` - function foo(){ - if (a) { - return 1; - } else { - return (b ? 2 : 3) + function foo(){ + if (a) { + return 1; + } else return b ? 2 : 3; } - }`, - errors: [ - {column: 3, line: 5, type: 'IfStatement'} - ] + `, + errors }, { code: outdent` - function foo(){ - if (a) { - if (b) { + function foo(){ + if (a) { return 1; } else { - return 2; + if (b) { + return 2; + } else { + return 3; + } } - } else { - return 3; } - }`, + `, output: outdent` - function foo(){ - if (a) { - return (b ? 1 : 2) - } else { - return 3; + function foo(){ + if (a) { + return 1; + } else { + return b ? 2 : 3; + } } - }`, - errors: [ - {column: 3, line: 3, type: 'IfStatement'} - ] + `, + errors + }, + { + code: outdent` + function foo(){ + if (a) { + if (b) { + return 1; + } else { + return 2; + } + } else { + return 3; + } + } + `, + output: outdent` + function foo(){ + if (a) { + return b ? 1 : 2; + } else { + return 3; + } + } + `, + errors } ] }); @@ -348,9 +669,9 @@ babelRuleTester.run('prefer-ternary', rule, { } `, output: outdent` - attributeValue = (enableTrustedTypesIntegration ? (value: any) : '' + (value: any)) + attributeValue = enableTrustedTypesIntegration ? (value: any) : '' + (value: any); `, - errors: [{}] + errors } ] }); From 59b9b979e85e0c17437781fd0e98a6ba893c92f0 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 16:16:17 +0800 Subject: [PATCH 23/47] Crazy cases --- test/prefer-ternary.js | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 2540f53f46..c95a7d6f21 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -112,7 +112,25 @@ ruleTester.run('prefer-ternary', rule, { } `, errors - } + }, + // Crazy nested + { + code: outdent` + async function* unicorn() { + if(test){ + return yield await (foo = a); + } else{ + return yield await (foo = b); + } + } + `, + output: outdent` + async function* unicorn() { + return yield await (foo = test ? a : b); + } + `, + errors + }, ] }); @@ -382,7 +400,25 @@ ruleTester.run('prefer-ternary', rule, { } `, errors - } + }, + // Crazy nested + { + code: outdent` + async function* unicorn() { + if(test){ + throw yield await (foo = a); + } else{ + throw yield await (foo = b); + } + } + `, + output: outdent` + async function* unicorn() { + throw yield await (foo = test ? a : b); + } + `, + errors + }, ] }); From 51515d8d911035530bc3748c2cd994e362379a5a Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 16:28:54 +0800 Subject: [PATCH 24/47] Fix lower precedence --- rules/prefer-ternary.js | 9 ++++++- test/prefer-ternary.js | 55 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 851535e8c7..a9c97d6aba 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -42,7 +42,14 @@ const create = context => { const getParenthesizedText = node => { const text = sourceCode.getText(node); - return (isParenthesized(node, sourceCode) || node.type === 'AwaitExpression') ? + return ( + isParenthesized(node, sourceCode) || + node.type === 'AwaitExpression' || + // Lower precedence, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence#Table + node.type === 'AssignmentExpression' || + node.type === 'YieldExpression' || + node.type === 'SequenceExpression' + ) ? `(${text})` : text; }; diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index c95a7d6f21..d60ecefa69 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -615,6 +615,61 @@ ruleTester.run('prefer-ternary', rule, { `, errors }, + { + code: outdent` + function unicorn() { + if (test) return a; + else return b; + } + `, + output: outdent` + function unicorn() { + return test ? a : b; + } + `, + errors + }, + + // Precedence + { + code: outdent` + if (a = b) { + foo = 1; + } else foo = 2; + `, + output: 'foo = (a = b) ? 1 : 2;', + errors + }, + { + code: outdent` + function* unicorn() { + if (yield a) { + foo = 1; + } else foo = 2; + } + `, + output: outdent` + function* unicorn() { + foo = (yield a) ? 1 : 2; + } + `, + errors + }, + { + code: outdent` + function* unicorn() { + if (yield* a) { + foo = 1; + } else foo = 2; + } + `, + output: outdent` + function* unicorn() { + foo = (yield* a) ? 1 : 2; + } + `, + errors + }, // Nested // [TBD]: this need discuss From 66885910c12d31d1f7e79fa55f306c8958e610c2 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 16:38:50 +0800 Subject: [PATCH 25/47] Add `()` to yield --- rules/prefer-ternary.js | 4 ++-- test/prefer-ternary.js | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index a9c97d6aba..8472689bd0 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -91,8 +91,8 @@ const create = context => { !isTernary(alternate.argument) ) { return merge({ - before: `${before}yield${consequent.delegate ? '*' : ''} `, - after, + before: `${before}yield${consequent.delegate ? '*' : ''} (`, + after: `)${after}`, consequent: consequent.argument, alternate: alternate.argument }); diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index d60ecefa69..c6ee6de664 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -126,7 +126,7 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` async function* unicorn() { - return yield await (foo = test ? a : b); + return yield (await (foo = test ? a : b)); } `, errors @@ -201,7 +201,7 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` function* unicorn() { - yield test ? a : b; + yield (test ? a : b); } `, errors @@ -218,7 +218,7 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` function* unicorn() { - yield* test ? a : b; + yield* (test ? a : b); } `, errors @@ -235,7 +235,7 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` async function* unicorn() { - yield test ? (await a) : b; + yield (test ? (await a) : b); } `, errors @@ -252,7 +252,7 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` async function* unicorn() { - yield await (test ? a : b); + yield (await (test ? a : b)); } `, errors @@ -414,7 +414,7 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` async function* unicorn() { - throw yield await (foo = test ? a : b); + throw yield (await (foo = test ? a : b)); } `, errors @@ -540,7 +540,7 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` async function* unicorn() { - foo = yield await (test ? a : b); + foo = yield (await (test ? a : b)); } `, errors From 8c3a70f1e23b78a8409fa506b153e266c518dbd5 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 16:49:13 +0800 Subject: [PATCH 26/47] Update examples --- docs/rules/prefer-ternary.md | 110 ++++++++++++++++++++++------------- test/prefer-ternary.js | 4 +- 2 files changed, 70 insertions(+), 44 deletions(-) diff --git a/docs/rules/prefer-ternary.md b/docs/rules/prefer-ternary.md index ee1efe94c3..c25d65c41d 100644 --- a/docs/rules/prefer-ternary.md +++ b/docs/rules/prefer-ternary.md @@ -8,89 +8,115 @@ Additionally, using an `if-else` statement can result in defining variables usin This rule is fixable. - ## Fail ```js -let foo ='' -if(bar){ - foo = 3 -} -else{ - foo = 4 +function unicorn() { + if(test){ + return a; + } else { + return b; + } } ``` ```js -if(bar){ - return 3 -} -else{ - return 4 +function* unicorn() { + if(test){ + yield a; + } else { + yield b; + } } ``` ```js -if(bar){ - await firstPromise() -} -else{ - await secondPromise() +async function unicorn() { + if(test){ + await a(); + } else { + await b(); + } } ``` ```js -if(bar){ - yield bat +if(test){ + throw new Error('foo'); +} else { + throw new Error('bar'); } -else{ - yield baz +``` + +```js +let foo; +if(test){ + foo = 1; +} else { + foo = 2; } ``` ## Pass ```js -let foo = bar ? 3 : 4 +function unicorn() { + return test ? a : b; +} ``` ```js -return bar ? 3 : 4 +function* unicorn() { + yield (test ? a : b); +} ``` ```js -await (bar ? 3 : 4) +async function unicorn() { + await (test ? a() : b()); +} ``` ```js -yield (bar ? 3 : 4) +throw test ? new Error('foo') : new Error('bar'); ``` ```js -let foo = '' -if(bar){ - baz() - foo = 3 -} -else{ - foo = 4 -} +let foo; +foo = test ? 1 : 2; ``` + ```js -if(bar){ - foo = 3 -} -else{ - return 4 +// Multiple expressions +let foo; +let bar; +if(test){ + foo = 1; + bar = 2; +} else{ + foo = 2; } ``` ```js -if(bar){ - foo = 3 +// Different expressions +function unicorn() { + if(test){ + return a; + } else { + throw new Error('error'); + } } -else{ - baz = 4 +``` + +```js +// Assign to different variable +let foo; +let bar; +if(test){ + foo = 1; +} else{ + baz = 2; } ``` diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index c6ee6de664..45f66029dc 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -130,7 +130,7 @@ ruleTester.run('prefer-ternary', rule, { } `, errors - }, + } ] }); @@ -418,7 +418,7 @@ ruleTester.run('prefer-ternary', rule, { } `, errors - }, + } ] }); From 133c2fab81c25d07cfe0bb5c25c7b33aa3520dba Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 16:51:31 +0800 Subject: [PATCH 27/47] More tests --- test/prefer-ternary.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 45f66029dc..30e8241d8a 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -295,6 +295,23 @@ ruleTester.run('prefer-ternary', rule, { ` ], invalid: [ + { + code: outdent` + async function unicorn() { + if(test){ + await doSomething1(); + } else{ + await doSomething2(); + } + } + `, + output: outdent` + async function unicorn() { + await (test ? doSomething1() : doSomething2()); + } + `, + errors + }, { code: outdent` async function unicorn() { @@ -350,6 +367,23 @@ ruleTester.run('prefer-ternary', rule, { ` ], invalid: [ + { + code: outdent` + function unicorn() { + if(test){ + throw new Error('a'); + } else{ + throw new TypeError('a'); + } + } + `, + output: outdent` + function unicorn() { + throw test ? new Error('a') : new TypeError('a'); + } + `, + errors + }, { code: outdent` function unicorn() { From 9a7fc8e0aaba1a53473ff0676adf25ab374fb960 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 16:52:45 +0800 Subject: [PATCH 28/47] Style --- docs/rules/prefer-ternary.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/rules/prefer-ternary.md b/docs/rules/prefer-ternary.md index c25d65c41d..f82d5c640f 100644 --- a/docs/rules/prefer-ternary.md +++ b/docs/rules/prefer-ternary.md @@ -12,7 +12,7 @@ This rule is fixable. ```js function unicorn() { - if(test){ + if (test) { return a; } else { return b; @@ -22,7 +22,7 @@ function unicorn() { ```js function* unicorn() { - if(test){ + if (test) { yield a; } else { yield b; @@ -32,7 +32,7 @@ function* unicorn() { ```js async function unicorn() { - if(test){ + if (test) { await a(); } else { await b(); @@ -41,7 +41,7 @@ async function unicorn() { ``` ```js -if(test){ +if (test) { throw new Error('foo'); } else { throw new Error('bar'); @@ -50,7 +50,7 @@ if(test){ ```js let foo; -if(test){ +if (test) { foo = 1; } else { foo = 2; @@ -91,7 +91,7 @@ foo = test ? 1 : 2; // Multiple expressions let foo; let bar; -if(test){ +if (test) { foo = 1; bar = 2; } else{ @@ -102,7 +102,7 @@ if(test){ ```js // Different expressions function unicorn() { - if(test){ + if (test) { return a; } else { throw new Error('error'); @@ -114,7 +114,7 @@ function unicorn() { // Assign to different variable let foo; let bar; -if(test){ +if (test) { foo = 1; } else{ baz = 2; From 428b3c0f98e0f69c866be2977a7a454e6de91be4 Mon Sep 17 00:00:00 2001 From: fisker Date: Thu, 7 May 2020 10:12:49 +0800 Subject: [PATCH 29/47] Test `operator` --- rules/prefer-ternary.js | 3 ++- test/prefer-ternary.js | 51 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 8472689bd0..1e44ef71b4 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -127,13 +127,14 @@ const create = context => { if ( type === 'AssignmentExpression' && isSameAssignmentLeft(consequent.left, alternate.left) && + consequent.operator === alternate.operator && !isTernary(consequent.left) && !isTernary(alternate.left) && !isTernary(consequent.right) && !isTernary(alternate.right) ) { return merge({ - before: `${before}${sourceCode.getText(consequent.left)} = `, + before: `${before}${sourceCode.getText(consequent.left)} ${consequent.operator} `, after, consequent: consequent.right, alternate: alternate.right diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 30e8241d8a..f8ae902d24 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -469,6 +469,16 @@ ruleTester.run('prefer-ternary', rule, { } } `, + // Different `operator` + outdent` + function unicorn() { + if(test){ + foo = a; + } else{ + foo *= b; + } + } + `, // Same `left`, but not handled outdent` function unicorn() { @@ -528,6 +538,23 @@ ruleTester.run('prefer-ternary', rule, { `, errors }, + { + code: outdent` + function unicorn() { + if(test){ + foo *= a; + } else{ + foo *= b; + } + } + `, + output: outdent` + function unicorn() { + foo *= test ? a : b; + } + `, + errors + }, { code: outdent` async function unicorn() { @@ -562,6 +589,7 @@ ruleTester.run('prefer-ternary', rule, { `, errors }, + // Crazy nested { code: outdent` async function* unicorn() { @@ -578,6 +606,29 @@ ruleTester.run('prefer-ternary', rule, { } `, errors + }, + { + code: outdent` + if(test){ + $0 |= $1 ^= $2 &= $3 >>>= $4 >>= $5 <<= $6 %= $7 /= $8 *= $9 **= $10 -= $11 += $12 = + _STOP_ = + $0 |= $1 ^= $2 &= $3 >>>= $4 >>= $5 <<= $6 %= $7 /= $8 *= $9 **= $10 -= $11 += $12 = + 1; + } else{ + $0 |= $1 ^= $2 &= $3 >>>= $4 >>= $5 <<= $6 %= $7 /= $8 *= $9 **= $10 -= $11 += $12 = + _STOP_2_ = + $0 |= $1 ^= $2 &= $3 >>>= $4 >>= $5 <<= $6 %= $7 /= $8 *= $9 **= $10 -= $11 += $12 = + 2; + } + `, + output: outdent` + $0 |= $1 ^= $2 &= $3 >>>= $4 >>= $5 <<= $6 %= $7 /= $8 *= $9 **= $10 -= $11 += $12 = test ? (_STOP_ = + $0 |= $1 ^= $2 &= $3 >>>= $4 >>= $5 <<= $6 %= $7 /= $8 *= $9 **= $10 -= $11 += $12 = + 1) : (_STOP_2_ = + $0 |= $1 ^= $2 &= $3 >>>= $4 >>= $5 <<= $6 %= $7 /= $8 *= $9 **= $10 -= $11 += $12 = + 2); + `, + errors } ] }); From 59a79700fdf8a5555485ea6b51dc516127310a19 Mon Sep 17 00:00:00 2001 From: fisker Date: Thu, 7 May 2020 10:23:06 +0800 Subject: [PATCH 30/47] Ignore `IfStatement.alternate` --- rules/prefer-ternary.js | 1 + test/prefer-ternary.js | 37 +++++++++++++++---------------------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 1e44ef71b4..e590e468a2 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -6,6 +6,7 @@ const messageId = 'prefer-ternary'; const selector = [ 'IfStatement', + ':not(IfStatement > IfStatement.alternate)', '[test.type!="ConditionalExpression"]', '[consequent]', '[alternate]' diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index f8ae902d24..6c703ec402 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -647,6 +647,21 @@ ruleTester.run('prefer-ternary', rule, { } else { b(); } + `, + + // + outdent` + function foo(){ + if (a) { + return 1; + } else if (b) { + return 2; + } else if (c) { + return 3; + } else { + return 4; + } + } ` ], invalid: [ @@ -757,28 +772,6 @@ ruleTester.run('prefer-ternary', rule, { }, // Nested - // [TBD]: this need discuss - { - code: outdent` - function foo(){ - if (a) { - return 1; - } else if (b) { - return 2; - } else { - return 3; - } - } - `, - output: outdent` - function foo(){ - if (a) { - return 1; - } else return b ? 2 : 3; - } - `, - errors - }, { code: outdent` function foo(){ From da5dfd5640eeef4ad4b7b4763e047416fdd6afd8 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 14:55:18 +0800 Subject: [PATCH 31/47] Handle `return/yield` undefined --- rules/prefer-ternary.js | 30 ++++++++------ test/prefer-ternary.js | 88 +++++++++++++++++++++++++++++++---------- 2 files changed, 85 insertions(+), 33 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index e590e468a2..ea3d135f80 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -70,32 +70,28 @@ const create = context => { if ( type === 'ReturnStatement' && - consequent.argument && - alternate.argument && - !isTernary(consequent.argument) && - !isTernary(alternate.argument) + (consequent.argument === null || !isTernary(consequent.argument)) && + (alternate.argument === null || !isTernary(alternate.argument)) ) { return merge({ before: `${before}return `, after, - consequent: consequent.argument, - alternate: alternate.argument + consequent: consequent.argument === null ? 'undefined' : consequent.argument, + alternate: alternate.argument === null ? 'undefined' : alternate.argument }); } if ( type === 'YieldExpression' && consequent.delegate === alternate.delegate && - consequent.argument && - alternate.argument && - !isTernary(consequent.argument) && - !isTernary(alternate.argument) + (consequent.argument === null || !isTernary(consequent.argument)) && + (alternate.argument === null || !isTernary(alternate.argument)) ) { return merge({ before: `${before}yield${consequent.delegate ? '*' : ''} (`, after: `)${after}`, - consequent: consequent.argument, - alternate: alternate.argument + consequent: consequent.argument === null ? 'undefined' : consequent.argument, + alternate: alternate.argument === null ? 'undefined' : alternate.argument }); } @@ -160,7 +156,15 @@ const create = context => { node, messageId, fix: fixer => { - const fixed = `${result.before}${getParenthesizedText(node.test)} ? ${getParenthesizedText(result.consequent)} : ${getParenthesizedText(result.alternate)}${result.after}`; + const testText = getParenthesizedText(node.test); + const consequentText = typeof result.consequent === 'string' ? + result.consequent : + getParenthesizedText(result.consequent); + const alternateText = typeof result.alternate === 'string' ? + result.alternate : + getParenthesizedText(result.alternate); + + const fixed = `${result.before}${testText} ? ${consequentText} : ${alternateText}${result.after}`; return fixer.replaceText( node, fixed diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 6c703ec402..551113f273 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -20,16 +20,6 @@ const errors = [{messageId}]; // ReturnStatement ruleTester.run('prefer-ternary', rule, { valid: [ - // When ReturnStatement has no argument, should fix to `test ? undefined : …`, not handled - outdent` - function unicorn() { - if(test){ - return; - } else{ - return b; - } - } - `, // Test is Ternary outdent` function unicorn() { @@ -113,6 +103,40 @@ ruleTester.run('prefer-ternary', rule, { `, errors }, + { + code: outdent` + function unicorn() { + if(test){ + return; + } else{ + return b; + } + } + `, + output: outdent` + function unicorn() { + return test ? undefined : b; + } + `, + errors + }, + { + code: outdent` + async function unicorn() { + if(test){ + return; + } else{ + return await b; + } + } + `, + output: outdent` + async function unicorn() { + return test ? undefined : (await b); + } + `, + errors + }, // Crazy nested { code: outdent` @@ -137,16 +161,6 @@ ruleTester.run('prefer-ternary', rule, { // YieldExpression ruleTester.run('prefer-ternary', rule, { valid: [ - // When YieldExpression has no argument, should fix to `test ? undefined : …`, not handled - outdent` - function* unicorn() { - if(test){ - yield; - } else{ - yield b; - } - } - `, // Different `delegate` outdent` function* unicorn() { @@ -206,6 +220,40 @@ ruleTester.run('prefer-ternary', rule, { `, errors }, + { + code: outdent` + function* unicorn() { + if(test){ + yield; + } else{ + yield b; + } + } + `, + output: outdent` + function* unicorn() { + yield (test ? undefined : b); + } + `, + errors + }, + { + code: outdent` + async function* unicorn() { + if(test){ + yield; + } else{ + yield await b; + } + } + `, + output: outdent` + async function* unicorn() { + yield (test ? undefined : (await b)); + } + `, + errors + }, { code: outdent` function* unicorn() { From e58868767e2e5fd9833f7c2321faf6490ff9e528 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 15:56:23 +0800 Subject: [PATCH 32/47] Fix throw --- rules/prefer-ternary.js | 76 ++++++++++++++++++---- test/prefer-ternary.js | 140 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 194 insertions(+), 22 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index ea3d135f80..6abb7ddc9d 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -1,6 +1,8 @@ 'use strict'; const {isParenthesized} = require('eslint-utils'); +const {flatten} = require('lodash'); const getDocumentationUrl = require('./utils/get-documentation-url'); +const avoidCapture = require('./utils/avoid-capture'); const messageId = 'prefer-ternary'; @@ -38,6 +40,19 @@ function isSameAssignmentLeft(node1, node2) { return node1.type === node2.type && node1.type === 'Identifier' && node1.name === node2.name; } +const getIndentString = (node, sourceCode) => { + const {line, column} = sourceCode.getLocFromIndex(node.range[0]); + const lines = sourceCode.getLines(); + const before = lines[line - 1].slice(0, column); + + return before.match(/\s*$/)[0]; +}; + +const getScopes = scope => [ + scope, + ...flatten(scope.childScopes.map(scope => getScopes(scope))) +]; + const create = context => { const sourceCode = context.getSourceCode(); @@ -54,7 +69,7 @@ const create = context => { `(${text})` : text; }; - function merge(options, returnFalseIfNotMergeable = false) { + function merge(options, mergeOptions) { const { before = '', after = ';', @@ -62,6 +77,15 @@ const create = context => { alternate } = options; + const { + checkThrowStatement, + returnFalseIfNotMergeable + } = { + checkThrowStatement: false, + returnFalseIfNotMergeable: false, + ...mergeOptions, + }; + if (!consequent || !alternate || consequent.type !== alternate.type) { return returnFalseIfNotMergeable ? false : options; } @@ -109,16 +133,18 @@ const create = context => { } if ( + checkThrowStatement && type === 'ThrowStatement' && !isTernary(consequent.argument) && !isTernary(alternate.argument) ) { - return merge({ - before: `${before}throw `, - after, + return { + type, + before: `${before}const {{ERROR_NAME}} = `, + after: `;\n{{INDENT_STRING}}throw {{ERROR_NAME}};`, consequent: consequent.argument, alternate: alternate.argument - }); + }; } if ( @@ -141,12 +167,21 @@ const create = context => { return returnFalseIfNotMergeable ? false : options; } + const scopeToNamesGeneratedByFixer = new WeakMap(); + const isSafeName = (name, scopes) => scopes.every(scope => { + const generatedNames = scopeToNamesGeneratedByFixer.get(scope); + return !generatedNames || !generatedNames.has(name); + }); + return { [selector](node) { const consequent = getNodeBody(node.consequent); const alternate = getNodeBody(node.alternate); - const result = merge({consequent, alternate}, true); + const result = merge({consequent, alternate}, { + checkThrowStatement: true, + returnFalseIfNotMergeable: true + }); if (!result) { return; @@ -164,11 +199,30 @@ const create = context => { result.alternate : getParenthesizedText(result.alternate); - const fixed = `${result.before}${testText} ? ${consequentText} : ${alternateText}${result.after}`; - return fixer.replaceText( - node, - fixed - ); + let {type, before, after} = result; + + if (type === 'ThrowStatement') { + const scopes = getScopes(context.getScope()); + const errorName = avoidCapture('error', scopes, context.parserOptions.ecmaVersion, isSafeName); + + for (const scope of scopes) { + if (!scopeToNamesGeneratedByFixer.has(scope)) { + scopeToNamesGeneratedByFixer.set(scope, new Set()); + } + + const generatedNames = scopeToNamesGeneratedByFixer.get(scope); + generatedNames.add(errorName); + } + + const indentString = getIndentString(node, sourceCode); + after = after + .replace('{{INDENT_STRING}}', indentString) + .replace('{{ERROR_NAME}}', errorName); + before = before.replace('{{ERROR_NAME}}', errorName); + } + + const fixed = `${before}${testText} ? ${consequentText} : ${alternateText}${after}`; + return fixer.replaceText(node, fixed); } }); } diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 551113f273..c6142b9ceb 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -427,7 +427,8 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` function unicorn() { - throw test ? new Error('a') : new TypeError('a'); + const error = test ? new Error('a') : new TypeError('a'); + throw error; } `, errors @@ -444,7 +445,8 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` function unicorn() { - throw test ? a : b; + const error = test ? a : b; + throw error; } `, errors @@ -461,11 +463,13 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` async function unicorn() { - throw test ? (await a) : b; + const error = test ? (await a) : b; + throw error; } `, errors }, + // ThrowStatement don't check nested { code: outdent` async function unicorn() { @@ -478,29 +482,143 @@ ruleTester.run('prefer-ternary', rule, { `, output: outdent` async function unicorn() { - throw await (test ? a : b); + const error = test ? (await a) : (await b); + throw error; } `, errors }, - // Crazy nested + // `error` is used { code: outdent` - async function* unicorn() { + function unicorn() { + const error = new Error(); if(test){ - throw yield await (foo = a); + throw a; } else{ - throw yield await (foo = b); + throw b; } } `, output: outdent` - async function* unicorn() { - throw yield (await (foo = test ? a : b)); + function unicorn() { + const error = new Error(); + const error_ = test ? a : b; + throw error_; } `, errors - } + }, + // Child scope + { + code: outdent` + function unicorn() { + if(test){ + throw a; + } else{ + throw b; + } + try {} catch(error) { + const error_ = new TypeError(error); + throw error_; + } + } + `, + output: outdent` + function unicorn() { + const error__ = test ? a : b; + throw error__; + try {} catch(error) { + const error_ = new TypeError(error); + throw error_; + } + } + `, + errors + }, + // Global + { + code: outdent` + function unicorn() { + if(test){ + throw a; + } else{ + throw b; + } + function foo() { + throw error; + } + } + `, + output: outdent` + function unicorn() { + const error_ = test ? a : b; + throw error_; + function foo() { + throw error; + } + } + `, + errors + }, + // Multiple + { + code: outdent` + function unicorn() { + if(test){ + throw a; + } else{ + throw b; + } + if(test){ + throw a; + } else{ + throw b; + } + } + `, + output: outdent` + function unicorn() { + const error = test ? a : b; + throw error; + const error_ = test ? a : b; + throw error_; + } + `, + errors: [...errors, ...errors] + }, + // Multiple nested + { + code: outdent` + function outer() { + if(test){ + throw a; + } else{ + throw b; + } + + function inner() { + if(test){ + throw a; + } else{ + throw b; + } + } + } + `, + output: outdent` + function outer() { + const error = test ? a : b; + throw error; + + function inner() { + const error_ = test ? a : b; + throw error_; + } + } + `, + errors: [...errors, ...errors] + }, ] }); From aca1f1e7029055f6ffc6ff9f1ebd4e60b214248a Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 16:07:21 +0800 Subject: [PATCH 33/47] Clean up --- rules/prefer-ternary.js | 9 ++-- test/prefer-ternary.js | 114 +++++++++++++++++++++++++++++++--------- 2 files changed, 93 insertions(+), 30 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 6abb7ddc9d..cc0e5c90b5 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -94,8 +94,8 @@ const create = context => { if ( type === 'ReturnStatement' && - (consequent.argument === null || !isTernary(consequent.argument)) && - (alternate.argument === null || !isTernary(alternate.argument)) + !isTernary(consequent.argument) && + !isTernary(alternate.argument) ) { return merge({ before: `${before}return `, @@ -108,8 +108,8 @@ const create = context => { if ( type === 'YieldExpression' && consequent.delegate === alternate.delegate && - (consequent.argument === null || !isTernary(consequent.argument)) && - (alternate.argument === null || !isTernary(alternate.argument)) + !isTernary(consequent.argument) && + !isTernary(alternate.argument) ) { return merge({ before: `${before}yield${consequent.delegate ? '*' : ''} (`, @@ -138,6 +138,7 @@ const create = context => { !isTernary(consequent.argument) && !isTernary(alternate.argument) ) { + // `ThrowStatement` don't check nested return { type, before: `${before}const {{ERROR_NAME}} = `, diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index c6142b9ceb..0b99e7cbf1 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -396,9 +396,9 @@ ruleTester.run('prefer-ternary', rule, { // Consequent is Ternary outdent` function unicorn() { - if(test){ + if (test) { throw a ? b : c; - } else{ + } else { throw b; } } @@ -406,9 +406,9 @@ ruleTester.run('prefer-ternary', rule, { // Alternate is Ternary outdent` function unicorn() { - if(test){ + if (test) { throw a; - } else{ + } else { throw a ? b : c; } } @@ -418,7 +418,7 @@ ruleTester.run('prefer-ternary', rule, { { code: outdent` function unicorn() { - if(test){ + if (test) { throw new Error('a'); } else{ throw new TypeError('a'); @@ -436,9 +436,9 @@ ruleTester.run('prefer-ternary', rule, { { code: outdent` function unicorn() { - if(test){ + if (test) { throw a; - } else{ + } else { throw b; } } @@ -451,12 +451,68 @@ ruleTester.run('prefer-ternary', rule, { `, errors }, + // Indention + { + code: outdent` + function unicorn() { + /* comment cause wrong indention */ if (test) { + throw a; + } else { + throw b; + } + } + `, + output: outdent` + function unicorn() { + /* comment cause wrong indention */ const error = test ? a : b; + throw error; + } + `, + errors + }, + { + code: outdent` + function unicorn() { + if (test) { + throw a; + } else { + throw b; + } + } + `, + output: outdent` + function unicorn() { + const error = test ? a : b; + throw error; + } + `, + errors + }, + // Space + { + code: outdent` + function unicorn() { + if (test) { + throw new Error('a'); + } else { + throw new TypeError('a'); + } + } + `.replace(/\t/g, ' '), + output: outdent` + function unicorn() { + const error = test ? new Error('a') : new TypeError('a'); + throw error; + } + `.replace(/\t/g, ' '), + errors + }, { code: outdent` async function unicorn() { - if(test){ + if (test) { throw await a; - } else{ + } else { throw b; } } @@ -469,13 +525,13 @@ ruleTester.run('prefer-ternary', rule, { `, errors }, - // ThrowStatement don't check nested + // `ThrowStatement` don't check nested { code: outdent` async function unicorn() { - if(test){ + if (test) { throw await a; - } else{ + } else { throw await b; } } @@ -493,9 +549,9 @@ ruleTester.run('prefer-ternary', rule, { code: outdent` function unicorn() { const error = new Error(); - if(test){ + if (test) { throw a; - } else{ + } else { throw b; } } @@ -513,11 +569,12 @@ ruleTester.run('prefer-ternary', rule, { { code: outdent` function unicorn() { - if(test){ + if (test) { throw a; - } else{ + } else { throw b; } + try {} catch(error) { const error_ = new TypeError(error); throw error_; @@ -528,6 +585,7 @@ ruleTester.run('prefer-ternary', rule, { function unicorn() { const error__ = test ? a : b; throw error__; + try {} catch(error) { const error_ = new TypeError(error); throw error_; @@ -540,11 +598,12 @@ ruleTester.run('prefer-ternary', rule, { { code: outdent` function unicorn() { - if(test){ + if (test) { throw a; - } else{ + } else { throw b; } + function foo() { throw error; } @@ -554,6 +613,7 @@ ruleTester.run('prefer-ternary', rule, { function unicorn() { const error_ = test ? a : b; throw error_; + function foo() { throw error; } @@ -565,14 +625,15 @@ ruleTester.run('prefer-ternary', rule, { { code: outdent` function unicorn() { - if(test){ + if (test) { throw a; - } else{ + } else { throw b; } - if(test){ + + if (test) { throw a; - } else{ + } else { throw b; } } @@ -581,6 +642,7 @@ ruleTester.run('prefer-ternary', rule, { function unicorn() { const error = test ? a : b; throw error; + const error_ = test ? a : b; throw error_; } @@ -591,16 +653,16 @@ ruleTester.run('prefer-ternary', rule, { { code: outdent` function outer() { - if(test){ + if (test) { throw a; - } else{ + } else { throw b; } function inner() { - if(test){ + if (test) { throw a; - } else{ + } else { throw b; } } From efff08c68dfb367272a662521f310b294010a9cc Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 16:11:10 +0800 Subject: [PATCH 34/47] Code style --- rules/prefer-ternary.js | 22 ++++++++++++---------- test/prefer-ternary.js | 2 +- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index cc0e5c90b5..ad146ff1a4 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -55,6 +55,11 @@ const getScopes = scope => [ const create = context => { const sourceCode = context.getSourceCode(); + const scopeToNamesGeneratedByFixer = new WeakMap(); + const isSafeName = (name, scopes) => scopes.every(scope => { + const generatedNames = scopeToNamesGeneratedByFixer.get(scope); + return !generatedNames || !generatedNames.has(name); + }); const getParenthesizedText = node => { const text = sourceCode.getText(node); @@ -83,7 +88,7 @@ const create = context => { } = { checkThrowStatement: false, returnFalseIfNotMergeable: false, - ...mergeOptions, + ...mergeOptions }; if (!consequent || !alternate || consequent.type !== alternate.type) { @@ -168,12 +173,6 @@ const create = context => { return returnFalseIfNotMergeable ? false : options; } - const scopeToNamesGeneratedByFixer = new WeakMap(); - const isSafeName = (name, scopes) => scopes.every(scope => { - const generatedNames = scopeToNamesGeneratedByFixer.get(scope); - return !generatedNames || !generatedNames.has(name); - }); - return { [selector](node) { const consequent = getNodeBody(node.consequent); @@ -188,6 +187,8 @@ const create = context => { return; } + const scope = context.getScope(); + context.report({ node, messageId, @@ -203,7 +204,7 @@ const create = context => { let {type, before, after} = result; if (type === 'ThrowStatement') { - const scopes = getScopes(context.getScope()); + const scopes = getScopes(scope); const errorName = avoidCapture('error', scopes, context.parserOptions.ecmaVersion, isSafeName); for (const scope of scopes) { @@ -216,9 +217,10 @@ const create = context => { } const indentString = getIndentString(node, sourceCode); + after = after - .replace('{{INDENT_STRING}}', indentString) - .replace('{{ERROR_NAME}}', errorName); + .replace('{{INDENT_STRING}}', indentString) + .replace('{{ERROR_NAME}}', errorName); before = before.replace('{{ERROR_NAME}}', errorName); } diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 0b99e7cbf1..068723412f 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -680,7 +680,7 @@ ruleTester.run('prefer-ternary', rule, { } `, errors: [...errors, ...errors] - }, + } ] }); From 003acc33f586c6da08e2e50a6ac8d0416795b6ff Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 16:13:24 +0800 Subject: [PATCH 35/47] Update docs --- docs/rules/prefer-ternary.md | 4 ++-- rules/prefer-ternary.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rules/prefer-ternary.md b/docs/rules/prefer-ternary.md index f82d5c640f..0dbeafdc9b 100644 --- a/docs/rules/prefer-ternary.md +++ b/docs/rules/prefer-ternary.md @@ -78,7 +78,8 @@ async function unicorn() { ``` ```js -throw test ? new Error('foo') : new Error('bar'); +const error = test ? new Error('foo') : new Error('bar'); +throw error; ``` ```js @@ -86,7 +87,6 @@ let foo; foo = test ? 1 : 2; ``` - ```js // Multiple expressions let foo; diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index ad146ff1a4..9af686aa5e 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -147,7 +147,7 @@ const create = context => { return { type, before: `${before}const {{ERROR_NAME}} = `, - after: `;\n{{INDENT_STRING}}throw {{ERROR_NAME}};`, + after: ';\n{{INDENT_STRING}}throw {{ERROR_NAME}};', consequent: consequent.argument, alternate: alternate.argument }; From 8768b41ce5af132c3b8c969725c72b5bfca52796 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 16:17:28 +0800 Subject: [PATCH 36/47] Comment --- rules/prefer-ternary.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 9af686aa5e..182132c642 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -36,7 +36,7 @@ function getNodeBody(node) { } function isSameAssignmentLeft(node1, node2) { - // [TBD]: Allow more types of left + // [TODO]: Allow more types of left return node1.type === node2.type && node1.type === 'Identifier' && node1.name === node2.name; } From 853acf96d2cd4305c97a272d6f774dcc5a182d2a Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 16:29:01 +0800 Subject: [PATCH 37/47] Fix lint --- rules/no-for-loop.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js index 07393681f9..a4ec89363b 100644 --- a/rules/no-for-loop.js +++ b/rules/no-for-loop.js @@ -384,11 +384,9 @@ const create = context => { } if (elementNode) { - if (removeDeclaration) { - yield fixer.removeRange(getRemovalRange(elementNode, sourceCode)); - } else { - yield fixer.replaceText(elementNode.init, element); - } + yield removeDeclaration ? + fixer.removeRange(getRemovalRange(elementNode, sourceCode)) : + fixer.replaceText(elementNode.init, element); } }; } From 397c74996f80622ed5e8ab7d11d70c5ad49a47a9 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 16:47:28 +0800 Subject: [PATCH 38/47] Add extra integration test --- test/integration/projects.js | 8 ++++++-- test/integration/test.js | 2 +- test/integration/unicorn/error-name-conflicts.js | 12 ++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 test/integration/unicorn/error-name-conflicts.js diff --git a/test/integration/projects.js b/test/integration/projects.js index 1c028914e3..ca323428b1 100644 --- a/test/integration/projects.js +++ b/test/integration/projects.js @@ -1,5 +1,5 @@ 'use strict'; - +const path = require('path'); const typescriptArguments = ['--parser', '@typescript-eslint/parser', '--ext', '.ts,.js']; const vueArguments = ['--parser', 'vue-eslint-parser', '--ext', '.vue,.js']; @@ -120,7 +120,11 @@ module.exports = [ 'https://github.com/chakra-ui/chakra-ui', 'https://github.com/ReactTraining/react-router', 'https://github.com/facebook/relay', - 'https://github.com/mozilla/pdf.js' + 'https://github.com/mozilla/pdf.js', + { + name: 'unicorn', + location: path.join(__dirname, 'unicorn'), + } ].map(project => { if (typeof project === 'string') { project = {repository: project}; diff --git a/test/integration/test.js b/test/integration/test.js index f1f5927775..600927fec8 100755 --- a/test/integration/test.js +++ b/test/integration/test.js @@ -81,7 +81,7 @@ const makeEslintTask = (project, destination) => { }; const execute = project => { - const destination = path.join(__dirname, 'fixtures', project.name); + const destination = project.location || path.join(__dirname, 'fixtures', project.name); return new Listr([ { diff --git a/test/integration/unicorn/error-name-conflicts.js b/test/integration/unicorn/error-name-conflicts.js new file mode 100644 index 0000000000..f42ad9e0ae --- /dev/null +++ b/test/integration/unicorn/error-name-conflicts.js @@ -0,0 +1,12 @@ +function foo() { + try { + } catch (err) { + console.log(err); + + if (test) { + throw a; + } else { + throw b; + } + } +} From f39f38f8be1d9f0b76a0a430a869b3a04bb420b0 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 16:48:00 +0800 Subject: [PATCH 39/47] Move to top --- test/integration/projects.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/projects.js b/test/integration/projects.js index ca323428b1..c7c29b5ed3 100644 --- a/test/integration/projects.js +++ b/test/integration/projects.js @@ -4,6 +4,10 @@ const typescriptArguments = ['--parser', '@typescript-eslint/parser', '--ext', ' const vueArguments = ['--parser', 'vue-eslint-parser', '--ext', '.vue,.js']; module.exports = [ + { + name: 'unicorn', + location: path.join(__dirname, 'unicorn'), + }, { repository: 'https://github.com/avajs/ava', extraArguments: [ @@ -120,11 +124,7 @@ module.exports = [ 'https://github.com/chakra-ui/chakra-ui', 'https://github.com/ReactTraining/react-router', 'https://github.com/facebook/relay', - 'https://github.com/mozilla/pdf.js', - { - name: 'unicorn', - location: path.join(__dirname, 'unicorn'), - } + 'https://github.com/mozilla/pdf.js' ].map(project => { if (typeof project === 'string') { project = {repository: project}; From 23974b367ff74379197861321ee670c6d3ba4acd Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 16:51:24 +0800 Subject: [PATCH 40/47] ignore --- package.json | 3 +++ test/integration/projects.js | 2 +- test/lint/lint.js | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index f8f1b8944c..e39c30c1a3 100644 --- a/package.json +++ b/package.json @@ -91,6 +91,9 @@ "extends": [ "plugin:eslint-plugin/all" ], + "ignores": [ + "test/integration/{fixtures,unicorn}/**" + ], "overrides": [ { "files": "rules/utils/*.js", diff --git a/test/integration/projects.js b/test/integration/projects.js index c7c29b5ed3..b046f3a291 100644 --- a/test/integration/projects.js +++ b/test/integration/projects.js @@ -6,7 +6,7 @@ const vueArguments = ['--parser', 'vue-eslint-parser', '--ext', '.vue,.js']; module.exports = [ { name: 'unicorn', - location: path.join(__dirname, 'unicorn'), + location: path.join(__dirname, 'unicorn') }, { repository: 'https://github.com/avajs/ava', diff --git a/test/lint/lint.js b/test/lint/lint.js index 9776b88101..5ef7f432a4 100755 --- a/test/lint/lint.js +++ b/test/lint/lint.js @@ -17,7 +17,8 @@ const eslint = new ESLint({ overrideConfig: { ignorePatterns: [ 'coverage', - 'test/integration/fixtures' + 'test/integration/fixtures', + 'test/integration/unicorn' ] } }); From 5b99354bad6dc1129b13e8345d5f7f3d2dab8f21 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 17:56:24 +0800 Subject: [PATCH 41/47] Fix error message --- test/integration/test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/integration/test.js b/test/integration/test.js index 600927fec8..18367bd5ff 100755 --- a/test/integration/test.js +++ b/test/integration/test.js @@ -150,7 +150,12 @@ list.run() const {file, project, destination} = error2.eslintJob; const {line} = error2.eslintMessage; - console.error(chalk.gray(`${project.repository}/tree/master/${path.relative(destination, file.filePath)}#L${line}`)); + if (project.repository) { + console.error(chalk.gray(`${project.repository}/blob/master/${path.relative(destination, file.filePath)}#L${line}`)); + } else { + console.error(chalk.gray(`${path.relative(destination, file.filePath)}#L${line}`)); + } + console.error(chalk.gray(JSON.stringify(error2.eslintMessage, undefined, 2))); } } From 7cb5ee27ea531099552041ded1dd72ce3fd4ef9e Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 2 Jun 2020 18:58:19 +0800 Subject: [PATCH 42/47] Coverage --- rules/prefer-ternary.js | 1 + test/prefer-ternary.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 182132c642..76a2363ea2 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -17,6 +17,7 @@ const selector = [ const isTernary = node => node && node.type === 'ConditionalExpression'; function getNodeBody(node) { + /* istanbul ignore next */ if (!node) { return; } diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 068723412f..2800e1a4ae 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -120,6 +120,23 @@ ruleTester.run('prefer-ternary', rule, { `, errors }, + { + code: outdent` + function unicorn() { + if(test){ + return; + } else{ + return; + } + } + `, + output: outdent` + function unicorn() { + return test ? undefined : undefined; + } + `, + errors + }, { code: outdent` async function unicorn() { @@ -237,6 +254,23 @@ ruleTester.run('prefer-ternary', rule, { `, errors }, + { + code: outdent` + function* unicorn() { + if(test){ + yield; + } else{ + yield; + } + } + `, + output: outdent` + function* unicorn() { + yield (test ? undefined : undefined); + } + `, + errors + }, { code: outdent` async function* unicorn() { From dd81f37524e6425159e02eadb6321bdd86477a23 Mon Sep 17 00:00:00 2001 From: fisker Date: Sun, 27 Sep 2020 10:57:06 +0800 Subject: [PATCH 43/47] Fix test --- rules/prefer-ternary.js | 12 +++++++- test/prefer-ternary.js | 66 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index 76a2363ea2..f0854db25f 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -1,6 +1,7 @@ 'use strict'; const {isParenthesized} = require('eslint-utils'); const {flatten} = require('lodash'); +const FixTracker = require('eslint/lib/rules/utils/fix-tracker'); const getDocumentationUrl = require('./utils/get-documentation-url'); const avoidCapture = require('./utils/avoid-capture'); @@ -189,6 +190,7 @@ const create = context => { } const scope = context.getScope(); + const sourceCode = context.getSourceCode(); context.report({ node, @@ -204,6 +206,7 @@ const create = context => { let {type, before, after} = result; + let generateNewVariables = false; if (type === 'ThrowStatement') { const scopes = getScopes(scope); const errorName = avoidCapture('error', scopes, context.parserOptions.ecmaVersion, isSafeName); @@ -223,10 +226,17 @@ const create = context => { .replace('{{INDENT_STRING}}', indentString) .replace('{{ERROR_NAME}}', errorName); before = before.replace('{{ERROR_NAME}}', errorName); + generateNewVariables = true; } const fixed = `${before}${testText} ? ${consequentText} : ${alternateText}${after}`; - return fixer.replaceText(node, fixed); + if (!generateNewVariables) { + return fixer.replaceText(node, fixed); + } + + return new FixTracker(fixer, sourceCode) + .retainRange(sourceCode.ast.range) + .replaceTextRange(node.range, fixed); } }); } diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 2800e1a4ae..6b468aa700 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -656,6 +656,7 @@ ruleTester.run('prefer-ternary', rule, { errors }, // Multiple + // This will fix one by one, see next test { code: outdent` function unicorn() { @@ -677,13 +678,42 @@ ruleTester.run('prefer-ternary', rule, { const error = test ? a : b; throw error; + if (test) { + throw a; + } else { + throw b; + } + } + `, + errors: [...errors, ...errors] + }, + // This `code` is `output` from previous test + { + code: outdent` + function unicorn() { + const error = test ? a : b; + throw error; + + if (test) { + throw a; + } else { + throw b; + } + } + `, + output: outdent` + function unicorn() { + const error = test ? a : b; + throw error; + const error_ = test ? a : b; throw error_; } `, - errors: [...errors, ...errors] + errors: errors }, // Multiple nested + // This will fix one by one, see next test { code: outdent` function outer() { @@ -707,13 +737,45 @@ ruleTester.run('prefer-ternary', rule, { const error = test ? a : b; throw error; + function inner() { + if (test) { + throw a; + } else { + throw b; + } + } + } + `, + errors: [...errors, ...errors] + }, + // This `code` is `output` from previous test + { + code: outdent` + function outer() { + const error = test ? a : b; + throw error; + + function inner() { + if (test) { + throw a; + } else { + throw b; + } + } + } + `, + output: outdent` + function outer() { + const error = test ? a : b; + throw error; + function inner() { const error_ = test ? a : b; throw error_; } } `, - errors: [...errors, ...errors] + errors: errors } ] }); From cee4f4dbf34e28ec3b517cb2fcb25c8efa487798 Mon Sep 17 00:00:00 2001 From: fisker Date: Sun, 27 Sep 2020 11:07:52 +0800 Subject: [PATCH 44/47] Handle `IfStatement` is body of some node --- rules/prefer-ternary.js | 29 ++++++++++++++++++++--------- test/prefer-ternary.js | 13 +++++++++++++ 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/rules/prefer-ternary.js b/rules/prefer-ternary.js index f0854db25f..350b4f3d22 100644 --- a/rules/prefer-ternary.js +++ b/rules/prefer-ternary.js @@ -81,7 +81,8 @@ const create = context => { before = '', after = ';', consequent, - alternate + alternate, + node } = options; const { @@ -108,7 +109,8 @@ const create = context => { before: `${before}return `, after, consequent: consequent.argument === null ? 'undefined' : consequent.argument, - alternate: alternate.argument === null ? 'undefined' : alternate.argument + alternate: alternate.argument === null ? 'undefined' : alternate.argument, + node }); } @@ -122,7 +124,8 @@ const create = context => { before: `${before}yield${consequent.delegate ? '*' : ''} (`, after: `)${after}`, consequent: consequent.argument === null ? 'undefined' : consequent.argument, - alternate: alternate.argument === null ? 'undefined' : alternate.argument + alternate: alternate.argument === null ? 'undefined' : alternate.argument, + node }); } @@ -135,7 +138,8 @@ const create = context => { before: `${before}await (`, after: `)${after}`, consequent: consequent.argument, - alternate: alternate.argument + alternate: alternate.argument, + node }); } @@ -146,10 +150,14 @@ const create = context => { !isTernary(alternate.argument) ) { // `ThrowStatement` don't check nested + + // If `IfStatement` is not a `BlockStatement`, need add `{}` + const {parent} = node; + const needBraces = parent && parent.type !== 'BlockStatement'; return { type, - before: `${before}const {{ERROR_NAME}} = `, - after: ';\n{{INDENT_STRING}}throw {{ERROR_NAME}};', + before: `${before}${needBraces ? '{\n{{INDENT_STRING}}' : ''}const {{ERROR_NAME}} = `, + after: `;\n{{INDENT_STRING}}throw {{ERROR_NAME}};${needBraces ? '\n}' : ''}`, consequent: consequent.argument, alternate: alternate.argument }; @@ -168,7 +176,8 @@ const create = context => { before: `${before}${sourceCode.getText(consequent.left)} ${consequent.operator} `, after, consequent: consequent.right, - alternate: alternate.right + alternate: alternate.right, + node }); } @@ -180,7 +189,7 @@ const create = context => { const consequent = getNodeBody(node.consequent); const alternate = getNodeBody(node.alternate); - const result = merge({consequent, alternate}, { + const result = merge({node, consequent, alternate}, { checkThrowStatement: true, returnFalseIfNotMergeable: true }); @@ -225,7 +234,9 @@ const create = context => { after = after .replace('{{INDENT_STRING}}', indentString) .replace('{{ERROR_NAME}}', errorName); - before = before.replace('{{ERROR_NAME}}', errorName); + before = before + .replace('{{INDENT_STRING}}', indentString) + .replace('{{ERROR_NAME}}', errorName); generateNewVariables = true; } diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index 6b468aa700..f00f0aedc3 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -776,6 +776,19 @@ ruleTester.run('prefer-ternary', rule, { } `, errors: errors + }, + // Need `{}` + { + code: outdent` + while (foo) if (test) {throw a} else {throw b} + `, + output: outdent` + while (foo) { + const error = test ? a : b; + throw error; + } + `, + errors: errors } ] }); From b37b4a976a52c25fc43df541e9af30bd93e418ae Mon Sep 17 00:00:00 2001 From: fisker Date: Sun, 27 Sep 2020 11:56:03 +0800 Subject: [PATCH 45/47] Fix code style --- test/prefer-ternary.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/prefer-ternary.js b/test/prefer-ternary.js index f00f0aedc3..652dbf8015 100644 --- a/test/prefer-ternary.js +++ b/test/prefer-ternary.js @@ -710,7 +710,7 @@ ruleTester.run('prefer-ternary', rule, { throw error_; } `, - errors: errors + errors }, // Multiple nested // This will fix one by one, see next test @@ -775,7 +775,7 @@ ruleTester.run('prefer-ternary', rule, { } } `, - errors: errors + errors }, // Need `{}` { @@ -788,7 +788,7 @@ ruleTester.run('prefer-ternary', rule, { throw error; } `, - errors: errors + errors } ] }); From 332ae2180d945c6243a1e6e00c3b3dc58910e4b3 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Tue, 29 Sep 2020 23:55:30 +0200 Subject: [PATCH 46/47] Update prefer-ternary.md --- docs/rules/prefer-ternary.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rules/prefer-ternary.md b/docs/rules/prefer-ternary.md index 0dbeafdc9b..67f916852b 100644 --- a/docs/rules/prefer-ternary.md +++ b/docs/rules/prefer-ternary.md @@ -1,10 +1,10 @@ # Prefer ternary expressions over simple `if-else` statements -This rule enforces the use of ternary expressions over 'simple' `if-else` statements where 'simple' means the consequent and alternate are each one line and have the same basic type and form. +This rule enforces the use of ternary expressions over 'simple' `if-else` statements, where 'simple' means the consequent and alternate are each one line and have the same basic type and form. -Using an `if-else` statement typically results in more lines of code than a single lined ternary expression, which leads to an unnecessarily larger codebase that is more difficult to maintain. +Using an `if-else` statement typically results in more lines of code than a single-line ternary expression, which leads to an unnecessarily larger codebase that is more difficult to maintain. -Additionally, using an `if-else` statement can result in defining variables using `let` or `var` solely to be reassigned within the blocks. This leads to varaibles being unnecessarily mutable and prevents `prefer-const` from flagging the variable. +Additionally, using an `if-else` statement can result in defining variables using `let` or `var` solely to be reassigned within the blocks. This leads to variables being unnecessarily mutable and prevents `prefer-const` from flagging the variable. This rule is fixable. From cab98306b8b76036784aeafc405cf96b35112050 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Tue, 29 Sep 2020 23:56:42 +0200 Subject: [PATCH 47/47] Update projects.js --- test/integration/projects.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration/projects.js b/test/integration/projects.js index 3ace851a17..0be3fbad02 100644 --- a/test/integration/projects.js +++ b/test/integration/projects.js @@ -1,5 +1,6 @@ 'use strict'; const path = require('path'); + const typescriptArguments = ['--parser', '@typescript-eslint/parser', '--ext', '.ts,.js']; const vueArguments = ['--parser', 'vue-eslint-parser', '--ext', '.vue,.js'];