forked from rust-lang/rust
-
-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of rust-lang#11669 - y21:issue11577, r=Jarcho
new lint: `unnecessary_fallible_conversions` Closes rust-lang#11577 A new lint that looks for calls such as `i64::try_from(1i32)` and suggests `i64::from(1i32)`. See lint description (and linked issue) for more details for why. There's a tiny bit of overlap with the `useless_conversion` lint, in that the other one warns `T::try_from(T)` (i.e., fallibly converting to the same type), so this lint ignores cases like `i32::try_from(1i32)` to avoid emitting two warnings for the same expression. Also, funnily enough, with this one exception, this lint would warn on exactly every case in the `useless_conversion_try` ui test that `useless_conversion` didn't cover (but never two warnings at the same time), which is neat. I did add an `#![allow]` though since we don't want interleaved warnings from multiple lints in the same uitest. changelog: new lint: `unnecessary_fallible_conversions`
- Loading branch information
Showing
14 changed files
with
291 additions
and
22 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
119 changes: 119 additions & 0 deletions
119
clippy_lints/src/methods/unnecessary_fallible_conversions.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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, | ||
| ); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| #![warn(clippy::unnecessary_fallible_conversions)] | ||
|
|
||
| fn main() { | ||
| let _: i64 = 0i32.into(); | ||
| let _: i64 = 0i32.into(); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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"); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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(); | ||
| } |
Oops, something went wrong.