Skip to content

Commit d420b82

Browse files
committed
[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.
1 parent a580b5c commit d420b82

File tree

26 files changed

+219
-29
lines changed

26 files changed

+219
-29
lines changed

compiler/rustc_hir_analysis/src/check/wfcheck.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1120,7 +1120,7 @@ fn check_type_defn<'tcx>(
11201120
} else {
11211121
// Evaluate the constant proactively, to emit an error if the constant has
11221122
// an unconditional error. We only do so if the const has no type params.
1123-
let _ = tcx.const_eval_poly(def_id.into());
1123+
let _ = tcx.const_eval_poly(def_id);
11241124
}
11251125
}
11261126
let field_id = field.did.expect_local();

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

+105-4
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::{
@@ -1304,9 +1304,9 @@ impl UnreachablePub {
13041304
cx.effective_visibilities.effective_vis(def_id).map(|effective_vis| {
13051305
effective_vis.at_level(rustc_middle::middle::privacy::Level::Reachable)
13061306
})
1307-
&& let parent_parent = cx.tcx.parent_module_from_def_id(
1308-
cx.tcx.parent_module_from_def_id(def_id.into()).into(),
1309-
)
1307+
&& let parent_parent = cx
1308+
.tcx
1309+
.parent_module_from_def_id(cx.tcx.parent_module_from_def_id(def_id).into())
13101310
&& *restricted_did == parent_parent.to_local_def_id()
13111311
&& !restricted_did.to_def_id().is_crate_root()
13121312
{
@@ -1589,6 +1589,7 @@ declare_lint_pass!(
15891589
SoftLints => [
15901590
WHILE_TRUE,
15911591
NON_SHORTHAND_FIELD_PATTERNS,
1592+
SELF_TYPE_CONVERSION,
15921593
UNSAFE_CODE,
15931594
MISSING_DOCS,
15941595
MISSING_COPY_IMPLEMENTATIONS,
@@ -3063,3 +3064,103 @@ impl EarlyLintPass for SpecialModuleName {
30633064
}
30643065
}
30653066
}
3067+
3068+
declare_lint! {
3069+
/// The `self_type_conversion` lint detects when a call to `.into()` does not have any effect.
3070+
///
3071+
/// ### Example
3072+
///
3073+
/// ```rust,compile_fail
3074+
/// fn main() {
3075+
/// let () = ().into();
3076+
/// }
3077+
/// ```
3078+
///
3079+
/// {{produces}}
3080+
///
3081+
/// ### Explanation
3082+
///
3083+
pub SELF_TYPE_CONVERSION,
3084+
Deny,
3085+
"",
3086+
}
3087+
3088+
pub struct SelfTypeConversion<'tcx> {
3089+
ignored_types: Vec<Ty<'tcx>>,
3090+
}
3091+
3092+
impl_lint_pass!(SelfTypeConversion<'_> => [SELF_TYPE_CONVERSION]);
3093+
3094+
impl SelfTypeConversion<'_> {
3095+
pub fn new() -> Self {
3096+
Self { ignored_types: vec![] }
3097+
}
3098+
}
3099+
3100+
impl<'tcx> LateLintPass<'tcx> for SelfTypeConversion<'tcx> {
3101+
fn check_item_post(&mut self, cx: &LateContext<'tcx>, item: &hir::Item<'_>) {
3102+
let hir::ItemKind::Use(path, kind) = item.kind else { return };
3103+
tracing::info!("{:#?}", item);
3104+
tracing::info!(?path, ?kind);
3105+
for res in &path.res {
3106+
let Res::Def(DefKind::TyAlias, def_id) = res else { continue };
3107+
let ty = cx.tcx.type_of(def_id).instantiate_identity();
3108+
let name = cx.tcx.item_name(*def_id);
3109+
// println!("{ty:?} {name:?}");
3110+
tracing::info!("ignoring {ty:?} {name:?}");
3111+
self.ignored_types.push(ty);
3112+
for stripped in cx.tcx.stripped_cfg_items(def_id.krate) {
3113+
if stripped.name.name == name {
3114+
tracing::info!("found stripped {name:#?}");
3115+
}
3116+
}
3117+
}
3118+
// FIXME: also look at conditional cfgd uses so to account for things like
3119+
// `use std::io::repr_bitpacked` and `std::io::repr_unpacked`.
3120+
}
3121+
3122+
fn check_expr_post(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'_>) {
3123+
let hir::ExprKind::MethodCall(_segment, rcvr, args, _) = expr.kind else { return };
3124+
if !args.is_empty() {
3125+
tracing::info!("non-empty args");
3126+
return;
3127+
}
3128+
let ty = cx.typeck_results().expr_ty(expr);
3129+
let rcvr_ty = cx.typeck_results().expr_ty(rcvr);
3130+
tracing::info!(?ty, ?rcvr_ty);
3131+
3132+
if ty != rcvr_ty {
3133+
tracing::info!("different types");
3134+
return;
3135+
}
3136+
if self.ignored_types.contains(&rcvr_ty) {
3137+
tracing::info!("ignored");
3138+
return;
3139+
}
3140+
let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) else {
3141+
tracing::info!("no type dependent def id");
3142+
return;
3143+
};
3144+
tracing::info!(?def_id);
3145+
if Some(def_id) != cx.tcx.get_diagnostic_item(sym::into_fn) {
3146+
tracing::info!("not into_fn {:?}", cx.tcx.get_diagnostic_item(sym::into_fn));
3147+
return;
3148+
}
3149+
tracing::info!(?def_id);
3150+
tracing::info!(?expr);
3151+
if expr.span.macro_backtrace().next().is_some() {
3152+
return;
3153+
}
3154+
if cx.tcx.sess.source_map().span_to_embeddable_string(expr.span).contains("symbolize/gimli")
3155+
{
3156+
// HACK
3157+
return;
3158+
}
3159+
// println!("{:#?}", self.ignored_types);
3160+
cx.emit_span_lint(SELF_TYPE_CONVERSION, expr.span, SelfTypeConversionDiag {
3161+
source: rcvr_ty,
3162+
target: ty,
3163+
});
3164+
// bug!("asdf");
3165+
}
3166+
}

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_mir_transform/src/lint_tail_expr_drop_order.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ impl Subdiagnostic for LocalLabel<'_> {
691691
for dtor in self.destructors {
692692
dtor.add_to_diag_with(diag, f);
693693
}
694-
let msg = f(diag, crate::fluent_generated::mir_transform_label_local_epilogue.into());
694+
let msg = f(diag, crate::fluent_generated::mir_transform_label_local_epilogue);
695695
diag.span_label(self.span, msg);
696696
}
697697
}

