-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
fix: ensure bad #[test]
invocs retain correct AST
#110035
fix: ensure bad #[test]
invocs retain correct AST
#110035
Conversation
r? @davidtwco (rustbot has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me after fixing these two comments
@@ -107,6 +107,36 @@ pub fn expand_test_or_bench( | |||
return vec![]; | |||
} | |||
|
|||
let not_testable_error = |item: Option<&ast::Item>| { | |||
let diag = &cx.sess.parse_sess.span_diagnostic; | |||
let msg = "the `#[test]` attribute may only be used on a non-associated function"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make this translatable? See #100717.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'd prefer not to do this right now. The whole test.rs
file hasn't been converted yet so I feel like that should happen all at once. Also there is some complexity with the error being downgraded to a lint in some cases.
I'm happy to do this if you really want, just not sure it's currently worth the effort.
@davidtwco If you are happy with the untranslated diagnostic, can this be merged? |
@bors r+ |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#109810 (Replace rustdoc-ui/{c,z}-help tests with a stable run-make test ) - rust-lang#110035 (fix: ensure bad `#[test]` invocs retain correct AST) - rust-lang#110089 (sync::mpsc: synchronize receiver disconnect with initialization) - rust-lang#110103 (Report overflows gracefully with new solver) - rust-lang#110122 (Fix x check --stage 1 when download-ci-llvm=false) - rust-lang#110133 (Do not use ImplDerivedObligationCause for inherent impl method error reporting) - rust-lang#110135 (Revert "Don't recover lifetimes/labels containing emojis as character literals") - rust-lang#110235 (Fix `--extend-css` option) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #109816
Ensures that a
StmtKind::Item
doesn't get converted into a plainItem
(causing the ICE from the linked issue) Also unifies the error path a bit.