Skip to content

Commit 107ceb8

Browse files
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 ff4a26d commit 107ceb8

File tree

5 files changed

+109
-3
lines changed

5 files changed

+109
-3
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: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
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::{get_parent_expr, is_trait_item, is_trait_method, is_ty_alias, path_to_local};
66
use rustc_errors::Applicability;
77
use rustc_hir::def_id::DefId;
88
use rustc_hir::{BindingMode, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
@@ -382,3 +382,26 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
382382
}
383383
}
384384
}
385+
386+
/// Check if `arg` is a `Into::into` or `From::from` applied to `receiver` to give `expr`.
387+
pub fn check_function_application(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, arg: &Expr<'_>) {
388+
let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
389+
if same_type_and_consts(expr_ty, cx.typeck_results().expr_ty_adjusted(receiver))
390+
&& (is_trait_item(cx, arg, sym::Into) || is_trait_item(cx, arg, sym::From))
391+
{
392+
span_lint_and_then(
393+
cx,
394+
USELESS_CONVERSION,
395+
expr.span.with_lo(receiver.span.hi()),
396+
format!("useless conversion to the same type: `{expr_ty}`"),
397+
|diag| {
398+
diag.suggest_remove_item(
399+
cx,
400+
expr.span.with_lo(receiver.span.hi()),
401+
"consider removing",
402+
Applicability::MachineApplicable,
403+
);
404+
},
405+
);
406+
}
407+
}

tests/ui/useless_conversion.fixed

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,3 +297,32 @@ 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+
}

tests/ui/useless_conversion.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,3 +297,32 @@ 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+
}

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: `std::result::Result<(), std::io::Error>`
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::result::Result<(), 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: `std::result::Result<(), std::io::Error>`
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::result::Result<(), 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)