Skip to content

Commit 9a692ec

Browse files
samueltardieusamueltardieu
committed
Add more cases to the useless_conversion lint
The new cases are `x.map(f)` and `x.map_err(f)` when `f` is `Into::into` or `From::from` with the same input and output types.
1 parent 650e0c8 commit 9a692ec

File tree

5 files changed

+129
-4
lines changed

5 files changed

+129
-4
lines changed

‎clippy_lints/src/methods/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4982,6 +4982,7 @@ impl Methods {
49824982
}
49834983
map_identity::check(cx, expr, recv, m_arg, name, span);
49844984
manual_inspect::check(cx, expr, m_arg, name, span, &self.msrv);
4985+
crate::useless_conversion::check_function_application(cx, expr, recv, m_arg);
49854986
},
49864987
("map_or", [def, map]) => {
49874988
option_map_or_none::check(cx, expr, recv, def, map);

‎clippy_lints/src/useless_conversion.rs

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
11
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
22
use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context};
3-
use clippy_utils::sugg::Sugg;
3+
use clippy_utils::sugg::{DiagExt as _, Sugg};
44
use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
5-
use clippy_utils::{get_parent_expr, is_trait_method, is_ty_alias, path_to_local};
5+
use clippy_utils::{
6+
get_parent_expr, is_inherent_method_call, is_trait_item, is_trait_method, is_ty_alias, path_to_local,
7+
};
68
use rustc_errors::Applicability;
79
use rustc_hir::def_id::DefId;
810
use rustc_hir::{BindingMode, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
911
use rustc_infer::infer::TyCtxtInferExt;
1012
use rustc_infer::traits::Obligation;
1113
use rustc_lint::{LateContext, LateLintPass};
1214
use rustc_middle::traits::ObligationCause;
13-
use rustc_middle::ty::{self, EarlyBinder, GenericArg, GenericArgsRef, Ty, TypeVisitableExt};
15+
use rustc_middle::ty::{self, AdtDef, EarlyBinder, GenericArg, GenericArgsRef, Ty, TypeVisitableExt};
1416
use rustc_session::impl_lint_pass;
1517
use rustc_span::{Span, sym};
1618
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
@@ -382,3 +384,41 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
382384
}
383385
}
384386
}
387+
388+
/// Check if `arg` is a `Into::into` or `From::from` applied to `receiver` to give `expr`, through a
389+
/// higher-order mapping function.
390+
pub fn check_function_application(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>) {
391+
if has_eligible_receiver(cx, recv, expr)
392+
&& (is_trait_item(cx, arg, sym::Into) || is_trait_item(cx, arg, sym::From))
393+
&& let ty::FnDef(_, args) = cx.typeck_results().expr_ty(arg).kind()
394+
&& let &[from_ty, to_ty] = args.into_type_list(cx.tcx).as_slice()
395+
&& same_type_and_consts(from_ty, to_ty)
396+
{
397+
span_lint_and_then(
398+
cx,
399+
USELESS_CONVERSION,
400+
expr.span.with_lo(recv.span.hi()),
401+
format!("useless conversion to the same type: `{from_ty}`"),
402+
|diag| {
403+
diag.suggest_remove_item(
404+
cx,
405+
expr.span.with_lo(recv.span.hi()),
406+
"consider removing",
407+
Applicability::MachineApplicable,
408+
);
409+
},
410+
);
411+
}
412+
}
413+
414+
fn has_eligible_receiver(cx: &LateContext<'_>, recv: &Expr<'_>, expr: &Expr<'_>) -> bool {
415+
let recv_ty = cx.typeck_results().expr_ty(recv);
416+
if is_inherent_method_call(cx, expr)
417+
&& let Some(recv_ty_defid) = recv_ty.ty_adt_def().map(AdtDef::did)
418+
&& let Some(diag_name) = cx.tcx.get_diagnostic_name(recv_ty_defid)
419+
&& matches!(diag_name, sym::Option | sym::Result)
420+
{
421+
return true;
422+
}
423+
false
424+
}

