Skip to content

fix: Do not replace items annotated with builtin attrs with the attr input #9943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 20, 2021
Merged
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
4 changes: 4 additions & 0 deletions crates/hir/src/semantics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
self.imp.descend_node_at_offset(node, offset).find_map(N::cast)
}

pub fn hir_file_for(&self, syntax_node: &SyntaxNode) -> HirFileId {
self.imp.find_file(syntax_node.clone()).file_id
}

pub fn original_range(&self, node: &SyntaxNode) -> FileRange {
self.imp.original_range(node)
}
Expand Down
3 changes: 0 additions & 3 deletions crates/hir_def/src/builtin_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ macro_rules! rustc_attr {
};
}

/// Built-in macro-like attributes.
pub const EXTRA_ATTRIBUTES: &[BuiltinAttribute] = &["test", "bench"];

/// "Inert" built-in attributes that have a special meaning to rustc or rustdoc.
#[rustfmt::skip]
pub const INERT_ATTRIBUTES: &[BuiltinAttribute] = &[
Expand Down
19 changes: 8 additions & 11 deletions crates/hir_def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1712,27 +1712,24 @@ impl ModCollector<'_, '_> {
if path.kind == PathKind::Plain {
if let Some(tool_module) = path.segments().first() {
let tool_module = tool_module.to_string();
if builtin_attr::TOOL_MODULES
let is_tool = builtin_attr::TOOL_MODULES
.iter()
.copied()
.chain(self.def_collector.registered_tools.iter().map(|s| &**s))
.any(|m| tool_module == *m)
{
.chain(self.def_collector.registered_tools.iter().map(AsRef::as_ref))
.any(|m| tool_module == *m);
if is_tool {
return true;
}
}

if let Some(name) = path.as_ident() {
let name = name.to_string();
if builtin_attr::INERT_ATTRIBUTES
let is_inert = builtin_attr::INERT_ATTRIBUTES
.iter()
.chain(builtin_attr::EXTRA_ATTRIBUTES)
.copied()
.chain(self.def_collector.registered_attrs.iter().map(|s| &**s))
.any(|attr| name == *attr)
{
return true;
}
.chain(self.def_collector.registered_attrs.iter().map(AsRef::as_ref))
.any(|attr| name == *attr);
return is_inert;
}
}

Expand Down
8 changes: 5 additions & 3 deletions crates/hir_expand/src/builtin_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ macro_rules! register_builtin {
db: &dyn AstDatabase,
id: MacroCallId,
tt: &tt::Subtree,
item: &tt::Subtree,
) -> Result<tt::Subtree, mbe::ExpandError> {
let expander = match *self {
$( BuiltinAttrExpander::$variant => $expand, )*
};
expander(db, id, tt)
expander(db, id, tt, item)
}

fn find_by_name(name: &name::Name) -> Option<Self> {
Expand Down Expand Up @@ -61,7 +62,8 @@ pub fn find_builtin_attr(
fn dummy_attr_expand(
_db: &dyn AstDatabase,
_id: MacroCallId,
tt: &tt::Subtree,
_tt: &tt::Subtree,
item: &tt::Subtree,
) -> Result<tt::Subtree, mbe::ExpandError> {
Ok(tt.clone())
Ok(item.clone())
}
7 changes: 6 additions & 1 deletion crates/hir_expand/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ impl TokenExpander {
TokenExpander::MacroDef { mac, .. } => mac.expand(tt),
TokenExpander::Builtin(it) => it.expand(db, id, tt),
// FIXME switch these to ExpandResult as well
TokenExpander::BuiltinAttr(it) => it.expand(db, id, tt).into(),
TokenExpander::BuiltinAttr(it) => match db.macro_arg(id) {
Some(macro_arg) => it.expand(db, id, tt, &macro_arg.0).into(),
None => mbe::ExpandResult::only_err(
mbe::ExpandError::Other("No item argument for attribute".to_string()).into(),
),
},
TokenExpander::BuiltinDerive(it) => it.expand(db, id, tt).into(),
TokenExpander::ProcMacro(_) => {
// We store the result in salsa db to prevent non-deterministic behavior in
Expand Down
21 changes: 19 additions & 2 deletions crates/hir_expand/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use either::Either;
pub use mbe::{ExpandError, ExpandResult};
pub use parser::FragmentKind;

use std::{hash::Hash, sync::Arc};
use std::{hash::Hash, iter, sync::Arc};

use base_db::{impl_intern_key, salsa, CrateId, FileId, FileRange};
use syntax::{
Expand Down Expand Up @@ -454,7 +454,7 @@ impl InFile<SyntaxNode> {
self,
db: &dyn db::AstDatabase,
) -> impl Iterator<Item = InFile<SyntaxNode>> + '_ {
std::iter::successors(Some(self), move |node| match node.value.parent() {
iter::successors(Some(self), move |node| match node.value.parent() {
Some(parent) => Some(node.with_value(parent)),
None => {
let parent_node = node.file_id.call_node(db)?;
Expand Down Expand Up @@ -562,6 +562,23 @@ impl<N: AstNode> InFile<N> {
pub fn syntax(&self) -> InFile<&SyntaxNode> {
self.with_value(self.value.syntax())
}

pub fn nodes_with_attributes<'db>(
self,
db: &'db dyn db::AstDatabase,
) -> impl Iterator<Item = InFile<N>> + 'db
where
N: 'db,
{
iter::successors(Some(self), move |node| {
let InFile { file_id, value } = node.file_id.call_node(db)?;
N::cast(value).map(|n| InFile::new(file_id, n))
})
}

pub fn node_with_attributes(self, db: &dyn db::AstDatabase) -> InFile<N> {
self.nodes_with_attributes(db).last().unwrap()
}
}

/// Given a `MacroCallId`, return what `FragmentKind` it belongs to.
Expand Down
21 changes: 14 additions & 7 deletions crates/ide/src/runnables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::fmt;
use ast::NameOwner;
use cfg::CfgExpr;
use either::Either;
use hir::{AsAssocItem, HasAttrs, HasSource, HirDisplay, Semantics};
use hir::{AsAssocItem, HasAttrs, HasSource, HirDisplay, InFile, Semantics};
use ide_assists::utils::test_related_attribute;
use ide_db::{
base_db::{FilePosition, FileRange},
Expand Down Expand Up @@ -232,22 +232,26 @@ fn find_related_tests(
let functions = refs.iter().filter_map(|(range, _)| {
let token = file.token_at_offset(range.start()).next()?;
let token = sema.descend_into_macros(token);
token.ancestors().find_map(ast::Fn::cast)
token
.ancestors()
.find_map(ast::Fn::cast)
.map(|f| hir::InFile::new(sema.hir_file_for(f.syntax()), f))
});

for fn_def in functions {
if let Some(runnable) = as_test_runnable(sema, &fn_def) {
// #[test/bench] expands to just the item causing us to lose the attribute, so recover them by going out of the attribute
let InFile { value: fn_def, .. } = &fn_def.node_with_attributes(sema.db);
if let Some(runnable) = as_test_runnable(sema, fn_def) {
// direct test
tests.insert(runnable);
} else if let Some(module) = parent_test_module(sema, &fn_def) {
} else if let Some(module) = parent_test_module(sema, fn_def) {
// indirect test
find_related_tests_in_module(sema, &fn_def, &module, tests);
find_related_tests_in_module(sema, fn_def, &module, tests);
}
}
}
}
}

fn find_related_tests_in_module(
sema: &Semantics<RootDatabase>,
fn_def: &ast::Fn,
Expand Down Expand Up @@ -292,7 +296,8 @@ fn parent_test_module(sema: &Semantics<RootDatabase>, fn_def: &ast::Fn) -> Optio
}

pub(crate) fn runnable_fn(sema: &Semantics<RootDatabase>, def: hir::Function) -> Option<Runnable> {
let func = def.source(sema.db)?;
// #[test/bench] expands to just the item causing us to lose the attribute, so recover them by going out of the attribute
let func = def.source(sema.db)?.node_with_attributes(sema.db);
let name_string = def.name(sema.db).to_string();

let root = def.module(sema.db).krate().root_module(sema.db);
Expand Down Expand Up @@ -499,6 +504,8 @@ fn has_test_function_or_multiple_test_submodules(
match item {
hir::ModuleDef::Function(f) => {
if let Some(it) = f.source(sema.db) {
// #[test/bench] expands to just the item causing us to lose the attribute, so recover them by going out of the attribute
let it = it.node_with_attributes(sema.db);
if test_related_attribute(&it.value).is_some() {
return true;
}
Expand Down