fix FP in lint `[needless_match]` by J-ZhengLi · Pull Request #8549 · rust-lang/...
source link: https://github.com/rust-lang/rust-clippy/pull/8549
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
New issue
fix FP in lint [needless_match]
#8549
Conversation
r? @giraffate
(rust-highfive has picked a reviewer for you, use r? to override)
added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label
I remove the ref x => *x match arm check as it's logic seems problematic, I was already unsure about whether I should or not including it when I add the lint. Right now, removing it does seem to be a quick fix of what @teor2345 mentioned in #8542 (comment) , I just don't see the need to add it back, but maybe I'll do it when someone actually suggesting it. 
/// Check if the type of the match operand differs with the assigned local or function return type
///
/// Can also be used to check for the `let_expr` in `IfLet` as well.
fn is_type_differ(cx: &LateContext<'_>, match_expr: &Expr<'_>, expr: &Expr<'_>) -> bool {
if let Some(p_node) = get_parent_node(cx.tcx, expr.hir_id) {
I'm not so sure about if this is the correct way to check for type coercion, I basically just check for the assigned variable/function return type to see if those are the same as match expression.
Although I saw methods like TypeckResult::is_coercion_cast or something, but I don't think those work in these case.
We have clippy_utils::is_adjusted(cx, expr), which should do the trick more easily.
We have
clippy_utils::is_adjusted(cx, expr), which should do the trick more easily.
Well... I later realized that I might still need to keep it this way, because I do need to make sure that the type of match input and output are the same, not just coercion but also as what #8599 suggested. unless there is another util function that I can use to do the check.
It's kinda harsh seeing my mistakes being addressed multiple times tbh 
, so if anyone is not busy at the moment (sorry to bother if you are), please help me reviewing this, thanks~~~
edit: Oh... here are some 'ol little pings @giraffate @llogiq
This looks good, I just have a very small suggestion. r=me when applied.
@@ -171,27 +191,30 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
false
}
fn has_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
fn same_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool {
I think we have an over function for such things skmewhere in clippy_utils.
Commit 85b081b has been approved by llogiq
Testing commit Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK