Skip to content

Commit

Permalink
Update: improve location for semi and comma-dangle (#12380)
Browse files Browse the repository at this point in the history 
* Update: improve report location for semi

* Update: improve report location for comma-dangle

* Make getNextLocation return null instead of undefined

* Add unit tests for astUtils.getNextLocation
  • Loading branch information
@golopot @kaicataldo
golopot authored and kaicataldo committed Nov 18, 2019
1 parent 0a480f8 commit 41a78fd
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 15 deletions.
7 changes: 5 additions & 2 deletions lib/rules/comma-dangle.js
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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, ",");
Expand Down
9 changes: 6 additions & 3 deletions lib/rules/semi.js
View file
Expand Up @@ -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) {

/*
Expand Down
17 changes: 17 additions & 0 deletions lib/rules/utils/ast-utils.js
View file
Expand Up @@ -1210,6 +1210,23 @@ module.exports = {
};
},

/**
* 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} | null} Next location
*/
getNextLocation(sourceCode, location) {
const index = sourceCode.getIndexFromLoc(location);

// Avoid out of bound location
if (index + 1 > sourceCode.text.length) {
return null;
}

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.
Expand Down
37 changes: 31 additions & 6 deletions tests/lib/rules/comma-dangle.js
View file
Expand Up @@ -486,7 +486,8 @@ ruleTester.run("comma-dangle", rule, {
messageId: "unexpected",
type: "Property",
line: 1,
column: 23
column: 23,
endColumn: 24
}
]
},
Expand All @@ -498,7 +499,8 @@ ruleTester.run("comma-dangle", rule, {
messageId: "unexpected",
type: "Property",
line: 2,
column: 11
column: 11,
endColumn: 12
}
]
},
Expand Down Expand Up @@ -639,7 +641,9 @@ ruleTester.run("comma-dangle", rule, {
messageId: "missing",
type: "Property",
line: 1,
column: 23
column: 23,
endLine: 1,
endColumn: 24
}
]
},
Expand All @@ -652,7 +656,9 @@ ruleTester.run("comma-dangle", rule, {
messageId: "missing",
type: "Property",
line: 2,
column: 11
column: 11,
endLine: 3,
endColumn: 1
}
]
},
Expand All @@ -665,7 +671,10 @@ ruleTester.run("comma-dangle", rule, {
messageId: "missing",
type: "Property",
line: 1,
column: 30
column: 30,
endLine: 1,
endColumn: 31

}
]
},
Expand All @@ -678,7 +687,9 @@ ruleTester.run("comma-dangle", rule, {
messageId: "missing",
type: "Property",
line: 3,
column: 12
column: 12,
endLine: 4,
endColumn: 1
}
]
},
Expand All @@ -695,6 +706,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]",
Expand Down
13 changes: 9 additions & 4 deletions tests/lib/rules/semi.js
View file
Expand Up @@ -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" }] },
Expand All @@ -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" }] },
Expand All @@ -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" }] },
Expand Down Expand Up @@ -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" }] },
Expand All @@ -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
{
Expand Down
36 changes: 36 additions & 0 deletions tests/lib/rules/utils/ast-utils.js
View file
Expand Up @@ -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)))",
Expand Down

0 comments on commit 41a78fd

Please sign in to comment.