Skip to content

Commit

Permalink
Auto merge of rust-lang#7779 - rust-lang:test_module, r=flip1995
Browse files Browse the repository at this point in the history
make test module detection more strict

I started with some small improvements to clippy_utils/src/lib.rs, but then found that our "test" module detection would also catch words containing "test" like e.g. "attestation". So I made this a bit more strict (splitting by `'_'` and checking for `test` or `tests`), adding a test case as I went.

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: none
  • Loading branch information
bors committed Oct 7, 2021
2 parents b7f3f7f + 86811e9 commit a04a7cd
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 27 deletions.
42 changes: 16 additions & 26 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,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 {
Expand Down Expand Up @@ -1285,10 +1281,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.
Expand All @@ -1315,7 +1310,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<Span> {
loop {
Expand All @@ -1338,13 +1333,13 @@ pub fn is_expn_of(mut span: Span, name: &str) -> Option<Span> {

/// 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<Span> {
if span.from_expansion() {
Expand Down Expand Up @@ -1467,11 +1462,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
Expand Down Expand Up @@ -2063,15 +2056,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
Expand All @@ -2083,7 +2073,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 {
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/wildcard_imports.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -230,4 +230,12 @@ mod super_imports {
let _ = foofoo();
}
}

mod attestation_should_be_replaced {
use super::foofoo;

fn with_explicit() {
let _ = foofoo();
}
}
}
8 changes: 8 additions & 0 deletions tests/ui/wildcard_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,4 +231,12 @@ mod super_imports {
let _ = foofoo();
}
}

mod attestation_should_be_replaced {
use super::*;

fn with_explicit() {
let _ = foofoo();
}
}
}
8 changes: 7 additions & 1 deletion tests/ui/wildcard_imports.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit a04a7cd

Please sign in to comment.