Skip to content

Commit

Permalink
Update: auto-fix for comma-style (fixes #6941) (#6957)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gyandeeps @mysticatea
gyandeeps authored and mysticatea committed Sep 8, 2016
1 parent 645dda5 commit 7502eed
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 10 deletions.
2 changes: 2 additions & 0 deletions docs/rules/comma-style.md
View file
@@ -1,5 +1,7 @@
# Comma style (comma-style)

(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixes problems reported by this rule.

The Comma Style rule enforces styles for comma-separated lists. There are two comma styles primarily used in JavaScript:

* The standard style, in which commas are placed at the end of the current line
Expand Down
77 changes: 67 additions & 10 deletions lib/rules/comma-style.js
View file
Expand Up @@ -18,7 +18,7 @@ module.exports = {
category: "Stylistic Issues",
recommended: false
},

fixable: "code",
schema: [
{
enum: ["first", "last"]
Expand Down Expand Up @@ -61,6 +61,49 @@ module.exports = {
return !!token && (token.type === "Punctuator") && (token.value === ",");
}

/**
* Modified text based on the style
* @param {string} styleType Style type
* @param {string} text Source code text
* @returns {string} modified text
* @private
*/
function getReplacedText(styleType, text) {
switch (styleType) {
case "between":
return `,${text.replace("\n", "")}`;

case "first":
return `${text},`;

case "last":
return `,${text}`;

This comment has been minimized.

Sign in to view

Copy link
@mikesherov

mikesherov Sep 8, 2016

Contributor

Is this case ever hit?

This comment has been minimized.

Sign in to view

Copy link
@mysticatea

mysticatea Sep 8, 2016

Member

Never.

https://coveralls.io/builds/7794753/source?filename=lib%2Frules%2Fcomma-style.js#L83
default:
return "";
}
}

/**
* Determines the fixer function for a given style.
* @param {string} styleType comma style
* @param {ASTNode} previousItemToken The token to check.
* @param {ASTNode} commaToken The token to check.
* @param {ASTNode} currentItemToken The token to check.
* @returns {Function} Fixer function
* @private
*/
function getFixerFunction(styleType, previousItemToken, commaToken, currentItemToken) {
const text =
sourceCode.text.slice(previousItemToken.range[1], commaToken.range[0]) +
sourceCode.text.slice(commaToken.range[1], currentItemToken.range[0]);
const range = [previousItemToken.range[1], currentItemToken.range[0]];

return function(fixer) {
return fixer.replaceTextRange(range, getReplacedText(styleType, text));
};
}

/**
* Validates the spacing around single items in lists.
* @param {Token} previousItemToken The last token from the previous item.
Expand All @@ -82,21 +125,35 @@ module.exports = {
!astUtils.isTokenOnSameLine(previousItemToken, commaToken)) {

// lone comma
context.report(reportItem, {
line: commaToken.loc.end.line,
column: commaToken.loc.start.column
}, "Bad line breaking before and after ','.");
context.report({
node: reportItem,
loc: {
line: commaToken.loc.end.line,
column: commaToken.loc.start.column
},
message: "Bad line breaking before and after ','.",
fix: getFixerFunction("between", previousItemToken, commaToken, currentItemToken)
});

} else if (style === "first" && !astUtils.isTokenOnSameLine(commaToken, currentItemToken)) {

context.report(reportItem, "',' should be placed first.");
context.report({
node: reportItem,
message: "',' should be placed first.",
fix: getFixerFunction(style, previousItemToken, commaToken, currentItemToken)
});

} else if (style === "last" && astUtils.isTokenOnSameLine(commaToken, currentItemToken)) {

context.report(reportItem, {
line: commaToken.loc.end.line,
column: commaToken.loc.end.column
}, "',' should be placed last.");
context.report({
node: reportItem,
loc: {
line: commaToken.loc.end.line,
column: commaToken.loc.end.column
},
message: "',' should be placed last.",
fix: getFixerFunction(style, previousItemToken, commaToken, currentItemToken)
});
}
}

Expand Down
52 changes: 52 additions & 0 deletions tests/lib/rules/comma-style.js
View file
Expand Up @@ -86,50 +86,89 @@ ruleTester.run("comma-style", rule, {
],

invalid: [
{
code: "var foo = { a: 1. //comment \n, b: 2\n}",
output: "var foo = { a: 1., //comment \n b: 2\n}",
errors: [{
message: LAST_MSG,
type: "Property"
}]
},
{
code: "var foo = { a: 1. //comment \n //comment1 \n //comment2 \n, b: 2\n}",
output: "var foo = { a: 1., //comment \n //comment1 \n //comment2 \n b: 2\n}",
errors: [{
message: LAST_MSG,
type: "Property"
}]
},
{
code: "var foo = 1\n,\nbar = 2;",
output: "var foo = 1,\nbar = 2;",
errors: [{
message: BAD_LN_BRK_MSG,
type: "VariableDeclarator"
}]
},
{
code: "var foo = 1 //comment\n,\nbar = 2;",
output: "var foo = 1, //comment\nbar = 2;",
errors: [{
message: BAD_LN_BRK_MSG,
type: "VariableDeclarator"
}]
},
{
code: "var foo = 1 //comment\n, // comment 2\nbar = 2;",
output: "var foo = 1, //comment // comment 2\nbar = 2;",
errors: [{
message: BAD_LN_BRK_MSG,
type: "VariableDeclarator"
}]
},
{
code: "var foo = 1\n,bar = 2;",
output: "var foo = 1,\nbar = 2;",
errors: [{
message: LAST_MSG,
type: "VariableDeclarator"
}]
},
{
code: "f([1,2\n,3]);",
output: "f([1,2,\n3]);",
errors: [{
message: LAST_MSG,
type: "Literal"
}]
},
{
code: "f([1,2\n,]);",
output: "f([1,2,\n]);",
errors: [{
message: LAST_MSG,
type: "Punctuator"
}]
},
{
code: "f([,2\n,3]);",
output: "f([,2,\n3]);",
errors: [{
message: LAST_MSG,
type: "Literal"
}]
},
{
code: "var foo = ['apples'\n, 'oranges'];",
output: "var foo = ['apples',\n 'oranges'];",
errors: [{
message: LAST_MSG,
type: "Literal"
}]
},
{
code: "var foo = 1,\nbar = 2;",
output: "var foo = 1\n,bar = 2;",
options: ["first"],
errors: [{
message: FIRST_MSG,
Expand All @@ -138,6 +177,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "f([1,\n2,3]);",
output: "f([1\n,2,3]);",
options: ["first"],
errors: [{
message: FIRST_MSG,
Expand All @@ -146,6 +186,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var foo = ['apples', \n 'oranges'];",
output: "var foo = ['apples' \n ,'oranges'];",
options: ["first"],
errors: [{
message: FIRST_MSG,
Expand All @@ -154,6 +195,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var foo = {'a': 1, \n 'b': 2\n ,'c': 3};",
output: "var foo = {'a': 1 \n ,'b': 2\n ,'c': 3};",
options: ["first"],
errors: [{
message: FIRST_MSG,
Expand All @@ -162,6 +204,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var foo = {'a': 1, \n 'b': 2\n ,'c': 3};",
output: "var foo = {'a': 1 \n ,'b': 2\n ,'c': 3};",
options: ["first"],
errors: [{
message: FIRST_MSG,
Expand All @@ -170,6 +213,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var a = 'a',\no = 'o',\narr = [1,\n2];",
output: "var a = 'a',\no = 'o',\narr = [1\n,2];",
options: ["first", {exceptions: {VariableDeclaration: true}}],
errors: [{
message: FIRST_MSG,
Expand All @@ -178,6 +222,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var a = 'a',\nobj = {a: 'a',\nb: 'b'};",
output: "var a = 'a',\nobj = {a: 'a'\n,b: 'b'};",
options: ["first", {exceptions: {VariableDeclaration: true}}],
errors: [{
message: FIRST_MSG,
Expand All @@ -186,6 +231,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var a = 'a',\nobj = {a: 'a',\nb: 'b'};",
output: "var a = 'a'\n,obj = {a: 'a',\nb: 'b'};",
options: ["first", {exceptions: {ObjectExpression: true}}],
errors: [{
message: FIRST_MSG,
Expand All @@ -194,6 +240,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var a = 'a',\narr = [1,\n2];",
output: "var a = 'a'\n,arr = [1,\n2];",
options: ["first", {exceptions: {ArrayExpression: true}}],
errors: [{
message: FIRST_MSG,
Expand All @@ -202,6 +249,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var ar =[1,\n{a: 'a',\nb: 'b'}];",
output: "var ar =[1,\n{a: 'a'\n,b: 'b'}];",
options: ["first", {exceptions: {ArrayExpression: true}}],
errors: [{
message: FIRST_MSG,
Expand All @@ -210,6 +258,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var ar =[1,\n{a: 'a',\nb: 'b'}];",
output: "var ar =[1\n,{a: 'a',\nb: 'b'}];",
options: ["first", {exceptions: {ObjectExpression: true}}],
errors: [{
message: FIRST_MSG,
Expand All @@ -218,6 +267,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var ar ={fst:1,\nsnd: [1,\n2]};",
output: "var ar ={fst:1,\nsnd: [1\n,2]};",
options: ["first", {exceptions: {ObjectExpression: true}}],
errors: [{
message: FIRST_MSG,
Expand All @@ -226,6 +276,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var ar ={fst:1,\nsnd: [1,\n2]};",
output: "var ar ={fst:1\n,snd: [1,\n2]};",
options: ["first", {exceptions: {ArrayExpression: true}}],
errors: [{
message: FIRST_MSG,
Expand All @@ -234,6 +285,7 @@ ruleTester.run("comma-style", rule, {
},
{
code: "var foo = [\n(bar\n)\n,\nbaz\n];",
output: "var foo = [\n(bar\n),\nbaz\n];",
errors: [{
message: BAD_LN_BRK_MSG,
type: "Identifier"
Expand Down

0 comments on commit 7502eed

Please sign in to comment.