-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Dots at the end of many messages are omitted #6762
Comments
As I noted in another issue, be careful to check this with a formatter that isn't "stylish", which does strip periods from all messages. (The ESLint demo is a sufficient tool for checking the error messages, as well as most if not all of the other formatters.) |
What do you mean? Can't we just check the source? If we want to act on it, then I would prefer to add |
Looking it from the other perspective we can just remove the dots from all the rules and re-add them with reporter, this would be an opposite of the That way, we wouldn't need to "remember" check those dots when reviewing new PRs |
With this path though, we would need to check for absence of those dots :), but I guess reporters could check for their presence, but that wouldn't work for custom reporters then, doesn't sound like a perfect solution |
How about updating the reporter to add the dot if and only if it's missing? That way 3rd party rules would also be normalized. (Not sure if that's a big or a feature. :)) |
@pmcelhaney it still be inconsistent in the sources though. btw, @wavebeem do you want to get in on this? |
Yeah. catching manually in PR < fixing at runtime < catching at build time < catching and fixing with a custom ESLint rule Just not sure what's possible at this point. |
Okay, maybe a check for proper formatting can be added to rule-tester.js. https://github.com/eslint/eslint/blob/master/lib/testers/rule-tester.js#L407 Is there a way to add that check only to this repo but not affect third-party rules that doesn't affect the spirit of rules being independent? |
Hmm, normalizing the presence/absence of |
We could add a custom internal rule for checking that the last dot is
|
I messed around with adding another if (messages[i].message) {
assert.equal(messages[i].message.slice(-1), ".", messages[i].message);
} And found the following messages. If there's an interest, I may go through and normalize all of the messages in the source code this weekend.
|
You mean custom ESLint rule? I'm not sure if that is possible for static analysis tool, for example, you would need to deal with cases like that - eslint/lib/rules/array-bracket-spacing.js Line 78 in ddea63a
I think this is our best bet, it shouldn't reflect on rules which are not part of our repo, so we wouldn't need to bump major or anything like that |
@markelog The change to rule-tester.js is simple, but how do we prevent it from affecting third party rules? Don't they use rule-tester.js in their tests? I was thinking maybe add a flag to package.json, so that it only affects the ESLint repo, but plugins can opt in to enforcing the strict message style if they want. |
Wow, you right indeed,
Seems a bit radical. Or we can add additional option to it signature like |
Yeah, a flag that would have to be set every time RuleTester is constructed would defeat the purpose. So maybe add a property to the constructor itself? RuleTester.isValidMessageFormat = function (message) {
return message.slice(-1) === ".";
} |
I know that keeping all of the error messages in the same style is important, but is it important enough to add something like that to RuleTester, or create additional complexity in some other way? |
Maybe not important per se, but I think it would be a good thing, from my perspective though not at this point of time, I think in scope of this ticket we can just add periods and probably continue this discussion at 4.0 time, sounds good? |
That sounds like a good plan to me. |
Okay, cool. @pmcelhaney, @wavebeem would any of you want to help with this one? |
Sure, I'll do it this weekend. |
Be my guest; my weekend is looking a little busy. Thanks. On Fri, Jul 29, 2016 at 2:15 PM, Patrick McElhaney <notifications@github.com
|
What version of ESLint are you using?
3.1.1
What parser (default, Babel-ESLint, etc.) are you using?
default
Please show your full configuration:
Example for
array-bracket-spacing
but this issue is not limited to this rule only -What did you do? Please include the actual source code causing the issue.
What did you expect to happen?
Have a dot at the end -
1:11 error There should be no space after '['. array-bracket-spacing
What actually happened? Please include the actual, raw output from ESLint.
It doesn't have a dot at the end -
1:11 error There should be no space after '[' array-bracket-spacing
I'm not sure if this is intentional or not, every message should have a dot at the end or there is a different policy in place? If so, I think it should be documented.
Beside that, if this is really a "bug" I think every rule should be fixed with one commit in order not to trash the commit history
The text was updated successfully, but these errors were encountered: