mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #13049 - Jarcho:is_in_test, r=Alexendoo

Browse Source 
Remove `is_in_test_module_or_function`

Uses are replaced with `is_in_test` for consistency with other lints and to simplify the implementation of the lints. This means the module name is no longer checked, but that was a horrible hack from a time when late passes couldn't see `#[cfg(..)]` attributes.

changelog: none
This commit is contained in:
bors 2024-07-05 11:33:46 +00:00
commit 3ef366705b
17 changed files with 59 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
clippy_lints/src/disallowed_names.rs
View File

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::is_test_module_or_function;
use clippy_utils::is_in_test;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::{Item, Pat, PatKind};
use rustc_hir::{Pat, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
@ -27,52 +27,30 @@
#[derive(Clone, Debug)]
pub struct DisallowedNames {
disallow: FxHashSet<String>,
test_modules_deep: u32,
}
impl DisallowedNames {
pub fn new(disallowed_names: &[String]) -> Self {
Self {
disallow: disallowed_names.iter().cloned().collect(),
test_modules_deep: 0,
}
}
fn in_test_module(&self) -> bool {
self.test_modules_deep != 0
}
}
impl_lint_pass!(DisallowedNames => [DISALLOWED_NAMES]);
impl<'tcx> LateLintPass<'tcx> for DisallowedNames {
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if is_test_module_or_function(cx.tcx, item) {
self.test_modules_deep = self.test_modules_deep.saturating_add(1);
}
}
fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
// Check whether we are under the `test` attribute.
if self.in_test_module() {
return;
}
if let PatKind::Binding(.., ident, _) = pat.kind {
if self.disallow.contains(&ident.name.to_string()) {
span_lint(
cx,
DISALLOWED_NAMES,
ident.span,
format!("use of a disallowed/placeholder name `{}`", ident.name),
);
}
}
}
fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if is_test_module_or_function(cx.tcx, item) {
self.test_modules_deep = self.test_modules_deep.saturating_sub(1);
if let PatKind::Binding(.., ident, _) = pat.kind
&& self.disallow.contains(&ident.name.to_string())
&& !is_in_test(cx.tcx, pat.hir_id)
{
span_lint(
cx,
DISALLOWED_NAMES,
ident.span,
format!("use of a disallowed/placeholder name `{}`", ident.name),
);
}
}
}

8
clippy_lints/src/functions/impl_trait_in_params.rs
View File

