Skip to content

Commit 9543f8e

Browse files
borsbors
committed
Auto merge of #129249 - estebank:useless-into, r=<try>
[Experimental] `<T as Into<T>>::into` lint Running crater to see how common that pattern is. The Lint would have to be at most warn-by-default because there are a handful of cases detected that are actually perfectly reasonable (`type` aliases with per-platform `cfg`, or macros) which are now at best half-heartedly handled. I've detected a handful of cases where we're calling `.into()` unnecessarily in the `rustc` codebase as well, and changed those. CC #127343.
2 parents 336209e + 399747c commit 9543f8e

19 files changed

+220
-15
lines changed

‎compiler/rustc_lint/messages.ftl

+2
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,8 @@ lint_reserved_prefix = prefix `{$prefix}` is unknown
740740
lint_reserved_string = will be parsed as a guarded string in Rust 2024
741741
.suggestion = insert whitespace here to avoid this being parsed as a guarded string in Rust 2024
742742
743+
lint_self_type_conversion = this conversion is useless `{$source}` to `{$target}`
744+
743745
lint_shadowed_into_iter =
744746
this method call resolves to `<&{$target} as IntoIterator>::into_iter` (due to backwards compatibility), but will resolve to `<{$target} as IntoIterator>::into_iter` in Rust {$edition}
745747
.use_iter_suggestion = use `.iter()` instead of `.into_iter()` to avoid ambiguity

‎compiler/rustc_lint/src/builtin.rs

+115-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ use crate::lints::{
5858
BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
5959
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
6060
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
61-
BuiltinWhileTrue, InvalidAsmLabel,
61+
BuiltinWhileTrue, InvalidAsmLabel, SelfTypeConversionDiag,
6262
};
6363
use crate::nonstandard_style::{MethodLateContext, method_context};
6464
use crate::{
@@ -1588,6 +1588,7 @@ declare_lint_pass!(
15881588
SoftLints => [
15891589
WHILE_TRUE,
15901590
NON_SHORTHAND_FIELD_PATTERNS,
1591+
SELF_TYPE_CONVERSION,
15911592
UNSAFE_CODE,
15921593
MISSING_DOCS,
15931594
MISSING_COPY_IMPLEMENTATIONS,
@@ -3062,3 +3063,116 @@ impl EarlyLintPass for SpecialModuleName {
30623063
}
30633064
}
30643065
}
3066+
3067+
declare_lint! {
3068+
/// The `self_type_conversion` lint detects when a call to `.into()` does not have any effect.
3069+
///
3070+
/// ### Example
3071+
///
3072+
/// ```rust,compile_fail
3073+
/// fn main() {
3074+
/// let () = ().into();
3075+
/// }
3076+
/// ```
3077+
///
3078+
/// {{produces}}
3079+
///
3080+
/// ### Explanation
3081+
///
3082+
pub SELF_TYPE_CONVERSION,
3083+
Deny,
3084+
"",
3085+
}
3086+
3087+
pub struct SelfTypeConversion<'tcx> {
3088+
ignored_types: Vec<Ty<'tcx>>,
3089+
}
3090+
3091+
impl_lint_pass!(SelfTypeConversion<'_> => [SELF_TYPE_CONVERSION]);
3092+
3093+
impl SelfTypeConversion<'_> {
3094+
pub fn new() -> Self {
3095+
Self { ignored_types: vec![] }
3096+
}
3097+
}
3098+
3099+
impl<'tcx> LateLintPass<'tcx> for SelfTypeConversion<'tcx> {
3100+
fn check_item_post(&mut self, cx: &LateContext<'tcx>, item: &hir::Item<'_>) {
3101+
let hir::ItemKind::Use(path, kind) = item.kind else { return };
3102+
tracing::info!("{:#?}", item);
3103+
tracing::info!(?path, ?kind);
3104+
for res in &path.res {
3105+
let Res::Def(DefKind::TyAlias, def_id) = res else { continue };
3106+
let ty = cx.tcx.type_of(def_id).instantiate_identity();
3107+
let name = cx.tcx.item_name(*def_id);
3108+
// println!("{ty:?} {name:?}");
3109+
tracing::info!("ignoring {ty:?} {name:?}");
3110+
self.ignored_types.push(ty);
3111+
for stripped in cx.tcx.stripped_cfg_items(def_id.krate) {
3112+
if stripped.name.name == name {
3113+
tracing::info!("found stripped {name:#?}");
3114+
}
3115+
}
3116+
}
3117+
// FIXME: also look at conditional cfgd uses so to account for things like
3118+
// `use std::io::repr_bitpacked` and `std::io::repr_unpacked`.
3119+
}
3120+
3121+
fn check_expr_post(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'_>) {
3122+
let hir::ExprKind::MethodCall(_segment, rcvr, args, _) = expr.kind else { return };
3123+
if !args.is_empty() {
3124+
tracing::info!("non-empty args");
3125+
return;
3126+
}
3127+
let ty = cx.typeck_results().expr_ty(expr);
3128+
let rcvr_ty = cx.typeck_results().expr_ty(rcvr);
3129+
tracing::info!(?ty, ?rcvr_ty);
3130+
3131+
if ty != rcvr_ty {
3132+
tracing::info!("different types");
3133+
return;
3134+
}
3135+
if self.ignored_types.contains(&rcvr_ty) {
3136+
tracing::info!("ignored");
3137+
return;
3138+
}
3139+
let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) else {
3140+
tracing::info!("no type dependent def id");
3141+
return;
3142+
};
3143+
tracing::info!(?def_id);
3144+
if Some(def_id) != cx.tcx.get_diagnostic_item(sym::into_fn) {
3145+
tracing::info!("not into_fn {:?}", cx.tcx.get_diagnostic_item(sym::into_fn));
3146+
return;
3147+
}
3148+
tracing::info!(?expr);
3149+
if expr.span.macro_backtrace().next().is_some() {
3150+
return;
3151+
}
3152+
if cx.tcx.sess.source_map().span_to_embeddable_string(expr.span).contains("symbolize/gimli")
3153+
|| cx
3154+
.tcx
3155+
.sess
3156+
.source_map()
3157+
.span_to_embeddable_string(expr.span)
3158+
.contains("crates/crates-io")
3159+
|| cx.tcx.sess.source_map().span_to_embeddable_string(expr.span).contains("cargo/core")
3160+
|| cx
3161+
.tcx
3162+
.sess
3163+
.source_map()
3164+
.span_to_embeddable_string(expr.span)
3165+
.contains("cargo/sources")
3166+
|| cx.tcx.sess.source_map().span_to_embeddable_string(expr.span).contains("cargo/util")
3167+
{
3168+
// HACK
3169+
return;
3170+
}
3171+
// println!("{:#?}", self.ignored_types);
3172+
cx.emit_span_lint(SELF_TYPE_CONVERSION, expr.span, SelfTypeConversionDiag {
3173+
source: rcvr_ty,
3174+
target: ty,
3175+
});
3176+
// bug!("asdf");
3177+
}
3178+
}