‎tests/ui/useless_conversion.fixed

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,3 +297,33 @@ impl From<Foo<'a'>> for Foo<'b'> {
297297
Foo
298298
}
299299
}
300+
301+
fn direct_application() {
302+
let _: Result<(), std::io::Error> = test_issue_3913();
303+
//~^ useless_conversion
304+
let _: Result<(), std::io::Error> = test_issue_3913();
305+
//~^ useless_conversion
306+
let _: Result<(), std::io::Error> = test_issue_3913();
307+
//~^ useless_conversion
308+
let _: Result<(), std::io::Error> = test_issue_3913();
309+
//~^ useless_conversion
310+
311+
struct Absorb;
312+
impl From<()> for Absorb {
313+
fn from(_: ()) -> Self {
314+
Self
315+
}
316+
}
317+
impl From<std::io::Error> for Absorb {
318+
fn from(_: std::io::Error) -> Self {
319+
Self
320+
}
321+
}
322+
323+
// No lint for those
324+
let _: Result<Absorb, std::io::Error> = test_issue_3913().map(Into::into);
325+
let _: Result<(), Absorb> = test_issue_3913().map_err(Into::into);
326+
let _: Result<Absorb, std::io::Error> = test_issue_3913().map(From::from);
327+
let _: Result<(), Absorb> = test_issue_3913().map_err(From::from);
328+
let _: Vec<u32> = [1u32].into_iter().map(Into::into).collect();
329+
}

‎tests/ui/useless_conversion.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,3 +297,33 @@ impl From<Foo<'a'>> for Foo<'b'> {
297297
Foo
298298
}
299299
}
300+
301+
fn direct_application() {
302+
let _: Result<(), std::io::Error> = test_issue_3913().map(Into::into);
303+
//~^ useless_conversion
304+
let _: Result<(), std::io::Error> = test_issue_3913().map_err(Into::into);
305+
//~^ useless_conversion
306+
let _: Result<(), std::io::Error> = test_issue_3913().map(From::from);
307+
//~^ useless_conversion
308+
let _: Result<(), std::io::Error> = test_issue_3913().map_err(From::from);
309+
//~^ useless_conversion
310+
311+
struct Absorb;
312+
impl From<()> for Absorb {
313+
fn from(_: ()) -> Self {
314+
Self
315+
}
316+
}
317+
impl From<std::io::Error> for Absorb {
318+
fn from(_: std::io::Error) -> Self {
319+
Self
320+
}
321+
}
322+
323+
// No lint for those
324+
let _: Result<Absorb, std::io::Error> = test_issue_3913().map(Into::into);
325+
let _: Result<(), Absorb> = test_issue_3913().map_err(Into::into);
326+
let _: Result<Absorb, std::io::Error> = test_issue_3913().map(From::from);
327+
let _: Result<(), Absorb> = test_issue_3913().map_err(From::from);
328+
let _: Vec<u32> = [1u32].into_iter().map(Into::into).collect();
329+
}

‎tests/ui/useless_conversion.stderr

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,5 +226,29 @@ note: this parameter accepts any `IntoIterator`, so you don't need to call `.int
226226
LL | J: IntoIterator,
227227
| ^^^^^^^^^^^^
228228

229-
error: aborting due to 28 previous errors
229+
error: useless conversion to the same type: `()`
230+
--> tests/ui/useless_conversion.rs:302:58
231+
|
232+
LL | let _: Result<(), std::io::Error> = test_issue_3913().map(Into::into);
233+
| ^^^^^^^^^^^^^^^^ help: consider removing
234+
235+
error: useless conversion to the same type: `std::io::Error`
236+
--> tests/ui/useless_conversion.rs:304:58
237+
|
238+
LL | let _: Result<(), std::io::Error> = test_issue_3913().map_err(Into::into);
239+
| ^^^^^^^^^^^^^^^^^^^^ help: consider removing
240+
241+
error: useless conversion to the same type: `()`
242+
--> tests/ui/useless_conversion.rs:306:58
243+
|
244+
LL | let _: Result<(), std::io::Error> = test_issue_3913().map(From::from);
245+
| ^^^^^^^^^^^^^^^^ help: consider removing
246+
247+
error: useless conversion to the same type: `std::io::Error`
248+
--> tests/ui/useless_conversion.rs:308:58
249+
|
250+
LL | let _: Result<(), std::io::Error> = test_issue_3913().map_err(From::from);
251+
| ^^^^^^^^^^^^^^^^^^^^ help: consider removing
252+
253+
error: aborting due to 32 previous errors
230254

0 commit comments

Comments
 (0)