8154: rewrite merge use trees assist to use muatable syntax trees r=matklad a=matklad

bors r+
🤖

8155: Fix confusion between parameters and the function r=jonas-schievink a=jonas-schievink

Fixes https://github.com/rust-analyzer/rust-analyzer/issues/8152

bors r+

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
Co-authored-by: Jonas Schievink <jonas.schievink@ferrous-systems.com>
This commit is contained in:
bors[bot] 2021-03-22 17:56:18 +00:00 committed by GitHub
commit 97fe64a5c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 161 additions and 88 deletions

View File

@ -322,6 +322,23 @@ pub(crate) fn resolve_path(
(res.resolved_def, res.segment_index) (res.resolved_def, res.segment_index)
} }
pub(crate) fn resolve_path_locally(
&self,
db: &dyn DefDatabase,
original_module: LocalModuleId,
path: &ModPath,
shadow: BuiltinShadowMode,
) -> (PerNs, Option<usize>) {
let res = self.resolve_path_fp_with_macro_single(
db,
ResolveMode::Other,
original_module,
path,
shadow,
);
(res.resolved_def, res.segment_index)
}
/// Ascends the `DefMap` hierarchy and calls `f` with every `DefMap` and containing module. /// Ascends the `DefMap` hierarchy and calls `f` with every `DefMap` and containing module.
/// ///
/// If `f` returns `Some(val)`, iteration is stopped and `Some(val)` is returned. If `f` returns /// If `f` returns `Some(val)`, iteration is stopped and `Some(val)` is returned. If `f` returns

View File