‎compiler/rustc_lint/src/lib.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ early_lint_methods!(
188188
late_lint_methods!(
189189
declare_combined_late_lint_pass,
190190
[
191-
BuiltinCombinedModuleLateLintPass,
191+
BuiltinCombinedModuleLateLintPass<'tcx>,
192192
[
193193
ForLoopsOverFallibles: ForLoopsOverFallibles,
194194
DefaultCouldBeDerived: DefaultCouldBeDerived::default(),
@@ -206,6 +206,7 @@ late_lint_methods!(
206206
UnitBindings: UnitBindings,
207207
NonUpperCaseGlobals: NonUpperCaseGlobals,
208208
NonShorthandFieldPatterns: NonShorthandFieldPatterns,
209+
SelfTypeConversion<'tcx>: SelfTypeConversion::new(),
209210
UnusedAllocation: UnusedAllocation,
210211
// Depends on types used in type definitions
211212
MissingCopyImplementations: MissingCopyImplementations,
@@ -275,6 +276,7 @@ fn register_builtins(store: &mut LintStore) {
275276
store.register_lints(&foreign_modules::get_lints());
276277
store.register_lints(&HardwiredLints::lint_vec());
277278

279+
store.register_late_pass(move |_tcx| Box::new(SelfTypeConversion::new()));
278280
add_lint_group!(
279281
"nonstandard_style",
280282
NON_CAMEL_CASE_TYPES,
@@ -305,6 +307,7 @@ fn register_builtins(store: &mut LintStore) {
305307
UNUSED_PARENS,
306308
UNUSED_BRACES,
307309
REDUNDANT_SEMICOLONS,
310+
// SELF_TYPE_CONVERSION,
308311
MAP_UNIT_FN
309312
);
310313

‎compiler/rustc_lint/src/lints.rs

+7
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,13 @@ pub(crate) struct BuiltinNonShorthandFieldPatterns {
7171
pub prefix: &'static str,
7272
}
7373

74+
#[derive(LintDiagnostic)]
75+
#[diag(lint_self_type_conversion)]
76+
pub(crate) struct SelfTypeConversionDiag<'t> {
77+
pub source: Ty<'t>,
78+
pub target: Ty<'t>,
79+
}
80+
7481
#[derive(LintDiagnostic)]
7582
pub(crate) enum BuiltinUnsafe {
7683
#[diag(lint_builtin_allow_internal_unsafe)]

‎compiler/rustc_lint/src/passes.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,13 @@ macro_rules! expand_combined_late_lint_pass_methods {
9595
/// runtime.
9696
#[macro_export]
9797
macro_rules! declare_combined_late_lint_pass {
98-
([$v:vis $name:ident, [$($pass:ident: $constructor:expr,)*]], $methods:tt) => (
98+
([$v:vis $name:ident$(<$lt:lifetime>)?, [$($pass:ident$(<$inner_lt:lifetime>)?: $constructor:expr,)*]], $methods:tt) => (
9999
#[allow(non_snake_case)]
100-
$v struct $name {
101-
$($pass: $pass,)*
100+
$v struct $name$(<$lt>)? {
101+
$($pass: $pass$(<$inner_lt>)?,)*
102102
}
103103

104-
impl $name {
104+
impl$(<$lt>)? $name$(<$lt>)? {
105105
$v fn new() -> Self {
106106
Self {
107107
$($pass: $constructor,)*
@@ -115,12 +115,12 @@ macro_rules! declare_combined_late_lint_pass {
115115
}
116116
}
117117

118-
impl<'tcx> $crate::LateLintPass<'tcx> for $name {
118+
impl<'tcx> $crate::LateLintPass<'tcx> for $name$(<$lt>)? {
119119
$crate::expand_combined_late_lint_pass_methods!([$($pass),*], $methods);
120120
}
121121

122122
#[allow(rustc::lint_pass_impl_without_macro)]
123-
impl $crate::LintPass for $name {
123+
impl$(<$lt>)? $crate::LintPass for $name$(<$lt>)? {
124124
fn name(&self) -> &'static str {
125125
panic!()
126126
}

‎compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1119,6 +1119,7 @@ symbols! {
11191119
integer_: "integer", // underscore to avoid clashing with the function `sym::integer` below
11201120
integral,
11211121
into_async_iter_into_iter,
1122+
into_fn,
11221123
into_future,
11231124
into_iter,
11241125
intra_doc_pointers,

‎library/core/src/convert/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@ pub trait AsMut<T: ?Sized> {
446446
#[doc(search_unbox)]
447447
pub trait Into<T>: Sized {
448448
/// Converts this type into the (usually inferred) input type.
449+
#[rustc_diagnostic_item = "into_fn"]
449450
#[must_use]
450451
#[stable(feature = "rust1", since = "1.0.0")]
451452
fn into(self) -> T;

‎library/std/src/os/fd/owned.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
#![stable(feature = "io_safety", since = "1.63.0")]
44
#![deny(unsafe_op_in_unsafe_fn)]
5-
5+
#![cfg_attr(not(bootstrap), allow(self_type_conversion))]
66
use super::raw::{AsRawFd, FromRawFd, IntoRawFd, RawFd};
77
use crate::marker::PhantomData;
88
use crate::mem::ManuallyDrop;

‎library/std/src/sys/pal/unix/process/process_unix.rs

+1
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ impl Command {
285285
// have the drop glue anyway because this code never returns (the
286286
// child will either exec() or invoke libc::exit)
287287
#[cfg(not(any(target_os = "tvos", target_os = "watchos")))]
288+
#[cfg_attr(not(bootstrap), allow(self_type_conversion))]
288289
unsafe fn do_exec(
289290
&mut self,
290291
stdio: ChildPipes,

‎library/std/src/sys_common/process.rs

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ impl fmt::Debug for CommandEnv {
2525

2626
impl CommandEnv {
2727
// Capture the current environment with these changes applied
28+
#[cfg_attr(not(bootstrap), allow(self_type_conversion))]
2829
pub fn capture(&self) -> BTreeMap<EnvKey, OsString> {
2930
let mut result = BTreeMap::<EnvKey, OsString>::new();
3031
if !self.clear {

‎src/tools/clippy/tests/ui/needless_return_with_question_mark.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ fn main() -> Result<(), ()> {
3030
return Ok::<(), ()>(());
3131
Err(())?;
3232
Ok::<(), ()>(());
33-
return Err(().into());
33+
return Err(());
3434
external! {
3535
return Err(())?;
3636
}

‎src/tools/clippy/tests/ui/needless_return_with_question_mark.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ fn main() -> Result<(), ()> {
3030
return Ok::<(), ()>(());
3131
Err(())?;
3232
Ok::<(), ()>(());
33-
return Err(().into());
33+
return Err(());
3434
external! {
3535
return Err(())?;
3636
}

‎src/tools/clippy/tests/ui/useless_conversion.stderr

+73-1
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,26 @@ error: useless conversion to the same type: `T`
1616
LL | val.into()
1717
| ^^^^^^^^^^ help: consider removing `.into()`: `val`
1818

19+
error: this conversion is useless `T` to `T`
20+
--> tests/ui/useless_conversion.rs:10:5
21+
|
22+
LL | val.into()
23+
| ^^^^^^^^^^
24+
|
25+
= note: `#[deny(self_type_conversion)]` on by default
26+
1927
error: useless conversion to the same type: `i32`
2028
--> tests/ui/useless_conversion.rs:22:22
2129
|
2230
LL | let _: i32 = 0i32.into();
2331
| ^^^^^^^^^^^ help: consider removing `.into()`: `0i32`
2432

33+
error: this conversion is useless `i32` to `i32`
34+
--> tests/ui/useless_conversion.rs:22:22
35+
|
36+
LL | let _: i32 = 0i32.into();
37+
| ^^^^^^^^^^^
38+
2539
error: useless conversion to the same type: `std::str::Lines<'_>`
2640
--> tests/ui/useless_conversion.rs:52:22
2741
|
@@ -58,6 +72,12 @@ error: useless conversion to the same type: `std::string::String`
5872
LL | let _: String = "foo".to_string().into();
5973
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()`
6074

75+
error: this conversion is useless `std::string::String` to `std::string::String`
76+
--> tests/ui/useless_conversion.rs:136:21
77+
|
78+
LL | let _: String = "foo".to_string().into();
79+
| ^^^^^^^^^^^^^^^^^^^^^^^^
80+
6181
error: useless conversion to the same type: `std::string::String`
6282
--> tests/ui/useless_conversion.rs:137:21
6383
|
@@ -94,6 +114,12 @@ error: useless conversion to the same type: `std::string::String`
94114
LL | let _: String = format!("Hello {}", "world").into();
95115
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")`
96116

117+
error: this conversion is useless `std::string::String` to `std::string::String`
118+
--> tests/ui/useless_conversion.rs:142:21
119+
|
120+
LL | let _: String = format!("Hello {}", "world").into();
121+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
122+
97123
error: useless conversion to the same type: `i32`
98124
--> tests/ui/useless_conversion.rs:147:13
99125
|
@@ -106,6 +132,12 @@ error: useless conversion to the same type: `Foo<'a'>`
106132
LL | let _: Foo<'a'> = s2.into();
107133
| ^^^^^^^^^ help: consider removing `.into()`: `s2`
108134

135+
error: this conversion is useless `Foo<'a'>` to `Foo<'a'>`
136+
--> tests/ui/useless_conversion.rs:153:23
137+
|
138+
LL | let _: Foo<'a'> = s2.into();
139+
| ^^^^^^^^^
140+
109141
error: useless conversion to the same type: `Foo<'a'>`
110142
--> tests/ui/useless_conversion.rs:155:13
111143
|
@@ -274,5 +306,45 @@ error: useless conversion to the same type: `T`
274306
LL | x.into_iter().map(Into::into).collect()
275307
| ^^^^^^^^^^^^^^^^ help: consider removing
276308

277-
error: aborting due to 36 previous errors
309+
error: this conversion is useless `T` to `T`
310+
--> tests/ui/useless_conversion.rs:10:5
311+
|
312+
LL | val.into()
313+
| ^^^^^^^^^^
314+
|
315+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
316+
317+
error: this conversion is useless `i32` to `i32`
318+
--> tests/ui/useless_conversion.rs:22:22
319+
|
320+
LL | let _: i32 = 0i32.into();
321+
| ^^^^^^^^^^^
322+
|
323+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
324+
325+
error: this conversion is useless `std::string::String` to `std::string::String`
326+
--> tests/ui/useless_conversion.rs:136:21
327+
|
328+
LL | let _: String = "foo".to_string().into();
329+
| ^^^^^^^^^^^^^^^^^^^^^^^^
330+
|
331+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
332+
333+
error: this conversion is useless `std::string::String` to `std::string::String`
334+
--> tests/ui/useless_conversion.rs:142:21
335+
|
336+
LL | let _: String = format!("Hello {}", "world").into();
337+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
338+
|
339+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
340+
341+
error: this conversion is useless `Foo<'a'>` to `Foo<'a'>`
342+
--> tests/ui/useless_conversion.rs:153:23
343+
|
344+
LL | let _: Foo<'a'> = s2.into();
345+
| ^^^^^^^^^
346+
|
347+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
348+
349+
error: aborting due to 46 previous errors
278350

0 commit comments

Comments
 (0)