Skip to content

Commit f111e45

Browse files
ljharbljharb
authored
Merge pull request #1180 from fatfisz/fix-newline-with-object-literals
Fix the newline with object literals bug
2 parents 3743e6d + 1534bdb commit f111e45

File tree

2 files changed

+95
-40
lines changed

2 files changed

+95
-40
lines changed

‎lib/rules/jsx-curly-spacing.js

+15-39
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,15 @@ module.exports = {
5454

5555
create: function(context) {
5656

57+
var DEFAULT_SPACING = SPACING.never;
5758
var DEFAULT_ALLOW_MULTILINE = true;
5859

5960
var sourceCode = context.getSourceCode();
60-
var spaced = context.options[0] === SPACING.always;
61+
var baseSpacing = context.options[0] || DEFAULT_SPACING;
6162
var config = context.options[1] || {};
6263
var multiline = has(config, 'allowMultiline') ? config.allowMultiline : DEFAULT_ALLOW_MULTILINE;
63-
var spacing = config.spacing || {};
64-
var defaultSpacing = spaced ? SPACING.always : SPACING.never;
65-
var objectLiteralSpacing = spacing.objectLiterals || (spaced ? SPACING.always : SPACING.never);
64+
var spacingConfig = config.spacing || {};
65+
var objectLiteralSpacing = spacingConfig.objectLiterals || baseSpacing;
6666

6767
// --------------------------------------------------------------------------
6868
// Helpers
@@ -84,14 +84,14 @@ module.exports = {
8484
* @param {Token} token - The token to use for the report.
8585
* @returns {void}
8686
*/
87-
function reportNoBeginningNewline(node, token) {
87+
function reportNoBeginningNewline(node, token, spacing) {
8888
context.report({
8989
node: node,
9090
loc: token.loc.start,
9191
message: `There should be no newline after '${token.value}'`,
9292
fix: function(fixer) {
9393
var nextToken = sourceCode.getTokenAfter(token);
94-
return fixer.replaceTextRange([token.range[1], nextToken.range[0]], spaced ? ' ' : '');
94+
return fixer.replaceTextRange([token.range[1], nextToken.range[0]], spacing === SPACING.always ? ' ' : '');
9595
}
9696
});
9797
}
@@ -102,14 +102,14 @@ module.exports = {
102102
* @param {Token} token - The token to use for the report.
103103
* @returns {void}
104104
*/
105-
function reportNoEndingNewline(node, token) {
105+
function reportNoEndingNewline(node, token, spacing) {
106106
context.report({
107107
node: node,
108108
loc: token.loc.start,
109109
message: `There should be no newline before '${token.value}'`,
110110
fix: function(fixer) {
111111
var previousToken = sourceCode.getTokenBefore(token);
112-
return fixer.replaceTextRange([previousToken.range[1], token.range[0]], spaced ? ' ' : '');
112+
return fixer.replaceTextRange([previousToken.range[1], token.range[0]], spacing === SPACING.always ? ' ' : '');
113113
}
114114
});
115115
}
@@ -217,53 +217,29 @@ module.exports = {
217217
}
218218

219219
var isObjectLiteral = first.value === second.value;
220-
if (isObjectLiteral) {
221-
if (objectLiteralSpacing === SPACING.never) {
222-
if (sourceCode.isSpaceBetweenTokens(first, second)) {
223-
reportNoBeginningSpace(node, first);
224-
} else if (!multiline && isMultiline(first, second)) {
225-
reportNoBeginningNewline(node, first);
226-
}
227-
if (sourceCode.isSpaceBetweenTokens(penultimate, last)) {
228-
reportNoEndingSpace(node, last);
229-
} else if (!multiline && isMultiline(penultimate, last)) {
230-
reportNoEndingNewline(node, last);
231-
}
232-
} else if (objectLiteralSpacing === SPACING.always) {
233-
if (!sourceCode.isSpaceBetweenTokens(first, second)) {
234-
reportRequiredBeginningSpace(node, first);
235-
} else if (!multiline && isMultiline(first, second)) {
236-
reportNoBeginningNewline(node, first);
237-
}
238-
if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
239-
reportRequiredEndingSpace(node, last);
240-
} else if (!multiline && isMultiline(penultimate, last)) {
241-
reportNoEndingNewline(node, last);
242-
}
243-
}
244-
} else if (defaultSpacing === SPACING.always) {
220+
var spacing = isObjectLiteral ? objectLiteralSpacing : baseSpacing;
221+
if (spacing === SPACING.always) {
245222
if (!sourceCode.isSpaceBetweenTokens(first, second)) {
246223
reportRequiredBeginningSpace(node, first);
247224
} else if (!multiline && isMultiline(first, second)) {
248-
reportNoBeginningNewline(node, first);
225+
reportNoBeginningNewline(node, first, spacing);
249226
}
250-
251227
if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
252228
reportRequiredEndingSpace(node, last);
253229
} else if (!multiline && isMultiline(penultimate, last)) {
254-
reportNoEndingNewline(node, last);
230+
reportNoEndingNewline(node, last, spacing);
255231
}
256-
} else if (defaultSpacing === SPACING.never) {
232+
} else if (spacing === SPACING.never) {
257233
if (isMultiline(first, second)) {
258234
if (!multiline) {
259-
reportNoBeginningNewline(node, first);
235+
reportNoBeginningNewline(node, first, spacing);
260236
}
261237
} else if (sourceCode.isSpaceBetweenTokens(first, second)) {
262238
reportNoBeginningSpace(node, first);
263239
}
264240
if (isMultiline(penultimate, last)) {
265241
if (!multiline) {
266-
reportNoEndingNewline(node, last);
242+
reportNoEndingNewline(node, last, spacing);
267243
}
268244
} else if (sourceCode.isSpaceBetweenTokens(penultimate, last)) {
269245
reportNoEndingSpace(node, last);

‎tests/lib/rules/jsx-curly-spacing.js

+80-1
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,22 @@ var ruleTester = new RuleTester({parserOptions});
2828
ruleTester.run('jsx-curly-spacing', rule, {
2929
valid: [{
3030
code: '<App foo={bar} />;'
31+
}, {
32+
code: [
33+
'<App foo={',
34+
'{ bar: true, baz: true }',
35+
'} />;'
36+
].join('\n')
3137
}, {
3238
code: '<App foo={bar} />;',
3339
options: ['never']
40+
}, {
41+
code: [
42+
'<App foo={',
43+
'{ bar: true, baz: true }',
44+
'} />;'
45+
].join('\n'),
46+
options: ['never', {spacing: {objectLiterals: 'never'}}]
3447
}, {
3548
code: '<App foo={ bar } />;',
3649
options: ['always']
@@ -40,13 +53,20 @@ ruleTester.run('jsx-curly-spacing', rule, {
4053
}, {
4154
code: '<App foo={{ bar:baz }} />;',
4255
options: ['never']
56+
}, {
57+
code: [
58+
'<App foo={',
59+
'{ bar: true, baz: true }',
60+
'} />;'
61+
].join('\n'),
62+
options: ['never']
4363
}, {
4464
code: '<App foo={ {bar:baz} } />;',
4565
options: ['always']
4666
}, {
4767
code: [
4868
'<App foo={',
49-
'bar',
69+
'{ bar: true, baz: true }',
5070
'} />;'
5171
].join('\n'),
5272
options: ['always']
@@ -92,6 +112,13 @@ ruleTester.run('jsx-curly-spacing', rule, {
92112
'} />;'
93113
].join('\n'),
94114
options: ['always', {allowMultiline: true}]
115+
}, {
116+
code: [
117+
'<App foo={',
118+
'{ bar: true, baz: true }',
119+
'} />;'
120+
].join('\n'),
121+
options: ['always', {spacing: {objectLiterals: 'never'}}]
95122
}, {
96123
code: '<App {...bar} />;'
97124
}, {
@@ -182,6 +209,58 @@ ruleTester.run('jsx-curly-spacing', rule, {
182209
}, {
183210
message: 'There should be no space before \'}\''
184211
}]
212+
}, {
213+
code: [
214+
'<App foo={',
215+
'{ bar: true, baz: true }',
216+
'} />;'
217+
].join('\n'),
218+
output: '<App foo={{ bar: true, baz: true }} />;',
219+
options: ['never', {allowMultiline: false, spacing: {objectLiterals: 'never'}}],
220+
errors: [{
221+
message: 'There should be no newline after \'{\''
222+
}, {
223+
message: 'There should be no newline before \'}\''
224+
}]
225+
}, {
226+
code: [
227+
'<App foo={',
228+
'{ bar: true, baz: true }',
229+
'} />;'
230+
].join('\n'),
231+
output: '<App foo={ { bar: true, baz: true } } />;',
232+
options: ['never', {allowMultiline: false, spacing: {objectLiterals: 'always'}}],
233+
errors: [{
234+
message: 'There should be no newline after \'{\''
235+
}, {
236+
message: 'There should be no newline before \'}\''
237+
}]
238+
}, {
239+
code: [
240+
'<App foo={',
241+
'{ bar: true, baz: true }',
242+
'} />;'
243+
].join('\n'),
244+
output: '<App foo={{ bar: true, baz: true }} />;',
245+
options: ['always', {allowMultiline: false, spacing: {objectLiterals: 'never'}}],
246+
errors: [{
247+
message: 'There should be no newline after \'{\''
248+
}, {
249+
message: 'There should be no newline before \'}\''
250+
}]
251+
}, {
252+
code: [
253+
'<App foo={',
254+
'{ bar: true, baz: true }',
255+
'} />;'
256+
].join('\n'),
257+
output: '<App foo={ { bar: true, baz: true } } />;',
258+
options: ['always', {allowMultiline: false, spacing: {objectLiterals: 'always'}}],
259+
errors: [{
260+
message: 'There should be no newline after \'{\''
261+
}, {
262+
message: 'There should be no newline before \'}\''
263+
}]
185264
}, {
186265
code: '<App foo={bar} />;',
187266
output: '<App foo={ bar } />;',

0 commit comments

Comments
 (0)