tests: add test that roughly ensures that our lint messages conform with the diagnostics convention of the rustc dev guide #6787
Conversation
|
r? @llogiq (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
matthiaskrgr
force-pushed
the
lint_msgs
branch
from
54cdcbf
to
b9d523e
Compare
|
I wondered if we could have some kind of internal lint for this but there are too many different ways how we construct lint messages inside the code base. |
matthiaskrgr
force-pushed
the
lint_msgs
branch
from
b9d523e
to
77c457b
Compare
| r"the compiler unexpectedly panicked. this is a bug.", | ||
| r".*help: I think you meant: .*", | ||
| r"Iterator.* will panic at runtime", | ||
| ]) |
There was a problem hiding this comment.
Can this be more generalized? For Iterator::step_by(0), you could detect Captialized::word. For did you mean unix, you could detect the question mark. You can have a whitelist of proper noun first words like "I", "Intel" or "C-like". For multiple sentences, you can detect . Capitalized...
There was a problem hiding this comment.
Hm. I would rather have a false positive (let's say "Iterator::count() foo bla") trigger a CI failure just to be immediately added to the allowlist than have a false negative slip through because allow-list regex is too general.
There was a problem hiding this comment.
Yeah, I'm also fine with having a hardcoded list. (With my two comments applying to 4 messsages above addressed).
tests/lint_message_convention.rs
Outdated
| let exceptions_set: RegexSet = RegexSet::new(&[ | ||
| r".*error: I see you're using a LinkedList! Perhaps you meant some other data structure?", | ||
| r".*C-like enum variant discriminant is not portable to 32-bit targets", | ||
| r".*Iterator::step_by(0) will panic at runtime", |
There was a problem hiding this comment.
This shouldn't be an exceptions, but in backticks.
tests/lint_message_convention.rs
Outdated
| r".*remove .*the return type...", | ||
| r"note: Clippy version: .*", | ||
| r"the compiler unexpectedly panicked. this is a bug.", | ||
| r".*help: I think you meant: .*", |
There was a problem hiding this comment.
I also don't think we should use the first person in messages.
| r"the compiler unexpectedly panicked. this is a bug.", | ||
| r".*help: I think you meant: .*", | ||
| r"Iterator.* will panic at runtime", | ||
| ]) |
There was a problem hiding this comment.
Yeah, I'm also fine with having a hardcoded list. (With my two comments applying to 4 messsages above addressed).
matthiaskrgr
force-pushed
the
lint_msgs
branch
from
d329344
to
4a1efa1
Compare
|
☔ The latest upstream changes (presumably #6800) made this pull request unmergeable. Please resolve the merge conflicts. |
matthiaskrgr
force-pushed
the
lint_msgs
branch
from
4a1efa1
to
0baf8ab
Compare
|
rebased, I also hope I addressed all the lint review remarks in the |
tests/lint_message_convention.rs
Outdated
| ]) | ||
| .unwrap(); | ||
|
|
||
| // sometimes the first character is capitalized and it is legal (like in "Iterator...") or |
There was a problem hiding this comment.
nit: The comment still has "Iterator", but this is a type which should be in backticks, so better use another example.
|
Apart from a very minuscule nit, this looks good. I like the idea of using the UI tests to check the messages. |
…ith the diagnostics convention of the rustc dev guide lint message should not start with uppercase letters lint messages should not have punctuation at the end of the last line https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-structure The test reads through all the .stderr files in the testsuit and checks lint messages that start with "help: ", "error: " etc. There is also an exception list for special messages that are deemed acceptable. changelog: make sure lint messages conform with the rustc dev guide and add test
matthiaskrgr
force-pushed
the
lint_msgs
branch
from
0baf8ab
to
b53937c
Compare
matthiaskrgr
force-pushed
the
lint_msgs
branch
from
b53937c
to
ebc5c8f
Compare
|
Thank you! @bors r+ |
|
📌 Commit ebc5c8f has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
lint message should not start with uppercase letters
lint messages should not have punctuation at the end of the last line
https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-structure
The test reads through all the .stderr files in the testsuit and checks lint messages that start with "help: ", "error: " etc.
There is also an exception list for special messages that are deemed acceptable.
changelog: make sure lint messages conform with the rustc dev guide and add test