Skip to content

Commit

Permalink
Get rid of $crate in expansions shown to the user
Browse files Browse the repository at this point in the history
Be it "Expand Macro Recursively", "Inline macro" or few other things.

We replace it with the crate name, as should've always been.
  • Loading branch information
ChayimFriedman2 committed Sep 18, 2024
1 parent b9be0f2 commit d878b53
Show file tree
Hide file tree
Showing 15 changed files with 396 additions and 64 deletions.
7 changes: 5 additions & 2 deletions src/tools/rust-analyzer/crates/hir-expand/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub mod span_map;

mod cfg_process;
mod fixup;
mod prettify_macro_expansion_;

use attrs::collect_attrs;
use rustc_hash::FxHashMap;
Expand Down Expand Up @@ -51,11 +52,13 @@ use crate::{
span_map::{ExpansionSpanMap, SpanMap},
};

pub use crate::files::{AstId, ErasedAstId, FileRange, InFile, InMacroFile, InRealFile};
pub use crate::{
files::{AstId, ErasedAstId, FileRange, InFile, InMacroFile, InRealFile},
prettify_macro_expansion_::prettify_macro_expansion,
};

pub use mbe::{DeclarativeMacro, ValueResult};
pub use span::{HirFileId, MacroCallId, MacroFileId};
pub use syntax_bridge::insert_whitespace_into_node;

