Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: derive source scope from syntax node to be transformed #14989

Merged
merged 2 commits into from
Jun 11, 2023
Merged

fix: derive source scope from syntax node to be transformed #14989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
008f506
fix(assist): derive source scope from syntax node to be transformed
lowr Jun 5, 2023
d091991
fix(completion): derive source scope from syntax node to be transformed
lowr Jun 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 0 additions & 10 deletions crates/hir/src/semantics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,10 +483,6 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
self.imp.scope_at_offset(node, offset)
}

pub fn scope_for_def(&self, def: Trait) -> SemanticsScope<'db> {
self.imp.scope_for_def(def)
}

pub fn assert_contains_node(&self, node: &SyntaxNode) {
self.imp.assert_contains_node(node)
}
Expand Down Expand Up @@ -1307,12 +1303,6 @@ impl<'db> SemanticsImpl<'db> {
)
}

fn scope_for_def(&self, def: Trait) -> SemanticsScope<'db> {
let file_id = self.db.lookup_intern_trait(def.id).id.file_id();
let resolver = def.id.resolver(self.db.upcast());
SemanticsScope { db: self.db, file_id, resolver }
}

fn source<Def: HasSource>(&self, def: Def) -> Option<InFile<Def::Ast>>
where
Def::Ast: AstNode,
Expand Down
87 changes: 74 additions & 13 deletions crates/ide-assists/src/handlers/add_missing_impl_members.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use hir::HasSource;
use ide_db::syntax_helpers::insert_whitespace_into_node::insert_ws_into;
use syntax::ast::{self, make, AstNode};

