From 757dce3d6eb08a09cb279248006d4369fd875347 Mon Sep 17 00:00:00 2001 From: Chiawen Chen Date: Thu, 3 Oct 2019 20:52:16 +0800 Subject: [PATCH 1/4] Update: improve report location for semi --- lib/rules/semi.js | 9 ++++++--- lib/rules/utils/ast-utils.js | 17 +++++++++++++++++ tests/lib/rules/semi.js | 13 +++++++++---- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/lib/rules/semi.js b/lib/rules/semi.js index a159e19d347..22e299efe72 100644 --- a/lib/rules/semi.js +++ b/lib/rules/semi.js @@ -93,17 +93,20 @@ module.exports = { const lastToken = sourceCode.getLastToken(node); let message, fix, - loc = lastToken.loc; + loc; if (!missing) { message = "Missing semicolon."; - loc = loc.end; + loc = { + start: lastToken.loc.end, + end: astUtils.getNextLocation(sourceCode, lastToken.loc.end) + }; fix = function(fixer) { return fixer.insertTextAfter(lastToken, ";"); }; } else { message = "Extra semicolon."; - loc = loc.start; + loc = lastToken.loc; fix = function(fixer) { /* diff --git a/lib/rules/utils/ast-utils.js b/lib/rules/utils/ast-utils.js index 17e056c240c..41ebac51183 100644 --- a/lib/rules/utils/ast-utils.js +++ b/lib/rules/utils/ast-utils.js @@ -1200,6 +1200,23 @@ module.exports = { }; }, + /** + * Gets next location when the result is not out of bound, otherwise returns undefined. + * @param {SourceCode} sourceCode The sourceCode + * @param {{line: number, column: number}} location The location + * @returns {{line: number, column: number} | undefined} Next location + */ + getNextLocation(sourceCode, location) { + const index = sourceCode.getIndexFromLoc(location); + + // Avoid out of bound location + if (index + 1 > sourceCode.text.length) { + return void 0; + } + + return sourceCode.getLocFromIndex(index + 1); + }, + /** * Gets the parenthesized text of a node. This is similar to sourceCode.getText(node), but it also includes any parentheses * surrounding the node. diff --git a/tests/lib/rules/semi.js b/tests/lib/rules/semi.js index e282d54e49c..90d328ad732 100644 --- a/tests/lib/rules/semi.js +++ b/tests/lib/rules/semi.js @@ -233,7 +233,7 @@ ruleTester.run("semi", rule, { } ], invalid: [ - { code: "import * as utils from './utils'", output: "import * as utils from './utils';", parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Missing semicolon.", type: "ImportDeclaration", column: 33 }] }, + { code: "import * as utils from './utils'", output: "import * as utils from './utils';", parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Missing semicolon.", type: "ImportDeclaration", column: 33, endLine: void 0, endColumn: void 0 }] }, { code: "import { square, diag } from 'lib'", output: "import { square, diag } from 'lib';", parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Missing semicolon.", type: "ImportDeclaration" }] }, { code: "import { default as foo } from 'lib'", output: "import { default as foo } from 'lib';", parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Missing semicolon.", type: "ImportDeclaration" }] }, { code: "import 'src/mylib'", output: "import 'src/mylib';", parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Missing semicolon.", type: "ImportDeclaration" }] }, @@ -245,7 +245,9 @@ ruleTester.run("semi", rule, { { code: "var x = 5", output: "var x = 5;", errors: [{ message: "Missing semicolon.", type: "VariableDeclaration" }] }, { code: "var x = 5, y", output: "var x = 5, y;", errors: [{ message: "Missing semicolon.", type: "VariableDeclaration" }] }, { code: "debugger", output: "debugger;", errors: [{ message: "Missing semicolon.", type: "DebuggerStatement" }] }, - { code: "foo()", output: "foo();", errors: [{ message: "Missing semicolon.", type: "ExpressionStatement" }] }, + { code: "foo()", output: "foo();", errors: [{ message: "Missing semicolon.", type: "ExpressionStatement", column: 6, endColumn: void 0 }] }, + { code: "foo()\n", output: "foo();\n", errors: [{ message: "Missing semicolon.", type: "ExpressionStatement", column: 6, endLine: 2, endColumn: 1 }] }, + { code: "foo()\nbar();", output: "foo();\nbar();", errors: [{ message: "Missing semicolon.", type: "ExpressionStatement", column: 6, endLine: 2, endColumn: 1 }] }, { code: "for (var a in b) var i ", output: "for (var a in b) var i; ", errors: [{ message: "Missing semicolon.", type: "VariableDeclaration" }] }, { code: "for (;;){var i}", output: "for (;;){var i;}", errors: [{ message: "Missing semicolon.", type: "VariableDeclaration" }] }, { code: "for (;;) var i ", output: "for (;;) var i; ", errors: [{ message: "Missing semicolon.", type: "VariableDeclaration" }] }, @@ -254,6 +256,9 @@ ruleTester.run("semi", rule, { { code: "var foo\nvar bar;", output: "var foo;\nvar bar;", errors: [{ message: "Missing semicolon.", type: "VariableDeclaration", line: 1 }] }, { code: "throw new Error('foo')", output: "throw new Error('foo');", errors: [{ message: "Missing semicolon.", type: "ThrowStatement", line: 1 }] }, { code: "do{}while(true)", output: "do{}while(true);", errors: [{ message: "Missing semicolon.", type: "DoWhileStatement", line: 1 }] }, + { code: "if (foo) {bar()}", output: "if (foo) {bar();}", errors: [{ message: "Missing semicolon.", column: 16, endColumn: 17 }] }, + { code: "if (foo) {bar()} ", output: "if (foo) {bar();} ", errors: [{ message: "Missing semicolon.", column: 16, endColumn: 17 }] }, + { code: "if (foo) {bar()\n}", output: "if (foo) {bar();\n}", errors: [{ message: "Missing semicolon.", column: 16, endLine: 2, endColumn: 1 }] }, { code: "throw new Error('foo');", output: "throw new Error('foo')", options: ["never"], errors: [{ message: "Extra semicolon.", type: "ThrowStatement", column: 23 }] }, { code: "function foo() { return []; }", output: "function foo() { return [] }", options: ["never"], errors: [{ message: "Extra semicolon.", type: "ReturnStatement" }] }, @@ -291,7 +296,7 @@ ruleTester.run("semi", rule, { { code: "export default foo += 42", output: "export default foo += 42;", parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Missing semicolon.", type: "ExportDefaultDeclaration" }] }, // exports, "never" - { code: "export * from 'foo';", output: "export * from 'foo'", options: ["never"], parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Extra semicolon.", type: "ExportAllDeclaration" }] }, + { code: "export * from 'foo';", output: "export * from 'foo'", options: ["never"], parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Extra semicolon.", type: "ExportAllDeclaration", column: 20, endColumn: 21 }] }, { code: "export { foo } from 'foo';", output: "export { foo } from 'foo'", options: ["never"], parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Extra semicolon.", type: "ExportNamedDeclaration" }] }, { code: "var foo = 0;export { foo };", output: "var foo = 0;export { foo }", options: ["never"], parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Extra semicolon.", type: "ExportNamedDeclaration" }] }, { code: "export var foo;", output: "export var foo", options: ["never"], parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Extra semicolon.", type: "VariableDeclaration" }] }, @@ -301,7 +306,7 @@ ruleTester.run("semi", rule, { { code: "export default (foo) => foo.bar();", output: "export default (foo) => foo.bar()", options: ["never"], parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Extra semicolon.", type: "ExportDefaultDeclaration" }] }, { code: "export default foo = 42;", output: "export default foo = 42", options: ["never"], parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Extra semicolon.", type: "ExportDefaultDeclaration" }] }, { code: "export default foo += 42;", output: "export default foo += 42", options: ["never"], parserOptions: { ecmaVersion: 6, sourceType: "module" }, errors: [{ message: "Extra semicolon.", type: "ExportDefaultDeclaration" }] }, - { code: "a;\n++b", output: "a\n++b", options: ["never"], errors: [{ message: "Extra semicolon." }] }, + { code: "a;\n++b", output: "a\n++b", options: ["never"], errors: [{ message: "Extra semicolon.", column: 2, endColumn: 3 }] }, // https://github.com/eslint/eslint/issues/7928 { From 09ba4256e5150a274eee50f93c8c509b875ff858 Mon Sep 17 00:00:00 2001 From: Chiawen Chen Date: Thu, 3 Oct 2019 20:52:33 +0800 Subject: [PATCH 2/4] Update: improve report location for comma-dangle --- lib/rules/comma-dangle.js | 7 +++++-- tests/lib/rules/comma-dangle.js | 37 +++++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/lib/rules/comma-dangle.js b/lib/rules/comma-dangle.js index 0c177296ea1..6eef11563e7 100644 --- a/lib/rules/comma-dangle.js +++ b/lib/rules/comma-dangle.js @@ -231,7 +231,7 @@ module.exports = { if (astUtils.isCommaToken(trailingToken)) { context.report({ node: lastItem, - loc: trailingToken.loc.start, + loc: trailingToken.loc, messageId: "unexpected", fix(fixer) { return fixer.remove(trailingToken); @@ -267,7 +267,10 @@ module.exports = { if (trailingToken.value !== ",") { context.report({ node: lastItem, - loc: trailingToken.loc.end, + loc: { + start: trailingToken.loc.end, + end: astUtils.getNextLocation(sourceCode, trailingToken.loc.end) + }, messageId: "missing", fix(fixer) { return fixer.insertTextAfter(trailingToken, ","); diff --git a/tests/lib/rules/comma-dangle.js b/tests/lib/rules/comma-dangle.js index 573c0c2bd42..317d296a475 100644 --- a/tests/lib/rules/comma-dangle.js +++ b/tests/lib/rules/comma-dangle.js @@ -412,7 +412,8 @@ ruleTester.run("comma-dangle", rule, { messageId: "unexpected", type: "Property", line: 1, - column: 23 + column: 23, + endColumn: 24 } ] }, @@ -424,7 +425,8 @@ ruleTester.run("comma-dangle", rule, { messageId: "unexpected", type: "Property", line: 2, - column: 11 + column: 11, + endColumn: 12 } ] }, @@ -565,7 +567,9 @@ ruleTester.run("comma-dangle", rule, { messageId: "missing", type: "Property", line: 1, - column: 23 + column: 23, + endLine: 1, + endColumn: 24 } ] }, @@ -578,7 +582,9 @@ ruleTester.run("comma-dangle", rule, { messageId: "missing", type: "Property", line: 2, - column: 11 + column: 11, + endLine: 3, + endColumn: 1 } ] }, @@ -591,7 +597,10 @@ ruleTester.run("comma-dangle", rule, { messageId: "missing", type: "Property", line: 1, - column: 30 + column: 30, + endLine: 1, + endColumn: 31 + } ] }, @@ -604,7 +613,9 @@ ruleTester.run("comma-dangle", rule, { messageId: "missing", type: "Property", line: 3, - column: 12 + column: 12, + endLine: 4, + endColumn: 1 } ] }, @@ -621,6 +632,20 @@ ruleTester.run("comma-dangle", rule, { } ] }, + { + code: "var foo = ['baz']", + output: "var foo = ['baz',]", + options: ["always"], + errors: [ + { + messageId: "missing", + type: "Literal", + line: 1, + column: 17, + endColumn: 18 + } + ] + }, { code: "var foo = [ 'baz'\n]", output: "var foo = [ 'baz',\n]", From 49668c15afb25a7789d7c10753a011ee21bc8af0 Mon Sep 17 00:00:00 2001 From: Chiawen Chen Date: Sun, 17 Nov 2019 14:38:37 +0800 Subject: [PATCH 3/4] Make getNextLocation return null instead of undefined --- lib/rules/utils/ast-utils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/utils/ast-utils.js b/lib/rules/utils/ast-utils.js index 41ebac51183..8168679a5f6 100644 --- a/lib/rules/utils/ast-utils.js +++ b/lib/rules/utils/ast-utils.js @@ -1201,17 +1201,17 @@ module.exports = { }, /** - * Gets next location when the result is not out of bound, otherwise returns undefined. + * Gets next location when the result is not out of bound, otherwise returns null. * @param {SourceCode} sourceCode The sourceCode * @param {{line: number, column: number}} location The location - * @returns {{line: number, column: number} | undefined} Next location + * @returns {{line: number, column: number} | null} Next location */ getNextLocation(sourceCode, location) { const index = sourceCode.getIndexFromLoc(location); // Avoid out of bound location if (index + 1 > sourceCode.text.length) { - return void 0; + return null; } return sourceCode.getLocFromIndex(index + 1); From 2f689b36edd9d1ab0580f127f73462cb72b45693 Mon Sep 17 00:00:00 2001 From: Chiawen Chen Date: Sun, 17 Nov 2019 15:16:22 +0800 Subject: [PATCH 4/4] Add unit tests for astUtils.getNextLocation --- tests/lib/rules/utils/ast-utils.js | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/lib/rules/utils/ast-utils.js b/tests/lib/rules/utils/ast-utils.js index 577f7dac1ea..fb5526ff610 100644 --- a/tests/lib/rules/utils/ast-utils.js +++ b/tests/lib/rules/utils/ast-utils.js @@ -829,6 +829,42 @@ describe("ast-utils", () => { }); }); + describe("getNextLocation", () => { + const code = "foo;\n"; + const ast = espree.parse(code, ESPREE_CONFIG); + const sourceCode = new SourceCode(code, ast); + + it("should handle normal case", () => { + assert.deepStrictEqual( + astUtils.getNextLocation( + sourceCode, + { line: 1, column: 0 } + ), + { line: 1, column: 1 } + ); + }); + + it("should handle linebreaks", () => { + assert.deepStrictEqual( + astUtils.getNextLocation( + sourceCode, + { line: 1, column: 4 } + ), + { line: 2, column: 0 } + ); + }); + + it("should return null when result is out of bound", () => { + assert.strictEqual( + astUtils.getNextLocation( + sourceCode, + { line: 2, column: 0 } + ), + null + ); + }); + }); + describe("getParenthesisedText", () => { const expectedResults = { "(((foo))); bar;": "(((foo)))",