Skip to content

Commit

Permalink
Update: Add "allow" option to allow specific operators (fixes #3308)
Browse files Browse the repository at this point in the history 
New "allow" option has been introduced to allow specific operators.
  • Loading branch information
@rajenp
rajenp committed Jan 14, 2016
1 parent 2dba9c7 commit 700b8bc
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 57 deletions.
44 changes: 39 additions & 5 deletions docs/rules/no-implicit-coercion.md
View file
Expand Up @@ -31,19 +31,27 @@ This rule is aimed to flag shorter notations for the type conversion, then sugge

### Options

This rule has three options.
This rule has three main options and one override option to allow some coercions as required.

```json
```js
{
"rules": {
"no-implicit-coercion": [2, {"boolean": true, "number": true, "string": true}]
}
"rules": {
"no-implicit-coercion": [2, {
"boolean": true,
"number": true,
"string": true,
"allow": [/* "!!", "~", "*", "+" */]
}]
}
}
```

* `"boolean"` (`true` by default) - When this is `true`, this rule warns shorter type conversions for `boolean` type.
* `"number"` (`true` by default) - When this is `true`, this rule warns shorter type conversions for `number` type.
* `"string"` (`true` by default) - When this is `true`, this rule warns shorter type conversions for `string` type.
* `"array"` (`empty` by default) - Each entry in this array can be one of `~`, `!!`, `+` or `*` that are to be allowed.

Note that operator `+` in `allow` list would allow `+foo` (number coercion) as well as `"" + foo` (string coercion).

#### `boolean`

Expand Down Expand Up @@ -110,6 +118,32 @@ The following patterns are not considered problems:
var b = String(foo);
```

#### Fine-grained control

Using `allow` list, we can override and allow specific operators.

For example, when the configuration is like this:

```json
{
"rules": {
"no-implicit-coercion": [2, {
"boolean": true,
"number": true,
"string": true,
"allow": ["!!", "~"]
}]
}
}
```

The following patterns are not considered problems:

```js
var b = !!foo;
var c = ~foo.indexOf(".");
```

## When Not to Use It

If you don't want to be notified about shorter notations for the type conversion, you can safely disable this rule.
95 changes: 57 additions & 38 deletions lib/rules/no-implicit-coercion.js
View file
Expand Up @@ -11,6 +11,7 @@
//------------------------------------------------------------------------------

var INDEX_OF_PATTERN = /^(?:i|lastI)ndexOf$/;
var ALLOWABLE_OPERATORS = ["~", "!!", "+", "*"];

/**
* Parses and normalizes an option object.
Expand All @@ -22,7 +23,8 @@ function parseOptions(options) {
return {
boolean: "boolean" in options ? Boolean(options.boolean) : true,
number: "number" in options ? Boolean(options.number) : true,
string: "string" in options ? Boolean(options.string) : true
string: "string" in options ? Boolean(options.string) : true,
allow: options.allow || []
};
}

Expand Down Expand Up @@ -90,7 +92,8 @@ function isNumeric(node) {
* @returns {ASTNode|null} The first non-numeric item in the BinaryExpression tree or null
*/
function getNonNumericOperand(node) {
var left = node.left, right = node.right;
var left = node.left,
right = node.right;

if (right.type !== "BinaryExpression" && !isNumeric(right)) {
return right;
Expand Down Expand Up @@ -136,87 +139,103 @@ function getOtherOperand(node, value) {
}
return node.left;
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = function(context) {
var options = parseOptions(context.options[0]);
var options = parseOptions(context.options[0]),
operatorAllowed = false;

return {
"UnaryExpression": function(node) {
// !!foo
if (options.boolean && isDoubleLogicalNegating(node)) {
operatorAllowed = options.allow.indexOf("!!") >= 0;
if (!operatorAllowed && options.boolean && isDoubleLogicalNegating(node)) {
context.report(
node,
"use 'Boolean({{code}})' instead.",
{code: context.getSource(node.argument.argument)});
"use `Boolean({{code}})` instead.", {
code: context.getSource(node.argument.argument)
});
}

// ~foo.indexOf(bar)
if (options.boolean && isBinaryNegatingOfIndexOf(node)) {
operatorAllowed = options.allow.indexOf("~") >= 0;
if (!operatorAllowed && options.boolean && isBinaryNegatingOfIndexOf(node)) {
context.report(
node,
"use '{{code}} !== -1' instead.",
{code: context.getSource(node.argument)});
"use `{{code}} !== -1` instead.", {
code: context.getSource(node.argument)
});
}

// +foo
if (options.number && node.operator === "+" && !isNumeric(node.argument)) {
operatorAllowed = options.allow.indexOf("+") >= 0;
if (!operatorAllowed && options.number && node.operator === "+" && !isNumeric(node.argument)) {
context.report(
node,
"use 'Number({{code}})' instead.",
{code: context.getSource(node.argument)});
"use `Number({{code}})` instead.", {
code: context.getSource(node.argument)
});
}
},

// Use `:exit` to prevent double reporting
"BinaryExpression:exit": function(node) {
// 1 * foo
var nonNumericOperand = options.number && isMultiplyByOne(node) && getNonNumericOperand(node);
operatorAllowed = options.allow.indexOf("*") >= 0;
var nonNumericOperand = !operatorAllowed && options.number && isMultiplyByOne(node) && getNonNumericOperand(node);
if (nonNumericOperand) {
context.report(
node,
"use 'Number({{code}})' instead.",
{code: context.getSource(nonNumericOperand)});
"use `Number({{code}})` instead.", {
code: context.getSource(nonNumericOperand)
});
}

// "" + foo
if (options.string && isConcatWithEmptyString(node)) {
operatorAllowed = options.allow.indexOf("+") >= 0;
if (!operatorAllowed && options.string && isConcatWithEmptyString(node)) {
context.report(
node,
"use 'String({{code}})' instead.",
{code: context.getSource(getOtherOperand(node, ""))});
"use `String({{code}})` instead.", {
code: context.getSource(getOtherOperand(node, ""))
});
}
},

"AssignmentExpression": function(node) {
// foo += ""
operatorAllowed = options.allow.indexOf("+") >= 0;
if (options.string && isAppendEmptyString(node)) {
context.report(
node,
"use '{{code}} = String({{code}})' instead.",
{code: context.getSource(getOtherOperand(node, ""))});
"use `{{code}} = String({{code}})` instead.", {
code: context.getSource(getOtherOperand(node, ""))
});
}
}
};
};

module.exports.schema = [
{
"type": "object",
"properties": {
"boolean": {
"type": "boolean"
},
"number": {
"type": "boolean"
},
"string": {
"type": "boolean"
}
module.exports.schema = [{
"type": "object",
"properties": {
"boolean": {
"type": "boolean"
},
"additionalProperties": false
}
];
"number": {
"type": "boolean"
},
"string": {
"type": "boolean"
},
"allow": {
"type": "array",
"items": {
"enum": ALLOWABLE_OPERATORS
},
"uniqueItems": true
}
},
"additionalProperties": false
}];
39 changes: 25 additions & 14 deletions tests/lib/rules/no-implicit-coercion.js
View file
Expand Up @@ -69,21 +69,32 @@ ruleTester.run("no-implicit-coercion", rule, {
{code: "+foo", options: [{number: false}]},
{code: "1*foo", options: [{number: false}]},
{code: "\"\"+foo", options: [{string: false}]},
{code: "foo += \"\"", options: [{string: false}]}
{code: "foo += \"\"", options: [{string: false}]},
{code: "var a = !!foo", options: [{boolean: true, allow: ["!!"]}]},
{code: "var a = ~foo.indexOf(1)", options: [{boolean: true, allow: ["~"]}]},
{code: "var a = ~foo", options: [{boolean: true}]},
{code: "var a = 1 * foo", options: [{boolean: true, allow: ["*"]}]},
{code: "var a = +foo", options: [{boolean: true, allow: ["+"]}]},
{code: "var a = \"\" + foo", options: [{boolean: true, string: true, allow: ["+"]}]}
],
invalid: [
{code: "!!foo", errors: [{message: "use 'Boolean(foo)' instead.", type: "UnaryExpression"}]},
{code: "!!(foo + bar)", errors: [{message: "use 'Boolean(foo + bar)' instead.", type: "UnaryExpression"}]},
{code: "~foo.indexOf(1)", errors: [{message: "use 'foo.indexOf(1) !== -1' instead.", type: "UnaryExpression"}]},
{code: "~foo.bar.indexOf(2)", errors: [{message: "use 'foo.bar.indexOf(2) !== -1' instead.", type: "UnaryExpression"}]},
{code: "+foo", errors: [{message: "use 'Number(foo)' instead.", type: "UnaryExpression"}]},
{code: "+foo.bar", errors: [{message: "use 'Number(foo.bar)' instead.", type: "UnaryExpression"}]},
{code: "1*foo", errors: [{message: "use 'Number(foo)' instead.", type: "BinaryExpression"}]},
{code: "foo*1", errors: [{message: "use 'Number(foo)' instead.", type: "BinaryExpression"}]},
{code: "1*foo.bar", errors: [{message: "use 'Number(foo.bar)' instead.", type: "BinaryExpression"}]},
{code: "\"\"+foo", errors: [{message: "use 'String(foo)' instead.", type: "BinaryExpression"}]},
{code: "foo+\"\"", errors: [{message: "use 'String(foo)' instead.", type: "BinaryExpression"}]},
{code: "\"\"+foo.bar", errors: [{message: "use 'String(foo.bar)' instead.", type: "BinaryExpression"}]},
{code: "foo += \"\"", errors: [{message: "use 'foo = String(foo)' instead.", type: "AssignmentExpression"}]}
{code: "!!foo", errors: [{message: "use `Boolean(foo)` instead.", type: "UnaryExpression"}]},
{code: "!!(foo + bar)", errors: [{message: "use `Boolean(foo + bar)` instead.", type: "UnaryExpression"}]},
{code: "~foo.indexOf(1)", errors: [{message: "use `foo.indexOf(1) !== -1` instead.", type: "UnaryExpression"}]},
{code: "~foo.bar.indexOf(2)", errors: [{message: "use `foo.bar.indexOf(2) !== -1` instead.", type: "UnaryExpression"}]},
{code: "+foo", errors: [{message: "use `Number(foo)` instead.", type: "UnaryExpression"}]},
{code: "+foo.bar", errors: [{message: "use `Number(foo.bar)` instead.", type: "UnaryExpression"}]},
{code: "1*foo", errors: [{message: "use `Number(foo)` instead.", type: "BinaryExpression"}]},
{code: "foo*1", errors: [{message: "use `Number(foo)` instead.", type: "BinaryExpression"}]},
{code: "1*foo.bar", errors: [{message: "use `Number(foo.bar)` instead.", type: "BinaryExpression"}]},
{code: "\"\"+foo", errors: [{message: "use `String(foo)` instead.", type: "BinaryExpression"}]},
{code: "foo+\"\"", errors: [{message: "use `String(foo)` instead.", type: "BinaryExpression"}]},
{code: "\"\"+foo.bar", errors: [{message: "use `String(foo.bar)` instead.", type: "BinaryExpression"}]},
{code: "foo += \"\"", errors: [{message: "use `foo = String(foo)` instead.", type: "AssignmentExpression"}]},
{code: "var a = !!foo", options: [{boolean: true, allow: ["~"]}], errors: [{message: "use `Boolean(foo)` instead.", type: "UnaryExpression"}]},
{code: "var a = ~foo.indexOf(1)", options: [{boolean: true, allow: ["!!"]}], errors: [{message: "use `foo.indexOf(1) !== -1` instead.", type: "UnaryExpression"}]},
{code: "var a = 1 * foo", options: [{boolean: true, allow: ["+"]}], errors: [{message: "use `Number(foo)` instead.", type: "BinaryExpression"}]},
{code: "var a = +foo", options: [{boolean: true, allow: ["*"]}], errors: [{message: "use `Number(foo)` instead.", type: "UnaryExpression"}]},
{code: "var a = \"\" + foo", options: [{boolean: true, allow: ["*"]}], errors: [{message: "use `String(foo)` instead.", type: "BinaryExpression"}]}
]
});

0 comments on commit 700b8bc

Please sign in to comment.