Skip to content

Commit

Permalink
Update: for yoda, add a fixer (#7199)
Browse files Browse the repository at this point in the history
  • Loading branch information
not-an-aardvark authored and vitorbal committed Sep 29, 2016
1 parent 742ae67 commit 89787b2
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 28 deletions.
2 changes: 2 additions & 0 deletions docs/rules/yoda.md
@@ -1,5 +1,7 @@
# Require or disallow Yoda Conditions (yoda)

(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixes problems reported by this rule.

Yoda conditions are so named because the literal value of the condition comes first while the variable comes second. For example, the following is a Yoda condition:

```js
Expand Down
67 changes: 40 additions & 27 deletions lib/rules/yoda.js
Expand Up @@ -141,7 +141,9 @@ module.exports = {
},
additionalProperties: false
}
]
],

fixable: "code"
},

create(context) {
Expand Down Expand Up @@ -219,46 +221,57 @@ module.exports = {
isParenWrapped());
}

const OPERATOR_FLIP_MAP = {
"===": "===",
"!==": "!==",
"==": "==",
"!=": "!=",
"<": ">",
">": "<",
"<=": ">=",
">=": "<="
};

/**
* Returns a string representation of a BinaryExpression node with its sides/operator flipped around.
* @param {ASTNode} node The BinaryExpression node
* @returns {string} A string representation of the node with the sides and operator flipped
*/
function getFlippedString(node) {
const operatorToken = sourceCode.getTokensBetween(node.left, node.right).find(token => token.value === node.operator);
const textBeforeOperator = sourceCode.getText().slice(node.left.range[1], operatorToken.range[0]);
const textAfterOperator = sourceCode.getText().slice(operatorToken.range[1], node.right.range[0]);
const leftText = sourceCode.getText(node.left);
const rightText = sourceCode.getText(node.right);

return rightText + textBeforeOperator + OPERATOR_FLIP_MAP[operatorToken.value] + textAfterOperator + leftText;
}

//--------------------------------------------------------------------------
// Public
//--------------------------------------------------------------------------

return {
BinaryExpression: always ? function(node) {

// Comparisons must always be yoda-style: if ("blue" === color)
if (
(node.right.type === "Literal" || looksLikeLiteral(node.right)) &&
!(node.left.type === "Literal" || looksLikeLiteral(node.left)) &&
!(!isEqualityOperator(node.operator) && onlyEquality) &&
isComparisonOperator(node.operator) &&
!(exceptRange && isRangeTest(context.getAncestors().pop()))
) {
context.report({
node,
message: "Expected literal to be on the left side of {{operator}}.",
data: {
operator: node.operator
}
});
}

} : function(node) {
BinaryExpression(node) {
const expectedLiteral = always ? node.left : node.right;
const expectedNonLiteral = always ? node.right : node.left;

// Comparisons must never be yoda-style (default)
// If `expectedLiteral` is not a literal, and `expectedNonLiteral` is a literal, raise an error.
if (
(node.left.type === "Literal" || looksLikeLiteral(node.left)) &&
!(node.right.type === "Literal" || looksLikeLiteral(node.right)) &&
(expectedNonLiteral.type === "Literal" || looksLikeLiteral(expectedNonLiteral)) &&
!(expectedLiteral.type === "Literal" || looksLikeLiteral(expectedLiteral)) &&
!(!isEqualityOperator(node.operator) && onlyEquality) &&
isComparisonOperator(node.operator) &&
!(exceptRange && isRangeTest(context.getAncestors().pop()))
) {
context.report({
node,
message: "Expected literal to be on the right side of {{operator}}.",
message: "Expected literal to be on the {{expectedSide}} side of {{operator}}.",
data: {
operator: node.operator
}
operator: node.operator,
expectedSide: always ? "left" : "right"
},
fix: fixer => fixer.replaceText(node, getFlippedString(node))
});
}

Expand Down
69 changes: 68 additions & 1 deletion tests/lib/rules/yoda.js
Expand Up @@ -79,6 +79,7 @@ ruleTester.run("yoda", rule, {

{
code: "if (\"red\" == value) {}",
output: "if (value == \"red\") {}",
options: ["never"],
errors: [
{
Expand All @@ -89,6 +90,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (true === value) {}",
output: "if (value === true) {}",
options: ["never"],
errors: [
{
Expand All @@ -99,6 +101,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (5 != value) {}",
output: "if (value != 5) {}",
options: ["never"],
errors: [
{
Expand All @@ -109,6 +112,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (null !== value) {}",
output: "if (value !== null) {}",
options: ["never"],
errors: [
{
Expand All @@ -119,6 +123,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (\"red\" <= value) {}",
output: "if (value >= \"red\") {}",
options: ["never"],
errors: [
{
Expand All @@ -129,6 +134,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (true >= value) {}",
output: "if (value <= true) {}",
options: ["never"],
errors: [
{
Expand All @@ -139,6 +145,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "var foo = (5 < value) ? true : false",
output: "var foo = (value > 5) ? true : false",
options: ["never"],
errors: [
{
Expand All @@ -149,6 +156,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "function foo() { return (null > value); }",
output: "function foo() { return (value < null); }",
options: ["never"],
errors: [
{
Expand All @@ -159,6 +167,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (-1 < str.indexOf(substr)) {}",
output: "if (str.indexOf(substr) > -1) {}",
options: ["never"],
errors: [
{
Expand All @@ -169,6 +178,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (value == \"red\") {}",
output: "if (\"red\" == value) {}",
options: ["always"],
errors: [
{
Expand All @@ -179,6 +189,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (value === true) {}",
output: "if (true === value) {}",
options: ["always"],
errors: [
{
Expand All @@ -189,6 +200,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (a < 0 && 0 <= b && b < 1) {}",
output: "if (a < 0 && b >= 0 && b < 1) {}",
options: ["never", { exceptRange: true }],
errors: [
{
Expand All @@ -199,6 +211,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (0 <= a && a < 1 && b < 1) {}",
output: "if (a >= 0 && a < 1 && b < 1) {}",
options: ["never", { exceptRange: true }],
errors: [
{
Expand All @@ -209,6 +222,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (1 < a && a < 0) {}",
output: "if (a > 1 && a < 0) {}",
options: ["never", { exceptRange: true }],
errors: [
{
Expand All @@ -219,6 +233,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "0 < a && a < 1",
output: "a > 0 && a < 1",
options: ["never", { exceptRange: true }],
errors: [
{
Expand All @@ -229,6 +244,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "var a = b < 0 || 1 <= b;",
output: "var a = b < 0 || b >= 1;",
options: ["never", { exceptRange: true }],
errors: [
{
Expand All @@ -239,6 +255,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (0 <= x && x < -1) {}",
output: "if (x >= 0 && x < -1) {}",
options: ["never", { exceptRange: true }],
errors: [
{
Expand All @@ -249,6 +266,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "var a = (b < 0 && 0 <= b);",
output: "var a = (0 > b && 0 <= b);",
options: ["always", { exceptRange: true }],
errors: [
{
Expand All @@ -259,6 +277,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (0 <= a[b] && a['b'] < 1) {}",
output: "if (a[b] >= 0 && a['b'] < 1) {}",
options: ["never", { exceptRange: true }],
errors: [
{
Expand All @@ -269,6 +288,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (0 <= a[b()] && a[b()] < 1) {}",
output: "if (a[b()] >= 0 && a[b()] < 1) {}",
options: ["never", { exceptRange: true }],
errors: [
{
Expand All @@ -279,6 +299,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (3 == a) {}",
output: "if (a == 3) {}",
options: ["never", { onlyEquality: true }],
errors: [
{
Expand All @@ -289,6 +310,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "foo(3 === a);",
output: "foo(a === 3);",
options: ["never", { onlyEquality: true }],
errors: [
{
Expand All @@ -299,6 +321,7 @@ ruleTester.run("yoda", rule, {
},
{
code: "foo(a === 3);",
output: "foo(3 === a);",
options: ["always", { onlyEquality: true }],
errors: [
{
Expand All @@ -309,13 +332,57 @@ ruleTester.run("yoda", rule, {
},
{
code: "if (0 <= x && x < 1) {}",
output: "if (x >= 0 && x < 1) {}",
errors: [
{
message: "Expected literal to be on the right side of <=.",
type: "BinaryExpression"
}
]
},
{
code: "if ( /* a */ 0 /* b */ < /* c */ foo /* d */ ) {}",
output: "if ( /* a */ foo /* b */ > /* c */ 0 /* d */ ) {}",
options: ["never"],
errors: [
{
message: "Expected literal to be on the right side of <.",
type: "BinaryExpression"
}
]
},
{
code: "if ( /* a */ foo /* b */ > /* c */ 0 /* d */ ) {}",
output: "if ( /* a */ 0 /* b */ < /* c */ foo /* d */ ) {}",
options: ["always"],
errors: [
{
message: "Expected literal to be on the left side of >.",
type: "BinaryExpression"
}
]
},
{
code: "if (foo()===1) {}",
output: "if (1===foo()) {}",
options: ["always"],
errors: [
{
message: "Expected literal to be on the left side of ===.",
type: "BinaryExpression"
}
]
},
{
code: "if (foo() === 1) {}",
output: "if (1 === foo()) {}",
options: ["always"],
errors: [
{
message: "Expected literal to be on the left side of ===.",
type: "BinaryExpression"
}
]
}

]
});

0 comments on commit 89787b2

Please sign in to comment.