Skip to content

Commit

Permalink
Auto merge of rust-lang#129249 - estebank:useless-into, r=<try>
Browse files Browse the repository at this point in the history
[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 rust-lang#127343.
  • Loading branch information
bors committed Jan 9, 2025
2 parents 251206c + 8118985 commit ecb994b
Show file tree
Hide file tree
Showing 20 changed files with 219 additions and 16 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,8 @@ lint_reserved_prefix = prefix `{$prefix}` is unknown
lint_reserved_string = will be parsed as a guarded string in Rust 2024
.suggestion = insert whitespace here to avoid this being parsed as a guarded string in Rust 2024
lint_self_type_conversion = this conversion is useless `{$source}` to `{$target}`
lint_shadowed_into_iter =
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}
.use_iter_suggestion = use `.iter()` instead of `.into_iter()` to avoid ambiguity
Expand Down
114 changes: 113 additions & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ use crate::lints::{
BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, InvalidAsmLabel,
BuiltinWhileTrue, InvalidAsmLabel, SelfTypeConversionDiag,
};
use crate::nonstandard_style::{MethodLateContext, method_context};
use crate::{
Expand Down Expand Up @@ -1588,6 +1588,7 @@ declare_lint_pass!(
SoftLints => [
WHILE_TRUE,
NON_SHORTHAND_FIELD_PATTERNS,
SELF_TYPE_CONVERSION,
UNSAFE_CODE,
MISSING_DOCS,
MISSING_COPY_IMPLEMENTATIONS,
Expand Down Expand Up @@ -3062,3 +3063,114 @@ impl EarlyLintPass for SpecialModuleName {
}
}
}

declare_lint! {
/// The `self_type_conversion` lint detects when a call to `.into()` does not have any effect.
///
/// ### Example
///
/// ```rust,compile_fail
/// fn main() {
/// let () = ().into();
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
pub SELF_TYPE_CONVERSION,
Deny,
"",
}

pub struct SelfTypeConversion<'tcx> {
ignored_types: Vec<Ty<'tcx>>,
}

impl_lint_pass!(SelfTypeConversion<'_> => [SELF_TYPE_CONVERSION]);

impl SelfTypeConversion<'_> {
pub fn new() -> Self {
Self { ignored_types: vec![] }
}
}

impl<'tcx> LateLintPass<'tcx> for SelfTypeConversion<'tcx> {
fn check_item_post(&mut self, cx: &LateContext<'tcx>, item: &hir::Item<'_>) {
let hir::ItemKind::Use(path, kind) = item.kind else { return };
tracing::info!("{:#?}", item);
tracing::info!(?path, ?kind);
for res in &path.res {
let Res::Def(DefKind::TyAlias, def_id) = res else { continue };
let ty = cx.tcx.type_of(def_id).instantiate_identity();
let name = cx.tcx.item_name(*def_id);
// println!("{ty:?} {name:?}");
tracing::info!("ignoring {ty:?} {name:?}");
self.ignored_types.push(ty);
for stripped in cx.tcx.stripped_cfg_items(def_id.krate) {
if stripped.name.name == name {
tracing::info!("found stripped {name:#?}");
}
}
}
// FIXME: also look at conditional cfgd uses so to account for things like
// `use std::io::repr_bitpacked` and `std::io::repr_unpacked`.
}

