-
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
Suggest 1-tuple parentheses on exprs without existing parens #91530
Conversation
r? @nagisa (rust-highfive has picked a reviewer for you, use r? to override) |
r? @camelid |
This comment has been minimized.
This comment has been minimized.
Hmm, I guess the reason this didn't use a multipart suggestion was in case there were already parentheses. |
Yeah I guess that's the trouble. I think the multipart suggestion is nicer, particularly for giving tools a minimal diff to work with rather than pasting the whole sub-expression over the top. I'd like to keep the multipart suggestion - would you be ok with something like this? if code.starts_with('(') && code.ends_with(')') {
<original span_suggestion>
} else {
<multipart_suggestion>
} |
What about this? if code.starts_with('(') && code.ends_with(')') {
<suggest adding a comma at span.with_hi(span.hi - 1)>
} else {
<multipart suggestion>
} |
☔ The latest upstream changes (presumably #93119) made this pull request unmergeable. Please resolve the merge conflicts. |
(holding off on this PR until its sibling #90677 is all good) |
d8f4d01
to
a57f345
Compare
Multi-part suggestion and comma insertion sorted, happy for another review |
This comment has been minimized.
This comment has been minimized.
a57f345
to
72d3b45
Compare
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.
Looks great, just a couple comments!
This comment has been minimized.
This comment has been minimized.
ed70f7a
to
4607c33
Compare
4607c33
to
e56bb6f
Compare
This comment has been minimized.
This comment has been minimized.
e56bb6f
to
82a0122
Compare
Thanks again! @bors r+ |
📌 Commit 82a0122 has been approved by |
… r=camelid Suggest 1-tuple parentheses on exprs without existing parens A follow-on from rust-lang#86116, split out from rust-lang#90677. This alters the suggestion to add a trailing comma to create a 1-tuple - previously we would only apply this if the relevant expression was parenthesised. We now make the suggestion regardless of parentheses, which reduces the fragility of the check (w.r.t formatting). e.g. ```rust let a: Option<(i32,)> = Some(3); ``` gets the below suggestion: ```rust let a: Option<(i32,)> = Some((3,)); // ^ ^^ ``` This change also improves the suggestion in other ways, such as by only making the suggestion if the types would match after the suggestion is applied and making the suggestion a multipart suggestion.
… r=camelid Suggest 1-tuple parentheses on exprs without existing parens A follow-on from rust-lang#86116, split out from rust-lang#90677. This alters the suggestion to add a trailing comma to create a 1-tuple - previously we would only apply this if the relevant expression was parenthesised. We now make the suggestion regardless of parentheses, which reduces the fragility of the check (w.r.t formatting). e.g. ```rust let a: Option<(i32,)> = Some(3); ``` gets the below suggestion: ```rust let a: Option<(i32,)> = Some((3,)); // ^ ^^ ``` This change also improves the suggestion in other ways, such as by only making the suggestion if the types would match after the suggestion is applied and making the suggestion a multipart suggestion.
Rollup of 13 pull requests Successful merges: - rust-lang#88313 (Make the pre-commit script pre-push instead) - rust-lang#91530 (Suggest 1-tuple parentheses on exprs without existing parens) - rust-lang#92724 (Cleanup c_str.rs) - rust-lang#93208 (Impl {Add,Sub,Mul,Div,Rem,BitXor,BitOr,BitAnd}Assign<$t> for Wrapping<$t> for rust 1.60.0) - rust-lang#93394 (Don't allow {} to refer to implicit captures in format_args.) - rust-lang#93416 (remove `allow_fail` test flag) - rust-lang#93487 (Fix linking stage1 toolchain in `./x.py setup`) - rust-lang#93673 (Linkify sidebar headings for sibling items) - rust-lang#93680 (Drop json::from_reader) - rust-lang#93682 (Update tracking issue for `const_fn_trait_bound`) - rust-lang#93722 (Use shallow clones for submodules managed by rustbuild, not just bootstrap.py) - rust-lang#93723 (Rerun bootstrap's build script when RUSTC changes) - rust-lang#93737 (bootstrap: prefer using '--config' over 'RUST_BOOTSTRAP_CONFIG') Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
A follow-on from #86116, split out from #90677.
This alters the suggestion to add a trailing comma to create a 1-tuple - previously we would only apply this if the relevant expression was parenthesised. We now make the suggestion regardless of parentheses, which reduces the fragility of the check (w.r.t formatting).
e.g.
gets the below suggestion:
This change also improves the suggestion in other ways, such as by only making the suggestion if the types would match after the suggestion is applied and making the suggestion a multipart suggestion.