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

fix: keep parentheses when the precedence of inner expr is lower than the outer one #17187

Merged
merged 2 commits into from May 13, 2024

Conversation

roife
Copy link
Contributor

@roife roife commented May 5, 2024

fix #17185

Additionally, this PR simplifies some code in apply_demorgan.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 5, 2024
@roife
Copy link
Contributor Author

roife commented May 5, 2024

Besides, I'm not sure about the commented-out test:

// FIXME : This needs to go.
// // https://github.com/rust-lang/rust-analyzer/issues/10963
// #[test]
// fn demorgan_doesnt_hang() {
// check_assist(
// apply_demorgan,
// "fn f() { 1 || 3 &&$0 4 || 5 }",
// "fn f() { !(!1 || !3 || !4) || 5 }",
// )
// }

1 || 3 &&$0 4 || 5 should be transformed into 1 || !(!3 || !4) || 5 (just as the assist does currently) instead of !(!1 || !3 || !4) || 5. Perhaps we should update the test and uncomment it.

@alibektas
Copy link
Member

Besides, I'm not sure about the commented-out test:

// FIXME : This needs to go.
// // https://github.com/rust-lang/rust-analyzer/issues/10963
// #[test]
// fn demorgan_doesnt_hang() {
// check_assist(
// apply_demorgan,
// "fn f() { 1 || 3 &&$0 4 || 5 }",
// "fn f() { !(!1 || !3 || !4) || 5 }",
// )
// }

1 || 3 &&$0 4 || 5 should be transformed into 1 || !(!3 || !4) || 5 (just as the assist does currently) instead of !(!1 || !3 || !4) || 5. Perhaps we should update the test and uncomment it.

Yes we can do definitely do that.

@Veykril
Copy link
Member

Veykril commented May 13, 2024

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented May 13, 2024

📌 Commit 4ada826 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented May 13, 2024

⌛ Testing commit 4ada826 with merge 7aecf1a...

@bors
Copy link
Collaborator

bors commented May 13, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 7aecf1a to master...

@bors bors merged commit 7aecf1a into rust-lang:master May 13, 2024
11 checks passed
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.

Apply De Morgan's Law misses precedence of operations
5 participants