Fix infinite loop in `cast_sign_loss` when peeling unwrap method calls by y21 ·...
source link: https://github.com/rust-lang/rust-clippy/pull/12508
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.
Fixes #12506
The lint wants to peel method calls but didn't actually reassign the expression, leading to an infinite loop.
changelog: Fix infinite loop in [cast_sign_loss] when having two chained .unwrap() calls
added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label
| @@ -134,11 +134,12 @@ fn expr_sign<'cx>(cx: &LateContext<'cx>, expr: &Expr<'_>, ty: impl Into<Option<T | ||
// Peel unwrap(), expect(), etc. |
||
while let Some(&found_name) = METHODS_UNWRAP.iter().find(|&name| &method_name == name) |
||
&& let Some(arglist) = method_chain_args(expr, &[found_name]) |
Member
Author
actually now that I'm taking a closer look at the lint code, this method_chain_args call seems redundant? It doesn't really seem to do anything in this context, except for returning the expression back that was passed in, as far as I can tell.
The line right above it already checks that it's a call to one of the unwrap family of methods.
I removed it locally and the tests are still passing, would be good to have someone else look over this before I actually remove it tho, maybe it's relevant for something else
Contributor
The only additional check it does is whether the method path or any of the rags come from a macro/expansion. If so, the call will return None.
I don't think that should matter in the context of this lint.
Member
Author
do you mean that we should just keep it then? I guess we can. this just seemed odd to me because the function is generally useful for finding method chains, but is being used for a very different purpose here and doesn't actually look for any chains the way it's used (perhaps confusingly, since the loop is what ends up looking for chains) and does a bunch of unnecessary work like building a vec
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK