Skip to content

Commit

Permalink
Fix jsx-curly-spacing alternative option
Browse files Browse the repository at this point in the history
  • Loading branch information
@ljharb @yannickcr
ljharb authored and yannickcr committed Jun 15, 2016
1 parent faaa823 commit 7586915
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 69 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
"react/jsx-uses-react": "error",
"react/jsx-uses-vars": "error",
}
```
```

# List of supported rules

Expand Down
22 changes: 15 additions & 7 deletions docs/rules/jsx-curly-spacing.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,26 +107,34 @@ The following patterns are not warnings:
<Hello name={ {firstname: 'John', lastname: 'Doe'} } />;
```

#### Alternative
#### Granular spacing controls

When setting the `alternative` option to `true` you must collapse the curly braces:
You can specify an additional `spacing` property that is an object with the following possible values:

```json
"jsx-curly-spacing": [2, "always", {"alternative": true}]
"jsx-curly-spacing": [2, "always", {"spacing": {
"objectLiterals": "never"
}}]
```

When `"always"` is used and `alternative` is `true`, the following pattern is not warnings:
* `objectLiterals`: This controls different spacing requirements when the value inside the jsx curly braces is an object literal.

All spacing options accept either the string `"always"` or the string `"never"`. Note that the default value for all "spacing" options matches the first "always"/"never" option provided.

When `"always"` is used but `objectLiterals` is `"never"`, the following pattern is not considered a warning:

```js
<App foo={{ bar: true, baz: true }} />;
<App blah={ 3 } foo={{ bar: true, baz: true }} />;
```

When `"always"` is used and `alternative` is `true`, the following pattern is considered warnings:
When `"never"` is used and `objectLiterals` is `"always"`, the following pattern is not considered a warning:

```js
<App foo={ {bar: true, baz: true} } />;
<App blah={3} foo={ {bar: true, baz: true} } />;
```

Please note that spacing of the object literal curly braces themselves is controlled by the built-in [`object-curly-spacing`](http://eslint.org/docs/rules/object-curly-spacing) rule.

## When Not To Use It

You can turn this rule off if you are not concerned with the consistency around the spacing inside of JSX attributes.
97 changes: 56 additions & 41 deletions lib/rules/jsx-curly-spacing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,20 @@
// Rule Definition
// ------------------------------------------------------------------------------

var SPACING = {
always: 'always',
never: 'never'
};
var SPACING_VALUES = [SPACING.always, SPACING.never];

module.exports = function(context) {

var sourceCode = context.getSourceCode();
var spaced = context.options[0] === 'always';
var spaced = context.options[0] === SPACING.always;
var multiline = context.options[1] ? context.options[1].allowMultiline : true;
var alternative = context.options[1] ? context.options[1].alternative : false;
var spacing = context.options[1] ? context.options[1].spacing || {} : {};
var defaultSpacing = spaced ? SPACING.always : SPACING.never;
var objectLiteralSpacing = spacing.objectLiterals || (spaced ? SPACING.always : SPACING.never);

// --------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -151,57 +159,59 @@ module.exports = function(context) {
var last = sourceCode.getLastToken(node);
var second = context.getTokenAfter(first);
var penultimate = sourceCode.getTokenBefore(last);
var third = context.getTokenAfter(second);
var antepenultimate = sourceCode.getTokenBefore(penultimate);

if (spaced) {
if (!alternative) {
if (!sourceCode.isSpaceBetweenTokens(first, second)) {
reportRequiredBeginningSpace(node, first);
} else if (!multiline && isMultiline(first, second)) {
reportNoBeginningNewline(node, first);
}

if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
reportRequiredEndingSpace(node, last);
} else if (!multiline && isMultiline(penultimate, last)) {
reportNoEndingNewline(node, last);
}

// Object literal
} else if (first.value === second.value) {
var isObjectLiteral = first.value === second.value;
if (isObjectLiteral) {
if (objectLiteralSpacing === SPACING.never) {
if (sourceCode.isSpaceBetweenTokens(first, second)) {
reportNoBeginningSpace(node, first);
} else if (!multiline && isMultiline(first, second)) {
reportNoBeginningNewline(node, first);
}
if (sourceCode.isSpaceBetweenTokens(penultimate, last)) {
reportNoEndingSpace(node, last);
} else if (!multiline && isMultiline(penultimate, last)) {
reportNoEndingNewline(node, last);
}
if (!sourceCode.isSpaceBetweenTokens(second, third)) {
reportRequiredBeginningSpace(node, second);
}
if (!sourceCode.isSpaceBetweenTokens(antepenultimate, penultimate)) {
reportRequiredEndingSpace(node, penultimate);
}

} else {
} else if (objectLiteralSpacing === SPACING.always) {
if (!sourceCode.isSpaceBetweenTokens(first, second)) {
reportRequiredBeginningSpace(node, first);
} else if (!multiline && isMultiline(first, second)) {
reportNoBeginningNewline(node, first);
}
if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
reportRequiredEndingSpace(node, last);
} else if (!multiline && isMultiline(penultimate, last)) {
reportNoEndingNewline(node, last);
}
}
} else if (defaultSpacing === SPACING.always) {
if (!sourceCode.isSpaceBetweenTokens(first, second)) {
reportRequiredBeginningSpace(node, first);
} else if (!multiline && isMultiline(first, second)) {
reportNoBeginningNewline(node, first);
}

return;
}

// "never" setting if we get here.
if (sourceCode.isSpaceBetweenTokens(first, second) && !(multiline && isMultiline(first, second))) {
reportNoBeginningSpace(node, first);
}

if (sourceCode.isSpaceBetweenTokens(penultimate, last) && !(multiline && isMultiline(penultimate, last))) {
reportNoEndingSpace(node, last);
if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
reportRequiredEndingSpace(node, last);
} else if (!multiline && isMultiline(penultimate, last)) {
reportNoEndingNewline(node, last);
}
} else if (defaultSpacing === SPACING.never) {
if (isMultiline(first, second)) {
if (!multiline) {
reportNoBeginningNewline(node, first);
}
} else if (sourceCode.isSpaceBetweenTokens(first, second)) {
reportNoBeginningSpace(node, first);
}
if (isMultiline(penultimate, last)) {
if (!multiline) {
reportNoEndingNewline(node, last);
}
} else if (sourceCode.isSpaceBetweenTokens(penultimate, last)) {
reportNoEndingSpace(node, last);
}
}
}

Expand All @@ -216,15 +226,20 @@ module.exports = function(context) {
};

module.exports.schema = [{
enum: ['always', 'never']
enum: SPACING_VALUES
}, {
type: 'object',
properties: {
allowMultiline: {
type: 'boolean'
},
alternative: {
type: 'boolean'
spacing: {
type: 'object',
properties: {
objectLiterals: {
enum: SPACING_VALUES
}
}
}
},
additionalProperties: false
Expand Down
54 changes: 34 additions & 20 deletions tests/lib/rules/jsx-curly-spacing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,19 @@ ruleTester.run('jsx-curly-spacing', rule, {
parserOptions: parserOptions
}, {
code: '<App foo={ bar } />;',
options: ['always', {alternative: true}],
options: ['always', {spacing: {}}],
parserOptions: parserOptions
}, {
code: '<App foo={{ bar: true, baz: true }} />;',
options: ['always', {alternative: true}],
options: ['always', {spacing: {objectLiterals: 'never'}}],
parserOptions: parserOptions
}, {
code: [
'<App foo={',
'bar',
'} />;'
].join('\n'),
options: ['always', {alternative: true}],
options: ['always', {allowMultiline: true}],
parserOptions: parserOptions
}, {
code: '<App {...bar} />;',
Expand Down Expand Up @@ -178,6 +178,10 @@ ruleTester.run('jsx-curly-spacing', rule, {
code: '<App foo={bar/* comment */} {...baz/* comment */} />;',
options: ['never'],
parserOptions: parserOptions
}, {
code: '<App foo={3} bar={ {a: 2} } />',
options: ['never', {spacing: {objectLiterals: 'always'}}],
parserOptions: parserOptions
}],

invalid: [{
Expand Down Expand Up @@ -261,9 +265,9 @@ ruleTester.run('jsx-curly-spacing', rule, {
output: '<App foo={bar} />;',
options: ['never', {allowMultiline: false}],
errors: [{
message: 'There should be no space after \'{\''
message: 'There should be no newline after \'{\''
}, {
message: 'There should be no space before \'}\''
message: 'There should be no newline before \'}\''
}],
parserOptions: parserOptions
}, {
Expand All @@ -283,7 +287,7 @@ ruleTester.run('jsx-curly-spacing', rule, {
}, {
code: '<App foo={bar} />;',
output: '<App foo={ bar } />;',
options: ['always', {alternative: true}],
options: ['always', {spacing: {}}],
errors: [{
message: 'A space is required after \'{\''
}, {
Expand All @@ -293,29 +297,25 @@ ruleTester.run('jsx-curly-spacing', rule, {
}, {
code: '<App foo={ bar} />;',
output: '<App foo={ bar } />;',
options: ['always', {alternative: true}],
options: ['always', {spacing: {}}],
errors: [{
message: 'A space is required before \'}\''
}],
parserOptions: parserOptions
}, {
code: '<App foo={bar } />;',
output: '<App foo={ bar } />;',
options: ['always', {alternative: true}],
options: ['always', {spacing: {}}],
errors: [{
message: 'A space is required after \'{\''
}],
parserOptions: parserOptions
}, {
code: '<App foo={ {bar: true, baz: true} } />;',
output: '<App foo={{ bar: true, baz: true }} />;',
options: ['always', {alternative: true}],
output: '<App foo={{bar: true, baz: true}} />;',
options: ['always', {spacing: {objectLiterals: 'never'}}],
errors: [{
message: 'There should be no space after \'{\''
}, {
message: 'A space is required after \'{\''
}, {
message: 'A space is required before \'}\''
}, {
message: 'There should be no space before \'}\''
}],
Expand Down Expand Up @@ -401,9 +401,9 @@ ruleTester.run('jsx-curly-spacing', rule, {
output: '<App {...bar} />;',
options: ['never', {allowMultiline: false}],
errors: [{
message: 'There should be no space after \'{\''
message: 'There should be no newline after \'{\''
}, {
message: 'There should be no space before \'}\''
message: 'There should be no newline before \'}\''
}],
parserOptions: parserOptions
}, {
Expand Down Expand Up @@ -527,13 +527,13 @@ ruleTester.run('jsx-curly-spacing', rule, {
output: '<App foo={bar} {...baz} />;',
options: ['never', {allowMultiline: false}],
errors: [{
message: 'There should be no space after \'{\''
message: 'There should be no newline after \'{\''
}, {
message: 'There should be no space before \'}\''
message: 'There should be no newline before \'}\''
}, {
message: 'There should be no space after \'{\''
message: 'There should be no newline after \'{\''
}, {
message: 'There should be no space before \'}\''
message: 'There should be no newline before \'}\''
}],
parserOptions: parserOptions
}, {
Expand All @@ -556,5 +556,19 @@ ruleTester.run('jsx-curly-spacing', rule, {
message: 'There should be no newline before \'}\''
}],
parserOptions: parserOptions
}, {
code: '<App foo={ 3 } bar={{a: 2}} />',
output: '<App foo={3} bar={ {a: 2} } />',
options: ['never', {spacing: {objectLiterals: 'always'}}],
errors: [{
message: 'There should be no space after \'{\''
}, {
message: 'There should be no space before \'}\''
}, {
message: 'A space is required after \'{\''
}, {
message: 'A space is required before \'}\''
}],
parserOptions: parserOptions
}]
});

0 comments on commit 7586915

Please sign in to comment.