From 3a776382754c8a65e2ba9a0508f6fb3ef37ae40b Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sun, 4 Jul 2021 13:44:00 +0530 Subject: [PATCH 1/6] Update: change reporting location for `curly` rule --- lib/rules/curly.js | 128 ++++++++- tests/lib/rules/curly.js | 542 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 652 insertions(+), 18 deletions(-) diff --git a/lib/rules/curly.js b/lib/rules/curly.js index 92d31a6476e..8b4f1110147 100644 --- a/lib/rules/curly.js +++ b/lib/rules/curly.js @@ -10,6 +10,28 @@ const astUtils = require("./utils/ast-utils"); +//------------------------------------------------------------------------------ +// Helper +//------------------------------------------------------------------------------ + +const blockStartLoc = new Set(["for", "for-of", "do", "for-in"]); + +/** + * Returns location of given position with `end` position next to `start` position. + * @param {Position} position position whose very next needs to be created. + * @returns {Location} the new location. + */ +function createNextLocation(position) { + + return { + start: position, + end: { + line: position.line, + column: position.column + 1 + } + }; +} + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -309,6 +331,104 @@ module.exports = { hasUnsafeIf(statement) && isFollowedByElseKeyword(node); } + /** + * Missing curly brace reporting location examples: + * + * The following location should be reported + *This conversation was marked as resolved by anikethsaha + * + * if (foo) bar() + * ^^^ + * + * while(foo) bar(); + * ^^^ + * + * do foo() while(bar) + * ^^^ + * + * for(;;) bar(); + * ^^^ + * + * for (var foo of bar) console.log(foo) + * ^^^ + * + * for (var foo in bar) console.log(foo) + * ^^^ + * + * if (foo) {bar()} else bar(1); + * ^^^^ + * + * Unexpected curly brace reporting location examples: + * + * The following location should be reported + * + * if (true) foo(); else { baz(); } + * ^ + * + * for (;;) { foo(); } + * ^ + * + * while (bar) { foo(); } + * ^ + * + * do{foo();} while(bar); + * ^ + * + * for (var foo of bar) {console.log(foo)} + * ^ + * + * if (foo) bar() else {bar(1)} + * ^^^^ + * + * Get the location of missing or non missing curly brace. + * @param {ASTNode} node the node to check. + * @param {string} name name to check. + * @param {boolean} isMissing is curly brace missing. + * @returns {Object} Position object. + */ + function getReportLoc(node, name, isMissing) { + if (name === "else") { + if (!isMissing) { + const token = sourceCode.getTokenAfter(getElseKeyword(node)); + + return token.loc; + } + + return getElseKeyword(node).loc; + } + + if (blockStartLoc.has(name)) { + if (isMissing) { + const token = sourceCode.getTokenBefore(node.body); + + return { + start: token.loc.start, + end: node.body.loc.start + }; + } + + return createNextLocation(node.body.loc.start); + } + + if (name === "if") { + return isMissing + ? { + start: node.test.loc.end, + end: node.consequent.loc.start + } + : createNextLocation(node.consequent.loc.start); + } + + if (name === "while") { + return isMissing ? { + start: node.test.loc.end, + end: node.body.loc.start + } : createNextLocation(node.body.loc.start); + } + + return node.loc; + } + /** * Prepares to check the body of a node to see if it's a block statement. * @param {ASTNode} node The node to report if there's a problem. @@ -359,9 +479,11 @@ module.exports = { check() { if (this.expected !== null && this.expected !== this.actual) { if (this.expected) { + const loc = getReportLoc(node, name, true); + context.report({ node, - loc: (name !== "else" ? node : getElseKeyword(node)).loc.start, + loc, messageId: opts && opts.condition ? "missingCurlyAfterCondition" : "missingCurlyAfter", data: { name @@ -369,9 +491,11 @@ module.exports = { fix: fixer => fixer.replaceText(body, `{${sourceCode.getText(body)}}`) }); } else { + const loc = getReportLoc(node, name, false); + context.report({ node, - loc: (name !== "else" ? node : getElseKeyword(node)).loc.start, + loc, messageId: opts && opts.condition ? "unexpectedCurlyAfterCondition" : "unexpectedCurlyAfter", data: { name diff --git a/tests/lib/rules/curly.js b/tests/lib/rules/curly.js index 155eec994da..2ade8161d44 100644 --- a/tests/lib/rules/curly.js +++ b/tests/lib/rules/curly.js @@ -429,7 +429,26 @@ ruleTester.run("curly", rule, { { messageId: "missingCurlyAfterCondition", data: { name: "if" }, - type: "IfStatement" + type: "IfStatement", + line: 1, + column: 8, + endLine: 1, + endColumn: 10 + } + ] + }, + { + code: "if (foo) \n bar()", + output: "if (foo) \n {bar()}", + errors: [ + { + messageId: "missingCurlyAfterCondition", + data: { name: "if" }, + type: "IfStatement", + line: 1, + column: 8, + endLine: 2, + endColumn: 2 } ] }, @@ -462,7 +481,26 @@ ruleTester.run("curly", rule, { { messageId: "missingCurlyAfterCondition", data: { name: "while" }, - type: "WhileStatement" + type: "WhileStatement", + line: 1, + column: 11, + endLine: 1, + endColumn: 13 + } + ] + }, + { + code: "while (foo) \n bar()", + output: "while (foo) \n {bar()}", + errors: [ + { + messageId: "missingCurlyAfterCondition", + data: { name: "while" }, + type: "WhileStatement", + line: 1, + column: 11, + endLine: 2, + endColumn: 2 } ] }, @@ -473,7 +511,26 @@ ruleTester.run("curly", rule, { { messageId: "missingCurlyAfter", data: { name: "do" }, - type: "DoWhileStatement" + type: "DoWhileStatement", + line: 1, + column: 1, + endLine: 1, + endColumn: 4 + } + ] + }, + { + code: "do \n bar(); while (foo)", + output: "do \n {bar();} while (foo)", + errors: [ + { + messageId: "missingCurlyAfter", + data: { name: "do" }, + type: "DoWhileStatement", + line: 1, + column: 1, + endLine: 2, + endColumn: 2 } ] }, @@ -507,7 +564,93 @@ ruleTester.run("curly", rule, { { messageId: "missingCurlyAfter", data: { name: "for-of" }, - type: "ForOfStatement" + type: "ForOfStatement", + line: 1, + column: 20, + endLine: 1, + endColumn: 22 + } + ] + }, + { + code: "for (var foo of bar) \n console.log(foo)", + output: "for (var foo of bar) \n {console.log(foo)}", + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "missingCurlyAfter", + data: { name: "for-of" }, + type: "ForOfStatement", + line: 1, + column: 20, + endLine: 2, + endColumn: 2 + } + ] + }, + { + code: "for (a;;) console.log(foo)", + output: "for (a;;) {console.log(foo)}", + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "missingCurlyAfterCondition", + data: { name: "for" }, + type: "ForStatement", + line: 1, + column: 9, + endLine: 1, + endColumn: 11 + } + ] + }, + { + code: "for (a;;) \n console.log(foo)", + output: "for (a;;) \n {console.log(foo)}", + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "missingCurlyAfterCondition", + data: { name: "for" }, + type: "ForStatement", + line: 1, + column: 9, + endLine: 2, + endColumn: 2 + } + ] + }, + { + code: "for (var foo of bar) {console.log(foo)}", + output: "for (var foo of bar) console.log(foo)", + options: ["multi"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "unexpectedCurlyAfter", + data: { name: "for-of" }, + type: "ForOfStatement", + line: 1, + column: 22, + endLine: 1, + endColumn: 23 + } + ] + }, + { + code: "do{foo();} while(bar);", + output: "do foo(); while(bar);", + options: ["multi"], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: "unexpectedCurlyAfter", + data: { name: "do" }, + type: "DoWhileStatement", + line: 1, + column: 3, + endLine: 1, + endColumn: 4 } ] }, @@ -519,7 +662,26 @@ ruleTester.run("curly", rule, { { messageId: "unexpectedCurlyAfterCondition", data: { name: "for" }, - type: "ForStatement" + type: "ForStatement", + line: 1, + column: 13, + endLine: 1, + endColumn: 14 + } + ] + }, + { + code: "for (;foo;) \n bar() ", + output: "for (;foo;) \n {bar()} ", + errors: [ + { + data: { name: "for" }, + type: "ForStatement", + messageId: "missingCurlyAfterCondition", + line: 1, + column: 11, + endLine: 2, + endColumn: 2 } ] }, @@ -531,7 +693,11 @@ ruleTester.run("curly", rule, { { messageId: "unexpectedCurlyAfterCondition", data: { name: "if" }, - type: "IfStatement" + type: "IfStatement", + line: 1, + column: 10, + endLine: 1, + endColumn: 11 } ] }, @@ -567,7 +733,11 @@ ruleTester.run("curly", rule, { { messageId: "unexpectedCurlyAfterCondition", data: { name: "while" }, - type: "WhileStatement" + type: "WhileStatement", + line: 1, + column: 13, + endLine: 1, + endColumn: 14 } ] }, @@ -653,7 +823,9 @@ ruleTester.run("curly", rule, { data: { name: "else" }, type: "IfStatement", line: 6, - column: 3 + column: 8, + endLine: 6, + endColumn: 9 } ] }, @@ -665,7 +837,11 @@ ruleTester.run("curly", rule, { { messageId: "unexpectedCurlyAfter", data: { name: "for-in" }, - type: "ForInStatement" + type: "ForInStatement", + line: 1, + column: 22, + endLine: 1, + endColumn: 23 } ] }, @@ -690,7 +866,26 @@ ruleTester.run("curly", rule, { { messageId: "missingCurlyAfterCondition", data: { name: "if" }, - type: "IfStatement" + type: "IfStatement", + line: 1, + column: 8, + endLine: 2, + endColumn: 2 + } + ] + }, + { + code: "if (foo) baz()", + output: "if (foo) {baz()}", + errors: [ + { + messageId: "missingCurlyAfterCondition", + data: { name: "if" }, + type: "IfStatement", + line: 1, + column: 8, + endLine: 1, + endColumn: 10 } ] }, @@ -742,6 +937,22 @@ ruleTester.run("curly", rule, { } ] }, + { + code: "do foo(); while (bar)", + output: "do {foo();} while (bar)", + options: ["all"], + errors: [ + { + messageId: "missingCurlyAfter", + data: { name: "do" }, + type: "DoWhileStatement", + line: 1, + column: 1, + endLine: 1, + endColumn: 4 + } + ] + }, { code: "do \n foo(); \n while (bar)", output: "do \n {foo();} \n while (bar)", @@ -750,7 +961,27 @@ ruleTester.run("curly", rule, { { messageId: "missingCurlyAfter", data: { name: "do" }, - type: "DoWhileStatement" + type: "DoWhileStatement", + line: 1, + column: 1, + endLine: 2, + endColumn: 2 + } + ] + }, + { + code: "for (var foo in bar) {console.log(foo)}", + output: "for (var foo in bar) console.log(foo)", + options: ["multi"], + errors: [ + { + messageId: "unexpectedCurlyAfter", + data: { name: "for-in" }, + type: "ForInStatement", + line: 1, + column: 22, + endLine: 1, + endColumn: 23 } ] }, @@ -787,7 +1018,11 @@ ruleTester.run("curly", rule, { { messageId: "missingCurlyAfter", data: { name: "for-of" }, - type: "ForOfStatement" + type: "ForOfStatement", + line: 1, + column: 20, + endLine: 2, + endColumn: 2 } ] }, @@ -910,7 +1145,45 @@ ruleTester.run("curly", rule, { { messageId: "unexpectedCurlyAfterCondition", data: { name: "for" }, - type: "ForStatement" + type: "ForStatement", + line: 1, + column: 27, + endLine: 1, + endColumn: 28 + } + ] + }, + { + code: "for (var foo in bar) if (foo) console.log(1); else console.log(2);", + output: "for (var foo in bar) {if (foo) console.log(1); else console.log(2);}", + options: ["all"], + errors: [ + { + messageId: "missingCurlyAfter", + data: { name: "for-in" }, + type: "ForInStatement", + line: 1, + column: 20, + endLine: 1, + endColumn: 22 + }, + { + messageId: "missingCurlyAfterCondition", + data: { name: "if" }, + type: "IfStatement", + line: 1, + column: 29, + endLine: 1, + endColumn: 31 + }, + { + messageId: "missingCurlyAfter", + data: { name: "else" }, + type: "IfStatement", + line: 1, + column: 47, + endLine: 1, + endColumn: 51 } ] }, @@ -922,7 +1195,11 @@ ruleTester.run("curly", rule, { { messageId: "missingCurlyAfter", data: { name: "for-in" }, - type: "ForInStatement" + type: "ForInStatement", + line: 1, + column: 20, + endLine: 2, + endColumn: 2 } ] }, @@ -996,7 +1273,11 @@ ruleTester.run("curly", rule, { { messageId: "unexpectedCurlyAfter", data: { name: "else" }, - type: "IfStatement" + type: "IfStatement", + line: 1, + column: 23, + endColumn: 24, + endLine: 1 } ] }, @@ -1041,6 +1322,70 @@ ruleTester.run("curly", rule, { } ] }, + { + code: "do\n{foo();} while (bar)", + output: "do\nfoo(); while (bar)", + options: ["multi"], + errors: [ + { + messageId: "unexpectedCurlyAfter", + data: { name: "do" }, + type: "DoWhileStatement", + line: 2, + column: 1, + endLine: 2, + endColumn: 2 + } + ] + }, + { + code: "while (bar) { foo(); }", + output: "while (bar) foo(); ", + options: ["multi"], + errors: [ + { + messageId: "unexpectedCurlyAfterCondition", + data: { name: "while" }, + type: "WhileStatement", + line: 1, + column: 13, + endLine: 1, + endColumn: 14 + } + ] + }, + { + code: "while (bar) \n{\n foo(); }", + output: "while (bar) \n\n foo(); ", + options: ["multi"], + errors: [ + { + messageId: "unexpectedCurlyAfterCondition", + data: { name: "while" }, + type: "WhileStatement", + line: 2, + column: 1, + endLine: 2, + endColumn: 2 + } + ] + }, + { + code: "for (;;) { foo(); }", + output: "for (;;) foo(); ", + options: ["multi"], + errors: [ + { + messageId: "unexpectedCurlyAfterCondition", + data: { name: "for" }, + type: "ForStatement", + line: 1, + column: 10, + endLine: 1, + endColumn: 11 + } + ] + }, { code: "do{[1, 2, 3].map(bar);} while (bar)", output: "do[1, 2, 3].map(bar); while (bar)", @@ -1313,7 +1658,132 @@ ruleTester.run("curly", rule, { code: "if (a) { while (cond) if (b) foo() } else bar();", output: "if (a) { while (cond) if (b) foo() } else {bar();}", options: ["multi", "consistent"], - errors: [{ messageId: "missingCurlyAfter", data: { name: "else" }, type: "IfStatement" }] + errors: [ + { + messageId: "missingCurlyAfter", + data: { name: "else" }, + type: "IfStatement", + line: 1, + column: 38, + endLine: 1, + endColumn: 42 + } + ] + }, + { + code: "if (a) while (cond) if (b) foo() \nelse\n {bar();}", + output: "if (a) while (cond) if (b) foo() \nelse\n bar();", + options: ["multi", "consistent"], + errors: [ + { + messageId: "unexpectedCurlyAfter", + data: { name: "else" }, + type: "IfStatement", + line: 3, + column: 2, + endLine: 3, + endColumn: 3 + } + ] + }, + { + code: "if (a) foo() \nelse\n bar();", + output: "if (a) {foo()} \nelse\n {bar();}", + errors: [{ + type: "IfStatement", + messageId: "missingCurlyAfterCondition", + line: 1, + column: 6, + endLine: 1, + endColumn: 8 + }, + { + type: "IfStatement", + messageId: "missingCurlyAfter", + line: 2, + column: 1, + endLine: 2, + endColumn: 5 + }] + }, + { + code: "if (a) { while (cond) if (b) foo() } ", + output: "if (a) while (cond) if (b) foo() ", + options: ["multi", "consistent"], + errors: [{ + messageId: "unexpectedCurlyAfterCondition", + data: { name: "if" }, + type: "IfStatement", + line: 1, + column: 8, + endLine: 1, + endColumn: 9 + }] + }, + { + code: "if(a) { if (b) foo(); } if (c) bar(); else if(foo){bar();}", + output: "if(a) if (b) foo(); if (c) bar(); else if(foo)bar();", + options: ["multi-or-nest"], + errors: [{ + type: "IfStatement", + data: { name: "if" }, + messageId: "unexpectedCurlyAfterCondition", + line: 1, + column: 7, + endLine: 1, + endColumn: 8 + }, + { + type: "IfStatement", + data: { name: "if" }, + messageId: "unexpectedCurlyAfterCondition", + line: 1, + column: 51, + endLine: 1, + endColumn: 52 + }] + }, + { + code: "if (true) [1, 2, 3]\n.bar()", + output: "if (true) {[1, 2, 3]\n.bar()}", + options: ["multi-line"], + errors: [{ + data: { name: "if" }, + type: "IfStatement", + messageId: "missingCurlyAfterCondition", + line: 1, + column: 9, + endLine: 1, + endColumn: 11 + }] + }, + { + code: "for(\n;\n;\n) {foo()}", + output: "for(\n;\n;\n) foo()", + options: ["multi"], + errors: [{ + data: { name: "for" }, + type: "ForStatement", + messageId: "unexpectedCurlyAfterCondition", + line: 4, + column: 3, + endLine: 4, + endColumn: 4 + }] + }, + { + code: "for(\n;\n;\n) \nfoo()\n", + output: "for(\n;\n;\n) \n{foo()}\n", + options: ["multi-line"], + errors: [{ + data: { name: "for" }, + type: "ForStatement", + messageId: "missingCurlyAfterCondition", + line: 4, + column: 1, + endLine: 5, + endColumn: 1 + }] }, { @@ -1330,6 +1800,46 @@ ruleTester.run("curly", rule, { { messageId: "unexpectedCurlyAfterCondition", data: { name: "if" }, type: "IfStatement" }, { messageId: "unexpectedCurlyAfterCondition", data: { name: "while" }, type: "WhileStatement" } ] + }, + { + code: "for(;;)foo()\n", + output: "for(;;){foo()}\n", + errors: [{ + data: { name: "for" }, + type: "ForStatement", + messageId: "missingCurlyAfterCondition", + line: 1, + column: 7, + endLine: 1, + endColumn: 8 + }] + }, + { + code: "for(var \ni \n in \n z)foo()\n", + output: "for(var \ni \n in \n z){foo()}\n", + errors: [{ + data: { name: "for-in" }, + type: "ForInStatement", + messageId: "missingCurlyAfter", + line: 4, + column: 3, + endLine: 4, + endColumn: 4 + }] + }, + { + code: "for(var i of \n z)\nfoo()\n", + output: "for(var i of \n z)\n{foo()}\n", + parserOptions: { ecmaVersion: 6 }, + errors: [{ + data: { name: "for-of" }, + type: "ForOfStatement", + messageId: "missingCurlyAfter", + line: 2, + column: 3, + endLine: 3, + endColumn: 1 + }] } ] }); From 535cd328f67a434a71cb477ea017190a3fc93e13 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sun, 11 Jul 2021 08:50:49 +0530 Subject: [PATCH 2/6] Chore: refactor code --- lib/rules/curly.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/rules/curly.js b/lib/rules/curly.js index 8b4f1110147..98280760da0 100644 --- a/lib/rules/curly.js +++ b/lib/rules/curly.js @@ -335,7 +335,6 @@ module.exports = { * Missing curly brace reporting location examples: * * The following location should be reported - *This conversation was marked as resolved by anikethsaha * * if (foo) bar() * ^^^ @@ -413,15 +412,15 @@ module.exports = { if (name === "if") { return isMissing ? { - start: node.test.loc.end, + start: sourceCode.getTokenBefore(node.consequent).loc.start, end: node.consequent.loc.start } - : createNextLocation(node.consequent.loc.start); + : sourceCode.getFirstToken(node.consequent).loc; } if (name === "while") { return isMissing ? { - start: node.test.loc.end, + start: sourceCode.getTokenBefore(node.consequent).loc.start, end: node.body.loc.start } : createNextLocation(node.body.loc.start); } From 6f65ba79a91d25cb766c31ea5efc5adac2f28873 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sun, 11 Jul 2021 09:07:17 +0530 Subject: [PATCH 3/6] Chore: refactor code --- lib/rules/curly.js | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/lib/rules/curly.js b/lib/rules/curly.js index 98280760da0..a17dd1d4b6c 100644 --- a/lib/rules/curly.js +++ b/lib/rules/curly.js @@ -10,28 +10,6 @@ const astUtils = require("./utils/ast-utils"); -//------------------------------------------------------------------------------ -// Helper -//------------------------------------------------------------------------------ - -const blockStartLoc = new Set(["for", "for-of", "do", "for-in"]); - -/** - * Returns location of given position with `end` position next to `start` position. - * @param {Position} position position whose very next needs to be created. - * @returns {Location} the new location. - */ -function createNextLocation(position) { - - return { - start: position, - end: { - line: position.line, - column: position.column + 1 - } - }; -} - //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -385,7 +363,9 @@ module.exports = { * @param {boolean} isMissing is curly brace missing. * @returns {Object} Position object. */ - function getReportLoc(node, name, isMissing) { + function getReportingLoc(node, name, isMissing) { + const blockStartLoc = new Set(["for", "for-of", "do", "for-in"]); + if (name === "else") { if (!isMissing) { const token = sourceCode.getTokenAfter(getElseKeyword(node)); @@ -406,7 +386,7 @@ module.exports = { }; } - return createNextLocation(node.body.loc.start); + return sourceCode.getFirstToken(node.body).loc; } if (name === "if") { @@ -420,9 +400,9 @@ module.exports = { if (name === "while") { return isMissing ? { - start: sourceCode.getTokenBefore(node.consequent).loc.start, + start: sourceCode.getTokenBefore(node.body).loc.start, end: node.body.loc.start - } : createNextLocation(node.body.loc.start); + } : sourceCode.getFirstToken(node.body).loc; } return node.loc; @@ -478,7 +458,7 @@ module.exports = { check() { if (this.expected !== null && this.expected !== this.actual) { if (this.expected) { - const loc = getReportLoc(node, name, true); + const loc = getReportingLoc(node, name, true); context.report({ node, @@ -490,7 +470,7 @@ module.exports = { fix: fixer => fixer.replaceText(body, `{${sourceCode.getText(body)}}`) }); } else { - const loc = getReportLoc(node, name, false); + const loc = getReportingLoc(node, name, false); context.report({ node, From 52fce99455fde6cf6ae914780c2af63b35c00d1d Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Fri, 16 Jul 2021 07:06:27 +0530 Subject: [PATCH 4/6] Update: report body/consequent/alternate node --- lib/rules/curly.js | 77 ++++------------ tests/lib/rules/curly.js | 192 +++++++++++++++++++-------------------- 2 files changed, 116 insertions(+), 153 deletions(-) diff --git a/lib/rules/curly.js b/lib/rules/curly.js index a17dd1d4b6c..f06f56913c9 100644 --- a/lib/rules/curly.js +++ b/lib/rules/curly.js @@ -131,15 +131,6 @@ module.exports = { return token.value === "else" && token.type === "Keyword"; } - /** - * Gets the `else` keyword token of a given `IfStatement` node. - * @param {ASTNode} node A `IfStatement` node to get. - * @returns {Token} The `else` keyword token. - */ - function getElseKeyword(node) { - return node.alternate && sourceCode.getFirstTokenBetween(node.consequent, node.alternate, isElseKeywordToken); - } - /** * Determines whether the given node has an `else` keyword token as the first token after. * @param {ASTNode} node The node to check. @@ -315,94 +306,66 @@ module.exports = { * The following location should be reported * * if (foo) bar() - * ^^^ + * ^^^^^ * * while(foo) bar(); - * ^^^ + * ^^^^^^ * * do foo() while(bar) * ^^^ * * for(;;) bar(); - * ^^^ + * ^^^^^^ * * for (var foo of bar) console.log(foo) - * ^^^ + * ^^^^^^^^^^^^^^^^ * * for (var foo in bar) console.log(foo) - * ^^^ + * ^^^^^^^^^^^^^^^ * * if (foo) {bar()} else bar(1); - * ^^^^ + * ^^^^^^^ * * Unexpected curly brace reporting location examples: * * The following location should be reported * * if (true) foo(); else { baz(); } - * ^ + * ^^^^^^^^^^ * * for (;;) { foo(); } - * ^ + * ^^^^^^^^^^ * * while (bar) { foo(); } - * ^ + * ^^^^^^^^^^ * * do{foo();} while(bar); - * ^ + * ^^^^^^^^ * * for (var foo of bar) {console.log(foo)} - * ^ + * ^^^^^^^^^^^^^^^^^^ * - * if (foo) bar() else {bar(1)} - * ^^^^ + * if (foo) bar() else { bar(1) } + * ^^^^^^^^^^ * * Get the location of missing or non missing curly brace. * @param {ASTNode} node the node to check. * @param {string} name name to check. - * @param {boolean} isMissing is curly brace missing. * @returns {Object} Position object. */ - function getReportingLoc(node, name, isMissing) { - const blockStartLoc = new Set(["for", "for-of", "do", "for-in"]); + function getReportingLoc(node, name) { + const blockStartLoc = new Set(["do", "for", "for-of", "for-in", "while"]); if (name === "else") { - if (!isMissing) { - const token = sourceCode.getTokenAfter(getElseKeyword(node)); - - return token.loc; - } - - return getElseKeyword(node).loc; + return node.alternate.loc; } if (blockStartLoc.has(name)) { - if (isMissing) { - const token = sourceCode.getTokenBefore(node.body); - - return { - start: token.loc.start, - end: node.body.loc.start - }; - } - - return sourceCode.getFirstToken(node.body).loc; + return node.body.loc; } if (name === "if") { - return isMissing - ? { - start: sourceCode.getTokenBefore(node.consequent).loc.start, - end: node.consequent.loc.start - } - : sourceCode.getFirstToken(node.consequent).loc; - } - - if (name === "while") { - return isMissing ? { - start: sourceCode.getTokenBefore(node.body).loc.start, - end: node.body.loc.start - } : sourceCode.getFirstToken(node.body).loc; + return node.consequent.loc; } return node.loc; @@ -458,7 +421,7 @@ module.exports = { check() { if (this.expected !== null && this.expected !== this.actual) { if (this.expected) { - const loc = getReportingLoc(node, name, true); + const loc = getReportingLoc(node, name); context.report({ node, @@ -470,7 +433,7 @@ module.exports = { fix: fixer => fixer.replaceText(body, `{${sourceCode.getText(body)}}`) }); } else { - const loc = getReportingLoc(node, name, false); + const loc = getReportingLoc(node, name); context.report({ node, diff --git a/tests/lib/rules/curly.js b/tests/lib/rules/curly.js index 2ade8161d44..c8121fe5d1e 100644 --- a/tests/lib/rules/curly.js +++ b/tests/lib/rules/curly.js @@ -431,9 +431,9 @@ ruleTester.run("curly", rule, { data: { name: "if" }, type: "IfStatement", line: 1, - column: 8, + column: 10, endLine: 1, - endColumn: 10 + endColumn: 15 } ] }, @@ -445,10 +445,10 @@ ruleTester.run("curly", rule, { messageId: "missingCurlyAfterCondition", data: { name: "if" }, type: "IfStatement", - line: 1, - column: 8, + line: 2, + column: 2, endLine: 2, - endColumn: 2 + endColumn: 7 } ] }, @@ -483,9 +483,9 @@ ruleTester.run("curly", rule, { data: { name: "while" }, type: "WhileStatement", line: 1, - column: 11, + column: 13, endLine: 1, - endColumn: 13 + endColumn: 18 } ] }, @@ -497,10 +497,10 @@ ruleTester.run("curly", rule, { messageId: "missingCurlyAfterCondition", data: { name: "while" }, type: "WhileStatement", - line: 1, - column: 11, + line: 2, + column: 2, endLine: 2, - endColumn: 2 + endColumn: 7 } ] }, @@ -513,9 +513,9 @@ ruleTester.run("curly", rule, { data: { name: "do" }, type: "DoWhileStatement", line: 1, - column: 1, + column: 4, endLine: 1, - endColumn: 4 + endColumn: 10 } ] }, @@ -527,10 +527,10 @@ ruleTester.run("curly", rule, { messageId: "missingCurlyAfter", data: { name: "do" }, type: "DoWhileStatement", - line: 1, - column: 1, + line: 2, + column: 2, endLine: 2, - endColumn: 2 + endColumn: 8 } ] }, @@ -566,9 +566,9 @@ ruleTester.run("curly", rule, { data: { name: "for-of" }, type: "ForOfStatement", line: 1, - column: 20, + column: 22, endLine: 1, - endColumn: 22 + endColumn: 38 } ] }, @@ -581,10 +581,10 @@ ruleTester.run("curly", rule, { messageId: "missingCurlyAfter", data: { name: "for-of" }, type: "ForOfStatement", - line: 1, - column: 20, + line: 2, + column: 2, endLine: 2, - endColumn: 2 + endColumn: 18 } ] }, @@ -598,9 +598,9 @@ ruleTester.run("curly", rule, { data: { name: "for" }, type: "ForStatement", line: 1, - column: 9, + column: 11, endLine: 1, - endColumn: 11 + endColumn: 27 } ] }, @@ -613,10 +613,10 @@ ruleTester.run("curly", rule, { messageId: "missingCurlyAfterCondition", data: { name: "for" }, type: "ForStatement", - line: 1, - column: 9, + line: 2, + column: 2, endLine: 2, - endColumn: 2 + endColumn: 18 } ] }, @@ -633,7 +633,7 @@ ruleTester.run("curly", rule, { line: 1, column: 22, endLine: 1, - endColumn: 23 + endColumn: 40 } ] }, @@ -650,7 +650,7 @@ ruleTester.run("curly", rule, { line: 1, column: 3, endLine: 1, - endColumn: 4 + endColumn: 11 } ] }, @@ -666,22 +666,22 @@ ruleTester.run("curly", rule, { line: 1, column: 13, endLine: 1, - endColumn: 14 + endColumn: 22 } ] }, { - code: "for (;foo;) \n bar() ", - output: "for (;foo;) \n {bar()} ", + code: "for (;foo;) \n bar()", + output: "for (;foo;) \n {bar()}", errors: [ { data: { name: "for" }, type: "ForStatement", messageId: "missingCurlyAfterCondition", - line: 1, - column: 11, + line: 2, + column: 2, endLine: 2, - endColumn: 2 + endColumn: 7 } ] }, @@ -697,7 +697,7 @@ ruleTester.run("curly", rule, { line: 1, column: 10, endLine: 1, - endColumn: 11 + endColumn: 19 } ] }, @@ -737,7 +737,7 @@ ruleTester.run("curly", rule, { line: 1, column: 13, endLine: 1, - endColumn: 14 + endColumn: 22 } ] }, @@ -824,8 +824,8 @@ ruleTester.run("curly", rule, { type: "IfStatement", line: 6, column: 8, - endLine: 6, - endColumn: 9 + endLine: 11, + endColumn: 2 } ] }, @@ -841,7 +841,7 @@ ruleTester.run("curly", rule, { line: 1, column: 22, endLine: 1, - endColumn: 23 + endColumn: 42 } ] }, @@ -867,10 +867,10 @@ ruleTester.run("curly", rule, { messageId: "missingCurlyAfterCondition", data: { name: "if" }, type: "IfStatement", - line: 1, - column: 8, + line: 2, + column: 2, endLine: 2, - endColumn: 2 + endColumn: 7 } ] }, @@ -883,9 +883,9 @@ ruleTester.run("curly", rule, { data: { name: "if" }, type: "IfStatement", line: 1, - column: 8, + column: 10, endLine: 1, - endColumn: 10 + endColumn: 15 } ] }, @@ -947,9 +947,9 @@ ruleTester.run("curly", rule, { data: { name: "do" }, type: "DoWhileStatement", line: 1, - column: 1, + column: 4, endLine: 1, - endColumn: 4 + endColumn: 10 } ] }, @@ -962,10 +962,10 @@ ruleTester.run("curly", rule, { messageId: "missingCurlyAfter", data: { name: "do" }, type: "DoWhileStatement", - line: 1, - column: 1, + line: 2, + column: 2, endLine: 2, - endColumn: 2 + endColumn: 8 } ] }, @@ -981,7 +981,7 @@ ruleTester.run("curly", rule, { line: 1, column: 22, endLine: 1, - endColumn: 23 + endColumn: 40 } ] }, @@ -1019,10 +1019,10 @@ ruleTester.run("curly", rule, { messageId: "missingCurlyAfter", data: { name: "for-of" }, type: "ForOfStatement", - line: 1, - column: 20, + line: 2, + column: 2, endLine: 2, - endColumn: 2 + endColumn: 18 } ] }, @@ -1148,8 +1148,8 @@ ruleTester.run("curly", rule, { type: "ForStatement", line: 1, column: 27, - endLine: 1, - endColumn: 28 + endLine: 3, + endColumn: 3 } ] }, @@ -1163,27 +1163,27 @@ ruleTester.run("curly", rule, { data: { name: "for-in" }, type: "ForInStatement", line: 1, - column: 20, + column: 22, endLine: 1, - endColumn: 22 + endColumn: 67 }, { messageId: "missingCurlyAfterCondition", data: { name: "if" }, type: "IfStatement", line: 1, - column: 29, + column: 31, endLine: 1, - endColumn: 31 + endColumn: 46 }, { messageId: "missingCurlyAfter", data: { name: "else" }, type: "IfStatement", line: 1, - column: 47, + column: 52, endLine: 1, - endColumn: 51 + endColumn: 67 } ] }, @@ -1196,10 +1196,10 @@ ruleTester.run("curly", rule, { messageId: "missingCurlyAfter", data: { name: "for-in" }, type: "ForInStatement", - line: 1, - column: 20, - endLine: 2, - endColumn: 2 + line: 2, + column: 2, + endLine: 3, + endColumn: 22 } ] }, @@ -1276,8 +1276,8 @@ ruleTester.run("curly", rule, { type: "IfStatement", line: 1, column: 23, - endColumn: 24, - endLine: 1 + endLine: 1, + endColumn: 33 } ] }, @@ -1334,7 +1334,7 @@ ruleTester.run("curly", rule, { line: 2, column: 1, endLine: 2, - endColumn: 2 + endColumn: 9 } ] }, @@ -1350,7 +1350,7 @@ ruleTester.run("curly", rule, { line: 1, column: 13, endLine: 1, - endColumn: 14 + endColumn: 23 } ] }, @@ -1365,8 +1365,8 @@ ruleTester.run("curly", rule, { type: "WhileStatement", line: 2, column: 1, - endLine: 2, - endColumn: 2 + endLine: 3, + endColumn: 10 } ] }, @@ -1382,7 +1382,7 @@ ruleTester.run("curly", rule, { line: 1, column: 10, endLine: 1, - endColumn: 11 + endColumn: 20 } ] }, @@ -1664,9 +1664,9 @@ ruleTester.run("curly", rule, { data: { name: "else" }, type: "IfStatement", line: 1, - column: 38, + column: 43, endLine: 1, - endColumn: 42 + endColumn: 49 } ] }, @@ -1682,7 +1682,7 @@ ruleTester.run("curly", rule, { line: 3, column: 2, endLine: 3, - endColumn: 3 + endColumn: 10 } ] }, @@ -1693,17 +1693,17 @@ ruleTester.run("curly", rule, { type: "IfStatement", messageId: "missingCurlyAfterCondition", line: 1, - column: 6, + column: 8, endLine: 1, - endColumn: 8 + endColumn: 13 }, { type: "IfStatement", messageId: "missingCurlyAfter", - line: 2, - column: 1, - endLine: 2, - endColumn: 5 + line: 3, + column: 2, + endLine: 3, + endColumn: 8 }] }, { @@ -1717,7 +1717,7 @@ ruleTester.run("curly", rule, { line: 1, column: 8, endLine: 1, - endColumn: 9 + endColumn: 37 }] }, { @@ -1731,7 +1731,7 @@ ruleTester.run("curly", rule, { line: 1, column: 7, endLine: 1, - endColumn: 8 + endColumn: 24 }, { type: "IfStatement", @@ -1740,7 +1740,7 @@ ruleTester.run("curly", rule, { line: 1, column: 51, endLine: 1, - endColumn: 52 + endColumn: 59 }] }, { @@ -1752,9 +1752,9 @@ ruleTester.run("curly", rule, { type: "IfStatement", messageId: "missingCurlyAfterCondition", line: 1, - column: 9, - endLine: 1, - endColumn: 11 + column: 11, + endLine: 2, + endColumn: 7 }] }, { @@ -1768,7 +1768,7 @@ ruleTester.run("curly", rule, { line: 4, column: 3, endLine: 4, - endColumn: 4 + endColumn: 10 }] }, { @@ -1779,10 +1779,10 @@ ruleTester.run("curly", rule, { data: { name: "for" }, type: "ForStatement", messageId: "missingCurlyAfterCondition", - line: 4, + line: 5, column: 1, endLine: 5, - endColumn: 1 + endColumn: 6 }] }, { @@ -1809,9 +1809,9 @@ ruleTester.run("curly", rule, { type: "ForStatement", messageId: "missingCurlyAfterCondition", line: 1, - column: 7, + column: 8, endLine: 1, - endColumn: 8 + endColumn: 13 }] }, { @@ -1822,9 +1822,9 @@ ruleTester.run("curly", rule, { type: "ForInStatement", messageId: "missingCurlyAfter", line: 4, - column: 3, + column: 4, endLine: 4, - endColumn: 4 + endColumn: 9 }] }, { @@ -1835,10 +1835,10 @@ ruleTester.run("curly", rule, { data: { name: "for-of" }, type: "ForOfStatement", messageId: "missingCurlyAfter", - line: 2, - column: 3, + line: 3, + column: 1, endLine: 3, - endColumn: 1 + endColumn: 6 }] } ] From 44eb6c82cff161c1b4c03d21ba76f423a60b654b Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Fri, 16 Jul 2021 07:08:49 +0530 Subject: [PATCH 5/6] Chore: update comment --- lib/rules/curly.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/curly.js b/lib/rules/curly.js index f06f56913c9..17ccdb97c64 100644 --- a/lib/rules/curly.js +++ b/lib/rules/curly.js @@ -312,7 +312,7 @@ module.exports = { * ^^^^^^ * * do foo() while(bar) - * ^^^ + * ^^^^^ * * for(;;) bar(); * ^^^^^^ @@ -339,8 +339,8 @@ module.exports = { * while (bar) { foo(); } * ^^^^^^^^^^ * - * do{foo();} while(bar); - * ^^^^^^^^ + * do {foo();} while(bar); + * ^^^^^^^^ * * for (var foo of bar) {console.log(foo)} * ^^^^^^^^^^^^^^^^^^ From faf229e9bc5311f4dc76a348cf38b68ee9e9c151 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Tue, 20 Jul 2021 14:26:42 +0530 Subject: [PATCH 6/6] Chore: refactor code --- lib/rules/curly.js | 79 ++-------------------------------------------- 1 file changed, 2 insertions(+), 77 deletions(-) diff --git a/lib/rules/curly.js b/lib/rules/curly.js index 17ccdb97c64..61dcd8024bb 100644 --- a/lib/rules/curly.js +++ b/lib/rules/curly.js @@ -300,77 +300,6 @@ module.exports = { hasUnsafeIf(statement) && isFollowedByElseKeyword(node); } - /** - * Missing curly brace reporting location examples: - * - * The following location should be reported - * - * if (foo) bar() - * ^^^^^ - * - * while(foo) bar(); - * ^^^^^^ - * - * do foo() while(bar) - * ^^^^^ - * - * for(;;) bar(); - * ^^^^^^ - * - * for (var foo of bar) console.log(foo) - * ^^^^^^^^^^^^^^^^ - * - * for (var foo in bar) console.log(foo) - * ^^^^^^^^^^^^^^^ - * - * if (foo) {bar()} else bar(1); - * ^^^^^^^ - * - * Unexpected curly brace reporting location examples: - * - * The following location should be reported - * - * if (true) foo(); else { baz(); } - * ^^^^^^^^^^ - * - * for (;;) { foo(); } - * ^^^^^^^^^^ - * - * while (bar) { foo(); } - * ^^^^^^^^^^ - * - * do {foo();} while(bar); - * ^^^^^^^^ - * - * for (var foo of bar) {console.log(foo)} - * ^^^^^^^^^^^^^^^^^^ - * - * if (foo) bar() else { bar(1) } - * ^^^^^^^^^^ - * - * Get the location of missing or non missing curly brace. - * @param {ASTNode} node the node to check. - * @param {string} name name to check. - * @returns {Object} Position object. - */ - function getReportingLoc(node, name) { - const blockStartLoc = new Set(["do", "for", "for-of", "for-in", "while"]); - - if (name === "else") { - return node.alternate.loc; - } - - if (blockStartLoc.has(name)) { - return node.body.loc; - } - - if (name === "if") { - return node.consequent.loc; - } - - return node.loc; - } - /** * Prepares to check the body of a node to see if it's a block statement. * @param {ASTNode} node The node to report if there's a problem. @@ -421,11 +350,9 @@ module.exports = { check() { if (this.expected !== null && this.expected !== this.actual) { if (this.expected) { - const loc = getReportingLoc(node, name); - context.report({ node, - loc, + loc: body.loc, messageId: opts && opts.condition ? "missingCurlyAfterCondition" : "missingCurlyAfter", data: { name @@ -433,11 +360,9 @@ module.exports = { fix: fixer => fixer.replaceText(body, `{${sourceCode.getText(body)}}`) }); } else { - const loc = getReportingLoc(node, name); - context.report({ node, - loc, + loc: body.loc, messageId: opts && opts.condition ? "unexpectedCurlyAfterCondition" : "unexpectedCurlyAfter", data: { name