compiler/rustc_parse/src/parser/tests.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -2366,8 +2366,7 @@ fn string_to_tts_1() {
23662366
token::Ident(sym::i32, IdentIsRaw::No),
23672367
sp(8, 11),
23682368
),
2369-
])
2370-
.into(),
2369+
]),
23712370
),
23722371
TokenTree::Delimited(
23732372
DelimSpan::from_pair(sp(13, 14), sp(18, 19)),
@@ -2383,8 +2382,7 @@ fn string_to_tts_1() {
23832382
),
23842383
// `Alone` because the `;` is followed by whitespace.
23852384
TokenTree::token_alone(token::Semi, sp(16, 17)),
2386-
])
2387-
.into(),
2385+
]),
23882386
),
23892387
]);
23902388

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,

compiler/rustc_trait_selection/src/traits/const_evaluatable.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ pub fn is_const_evaluatable<'tcx>(
7979
Err(
8080
EvaluateConstErr::EvaluationFailure(e)
8181
| EvaluateConstErr::InvalidConstParamTy(e),
82-
) => Err(NotConstEvaluatable::Error(e.into())),
82+
) => Err(NotConstEvaluatable::Error(e)),
8383
Ok(_) => Ok(()),
8484
}
8585
}
@@ -140,7 +140,7 @@ pub fn is_const_evaluatable<'tcx>(
140140
}
141141
Err(
142142
EvaluateConstErr::EvaluationFailure(e) | EvaluateConstErr::InvalidConstParamTy(e),
143-
) => Err(NotConstEvaluatable::Error(e.into())),
143+
) => Err(NotConstEvaluatable::Error(e)),
144144
Ok(_) => Ok(()),
145145
}
146146
}

library/alloc/src/rc.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1789,7 +1789,7 @@ impl<T: ?Sized, A: Allocator> Rc<T, A> {
17891789
/// let x: Rc<&str> = Rc::new("Hello, world!");
17901790
/// {
17911791
/// let s = String::from("Oh, no!");
1792-
/// let mut y: Rc<&str> = x.clone().into();
1792+
/// let mut y: Rc<&str> = x.clone();
17931793
/// unsafe {
17941794
/// // this is Undefined Behavior, because x's inner type
17951795
/// // is &'long str, not &'short str

library/alloc/src/sync.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2468,7 +2468,7 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
24682468
/// let x: Arc<&str> = Arc::new("Hello, world!");
24692469
/// {
24702470
/// let s = String::from("Oh, no!");
2471-
/// let mut y: Arc<&str> = x.clone().into();
2471+
/// let mut y: Arc<&str> = x.clone();
24722472
/// unsafe {
24732473
/// // this is Undefined Behavior, because x's inner type
24742474
/// // is &'long str, not &'short str

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 {

library/std/src/thread/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ impl Builder {
502502

503503
let id = ThreadId::new();
504504
let my_thread = match name {
505-
Some(name) => Thread::new(id, name.into()),
505+
Some(name) => Thread::new(id, name),
506506
None => Thread::new_unnamed(id),
507507
};
508508

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
}

0 commit comments

Comments
 (0)