You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is analogous to already stable lint option_as_ref_deref. It should suggest using Result::as_deref[_mut] instead of Result::as_ref followed by Result::map(fn preforming deref).
I was quite surprised that this wasn't already implemented. I thought that option_as_ref_deref covered generic pattern of as_ref().map(Deref::deref) and was not limited only to Option type. Adding this lint would not really be adding a new "feature", and rather it would close a gap in current "implementation".
Advantage
Advantages are the same as with option_as_ref_deref lint.
Drawbacks
Other than adding a new lint, I think than naming is quite unfortunate. I personally would prefer one lint (for example called manual_as_deref) which would check both Option and Result, rather than two separate ones. However it is probably to late for that, since extending current lint to Result would be really misleading as lint's name refers to Option. And changing lint name would be to breaking.
Example
let x:Result<String, !> = todo!();let y:&str = x.as_ref().map(String::as_str).unwrap_or("foo");
Could be written as:
let x:Result<String, !> = todo!();let y:&str = x.as_deref().unwrap_or("foo");
The text was updated successfully, but these errors were encountered:
If this proposition is accepted, I would like to volunteer myself to implement it. I haven't contributed to clippy yet, but I would like to. And I think it won't be to hard, given that option_as_ref_deref is already implemented, so implementing this lint should be just extending existing code.
Feel free to go ahead. New lints formally go through a short period to see if there are any objections. We've been starting this when PR's are made, but I don't see any problems since it's basically the same as an existing lint.
This might be worth renaming option_as_ref_deref and handling both under the same lint. e.g. manual_fallible_as_deref.
What it does
This is analogous to already stable lint
option_as_ref_deref
. It should suggest usingResult::as_deref[_mut]
instead ofResult::as_ref
followed byResult::map(fn preforming deref)
.I was quite surprised that this wasn't already implemented. I thought that
option_as_ref_deref
covered generic pattern ofas_ref().map(Deref::deref)
and was not limited only toOption
type. Adding this lint would not really be adding a new "feature", and rather it would close a gap in current "implementation".Advantage
Advantages are the same as with
option_as_ref_deref
lint.Drawbacks
Other than adding a new lint, I think than naming is quite unfortunate. I personally would prefer one lint (for example called
manual_as_deref
) which would check bothOption
andResult
, rather than two separate ones. However it is probably to late for that, since extending current lint toResult
would be really misleading as lint's name refers toOption
. And changing lint name would be to breaking.Example
Could be written as:
The text was updated successfully, but these errors were encountered: