Skip to content

Commit

Permalink
[Experimental] <T as Into<T>>::into lint
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
estebank committed Aug 19, 2024
1 parent 6de928d commit 124ff7d
Show file tree
Hide file tree
Showing 28 changed files with 221 additions and 34 deletions.
6 changes: 2 additions & 4 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3464,7 +3464,7 @@ impl From<ForeignItemKind> for ItemKind {
fn from(foreign_item_kind: ForeignItemKind) -> ItemKind {
match foreign_item_kind {
ForeignItemKind::Static(box static_foreign_item) => {
ItemKind::Static(Box::new(static_foreign_item.into()))
ItemKind::Static(Box::new(static_foreign_item))
}
ForeignItemKind::Fn(fn_kind) => ItemKind::Fn(fn_kind),
ForeignItemKind::TyAlias(ty_alias_kind) => ItemKind::TyAlias(ty_alias_kind),
Expand All @@ -3478,9 +3478,7 @@ impl TryFrom<ItemKind> for ForeignItemKind {

fn try_from(item_kind: ItemKind) -> Result<ForeignItemKind, ItemKind> {
Ok(match item_kind {
ItemKind::Static(box static_item) => {
ForeignItemKind::Static(Box::new(static_item.into()))
}
ItemKind::Static(box static_item) => ForeignItemKind::Static(Box::new(static_item)),
ItemKind::Fn(fn_kind) => ForeignItemKind::Fn(fn_kind),
ItemKind::TyAlias(ty_alias_kind) => ForeignItemKind::TyAlias(ty_alias_kind),
ItemKind::MacCall(a) => ForeignItemKind::MacCall(a),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>(
ecx.push_stack_frame_raw(
cid.instance,
body,
&ret.clone().into(),
&ret.clone(),
StackPopCleanup::Root { cleanup: false },
)?;
ecx.storage_live_for_always_live_locals()?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
(Abi::Rust, fn_abi),
&[FnArg::Copy(arg.into())],
false,
&ret.into(),
&ret,
Some(target),
unwind,
)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2533,7 +2533,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
other_generic_param.name.ident() == generic_param.name.ident()
},
) {
idxs_matched.push(other_idx.into());
idxs_matched.push(other_idx);
}

if idxs_matched.is_empty() {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,8 @@ lint_reserved_prefix = prefix `{$prefix}` is unknown
.label = unknown prefix
.suggestion = insert whitespace here to avoid this being parsed as a prefix in Rust 2021
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
100 changes: 99 additions & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use crate::lints::{
BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, InvalidAsmLabel,
BuiltinWhileTrue, InvalidAsmLabel, SelfTypeConversionDiag,
};
use crate::nonstandard_style::{method_context, MethodLateContext};
use crate::{
Expand Down Expand Up @@ -1604,6 +1604,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 @@ -3064,3 +3065,100 @@ 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:?}");
self.ignored_types.push(ty);
for stripped in cx.tcx.stripped_cfg_items(def_id.krate) {
if stripped.name.name == name {
tracing::info!("{name:#?}");
}
}
}
}

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) {
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!(?def_id);
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")
{
// 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 @@ -185,7 +185,7 @@ early_lint_methods!(
late_lint_methods!(
declare_combined_late_lint_pass,
[
BuiltinCombinedModuleLateLintPass,
BuiltinCombinedModuleLateLintPass<'tcx>,
[
ForLoopsOverFallibles: ForLoopsOverFallibles,
DerefIntoDynSupertrait: DerefIntoDynSupertrait,
Expand All @@ -203,6 +203,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 @@ -268,6 +269,7 @@ fn register_builtins(store: &mut LintStore) {
store.register_lints(&BuiltinCombinedModuleLateLintPass::get_lints());
store.register_lints(&foreign_modules::get_lints());

store.register_late_pass(move |_tcx| Box::new(SelfTypeConversion::new()));
add_lint_group!(
"nonstandard_style",
NON_CAMEL_CASE_TYPES,
Expand Down Expand Up @@ -298,6 +300,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 @@ -72,6 +72,13 @@ pub struct BuiltinNonShorthandFieldPatterns {
pub prefix: &'static str,
}

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

#[derive(LintDiagnostic)]
pub 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 @@ -93,13 +93,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 @@ -113,12 +113,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
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ impl<'tcx> Const<'tcx> {
Ok((tcx.type_of(unevaluated.def).instantiate(tcx, unevaluated.args), c))
}
Ok(Err(bad_ty)) => Err(Either::Left(bad_ty)),
Err(err) => Err(Either::Right(err.into())),
Err(err) => Err(Either::Right(err)),
}
}
ConstKind::Value(ty, val) => Ok((ty, val)),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1526,7 +1526,7 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {

let precedence = |binop: rustc_middle::mir::BinOp| {
use rustc_ast::util::parser::AssocOp;
AssocOp::from_ast_binop(binop.to_hir_binop().into()).precedence()
AssocOp::from_ast_binop(binop.to_hir_binop()).precedence()
};
let op_precedence = precedence(op);
let formatted_op = op.to_hir_binop().as_str();
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1586,7 +1586,7 @@ impl<'a> Parser<'a> {
(thin_vec![], Recovered::Yes(guar))
}
};
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
} else if this.check(&token::OpenDelim(Delimiter::Parenthesis)) {
let body = match this.parse_tuple_struct_body() {
Ok(body) => body,
Expand Down Expand Up @@ -1670,7 +1670,7 @@ impl<'a> Parser<'a> {
class_name.span,
generics.where_clause.has_where_token,
)?;
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
}
// No `where` so: `struct Foo<T>;`
} else if self.eat(&token::Semi) {
Expand All @@ -1682,7 +1682,7 @@ impl<'a> Parser<'a> {
class_name.span,
generics.where_clause.has_where_token,
)?;
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
// Tuple-style struct definition with optional where-clause.
} else if self.token == token::OpenDelim(Delimiter::Parenthesis) {
let body = VariantData::Tuple(self.parse_tuple_struct_body()?, DUMMY_NODE_ID);
Expand Down Expand Up @@ -1711,14 +1711,14 @@ impl<'a> Parser<'a> {
class_name.span,
generics.where_clause.has_where_token,
)?;
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
} else if self.token == token::OpenDelim(Delimiter::Brace) {
let (fields, recovered) = self.parse_record_struct_body(
"union",
class_name.span,
generics.where_clause.has_where_token,
)?;
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
} else {
let token_str = super::token_descr(&self.token);
let msg = format!("expected `where` or `{{` after union name, found {token_str}");
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 @@ -1045,6 +1045,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
2 changes: 1 addition & 1 deletion compiler/rustc_symbol_mangling/src/v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ impl<'tcx> Printer<'tcx> for SymbolMangler<'tcx> {
let consts = [
start.unwrap_or(self.tcx.consts.unit),
end.unwrap_or(self.tcx.consts.unit),
ty::Const::from_bool(self.tcx, include_end).into(),
ty::Const::from_bool(self.tcx, include_end),
];
// HACK: Represent as tuple until we have something better.
// HACK: constants are used in arrays, even if the types don't match.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ pub(super) fn opt_normalize_projection_term<'a, 'b, 'tcx>(
debug!("opt_normalize_projection_type: found error");
let result = normalize_to_error(selcx, param_env, projection_term, cause, depth);
obligations.extend(result.obligations);
return Ok(Some(result.value.into()));
return Ok(Some(result.value));
}
}

Expand Down Expand Up @@ -476,7 +476,7 @@ pub(super) fn opt_normalize_projection_term<'a, 'b, 'tcx>(
}
let result = normalize_to_error(selcx, param_env, projection_term, cause, depth);
obligations.extend(result.obligations);
Ok(Some(result.value.into()))
Ok(Some(result.value))
}
}
}
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 @@ -445,6 +445,7 @@ pub trait AsMut<T: ?Sized> {
#[stable(feature = "rust1", since = "1.0.0")]
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
6 changes: 3 additions & 3 deletions library/std/src/os/fd/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ impl From<crate::net::TcpStream> for OwnedFd {
/// Takes ownership of a [`TcpStream`](crate::net::TcpStream)'s socket file descriptor.
#[inline]
fn from(tcp_stream: crate::net::TcpStream) -> OwnedFd {
tcp_stream.into_inner().into_socket().into_inner().into_inner().into()
tcp_stream.into_inner().into_socket().into_inner().into_inner()
}
}

Expand All @@ -355,7 +355,7 @@ impl From<crate::net::TcpListener> for OwnedFd {
/// Takes ownership of a [`TcpListener`](crate::net::TcpListener)'s socket file descriptor.
#[inline]
fn from(tcp_listener: crate::net::TcpListener) -> OwnedFd {
tcp_listener.into_inner().into_socket().into_inner().into_inner().into()
tcp_listener.into_inner().into_socket().into_inner().into_inner()
}
}

Expand All @@ -382,7 +382,7 @@ impl From<crate::net::UdpSocket> for OwnedFd {
/// Takes ownership of a [`UdpSocket`](crate::net::UdpSocket)'s file descriptor.
#[inline]
fn from(udp_socket: crate::net::UdpSocket) -> OwnedFd {
udp_socket.into_inner().into_socket().into_inner().into_inner().into()
udp_socket.into_inner().into_socket().into_inner().into_inner()
}
}

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 @@ -282,6 +282,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
Loading

0 comments on commit 124ff7d

Please sign in to comment.