use crate::{
Expand Down Expand Up @@ -129,20 +128,9 @@ fn add_missing_impl_members_inner(
let target = impl_def.syntax().text_range();
acc.add(AssistId(assist_id, AssistKind::QuickFix), label, target, |edit| {
let new_impl_def = edit.make_mut(impl_def.clone());
let missing_items = missing_items
.into_iter()
.map(|it| {
if ctx.sema.hir_file_for(it.syntax()).is_macro() {
if let Some(it) = ast::AssocItem::cast(insert_ws_into(it.syntax().clone())) {
return it;
}
}
it.clone_for_update()
})
.collect();
let first_new_item = add_trait_assoc_items_to_impl(
&ctx.sema,
missing_items,
&missing_items,
trait_,
&new_impl_def,
target_scope,
Expand Down Expand Up @@ -1730,4 +1718,77 @@ impl m::Foo for S {
}"#,
)
}

#[test]
fn nested_macro_should_not_cause_crash() {
check_assist(
add_missing_impl_members,
r#"
macro_rules! ty { () => { i32 } }
trait SomeTrait { type Output; }
impl SomeTrait for i32 { type Output = i64; }
macro_rules! define_method {
() => {
fn method(&mut self, params: <ty!() as SomeTrait>::Output);
};
}
trait AnotherTrait { define_method!(); }
impl $0AnotherTrait for () {
}
"#,
r#"
macro_rules! ty { () => { i32 } }
trait SomeTrait { type Output; }
impl SomeTrait for i32 { type Output = i64; }
macro_rules! define_method {
() => {
fn method(&mut self, params: <ty!() as SomeTrait>::Output);
};
}
trait AnotherTrait { define_method!(); }
impl AnotherTrait for () {
$0fn method(&mut self,params: <ty!()as SomeTrait>::Output) {
todo!()
}
}
"#,
);
}

// FIXME: `T` in `ty!(T)` should be replaced by `PathTransform`.
#[test]
fn paths_in_nested_macro_should_get_transformed() {
check_assist(
add_missing_impl_members,
r#"
macro_rules! ty { ($me:ty) => { $me } }
trait SomeTrait { type Output; }
impl SomeTrait for i32 { type Output = i64; }
macro_rules! define_method {
($t:ty) => {
fn method(&mut self, params: <ty!($t) as SomeTrait>::Output);
};
}
trait AnotherTrait<T: SomeTrait> { define_method!(T); }
impl $0AnotherTrait<i32> for () {
}
"#,
r#"
macro_rules! ty { ($me:ty) => { $me } }
trait SomeTrait { type Output; }
impl SomeTrait for i32 { type Output = i64; }
macro_rules! define_method {
($t:ty) => {
fn method(&mut self, params: <ty!($t) as SomeTrait>::Output);
};
}
trait AnotherTrait<T: SomeTrait> { define_method!(T); }
impl AnotherTrait<i32> for () {
$0fn method(&mut self,params: <ty!(T)as SomeTrait>::Output) {
todo!()
}
}
"#,
);
}
}
34 changes: 18 additions & 16 deletions crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use hir::{InFile, ModuleDef};
use ide_db::{
helpers::mod_path_to_ast, imports::import_assets::NameToImport, items_locator,
syntax_helpers::insert_whitespace_into_node::insert_ws_into,
};
use ide_db::{helpers::mod_path_to_ast, imports::import_assets::NameToImport, items_locator};
use itertools::Itertools;
use syntax::{
ast::{self, AstNode, HasName},
Expand Down Expand Up @@ -202,19 +199,24 @@ fn impl_def_from_trait(
node
};

let trait_items = trait_items
.into_iter()
.map(|it| {
if sema.hir_file_for(it.syntax()).is_macro() {
if let Some(it) = ast::AssocItem::cast(insert_ws_into(it.syntax().clone())) {
return it;
}
}
it.clone_for_update()
})
.collect();
// <<<<<<< HEAD
// let trait_items = trait_items
// .into_iter()
// .map(|it| {
// if sema.hir_file_for(it.syntax()).is_macro() {
// if let Some(it) = ast::AssocItem::cast(insert_ws_into(it.syntax().clone())) {
// return it;
// }
// }
// it.clone_for_update()
// })
// .collect();
// let first_assoc_item =
// add_trait_assoc_items_to_impl(sema, trait_items, trait_, &impl_def, target_scope);
// =======
let first_assoc_item =
add_trait_assoc_items_to_impl(sema, trait_items, trait_, &impl_def, target_scope);
add_trait_assoc_items_to_impl(sema, &trait_items, trait_, &impl_def, target_scope);
// >>>>>>> fix(assist): derive source scope from syntax node to be transformed

// Generate a default `impl` function body for the derived trait.
if let ast::AssocItem::Fn(ref func) = first_assoc_item {
Expand Down
85 changes: 54 additions & 31 deletions crates/ide-assists/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
use std::ops;

pub(crate) use gen_trait_fn_body::gen_trait_fn_body;
use hir::{db::HirDatabase, HirDisplay, Semantics};
use ide_db::{famous_defs::FamousDefs, path_transform::PathTransform, RootDatabase, SnippetCap};
use hir::{db::HirDatabase, HirDisplay, InFile, Semantics};
use ide_db::{
famous_defs::FamousDefs, path_transform::PathTransform,
syntax_helpers::insert_whitespace_into_node::insert_ws_into, RootDatabase, SnippetCap,
};
use stdx::format_to;
use syntax::{
ast::{
Expand Down Expand Up @@ -91,30 +94,21 @@ pub fn filter_assoc_items(
sema: &Semantics<'_, RootDatabase>,
items: &[hir::AssocItem],
default_methods: DefaultMethods,
) -> Vec<ast::AssocItem> {
fn has_def_name(item: &ast::AssocItem) -> bool {
match item {
ast::AssocItem::Fn(def) => def.name(),
ast::AssocItem::TypeAlias(def) => def.name(),
ast::AssocItem::Const(def) => def.name(),
ast::AssocItem::MacroCall(_) => None,
}
.is_some()
}

items
) -> Vec<InFile<ast::AssocItem>> {
return items
.iter()
// Note: This throws away items with no source.
.filter_map(|&i| {
let item = match i {
hir::AssocItem::Function(i) => ast::AssocItem::Fn(sema.source(i)?.value),
hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(sema.source(i)?.value),
hir::AssocItem::Const(i) => ast::AssocItem::Const(sema.source(i)?.value),
.copied()
.filter_map(|assoc_item| {
let item = match assoc_item {
hir::AssocItem::Function(it) => sema.source(it)?.map(ast::AssocItem::Fn),
hir::AssocItem::TypeAlias(it) => sema.source(it)?.map(ast::AssocItem::TypeAlias),
hir::AssocItem::Const(it) => sema.source(it)?.map(ast::AssocItem::Const),
};
Some(item)
})
.filter(has_def_name)
.filter(|it| match it {
.filter(|it| match &it.value {
ast::AssocItem::Fn(def) => matches!(
(default_methods, def.body()),
(DefaultMethods::Only, Some(_)) | (DefaultMethods::No, None)
Expand All @@ -125,26 +119,55 @@ pub fn filter_assoc_items(
),
_ => default_methods == DefaultMethods::No,
})
.collect::<Vec<_>>()
.collect();

fn has_def_name(item: &InFile<ast::AssocItem>) -> bool {
match &item.value {
ast::AssocItem::Fn(def) => def.name(),
ast::AssocItem::TypeAlias(def) => def.name(),
ast::AssocItem::Const(def) => def.name(),
ast::AssocItem::MacroCall(_) => None,
}
.is_some()
}
}

/// Given `original_items` retrieved from the trait definition (usually by
/// [`filter_assoc_items()`]), clones each item for update and applies path transformation to it,
/// then inserts into `impl_`. Returns the modified `impl_` and the first associated item that got
/// inserted.
pub fn add_trait_assoc_items_to_impl(
sema: &Semantics<'_, RootDatabase>,
items: Vec<ast::AssocItem>,
original_items: &[InFile<ast::AssocItem>],
trait_: hir::Trait,
impl_: &ast::Impl,
target_scope: hir::SemanticsScope<'_>,
) -> ast::AssocItem {
let source_scope = sema.scope_for_def(trait_);

let transform = PathTransform::trait_impl(&target_scope, &source_scope, trait_, impl_.clone());

let new_indent_level = IndentLevel::from_node(impl_.syntax()) + 1;
let items = items.into_iter().map(|assoc_item| {
transform.apply(assoc_item.syntax());
assoc_item.remove_attrs_and_docs();
assoc_item.reindent_to(new_indent_level);
assoc_item
let items = original_items.into_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()))
{
return formatted;
} else {
stdx::never!("formatted `AssocItem` could not be cast back to `AssocItem`");
}
}
original_item.clone_for_update()
};

if let Some(source_scope) = sema.scope(original_item.syntax()) {
// FIXME: Paths in nested macros are not handled well. See
// `add_missing_impl_members::paths_in_nested_macro_should_get_transformed` test.
let transform =
PathTransform::trait_impl(&target_scope, &source_scope, trait_, impl_.clone());
transform.apply(cloned_item.syntax());
}
cloned_item.remove_attrs_and_docs();
cloned_item.reindent_to(new_indent_level);
cloned_item
});

let assoc_item_list = impl_.get_or_create_assoc_item_list();
Expand Down