pub mod tt {
pub use span::Span;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
//! Pretty printing of macros output.
use base_db::CrateId;
use rustc_hash::FxHashMap;
use syntax::NodeOrToken;
use syntax::{ast::make, SyntaxNode};

use crate::{db::ExpandDatabase, span_map::ExpansionSpanMap};

/// Inserts whitespace and replaces `$crate` in macro expansions.
#[expect(deprecated)]
pub fn prettify_macro_expansion(
db: &dyn ExpandDatabase,
syn: SyntaxNode,
span_map: &ExpansionSpanMap,
target_crate_id: CrateId,
) -> SyntaxNode {
let crate_graph = db.crate_graph();
let target_crate = &crate_graph[target_crate_id];
let mut syntax_ctx_id_to_dollar_crate_replacement = FxHashMap::default();
syntax_bridge::prettify_macro_expansion::prettify_macro_expansion(syn, &mut |dollar_crate| {
let ctx = span_map.span_at(dollar_crate.text_range().start()).ctx;
let replacement =
syntax_ctx_id_to_dollar_crate_replacement.entry(ctx).or_insert_with(|| {
let ctx_data = db.lookup_intern_syntax_context(ctx);
let macro_call_id =
ctx_data.outer_expn.expect("`$crate` cannot come from `SyntaxContextId::ROOT`");
let macro_call = db.lookup_intern_macro_call(macro_call_id);
let macro_def_crate = macro_call.def.krate;
// First, if this is the same crate as the macro, nothing will work but `crate`.
// If not, if the target trait has the macro's crate as a dependency, using the dependency name
// will work in inserted code and match the user's expectation.
// If not, the crate's display name is what the dependency name is likely to be once such dependency
// is inserted, and also understandable to the user.
// Lastly, if nothing else found, resort to leaving `$crate`.
if target_crate_id == macro_def_crate {
make::tokens::crate_kw()
} else if let Some(dep) =
target_crate.dependencies.iter().find(|dep| dep.crate_id == macro_def_crate)
{
make::tokens::ident(&dep.name)
} else if let Some(crate_name) = &crate_graph[macro_def_crate].display_name {
make::tokens::ident(crate_name.crate_name())
} else {
return dollar_crate.clone();
}
});
if replacement.text() == "$crate" {
// The parent may have many children, and looking for the token may yield incorrect results.
return dollar_crate.clone();
}
// We need to `clone_subtree()` but rowan doesn't provide such operation for tokens.
let parent = replacement.parent().unwrap().clone_subtree().clone_for_update();
parent
.children_with_tokens()
.filter_map(NodeOrToken::into_token)
.find(|it| it.kind() == replacement.kind())
.unwrap()
})
}
2 changes: 1 addition & 1 deletion src/tools/rust-analyzer/crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ pub use {
},
hygiene::{marks_rev, SyntaxContextExt},
inert_attr_macro::AttributeTemplate,
insert_whitespace_into_node,
name::Name,
prettify_macro_expansion,
proc_macro::{ProcMacros, ProcMacrosBuilder},
tt, ExpandResult, HirFileId, HirFileIdExt, MacroFileId, MacroFileIdExt,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@ use std::collections::BTreeSet;

use ast::make;
use either::Either;
use hir::{db::HirDatabase, sym, FileRange, PathResolution, Semantics, TypeInfo};
use hir::{
db::{ExpandDatabase, HirDatabase},
sym, FileRange, PathResolution, Semantics, TypeInfo,
};
use ide_db::{
base_db::CrateId,
defs::Definition,
imports::insert_use::remove_path_if_in_use_stmt,
path_transform::PathTransform,
search::{FileReference, FileReferenceNode, SearchScope},
source_change::SourceChangeBuilder,
syntax_helpers::{insert_whitespace_into_node::insert_ws_into, node_ext::expr_as_name_ref},
syntax_helpers::{node_ext::expr_as_name_ref, prettify_macro_expansion},
EditionedFileId, RootDatabase,
};
use itertools::{izip, Itertools};
Expand Down Expand Up @@ -102,12 +106,13 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) ->
let mut remove_def = true;
let mut inline_refs_for_file = |file_id, refs: Vec<FileReference>| {
builder.edit_file(file_id);
let call_krate = ctx.sema.file_to_module_def(file_id).map(|it| it.krate());
let count = refs.len();
// The collects are required as we are otherwise iterating while mutating 🙅‍♀️🙅‍♂️
let (name_refs, name_refs_use) = split_refs_and_uses(builder, refs, Some);
let call_infos: Vec<_> = name_refs
.into_iter()
.filter_map(CallInfo::from_name_ref)
.filter_map(|it| CallInfo::from_name_ref(it, call_krate?.into()))
// FIXME: do not handle callsites in macros' parameters, because
// directly inlining into macros may cause errors.
.filter(|call_info| !ctx.sema.hir_file_for(call_info.node.syntax()).is_macro())
Expand Down Expand Up @@ -185,7 +190,10 @@ pub(super) fn split_refs_and_uses<T: ast::AstNode>(
// ```
pub(crate) fn inline_call(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let name_ref: ast::NameRef = ctx.find_node_at_offset()?;
let call_info = CallInfo::from_name_ref(name_ref.clone())?;
let call_info = CallInfo::from_name_ref(
name_ref.clone(),
ctx.sema.file_to_module_def(ctx.file_id())?.krate().into(),
)?;
let (function, label) = match &call_info.node {
ast::CallableExpr::Call(call) => {
let path = match call.expr()? {
Expand Down Expand Up @@ -243,10 +251,11 @@ struct CallInfo {
node: ast::CallableExpr,
arguments: Vec<ast::Expr>,
generic_arg_list: Option<ast::GenericArgList>,
krate: CrateId,
}

impl CallInfo {
fn from_name_ref(name_ref: ast::NameRef) -> Option<CallInfo> {
fn from_name_ref(name_ref: ast::NameRef, krate: CrateId) -> Option<CallInfo> {
let parent = name_ref.syntax().parent()?;
if let Some(call) = ast::MethodCallExpr::cast(parent.clone()) {
let receiver = call.receiver()?;
Expand All @@ -256,6 +265,7 @@ impl CallInfo {
generic_arg_list: call.generic_arg_list(),
node: ast::CallableExpr::MethodCall(call),
arguments,
krate,
})
} else if let Some(segment) = ast::PathSegment::cast(parent) {
let path = segment.syntax().parent().and_then(ast::Path::cast)?;
Expand All @@ -266,6 +276,7 @@ impl CallInfo {
arguments: call.arg_list()?.args().collect(),
node: ast::CallableExpr::Call(call),
generic_arg_list: segment.generic_arg_list(),
krate,
})
} else {
None
Expand Down Expand Up @@ -307,11 +318,15 @@ fn inline(
function: hir::Function,
fn_body: &ast::BlockExpr,
params: &[(ast::Pat, Option<ast::Type>, hir::Param)],
CallInfo { node, arguments, generic_arg_list }: &CallInfo,
CallInfo { node, arguments, generic_arg_list, krate }: &CallInfo,
) -> ast::Expr {
let mut body = if sema.hir_file_for(fn_body.syntax()).is_macro() {
let file_id = sema.hir_file_for(fn_body.syntax());
let mut body = if let Some(macro_file) = file_id.macro_file() {
cov_mark::hit!(inline_call_defined_in_macro);
if let Some(body) = ast::BlockExpr::cast(insert_ws_into(fn_body.syntax().clone())) {
let span_map = sema.db.expansion_span_map(macro_file);
let body_prettified =
prettify_macro_expansion(sema.db, fn_body.syntax().clone(), &span_map, *krate);
if let Some(body) = ast::BlockExpr::cast(body_prettified) {
body
} else {
fn_body.clone_for_update()
Expand Down Expand Up @@ -420,8 +435,16 @@ fn inline(

let mut insert_let_stmt = || {
let param_ty = param_ty.clone().map(|param_ty| {
if sema.hir_file_for(param_ty.syntax()).is_macro() {
ast::Type::cast(insert_ws_into(param_ty.syntax().clone())).unwrap_or(param_ty)
let file_id = sema.hir_file_for(param_ty.syntax());
if let Some(macro_file) = file_id.macro_file() {
let span_map = sema.db.expansion_span_map(macro_file);
let param_ty_prettified = prettify_macro_expansion(
sema.db,
param_ty.syntax().clone(),
&span_map,
*krate,
);
ast::Type::cast(param_ty_prettified).unwrap_or(param_ty)
} else {
param_ty
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use ide_db::syntax_helpers::insert_whitespace_into_node::insert_ws_into;
use hir::db::ExpandDatabase;
use ide_db::syntax_helpers::prettify_macro_expansion;
use syntax::ast::{self, AstNode};

use crate::{AssistContext, AssistId, AssistKind, Assists};
Expand Down Expand Up @@ -36,7 +37,15 @@ use crate::{AssistContext, AssistId, AssistKind, Assists};
// ```
pub(crate) fn inline_macro(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let unexpanded = ctx.find_node_at_offset::<ast::MacroCall>()?;
let expanded = insert_ws_into(ctx.sema.expand(&unexpanded)?.clone_for_update());
let macro_call = ctx.sema.to_def(&unexpanded)?;
let expanded = ctx.sema.parse_or_expand(macro_call.as_file());
let span_map = ctx.sema.db.expansion_span_map(macro_call.as_macro_file());
let expanded = prettify_macro_expansion(
ctx.db(),
expanded,
&span_map,
ctx.sema.file_to_module_def(ctx.file_id())?.krate().into(),
);
let text_range = unexpanded.syntax().text_range();

acc.add(
Expand Down Expand Up @@ -295,6 +304,75 @@ fn main() {
}
};
}
"#,
);
}

#[test]
fn dollar_crate() {
check_assist(
inline_macro,
r#"
pub struct Foo;
#[macro_export]
macro_rules! m {
() => { $crate::Foo };
}
fn bar() {
m$0!();
}
"#,
r#"
pub struct Foo;
#[macro_export]
macro_rules! m {
() => { $crate::Foo };
}
fn bar() {
crate::Foo;
}
"#,
);
check_assist(
inline_macro,
r#"
//- /a.rs crate:a
pub struct Foo;
#[macro_export]
macro_rules! m {
() => { $crate::Foo };
}
//- /b.rs crate:b deps:a
fn bar() {
a::m$0!();
}
"#,
r#"
fn bar() {
a::Foo;
}
"#,
);
check_assist(
inline_macro,
r#"
//- /a.rs crate:a
pub struct Foo;
#[macro_export]
macro_rules! m {
() => { $crate::Foo };
}
//- /b.rs crate:b deps:a
pub use a::m;
//- /c.rs crate:c deps:b
fn bar() {
b::m$0!();
}
"#,
r#"
fn bar() {
a::Foo;
}
"#,
);
}
Expand Down
20 changes: 14 additions & 6 deletions src/tools/rust-analyzer/crates/ide-assists/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
//! Assorted functions shared by several assists.
pub(crate) use gen_trait_fn_body::gen_trait_fn_body;
use hir::{db::HirDatabase, HasAttrs as HirHasAttrs, HirDisplay, InFile, Semantics};
use hir::{
db::{ExpandDatabase, HirDatabase},
HasAttrs as HirHasAttrs, HirDisplay, InFile, Semantics,
};
use ide_db::{
famous_defs::FamousDefs, path_transform::PathTransform,
syntax_helpers::insert_whitespace_into_node::insert_ws_into, RootDatabase,
syntax_helpers::prettify_macro_expansion, RootDatabase,
};
use stdx::format_to;
use syntax::{
Expand Down Expand Up @@ -178,10 +181,15 @@ pub fn add_trait_assoc_items_to_impl(
let new_indent_level = IndentLevel::from_node(impl_.syntax()) + 1;
let items = original_items.iter().map(|InFile { file_id, value: original_item }| {
let cloned_item = {
if file_id.is_macro() {
if let Some(formatted) =
ast::AssocItem::cast(insert_ws_into(original_item.syntax().clone()))
{
if let Some(macro_file) = file_id.macro_file() {
let span_map = sema.db.expansion_span_map(macro_file);
let item_prettified = prettify_macro_expansion(
sema.db,
original_item.syntax().clone(),
&span_map,
target_scope.krate().into(),
);
if let Some(formatted) = ast::AssocItem::cast(item_prettified) {
return formatted;
} else {
stdx::never!("formatted `AssocItem` could not be cast back to `AssocItem`");
Expand Down
Loading

0 comments on commit d878b53

Please sign in to comment.