Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update: improve location for semi and comma-dangle #12380

Merged
merged 4 commits into from Nov 18, 2019
Merged

Update: improve location for semi and comma-dangle #12380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
757dce3
Update: improve report location for semi
golopot Oct 3, 2019
09ba425
Update: improve report location for comma-dangle
golopot Oct 3, 2019
49668c1
Make getNextLocation return null instead of undefined
golopot Nov 17, 2019
2f689b3
Add unit tests for astUtils.getNextLocation
golopot Nov 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/rules/comma-dangle.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
Expand Up @@ -1200,6 +1200,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
Open in desktop
Expand Up @@ -412,7 +412,8 @@ ruleTester.run("comma-dangle", rule, {
messageId: "unexpected",
type: "Property",
line: 1,
column: 23
column: 23,
endColumn: 24
}
]
},
Expand All @@ -424,7 +425,8 @@ ruleTester.run("comma-dangle", rule, {
messageId: "unexpected",
type: "Property",
line: 2,
column: 11
column: 11,
endColumn: 12
}
]
},
Expand Down Expand Up @@ -565,7 +567,9 @@ ruleTester.run("comma-dangle", rule, {
messageId: "missing",
type: "Property",
line: 1,
column: 23
column: 23,
endLine: 1,
endColumn: 24
}
]
},
Expand All @@ -578,7 +582,9 @@ ruleTester.run("comma-dangle", rule, {
messageId: "missing",
type: "Property",
line: 2,
column: 11
column: 11,
endLine: 3,
endColumn: 1
}
]
},
Expand All @@ -591,7 +597,10 @@ ruleTester.run("comma-dangle", rule, {
messageId: "missing",
type: "Property",
line: 1,
column: 30
column: 30,
endLine: 1,
endColumn: 31

}
]
},
Expand All @@ -604,7 +613,9 @@ ruleTester.run("comma-dangle", rule, {
messageId: "missing",
type: "Property",
line: 3,
column: 12
column: 12,
endLine: 4,
endColumn: 1
}
]
},
Expand All @@ -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]",
Expand Down
13 changes: 9 additions & 4 deletions tests/lib/rules/semi.js
View file
Open in desktop
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
Open in desktop
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