Skip to content
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

Do not remove required parentheses in borrow_as_ptr suggestion #13884

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

samueltardieu
Copy link
Contributor

Also, simplify boolean shortcut expression, and ensure that applicability is properly applied, as it was ignored and MachineApplicable was always used.

changelog: [borrow_as_ptr]: do not remove required parentheses in autofix suggestion

Close #13882

@rustbot
Copy link
Collaborator

rustbot commented Dec 27, 2024

r? @y21

rustbot has assigned @y21.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 27, 2024
@blyxyas
Copy link
Member

blyxyas commented Dec 28, 2024

Timo has a lot of work, I'll reassign to myself the ones with no review yet.

r? blyxyas

@rustbot rustbot assigned blyxyas and unassigned y21 Dec 28, 2024
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a little comment explaining this change? I would just let it pass if there wasn't anything left to do, but we still need to squash the commits into one.

So, just a mini-nit.

let operator_kind = match mutability {
Mutability::Not => "const",
Mutability::Mut => "mut",
};
format!("&raw {operator_kind} {snip}")
let span = if has_enclosing_paren(snippet_with_applicability(cx, expr.span, "", &mut app)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let span = if has_enclosing_paren(snippet_with_applicability(cx, expr.span, "", &mut app)) {
// Make sure that the span to be replaced doesn't include parenthesis, that could break the suggestion.
let span = if has_enclosing_paren(snippet_with_applicability(cx, expr.span, "", &mut app)) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

borrow-as-ptr eats parenthesis which break code
4 participants