@ -156,7 +156,7 @@ pub(super) fn resolve_path_fp_with_macro(
} }
} }
fn resolve_path_fp_with_macro_single( pub(super) fn resolve_path_fp_with_macro_single(
&self, &self,
db: &dyn DefDatabase, db: &dyn DefDatabase,
mode: ResolveMode, mode: ResolveMode,

View File

@ -548,7 +548,7 @@ fn resolve_path_in_value_ns(
path: &ModPath, path: &ModPath,
) -> Option<ResolveValueResult> { ) -> Option<ResolveValueResult> {
let (module_def, idx) = let (module_def, idx) =
self.def_map.resolve_path(db, self.module_id, &path, BuiltinShadowMode::Other); self.def_map.resolve_path_locally(db, self.module_id, &path, BuiltinShadowMode::Other);
match idx { match idx {
None => { None => {
let value = to_value_ns(module_def)?; let value = to_value_ns(module_def)?;
@ -578,7 +578,7 @@ fn resolve_path_in_type_ns(
path: &ModPath, path: &ModPath,
) -> Option<(TypeNs, Option<usize>)> { ) -> Option<(TypeNs, Option<usize>)> {
let (module_def, idx) = let (module_def, idx) =
self.def_map.resolve_path(db, self.module_id, &path, BuiltinShadowMode::Other); self.def_map.resolve_path_locally(db, self.module_id, &path, BuiltinShadowMode::Other);
let res = to_type_ns(module_def)?; let res = to_type_ns(module_def)?;
Some((res, idx)) Some((res, idx))
} }
@ -627,8 +627,18 @@ pub trait HasResolver: Copy {
impl HasResolver for ModuleId { impl HasResolver for ModuleId {
fn resolver(self, db: &dyn DefDatabase) -> Resolver { fn resolver(self, db: &dyn DefDatabase) -> Resolver {
let def_map = self.def_map(db); let mut def_map = self.def_map(db);
Resolver::default().push_module_scope(def_map, self.local_id) let mut modules = Vec::new();
modules.push((def_map.clone(), self.local_id));
while let Some(parent) = def_map.parent() {
def_map = parent.def_map(db);
modules.push((def_map.clone(), parent.local_id));
}
let mut resolver = Resolver::default();
for (def_map, module) in modules.into_iter().rev() {
resolver = resolver.push_module_scope(def_map, module);
}
resolver
} }
} }

View File

@ -961,3 +961,16 @@ fn flush(&self) {
"#]], "#]],
); );
} }
#[test]
fn param_overrides_fn() {
check_types(
r#"
fn example(example: i32) {
fn f() {}
example;
//^^^^^^^ i32
}
"#,
)
}

View File

@ -1,8 +1,5 @@
use ide_db::helpers::insert_use::{try_merge_imports, try_merge_trees, MergeBehavior}; use ide_db::helpers::insert_use::{try_merge_imports, try_merge_trees, MergeBehavior};
use syntax::{ use syntax::{algo::neighbor, ast, ted, AstNode};
algo::{neighbor, SyntaxRewriter},
ast, AstNode,
};
use crate::{ use crate::{
assist_context::{AssistContext, Assists}, assist_context::{AssistContext, Assists},
@ -24,33 +21,29 @@
// ``` // ```
pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
let tree: ast::UseTree = ctx.find_node_at_offset()?; let tree: ast::UseTree = ctx.find_node_at_offset()?;
let mut rewriter = SyntaxRewriter::default(); let original_parent = tree.syntax().ancestors().last()?;
let tree = tree.clone_for_update();
let new_parent = tree.syntax().ancestors().last()?;
let mut offset = ctx.offset(); let mut offset = ctx.offset();
let mut imports = None;
let mut uses = None;
if let Some(use_item) = tree.syntax().parent().and_then(ast::Use::cast) { if let Some(use_item) = tree.syntax().parent().and_then(ast::Use::cast) {
let (merged, to_delete) = let (merged, to_remove) =
next_prev().filter_map(|dir| neighbor(&use_item, dir)).find_map(|use_item2| { next_prev().filter_map(|dir| neighbor(&use_item, dir)).find_map(|use_item2| {
try_merge_imports(&use_item, &use_item2, MergeBehavior::Full).zip(Some(use_item2)) try_merge_imports(&use_item, &use_item2, MergeBehavior::Full).zip(Some(use_item2))
})?; })?;
rewriter.replace_ast(&use_item, &merged); imports = Some((use_item, merged, to_remove));
rewriter += to_delete.remove();
if to_delete.syntax().text_range().end() < offset {
offset -= to_delete.syntax().text_range().len();
}
} else { } else {
let (merged, to_delete) = let (merged, to_remove) =
next_prev().filter_map(|dir| neighbor(&tree, dir)).find_map(|use_tree| { next_prev().filter_map(|dir| neighbor(&tree, dir)).find_map(|use_tree| {
try_merge_trees(&tree, &use_tree, MergeBehavior::Full).zip(Some(use_tree)) try_merge_trees(&tree, &use_tree, MergeBehavior::Full).zip(Some(use_tree))
})?; })?;
rewriter.replace_ast(&tree, &merged); uses = Some((tree.clone(), merged, to_remove))
rewriter += to_delete.remove();
if to_delete.syntax().text_range().end() < offset {
offset -= to_delete.syntax().text_range().len();
}
}; };
let target = tree.syntax().text_range(); let target = tree.syntax().text_range();
@ -59,7 +52,23 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()
"Merge imports", "Merge imports",
target, target,
|builder| { |builder| {
builder.rewrite(rewriter); if let Some((to_replace, replacement, to_remove)) = imports {
if to_remove.syntax().text_range().end() < offset {
offset -= to_remove.syntax().text_range().len();
}
ted::replace(to_replace.syntax().clone(), replacement.syntax().clone());
to_remove.remove();
}
if let Some((to_replace, replacement, to_remove)) = uses {
if to_remove.syntax().text_range().end() < offset {
offset -= to_remove.syntax().text_range().len();
}
ted::replace(to_replace.syntax().clone(), replacement.syntax().clone());
to_remove.remove()
}
builder.replace(original_parent.text_range(), new_parent.to_string())
}, },
) )
} }

View File

@ -1,6 +1,6 @@
use syntax::{ use syntax::{
algo::SyntaxRewriter, ast::{self, VisibilityOwner},
ast::{self, edit::AstNodeEdit, VisibilityOwner}, ted::{self, Position},
AstNode, SyntaxKind, AstNode, SyntaxKind,
}; };
@ -22,7 +22,7 @@
// use std::fmt::Display; // use std::fmt::Display;
// ``` // ```
pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
let tree: ast::UseTree = ctx.find_node_at_offset()?; let tree: ast::UseTree = ctx.find_node_at_offset::<ast::UseTree>()?.clone_for_update();
let tree_list = tree.syntax().parent().and_then(ast::UseTreeList::cast)?; let tree_list = tree.syntax().parent().and_then(ast::UseTreeList::cast)?;
if tree_list.use_trees().count() < 2 { if tree_list.use_trees().count() < 2 {
@ -33,6 +33,9 @@ pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
let use_: ast::Use = tree_list.syntax().ancestors().find_map(ast::Use::cast)?; let use_: ast::Use = tree_list.syntax().ancestors().find_map(ast::Use::cast)?;
let path = resolve_full_path(&tree)?; let path = resolve_full_path(&tree)?;
let old_parent_range = use_.syntax().parent()?.text_range();
let new_parent = use_.syntax().parent()?;
let target = tree.syntax().text_range(); let target = tree.syntax().text_range();
acc.add( acc.add(
AssistId("unmerge_use", AssistKind::RefactorRewrite), AssistId("unmerge_use", AssistKind::RefactorRewrite),
@ -47,20 +50,13 @@ pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
tree.rename(), tree.rename(),
tree.star_token().is_some(), tree.star_token().is_some(),
), ),
); )
.clone_for_update();
let mut rewriter = SyntaxRewriter::default(); tree.remove();
rewriter += tree.remove(); ted::insert(Position::after(use_.syntax()), new_use.syntax());
rewriter.insert_after(use_.syntax(), &ast::make::tokens::single_newline());
if let ident_level @ 1..=usize::MAX = use_.indent_level().0 as usize {
rewriter.insert_after(
use_.syntax(),
&ast::make::tokens::whitespace(&" ".repeat(4 * ident_level)),
);
}
rewriter.insert_after(use_.syntax(), new_use.syntax());
builder.rewrite(rewriter); builder.replace(old_parent_range, new_parent.to_string());
}, },
) )
} }

