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 18, 2024
1 parent 6de928d commit f373402
Show file tree
Hide file tree
Showing 28 changed files with 216 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 @@ -129,6 +129,8 @@ lint_builtin_no_mangle_static = declaration of a `no_mangle` static
lint_builtin_non_shorthand_field_patterns = the `{$ident}:` in this pattern is redundant
.suggestion = use shorthand field pattern
lint_self_type_conversion = this conversion is useless `{$source}` to `{$target}`
lint_builtin_overridden_symbol_name =
the linker's behavior with multiple libraries exporting duplicate symbol names is undefined and Rust cannot provide guarantees when you manually override them
Expand Down
95 changes: 94 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,95 @@ 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;
}
// 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
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 @@ -31,6 +31,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
Loading

0 comments on commit f373402

Please sign in to comment.