Skip to content

Commit

Permalink
Merge pull request #4233 from netei/issue4232
Browse files Browse the repository at this point in the history 
Add fix option to comma-spacing (fixes #4232)
  • Loading branch information
@nzakas
nzakas committed Oct 27, 2015
2 parents f163ee6 + 10e692e commit 34e8700
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 37 deletions.
2 changes: 1 addition & 1 deletion docs/rules/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ These rules are purely matters of style and are quite subjective.
* [block-spacing](block-spacing.md) - disallow or enforce spaces inside of single line blocks (fixable)
* [brace-style](brace-style.md) - enforce one true brace style
* [camelcase](camelcase.md) - require camel case names
* [comma-spacing](comma-spacing.md) - enforce spacing before and after comma
* [comma-spacing](comma-spacing.md) - enforce spacing before and after comma (fixable)
* [comma-style](comma-style.md) - enforce one true comma style
* [computed-property-spacing](computed-property-spacing.md) - require or disallow padding inside computed properties (fixable)
* [consistent-this](consistent-this.md) - enforce consistent naming when capturing the current execution context
Expand Down
2 changes: 2 additions & 0 deletions docs/rules/comma-spacing.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Enforces spacing around commas (comma-spacing)

**Fixable:** This rule is automatically fixable using the `--fix` flag on the command line.

Spacing around commas improve readability of a list of items. Although most of the style guidelines for languages prescribe adding a space after a comma and not before it, it is subjective to the preferences of a project.

```js
Expand Down
108 changes: 72 additions & 36 deletions lib/rules/comma-spacing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,42 +41,6 @@ module.exports = function(context) {
return !!token && (token.type === "Punctuator") && (token.value === ",");
}

/**
* Reports a spacing error with an appropriate message.
* @param {ASTNode} node The binary expression node to report.
* @param {string} dir Is the error "before" or "after" the comma?
* @returns {void}
* @private
*/
function report(node, dir) {
context.report(node, options[dir] ?
"A space is required " + dir + " ','." :
"There should be no space " + dir + " ','.");
}

/**
* Validates the spacing around a comma token.
* @param {Object} tokens - The tokens to be validated.
* @param {Token} tokens.comma The token representing the comma.
* @param {Token} [tokens.left] The last token before the comma.
* @param {Token} [tokens.right] The first token after the comma.
* @param {Token|ASTNode} reportItem The item to use when reporting an error.
* @returns {void}
* @private
*/
function validateCommaItemSpacing(tokens, reportItem) {
if (tokens.left && astUtils.isTokenOnSameLine(tokens.left, tokens.comma) &&
(options.before !== sourceCode.isSpaceBetweenTokens(tokens.left, tokens.comma))
) {
report(reportItem, "before");
}
if (tokens.right && astUtils.isTokenOnSameLine(tokens.comma, tokens.right) &&
(options.after !== sourceCode.isSpaceBetweenTokens(tokens.comma, tokens.right))
) {
report(reportItem, "after");
}
}

/**
* Determines if a given source index is in a comment or not by checking
* the index against the comment range. Since the check goes straight
Expand All @@ -90,6 +54,7 @@ module.exports = function(context) {
function isIndexInComment(index, comments) {

var comment;
lastCommentIndex = 0;

while (lastCommentIndex < comments.length) {

Expand All @@ -108,6 +73,77 @@ module.exports = function(context) {
return false;
}


/**
* Reports a spacing error with an appropriate message.
* @param {ASTNode} node The binary expression node to report.
* @param {string} dir Is the error "before" or "after" the comma?
* @param {ASTNode} otherNode The node at the left or right of `node`
* @returns {void}
* @private
*/
function report(node, dir, otherNode) {
context.report({
node: node,
fix: function(fixer) {
if (options[dir]) {
if (dir === "before") {
return fixer.insertTextBefore(node, " ");
} else {
return fixer.insertTextAfter(node, " ");
}
} else {
/*
* Comments handling
*/
var start, end;
var newText = "";

if (dir === "before") {
start = otherNode.range[1];
end = node.range[0];
} else {
start = node.range[1];
end = otherNode.range[0];
}

for (var i = start; i < end; i++) {
if (isIndexInComment(i, allComments)) {
newText += context.getSource()[i];
}
}
return fixer.replaceTextRange([start, end], newText);
}
},
message: options[dir] ?
"A space is required " + dir + " ','." :
"There should be no space " + dir + " ','."
});
}

/**
* Validates the spacing around a comma token.
* @param {Object} tokens - The tokens to be validated.
* @param {Token} tokens.comma The token representing the comma.
* @param {Token} [tokens.left] The last token before the comma.
* @param {Token} [tokens.right] The first token after the comma.
* @param {Token|ASTNode} reportItem The item to use when reporting an error.
* @returns {void}
* @private
*/
function validateCommaItemSpacing(tokens, reportItem) {
if (tokens.left && astUtils.isTokenOnSameLine(tokens.left, tokens.comma) &&
(options.before !== sourceCode.isSpaceBetweenTokens(tokens.left, tokens.comma))
) {
report(reportItem, "before", tokens.left);
}
if (tokens.right && astUtils.isTokenOnSameLine(tokens.comma, tokens.right) &&
(options.after !== sourceCode.isSpaceBetweenTokens(tokens.comma, tokens.right))
) {
report(reportItem, "after", tokens.right);
}
}

/**
* Adds null elements of the given ArrayExpression or ArrayPattern node to the ignore list.
* @param {ASTNode} node An ArrayExpression or ArrayPattern node.
Expand Down
26 changes: 26 additions & 0 deletions tests/lib/rules/comma-spacing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ ruleTester.run("comma-spacing", rule, {
invalid: [
{
code: "a(b,c)",
output: "a(b , c)",
options: [{before: true, after: true}],
errors: [
{
Expand All @@ -139,6 +140,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "new A(b,c)",
output: "new A(b , c)",
options: [{before: true, after: true}],
errors: [
{
Expand All @@ -153,6 +155,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var a = 1 ,b = 2;",
output: "var a = 1, b = 2;",
errors: [
{
message: "There should be no space before ','.",
Expand All @@ -166,6 +169,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1 , 2];",
output: "var arr = [1, 2];",
errors: [
{
message: "There should be no space before ','.",
Expand All @@ -175,6 +179,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1 , ];",
output: "var arr = [1, ];",
errors: [
{
message: "There should be no space before ','.",
Expand All @@ -184,6 +189,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1 , ];",
output: "var arr = [1 ,];",
options: [{ before: true, after: false }],
errors: [
{
Expand All @@ -194,6 +200,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1 ,2];",
output: "var arr = [1, 2];",
errors: [
{
message: "There should be no space before ','.",
Expand All @@ -207,6 +214,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [(1) , 2];",
output: "var arr = [(1), 2];",
errors: [
{
message: "There should be no space before ','.",
Expand All @@ -216,6 +224,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1, 2];",
output: "var arr = [1 ,2];",
options: [{before: true, after: false}],
errors: [
{
Expand All @@ -230,6 +239,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1\n , 2];",
output: "var arr = [1\n ,2];",
options: [{before: false, after: false}],
errors: [
{
Expand All @@ -240,6 +250,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1,\n 2];",
output: "var arr = [1 ,\n 2];",
options: [{before: true, after: false}],
errors: [
{
Expand All @@ -250,6 +261,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var obj = {'foo':\n'bar', 'baz':\n'qur'};",
output: "var obj = {'foo':\n'bar' ,'baz':\n'qur'};",
options: [{before: true, after: false}],
errors: [
{
Expand All @@ -264,6 +276,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var obj = {a: 1\n ,b: 2};",
output: "var obj = {a: 1\n , b: 2};",
options: [{before: false, after: true}],
errors: [
{
Expand All @@ -274,6 +287,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var obj = {a: 1 ,\n b: 2};",
output: "var obj = {a: 1,\n b: 2};",
options: [{before: false, after: false}],
errors: [
{
Expand All @@ -284,6 +298,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1 ,2];",
output: "var arr = [1 , 2];",
options: [{before: true, after: true}],
errors: [
{
Expand All @@ -294,6 +309,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1,2];",
output: "var arr = [1 , 2];",
options: [{before: true, after: true}],
errors: [
{
Expand All @@ -308,6 +324,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var obj = {'foo':\n'bar','baz':\n'qur'};",
output: "var obj = {'foo':\n'bar' , 'baz':\n'qur'};",
options: [{before: true, after: true}],
errors: [
{
Expand All @@ -322,6 +339,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var arr = [1 , 2];",
output: "var arr = [1,2];",
options: [{before: false, after: false}],
errors: [
{
Expand All @@ -336,6 +354,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "a ,b",
output: "a, b",
options: [{before: false, after: true}],
errors: [
{
Expand All @@ -350,6 +369,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "function foo(a,b){}",
output: "function foo(a , b){}",
options: [{before: true, after: true}],
errors: [
{
Expand All @@ -364,6 +384,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var foo = (a,b) => {}",
output: "var foo = (a , b) => {}",
ecmaFeatures: { arrowFunctions: true },
options: [{before: true, after: true}],
errors: [
Expand All @@ -379,6 +400,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "var foo = (a = 1,b) => {}",
output: "var foo = (a = 1 , b) => {}",
ecmaFeatures: { arrowFunctions: true, defaultParams: true },
options: [{before: true, after: true}],
errors: [
Expand All @@ -394,6 +416,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "function foo(a = 1 ,b = 2) {}",
output: "function foo(a = 1, b = 2) {}",
ecmaFeatures: { defaultParams: true },
options: [{before: false, after: true}],
errors: [
Expand All @@ -409,6 +432,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "<a>{foo(1 ,2)}</a>",
output: "<a>{foo(1, 2)}</a>",
ecmaFeatures: { jsx: true },
errors: [
{
Expand All @@ -423,6 +447,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "myfunc(404, true/* bla bla bla */ , 'hello');",
output: "myfunc(404, true/* bla bla bla */, 'hello');",
errors: [
{
message: "There should be no space before ','.",
Expand All @@ -432,6 +457,7 @@ ruleTester.run("comma-spacing", rule, {
},
{
code: "myfunc(404, true/* bla bla bla */ /* hi */, 'hello');",
output: "myfunc(404, true/* bla bla bla *//* hi */, 'hello');",
errors: [
{
message: "There should be no space before ','.",
Expand Down

0 comments on commit 34e8700

Please sign in to comment.