View File

@ -213,7 +213,7 @@ pub fn try_merge_imports(
let lhs_tree = lhs.use_tree()?; let lhs_tree = lhs.use_tree()?;
let rhs_tree = rhs.use_tree()?; let rhs_tree = rhs.use_tree()?;
let merged = try_merge_trees(&lhs_tree, &rhs_tree, merge_behavior)?; let merged = try_merge_trees(&lhs_tree, &rhs_tree, merge_behavior)?;
Some(lhs.with_use_tree(merged)) Some(lhs.with_use_tree(merged).clone_for_update())
} }
pub fn try_merge_trees( pub fn try_merge_trees(
@ -234,7 +234,7 @@ pub fn try_merge_trees(
} else { } else {
(lhs.split_prefix(&lhs_prefix), rhs.split_prefix(&rhs_prefix)) (lhs.split_prefix(&lhs_prefix), rhs.split_prefix(&rhs_prefix))
}; };
recursive_merge(&lhs, &rhs, merge) recursive_merge(&lhs, &rhs, merge).map(|it| it.clone_for_update())
} }
/// Recursively "zips" together lhs and rhs. /// Recursively "zips" together lhs and rhs.

View File

@ -9,7 +9,7 @@
use arrayvec::ArrayVec; use arrayvec::ArrayVec;
use crate::{ use crate::{
algo::{self, neighbor, SyntaxRewriter}, algo::{self, SyntaxRewriter},
ast::{ ast::{
self, self,
make::{self, tokens}, make::{self, tokens},
@ -322,27 +322,6 @@ pub fn with_use_tree(&self, use_tree: ast::UseTree) -> ast::Use {
} }
self.clone() self.clone()
} }
pub fn remove(&self) -> SyntaxRewriter<'static> {
let mut res = SyntaxRewriter::default();
res.delete(self.syntax());
let next_ws = self
.syntax()
.next_sibling_or_token()
.and_then(|it| it.into_token())
.and_then(ast::Whitespace::cast);
if let Some(next_ws) = next_ws {
let ws_text = next_ws.syntax().text();
if let Some(rest) = ws_text.strip_prefix('\n') {
if rest.is_empty() {
res.delete(next_ws.syntax())
} else {
res.replace(next_ws.syntax(), &make::tokens::whitespace(rest));
}
}
}
res
}
} }
impl ast::UseTree { impl ast::UseTree {
@ -396,22 +375,6 @@ fn split_path_prefix(prefix: &ast::Path) -> Option<ast::Path> {
Some(res) Some(res)
} }
} }
pub fn remove(&self) -> SyntaxRewriter<'static> {
let mut res = SyntaxRewriter::default();
res.delete(self.syntax());
for &dir in [Direction::Next, Direction::Prev].iter() {
if let Some(nb) = neighbor(self, dir) {
self.syntax()
.siblings_with_tokens(dir)
.skip(1)
.take_while(|it| it.as_node() != Some(nb.syntax()))
.for_each(|el| res.delete(&el));
return res;
}
}
res
}
} }
impl ast::MatchArmList { impl ast::MatchArmList {
@ -592,6 +555,13 @@ fn add(self, rhs: u8) -> IndentLevel {
} }
impl IndentLevel { impl IndentLevel {
pub fn from_element(element: &SyntaxElement) -> IndentLevel {
match element {
rowan::NodeOrToken::Node(it) => IndentLevel::from_node(it),
rowan::NodeOrToken::Token(it) => IndentLevel::from_token(it),
}
}
pub fn from_node(node: &SyntaxNode) -> IndentLevel { pub fn from_node(node: &SyntaxNode) -> IndentLevel {
match node.first_token() { match node.first_token() {
Some(it) => Self::from_token(&it), Some(it) => Self::from_token(&it),

View File

@ -2,13 +2,13 @@
use std::iter::empty; use std::iter::empty;
use ast::{edit::AstNodeEdit, make, GenericParamsOwner, WhereClause};
use parser::T; use parser::T;
use crate::{ use crate::{
ast, algo::neighbor,
ast::{self, edit::AstNodeEdit, make, GenericParamsOwner, WhereClause},
ted::{self, Position}, ted::{self, Position},
AstNode, Direction, AstNode, AstToken, Direction,
}; };
use super::NameOwner; use super::NameOwner;
@ -126,3 +126,41 @@ pub fn remove(&self) {
} }
} }
} }
impl ast::UseTree {
pub fn remove(&self) {
for &dir in [Direction::Next, Direction::Prev].iter() {
if let Some(next_use_tree) = neighbor(self, dir) {
let separators = self
.syntax()
.siblings_with_tokens(dir)
.skip(1)
.take_while(|it| it.as_node() != Some(next_use_tree.syntax()));
ted::remove_all_iter(separators);
break;
}
}
ted::remove(self.syntax())
}
}
impl ast::Use {
pub fn remove(&self) {
let next_ws = self
.syntax()
.next_sibling_or_token()
.and_then(|it| it.into_token())
.and_then(ast::Whitespace::cast);
if let Some(next_ws) = next_ws {
let ws_text = next_ws.syntax().text();
if let Some(rest) = ws_text.strip_prefix('\n') {
if rest.is_empty() {
ted::remove(next_ws.syntax())
} else {
ted::replace(next_ws.syntax(), make::tokens::whitespace(rest))
}
}
}
ted::remove(self.syntax())
}
}

View File

@ -560,7 +560,7 @@ pub fn single_space() -> SyntaxToken {
pub fn whitespace(text: &str) -> SyntaxToken { pub fn whitespace(text: &str) -> SyntaxToken {
assert!(text.trim().is_empty()); assert!(text.trim().is_empty());
let sf = SourceFile::parse(text).ok().unwrap(); let sf = SourceFile::parse(text).ok().unwrap();
sf.syntax().first_child_or_token().unwrap().into_token().unwrap() sf.syntax().clone_for_update().first_child_or_token().unwrap().into_token().unwrap()
} }
pub fn doc_comment(text: &str) -> SyntaxToken { pub fn doc_comment(text: &str) -> SyntaxToken {

View File

@ -2,11 +2,14 @@
//! //!
//! The `_raw`-suffixed functions insert elements as is, unsuffixed versions fix //! The `_raw`-suffixed functions insert elements as is, unsuffixed versions fix
//! up elements around the edges. //! up elements around the edges.
use std::ops::RangeInclusive; use std::{mem, ops::RangeInclusive};
use parser::T; use parser::T;
use crate::{ast::make, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken}; use crate::{
ast::{edit::IndentLevel, make},
SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken,
};
/// Utility trait to allow calling `ted` functions with references or owned /// Utility trait to allow calling `ted` functions with references or owned
/// nodes. Do not use outside of this module. /// nodes. Do not use outside of this module.
@ -101,12 +104,25 @@ pub fn insert_all_raw(position: Position, elements: Vec<SyntaxElement>) {
} }
pub fn remove(elem: impl Element) { pub fn remove(elem: impl Element) {
let elem = elem.syntax_element(); elem.syntax_element().detach()
remove_all(elem.clone()..=elem)
} }
pub fn remove_all(range: RangeInclusive<SyntaxElement>) { pub fn remove_all(range: RangeInclusive<SyntaxElement>) {
replace_all(range, Vec::new()) replace_all(range, Vec::new())
} }
pub fn remove_all_iter(range: impl IntoIterator<Item = SyntaxElement>) {
let mut it = range.into_iter();
if let Some(mut first) = it.next() {
match it.last() {
Some(mut last) => {
if first.index() > last.index() {
mem::swap(&mut first, &mut last)
}
remove_all(first..=last)
}
None => remove(first),
}
}
}
pub fn replace(old: impl Element, new: impl Element) { pub fn replace(old: impl Element, new: impl Element) {
let old = old.syntax_element(); let old = old.syntax_element();
@ -149,5 +165,9 @@ fn ws_between(left: &SyntaxElement, right: &SyntaxElement) -> Option<SyntaxToken
if right.kind() == T![;] || right.kind() == T![,] { if right.kind() == T![;] || right.kind() == T![,] {
return None; return None;
} }
if right.kind() == SyntaxKind::USE {
let indent = IndentLevel::from_element(left);
return Some(make::tokens::whitespace(&format!("\n{}", indent)));
}
Some(make::tokens::single_space()) Some(make::tokens::single_space())
} }