Skip to content

Commit

Permalink
Update: fix false positive/negative of yoda rule (fixes #7676) (#7695)
Browse files Browse the repository at this point in the history
* Update: fix false negative of yoda about computed (fixes #7676)

* Fix: false positive of yoda about ranges (fixes #7676)

* Fix: use `astUtils.getStaticPropertyName`

* Chore: update jsdoc comments
  • Loading branch information
mysticatea authored and btmills committed Dec 9, 2016
1 parent e95a230 commit e569225
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 10 deletions.
51 changes: 41 additions & 10 deletions lib/rules/yoda.js
Expand Up @@ -4,6 +4,12 @@
*/
"use strict";

//--------------------------------------------------------------------------
// Requirements
//--------------------------------------------------------------------------

const astUtils = require("../ast-utils");

//--------------------------------------------------------------------------
// Helpers
//--------------------------------------------------------------------------
Expand Down Expand Up @@ -54,13 +60,16 @@ function looksLikeLiteral(node) {
/**
* Attempts to derive a Literal node from nodes that are treated like literals.
* @param {ASTNode} node Node to normalize.
* @returns {ASTNode} The original node if the node is already a Literal, or a
* normalized Literal node with the negative number as the
* value if the node represents a negative number literal,
* otherwise null if the node cannot be converted to a
* normalized literal.
* @param {number} [defaultValue] The default value to be returned if the node
* is not a Literal.
* @returns {ASTNode} One of the following options.
* 1. The original node if the node is already a Literal
* 2. A normalized Literal node with the negative number as the value if the
* node represents a negative number literal.
* 3. The Literal node which has the `defaultValue` argument if it exists.
* 4. Otherwise `null`.
*/
function getNormalizedLiteral(node) {
function getNormalizedLiteral(node, defaultValue) {
if (node.type === "Literal") {
return node;
}
Expand All @@ -73,6 +82,14 @@ function getNormalizedLiteral(node) {
};
}

if (defaultValue) {
return {
type: "Literal",
value: defaultValue,
raw: String(defaultValue)
};
}

return null;
}

Expand All @@ -98,12 +115,26 @@ function same(a, b) {
case "Literal":
return a.value === b.value;

case "MemberExpression":
case "MemberExpression": {
const nameA = astUtils.getStaticPropertyName(a);

// x.y = x["y"]
if (nameA) {
return (
same(a.object, b.object) &&
nameA === astUtils.getStaticPropertyName(b)
);
}

// x[0] = x[0]
// x[y] = x[y]
// x.y = x.y
return same(a.object, b.object) && same(a.property, b.property);
return (
a.computed === b.computed &&
same(a.object, b.object) &&
same(a.property, b.property)
);
}

case "ThisExpression":
return true;
Expand Down Expand Up @@ -178,7 +209,7 @@ module.exports = {

return (node.operator === "&&" &&
(leftLiteral = getNormalizedLiteral(left.left)) &&
(rightLiteral = getNormalizedLiteral(right.right)) &&
(rightLiteral = getNormalizedLiteral(right.right, Number.POSITIVE_INFINITY)) &&
leftLiteral.value <= rightLiteral.value &&
same(left.right, right.left));
}
Expand All @@ -191,7 +222,7 @@ module.exports = {
let leftLiteral, rightLiteral;

return (node.operator === "||" &&
(leftLiteral = getNormalizedLiteral(left.right)) &&
(leftLiteral = getNormalizedLiteral(left.right, Number.NEGATIVE_INFINITY)) &&
(rightLiteral = getNormalizedLiteral(right.left)) &&
leftLiteral.value <= rightLiteral.value &&
same(left.left, right.right));
Expand Down
26 changes: 26 additions & 0 deletions tests/lib/rules/yoda.js
Expand Up @@ -68,6 +68,21 @@ ruleTester.run("yoda", rule, {
}, {
code: "if (0 <= this.prop && this.prop <= 1) {}",
options: ["never", { exceptRange: true }]
}, {
code: "if (0 <= index && index < list.length) {}",
options: ["never", { exceptRange: true }]
}, {
code: "if (ZERO <= index && index < 100) {}",
options: ["never", { exceptRange: true }]
}, {
code: "if (value <= MIN || 10 < value) {}",
options: ["never", { exceptRange: true }]
}, {
code: "if (value <= 0 || MAX < value) {}",
options: ["never", { exceptRange: true }]
}, {
code: "if (0 <= a.b && a[\"b\"] <= 100) {}",
options: ["never", { exceptRange: true }]
},

// onlyEquality
Expand Down Expand Up @@ -286,6 +301,17 @@ 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: [
{
message: "Expected literal to be on the right side of <=.",
type: "BinaryExpression"
}
]
},
{
code: "if (0 <= a[b()] && a[b()] < 1) {}",
output: "if (a[b()] >= 0 && a[b()] < 1) {}",
Expand Down

0 comments on commit e569225

Please sign in to comment.