-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Be more precise when suggesting removal of parens on unit ctor #99350
Conversation
compiler-errors
commented
Jul 16, 2022
- Fixes Diagnostic misclassifies a complex expression as a unit enum variant when flagging an attempt to call a non-function #99240 by only suggesting to remove parens on path exprs, not arbitrary expressions with enum type
- Generalizes by suggesting removal of parens on unit struct, too, because why not?
r? @fee1-dead (rust-highfive has picked a reviewer for you, use r? to override) |
= self.typeck_results.borrow().qpath_res(qpath, callee_expr.hir_id) | ||
// Only suggest removing parens if there are no arguments | ||
&& arg_exprs.is_empty() | ||
&& let Ok(path) = self.tcx.sess.source_map().span_to_snippet(callee_expr.span) |
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.
I would like to use tcx.def_path_str
instead of span_to_snippet
, but that does not preserve aliases, and causes regressions in src/test/ui/type-alias-enum-variants/incorrect-variant-form-through-alias-caught.rs
by changing Alias::Unit
to Enum::Unit
😢
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.
I could possibly find a way to format qpath
into a string by joining all of its segments, but that's a bit overkill and this PR does not represent a regression in the status quo.
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.
It wouldn't work well with improperly formatted code, right? If the path is a macro expansion what would happen..? To me I don't think it should preserve the alias path.
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.
Hm, you're right. If we had like
path::
split::
up::
over_many_lines
then we shouldn't use that in diagnostics.
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.
I just used rustc_hir_pretty
which does hir -> string for us. Avoids weirdness like:
enum Enum { Braced {} }
type Alias = Enum;
fn main() {
Alias::
Braced;
}
error[[E0533]](https://doc.rust-lang.org/stable/error-index.html#E0533): expected unit struct, unit variant or constant, found struct variant `Alias::
Braced`
--> src/main.rs:5:5
|
5 | / Alias::
6 | | Braced;
| |__________^
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.
Nice, please add that as a test.
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.
Done
e678354
to
7a45a60
Compare
@bors r+ |
…-dead Be more precise when suggesting removal of parens on unit ctor * Fixes rust-lang#99240 by only suggesting to remove parens on path exprs, not arbitrary expressions with enum type * Generalizes by suggesting removal of parens on unit struct, too, because why not?
Rollup of 6 pull requests Successful merges: - rust-lang#98383 (Remove restrictions on compare-exchange memory ordering.) - rust-lang#99350 (Be more precise when suggesting removal of parens on unit ctor) - rust-lang#99356 (Do not constraint TAITs when checking impl/trait item compatibility) - rust-lang#99360 (Do not ICE when we have `-Zunpretty=expanded` with invalid ABI) - rust-lang#99373 (Fix source code sidebar tree auto-expand) - rust-lang#99374 (Fix doc for `rchunks_exact`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup