Skip to content

Commit

Permalink
Auto merge of #13050 - Jarcho:misc_small, r=xFrednet
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Jul 22, 2024
2 parents f9c2407 + 9f59e9a commit df1baed
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 121 deletions.
15 changes: 7 additions & 8 deletions clippy_lints/src/arc_with_non_send_sync.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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| {
Expand Down
44 changes: 20 additions & 24 deletions clippy_lints/src/borrow_deref_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
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;
&& 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,
}

// 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;
}

})
&& !is_from_proc_macro(cx, e)
{
span_lint_and_then(
cx,
BORROW_DEREF_REF,
Expand Down
33 changes: 12 additions & 21 deletions clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
Expand Down Expand Up @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/collection_is_never_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
7 changes: 3 additions & 4 deletions clippy_lints/src/crate_in_macro_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 1 addition & 4 deletions clippy_lints/src/else_if_without_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
34 changes: 15 additions & 19 deletions clippy_lints/src/empty_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
}
}
}
4 changes: 2 additions & 2 deletions clippy_lints/src/equatable_if_let.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 3 additions & 6 deletions clippy_lints/src/error_impl_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -56,17 +53,17 @@ 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) =>
{
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| {
Expand Down
26 changes: 15 additions & 11 deletions clippy_lints/src/exhaustive_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/exit.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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())
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/float_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions clippy_lints/src/from_over_into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down
19 changes: 10 additions & 9 deletions clippy_lints/src/from_str_radix_10.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,23 @@ 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
&& let ty_res = cx.qpath_res(ty_qpath, ty.hir_id)
&& 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);
Expand Down

0 comments on commit df1baed

Please sign in to comment.