Skip to content

Commit

Permalink
Update: New parameters for quote-props rule (fixes #1283, fixes #1658)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tomasz Olędzki authored and Ian VanSchooten committed Jul 29, 2015
1 parent 547c62b commit 6af4b35
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 12 deletions.
79 changes: 76 additions & 3 deletions docs/rules/quote-props.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,20 @@ This may look alright at first sight, but this code in fact throws a syntax erro

## Rule Details

This rule aims to enforce use of quotes in property names and as such will flag any properties that don't use quotes.
This rule aims to enforce use of quotes in property names and as such will flag any properties that don't use quotes (default behavior).

### Options

There are two behaviors for this rule: `"always"` (default) and `"as-needed"`. You can define these options in your configuration as:
There are four behaviors for this rule: `"always"` (default), `"as-needed"`, `"consistent"` and `"consistent-as-needed"`. You can define these options in your configuration as:

```json
{
"quote-props": [2, "as-needed"]
}
```

#### always

When configured with `"always"` as the first option (the default), quoting for all properties will be enforced. Some believe that ensuring property names in object literals are always wrapped in quotes is generally a good idea, since [depending on the property name you may need to quote them anyway](https://mathiasbynens.be/notes/javascript-properties). Consider this example:

```js
Expand All @@ -52,7 +54,7 @@ var object = {
};
```

Here, the properties `foo` and `baz` are not wrapped in quotes, but `qux-lorem` is, because it doesn’t work without the quotes. This is rather inconsistent. Instead, you may prefer to quote property names consistently:
Here, the properties `foo` and `baz` are not wrapped in quotes, but `qux-lorem` is, because it doesn’t work without the quotes. This is rather inconsistent. Instead, you may prefer to quote names of all properties:

```js
var object = {
Expand Down Expand Up @@ -104,6 +106,8 @@ var object3 = {
};
```

#### as-needed

When configured with `"as-needed"` as the first option (the default), the following patterns are considered warnings:

```js
Expand Down Expand Up @@ -136,6 +140,75 @@ var object3 = {
};
```

#### consistent

When configured with `"consistent"`, the patterns below are considered warnings. Basically `"consistent"` means all or no properties are expected to be quoted, in other words quoting style can't be mixed within an object. Please note the latter situation (no quotation at all) isn't always possible as some property names require quoting.

```js
var object1 = {
foo: "bar",
"baz": 42,
"qux-lorem": true
};

var object2 = {
'foo': 'bar',
baz: 42
};
```

The following patterns are considered okay and do not cause warnings:

```js
var object1 = {
"foo": "bar",
"baz": 42,
"qux-lorem": true
};

var object2 = {
'foo': 'bar',
'baz': 42
};

var object3 = {
foo: 'bar',
baz: 42
};
```

#### consistent-as-needed

When configured with `"consistent-as-needed"`, the behavior is similar to `"consistent"` with one difference. Namely, properties' quoting should be consistent (as in `"consistent"`) but whenever all quotes are redundant a warning is raised. In other words if at least one property name has to be quoted (like `qux-lorem`) then all property names must be quoted, otherwise no properties can be quoted. The following patterns are considered warnings:

```js
var object1 = {
foo: "bar",
"baz": 42,
"qux-lorem": true
};

var object2 = {
'foo': 'bar',
'baz': 42
};
```

The following patterns are considered okay and do not cause warnings:

```js
var object1 = {
"foo": "bar",
"baz": 42,
"qux-lorem": true
};

var object2 = {
foo: 'bar',
baz: 42
};
```

## When Not To Use It

If you don't care if property names are consistently wrapped in quotes or not, turn this rule off.
83 changes: 75 additions & 8 deletions lib/rules/quote-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* @fileoverview Rule to flag non-quoted property names in object literals.
* @author Mathias Bynens <http://mathiasbynens.be/>
* @copyright 2014 Brandon Mills. All rights reserved.
* @copyright 2015 Tomasz Olędzki. All rights reserved.
*/
"use strict";

Expand All @@ -19,12 +20,24 @@ module.exports = function(context) {

var MODE = context.options[0];

/**
* Checks if an espree-tokenized key has redundant quotes (i.e. whether quotes are unnecessary)
* @param {espreeTokens} tokens The espree-tokenized node key
* @returns {boolean} Whether or not a key has redundant quotes.
* @private
*/
function areQuotesRedundant(tokens) {
return tokens.length === 1 &&
(["Identifier", "Null", "Boolean"].indexOf(tokens[0].type) >= 0 ||
(tokens[0].type === "Numeric" && "" + +tokens[0].value === tokens[0].value));
}

/**
* Ensures that a property's key is quoted only when necessary
* @param {ASTNode} node Property AST node
* @returns {void}
*/
function asNeeded(node) {
function checkUnnecessaryQuotes(node) {
var key = node.key,
tokens;

Expand All @@ -35,10 +48,7 @@ module.exports = function(context) {
return;
}

if (tokens.length === 1 &&
(["Identifier", "Null", "Boolean"].indexOf(tokens[0].type) >= 0 ||
(tokens[0].type === "Numeric" && "" + +tokens[0].value === tokens[0].value))
) {
if (areQuotesRedundant(tokens)) {
context.report(node, "Unnecessarily quoted property `{{value}}` found.", key);
}
}
Expand All @@ -49,7 +59,7 @@ module.exports = function(context) {
* @param {ASTNode} node Property AST node
* @returns {void}
*/
function always(node) {
function checkOmittedQuotes(node) {
var key = node.key;

if (!node.method && !(key.type === "Literal" && typeof key.value === "string")) {
Expand All @@ -59,14 +69,71 @@ module.exports = function(context) {
}
}

/**
* Ensures that an object's keys are consistenly quoted, optionally checks for redundancy of quotes
* @param {ASTNode} node Property AST node
* @param {boolean} checkQuotesRedundancy Whether to check quotes' redundancy
* @returns {void}
*/
function checkConsistency(node, checkQuotesRedundancy) {
var quotes = false,
lackOfQuotes = false,
necessaryQuotes = false;

node.properties.forEach(function(property) {
var key = property.key,
tokens;

if (key.type === "Literal" && typeof key.value === "string") {
quotes = true;
if (checkQuotesRedundancy) {
try {
tokens = espree.tokenize(key.value);
} catch (e) {
necessaryQuotes = true;
return;
}
necessaryQuotes = necessaryQuotes || !areQuotesRedundant(tokens);
}
} else {
lackOfQuotes = true;
}

if (quotes && lackOfQuotes) {
context.report(node, "Inconsistently quoted property `{{key}}` found.", {
key: key.name || key.value
});
}
});

if (checkQuotesRedundancy && quotes && !necessaryQuotes) {
context.report(node, "Properties shouldn't be quoted as all quotes are redundant.");
}
}

return {
"Property": MODE === "as-needed" ? asNeeded : always
"Property": function(node) {
if (MODE === "always" || !MODE) {
checkOmittedQuotes(node);
}
if (MODE === "as-needed") {
checkUnnecessaryQuotes(node);
}
},
"ObjectExpression": function(node) {
if (MODE === "consistent") {
checkConsistency(node, false);
}
if (MODE === "consistent-as-needed") {
checkConsistency(node, true);
}
}
};

};

module.exports.schema = [
{
"enum": ["always", "as-needed"]
"enum": ["always", "as-needed", "consistent", "consistent-as-needed"]
}
];
41 changes: 40 additions & 1 deletion tests/lib/rules/quote-props.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* @fileoverview Tests for quote-props rule.
* @author Mathias Bynens <http://mathiasbynens.be/>
* @copyright 2015 Tomasz Olędzki. All rights reserved.
*/

"use strict";
Expand Down Expand Up @@ -34,7 +35,15 @@ eslintTester.addRuleTest("lib/rules/quote-props", {
{ code: "({ a: 0, 'if': 0 })", options: ["as-needed"] },
{ code: "({ a: 0, '@': 0 })", options: ["as-needed"] },
{ code: "({ a: 0, 0: 0 })", options: ["as-needed"] },
{ code: "({ a: 0, '0x0': 0 })", options: ["as-needed"] }
{ code: "({ a: 0, '0x0': 0 })", options: ["as-needed"] },
{ code: "({ 'a': 0, '-b': 0 })", options: ["consistent"] },
{ code: "({ 'true': 0, 'b': 0 })", options: ["consistent"] },
{ code: "({ null: 0, a: 0 })", options: ["consistent"] },
{ code: "({ a: 0, b: 0 })", options: ["consistent"] },
{ code: "({ a: 0, b: 0 })", options: ["consistent-as-needed"] },
{ code: "({ a: 0, null: 0 })", options: ["consistent-as-needed"] },
{ code: "({ 'a': 0, '-b': 0 })", options: ["consistent-as-needed"] },
{ code: "({ '@': 0, 'B': 0 })", options: ["consistent-as-needed"] }
],
invalid: [{
code: "({ a: 0 })",
Expand Down Expand Up @@ -70,5 +79,35 @@ eslintTester.addRuleTest("lib/rules/quote-props", {
errors: [{
message: "Unnecessarily quoted property `0` found.", type: "Property"
}]
}, {
code: "({ '-a': 0, b: 0 })",
options: ["consistent"],
errors: [{
message: "Inconsistently quoted property `b` found.", type: "ObjectExpression"
}]
}, {
code: "({ a: 0, 'b': 0 })",
options: ["consistent"],
errors: [{
message: "Inconsistently quoted property `b` found.", type: "ObjectExpression"
}]
}, {
code: "({ '-a': 0, b: 0 })",
options: ["consistent-as-needed"],
errors: [{
message: "Inconsistently quoted property `b` found.", type: "ObjectExpression"
}]
}, {
code: "({ 'a': 0, 'b': 0 })",
options: ["consistent-as-needed"],
errors: [{
message: "Properties shouldn't be quoted as all quotes are redundant.", type: "ObjectExpression"
}]
}, {
code: "({ 'true': 0, 'null': 0 })",
options: ["consistent-as-needed"],
errors: [{
message: "Properties shouldn't be quoted as all quotes are redundant.", type: "ObjectExpression"
}]
}]
});

0 comments on commit 6af4b35

Please sign in to comment.