Skip to content

Commit

Permalink
Fix: enforce consistent parens spacing (fixes #10905)
Browse files Browse the repository at this point in the history 
Updates space-in-parens linting error messages to be more specific
Refactors and removes superfluous code
  • Loading branch information
Che Fisher committed Aug 12, 2019
1 parent a675c89 commit 91b470b
Show file tree
Hide file tree
Showing 3 changed files with 322 additions and 176 deletions.
46 changes: 38 additions & 8 deletions docs/rules/space-in-parens.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var x = (1 + 2) * 3;

## Rule Details

This rule will enforce consistency of spacing directly inside of parentheses, by disallowing or requiring one or more spaces to the right of `(` and to the left of `)`. In either case, `()` will still be allowed.
This rule will enforce consistent spacing directly inside of parentheses, by disallowing or requiring one or more spaces to the right of `(` and to the left of `)`.

## Options

Expand Down Expand Up @@ -66,6 +66,8 @@ foo( 'bar');
foo('bar' );
foo('bar');

foo();

var foo = (1 + 2) * 3;
(function () { return 'bar'; }());
```
Expand All @@ -75,18 +77,20 @@ Examples of **correct** code for this rule with the `"always"` option:
```js
/*eslint space-in-parens: ["error", "always"]*/

foo();

foo( 'bar' );

foo( );

var foo = ( 1 + 2 ) * 3;
( function () { return 'bar'; }() );
( function ( ) { return 'bar'; }( ) );
```

### Exceptions

An object literal may be used as a third array item to specify exceptions, with the key `"exceptions"` and an array as the value. These exceptions work in the context of the first option. That is, if `"always"` is set to enforce spacing, then any "exception" will *disallow* spacing. Conversely, if `"never"` is set to disallow spacing, then any "exception" will *enforce* spacing.

Note that this rule only enforces spacing within parentheses; it does not check spacing within curly or square brackets, but will enforce or disallow spacing of those types of brackets if and only if they are adjacent to an opening or closing parenthesis.

The following exceptions are available: `["{}", "[]", "()", "empty"]`.

Examples of **incorrect** code for this rule with the `"never", { "exceptions": ["{}"] }` option:
Expand Down Expand Up @@ -175,8 +179,8 @@ Examples of **correct** code for this rule with the `"never", { "exceptions": ["
```js
/*eslint space-in-parens: ["error", "never", { "exceptions": ["()"] }]*/

foo( (1 + 2) );
foo( (1 + 2), 1);
foo( ( 1 + 2 ) );
foo( ( 1 + 2 ), 1);
```

Examples of **incorrect** code for this rule with the `"always", { "exceptions": ["()"] }]` option:
Expand All @@ -193,8 +197,8 @@ Examples of **correct** code for this rule with the `"always", { "exceptions": [
```js
/*eslint space-in-parens: ["error", "always", { "exceptions": ["()"] }]*/

foo(( 1 + 2 ));
foo(( 1 + 2 ), 1 );
foo((1 + 2));
foo((1 + 2), 1);
```

The `"empty"` exception concerns empty parentheses, and works the same way as the other exceptions, inverting the first option.
Expand Down Expand Up @@ -253,6 +257,32 @@ baz( 1, [1,2]);
foo({bar: 'baz'}, [1, 2]);
```

### Allow Empty Method Calls

A common enough requirement is enforce spacing within parentheses when object or array literals are used but still allow empty method calls. Here is one way to achieve that functionality.

Examples of **incorrect** code for this rule with the `"always", { "exceptions": ["empty", "()"] }]` option:

```js
/*eslint space-in-parens: ["error", "always", { "exceptions": ["empty", "()"] }]*/

bar( x, {bar:'baz'} );
baz([1,2], 1);
foo({bar: 'baz'}, [1, 2]);
foo( )
```

Examples of **correct** code for this rule with the `"always", { "exceptions": ["empty", "()"] }]` option:

```js
/*eslint space-in-parens: ["error", "always", { "exceptions": ["empty", "()"] }]*/

bar(x, {bar:'baz'} );
baz( [1,2], 1);
foo( {bar: 'baz'}, [1, 2] );
foo()
```

## When Not To Use It

You can turn this rule off if you are not concerned with the consistency of spacing between parentheses.
Expand Down
131 changes: 72 additions & 59 deletions lib/rules/space-in-parens.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* @fileoverview Disallows or enforces spaces inside of parentheses.
* @author Jonathan Rajavuori
* @author Jonathan Rajavuori and Che Fisher
*/
"use strict";

Expand Down Expand Up @@ -45,18 +45,21 @@ module.exports = {

create(context) {

const MISSING_SPACE_MESSAGE = "There must be a space inside this paren.",
REJECTED_SPACE_MESSAGE = "There should be no spaces inside this paren.",
const MISSING_OPENING_SPACE_MESSAGE = "There must be a space after this paren",
MISSING_CLOSING_SPACE_MESSAGE = "There must be a space before this paren",
REJECTED_OPENING_SPACE_MESSAGE = "There should be no space after this paren",
REJECTED_CLOSING_SPACE_MESSAGE = "There should be no space before this paren",
ALWAYS = context.options[0] === "always",
exceptionsArrayOptions = (context.options[1] && context.options[1].exceptions) || [],
options = {};

let exceptions;

if (exceptionsArrayOptions.length) {
options.braceException = exceptionsArrayOptions.indexOf("{}") !== -1;
options.bracketException = exceptionsArrayOptions.indexOf("[]") !== -1;
options.parenException = exceptionsArrayOptions.indexOf("()") !== -1;
options.empty = exceptionsArrayOptions.indexOf("empty") !== -1;
options.braceException = exceptionsArrayOptions.includes("{}");
options.bracketException = exceptionsArrayOptions.includes("[]");
options.parenException = exceptionsArrayOptions.includes("()");
options.empty = exceptionsArrayOptions.includes("empty");
}

/**
Expand Down Expand Up @@ -106,7 +109,7 @@ module.exports = {
* @returns {boolean} True if the token is one of the exceptions for the opener paren
*/
function isOpenerException(token) {
return token.type === "Punctuator" && exceptions.openers.indexOf(token.value) >= 0;
return exceptions.openers.includes(token.value);
}

/**
Expand All @@ -115,38 +118,30 @@ module.exports = {
* @returns {boolean} True if the token is one of the exceptions for the closer paren
*/
function isCloserException(token) {
return token.type === "Punctuator" && exceptions.closers.indexOf(token.value) >= 0;
return exceptions.closers.includes(token.value);
}

/**
* Determines if an opener paren should have a missing space after it
* @param {Object} left The paren token
* @param {Object} right The token after it
* @returns {boolean} True if the paren should have a space
* Determines if the first token is a Punctuator, returns second token if not
* @param {Object} first The first token (which has its type checked)
* @param {Object} second The second token
* @returns {Object} Token the first token is returned if it is a punctuator, else second
*/
function shouldOpenerHaveSpace(left, right) {
if (sourceCode.isSpaceBetweenTokens(left, right)) {
return false;
function checkPunctuator(first, second) {
if (first.type !== "Punctuator") {
return second;
}

if (ALWAYS) {
if (astUtils.isClosingParenToken(right)) {
return false;
}
return !isOpenerException(right);
}
return isOpenerException(right);

return first;
}

/**
* Determines if an closer paren should have a missing space after it
* @param {Object} left The token before the paren
* @param {Object} right The paren token
* @returns {boolean} True if the paren should have a space
* Determines if an opening paren has a space immediatley after it
* @param {Object} left The paren token
* @param {Object} right The token after it
* @returns {boolean} True if the opening paren is missing a required space
*/
function shouldCloserHaveSpace(left, right) {
if (astUtils.isOpeningParenToken(left)) {
function openerMissingSpace(left, right) {
if (!astUtils.isTokenOnSameLine(left, right)) {
return false;
}

Expand All @@ -155,24 +150,23 @@ module.exports = {
}

if (ALWAYS) {
return !isCloserException(left);
return !isOpenerException(checkPunctuator(right, left));
}
return isCloserException(left);

return isOpenerException(checkPunctuator(right, left));
}

/**
* Determines if an opener paren should not have an existing space after it
* Determines if an opening paren has a space immediately after it
* @param {Object} left The paren token
* @param {Object} right The token after it
* @returns {boolean} True if the paren should reject the space
* @returns {boolean} True if the opening paren has a disallowed space following it
*/
function shouldOpenerRejectSpace(left, right) {
if (right.type === "Line") {
function openerRejectsSpace(left, right) {
if (!astUtils.isTokenOnSameLine(left, right)) {
return false;
}

if (!astUtils.isTokenOnSameLine(left, right)) {
if (right.type === "Line") {
return false;
}

Expand All @@ -181,23 +175,35 @@ module.exports = {
}

if (ALWAYS) {
return isOpenerException(right);
return isOpenerException(checkPunctuator(right, left));
}
return !isOpenerException(right);

return !isOpenerException(checkPunctuator(right, left));
}

/**
* Determines if an closer paren should not have an existing space after it
* Determines if a closing paren has a space immediately before it
* @param {Object} left The token before the paren
* @param {Object} right The paren token
* @returns {boolean} True if the paren should reject the space
* @returns {boolean} True if the paren is missing the required space
*/
function shouldCloserRejectSpace(left, right) {
if (astUtils.isOpeningParenToken(left)) {
function closerMissingSpace(left, right) {
if (sourceCode.isSpaceBetweenTokens(left, right)) {
return false;
}

if (ALWAYS) {
return !isCloserException(checkPunctuator(left, right));
}
return isCloserException(checkPunctuator(left, right));
}

/**
* Determines if a closer paren has a space immediately before it
* @param {Object} left The token before the paren
* @param {Object} right The paren token
* @returns {boolean} True if the paren has a space before it
*/
function closerRejectsSpace(left, right) {
if (!astUtils.isTokenOnSameLine(left, right)) {
return false;
}
Expand All @@ -207,10 +213,9 @@ module.exports = {
}

if (ALWAYS) {
return isCloserException(left);
return isCloserException(checkPunctuator(left, right));
}
return !isCloserException(left);

return !isCloserException(checkPunctuator(left, right));
}

//--------------------------------------------------------------------------
Expand All @@ -226,44 +231,53 @@ module.exports = {
const prevToken = tokens[i - 1];
const nextToken = tokens[i + 1];

// if token is not an opening or closing paren token, do nothing
if (!astUtils.isOpeningParenToken(token) && !astUtils.isClosingParenToken(token)) {
return;
}

if (token.value === "(" && shouldOpenerHaveSpace(token, nextToken)) {
// if token is an opening paren and is not followed by a required space
if (token.value === "(" && openerMissingSpace(token, nextToken)) {
context.report({
node,
loc: token.loc.start,
message: MISSING_SPACE_MESSAGE,
message: MISSING_OPENING_SPACE_MESSAGE,
fix(fixer) {
return fixer.insertTextAfter(token, " ");
}
});
} else if (token.value === "(" && shouldOpenerRejectSpace(token, nextToken)) {
}

// if token is an opening paren and is followed by a disallowed space
if (token.value === "(" && openerRejectsSpace(token, nextToken)) {
context.report({
node,
loc: token.loc.start,
message: REJECTED_SPACE_MESSAGE,
message: REJECTED_OPENING_SPACE_MESSAGE,
fix(fixer) {
return fixer.removeRange([token.range[1], nextToken.range[0]]);
}
});
} else if (token.value === ")" && shouldCloserHaveSpace(prevToken, token)) {
}

// context.report(node, token.loc.start, MISSING_SPACE_MESSAGE);
// if token is a closing paren and is not preceded by a required space
if (token.value === ")" && closerMissingSpace(prevToken, token)) {
context.report({
node,
loc: token.loc.start,
message: MISSING_SPACE_MESSAGE,
message: MISSING_CLOSING_SPACE_MESSAGE,
fix(fixer) {
return fixer.insertTextBefore(token, " ");
}
});
} else if (token.value === ")" && shouldCloserRejectSpace(prevToken, token)) {
}

// if token is a closing paren and is preceded by a disallowed space
if (token.value === ")" && closerRejectsSpace(prevToken, token)) {
context.report({
node,
loc: token.loc.start,
message: REJECTED_SPACE_MESSAGE,
message: REJECTED_CLOSING_SPACE_MESSAGE,
fix(fixer) {
return fixer.removeRange([prevToken.range[1], token.range[0]]);
}
Expand All @@ -272,6 +286,5 @@ module.exports = {
});
}
};

}
};
Loading

0 comments on commit 91b470b

Please sign in to comment.