dnrops
/
rust-clippy
mirror of https://github.com/rust-lang/rust-clippy.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

new lint: `unnecessary_fallible_conversions`

This commit is contained in:
y21 2023-10-15 00:57:33 +02:00
parent cdc4d56ae7
commit 69c3b9c252
14 changed files with 291 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -5528,6 +5528,7 @@ Released 2018-09-13
[`unknown_clippy_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints
[`unnecessary_box_returns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns
[`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
[`unnecessary_fallible_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions
[`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map
[`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map
[`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold

1
clippy_lints/src/declared_lints.rs
View File

@ -430,6 +430,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::TYPE_ID_ON_BOX_INFO,
crate::methods::UNINIT_ASSUMED_INIT_INFO,
crate::methods::UNIT_HASH_INFO,
crate::methods::UNNECESSARY_FALLIBLE_CONVERSIONS_INFO,
crate::methods::UNNECESSARY_FILTER_MAP_INFO,
crate::methods::UNNECESSARY_FIND_MAP_INFO,
crate::methods::UNNECESSARY_FOLD_INFO,

24
clippy_lints/src/implicit_saturating_add.rs
View File

@ -82,18 +82,18 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingAdd {
fn get_int_max(ty: Ty<'_>) -> Option<u128> {
match ty.peel_refs().kind() {
Int(IntTy::I8) => i8::max_value().try_into().ok(),
Int(IntTy::I16) => i16::max_value().try_into().ok(),
Int(IntTy::I32) => i32::max_value().try_into().ok(),
Int(IntTy::I64) => i64::max_value().try_into().ok(),
Int(IntTy::I128) => i128::max_value().try_into().ok(),
Int(IntTy::Isize) => isize::max_value().try_into().ok(),
Uint(UintTy::U8) => u8::max_value().try_into().ok(),
Uint(UintTy::U16) => u16::max_value().try_into().ok(),
Uint(UintTy::U32) => u32::max_value().try_into().ok(),
Uint(UintTy::U64) => u64::max_value().try_into().ok(),
Uint(UintTy::U128) => Some(u128::max_value()),
Uint(UintTy::Usize) => usize::max_value().try_into().ok(),
Int(IntTy::I8) => i8::MAX.try_into().ok(),
Int(IntTy::I16) => i16::MAX.try_into().ok(),
Int(IntTy::I32) => i32::MAX.try_into().ok(),
Int(IntTy::I64) => i64::MAX.try_into().ok(),
Int(IntTy::I128) => i128::MAX.try_into().ok(),
Int(IntTy::Isize) => isize::MAX.try_into().ok(),
Uint(UintTy::U8) => Some(u8::MAX.into()),
Uint(UintTy::U16) => Some(u16::MAX.into()),
Uint(UintTy::U32) => Some(u32::MAX.into()),
Uint(UintTy::U64) => Some(u64::MAX.into()),
Uint(UintTy::U128) => Some(u128::MAX),
Uint(UintTy::Usize) => usize::MAX.try_into().ok(),
_ => None,
}
}

33
clippy_lints/src/methods/mod.rs
View File

@ -99,6 +99,7 @@ mod suspicious_to_owned;
mod type_id_on_box;
mod uninit_assumed_init;
mod unit_hash;
mod unnecessary_fallible_conversions;
mod unnecessary_filter_map;
mod unnecessary_fold;
mod unnecessary_iter_cloned;
@ -3655,6 +3656,33 @@ declare_clippy_lint! {
"cloning a `Waker` only to wake it"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for calls to `TryInto::try_into` and `TryFrom::try_from` when their infallible counterparts
/// could be used.
///
/// ### Why is this bad?
/// In those cases, the `TryInto` and `TryFrom` trait implementation is a blanket impl that forwards
/// to `Into` or `From`, which always succeeds.
/// The returned `Result<_, Infallible>` requires error handling to get the contained value
/// even though the conversion can never fail.
///
/// ### Example
/// ```rust
/// let _: Result<i64, _> = 1i32.try_into();
/// let _: Result<i64, _> = <_>::try_from(1i32);
/// ```
/// Use `from`/`into` instead:
/// ```rust
/// let _: i64 = 1i32.into();
/// let _: i64 = <_>::from(1i32);
/// ```
#[clippy::version = "1.75.0"]
pub UNNECESSARY_FALLIBLE_CONVERSIONS,
style,
"calling the `try_from` and `try_into` trait methods when `From`/`Into` is implemented"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
@ -3801,6 +3829,7 @@ impl_lint_pass!(Methods => [
PATH_ENDS_WITH_EXT,
REDUNDANT_AS_STR,
WAKER_CLONE_WAKE,
UNNECESSARY_FALLIBLE_CONVERSIONS,
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -3827,6 +3856,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
match expr.kind {
hir::ExprKind::Call(func, args) => {
from_iter_instead_of_collect::check(cx, expr, args, func);
unnecessary_fallible_conversions::check_function(cx, expr, func);
},
hir::ExprKind::MethodCall(method_call, receiver, args, _) => {
let method_span = method_call.ident.span;
@ -4316,6 +4346,9 @@ impl Methods {
}
unnecessary_lazy_eval::check(cx, expr, recv, arg, "then_some");
},
("try_into", []) if is_trait_method(cx, expr, sym::TryInto) => {
unnecessary_fallible_conversions::check_method(cx, expr);
}
("to_owned", []) => {
if !suspicious_to_owned::check(cx, expr, recv) {
implicit_clone::check(cx, name, expr, recv);

119
clippy_lints/src/methods/unnecessary_fallible_conversions.rs Normal file
View File

@ -0,0 +1,119 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::get_parent_expr;
use clippy_utils::ty::implements_trait;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::{sym, Span};
use super::UNNECESSARY_FALLIBLE_CONVERSIONS;
/// What function is being called and whether that call is written as a method call or a function
/// call
#[derive(Copy, Clone)]
#[expect(clippy::enum_variant_names)]
enum FunctionKind {
/// `T::try_from(U)`
TryFromFunction,
/// `t.try_into()`
TryIntoMethod,
/// `U::try_into(t)`
TryIntoFunction,
}
fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &Expr<'_>,
node_args: ty::GenericArgsRef<'tcx>,
kind: FunctionKind,
primary_span: Span,
) {
if let &[self_ty, other_ty] = node_args.as_slice()
// useless_conversion already warns `T::try_from(T)`, so ignore it here
&& self_ty != other_ty
&& let Some(self_ty) = self_ty.as_type()
&& let Some(from_into_trait) = cx.tcx.get_diagnostic_item(match kind {
FunctionKind::TryFromFunction => sym::From,
FunctionKind::TryIntoMethod | FunctionKind::TryIntoFunction => sym::Into,
})
// If `T: TryFrom<U>` and `T: From<U>` both exist, then that means that the `TryFrom`
// _must_ be from the blanket impl and cannot have been manually implemented
// (else there would be conflicting impls, even with #![feature(spec)]), so we don't even need to check
// what `<T as TryFrom<U>>::Error` is: it's always `Infallible`
&& implements_trait(cx, self_ty, from_into_trait, &[other_ty])
{
let parent_unwrap_call = get_parent_expr(cx, expr)
.and_then(|parent| {
if let ExprKind::MethodCall(path, .., span) = parent.kind
&& let sym::unwrap | sym::expect = path.ident.name
{
Some(span)
} else {
None
}
});
let (sugg, span, applicability) = match kind {
FunctionKind::TryIntoMethod if let Some(unwrap_span) = parent_unwrap_call => {
// Extend the span to include the unwrap/expect call:
// `foo.try_into().expect("..")`
// ^^^^^^^^^^^^^^^^^^^^^^^
//
// `try_into().unwrap()` specifically can be trivially replaced with just `into()`,
// so that can be machine-applicable
("into()", primary_span.with_hi(unwrap_span.hi()), Applicability::MachineApplicable)
}
FunctionKind::TryFromFunction => ("From::from", primary_span, Applicability::Unspecified),
FunctionKind::TryIntoFunction => ("Into::into", primary_span, Applicability::Unspecified),
FunctionKind::TryIntoMethod => ("into", primary_span, Applicability::Unspecified),
};
span_lint_and_sugg(
cx,
UNNECESSARY_FALLIBLE_CONVERSIONS,
span,
"use of a fallible conversion when an infallible one could be used",
"use",
sugg.into(),
applicability
);
}
}
/// Checks method call exprs:
/// - `0i32.try_into()`
pub(super) fn check_method(cx: &LateContext<'_>, expr: &Expr<'_>) {
if let ExprKind::MethodCall(path, ..) = expr.kind {
check(
cx,
expr,
cx.typeck_results().node_args(expr.hir_id),
FunctionKind::TryIntoMethod,
path.ident.span,
);
}
}
/// Checks function call exprs:
/// - `<i64 as TryFrom<_>>::try_from(0i32)`
/// - `<_ as TryInto<i64>>::try_into(0i32)`
pub(super) fn check_function(cx: &LateContext<'_>, expr: &Expr<'_>, callee: &Expr<'_>) {
if let ExprKind::Path(ref qpath) = callee.kind
&& let Some(item_def_id) = cx.qpath_res(qpath, callee.hir_id).opt_def_id()
&& let Some(trait_def_id) = cx.tcx.trait_of_item(item_def_id)
{
check(
cx,
expr,
cx.typeck_results().node_args(callee.hir_id),
match cx.tcx.get_diagnostic_name(trait_def_id) {
Some(sym::TryFrom) => FunctionKind::TryFromFunction,
Some(sym::TryInto) => FunctionKind::TryIntoFunction,
_ => return,
},
callee.span,
);
}
}

1
tests/ui/manual_string_new.fixed
View File

@ -1,4 +1,5 @@
#![warn(clippy::manual_string_new)]
#![allow(clippy::unnecessary_fallible_conversions)]
macro_rules! create_strings_from_macro {
// When inside a macro, nothing should warn to prevent false positives.

1
tests/ui/manual_string_new.rs
View File

@ -1,4 +1,5 @@
#![warn(clippy::manual_string_new)]
#![allow(clippy::unnecessary_fallible_conversions)]
macro_rules! create_strings_from_macro {
// When inside a macro, nothing should warn to prevent false positives.

18
tests/ui/manual_string_new.stderr
View File

@ -1,5 +1,5 @@
error: empty String is being created manually
--> $DIR/manual_string_new.rs:13:13
--> $DIR/manual_string_new.rs:14:13
|
LL | let _ = "".to_string();
| ^^^^^^^^^^^^^^ help: consider using: `String::new()`
@ -8,49 +8,49 @@ LL | let _ = "".to_string();
= help: to override `-D warnings` add `#[allow(clippy::manual_string_new)]`
error: empty String is being created manually
--> $DIR/manual_string_new.rs:16:13
--> $DIR/manual_string_new.rs:17:13
|
LL | let _ = "".to_owned();
| ^^^^^^^^^^^^^ help: consider using: `String::new()`
error: empty String is being created manually
--> $DIR/manual_string_new.rs:19:21
--> $DIR/manual_string_new.rs:20:21
|
LL | let _: String = "".into();
| ^^^^^^^^^ help: consider using: `String::new()`
error: empty String is being created manually
--> $DIR/manual_string_new.rs:26:13
--> $DIR/manual_string_new.rs:27:13
|
LL | let _ = String::from("");
| ^^^^^^^^^^^^^^^^ help: consider using: `String::new()`
error: empty String is being created manually
--> $DIR/manual_string_new.rs:27:13
--> $DIR/manual_string_new.rs:28:13
|
LL | let _ = <String>::from("");
| ^^^^^^^^^^^^^^^^^^ help: consider using: `String::new()`
error: empty String is being created manually
--> $DIR/manual_string_new.rs:32:13
--> $DIR/manual_string_new.rs:33:13
|
LL | let _ = String::try_from("").unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `String::new()`
error: empty String is being created manually
--> $DIR/manual_string_new.rs:38:21
--> $DIR/manual_string_new.rs:39:21
|
LL | let _: String = From::from("");
| ^^^^^^^^^^^^^^ help: consider using: `String::new()`
error: empty String is being created manually
--> $DIR/manual_string_new.rs:43:21
--> $DIR/manual_string_new.rs:44:21
|
LL | let _: String = TryFrom::try_from("").unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `String::new()`
error: empty String is being created manually
--> $DIR/manual_string_new.rs:46:21
--> $DIR/manual_string_new.rs:47:21
|
LL | let _: String = TryFrom::try_from("").expect("this should warn");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `String::new()`

6
tests/ui/unnecessary_fallible_conversions.fixed Normal file
View File

@ -0,0 +1,6 @@
#![warn(clippy::unnecessary_fallible_conversions)]
fn main() {
let _: i64 = 0i32.into();
let _: i64 = 0i32.into();
}

6
tests/ui/unnecessary_fallible_conversions.rs Normal file
View File

@ -0,0 +1,6 @@
#![warn(clippy::unnecessary_fallible_conversions)]
fn main() {
let _: i64 = 0i32.try_into().unwrap();
let _: i64 = 0i32.try_into().expect("can't happen");
}

17
tests/ui/unnecessary_fallible_conversions.stderr Normal file
View File

@ -0,0 +1,17 @@
error: use of a fallible conversion when an infallible one could be used
--> $DIR/unnecessary_fallible_conversions.rs:4:23
|
LL | let _: i64 = 0i32.try_into().unwrap();
| ^^^^^^^^^^^^^^^^^^^ help: use: `into()`
|
= note: `-D clippy::unnecessary-fallible-conversions` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_fallible_conversions)]`
error: use of a fallible conversion when an infallible one could be used
--> $DIR/unnecessary_fallible_conversions.rs:5:23
|
LL | let _: i64 = 0i32.try_into().expect("can't happen");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `into()`
error: aborting due to 2 previous errors

43
tests/ui/unnecessary_fallible_conversions_unfixable.rs Normal file
View File

@ -0,0 +1,43 @@
//@aux-build:proc_macros.rs
//@no-rustfix
#![warn(clippy::unnecessary_fallible_conversions)]
extern crate proc_macros;
struct Foo;
impl TryFrom<i32> for Foo {
type Error = ();
fn try_from(_: i32) -> Result<Self, Self::Error> {
Ok(Foo)
}
}
impl From<i64> for Foo {
fn from(_: i64) -> Self {
Foo
}
}
fn main() {
// `Foo` only implements `TryFrom<i32>` and not `From<i32>`, so don't lint
let _: Result<Foo, _> = 0i32.try_into();
let _: Result<Foo, _> = i32::try_into(0i32);
let _: Result<Foo, _> = Foo::try_from(0i32);
// ... it does impl From<i64> however
let _: Result<Foo, _> = 0i64.try_into();
//~^ ERROR: use of a fallible conversion when an infallible one could be used
let _: Result<Foo, _> = i64::try_into(0i64);
//~^ ERROR: use of a fallible conversion when an infallible one could be used
let _: Result<Foo, _> = Foo::try_from(0i64);
//~^ ERROR: use of a fallible conversion when an infallible one could be used
let _: Result<i64, _> = 0i32.try_into();
//~^ ERROR: use of a fallible conversion when an infallible one could be used
let _: Result<i64, _> = i32::try_into(0i32);
//~^ ERROR: use of a fallible conversion when an infallible one could be used
let _: Result<i64, _> = <_>::try_from(0i32);
//~^ ERROR: use of a fallible conversion when an infallible one could be used
// From a macro
let _: Result<i64, _> = proc_macros::external!(0i32).try_into();
}

41
tests/ui/unnecessary_fallible_conversions_unfixable.stderr Normal file
View File

@ -0,0 +1,41 @@
error: use of a fallible conversion when an infallible one could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:27:34
|
LL | let _: Result<Foo, _> = 0i64.try_into();
| ^^^^^^^^ help: use: `into`
|
= note: `-D clippy::unnecessary-fallible-conversions` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_fallible_conversions)]`
error: use of a fallible conversion when an infallible one could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:29:29
|
LL | let _: Result<Foo, _> = i64::try_into(0i64);
| ^^^^^^^^^^^^^ help: use: `Into::into`
error: use of a fallible conversion when an infallible one could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:31:29
|
LL | let _: Result<Foo, _> = Foo::try_from(0i64);
| ^^^^^^^^^^^^^ help: use: `From::from`
error: use of a fallible conversion when an infallible one could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:34:34
|
LL | let _: Result<i64, _> = 0i32.try_into();
| ^^^^^^^^ help: use: `into`
error: use of a fallible conversion when an infallible one could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:36:29
|
LL | let _: Result<i64, _> = i32::try_into(0i32);
| ^^^^^^^^^^^^^ help: use: `Into::into`
error: use of a fallible conversion when an infallible one could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:38:29
|
LL | let _: Result<i64, _> = <_>::try_from(0i32);
| ^^^^^^^^^^^^^ help: use: `From::from`
error: aborting due to 6 previous errors

2
tests/ui/useless_conversion_try.rs
View File

@ -1,5 +1,5 @@
#![deny(clippy::useless_conversion)]
#![allow(clippy::needless_if)]
#![allow(clippy::needless_if, clippy::unnecessary_fallible_conversions)]
fn test_generic<T: Copy>(val: T) -> T {
let _ = T::try_from(val).unwrap();