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.
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
Return range in the lint result #3307
Comments
Thanks for the issue! We get a lot of issues, so this message is automatically posted to each one to help you check that you've included all of the information we need to help you. Reporting a bug? Please be sure to include:
Requesting a new rule? Please be sure to include:
Requesting a feature? Please be sure to include:
Including this information in your issue helps us to triage it and get you a response as quickly as possible. Thanks! |
👍 However, I guess it shows unexpected ranges if semicolons are omitted. |
I think this would be great to add. Unfortunately, it requires manually modifying every single rule we have (or at least most of them). |
Yeah, this would be a significant change (need to update every rule, plus change the signature of @sindresorhus if only a handful of rules returned range information, would that be useful to you? |
@nzakas We could probably modify it so that any time we use |
You don't have to assume that though. How about adding a
Yes, even just a few rules supporting it would improve the UI for editor linters. |
OK, I'm not sure we can commit to this at this point. Right now, each message contains the exact same data, and it's hard to tell what effect having different data on different messages would have in the ecosystem. |
I marked this as backlog, because we can't commit to this right now and put it on the roadmap, but I think ideally, this is something we want to do at some point. |
👍 |
I will work on this. My plan is that I add {
fatal: false,
severity: 2,
ruleId: "semi",
line: 1,
column: 23,
lineEnd: 1,
columnEnd: 23,
message: "Expected a semicolon.",
fix: {
range: [1, 15],
text: ";"
}
} Those properties are calculated from the arguments of var line, column, lineEnd, columnEnd;
var loc = params.loc || params.node.loc.start;
if (loc.end) {
line = loc.start.line;
column = loc.start.column;
lineEnd = loc.end.line;
columnEnd = loc.end.column;
} else {
line = loc.line;
column = loc.column;
} For Example: // Pattern 1
context.report({
node: node,
loc: token.loc, // lineEnd = token.loc.end.line;
message: "test"
});
// Pattern 2
context.report({
node: node,
loc: token.loc.start, // lineEnd is undefined;
message: "test"
});
// Pattern 3
context.report({
node: node, // lineEnd is undefined;
message: "test"
}); In short, the current code of core rules and plugins don't make This is in order to keep compatibility of editors. @eslint/eslint-team I want to ask opinions. |
var loc = params.loc || params.node.loc.start; This wouldn't work if only node is passed in, but I get the general idea. 👍 from me |
Sounds good to me, too. |
@mysticatea was lineEnd added? |
The implement of this issue has done in #6640. |
Thanks to @IanVS's brilliant idea to actually search, it looks like at least |
Yes, only |
@mysticatea if you want to keep this issue open, can you make a list of rules to that need to be addressed? |
Is this still being worked on? |
Yes, I will do. I'm sorry, I'm busy for now. I think I can get time within the next month. |
I think this is basically fixed now that b0c63f0 has been merged, so I'm closing this issue. |
* Remove bypass for eslint/eslint#3307 * Update tests to match updated error ranges * Update browserified eslint to v4.5.0 for codemirror demo page * Update travis configuration to match eslint ^4.0.0
Currently ESLint returns the
line
andcolumn
when linting. However, with editor plugins it would be useful to get the full range so instead of just visually showing where the violation starts it could mark the whole range of the violation.For example, here the Atomlinter ESLint plugin could use the range instead of just marking the first character.
Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.
The text was updated successfully, but these errors were encountered: