Skip to content

Commit

Permalink
Auto merge of #7379 - popzxc:issue-7305, r=flip1995
Browse files Browse the repository at this point in the history
Do not spawn blacklisted_name lint in test context

---

fixed #7305

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

changelog: `blacklisted_name` lint is not spawned in the test context anymore.
  • Loading branch information
bors committed Jun 25, 2021
2 parents a03dfb9 + 28d3873 commit 8d427b6
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 11 deletions.
31 changes: 28 additions & 3 deletions clippy_lints/src/blacklisted_name.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::{diagnostics::span_lint, is_test_module_or_function};
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::{Pat, PatKind};
use rustc_hir::{Item, Pat, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};

Expand All @@ -25,18 +25,37 @@ declare_clippy_lint! {
#[derive(Clone, Debug)]
pub struct BlacklistedName {
blacklist: FxHashSet<String>,
test_modules_deep: u32,
}

impl BlacklistedName {
pub fn new(blacklist: FxHashSet<String>) -> Self {
Self { blacklist }
Self {
blacklist,
test_modules_deep: 0,
}
}

fn in_test_module(&self) -> bool {
self.test_modules_deep != 0
}
}

impl_lint_pass!(BlacklistedName => [BLACKLISTED_NAME]);

impl<'tcx> LateLintPass<'tcx> for BlacklistedName {
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.blacklist.contains(&ident.name.to_string()) {
span_lint(
Expand All @@ -48,4 +67,10 @@ impl<'tcx> LateLintPass<'tcx> for BlacklistedName {
}
}
}

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);
}
}
}
12 changes: 4 additions & 8 deletions clippy_lints/src/wildcard_imports.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::in_macro;
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::{in_macro, is_test_module_or_function};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{
Expand Down Expand Up @@ -106,7 +106,7 @@ impl_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS]);

impl LateLintPass<'_> for WildcardImports {
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if is_test_module_or_function(item) {
if is_test_module_or_function(cx.tcx, item) {
self.test_modules_deep = self.test_modules_deep.saturating_add(1);
}
if item.vis.node.is_pub() || item.vis.node.is_pub_restricted() {
Expand Down Expand Up @@ -183,8 +183,8 @@ impl LateLintPass<'_> for WildcardImports {
}
}

fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) {
if is_test_module_or_function(item) {
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);
}
}
Expand All @@ -208,7 +208,3 @@ fn is_prelude_import(segments: &[PathSegment<'_>]) -> bool {
fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool {
segments.len() == 1 && segments[0].ident.name == kw::Super
}

fn is_test_module_or_function(item: &Item<'_>) -> bool {
matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test")
}
12 changes: 12 additions & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1727,3 +1727,15 @@ pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
}
}
}

/// Checks whether item either has `test` attribute applied, or
/// is a module with `test` in its name.
pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
if let Some(def_id) = tcx.hir().opt_local_def_id(item.hir_id()) {
if tcx.has_attr(def_id.to_def_id(), sym::test) {
return true;
}
}

matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test")
}
12 changes: 12 additions & 0 deletions tests/ui/blacklisted_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,15 @@ fn issue_1647_ref_mut() {
let ref mut baz = 0;
if let Some(ref mut quux) = Some(42) {}
}

mod tests {
fn issue_7305() {
// `blackisted_name` lint should not be triggered inside of the test code.
let foo = 0;

// Check that even in nested functions warning is still not triggere.
fn nested() {
let foo = 0;
}
}
}

0 comments on commit 8d427b6

Please sign in to comment.