Auto merge of #13067 - Jarcho:misc_small3, r=Manishearth

Misc refactorings part 3

And some more changes. Same general idea of checking the HIR tree first when linting.

changelog: none
This commit is contained in:
bors 2024-07-08 17:52:47 +00:00
commit 510cfe3dc4
11 changed files with 120 additions and 161 deletions

View File

@ -91,10 +91,6 @@
impl<'tcx> LateLintPass<'tcx> for InherentToString {
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
if impl_item.span.from_expansion() {
return;
}
// Check if item is a method called `to_string` and has a parameter 'self'
if let ImplItemKind::Fn(ref signature, _) = impl_item.kind
// #11201
@ -106,6 +102,7 @@ fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<
&& decl.implicit_self.has_implicit_self()
&& decl.inputs.len() == 1
&& impl_item.generics.params.iter().all(|p| matches!(p.kind, GenericParamKind::Lifetime { .. }))
&& !impl_item.span.from_expansion()
// Check if return type is String
&& is_type_lang_item(cx, return_ty(cx, impl_item.owner_id), LangItem::String)
// Filters instances of to_string which are required by a trait

View File

@ -2,12 +2,11 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::sugg::DiagExt;
use rustc_ast::ast::Attribute;
use rustc_errors::Applicability;
use rustc_hir::{TraitFn, TraitItem, TraitItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::{sym, Symbol};
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
@ -34,27 +33,23 @@
impl<'tcx> LateLintPass<'tcx> for InlineFnWithoutBody {
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
if let TraitItemKind::Fn(_, TraitFn::Required(_)) = item.kind {
let attrs = cx.tcx.hir().attrs(item.hir_id());
check_attrs(cx, item.ident.name, attrs);
if let TraitItemKind::Fn(_, TraitFn::Required(_)) = item.kind
&& let Some(attr) = cx
.tcx
.hir()
.attrs(item.hir_id())
.iter()
.find(|a| a.has_name(sym::inline))
{
span_lint_and_then(
cx,
INLINE_FN_WITHOUT_BODY,
attr.span,
format!("use of `#[inline]` on trait method `{}` which has no body", item.ident),
|diag| {
diag.suggest_remove_item(cx, attr.span, "remove", Applicability::MachineApplicable);
},
);
}
}
}
fn check_attrs(cx: &LateContext<'_>, name: Symbol, attrs: &[Attribute]) {
for attr in attrs {
if !attr.has_name(sym::inline) {
continue;
}
span_lint_and_then(
cx,
INLINE_FN_WITHOUT_BODY,
attr.span,
format!("use of `#[inline]` on trait method `{name}` which has no body"),
|diag| {
diag.suggest_remove_item(cx, attr.span, "remove", Applicability::MachineApplicable);
},
);
}
}

View File

@ -85,16 +85,19 @@ fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
lhs,
rhs,
) = expr.kind
&& let typeck = cx.typeck_results()
&& ty::is_type_diagnostic_item(cx, typeck.expr_ty(lhs), sym::Instant)
{
let rhs_ty = typeck.expr_ty(rhs);
if is_instant_now_call(cx, lhs)
&& is_an_instant(cx, rhs)
&& ty::is_type_diagnostic_item(cx, rhs_ty, sym::Instant)
&& let Some(sugg) = Sugg::hir_opt(cx, rhs)
{
print_manual_instant_elapsed_sugg(cx, expr, sugg);
} else if !expr.span.from_expansion()
} else if ty::is_type_diagnostic_item(cx, rhs_ty, sym::Duration)
&& !expr.span.from_expansion()
&& self.msrv.meets(msrvs::TRY_FROM)
&& is_an_instant(cx, lhs)
&& is_a_duration(cx, rhs)
{
print_unchecked_duration_subtraction_sugg(cx, lhs, rhs, expr);
}
@ -115,16 +118,6 @@ fn is_instant_now_call(cx: &LateContext<'_>, expr_block: &'_ Expr<'_>) -> bool {
}
}
fn is_an_instant(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let expr_ty = cx.typeck_results().expr_ty(expr);
ty::is_type_diagnostic_item(cx, expr_ty, sym::Instant)
}
fn is_a_duration(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let expr_ty = cx.typeck_results().expr_ty(expr);
ty::is_type_diagnostic_item(cx, expr_ty, sym::Duration)
}
fn print_manual_instant_elapsed_sugg(cx: &LateContext<'_>, expr: &Expr<'_>, sugg: Sugg<'_>) {
span_lint_and_sugg(
cx,

View File

@ -54,36 +54,35 @@
impl LateLintPass<'_> for ItemsAfterStatements {
fn check_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>) {
if in_external_macro(cx.sess(), block.span) {
return;
}
// skip initial items
let stmts = block
.stmts
.iter()
.skip_while(|stmt| matches!(stmt.kind, StmtKind::Item(..)));
// lint on all further items
for stmt in stmts {
if let StmtKind::Item(item_id) = stmt.kind {
let item = cx.tcx.hir().item(item_id);
if in_external_macro(cx.sess(), item.span) || !item.span.eq_ctxt(block.span) {
return;
}
if let ItemKind::Macro(..) = item.kind {
// do not lint `macro_rules`, but continue processing further statements
continue;
}
span_lint_hir(
cx,
ITEMS_AFTER_STATEMENTS,
item.hir_id(),
item.span,
"adding items after statements is confusing, since items exist from the \
start of the scope",
);
}
if block.stmts.len() > 1 {
let ctxt = block.span.ctxt();
let mut in_external = None;
block
.stmts
.iter()
.skip_while(|stmt| matches!(stmt.kind, StmtKind::Item(..)))
.filter_map(|stmt| match stmt.kind {
StmtKind::Item(id) => Some(cx.tcx.hir().item(id)),
_ => None,
})
// Ignore macros since they can only see previously defined locals.
.filter(|item| !matches!(item.kind, ItemKind::Macro(..)))
// Stop linting if macros define items.
.take_while(|item| item.span.ctxt() == ctxt)
// Don't use `next` due to the complex filter chain.
.for_each(|item| {
// Only do the macro check once, but delay it until it's needed.
if !*in_external.get_or_insert_with(|| in_external_macro(cx.sess(), block.span)) {
span_lint_hir(
cx,
ITEMS_AFTER_STATEMENTS,
item.hir_id(),
item.span,
"adding items after statements is confusing, since items exist from the \
start of the scope",
);
}
});
}
}
}

View File

@ -4,7 +4,7 @@
use rustc_hir::{FnSig, ImplItem, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::symbol::sym;
use rustc_span::{sym, Symbol};
declare_clippy_lint! {
/// ### What it does
@ -43,30 +43,27 @@
impl<'tcx> LateLintPass<'tcx> for IterNotReturningIterator {
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
let name = item.ident.name.as_str();
if matches!(name, "iter" | "iter_mut") {
if let TraitItemKind::Fn(fn_sig, _) = &item.kind {
check_sig(cx, name, fn_sig, item.owner_id.def_id);
}
if let TraitItemKind::Fn(fn_sig, _) = &item.kind
&& matches!(item.ident.name, sym::iter | sym::iter_mut)
{
check_sig(cx, item.ident.name, fn_sig, item.owner_id.def_id);
}
}
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) {
let name = item.ident.name.as_str();
if matches!(name, "iter" | "iter_mut")
if let ImplItemKind::Fn(fn_sig, _) = &item.kind
&& matches!(item.ident.name, sym::iter | sym::iter_mut)
&& !matches!(
cx.tcx.parent_hir_node(item.hir_id()),
Node::Item(Item { kind: ItemKind::Impl(i), .. }) if i.of_trait.is_some()
)
{
if let ImplItemKind::Fn(fn_sig, _) = &item.kind {
check_sig(cx, name, fn_sig, item.owner_id.def_id);
}
check_sig(cx, item.ident.name, fn_sig, item.owner_id.def_id);
}
}
}
fn check_sig(cx: &LateContext<'_>, name: &str, sig: &FnSig<'_>, fn_id: LocalDefId) {
fn check_sig(cx: &LateContext<'_>, name: Symbol, sig: &FnSig<'_>, fn_id: LocalDefId) {
if sig.decl.implicit_self.has_implicit_self() {
let ret_ty = cx
.tcx

View File

@ -125,13 +125,13 @@ fn is_ty_exported(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
impl LateLintPass<'_> for IterWithoutIntoIter {
fn check_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::Item<'_>) {
if !in_external_macro(cx.sess(), item.span)
&& let ItemKind::Impl(imp) = item.kind
if let ItemKind::Impl(imp) = item.kind
&& let TyKind::Ref(_, self_ty_without_ref) = &imp.self_ty.kind
&& let Some(trait_ref) = imp.of_trait
&& trait_ref
.trait_def_id()
.is_some_and(|did| cx.tcx.is_diagnostic_item(sym::IntoIterator, did))
&& !in_external_macro(cx.sess(), item.span)
&& let &ty::Ref(_, ty, mtbl) = cx.tcx.type_of(item.owner_id).instantiate_identity().kind()
&& let expected_method_name = match mtbl {
Mutability::Mut => sym::iter_mut,

View File

@ -46,12 +46,12 @@ pub fn new(maximum_allowed_size: u128) -> Self {
impl<'tcx> LateLintPass<'tcx> for LargeConstArrays {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
if !item.span.from_expansion()
&& let ItemKind::Const(_, generics, _) = &item.kind
if let ItemKind::Const(_, generics, _) = &item.kind
// Since static items may not have generics, skip generic const items.
// FIXME(generic_const_items): I don't think checking `generics.hwcp` suffices as it
// doesn't account for empty where-clauses that only consist of keyword `where` IINM.
&& generics.params.is_empty() && !generics.has_where_clause_predicates
&& !item.span.from_expansion()
&& let ty = cx.tcx.type_of(item.owner_id).instantiate_identity()
&& let ty::Array(element_type, cst) = ty.kind()
&& let ConstKind::Value(_, ty::ValTree::Leaf(element_count)) = cst.kind()

View File

@ -77,17 +77,12 @@ pub fn new(maximum_size_difference_allowed: u64) -> Self {
impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) {
if in_external_macro(cx.tcx.sess, item.span) {
return;
}
if let ItemKind::Enum(ref def, _) = item.kind {
let ty = cx.tcx.type_of(item.owner_id).instantiate_identity();
let ty::Adt(adt, subst) = ty.kind() else {
panic!("already checked whether this is an enum")
};
if adt.variants().len() <= 1 {
return;
}
if let ItemKind::Enum(ref def, _) = item.kind
&& let ty = cx.tcx.type_of(item.owner_id).instantiate_identity()
&& let ty::Adt(adt, subst) = ty.kind()
&& adt.variants().len() > 1
&& !in_external_macro(cx.tcx.sess, item.span)
{
let variants_size = AdtVariantInfo::new(cx, *adt, subst);
let mut difference = variants_size[0].size - variants_size[1].size;

View File

@ -54,29 +54,26 @@ pub fn new(future_size_threshold: u64) -> Self {
impl<'tcx> LateLintPass<'tcx> for LargeFuture {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if matches!(expr.span.ctxt().outer_expn_data().kind, rustc_span::ExpnKind::Macro(..)) {
return;
}
if let ExprKind::Match(expr, _, MatchSource::AwaitDesugar) = expr.kind {
if let ExprKind::Call(func, [expr, ..]) = expr.kind
&& let ExprKind::Path(QPath::LangItem(LangItem::IntoFutureIntoFuture, ..)) = func.kind
&& let ty = cx.typeck_results().expr_ty(expr)
&& let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait()
&& implements_trait(cx, ty, future_trait_def_id, &[])
&& let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty))
&& let size = layout.layout.size()
&& size >= Size::from_bytes(self.future_size_threshold)
{
span_lint_and_sugg(
cx,
LARGE_FUTURES,
expr.span,
format!("large future with a size of {} bytes", size.bytes()),
"consider `Box::pin` on it",
format!("Box::pin({})", snippet(cx, expr.span, "..")),
Applicability::Unspecified,
);
}
if let ExprKind::Match(scrutinee, _, MatchSource::AwaitDesugar) = expr.kind
&& let ExprKind::Call(func, [arg, ..]) = scrutinee.kind
&& let ExprKind::Path(QPath::LangItem(LangItem::IntoFutureIntoFuture, ..)) = func.kind
&& !expr.span.from_expansion()
&& let ty = cx.typeck_results().expr_ty(arg)
&& let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait()
&& implements_trait(cx, ty, future_trait_def_id, &[])
&& let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty))
&& let size = layout.layout.size()
&& size >= Size::from_bytes(self.future_size_threshold)
{
span_lint_and_sugg(
cx,
LARGE_FUTURES,
arg.span,
format!("large future with a size of {} bytes", size.bytes()),
"consider `Box::pin` on it",
format!("Box::pin({})", snippet(cx, arg.span, "..")),
Applicability::Unspecified,
);
}
}
}

View File

@ -1,5 +1,4 @@
use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::is_lint_allowed;
use clippy_utils::macros::root_macro_call_first_node;
use rustc_ast::LitKind;
use rustc_hir::{Expr, ExprKind};
@ -52,24 +51,19 @@ pub fn new(max_file_size: u64) -> Self {
impl LateLintPass<'_> for LargeIncludeFile {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
if let Some(macro_call) = root_macro_call_first_node(cx, expr)
&& !is_lint_allowed(cx, LARGE_INCLUDE_FILE, expr.hir_id)
&& (cx.tcx.is_diagnostic_item(sym::include_bytes_macro, macro_call.def_id)
|| cx.tcx.is_diagnostic_item(sym::include_str_macro, macro_call.def_id))
&& let ExprKind::Lit(lit) = &expr.kind
{
let len = match &lit.node {
if let ExprKind::Lit(lit) = &expr.kind
&& let len = match &lit.node {
// include_bytes
LitKind::ByteStr(bstr, _) => bstr.len(),
// include_str
LitKind::Str(sym, _) => sym.as_str().len(),
_ => return,
};
if len as u64 <= self.max_file_size {
return;
}
&& len as u64 > self.max_file_size
&& let Some(macro_call) = root_macro_call_first_node(cx, expr)
&& (cx.tcx.is_diagnostic_item(sym::include_bytes_macro, macro_call.def_id)
|| cx.tcx.is_diagnostic_item(sym::include_str_macro, macro_call.def_id))
{
span_lint_and_note(
cx,
LARGE_INCLUDE_FILE,

View File

@ -48,15 +48,11 @@ pub fn new(msrv: Msrv) -> Self {
impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
let Self { msrv } = self;
if !msrv.meets(NUMERIC_ASSOCIATED_CONSTANTS) || in_external_macro(cx.sess(), item.span) {
return;
}
// Integer modules are "TBD" deprecated, and the contents are too,
// so lint on the `use` statement directly.
if let ItemKind::Use(path, kind @ (UseKind::Single | UseKind::Glob)) = item.kind
&& self.msrv.meets(NUMERIC_ASSOCIATED_CONSTANTS)
&& !in_external_macro(cx.sess(), item.span)
&& let Some(def_id) = path.res[0].opt_def_id()
{
let module = if is_integer_module(cx, def_id) {
@ -103,12 +99,7 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
}
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
let Self { msrv } = self;
if !msrv.meets(NUMERIC_ASSOCIATED_CONSTANTS) || in_external_macro(cx.sess(), expr.span) {
return;
}
let ExprKind::Path(qpath) = expr.kind else {
let ExprKind::Path(qpath) = &expr.kind else {
return;
};
@ -129,10 +120,10 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tc
)
// `<integer>::xxx_value` check
} else if let QPath::TypeRelative(_, last_segment) = qpath
&& let Some(def_id) = cx.qpath_res(&qpath, expr.hir_id).opt_def_id()
&& is_integer_method(cx, def_id)
&& let Some(def_id) = cx.qpath_res(qpath, expr.hir_id).opt_def_id()
&& let Some(par_expr) = get_parent_expr(cx, expr)
&& let ExprKind::Call(_, _) = par_expr.kind
&& let ExprKind::Call(_, []) = par_expr.kind
&& is_integer_method(cx, def_id)
{
let name = last_segment.ident.name.as_str();
@ -145,19 +136,20 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tc
return;
};
if is_from_proc_macro(cx, expr) {
return;
if self.msrv.meets(NUMERIC_ASSOCIATED_CONSTANTS)
&& !in_external_macro(cx.sess(), expr.span)
&& !is_from_proc_macro(cx, expr)
{
span_lint_hir_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.hir_id, span, msg, |diag| {
diag.span_suggestion_with_style(
span,
"use the associated constant instead",
sugg,
Applicability::MaybeIncorrect,
SuggestionStyle::ShowAlways,
);
});
}
span_lint_hir_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.hir_id, span, msg, |diag| {
diag.span_suggestion_with_style(
span,
"use the associated constant instead",
sugg,
Applicability::MaybeIncorrect,
SuggestionStyle::ShowAlways,
);
});
}
extract_msrv_attr!(LateContext);