Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/hir-def/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl Attrs {

#[inline]
pub fn cfg(&self) -> Option<CfgExpr> {
let mut cfgs = self.by_key(sym::cfg).tt_values().map(CfgExpr::parse);
let mut cfgs = self.cfgs();
let first = cfgs.next()?;
match cfgs.next() {
Some(second) => {
Expand Down
1 change: 1 addition & 0 deletions crates/ide-db/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ doctest = false

[dependencies]
cov-mark = "2.0.0"
cfg.workspace = true
crossbeam-channel.workspace = true
tracing.workspace = true
rayon.workspace = true
Expand Down
83 changes: 76 additions & 7 deletions crates/ide-db/src/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use std::{cell::LazyCell, cmp::Reverse};
use base_db::{RootQueryDb, SourceDatabase};
use either::Either;
use hir::{
Adt, AsAssocItem, DefWithBody, EditionedFileId, FileRange, FileRangeWrapper, HasAttrs,
HasContainer, HasSource, InFile, InFileWrapper, InRealFile, InlineAsmOperand, ItemContainer,
ModuleSource, PathResolution, Semantics, Visibility, sym,
Adt, AsAssocItem, AttrsWithOwner, CfgExpr, DefWithBody, EditionedFileId, FileRange,
FileRangeWrapper, HasAttrs, HasContainer, HasSource, InFile, InFileWrapper, InRealFile,
InlineAsmOperand, ItemContainer, ModuleSource, PathResolution, Semantics, Visibility, sym,
};
use memchr::memmem::Finder;
use parser::SyntaxKind;
Expand Down Expand Up @@ -1370,8 +1370,77 @@ fn is_name_ref_in_import(name_ref: &ast::NameRef) -> bool {
}

fn is_name_ref_in_test(sema: &Semantics<'_, RootDatabase>, name_ref: &ast::NameRef) -> bool {
name_ref.syntax().ancestors().any(|node| match ast::Fn::cast(node) {
Some(it) => sema.to_def(&it).is_some_and(|func| func.is_test(sema.db)),
None => false,
})
name_ref
.syntax()
.ancestors()
.any(|node| is_test_function(sema, &node) || node_has_cfg_test(sema, &node))
|| current_module_is_declared_in_cfg_test(sema, name_ref)
}

/// Returns true if the node is a function with the `#[test]` attribute.
fn is_test_function(sema: &Semantics<'_, RootDatabase>, node: &syntax::SyntaxNode) -> bool {
ast::Fn::cast(node.clone())
.is_some_and(|func| sema.to_def(&func).is_some_and(|func| func.is_test(sema.db)))
}

/// Returns true if the node is only enabled in test configurations.
fn node_has_cfg_test(sema: &Semantics<'_, RootDatabase>, node: &syntax::SyntaxNode) -> bool {
macro_rules! attrs {
($it:expr) => {
sema.to_def($it).map(|node| node.attrs(sema.db))
};
}
use ast::*;
let attrs = match_ast! {
match node {
Adt(it) => attrs!(&it),
Const(it) => attrs!(&it),
ConstParam(it) => attrs!(&it),
Enum(it) => attrs!(&it),
ExternCrate(it) => attrs!(&it),
Fn(it) => attrs!(&it),
GenericParam(it) => attrs!(&it),
Impl(it) => attrs!(&it),
LifetimeParam(it) => attrs!(&it),
Macro(it) => attrs!(&it),
Module(it) => attrs!(&it),
RecordField(it) => attrs!(&it),
SourceFile(it) => attrs!(&it),
Static(it) => attrs!(&it),
Struct(it) => attrs!(&it),
Trait(it) => attrs!(&it),
TypeAlias(it) => attrs!(&it),
TypeParam(it) => attrs!(&it),
Union(it) => attrs!(&it),
Variant(it) => attrs!(&it),
_ => None,
}
};
attrs.as_ref().is_some_and(has_cfg_test)
}

/// Returns true if the given attributes enable code only in test configurations.
pub fn has_cfg_test(attrs: &AttrsWithOwner) -> bool {
fn is_cfg_test(cfg_expr: &CfgExpr) -> bool {
use CfgExpr::*;
use cfg::CfgAtom;
match cfg_expr {
Atom(CfgAtom::Flag(flag)) => *flag == sym::test,
All(exprs) => exprs.iter().any(is_cfg_test),
// N.B. possible false negatives here.
Invalid | Atom(CfgAtom::KeyValue { .. }) | Any(_) | Not(_) => false,
Comment on lines +1430 to +1431

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something similar to CfgExpr::fold can be used for more complex cases, e.g.:

diff --git a/crates/ide-db/src/search.rs b/crates/ide-db/src/search.rs
index 41a2ea8fe0..ad3af99fb9 100644
--- a/crates/ide-db/src/search.rs
+++ b/crates/ide-db/src/search.rs
@@ -1421,17 +1421,26 @@
 
 /// Returns true if the given attributes enable code only in test configurations.
 pub fn has_cfg_test(attrs: &AttrsWithOwner) -> bool {
-    fn is_cfg_test(cfg_expr: &CfgExpr) -> bool {
+    fn is_cfg_test(cfg_expr: &CfgExpr) -> Option<bool> {
         use CfgExpr::*;
         use cfg::CfgAtom;
         match cfg_expr {
-            Atom(CfgAtom::Flag(flag)) => *flag == sym::test,
-            All(exprs) => exprs.iter().any(is_cfg_test),
-            // N.B. possible false negatives here.
-            Invalid | Atom(CfgAtom::KeyValue { .. }) | Any(_) | Not(_) => false,
+            Invalid => None,
+            Atom(CfgAtom::Flag(flag)) => Some(*flag == sym::test),
+            Atom(CfgAtom::KeyValue { .. }) => Some(false),
+            All(exprs) => {
+                exprs.iter().try_fold(false, |acc, expr| is_cfg_test(expr).map(|res| acc || res))
+            }
+            Any(exprs) => {
+                exprs.iter().try_fold(true, |acc, expr| is_cfg_test(expr).map(|res| acc && res))
+            }
+            Not(expr) => is_cfg_test(expr).map(|is_test| !is_test),
         }
     }
-    attrs.cfgs().any(|cfg_expr| is_cfg_test(&cfg_expr))
+    attrs
+        .cfgs()
+        .try_fold(false, |acc, cfg_expr| is_cfg_test(&cfg_expr).map(|is_test| acc || is_test))
+        .unwrap_or(false)
 }
 
 /// Returns true if this reference's enclosing module is declared conditional on `cfg(test)`.

I noticed that expressions annotated with #[cfg(test)] are not handled by this PR, and it seems that the approach used for other ast types does not work for ast::Expr. Does the ast::HasAttrs trait correspond to the types which can be annotated with #[cfg(test)]? If so, maybe the implementation can be based around that. It might be unnecessary to handle some of these types, since I don't know why anyone would put #[cfg(test)] on something like an expression.

}
}
attrs.cfgs().any(|cfg_expr| is_cfg_test(&cfg_expr))
}

/// Returns true if this reference's enclosing module is declared conditional on `cfg(test)`.
/// E.g. it's declared like `#[cfg(test)] mod tests;` in its parent module.
fn current_module_is_declared_in_cfg_test(
sema: &Semantics<'_, RootDatabase>,
name_ref: &ast::NameRef,
) -> bool {
let file_id = sema.original_range(name_ref.syntax()).file_id;
sema.file_to_module_def(file_id.file_id(sema.db))
.is_some_and(|mod_def| has_cfg_test(&mod_def.attrs(sema.db)))
}
22 changes: 18 additions & 4 deletions crates/ide/src/references.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ mod tests {
fn exclude_tests() {
check(
r#"
fn test_func() {}
pub fn test_func() {}

fn func() {
test_func$0();
Expand All @@ -477,12 +477,26 @@ fn func() {
fn test() {
test_func();
}
#[cfg(test)]
fn cfg_test_fn() {
test_func();
}
#[cfg(test)]
mod cfg_test_mod {
use super::test_func;
fn test() {
test_func();
}
}
"#,
// TODO The last two should be "import test" and "test".
expect![[r#"
test_func Function FileId(0) 0..17 3..12
test_func Function FileId(0) 0..21 7..16

FileId(0) 35..44
FileId(0) 75..84 test
FileId(0) 39..48
FileId(0) 79..88 test
FileId(0) 130..139
FileId(0) 227..236
"#]],
);

Expand Down
19 changes: 6 additions & 13 deletions crates/ide/src/runnables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@ use std::{fmt, sync::OnceLock};

use arrayvec::ArrayVec;
use ast::HasName;
use cfg::{CfgAtom, CfgExpr};
use hir::{
AsAssocItem, AttrsWithOwner, HasAttrs, HasCrate, HasSource, Semantics, Symbol, db::HirDatabase,
sym,
};
use cfg::CfgExpr;
use hir::{AsAssocItem, HasAttrs, HasCrate, HasSource, Semantics, Symbol, db::HirDatabase};
use ide_assists::utils::{has_test_related_attribute, test_related_attribute_syn};
use ide_db::impl_empty_upmap_from_ra_fixture;
use ide_db::{
Expand All @@ -15,7 +12,7 @@ use ide_db::{
defs::Definition,
documentation::docs_from_attrs,
helpers::visit_file_defs,
search::{FileReferenceNode, SearchScope},
search::{FileReferenceNode, SearchScope, has_cfg_test},
};
use itertools::Itertools;
use macros::UpmapFromRaFixture;
Expand Down Expand Up @@ -323,7 +320,7 @@ pub(crate) fn runnable_fn(
def: hir::Function,
) -> Option<Runnable> {
let edition = def.krate(sema.db).edition(sema.db);
let under_cfg_test = has_cfg_test(def.module(sema.db).attrs(sema.db));
let under_cfg_test = has_cfg_test(&def.module(sema.db).attrs(sema.db));
let kind = if !under_cfg_test && def.is_main(sema.db) {
RunnableKind::Bin
} else {
Expand Down Expand Up @@ -366,7 +363,7 @@ pub(crate) fn runnable_mod(
sema: &Semantics<'_, RootDatabase>,
def: hir::Module,
) -> Option<Runnable> {
if !has_test_function_or_multiple_test_submodules(sema, &def, has_cfg_test(def.attrs(sema.db)))
if !has_test_function_or_multiple_test_submodules(sema, &def, has_cfg_test(&def.attrs(sema.db)))
{
return None;
}
Expand Down Expand Up @@ -442,18 +439,14 @@ pub(crate) fn runnable_impl(
})
}

fn has_cfg_test(attrs: AttrsWithOwner) -> bool {
attrs.cfgs().any(|cfg| matches!(&cfg, CfgExpr::Atom(CfgAtom::Flag(s)) if *s == sym::test))
}

/// Creates a test mod runnable for outline modules at the top of their definition.
fn runnable_mod_outline_definition(
sema: &Semantics<'_, RootDatabase>,
def: hir::Module,
) -> Option<Runnable> {
def.as_source_file_id(sema.db)?;

if !has_test_function_or_multiple_test_submodules(sema, &def, has_cfg_test(def.attrs(sema.db)))
if !has_test_function_or_multiple_test_submodules(sema, &def, has_cfg_test(&def.attrs(sema.db)))
{
return None;
}
Expand Down