Skip to content

Commit

Permalink
Merge pull request #1180 from fatfisz/fix-newline-with-object-literals
Browse files Browse the repository at this point in the history 
Fix the newline with object literals bug
  • Loading branch information
@ljharb
ljharb authored May 9, 2017
2 parents 3743e6d + 1534bdb commit f111e45
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 40 deletions.
54 changes: 15 additions & 39 deletions lib/rules/jsx-curly-spacing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ module.exports = {

create: function(context) {

var DEFAULT_SPACING = SPACING.never;
var DEFAULT_ALLOW_MULTILINE = true;

var sourceCode = context.getSourceCode();
var spaced = context.options[0] === SPACING.always;
var baseSpacing = context.options[0] || DEFAULT_SPACING;
var config = context.options[1] || {};
var multiline = has(config, 'allowMultiline') ? config.allowMultiline : DEFAULT_ALLOW_MULTILINE;
var spacing = config.spacing || {};
var defaultSpacing = spaced ? SPACING.always : SPACING.never;
var objectLiteralSpacing = spacing.objectLiterals || (spaced ? SPACING.always : SPACING.never);
var spacingConfig = config.spacing || {};
var objectLiteralSpacing = spacingConfig.objectLiterals || baseSpacing;

// --------------------------------------------------------------------------
// Helpers
Expand All @@ -84,14 +84,14 @@ module.exports = {
* @param {Token} token - The token to use for the report.
* @returns {void}
*/
function reportNoBeginningNewline(node, token) {
function reportNoBeginningNewline(node, token, spacing) {
context.report({
node: node,
loc: token.loc.start,
message: `There should be no newline after '${token.value}'`,
fix: function(fixer) {
var nextToken = sourceCode.getTokenAfter(token);
return fixer.replaceTextRange([token.range[1], nextToken.range[0]], spaced ? ' ' : '');
return fixer.replaceTextRange([token.range[1], nextToken.range[0]], spacing === SPACING.always ? ' ' : '');
}
});
}
Expand All @@ -102,14 +102,14 @@ module.exports = {
* @param {Token} token - The token to use for the report.
* @returns {void}
*/
function reportNoEndingNewline(node, token) {
function reportNoEndingNewline(node, token, spacing) {
context.report({
node: node,
loc: token.loc.start,
message: `There should be no newline before '${token.value}'`,
fix: function(fixer) {
var previousToken = sourceCode.getTokenBefore(token);
return fixer.replaceTextRange([previousToken.range[1], token.range[0]], spaced ? ' ' : '');
return fixer.replaceTextRange([previousToken.range[1], token.range[0]], spacing === SPACING.always ? ' ' : '');
}
});
}
Expand Down Expand Up @@ -217,53 +217,29 @@ module.exports = {
}

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);
}
} 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) {
var spacing = isObjectLiteral ? objectLiteralSpacing : baseSpacing;
if (spacing === SPACING.always) {
if (!sourceCode.isSpaceBetweenTokens(first, second)) {
reportRequiredBeginningSpace(node, first);
} else if (!multiline && isMultiline(first, second)) {
reportNoBeginningNewline(node, first);
reportNoBeginningNewline(node, first, spacing);
}

if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
reportRequiredEndingSpace(node, last);
} else if (!multiline && isMultiline(penultimate, last)) {
reportNoEndingNewline(node, last);
reportNoEndingNewline(node, last, spacing);
}
} else if (defaultSpacing === SPACING.never) {
} else if (spacing === SPACING.never) {
if (isMultiline(first, second)) {
if (!multiline) {
reportNoBeginningNewline(node, first);
reportNoBeginningNewline(node, first, spacing);
}
} else if (sourceCode.isSpaceBetweenTokens(first, second)) {
reportNoBeginningSpace(node, first);
}
if (isMultiline(penultimate, last)) {
if (!multiline) {
reportNoEndingNewline(node, last);
reportNoEndingNewline(node, last, spacing);
}
} else if (sourceCode.isSpaceBetweenTokens(penultimate, last)) {
reportNoEndingSpace(node, last);
Expand Down
81 changes: 80 additions & 1 deletion tests/lib/rules/jsx-curly-spacing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,22 @@ var ruleTester = new RuleTester({parserOptions});
ruleTester.run('jsx-curly-spacing', rule, {
valid: [{
code: '<App foo={bar} />;'
}, {
code: [
'<App foo={',
'{ bar: true, baz: true }',
'} />;'
].join('\n')
}, {
code: '<App foo={bar} />;',
options: ['never']
}, {
code: [
'<App foo={',
'{ bar: true, baz: true }',
'} />;'
].join('\n'),
options: ['never', {spacing: {objectLiterals: 'never'}}]
}, {
code: '<App foo={ bar } />;',
options: ['always']
Expand All @@ -40,13 +53,20 @@ ruleTester.run('jsx-curly-spacing', rule, {
}, {
code: '<App foo={{ bar:baz }} />;',
options: ['never']
}, {
code: [
'<App foo={',
'{ bar: true, baz: true }',
'} />;'
].join('\n'),
options: ['never']
}, {
code: '<App foo={ {bar:baz} } />;',
options: ['always']
}, {
code: [
'<App foo={',
'bar',
'{ bar: true, baz: true }',
'} />;'
].join('\n'),
options: ['always']
Expand Down Expand Up @@ -92,6 +112,13 @@ ruleTester.run('jsx-curly-spacing', rule, {
'} />;'
].join('\n'),
options: ['always', {allowMultiline: true}]
}, {
code: [
'<App foo={',
'{ bar: true, baz: true }',
'} />;'
].join('\n'),
options: ['always', {spacing: {objectLiterals: 'never'}}]
}, {
code: '<App {...bar} />;'
}, {
Expand Down Expand Up @@ -182,6 +209,58 @@ ruleTester.run('jsx-curly-spacing', rule, {
}, {
message: 'There should be no space before \'}\''
}]
}, {
code: [
'<App foo={',
'{ bar: true, baz: true }',
'} />;'
].join('\n'),
output: '<App foo={{ bar: true, baz: true }} />;',
options: ['never', {allowMultiline: false, spacing: {objectLiterals: 'never'}}],
errors: [{
message: 'There should be no newline after \'{\''
}, {
message: 'There should be no newline before \'}\''
}]
}, {
code: [
'<App foo={',
'{ bar: true, baz: true }',
'} />;'
].join('\n'),
output: '<App foo={ { bar: true, baz: true } } />;',
options: ['never', {allowMultiline: false, spacing: {objectLiterals: 'always'}}],
errors: [{
message: 'There should be no newline after \'{\''
}, {
message: 'There should be no newline before \'}\''
}]
}, {
code: [
'<App foo={',
'{ bar: true, baz: true }',
'} />;'
].join('\n'),
output: '<App foo={{ bar: true, baz: true }} />;',
options: ['always', {allowMultiline: false, spacing: {objectLiterals: 'never'}}],
errors: [{
message: 'There should be no newline after \'{\''
}, {
message: 'There should be no newline before \'}\''
}]
}, {
code: [
'<App foo={',
'{ bar: true, baz: true }',
'} />;'
].join('\n'),
output: '<App foo={ { bar: true, baz: true } } />;',
options: ['always', {allowMultiline: false, spacing: {objectLiterals: 'always'}}],
errors: [{
message: 'There should be no newline after \'{\''
}, {
message: 'There should be no newline before \'}\''
}]
}, {
code: '<App foo={bar} />;',
output: '<App foo={ bar } />;',
Expand Down

0 comments on commit f111e45

Please sign in to comment.