From 86811e9b1ba28abd8ba00ece1670c7cfd1284817 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Thu, 7 Oct 2021 00:14:06 +0200 Subject: [PATCH] make test module detection more strict --- clippy_utils/src/lib.rs | 42 ++++++++++++-------------------- tests/ui/wildcard_imports.fixed | 8 ++++++ tests/ui/wildcard_imports.rs | 8 ++++++ tests/ui/wildcard_imports.stderr | 8 +++++- 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 00cf8a4d077..673830eea8a 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -252,11 +252,7 @@ pub fn is_lang_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, lang_item: LangItem /// Returns `true` if this `span` was expanded by any macro. #[must_use] pub fn in_macro(span: Span) -> bool { - if span.from_expansion() { - !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..)) - } else { - false - } + span.from_expansion() && !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..)) } pub fn is_unit_expr(expr: &Expr<'_>) -> bool { @@ -1286,10 +1282,9 @@ pub fn is_integer_const(cx: &LateContext<'_>, e: &Expr<'_>, value: u128) -> bool } let enclosing_body = cx.tcx.hir().local_def_id(cx.tcx.hir().enclosing_body_owner(e.hir_id)); if let Some((Constant::Int(v), _)) = constant(cx, cx.tcx.typeck(enclosing_body), e) { - value == v - } else { - false + return value == v; } + false } /// Checks whether the given expression is a constant literal of the given value. @@ -1316,7 +1311,7 @@ pub fn is_adjusted(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { /// Returns the pre-expansion span if is this comes from an expansion of the /// macro `name`. -/// See also `is_direct_expn_of`. +/// See also [`is_direct_expn_of`]. #[must_use] pub fn is_expn_of(mut span: Span, name: &str) -> Option { loop { @@ -1339,13 +1334,13 @@ pub fn is_expn_of(mut span: Span, name: &str) -> Option { /// Returns the pre-expansion span if the span directly comes from an expansion /// of the macro `name`. -/// The difference with `is_expn_of` is that in -/// ```rust,ignore +/// The difference with [`is_expn_of`] is that in +/// ```rust +/// # macro_rules! foo { ($e:tt) => { $e } }; macro_rules! bar { ($e:expr) => { $e } } /// foo!(bar!(42)); /// ``` /// `42` is considered expanded from `foo!` and `bar!` by `is_expn_of` but only -/// `bar!` by -/// `is_direct_expn_of`. +/// from `bar!` by `is_direct_expn_of`. #[must_use] pub fn is_direct_expn_of(span: Span, name: &str) -> Option { if span.from_expansion() { @@ -1468,11 +1463,9 @@ pub fn is_self(slf: &Param<'_>) -> bool { } pub fn is_self_ty(slf: &hir::Ty<'_>) -> bool { - if_chain! { - if let TyKind::Path(QPath::Resolved(None, path)) = slf.kind; - if let Res::SelfTy(..) = path.res; - then { - return true + if let TyKind::Path(QPath::Resolved(None, path)) = slf.kind { + if let Res::SelfTy(..) = path.res { + return true; } } false @@ -2064,15 +2057,12 @@ macro_rules! unwrap_cargo_metadata { } pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool { - if_chain! { - if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind; - if let Res::Def(_, def_id) = path.res; - then { - cx.tcx.has_attr(def_id, sym::cfg) || cx.tcx.has_attr(def_id, sym::cfg_attr) - } else { - false + if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind { + if let Res::Def(_, def_id) = path.res { + return cx.tcx.has_attr(def_id, sym::cfg) || cx.tcx.has_attr(def_id, sym::cfg_attr); } } + false } /// Checks whether item either has `test` attribute applied, or @@ -2084,7 +2074,7 @@ pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool { } } - matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test") + matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests") } macro_rules! op_utils { diff --git a/tests/ui/wildcard_imports.fixed b/tests/ui/wildcard_imports.fixed index ee9c9045fff..98bc1e80731 100644 --- a/tests/ui/wildcard_imports.fixed +++ b/tests/ui/wildcard_imports.fixed @@ -230,4 +230,12 @@ mod super_imports { let _ = foofoo(); } } + + mod attestation_should_be_replaced { + use super::foofoo; + + fn with_explicit() { + let _ = foofoo(); + } + } } diff --git a/tests/ui/wildcard_imports.rs b/tests/ui/wildcard_imports.rs index efaa8f9ef66..4ef61f9245b 100644 --- a/tests/ui/wildcard_imports.rs +++ b/tests/ui/wildcard_imports.rs @@ -231,4 +231,12 @@ mod super_imports { let _ = foofoo(); } } + + mod attestation_should_be_replaced { + use super::*; + + fn with_explicit() { + let _ = foofoo(); + } + } } diff --git a/tests/ui/wildcard_imports.stderr b/tests/ui/wildcard_imports.stderr index 66267dd27b8..d7af0c046e8 100644 --- a/tests/ui/wildcard_imports.stderr +++ b/tests/ui/wildcard_imports.stderr @@ -122,5 +122,11 @@ error: usage of wildcard import LL | use super::super::super_imports::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo` -error: aborting due to 20 previous errors +error: usage of wildcard import + --> $DIR/wildcard_imports.rs:236:13 + | +LL | use super::*; + | ^^^^^^^^ help: try: `super::foofoo` + +error: aborting due to 21 previous errors