trigger obfuscated_if_else for .then(..).unwrap_or(..)
#14021
Conversation
rustbot
added
the
S-waiting-on-review
lapla-cogito
force-pushed
the
obfusifelse_then_unwrapor
branch
from
0fb2747 to
4eb060d
Compare
|
|
||
| // Remove the enclosing parentheses if they exist | ||
| // e.g. "(a == 1)" -> "a == 1" | ||
| fn remove_needless_parentheses(s: &str) -> String { |
There was a problem hiding this comment.
In the first implementation, I noticed that some suggestions may contain needless parentheses (see this lintcheck run). Hence I implemented this.
There was a problem hiding this comment.
Can't you work with Sugg (from clippy_utils/src/sugg.rs) which already manipulates parentheses, instead of string snippets, starting from HIR nodes?
lapla-cogito
force-pushed
the
obfusifelse_then_unwrapor
branch
2 times, most recently
from
55f96f9 to
52e688c
Compare
lapla-cogito
force-pushed
the
obfusifelse_then_unwrapor
branch
from
52e688c to
ef52d42
Compare
|
Also I'm willing to implement for other patterns that should be covered by |
|
|
||
| // Remove the enclosing parentheses if they exist | ||
| // e.g. "(a == 1)" -> "a == 1" | ||
| fn remove_needless_parentheses(s: &str) -> String { |
There was a problem hiding this comment.
Can't you work with Sugg (from clippy_utils/src/sugg.rs) which already manipulates parentheses, instead of string snippets, starting from HIR nodes?
| fn remove_needless_parentheses(s: &str) -> String { | ||
| if has_enclosing_paren(s) { | ||
| s[1..s.len() - 1].to_string() | ||
| } else { |
There was a problem hiding this comment.
Even if you didn't use Sugg, you could return a &str instead of allocating a String
| --> tests/ui/obfuscated_if_else.rs:9:5 | ||
| | | ||
| LL | (a == 1).then_some("a").unwrap_or("b"); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if a == 1 { "a" } else { "b" }` |
There was a problem hiding this comment.
You should check for side effects, as the arguments to then_some() and unwrap_or() will always be evaluated, while the one in then and else clauses will not.
part of #9100
The
obfuscated_if_elselint currently only triggers for the pattern.then_some(..).unwrap_or(..), but there're other cases where this lint should be triggered, one of which is.then(..).unwrap_or(..).changelog: [
obfuscated_if_else]: trigger lint for the.then(..).unwrap_or(..)pattern as well