From 9cbf09ec4f24aa30af1d9855a909a6cfc67188f7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 18 Mar 2021 12:57:55 +0300 Subject: [PATCH 1/2] rewrite merge use trees assist to use muatable syntax trees changelog internal --- .../ide_assists/src/handlers/merge_imports.rs | 49 +++++++++++-------- .../ide_assists/src/handlers/unmerge_use.rs | 26 +++++----- crates/ide_db/src/helpers/insert_use.rs | 4 +- crates/syntax/src/ast/edit.rs | 46 +++-------------- crates/syntax/src/ast/edit_in_place.rs | 44 +++++++++++++++-- crates/syntax/src/ast/make.rs | 2 +- crates/syntax/src/ted.rs | 28 +++++++++-- 7 files changed, 116 insertions(+), 83 deletions(-) diff --git a/crates/ide_assists/src/handlers/merge_imports.rs b/crates/ide_assists/src/handlers/merge_imports.rs index 7bd7e1e3697..cfc472a3258 100644 --- a/crates/ide_assists/src/handlers/merge_imports.rs +++ b/crates/ide_assists/src/handlers/merge_imports.rs @@ -1,8 +1,5 @@ use ide_db::helpers::insert_use::{try_merge_imports, try_merge_trees, MergeBehavior}; -use syntax::{ - algo::{neighbor, SyntaxRewriter}, - ast, AstNode, -}; +use syntax::{algo::neighbor, ast, ted, AstNode}; use crate::{ assist_context::{AssistContext, Assists}, @@ -24,33 +21,29 @@ // ``` pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { 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 imports = None; + let mut uses = None; 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| { try_merge_imports(&use_item, &use_item2, MergeBehavior::Full).zip(Some(use_item2)) })?; - rewriter.replace_ast(&use_item, &merged); - rewriter += to_delete.remove(); - - if to_delete.syntax().text_range().end() < offset { - offset -= to_delete.syntax().text_range().len(); - } + imports = Some((use_item, merged, to_remove)); } else { - let (merged, to_delete) = + let (merged, to_remove) = next_prev().filter_map(|dir| neighbor(&tree, dir)).find_map(|use_tree| { try_merge_trees(&tree, &use_tree, MergeBehavior::Full).zip(Some(use_tree)) })?; - rewriter.replace_ast(&tree, &merged); - rewriter += to_delete.remove(); - - if to_delete.syntax().text_range().end() < offset { - offset -= to_delete.syntax().text_range().len(); - } + uses = Some((tree.clone(), merged, to_remove)) }; let target = tree.syntax().text_range(); @@ -59,7 +52,23 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext) -> Option<() "Merge imports", target, |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()) }, ) } diff --git a/crates/ide_assists/src/handlers/unmerge_use.rs b/crates/ide_assists/src/handlers/unmerge_use.rs index 616af7c2e6e..8d271e056f5 100644 --- a/crates/ide_assists/src/handlers/unmerge_use.rs +++ b/crates/ide_assists/src/handlers/unmerge_use.rs @@ -1,6 +1,6 @@ use syntax::{ - algo::SyntaxRewriter, - ast::{self, edit::AstNodeEdit, VisibilityOwner}, + ast::{self, VisibilityOwner}, + ted::{self, Position}, AstNode, SyntaxKind, }; @@ -22,7 +22,7 @@ // use std::fmt::Display; // ``` 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::()?.clone_for_update(); let tree_list = tree.syntax().parent().and_then(ast::UseTreeList::cast)?; 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 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(); acc.add( AssistId("unmerge_use", AssistKind::RefactorRewrite), @@ -47,20 +50,13 @@ pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext) -> Option<()> tree.rename(), tree.star_token().is_some(), ), - ); + ) + .clone_for_update(); - let mut rewriter = SyntaxRewriter::default(); - rewriter += tree.remove(); - 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()); + tree.remove(); + ted::insert(Position::after(use_.syntax()), new_use.syntax()); - builder.rewrite(rewriter); + builder.replace(old_parent_range, new_parent.to_string()); }, ) } diff --git a/crates/ide_db/src/helpers/insert_use.rs b/crates/ide_db/src/helpers/insert_use.rs index 37acf95f0fd..20c195f8251 100644 --- a/crates/ide_db/src/helpers/insert_use.rs +++ b/crates/ide_db/src/helpers/insert_use.rs @@ -213,7 +213,7 @@ pub fn try_merge_imports( let lhs_tree = lhs.use_tree()?; let rhs_tree = rhs.use_tree()?; 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( @@ -234,7 +234,7 @@ pub fn try_merge_trees( } else { (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. diff --git a/crates/syntax/src/ast/edit.rs b/crates/syntax/src/ast/edit.rs index 347862b8a4c..18820786a5c 100644 --- a/crates/syntax/src/ast/edit.rs +++ b/crates/syntax/src/ast/edit.rs @@ -9,7 +9,7 @@ use arrayvec::ArrayVec; use crate::{ - algo::{self, neighbor, SyntaxRewriter}, + algo::{self, SyntaxRewriter}, ast::{ self, make::{self, tokens}, @@ -322,27 +322,6 @@ pub fn with_use_tree(&self, use_tree: ast::UseTree) -> ast::Use { } 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 { @@ -396,22 +375,6 @@ fn split_path_prefix(prefix: &ast::Path) -> Option { 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 { @@ -592,6 +555,13 @@ fn add(self, rhs: u8) -> 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 { match node.first_token() { Some(it) => Self::from_token(&it), diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index b1eed0a2cca..529bd0eb14c 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -2,13 +2,13 @@ use std::iter::empty; -use ast::{edit::AstNodeEdit, make, GenericParamsOwner, WhereClause}; use parser::T; use crate::{ - ast, + algo::neighbor, + ast::{self, edit::AstNodeEdit, make, GenericParamsOwner, WhereClause}, ted::{self, Position}, - AstNode, Direction, + AstNode, AstToken, Direction, }; 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()) + } +} diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 7049affd95f..c08f2c14f56 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -560,7 +560,7 @@ pub fn single_space() -> SyntaxToken { pub fn whitespace(text: &str) -> SyntaxToken { assert!(text.trim().is_empty()); 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 { diff --git a/crates/syntax/src/ted.rs b/crates/syntax/src/ted.rs index be2b846b151..177d4ff67a0 100644 --- a/crates/syntax/src/ted.rs +++ b/crates/syntax/src/ted.rs @@ -2,11 +2,14 @@ //! //! The `_raw`-suffixed functions insert elements as is, unsuffixed versions fix //! up elements around the edges. -use std::ops::RangeInclusive; +use std::{mem, ops::RangeInclusive}; 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 /// nodes. Do not use outside of this module. @@ -101,12 +104,25 @@ pub fn insert_all_raw(position: Position, elements: Vec) { } pub fn remove(elem: impl Element) { - let elem = elem.syntax_element(); - remove_all(elem.clone()..=elem) + elem.syntax_element().detach() } pub fn remove_all(range: RangeInclusive) { replace_all(range, Vec::new()) } +pub fn remove_all_iter(range: impl IntoIterator) { + 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) { let old = old.syntax_element(); @@ -149,5 +165,9 @@ fn ws_between(left: &SyntaxElement, right: &SyntaxElement) -> Option Date: Mon, 22 Mar 2021 18:47:19 +0100 Subject: [PATCH 2/2] resolver: manually traverse nested block scopes --- crates/hir_def/src/nameres.rs | 17 +++++++++++++++++ crates/hir_def/src/nameres/path_resolution.rs | 2 +- crates/hir_def/src/resolver.rs | 18 ++++++++++++++---- crates/hir_ty/src/tests/regression.rs | 13 +++++++++++++ 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs index 0d3a0b54fae..9e8e4e9ec67 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -322,6 +322,23 @@ pub(crate) fn resolve_path( (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) { + 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. /// /// If `f` returns `Some(val)`, iteration is stopped and `Some(val)` is returned. If `f` returns diff --git a/crates/hir_def/src/nameres/path_resolution.rs b/crates/hir_def/src/nameres/path_resolution.rs index db459b1ed84..60471937c0c 100644 --- a/crates/hir_def/src/nameres/path_resolution.rs +++ b/crates/hir_def/src/nameres/path_resolution.rs @@ -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, db: &dyn DefDatabase, mode: ResolveMode, diff --git a/crates/hir_def/src/resolver.rs b/crates/hir_def/src/resolver.rs index 04ea9c5d7f9..a73585ee798 100644 --- a/crates/hir_def/src/resolver.rs +++ b/crates/hir_def/src/resolver.rs @@ -548,7 +548,7 @@ fn resolve_path_in_value_ns( path: &ModPath, ) -> Option { 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 { None => { let value = to_value_ns(module_def)?; @@ -578,7 +578,7 @@ fn resolve_path_in_type_ns( path: &ModPath, ) -> Option<(TypeNs, Option)> { 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)?; Some((res, idx)) } @@ -627,8 +627,18 @@ pub trait HasResolver: Copy { impl HasResolver for ModuleId { fn resolver(self, db: &dyn DefDatabase) -> Resolver { - let def_map = self.def_map(db); - Resolver::default().push_module_scope(def_map, self.local_id) + let mut def_map = self.def_map(db); + 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 } } diff --git a/crates/hir_ty/src/tests/regression.rs b/crates/hir_ty/src/tests/regression.rs index 69314e245bd..b69f860502d 100644 --- a/crates/hir_ty/src/tests/regression.rs +++ b/crates/hir_ty/src/tests/regression.rs @@ -961,3 +961,16 @@ fn flush(&self) { "#]], ); } + +#[test] +fn param_overrides_fn() { + check_types( + r#" + fn example(example: i32) { + fn f() {} + example; + //^^^^^^^ i32 + } + "#, + ) +}