fn check_expr_post(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'_>) {
let hir::ExprKind::MethodCall(_segment, rcvr, args, _) = expr.kind else { return };
if !args.is_empty() {
tracing::info!("non-empty args");
return;
}
let ty = cx.typeck_results().expr_ty(expr);
let rcvr_ty = cx.typeck_results().expr_ty(rcvr);
tracing::info!(?ty, ?rcvr_ty);

if ty != rcvr_ty {
tracing::info!("different types");
return;
}
if self.ignored_types.contains(&rcvr_ty) {
tracing::info!("ignored");
return;
}
let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) else {
tracing::info!("no type dependent def id");
return;
};
tracing::info!(?def_id);
if Some(def_id) != cx.tcx.get_diagnostic_item(sym::into_fn) {
tracing::info!("not into_fn {:?}", cx.tcx.get_diagnostic_item(sym::into_fn));
return;
}
tracing::info!(?expr);
if expr.span.macro_backtrace().next().is_some() {
return;
}
if cx.tcx.sess.source_map().span_to_embeddable_string(expr.span).contains("symbolize/gimli")
|| cx
.tcx
.sess
.source_map()
.span_to_embeddable_string(expr.span)
.contains("crates/crates-io")
|| cx
.tcx
.sess
.source_map()
.span_to_embeddable_string(expr.span)
.contains("cargo/src/cargo")
{
// HACK
return;
}
// println!("{:#?}", self.ignored_types);
cx.emit_span_lint(SELF_TYPE_CONVERSION, expr.span, SelfTypeConversionDiag {
source: rcvr_ty,
target: ty,
});
// bug!("asdf");
}
}
5 changes: 4 additions & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ early_lint_methods!(
late_lint_methods!(
declare_combined_late_lint_pass,
[
BuiltinCombinedModuleLateLintPass,
BuiltinCombinedModuleLateLintPass<'tcx>,
[
ForLoopsOverFallibles: ForLoopsOverFallibles,
DefaultCouldBeDerived: DefaultCouldBeDerived::default(),
Expand All @@ -206,6 +206,7 @@ late_lint_methods!(
UnitBindings: UnitBindings,
NonUpperCaseGlobals: NonUpperCaseGlobals,
NonShorthandFieldPatterns: NonShorthandFieldPatterns,
SelfTypeConversion<'tcx>: SelfTypeConversion::new(),
UnusedAllocation: UnusedAllocation,
// Depends on types used in type definitions
MissingCopyImplementations: MissingCopyImplementations,
Expand Down Expand Up @@ -275,6 +276,7 @@ fn register_builtins(store: &mut LintStore) {
store.register_lints(&foreign_modules::get_lints());
store.register_lints(&HardwiredLints::lint_vec());

store.register_late_pass(move |_tcx| Box::new(SelfTypeConversion::new()));
add_lint_group!(
"nonstandard_style",
NON_CAMEL_CASE_TYPES,
Expand Down Expand Up @@ -305,6 +307,7 @@ fn register_builtins(store: &mut LintStore) {
UNUSED_PARENS,
UNUSED_BRACES,
REDUNDANT_SEMICOLONS,
// SELF_TYPE_CONVERSION,
MAP_UNIT_FN
);

Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ pub(crate) struct BuiltinNonShorthandFieldPatterns {
pub prefix: &'static str,
}

#[derive(LintDiagnostic)]
#[diag(lint_self_type_conversion)]
pub(crate) struct SelfTypeConversionDiag<'t> {
pub source: Ty<'t>,
pub target: Ty<'t>,
}

#[derive(LintDiagnostic)]
pub(crate) enum BuiltinUnsafe {
#[diag(lint_builtin_allow_internal_unsafe)]
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_lint/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ macro_rules! expand_combined_late_lint_pass_methods {
/// runtime.
#[macro_export]
macro_rules! declare_combined_late_lint_pass {
([$v:vis $name:ident, [$($pass:ident: $constructor:expr,)*]], $methods:tt) => (
([$v:vis $name:ident$(<$lt:lifetime>)?, [$($pass:ident$(<$inner_lt:lifetime>)?: $constructor:expr,)*]], $methods:tt) => (
#[allow(non_snake_case)]
$v struct $name {
$($pass: $pass,)*
$v struct $name$(<$lt>)? {
$($pass: $pass$(<$inner_lt>)?,)*
}

impl $name {
impl$(<$lt>)? $name$(<$lt>)? {
$v fn new() -> Self {
Self {
$($pass: $constructor,)*
Expand All @@ -115,12 +115,12 @@ macro_rules! declare_combined_late_lint_pass {
}
}

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

#[allow(rustc::lint_pass_impl_without_macro)]
impl $crate::LintPass for $name {
impl$(<$lt>)? $crate::LintPass for $name$(<$lt>)? {
fn name(&self) -> &'static str {
panic!()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ impl Subdiagnostic for LocalLabel<'_> {
for dtor in self.destructors {
dtor.add_to_diag_with(diag, f);
}
let msg = f(diag, crate::fluent_generated::mir_transform_label_local_epilogue.into());
let msg = f(diag, crate::fluent_generated::mir_transform_label_local_epilogue);
diag.span_label(self.span, msg);
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,7 @@ symbols! {
integer_: "integer", // underscore to avoid clashing with the function `sym::integer` below
integral,
into_async_iter_into_iter,
into_fn,
into_future,
into_iter,
intra_doc_pointers,
Expand Down
1 change: 1 addition & 0 deletions library/core/src/convert/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ pub trait AsMut<T: ?Sized> {
#[doc(search_unbox)]
pub trait Into<T>: Sized {
/// Converts this type into the (usually inferred) input type.
#[rustc_diagnostic_item = "into_fn"]
#[must_use]
#[stable(feature = "rust1", since = "1.0.0")]
fn into(self) -> T;
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/os/fd/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#![stable(feature = "io_safety", since = "1.63.0")]
#![deny(unsafe_op_in_unsafe_fn)]

#![cfg_attr(not(bootstrap), allow(self_type_conversion))]
use super::raw::{AsRawFd, FromRawFd, IntoRawFd, RawFd};
use crate::marker::PhantomData;
use crate::mem::ManuallyDrop;
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys/pal/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ impl Command {
// have the drop glue anyway because this code never returns (the
// child will either exec() or invoke libc::exit)
#[cfg(not(any(target_os = "tvos", target_os = "watchos")))]
#[cfg_attr(not(bootstrap), allow(self_type_conversion))]
unsafe fn do_exec(
&mut self,
stdio: ChildPipes,
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys_common/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ impl fmt::Debug for CommandEnv {

impl CommandEnv {
// Capture the current environment with these changes applied
#[cfg_attr(not(bootstrap), allow(self_type_conversion))]
pub fn capture(&self) -> BTreeMap<EnvKey, OsString> {
let mut result = BTreeMap::<EnvKey, OsString>::new();
if !self.clear {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn main() -> Result<(), ()> {
return Ok::<(), ()>(());
Err(())?;
Ok::<(), ()>(());
return Err(().into());
return Err(());
external! {
return Err(())?;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn main() -> Result<(), ()> {
return Ok::<(), ()>(());
Err(())?;
Ok::<(), ()>(());
return Err(().into());
return Err(());
external! {
return Err(())?;
}
Expand Down
Loading

0 comments on commit ecb994b

Please sign in to comment.