A few fixes to lints and error messages #17215
Conversation
|
The non-uppercase statics lint was originally warn by default, and was actually switched to allow because some people found it too annoying. (Also, fwiw, it would've been nice to have that change in a separate commit, since it entails a lot of non-interesting mechanical changes, obscuring the 'core' of this PR.) |
|
In fact there's a whole pile of unrelated changes here that should be in separate commits. |
|
OK, I separated the changes into their own commits. |
ftxqxd
force-pushed
the
lints
branch
2 times, most recently
from
717bad7
to
a44fbcb
Compare
|
I just threw in a fix for #17263 because I think bors’s queue is full enough as it is, and it seems to fit in the scope of this PR quite nicely. |
ftxqxd
force-pushed
the
lints
branch
2 times, most recently
from
4e534a8
to
bc5bc11
Compare
|
@P1start this looks great, thanks! As @huonw mentioned there's a lot of changes here, not all of which are entirely related. Can you please separate out changes like the borrowck ones to separate PRs? Also, can you make sure that there are tests for all issues fixed? I was unable to find one for #15657 for example. |
|
I don’t think that #15657 can be tested because it relies on the order errors are output, which can’t be tested the normal way. Or is there some other kind of test which can match on error messages more directly? So how many PRs would this need to be split into? I don’t want to make 11 different pull requests as that would obviously take some time to go through bors, so would something like this work:
? Or would the fixes need to be separated into finer-grained PRs? |
|
I'm personally not super comfortable reviewing borrowck-related modifications, but the misc error messages and lints can probably be grouped together. Sorry for being a little slow to get back to you! |
|
I’ve moved the borrowck-error-related changes into #17434. |
This updates the `unused` lint group to include more lints, updates the `non_snake_case` lint to give better suggestions, adds a note explaining why a lifetime cannot be elided, and tweaks various error messages. This also updates the `non_uppercase_statics` lint to be warn-by-default to match the other naming lints. For statics, this lint is particularly useful, because a non-uppercase static can easily collide with a pattern binding, resulting in very confusing errors. Closes #15914. Closes #15657. Closes #17337.
ftxqxd
force-pushed
the
lints
branch
4 times, most recently
from
12beb0f
to
8e22d4c
Compare
|
Ah, the test that failed always fails locally (among with some others), so I ignored it. :/ Should be fixed now. re-r? |
|
Argh, the rollup made a merge conflict. Rebased, so once more, r? |
This updates the
unusedlint group to include more lints, updates thenon_snake_caselint to give better suggestions, adds a note explaining why a lifetime cannot be elided, and tweaks various error messages. This also updates thenon_uppercase_staticslint to be warn-by-default to match the other naming lints. For statics, this lint is particularly useful, because a non-uppercase static can easily collide with a pattern binding, resulting in very confusing errors.Closes #15914.
Closes #15657.
Closes #17337.