From 10e692e088825f8d0f5e402016c54a53a1acfe99 Mon Sep 17 00:00:00 2001 From: "HIPP Edgar (PRESTA EXT)" Date: Fri, 23 Oct 2015 09:08:00 +0200 Subject: [PATCH] Update: fix option for comma-spacing (fixes #4232) --- docs/rules/README.md | 2 +- docs/rules/comma-spacing.md | 2 + lib/rules/comma-spacing.js | 108 ++++++++++++++++++++----------- tests/lib/rules/comma-spacing.js | 26 ++++++++ 4 files changed, 101 insertions(+), 37 deletions(-) diff --git a/docs/rules/README.md b/docs/rules/README.md index 4bc79fc0931..f0dbb2b0b5a 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -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 diff --git a/docs/rules/comma-spacing.md b/docs/rules/comma-spacing.md index 194fde0b02c..d1f1f5abc99 100644 --- a/docs/rules/comma-spacing.md +++ b/docs/rules/comma-spacing.md @@ -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 diff --git a/lib/rules/comma-spacing.js b/lib/rules/comma-spacing.js index 07b8624f3c4..f9474be6455 100644 --- a/lib/rules/comma-spacing.js +++ b/lib/rules/comma-spacing.js @@ -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 @@ -90,6 +54,7 @@ module.exports = function(context) { function isIndexInComment(index, comments) { var comment; + lastCommentIndex = 0; while (lastCommentIndex < comments.length) { @@ -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. diff --git a/tests/lib/rules/comma-spacing.js b/tests/lib/rules/comma-spacing.js index 10fd17f432e..ac939274f0c 100644 --- a/tests/lib/rules/comma-spacing.js +++ b/tests/lib/rules/comma-spacing.js @@ -125,6 +125,7 @@ ruleTester.run("comma-spacing", rule, { invalid: [ { code: "a(b,c)", + output: "a(b , c)", options: [{before: true, after: true}], errors: [ { @@ -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: [ { @@ -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 ','.", @@ -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 ','.", @@ -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 ','.", @@ -184,6 +189,7 @@ ruleTester.run("comma-spacing", rule, { }, { code: "var arr = [1 , ];", + output: "var arr = [1 ,];", options: [{ before: true, after: false }], errors: [ { @@ -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 ','.", @@ -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 ','.", @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -336,6 +354,7 @@ ruleTester.run("comma-spacing", rule, { }, { code: "a ,b", + output: "a, b", options: [{before: false, after: true}], errors: [ { @@ -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: [ { @@ -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: [ @@ -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: [ @@ -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: [ @@ -409,6 +432,7 @@ ruleTester.run("comma-spacing", rule, { }, { code: "{foo(1 ,2)}", + output: "{foo(1, 2)}", ecmaFeatures: { jsx: true }, errors: [ { @@ -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 ','.", @@ -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 ','.",