Auto merge of #13050 - Jarcho:misc_small, r=xFrednet

Misc refactorings

Various small re-orderings to check the HIR tree or AST before doing other checks. Also includes a small bug fix for `arc_with_small_send_sync` not actually checking for `Arc::new`.

changelog: none
This commit is contained in:
bors 2024-07-22 20:25:05 +00:00
commit df1baedfde
14 changed files with 98 additions and 121 deletions

View file

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_from_proc_macro;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{is_from_proc_macro, last_path_segment};
use rustc_hir::{Expr, ExprKind};
use rustc_hir::{Expr, ExprKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_middle::ty::print::with_forced_trimmed_paths;
@ -42,12 +42,11 @@ declare_lint_pass!(ArcWithNonSendSync => [ARC_WITH_NON_SEND_SYNC]);
impl<'tcx> LateLintPass<'tcx> for ArcWithNonSendSync {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if !expr.span.from_expansion()
&& let ty = cx.typeck_results().expr_ty(expr)
&& is_type_diagnostic_item(cx, ty, sym::Arc)
&& let ExprKind::Call(func, [arg]) = expr.kind
&& let ExprKind::Path(func_path) = func.kind
&& last_path_segment(&func_path).ident.name == sym::new
if let ExprKind::Call(func, [arg]) = expr.kind
&& let ExprKind::Path(QPath::TypeRelative(func_ty, func_name)) = func.kind
&& func_name.ident.name == sym::new
&& !expr.span.from_expansion()
&& is_type_diagnostic_item(cx, cx.typeck_results().node_type(func_ty.hir_id), sym::Arc)
&& let arg_ty = cx.typeck_results().expr_ty(arg)
// make sure that the type is not and does not contain any type parameters
&& arg_ty.walk().all(|arg| {

View file

@ -49,35 +49,31 @@ declare_lint_pass!(BorrowDerefRef => [BORROW_DEREF_REF]);
impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &rustc_hir::Expr<'tcx>) {
if !e.span.from_expansion()
&& let ExprKind::AddrOf(_, Mutability::Not, addrof_target) = e.kind
&& !addrof_target.span.from_expansion()
if let ExprKind::AddrOf(_, Mutability::Not, addrof_target) = e.kind
&& let ExprKind::Unary(UnOp::Deref, deref_target) = addrof_target.kind
&& !deref_target.span.from_expansion()
&& !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..))
&& !e.span.from_expansion()
&& !deref_target.span.from_expansion()
&& !addrof_target.span.from_expansion()
&& let ref_ty = cx.typeck_results().expr_ty(deref_target)
&& let ty::Ref(_, inner_ty, Mutability::Not) = ref_ty.kind()
&& get_parent_expr(cx, e).map_or(true, |parent| {
match parent.kind {
// `*&*foo` should lint `deref_addrof` instead.
ExprKind::Unary(UnOp::Deref, _) => is_lint_allowed(cx, DEREF_ADDROF, parent.hir_id),
// `&*foo` creates a distinct temporary from `foo`
ExprKind::AddrOf(_, Mutability::Mut, _) => !matches!(
deref_target.kind,
ExprKind::Path(..)
| ExprKind::Field(..)
| ExprKind::Index(..)
| ExprKind::Unary(UnOp::Deref, ..)
),
_ => true,
}
})
&& !is_from_proc_macro(cx, e)
{
if let Some(parent_expr) = get_parent_expr(cx, e) {
if matches!(parent_expr.kind, ExprKind::Unary(UnOp::Deref, ..))
&& !is_lint_allowed(cx, DEREF_ADDROF, parent_expr.hir_id)
{
return;
}
// modification to `&mut &*x` is different from `&mut x`
if matches!(
deref_target.kind,
ExprKind::Path(..) | ExprKind::Field(..) | ExprKind::Index(..) | ExprKind::Unary(UnOp::Deref, ..)
) && matches!(parent_expr.kind, ExprKind::AddrOf(_, Mutability::Mut, _))
{
return;
}
}
if is_from_proc_macro(cx, e) {
return;
}
span_lint_and_then(
cx,
BORROW_DEREF_REF,

View file

@ -93,20 +93,14 @@ declare_lint_pass!(CollapsibleIf => [COLLAPSIBLE_IF, COLLAPSIBLE_ELSE_IF]);
impl EarlyLintPass for CollapsibleIf {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
if !expr.span.from_expansion() {
check_if(cx, expr);
}
}
}
fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
if let ast::ExprKind::If(check, then, else_) = &expr.kind {
if let Some(else_) = else_ {
check_collapsible_maybe_if_let(cx, then.span, else_);
} else if let ast::ExprKind::Let(..) = check.kind {
// Prevent triggering on `if let a = b { if c { .. } }`.
} else {
check_collapsible_no_if_let(cx, expr, check, then);
if let ast::ExprKind::If(cond, then, else_) = &expr.kind
&& !expr.span.from_expansion()
{
if let Some(else_) = else_ {
check_collapsible_maybe_if_let(cx, then.span, else_);
} else if !matches!(cond.kind, ast::ExprKind::Let(..)) {
check_collapsible_no_if_let(cx, expr, cond, then);
}
}
}
}
@ -189,13 +183,10 @@ fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &
/// If the block contains only one expression, return it.
fn expr_block(block: &ast::Block) -> Option<&ast::Expr> {
let mut it = block.stmts.iter();
if let (Some(stmt), None) = (it.next(), it.next()) {
match stmt.kind {
ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => Some(expr),
_ => None,
}
if let [stmt] = &*block.stmts
&& let ast::StmtKind::Expr(expr) | ast::StmtKind::Semi(expr) = &stmt.kind
{
Some(expr)
} else {
None
}

View file

@ -59,9 +59,9 @@ static COLLECTIONS: [Symbol; 9] = [
impl<'tcx> LateLintPass<'tcx> for CollectionIsNeverRead {
fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx LetStmt<'tcx>) {
// Look for local variables whose type is a container. Search surrounding bock for read access.
if match_acceptable_type(cx, local, &COLLECTIONS)
&& let PatKind::Binding(_, local_id, _, _) = local.pat.kind
// Look for local variables whose type is a container. Search surrounding block for read access.
if let PatKind::Binding(_, local_id, _, _) = local.pat.kind
&& match_acceptable_type(cx, local, &COLLECTIONS)
&& let Some(enclosing_block) = get_enclosing_block(cx, local.hir_id)
&& has_no_read_access(cx, local_id, enclosing_block)
{

View file

@ -53,10 +53,9 @@ declare_lint_pass!(CrateInMacroDef => [CRATE_IN_MACRO_DEF]);
impl EarlyLintPass for CrateInMacroDef {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
if item.attrs.iter().any(is_macro_export)
&& let ItemKind::MacroDef(macro_def) = &item.kind
&& let tts = macro_def.body.tokens.clone()
&& let Some(span) = contains_unhygienic_crate_reference(&tts)
if let ItemKind::MacroDef(macro_def) = &item.kind
&& item.attrs.iter().any(is_macro_export)
&& let Some(span) = contains_unhygienic_crate_reference(&macro_def.body.tokens)
{
span_lint_and_sugg(
cx,

View file

@ -50,12 +50,9 @@ declare_lint_pass!(ElseIfWithoutElse => [ELSE_IF_WITHOUT_ELSE]);
impl EarlyLintPass for ElseIfWithoutElse {
fn check_expr(&mut self, cx: &EarlyContext<'_>, item: &Expr) {
if in_external_macro(cx.sess(), item.span) {
return;
}
if let ExprKind::If(_, _, Some(ref els)) = item.kind
&& let ExprKind::If(_, _, None) = els.kind
&& !in_external_macro(cx.sess(), item.span)
{
span_lint_and_help(
cx,

View file

@ -64,25 +64,21 @@ declare_lint_pass!(EmptyEnum => [EMPTY_ENUM]);
impl<'tcx> LateLintPass<'tcx> for EmptyEnum {
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
// Only suggest the `never_type` if the feature is enabled
if !cx.tcx.features().never_type {
return;
}
if let ItemKind::Enum(..) = item.kind {
let ty = cx.tcx.type_of(item.owner_id).instantiate_identity();
let adt = ty.ty_adt_def().expect("already checked whether this is an enum");
if adt.variants().is_empty() {
span_lint_and_help(
cx,
EMPTY_ENUM,
item.span,
"enum with no variants",
None,
"consider using the uninhabited type `!` (never type) or a wrapper \
around it to introduce a type which can't be instantiated",
);
}
if let ItemKind::Enum(..) = item.kind
// Only suggest the `never_type` if the feature is enabled
&& cx.tcx.features().never_type
&& let Some(adt) = cx.tcx.type_of(item.owner_id).instantiate_identity().ty_adt_def()
&& adt.variants().is_empty()
{
span_lint_and_help(
cx,
EMPTY_ENUM,
item.span,
"enum with no variants",
None,
"consider using the uninhabited type `!` (never type) or a wrapper \
around it to introduce a type which can't be instantiated",
);
}
}
}

View file

@ -70,9 +70,9 @@ fn is_structural_partial_eq<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, other: T
impl<'tcx> LateLintPass<'tcx> for PatternEquality {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if !in_external_macro(cx.sess(), expr.span)
&& let ExprKind::Let(let_expr) = expr.kind
if let ExprKind::Let(let_expr) = expr.kind
&& unary_pattern(let_expr.pat)
&& !in_external_macro(cx.sess(), expr.span)
{
let exp_ty = cx.typeck_results().expr_ty(let_expr.init);
let pat_ty = cx.typeck_results().pat_ty(let_expr.pat);

View file

@ -36,15 +36,12 @@ declare_lint_pass!(ErrorImplError => [ERROR_IMPL_ERROR]);
impl<'tcx> LateLintPass<'tcx> for ErrorImplError {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
let Some(error_def_id) = cx.tcx.get_diagnostic_item(sym::Error) else {
return;
};
match item.kind {
ItemKind::TyAlias(..)
if item.ident.name == sym::Error
&& is_visible_outside_module(cx, item.owner_id.def_id)
&& let ty = cx.tcx.type_of(item.owner_id).instantiate_identity()
&& let Some(error_def_id) = cx.tcx.get_diagnostic_item(sym::Error)
&& implements_trait(cx, ty, error_def_id, &[]) =>
{
span_lint(
@ -56,9 +53,9 @@ impl<'tcx> LateLintPass<'tcx> for ErrorImplError {
},
ItemKind::Impl(imp)
if let Some(trait_def_id) = imp.of_trait.and_then(|t| t.trait_def_id())
&& let Some(error_def_id) = cx.tcx.get_diagnostic_item(sym::Error)
&& error_def_id == trait_def_id
&& let Some(def_id) = path_res(cx, imp.self_ty).opt_def_id().and_then(DefId::as_local)
&& let hir_id = cx.tcx.local_def_id_to_hir_id(def_id)
&& let Some(ident) = cx.tcx.opt_item_ident(def_id.to_def_id())
&& ident.name == sym::Error
&& is_visible_outside_module(cx, def_id) =>
@ -66,7 +63,7 @@ impl<'tcx> LateLintPass<'tcx> for ErrorImplError {
span_lint_hir_and_then(
cx,
ERROR_IMPL_ERROR,
hir_id,
cx.tcx.local_def_id_to_hir_id(def_id),
ident.span,
"exported type named `Error` that implements `Error`",
|diag| {

View file

@ -70,20 +70,24 @@ declare_lint_pass!(ExhaustiveItems => [EXHAUSTIVE_ENUMS, EXHAUSTIVE_STRUCTS]);
impl LateLintPass<'_> for ExhaustiveItems {
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if let ItemKind::Enum(..) | ItemKind::Struct(..) = item.kind
&& cx.effective_visibilities.is_exported(item.owner_id.def_id)
let (lint, msg, fields) = match item.kind {
ItemKind::Enum(..) => (
EXHAUSTIVE_ENUMS,
"exported enums should not be exhaustive",
[].as_slice(),
),
ItemKind::Struct(v, ..) => (
EXHAUSTIVE_STRUCTS,
"exported structs should not be exhaustive",
v.fields(),
),
_ => return,
};
if cx.effective_visibilities.is_exported(item.owner_id.def_id)
&& let attrs = cx.tcx.hir().attrs(item.hir_id())
&& !attrs.iter().any(|a| a.has_name(sym::non_exhaustive))
&& fields.iter().all(|f| cx.tcx.visibility(f.def_id).is_public())
{
let (lint, msg) = if let ItemKind::Struct(ref v, ..) = item.kind {
if v.fields().iter().any(|f| !cx.tcx.visibility(f.def_id).is_public()) {
// skip structs with private fields
return;
}
(EXHAUSTIVE_STRUCTS, "exported structs should not be exhaustive")
} else {
(EXHAUSTIVE_ENUMS, "exported enums should not be exhaustive")
};
let suggestion_span = item.span.shrink_to_lo();
let indent = " ".repeat(indent_of(cx, item.span).unwrap_or(0));
span_lint_and_then(cx, lint, item.span, msg, |diag| {

View file

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::is_entrypoint_fn;
use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node};
use rustc_hir::{Expr, ExprKind, Item, ItemKind, OwnerNode};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::sym;
@ -47,8 +47,8 @@ impl<'tcx> LateLintPass<'tcx> for Exit {
&& let ExprKind::Path(ref path) = path_expr.kind
&& let Some(def_id) = cx.qpath_res(path, path_expr.hir_id).opt_def_id()
&& cx.tcx.is_diagnostic_item(sym::process_exit, def_id)
&& let parent = cx.tcx.hir().get_parent_item(e.hir_id).def_id
&& let Node::Item(Item{kind: ItemKind::Fn(..), ..}) = cx.tcx.hir_node_by_def_id(parent)
&& let parent = cx.tcx.hir().get_parent_item(e.hir_id)
&& let OwnerNode::Item(Item{kind: ItemKind::Fn(..), ..}) = cx.tcx.hir_owner_node(parent)
// If the next item up is a function we check if it is an entry point
// and only then emit a linter warning
&& !is_entrypoint_fn(cx, parent.to_def_id())

View file

@ -62,10 +62,9 @@ declare_lint_pass!(FloatLiteral => [EXCESSIVE_PRECISION, LOSSY_FLOAT_LITERAL]);
impl<'tcx> LateLintPass<'tcx> for FloatLiteral {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
let ty = cx.typeck_results().expr_ty(expr);
if let ty::Float(fty) = *ty.kind()
&& let hir::ExprKind::Lit(lit) = expr.kind
if let hir::ExprKind::Lit(lit) = expr.kind
&& let LitKind::Float(sym, lit_float_ty) = lit.node
&& let ty::Float(fty) = *cx.typeck_results().expr_ty(expr).kind()
{
let sym_str = sym.as_str();
let formatter = FloatFormat::new(sym_str);

View file

@ -66,10 +66,6 @@ impl_lint_pass!(FromOverInto => [FROM_OVER_INTO]);
impl<'tcx> LateLintPass<'tcx> for FromOverInto {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
if !self.msrv.meets(msrvs::RE_REBALANCING_COHERENCE) || !span_is_local(item.span) {
return;
}
if let ItemKind::Impl(Impl {
of_trait: Some(hir_trait_ref),
self_ty,
@ -79,6 +75,8 @@ impl<'tcx> LateLintPass<'tcx> for FromOverInto {
&& let Some(into_trait_seg) = hir_trait_ref.path.segments.last()
// `impl Into<target_ty> for self_ty`
&& let Some(GenericArgs { args: [GenericArg::Type(target_ty)], .. }) = into_trait_seg.args
&& self.msrv.meets(msrvs::RE_REBALANCING_COHERENCE)
&& span_is_local(item.span)
&& let Some(middle_trait_ref) = cx.tcx.impl_trait_ref(item.owner_id)
.map(ty::EarlyBinder::instantiate_identity)
&& cx.tcx.is_diagnostic_item(sym::Into, middle_trait_ref.def_id)

View file

@ -47,9 +47,13 @@ impl<'tcx> LateLintPass<'tcx> for FromStrRadix10 {
fn check_expr(&mut self, cx: &LateContext<'tcx>, exp: &Expr<'tcx>) {
if let ExprKind::Call(maybe_path, [src, radix]) = &exp.kind
&& let ExprKind::Path(QPath::TypeRelative(ty, pathseg)) = &maybe_path.kind
// do not lint in constant context, because the suggestion won't work.
// NB: keep this check until a new `const_trait_impl` is available and stablized.
&& !in_constant(cx, exp.hir_id)
// check if the second argument is a primitive `10`
&& is_integer_literal(radix, 10)
// check if the second part of the path indeed calls the associated
// function `from_str_radix`
&& pathseg.ident.name.as_str() == "from_str_radix"
// check if the first part of the path is some integer primitive
&& let TyKind::Path(ty_qpath) = &ty.kind
@ -57,12 +61,9 @@ impl<'tcx> LateLintPass<'tcx> for FromStrRadix10 {
&& let def::Res::PrimTy(prim_ty) = ty_res
&& matches!(prim_ty, PrimTy::Int(_) | PrimTy::Uint(_))
// check if the second part of the path indeed calls the associated
// function `from_str_radix`
&& pathseg.ident.name.as_str() == "from_str_radix"
// check if the second argument is a primitive `10`
&& is_integer_literal(radix, 10)
// do not lint in constant context, because the suggestion won't work.
// NB: keep this check until a new `const_trait_impl` is available and stablized.
&& !in_constant(cx, exp.hir_id)
{
let expr = if let ExprKind::AddrOf(_, _, expr) = &src.kind {
let ty = cx.typeck_results().expr_ty(expr);