@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_in_test_function;
use clippy_utils::is_in_test;
use rustc_hir as hir;
use rustc_hir::intravisit::FnKind;
@ -41,7 +41,7 @@ fn report(cx: &LateContext<'_>, param: &GenericParam<'_>, generics: &Generics<'_
pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: &'tcx Body<'_>, hir_id: HirId) {
if let FnKind::ItemFn(_, generics, _) = kind
&& cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public()
&& !is_in_test_function(cx.tcx, hir_id)
&& !is_in_test(cx.tcx, hir_id)
{
for param in generics.params {
if param.is_impl_trait() {
@ -59,7 +59,7 @@ pub(super) fn check_impl_item(cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
&& of_trait.is_none()
&& let body = cx.tcx.hir().body(body_id)
&& cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public()
&& !is_in_test_function(cx.tcx, impl_item.hir_id())
&& !is_in_test(cx.tcx, impl_item.hir_id())
{
for param in impl_item.generics.params {
if param.is_impl_trait() {
@ -75,7 +75,7 @@ pub(super) fn check_trait_item(cx: &LateContext<'_>, trait_item: &TraitItem<'_>,
&& let hir::Node::Item(item) = cx.tcx.parent_hir_node(trait_item.hir_id())
// ^^ (Will always be a trait)
&& !item.vis_span.is_empty() // Is public
&& !is_in_test_function(cx.tcx, trait_item.hir_id())
&& !is_in_test(cx.tcx, trait_item.hir_id())
{
for param in trait_item.generics.params {
if param.is_impl_trait() {

4
clippy_lints/src/incompatible_msrv.rs
View File

@ -1,6 +1,6 @@
use clippy_config::msrvs::Msrv;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::is_in_test_function;
use clippy_utils::is_in_test;
use rustc_attr::{StabilityLevel, StableSince};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{Expr, ExprKind, HirId};
@ -88,7 +88,7 @@ fn emit_lint_if_under_msrv(&mut self, cx: &LateContext<'_>, def_id: DefId, node:
return;
}
let version = self.get_def_id_version(cx.tcx, def_id);
if self.msrv.meets(version) || is_in_test_function(cx.tcx, node) {
if self.msrv.meets(version) || is_in_test(cx.tcx, node) {
return;
}
if let ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) = span.ctxt().outer_expn_data().kind {

4
clippy_lints/src/methods/unwrap_expect_used.rs
View File

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::ty::{is_never_like, is_type_diagnostic_item};
use clippy_utils::{is_in_cfg_test, is_in_test_function, is_lint_allowed};
use clippy_utils::{is_in_test, is_lint_allowed};
use rustc_hir::Expr;
use rustc_lint::{LateContext, Lint};
use rustc_middle::ty;
@ -61,7 +61,7 @@ pub(super) fn check(
let method_suffix = if is_err { "_err" } else { "" };
if allow_unwrap_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id)) {
if allow_unwrap_in_tests && is_in_test(cx.tcx, expr.hir_id) {
return;
}

4
clippy_lints/src/missing_assert_message.rs
View File

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_in_test;
use clippy_utils::macros::{find_assert_args, find_assert_eq_args, root_macro_call_first_node, PanicExpn};
use clippy_utils::{is_in_cfg_test, is_in_test_function};
use rustc_hir::Expr;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
@ -62,7 +62,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
};
// This lint would be very noisy in tests, so just ignore if we're in test context
if is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id) {
if is_in_test(cx.tcx, expr.hir_id) {
return;
}

19
clippy_lints/src/wildcard_imports.rs
View File

@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_test_module_or_function;
use clippy_utils::is_in_test;
use clippy_utils::source::{snippet, snippet_with_applicability};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
@ -100,7 +100,6 @@
#[derive(Default)]
pub struct WildcardImports {
warn_on_all: bool,
test_modules_deep: u32,
allowed_segments: FxHashSet<String>,
}
@ -108,7 +107,6 @@ impl WildcardImports {
pub fn new(warn_on_all: bool, allowed_wildcard_imports: FxHashSet<String>) -> Self {
Self {
warn_on_all,
test_modules_deep: 0,
allowed_segments: allowed_wildcard_imports,
}
}
@ -122,15 +120,12 @@ fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
return;
}
if is_test_module_or_function(cx.tcx, item) {
self.test_modules_deep = self.test_modules_deep.saturating_add(1);
}
let module = cx.tcx.parent_module_from_def_id(item.owner_id.def_id);
if cx.tcx.visibility(item.owner_id.def_id) != ty::Visibility::Restricted(module.to_def_id()) {
return;
}
if let ItemKind::Use(use_path, UseKind::Glob) = &item.kind
&& (self.warn_on_all || !self.check_exceptions(item, use_path.segments))
&& (self.warn_on_all || !self.check_exceptions(cx, item, use_path.segments))
&& let used_imports = cx.tcx.names_imported_by_glob_use(item.owner_id.def_id)
&& !used_imports.is_empty() // Already handled by `unused_imports`
&& !used_imports.contains(&kw::Underscore)
@ -180,20 +175,14 @@ fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
span_lint_and_sugg(cx, lint, span, message, "try", sugg, applicability);
}
}
fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if is_test_module_or_function(cx.tcx, item) {
self.test_modules_deep = self.test_modules_deep.saturating_sub(1);
}
}
}
impl WildcardImports {
fn check_exceptions(&self, item: &Item<'_>, segments: &[PathSegment<'_>]) -> bool {
fn check_exceptions(&self, cx: &LateContext<'_>, item: &Item<'_>, segments: &[PathSegment<'_>]) -> bool {
item.span.from_expansion()
|| is_prelude_import(segments)
|| (is_super_only_import(segments) && self.test_modules_deep > 0)
|| is_allowed_via_config(segments, &self.allowed_segments)
|| (is_super_only_import(segments) && is_in_test(cx.tcx, item.hir_id()))
}
}

5
clippy_lints/src/write.rs
View File

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
use clippy_utils::is_in_test;
use clippy_utils::macros::{format_arg_removal_span, root_macro_call_first_node, FormatArgsStorage, MacroCall};
use clippy_utils::source::{expand_past_previous_comma, snippet_opt};
use clippy_utils::{is_in_cfg_test, is_in_test_function};
use rustc_ast::token::LitKind;
use rustc_ast::{
FormatArgPosition, FormatArgPositionKind, FormatArgs, FormatArgsPiece, FormatOptions, FormatPlaceholder,
@ -297,8 +297,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
.as_ref()
.map_or(false, |crate_name| crate_name == "build_script_build");
let allowed_in_tests = self.allow_print_in_tests
&& (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id));
let allowed_in_tests = self.allow_print_in_tests && is_in_test(cx.tcx, expr.hir_id);
match diag_name {
sym::print_macro | sym::println_macro if !allowed_in_tests => {
if !is_build_script {

10
clippy_utils/src/lib.rs
View File

@ -2625,16 +2625,6 @@ pub fn inherits_cfg(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
.any(|attr| attr.has_name(sym::cfg))
}
/// Checks whether item either has `test` attribute applied, or
/// is a module with `test` in its name.
///
/// Note: Add `//@compile-flags: --test` to UI tests with a `#[test]` function
pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
is_in_test_function(tcx, item.hir_id())
|| matches!(item.kind, ItemKind::Mod(..))
&& item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
}
/// Walks up the HIR tree from the given expression in an attempt to find where the value is
/// consumed.
///

1
tests/ui/disallowed_names.rs
View File

@ -60,6 +60,7 @@ fn issue_1647_ref_mut() {
//~^ ERROR: use of a disallowed/placeholder name `quux`
}
#[cfg(test)]
mod tests {
fn issue_7305() {
// `disallowed_names` lint should not be triggered inside of the test code.

3
tests/ui/wildcard_imports.fixed
View File

@ -204,6 +204,7 @@ mod super_imports {
}
}
#[cfg(test)]
mod test_should_pass {
use super::*;
@ -212,6 +213,7 @@ mod super_imports {
}
}
#[cfg(test)]
mod test_should_pass_inside_function {
fn with_super_inside_function() {
use super::*;
@ -219,6 +221,7 @@ mod super_imports {
}
}
#[cfg(test)]
mod test_should_pass_further_inside {
fn insidefoo() {}
mod inner {

3
tests/ui/wildcard_imports.rs
View File

@ -205,6 +205,7 @@ fn with_super() {
}
}
#[cfg(test)]
mod test_should_pass {
use super::*;
@ -213,6 +214,7 @@ fn with_super() {
}
}
#[cfg(test)]
mod test_should_pass_inside_function {
fn with_super_inside_function() {
use super::*;
@ -220,6 +222,7 @@ fn with_super_inside_function() {
}
}
#[cfg(test)]
mod test_should_pass_further_inside {
fn insidefoo() {}
mod inner {

10
tests/ui/wildcard_imports.stderr
View File

@ -106,31 +106,31 @@ LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:236:17
--> tests/ui/wildcard_imports.rs:239:17
|
LL | use super::*;
| ^^^^^^^^ help: try: `super::insidefoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:244:13
--> tests/ui/wildcard_imports.rs:247:13
|
LL | use crate::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::super_imports::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:253:17
--> tests/ui/wildcard_imports.rs:256:17
|
LL | use super::super::*;
| ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:262:13
--> tests/ui/wildcard_imports.rs:265:13
|
LL | use super::super::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:270:13
--> tests/ui/wildcard_imports.rs:273:13
|
LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo`

3
tests/ui/wildcard_imports_2021.edition2018.fixed
View File

@ -198,6 +198,7 @@ mod super_imports {
}
}
#[cfg(test)]
mod test_should_pass {
use super::*;
@ -206,6 +207,7 @@ mod super_imports {
}
}
#[cfg(test)]
mod test_should_pass_inside_function {
fn with_super_inside_function() {
use super::*;
@ -213,6 +215,7 @@ mod super_imports {
}
}
#[cfg(test)]
mod test_should_pass_further_inside {
fn insidefoo() {}
mod inner {

10
tests/ui/wildcard_imports_2021.edition2018.stderr
View File

@ -106,31 +106,31 @@ LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:230:17
--> tests/ui/wildcard_imports_2021.rs:233:17
|
LL | use super::*;
| ^^^^^^^^ help: try: `super::insidefoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:238:13
--> tests/ui/wildcard_imports_2021.rs:241:13
|
LL | use crate::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::super_imports::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:247:17
--> tests/ui/wildcard_imports_2021.rs:250:17
|
LL | use super::super::*;
| ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:256:13
--> tests/ui/wildcard_imports_2021.rs:259:13
|
LL | use super::super::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:264:13
--> tests/ui/wildcard_imports_2021.rs:267:13
|
LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo`

3
tests/ui/wildcard_imports_2021.edition2021.fixed
View File

@ -198,6 +198,7 @@ mod super_imports {
}
}
#[cfg(test)]
mod test_should_pass {
use super::*;
@ -206,6 +207,7 @@ mod super_imports {
}
}
#[cfg(test)]
mod test_should_pass_inside_function {
fn with_super_inside_function() {
use super::*;
@ -213,6 +215,7 @@ mod super_imports {
}
}
#[cfg(test)]
mod test_should_pass_further_inside {
fn insidefoo() {}
mod inner {

10
tests/ui/wildcard_imports_2021.edition2021.stderr
View File

@ -106,31 +106,31 @@ LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:230:17
--> tests/ui/wildcard_imports_2021.rs:233:17
|
LL | use super::*;
| ^^^^^^^^ help: try: `super::insidefoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:238:13
--> tests/ui/wildcard_imports_2021.rs:241:13
|
LL | use crate::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::super_imports::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:247:17
--> tests/ui/wildcard_imports_2021.rs:250:17
|
LL | use super::super::*;
| ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:256:13
--> tests/ui/wildcard_imports_2021.rs:259:13
|
LL | use super::super::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo`
error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:264:13
--> tests/ui/wildcard_imports_2021.rs:267:13
|
LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo`

3
tests/ui/wildcard_imports_2021.rs
View File

@ -199,6 +199,7 @@ fn with_super() {
}
}
#[cfg(test)]
mod test_should_pass {
use super::*;
@ -207,6 +208,7 @@ fn with_super() {
}
}
#[cfg(test)]
mod test_should_pass_inside_function {
fn with_super_inside_function() {
use super::*;
@ -214,6 +216,7 @@ fn with_super_inside_function() {
}
}
#[cfg(test)]
mod test_should_pass_further_inside {
fn insidefoo() {}
mod inner {