From 2311e96ca872abf3844fc15b81c20bb3f51809a5 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Sat, 27 Jul 2024 17:09:53 -0400 Subject: [PATCH 01/11] wip: new syntax tree editor --- .../rust-analyzer/crates/syntax/src/lib.rs | 1 + .../crates/syntax/src/syntax_editor.rs | 333 ++++++++++++++++++ .../syntax/src/syntax_editor/edit_algo.rs | 215 +++++++++++ .../syntax/src/syntax_editor/mapping.rs | 174 +++++++++ 4 files changed, 723 insertions(+) create mode 100644 src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs create mode 100644 src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs create mode 100644 src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs diff --git a/src/tools/rust-analyzer/crates/syntax/src/lib.rs b/src/tools/rust-analyzer/crates/syntax/src/lib.rs index b68374848b9..dbbb290b4f1 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/lib.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/lib.rs @@ -40,6 +40,7 @@ #[doc(hidden)] pub mod fuzz; pub mod hacks; +pub mod syntax_editor; pub mod ted; pub mod utils; diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs new file mode 100644 index 00000000000..3ec5abcaeaf --- /dev/null +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs @@ -0,0 +1,333 @@ +//! Syntax Tree editor +//! +//! Inspired by Roslyn's [`SyntaxEditor`], but is temporarily built upon mutable syntax tree editing. +//! +//! [`SyntaxEditor`]: https://github.com/dotnet/roslyn/blob/43b0b05cc4f492fd5de00f6f6717409091df8daa/src/Workspaces/Core/Portable/Editing/SyntaxEditor.cs + +use std::{ + num::NonZeroU32, + sync::atomic::{AtomicU32, Ordering}, +}; + +use rowan::TextRange; +use rustc_hash::FxHashMap; + +use crate::{SyntaxElement, SyntaxNode, SyntaxToken}; + +mod edit_algo; +mod mapping; + +pub use mapping::{SyntaxMapping, SyntaxMappingBuilder}; + +#[derive(Debug)] +pub struct SyntaxEditor { + root: SyntaxNode, + changes: Vec, + mappings: SyntaxMapping, + annotations: Vec<(SyntaxElement, SyntaxAnnotation)>, +} + +impl SyntaxEditor { + /// Creates a syntax editor to start editing from `root` + pub fn new(root: SyntaxNode) -> Self { + Self { root, changes: vec![], mappings: SyntaxMapping::new(), annotations: vec![] } + } + + pub fn add_annotation(&mut self, element: impl Element, annotation: SyntaxAnnotation) { + self.annotations.push((element.syntax_element(), annotation)) + } + + pub fn combine(&mut self, other: SyntaxEditor) { + todo!() + } + + pub fn delete(&mut self, element: impl Element) { + self.changes.push(Change::Replace(element.syntax_element(), None)); + } + + pub fn replace(&mut self, old: impl Element, new: impl Element) { + self.changes.push(Change::Replace(old.syntax_element(), Some(new.syntax_element()))); + } + + pub fn finish(self) -> SyntaxEdit { + edit_algo::apply_edits(self) + } +} + +pub struct SyntaxEdit { + root: SyntaxNode, + changed_elements: Vec, + annotations: FxHashMap>, +} + +impl SyntaxEdit { + pub fn root(&self) -> &SyntaxNode { + &self.root + } + + pub fn changed_elements(&self) -> &[SyntaxElement] { + self.changed_elements.as_slice() + } + + pub fn find_annotation(&self, annotation: SyntaxAnnotation) -> Option<&[SyntaxElement]> { + self.annotations.get(&annotation).as_ref().map(|it| it.as_slice()) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[repr(transparent)] +pub struct SyntaxAnnotation(NonZeroU32); + +impl SyntaxAnnotation { + /// Creates a unique syntax annotation to attach data to. + pub fn new() -> Self { + static COUNTER: AtomicU32 = AtomicU32::new(1); + + // We want the id to be unique across threads, but we don't want to + // tie it to other `SeqCst` operations. + let id = COUNTER.fetch_add(1, Ordering::AcqRel); + + Self(NonZeroU32::new(id).expect("syntax annotation id overflow")) + } +} + +/// Position describing where to insert elements +#[derive(Debug)] +pub struct Position { + repr: PositionRepr, +} + +#[derive(Debug)] +enum PositionRepr { + FirstChild(SyntaxNode), + After(SyntaxElement), +} + +impl Position { + pub fn after(elem: impl Element) -> Position { + let repr = PositionRepr::After(elem.syntax_element()); + Position { repr } + } + + pub fn before(elem: impl Element) -> Position { + let elem = elem.syntax_element(); + let repr = match elem.prev_sibling_or_token() { + Some(it) => PositionRepr::After(it), + None => PositionRepr::FirstChild(elem.parent().unwrap()), + }; + Position { repr } + } + + pub fn first_child_of(node: &(impl Into + Clone)) -> Position { + let repr = PositionRepr::FirstChild(node.clone().into()); + Position { repr } + } + + pub fn last_child_of(node: &(impl Into + Clone)) -> Position { + let node = node.clone().into(); + let repr = match node.last_child_or_token() { + Some(it) => PositionRepr::After(it), + None => PositionRepr::FirstChild(node), + }; + Position { repr } + } +} + +#[derive(Debug)] +enum Change { + /// Represents both a replace single element and a delete element operation. + Replace(SyntaxElement, Option), +} + +impl Change { + fn target_range(&self) -> TextRange { + match self { + Change::Replace(target, _) => target.text_range(), + } + } + + fn target_parent(&self) -> SyntaxNode { + match self { + Change::Replace(target, _) => target.parent().unwrap(), + } + } + + fn change_kind(&self) -> ChangeKind { + match self { + Change::Replace(_, _) => ChangeKind::Replace, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +enum ChangeKind { + Insert, + // TODO: deal with replace spans + Replace, +} + +/// Utility trait to allow calling syntax editor functions with references or owned +/// nodes. Do not use outside of this module. +pub trait Element { + fn syntax_element(self) -> SyntaxElement; +} + +impl Element for &'_ E { + fn syntax_element(self) -> SyntaxElement { + self.clone().syntax_element() + } +} + +impl Element for SyntaxElement { + fn syntax_element(self) -> SyntaxElement { + self + } +} + +impl Element for SyntaxNode { + fn syntax_element(self) -> SyntaxElement { + self.into() + } +} + +impl Element for SyntaxToken { + fn syntax_element(self) -> SyntaxElement { + self.into() + } +} + +#[cfg(test)] +mod tests { + use expect_test::expect; + use itertools::Itertools; + + use crate::{ + ast::{self, make, HasName}, + AstNode, + }; + + use super::*; + + fn make_ident_pat( + editor: Option<&mut SyntaxEditor>, + ref_: bool, + mut_: bool, + name: ast::Name, + ) -> ast::IdentPat { + let ast = make::ident_pat(ref_, mut_, name.clone()).clone_for_update(); + + if let Some(editor) = editor { + let mut mapping = SyntaxMappingBuilder::new(ast.syntax().clone()); + mapping.map_node(name.syntax().clone(), ast.name().unwrap().syntax().clone()); + mapping.finish(editor); + } + + ast + } + + fn make_let_stmt( + editor: Option<&mut SyntaxEditor>, + pattern: ast::Pat, + ty: Option, + initializer: Option, + ) -> ast::LetStmt { + let ast = + make::let_stmt(pattern.clone(), ty.clone(), initializer.clone()).clone_for_update(); + + if let Some(editor) = editor { + let mut mapping = SyntaxMappingBuilder::new(ast.syntax().clone()); + mapping.map_node(pattern.syntax().clone(), ast.pat().unwrap().syntax().clone()); + if let Some(input) = ty { + mapping.map_node(input.syntax().clone(), ast.ty().unwrap().syntax().clone()); + } + if let Some(input) = initializer { + mapping + .map_node(input.syntax().clone(), ast.initializer().unwrap().syntax().clone()); + } + mapping.finish(editor); + } + + ast + } + + fn make_block_expr( + editor: Option<&mut SyntaxEditor>, + stmts: impl IntoIterator, + tail_expr: Option, + ) -> ast::BlockExpr { + let stmts = stmts.into_iter().collect_vec(); + let input = stmts.iter().map(|it| it.syntax().clone()).collect_vec(); + + let ast = make::block_expr(stmts, tail_expr.clone()).clone_for_update(); + + if let Some((editor, stmt_list)) = editor.zip(ast.stmt_list()) { + let mut mapping = SyntaxMappingBuilder::new(stmt_list.syntax().clone()); + + mapping.map_children( + input.into_iter(), + stmt_list.statements().map(|it| it.syntax().clone()), + ); + + if let Some((input, output)) = tail_expr.zip(stmt_list.tail_expr()) { + mapping.map_node(input.syntax().clone(), output.syntax().clone()); + } + + mapping.finish(editor); + } + + ast + } + + #[test] + fn it() { + let root = make::match_arm( + [make::wildcard_pat().into()], + None, + make::expr_tuple([ + make::expr_bin_op( + make::expr_literal("2").into(), + ast::BinaryOp::ArithOp(ast::ArithOp::Add), + make::expr_literal("2").into(), + ), + make::expr_literal("true").into(), + ]), + ); + + let to_wrap = root.syntax().descendants().find_map(ast::TupleExpr::cast).unwrap(); + let to_replace = root.syntax().descendants().find_map(ast::BinExpr::cast).unwrap(); + + let mut editor = SyntaxEditor::new(root.syntax().clone()); + + let name = make::name("var_name"); + let name_ref = make::name_ref("var_name").clone_for_update(); + + let placeholder_snippet = SyntaxAnnotation::new(); + editor.add_annotation(name.syntax(), placeholder_snippet); + editor.add_annotation(name_ref.syntax(), placeholder_snippet); + + let make_ident_pat = make_ident_pat(Some(&mut editor), false, false, name); + let make_let_stmt = make_let_stmt( + Some(&mut editor), + make_ident_pat.into(), + None, + Some(to_replace.clone().into()), + ); + let new_block = make_block_expr( + Some(&mut editor), + [make_let_stmt.into()], + Some(to_wrap.clone().into()), + ); + + // should die: + editor.replace(to_replace.syntax(), name_ref.syntax()); + editor.replace(to_wrap.syntax(), new_block.syntax()); + // editor.replace(to_replace.syntax(), name_ref.syntax()); + + // dbg!(&editor.mappings); + let edit = editor.finish(); + + let expect = expect![]; + expect.assert_eq(&edit.root.to_string()); + assert_eq!(edit.find_annotation(placeholder_snippet).map(|it| it.len()), Some(2)); + } +} diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs new file mode 100644 index 00000000000..f8354c5eb67 --- /dev/null +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs @@ -0,0 +1,215 @@ +use std::{collections::VecDeque, ops::RangeInclusive}; + +use rowan::TextRange; + +use crate::{ + syntax_editor::{Change, ChangeKind}, + ted, SyntaxElement, SyntaxNode, SyntaxNodePtr, +}; + +use super::{SyntaxEdit, SyntaxEditor}; + +pub(super) fn apply_edits(editor: SyntaxEditor) -> SyntaxEdit { + // Algorithm overview: + // + // - Sort changes by (range, type) + // - Ensures that parent edits are before child edits + // - Ensures that inserts will be guaranteed to be inserted at the right range + // - Validate changes + // - Checking for invalid changes is easy since the changes will be sorted by range + // - Fixup change targets + // - standalone change? map to original syntax tree + // - dependent change? + // - try to map to parent change (either independent or another dependent) + // - note: need to keep track of a parent change stack, since a change can be a parent of multiple changes + // - Apply changes + // - find changes to apply to real tree by applying nested changes first + // - changed nodes become part of the changed node set (useful for the formatter to only change those parts) + // - Propagate annotations + + let SyntaxEditor { root, mut changes, mappings, annotations } = editor; + + dbg!(("initial: ", &root)); + dbg!(&changes); + + // Sort changes by range then change kind, so that we can: + // - ensure that parent edits are ordered before child edits + // - ensure that inserts will be guaranteed to be inserted at the right range + // - easily check for disjoint replace ranges + changes.sort_by(|a, b| { + a.target_range() + .start() + .cmp(&b.target_range().start()) + .then(a.change_kind().cmp(&b.change_kind())) + }); + + let disjoint_replaces_ranges = changes.iter().zip(changes.iter().skip(1)).all(|(l, r)| { + l.change_kind() == ChangeKind::Replace + && r.change_kind() == ChangeKind::Replace + && (l.target_parent() != r.target_parent() + || l.target_range().intersect(r.target_range()).is_none()) + }); + + if stdx::never!( + !disjoint_replaces_ranges, + "some replace change ranges intersect: {:?}", + changes + ) { + return SyntaxEdit { root, annotations: Default::default(), changed_elements: vec![] }; + } + + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] + struct DependentChange { + parent: u32, + child: u32, + } + + // Build change tree + let mut changed_ancestors: VecDeque = VecDeque::new(); + let mut dependent_changes = vec![]; + let mut independent_changes = vec![]; + + for (change_index, change) in changes.iter().enumerate() { + // Check if this change is dependent on another change (i.e. it's contained within another range) + if let Some(index) = changed_ancestors + .iter() + .rev() + .position(|ancestor| ancestor.affected_range().contains_range(change.target_range())) + { + // Pop off any ancestors that aren't applicable + changed_ancestors.drain((index + 1)..); + + let ancestor = &changed_ancestors[index]; + + dependent_changes.push(DependentChange { + parent: ancestor.change_index as u32, + child: change_index as u32, + }); + } else { + // This change is independent of any other change + + // Drain the changed ancestors since we're no longer in a set of dependent changes + changed_ancestors.drain(..); + + independent_changes.push(change_index as u32); + } + + // Add to changed ancestors, if applicable + match change { + Change::Replace(target, _) => { + changed_ancestors.push_back(ChangedAncestor::single(target, change_index)) + } + } + } + + dbg!(("before: ", &changes, &dependent_changes, &independent_changes)); + + // Map change targets to the correct syntax nodes + let tree_mutator = TreeMutator::new(&root); + + for index in independent_changes { + match &mut changes[index as usize] { + Change::Replace(target, _) => { + *target = tree_mutator.make_element_mut(target); + } + } + } + + for DependentChange { parent, child } in dependent_changes.into_iter() { + let (input_ancestor, output_ancestor) = match &changes[parent as usize] { + // insert? unreachable + Change::Replace(target, Some(new_target)) => { + (to_owning_node(target), to_owning_node(new_target)) + } + Change::Replace(_, None) => continue, // silently drop outdated change + }; + + match &mut changes[child as usize] { + Change::Replace(target, _) => { + *target = mappings.upmap_child_element(target, &input_ancestor, output_ancestor) + } + } + } + + dbg!(("after: ", &changes)); + + // Apply changes + for change in changes { + match change { + Change::Replace(target, None) => ted::remove(target), + Change::Replace(target, Some(new_target)) => ted::replace(target, new_target), + } + } + + dbg!(("modified:", tree_mutator.mutable_clone)); + + todo!("draw the rest of the owl") +} + +fn to_owning_node(element: &SyntaxElement) -> SyntaxNode { + match element { + SyntaxElement::Node(node) => node.clone(), + SyntaxElement::Token(token) => token.parent().unwrap().clone(), + } +} + +struct ChangedAncestor { + kind: ChangedAncestorKind, + change_index: usize, +} + +enum ChangedAncestorKind { + Single { node: SyntaxNode }, + Range { changed_nodes: RangeInclusive, in_parent: SyntaxNode }, +} + +impl ChangedAncestor { + fn single(element: &SyntaxElement, change_index: usize) -> Self { + let kind = match element { + SyntaxElement::Node(node) => ChangedAncestorKind::Single { node: node.clone() }, + SyntaxElement::Token(token) => { + ChangedAncestorKind::Single { node: token.parent().unwrap() } + } + }; + + Self { kind, change_index } + } + + fn affected_range(&self) -> TextRange { + match &self.kind { + ChangedAncestorKind::Single { node } => node.text_range(), + ChangedAncestorKind::Range { changed_nodes, in_parent: _ } => TextRange::new( + changed_nodes.start().text_range().start(), + changed_nodes.end().text_range().end(), + ), + } + } +} + +struct TreeMutator { + immutable: SyntaxNode, + mutable_clone: SyntaxNode, +} + +impl TreeMutator { + fn new(immutable: &SyntaxNode) -> TreeMutator { + let immutable = immutable.clone(); + let mutable_clone = immutable.clone_for_update(); + TreeMutator { immutable, mutable_clone } + } + + fn make_element_mut(&self, element: &SyntaxElement) -> SyntaxElement { + match element { + SyntaxElement::Node(node) => SyntaxElement::Node(self.make_syntax_mut(&node)), + SyntaxElement::Token(token) => { + let parent = self.make_syntax_mut(&token.parent().unwrap()); + parent.children_with_tokens().nth(token.index()).unwrap() + } + } + } + + fn make_syntax_mut(&self, node: &SyntaxNode) -> SyntaxNode { + let ptr = SyntaxNodePtr::new(node); + ptr.to_node(&self.mutable_clone) + } +} diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs new file mode 100644 index 00000000000..11c7b395b37 --- /dev/null +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs @@ -0,0 +1,174 @@ +use itertools::Itertools; +use rustc_hash::FxHashMap; + +use crate::{SyntaxElement, SyntaxNode}; + +use super::SyntaxEditor; + +#[derive(Debug, Default)] +pub struct SyntaxMapping { + // important information to keep track of: + // node -> node + // token -> token (implicit in mappings) + // input parent -> output parent (for deep lookups) + + // mappings -> parents + entry_parents: Vec, + node_mappings: FxHashMap, +} + +impl SyntaxMapping { + pub fn new() -> Self { + Self::default() + } + + pub fn upmap_child_element( + &self, + child: &SyntaxElement, + input_ancestor: &SyntaxNode, + output_ancestor: SyntaxNode, + ) -> SyntaxElement { + match child { + SyntaxElement::Node(node) => { + SyntaxElement::Node(self.upmap_child(node, input_ancestor, output_ancestor)) + } + SyntaxElement::Token(token) => { + let upmap_parent = + self.upmap_child(&token.parent().unwrap(), input_ancestor, output_ancestor); + + let element = upmap_parent.children_with_tokens().nth(token.index()).unwrap(); + debug_assert!( + element.as_token().is_some_and(|it| it.kind() == token.kind()), + "token upmapping mapped to the wrong node ({token:?} -> {element:?})" + ); + + element + } + } + } + + pub fn upmap_child( + &self, + child: &SyntaxNode, + input_ancestor: &SyntaxNode, + output_ancestor: SyntaxNode, + ) -> SyntaxNode { + debug_assert!(child.ancestors().any(|ancestor| &ancestor == input_ancestor)); + + // Build a list mapping up to the first mappable ancestor + let to_first_upmap = + std::iter::successors(Some((child.index(), child.clone())), |(_, current)| { + let parent = current.parent().unwrap(); + + if &parent == input_ancestor { + return None; + } + + Some((parent.index(), parent)) + }) + .map(|(i, _)| i) + .collect::>(); + + // Progressively up-map the input ancestor until we get to the output ancestor + let to_output_ancestor = if input_ancestor != &output_ancestor { + std::iter::successors(Some((input_ancestor.index(), self.upmap_node(input_ancestor).unwrap_or_else(|| input_ancestor.clone()))), |(_, current)| { + let Some(parent) = current.parent() else { + unreachable!("no mappings exist between {current:?} (ancestor of {input_ancestor:?}) and {output_ancestor:?}") + }; + + if &parent == &output_ancestor { + return None; + } + + if let Some(next) = self.upmap_node(&parent) { + Some((parent.index(), next)) + } else { + Some((parent.index(), parent)) + } + }).map(|(i, _)| i).collect::>() + } else { + vec![] + }; + + let to_map_down = + to_output_ancestor.into_iter().rev().chain(to_first_upmap.into_iter().rev()); + + let mut target = output_ancestor; + + for index in to_map_down { + target = target + .children_with_tokens() + .nth(index) + .and_then(|it| it.into_node()) + .expect("yep"); + } + + debug_assert_eq!(child.kind(), target.kind()); + + target + } + + pub fn upmap_node(&self, input: &SyntaxNode) -> Option { + let (parent, child_slot) = self.node_mappings.get(input)?; + + let output = self.entry_parents[*parent as usize] + .children_with_tokens() + .nth(*child_slot as usize) + .and_then(SyntaxElement::into_node) + .unwrap(); + + debug_assert_eq!(input.kind(), output.kind()); + Some(output) + } + + fn add_mapping(&mut self, syntax_mapping: SyntaxMappingBuilder) { + let SyntaxMappingBuilder { parent_node, node_mappings } = syntax_mapping; + + let parent_entry: u32 = self.entry_parents.len() as u32; + self.entry_parents.push(parent_node); + + let node_entries = + node_mappings.into_iter().map(|(node, slot)| (node, (parent_entry, slot))); + + self.node_mappings.extend(node_entries); + } +} + +#[derive(Debug)] +pub struct SyntaxMappingBuilder { + parent_node: SyntaxNode, + node_mappings: Vec<(SyntaxNode, u32)>, +} + +impl SyntaxMappingBuilder { + pub fn new(parent_node: SyntaxNode) -> Self { + Self { parent_node, node_mappings: vec![] } + } + + pub fn map_node(&mut self, input: SyntaxNode, output: SyntaxNode) { + debug_assert_eq!(output.parent().as_ref(), Some(&self.parent_node)); + self.node_mappings.push((input, output.index() as u32)); + } + + pub fn map_children( + &mut self, + input: impl Iterator, + output: impl Iterator, + ) { + for pairs in input.zip_longest(output) { + let (input, output) = match pairs { + itertools::EitherOrBoth::Both(l, r) => (l, r), + itertools::EitherOrBoth::Left(_) => { + unreachable!("mapping more input nodes than there are output nodes") + } + itertools::EitherOrBoth::Right(_) => break, + }; + + self.map_node(input, output); + } + } + + pub fn finish(self, editor: &mut SyntaxEditor) { + editor.mappings.add_mapping(self); + } +} From 5e33650b7277543d793a9abe69b298e73b7531e7 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Sun, 1 Sep 2024 23:48:14 -0400 Subject: [PATCH 02/11] elaborate SyntaxEdit comments --- .../crates/syntax/src/syntax_editor.rs | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs index 3ec5abcaeaf..240bba7b483 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs @@ -61,16 +61,28 @@ pub struct SyntaxEdit { } impl SyntaxEdit { + /// Root of the modified syntax tree pub fn root(&self) -> &SyntaxNode { &self.root } + /// Which syntax elements in the modified syntax tree were modified as part + /// of the edit. + /// + /// Note that for syntax nodes, only the upper-most parent of a set of + /// changes is included, not any child elements that may have been modified. pub fn changed_elements(&self) -> &[SyntaxElement] { self.changed_elements.as_slice() } - pub fn find_annotation(&self, annotation: SyntaxAnnotation) -> Option<&[SyntaxElement]> { - self.annotations.get(&annotation).as_ref().map(|it| it.as_slice()) + /// Finds which syntax elements have been annotated with the given + /// annotation. + /// + /// Note that an annotation might not appear in the modified syntax tree if + /// the syntax elements that were annotated did not make it into the final + /// syntax tree. + pub fn find_annotation(&self, annotation: SyntaxAnnotation) -> &[SyntaxElement] { + self.annotations.get(&annotation).as_ref().map_or(&[], |it| it.as_slice()) } } @@ -83,9 +95,8 @@ impl SyntaxAnnotation { pub fn new() -> Self { static COUNTER: AtomicU32 = AtomicU32::new(1); - // We want the id to be unique across threads, but we don't want to - // tie it to other `SeqCst` operations. - let id = COUNTER.fetch_add(1, Ordering::AcqRel); + // Only consistency within a thread matters, as SyntaxElements are !Send + let id = COUNTER.fetch_add(1, Ordering::Relaxed); Self(NonZeroU32::new(id).expect("syntax annotation id overflow")) } @@ -328,6 +339,6 @@ fn it() { let expect = expect![]; expect.assert_eq(&edit.root.to_string()); - assert_eq!(edit.find_annotation(placeholder_snippet).map(|it| it.len()), Some(2)); + assert_eq!(edit.find_annotation(placeholder_snippet).len(), 2); } } From 2094a2b117fa0a00a3dcb0a239954b84eeda031c Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 2 Sep 2024 00:01:24 -0400 Subject: [PATCH 03/11] handle merging two syntax editors together --- .../crates/syntax/src/syntax_editor.rs | 15 ++++++-- .../syntax/src/syntax_editor/mapping.rs | 38 +++++++++++++------ 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs index 240bba7b483..42373eba827 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs @@ -37,8 +37,17 @@ pub fn add_annotation(&mut self, element: impl Element, annotation: SyntaxAnnota self.annotations.push((element.syntax_element(), annotation)) } - pub fn combine(&mut self, other: SyntaxEditor) { - todo!() + pub fn merge(&mut self, mut other: SyntaxEditor) { + debug_assert!( + self.root == other.root || other.root.ancestors().any(|node| node == self.root), + "{:?} is not in the same tree as {:?}", + other.root, + self.root + ); + + self.changes.append(&mut other.changes); + self.mappings.merge(other.mappings); + self.annotations.append(&mut other.annotations); } pub fn delete(&mut self, element: impl Element) { @@ -290,7 +299,7 @@ fn make_block_expr( } #[test] - fn it() { + fn basic_usage() { let root = make::match_arm( [make::wildcard_pat().into()], None, diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs index 11c7b395b37..8a79f7e186e 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs @@ -14,7 +14,7 @@ pub struct SyntaxMapping { // mappings -> parents entry_parents: Vec, - node_mappings: FxHashMap, + node_mappings: FxHashMap, } impl SyntaxMapping { @@ -80,11 +80,10 @@ pub fn upmap_child( return None; } - if let Some(next) = self.upmap_node(&parent) { - Some((parent.index(), next)) - } else { - Some((parent.index(), parent)) - } + Some((parent.index(), match self.upmap_node(&parent) { + Some(next) => next, + None => parent + })) }).map(|(i, _)| i).collect::>() } else { vec![] @@ -100,7 +99,7 @@ pub fn upmap_child( .children_with_tokens() .nth(index) .and_then(|it| it.into_node()) - .expect("yep"); + .expect("equivalent ancestor node should be present in target tree"); } debug_assert_eq!(child.kind(), target.kind()); @@ -109,7 +108,7 @@ pub fn upmap_child( } pub fn upmap_node(&self, input: &SyntaxNode) -> Option { - let (parent, child_slot) = self.node_mappings.get(input)?; + let MappingEntry { parent, child_slot } = self.node_mappings.get(input)?; let output = self.entry_parents[*parent as usize] .children_with_tokens() @@ -121,14 +120,25 @@ pub fn upmap_node(&self, input: &SyntaxNode) -> Option { Some(output) } + pub fn merge(&mut self, mut other: SyntaxMapping) { + // Remap other's entry parents to be after the current list of entry parents + let remap_base: u32 = self.entry_parents.len().try_into().unwrap(); + + self.entry_parents.append(&mut other.entry_parents); + self.node_mappings.extend(other.node_mappings.into_iter().map(|(node, entry)| { + (node, MappingEntry { parent: entry.parent + remap_base, ..entry }) + })); + } + fn add_mapping(&mut self, syntax_mapping: SyntaxMappingBuilder) { let SyntaxMappingBuilder { parent_node, node_mappings } = syntax_mapping; - let parent_entry: u32 = self.entry_parents.len() as u32; + let parent_entry: u32 = self.entry_parents.len().try_into().unwrap(); self.entry_parents.push(parent_node); - let node_entries = - node_mappings.into_iter().map(|(node, slot)| (node, (parent_entry, slot))); + let node_entries = node_mappings + .into_iter() + .map(|(node, slot)| (node, MappingEntry { parent: parent_entry, child_slot: slot })); self.node_mappings.extend(node_entries); } @@ -172,3 +182,9 @@ pub fn finish(self, editor: &mut SyntaxEditor) { editor.mappings.add_mapping(self); } } + +#[derive(Debug, Clone, Copy)] +struct MappingEntry { + parent: u32, + child_slot: u32, +} From e4bce9866e661bd875f135cc4ba027ca58c20f7a Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 2 Sep 2024 18:24:47 -0400 Subject: [PATCH 04/11] propagate annotations to mapped elements --- .../crates/syntax/src/syntax_editor.rs | 19 ++- .../syntax/src/syntax_editor/edit_algo.rs | 50 ++++-- .../syntax/src/syntax_editor/mapping.rs | 151 +++++++++++++----- 3 files changed, 168 insertions(+), 52 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs index 42373eba827..75347aeeed9 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs @@ -75,8 +75,8 @@ pub fn root(&self) -> &SyntaxNode { &self.root } - /// Which syntax elements in the modified syntax tree were modified as part - /// of the edit. + /// Which syntax elements in the modified syntax tree were inserted or + /// modified as part of the edit. /// /// Note that for syntax nodes, only the upper-most parent of a set of /// changes is included, not any child elements that may have been modified. @@ -343,11 +343,22 @@ fn basic_usage() { editor.replace(to_wrap.syntax(), new_block.syntax()); // editor.replace(to_replace.syntax(), name_ref.syntax()); - // dbg!(&editor.mappings); let edit = editor.finish(); - let expect = expect![]; + dbg!(&edit.annotations); + + let expect = expect![[r#" + _ => { + let var_name = 2 + 2; + (var_name, true) + }"#]]; expect.assert_eq(&edit.root.to_string()); + assert_eq!(edit.find_annotation(placeholder_snippet).len(), 2); + assert!(edit + .annotations + .iter() + .flat_map(|(_, elements)| elements) + .all(|element| element.ancestors().any(|it| &it == edit.root()))) } } diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs index f8354c5eb67..734a26b6003 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs @@ -1,9 +1,10 @@ use std::{collections::VecDeque, ops::RangeInclusive}; use rowan::TextRange; +use rustc_hash::{FxHashMap, FxHashSet}; use crate::{ - syntax_editor::{Change, ChangeKind}, + syntax_editor::{mapping::MissingMapping, Change, ChangeKind}, ted, SyntaxElement, SyntaxNode, SyntaxNodePtr, }; @@ -29,9 +30,6 @@ pub(super) fn apply_edits(editor: SyntaxEditor) -> SyntaxEdit { let SyntaxEditor { root, mut changes, mappings, annotations } = editor; - dbg!(("initial: ", &root)); - dbg!(&changes); - // Sort changes by range then change kind, so that we can: // - ensure that parent edits are ordered before child edits // - ensure that inserts will be guaranteed to be inserted at the right range @@ -102,15 +100,18 @@ struct DependentChange { } } - dbg!(("before: ", &changes, &dependent_changes, &independent_changes)); - // Map change targets to the correct syntax nodes let tree_mutator = TreeMutator::new(&root); + let mut changed_elements = vec![]; for index in independent_changes { match &mut changes[index as usize] { - Change::Replace(target, _) => { + Change::Replace(target, new_node) => { *target = tree_mutator.make_element_mut(target); + + if let Some(new_node) = new_node { + changed_elements.push(new_node.clone()); + } } } } @@ -124,15 +125,20 @@ struct DependentChange { Change::Replace(_, None) => continue, // silently drop outdated change }; + let upmap_target = |target: &SyntaxElement| { + match mappings.upmap_child_element(target, &input_ancestor, &output_ancestor) { + Ok(it) => it, + Err(MissingMapping(current)) => unreachable!("no mappings exist between {current:?} (ancestor of {input_ancestor:?}) and {output_ancestor:?}"), + } + }; + match &mut changes[child as usize] { Change::Replace(target, _) => { - *target = mappings.upmap_child_element(target, &input_ancestor, output_ancestor) + *target = upmap_target(&target); } } } - dbg!(("after: ", &changes)); - // Apply changes for change in changes { match change { @@ -141,9 +147,29 @@ struct DependentChange { } } - dbg!(("modified:", tree_mutator.mutable_clone)); + // Propagate annotations + let annotations = annotations.into_iter().filter_map(|(element, annotation)| { + match mappings.upmap_element(&element, &tree_mutator.mutable_clone) { + // Needed to follow the new tree to find the resulting element + Some(Ok(mapped)) => Some((mapped, annotation)), + // Element did not need to be mapped + None => Some((element, annotation)), + // Element did not make it to the final tree + Some(Err(_)) => None, + } + }); - todo!("draw the rest of the owl") + let mut annotation_groups = FxHashMap::default(); + + for (element, annotation) in annotations { + annotation_groups.entry(annotation).or_insert(vec![]).push(element); + } + + SyntaxEdit { + root: tree_mutator.mutable_clone, + changed_elements, + annotations: annotation_groups, + } } fn to_owning_node(element: &SyntaxElement) -> SyntaxNode { diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs index 8a79f7e186e..f14d0b347d7 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs @@ -22,19 +22,20 @@ pub fn new() -> Self { Self::default() } + /// Like [`SyntaxMapping::upmap_child`] but for syntax elements. pub fn upmap_child_element( &self, child: &SyntaxElement, input_ancestor: &SyntaxNode, - output_ancestor: SyntaxNode, - ) -> SyntaxElement { + output_ancestor: &SyntaxNode, + ) -> Result { match child { SyntaxElement::Node(node) => { - SyntaxElement::Node(self.upmap_child(node, input_ancestor, output_ancestor)) + self.upmap_child(node, input_ancestor, output_ancestor).map(SyntaxElement::Node) } SyntaxElement::Token(token) => { let upmap_parent = - self.upmap_child(&token.parent().unwrap(), input_ancestor, output_ancestor); + self.upmap_child(&token.parent().unwrap(), input_ancestor, output_ancestor)?; let element = upmap_parent.children_with_tokens().nth(token.index()).unwrap(); debug_assert!( @@ -42,21 +43,26 @@ pub fn upmap_child_element( "token upmapping mapped to the wrong node ({token:?} -> {element:?})" ); - element + Ok(element) } } } + /// Maps a child node of the input ancestor to the corresponding node in + /// the output ancestor. pub fn upmap_child( &self, child: &SyntaxNode, input_ancestor: &SyntaxNode, - output_ancestor: SyntaxNode, - ) -> SyntaxNode { - debug_assert!(child.ancestors().any(|ancestor| &ancestor == input_ancestor)); + output_ancestor: &SyntaxNode, + ) -> Result { + debug_assert!( + child == input_ancestor + || child.ancestors().any(|ancestor| &ancestor == input_ancestor) + ); // Build a list mapping up to the first mappable ancestor - let to_first_upmap = + let to_first_upmap = if child != input_ancestor { std::iter::successors(Some((child.index(), child.clone())), |(_, current)| { let parent = current.parent().unwrap(); @@ -67,24 +73,14 @@ pub fn upmap_child( Some((parent.index(), parent)) }) .map(|(i, _)| i) - .collect::>(); + .collect::>() + } else { + vec![] + }; // Progressively up-map the input ancestor until we get to the output ancestor - let to_output_ancestor = if input_ancestor != &output_ancestor { - std::iter::successors(Some((input_ancestor.index(), self.upmap_node(input_ancestor).unwrap_or_else(|| input_ancestor.clone()))), |(_, current)| { - let Some(parent) = current.parent() else { - unreachable!("no mappings exist between {current:?} (ancestor of {input_ancestor:?}) and {output_ancestor:?}") - }; - - if &parent == &output_ancestor { - return None; - } - - Some((parent.index(), match self.upmap_node(&parent) { - Some(next) => next, - None => parent - })) - }).map(|(i, _)| i).collect::>() + let to_output_ancestor = if input_ancestor != output_ancestor { + self.upmap_to_ancestor(input_ancestor, output_ancestor)? } else { vec![] }; @@ -92,7 +88,7 @@ pub fn upmap_child( let to_map_down = to_output_ancestor.into_iter().rev().chain(to_first_upmap.into_iter().rev()); - let mut target = output_ancestor; + let mut target = output_ancestor.clone(); for index in to_map_down { target = target @@ -104,20 +100,86 @@ pub fn upmap_child( debug_assert_eq!(child.kind(), target.kind()); - target + Ok(target) } - pub fn upmap_node(&self, input: &SyntaxNode) -> Option { - let MappingEntry { parent, child_slot } = self.node_mappings.get(input)?; + fn upmap_to_ancestor( + &self, + input_ancestor: &SyntaxNode, + output_ancestor: &SyntaxNode, + ) -> Result, MissingMapping> { + eprintln!("mapping ancestor {input_ancestor:#?} to {output_ancestor:#?}"); + let mut current = + self.upmap_node_single(input_ancestor).unwrap_or_else(|| input_ancestor.clone()); + let mut upmap_chain = vec![current.index()]; - let output = self.entry_parents[*parent as usize] - .children_with_tokens() - .nth(*child_slot as usize) - .and_then(SyntaxElement::into_node) - .unwrap(); + loop { + let Some(parent) = current.parent() else { break }; - debug_assert_eq!(input.kind(), output.kind()); - Some(output) + if &parent == output_ancestor { + return Ok(upmap_chain); + } + + current = match self.upmap_node_single(&parent) { + Some(next) => next, + None => parent, + }; + upmap_chain.push(current.index()); + } + + Err(MissingMapping(current)) + } + + pub fn upmap_element( + &self, + input: &SyntaxElement, + output_root: &SyntaxNode, + ) -> Option> { + match input { + SyntaxElement::Node(node) => { + Some(self.upmap_node(node, output_root)?.map(SyntaxElement::Node)) + } + SyntaxElement::Token(token) => { + let upmap_parent = match self.upmap_node(&token.parent().unwrap(), output_root)? { + Ok(it) => it, + Err(err) => return Some(Err(err)), + }; + + let element = upmap_parent.children_with_tokens().nth(token.index()).unwrap(); + debug_assert!( + element.as_token().is_some_and(|it| it.kind() == token.kind()), + "token upmapping mapped to the wrong node ({token:?} -> {element:?})" + ); + + Some(Ok(element)) + } + } + } + + pub fn upmap_node( + &self, + input: &SyntaxNode, + output_root: &SyntaxNode, + ) -> Option> { + // Try to follow the mapping tree, if it exists + let input_mapping = self.upmap_node_single(input); + let input_ancestor = + input.ancestors().find_map(|ancestor| self.upmap_node_single(&ancestor)); + + match (input_mapping, input_ancestor) { + (Some(input_mapping), _) => { + // A mapping exists at the input, follow along the tree + Some(self.upmap_child(&input_mapping, &input_mapping, &output_root)) + } + (None, Some(input_ancestor)) => { + // A mapping exists at an ancestor, follow along the tree + Some(self.upmap_child(input, &input_ancestor, &output_root)) + } + (None, None) => { + // No mapping exists at all, is the same position in the final tree + None + } + } } pub fn merge(&mut self, mut other: SyntaxMapping) { @@ -130,6 +192,20 @@ pub fn merge(&mut self, mut other: SyntaxMapping) { })); } + /// Follows the input one step along the syntax mapping tree + fn upmap_node_single(&self, input: &SyntaxNode) -> Option { + let MappingEntry { parent, child_slot } = self.node_mappings.get(input)?; + + let output = self.entry_parents[*parent as usize] + .children_with_tokens() + .nth(*child_slot as usize) + .and_then(SyntaxElement::into_node) + .unwrap(); + + debug_assert_eq!(input.kind(), output.kind()); + Some(output) + } + fn add_mapping(&mut self, syntax_mapping: SyntaxMappingBuilder) { let SyntaxMappingBuilder { parent_node, node_mappings } = syntax_mapping; @@ -183,6 +259,9 @@ pub fn finish(self, editor: &mut SyntaxEditor) { } } +#[derive(Debug)] +pub struct MissingMapping(pub SyntaxNode); + #[derive(Debug, Clone, Copy)] struct MappingEntry { parent: u32, From 8104457a1197353f1ed276117a4603076f26e42f Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 2 Sep 2024 19:11:39 -0400 Subject: [PATCH 05/11] support insert{_all} --- .../crates/syntax/src/syntax_editor.rs | 32 +++++++++ .../syntax/src/syntax_editor/edit_algo.rs | 69 +++++++++++++++---- 2 files changed, 89 insertions(+), 12 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs index 75347aeeed9..4b5134249c8 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs @@ -50,6 +50,14 @@ pub fn merge(&mut self, mut other: SyntaxEditor) { self.annotations.append(&mut other.annotations); } + pub fn insert(&mut self, position: Position, element: impl Element) { + self.changes.push(Change::Insert(position, element.syntax_element())) + } + + pub fn insert_all(&mut self, position: Position, elements: Vec) { + self.changes.push(Change::InsertAll(position, elements)) + } + pub fn delete(&mut self, element: impl Element) { self.changes.push(Change::Replace(element.syntax_element(), None)); } @@ -117,6 +125,19 @@ pub struct Position { repr: PositionRepr, } +impl Position { + pub(crate) fn parent(&self) -> SyntaxNode { + self.place().0 + } + + pub(crate) fn place(&self) -> (SyntaxNode, usize) { + match &self.repr { + PositionRepr::FirstChild(parent) => (parent.clone(), 0), + PositionRepr::After(child) => (child.parent().unwrap(), child.index() + 1), + } + } +} + #[derive(Debug)] enum PositionRepr { FirstChild(SyntaxNode), @@ -155,6 +176,8 @@ pub fn last_child_of(node: &(impl Into + Clone)) -> Position { #[derive(Debug)] enum Change { + Insert(Position, SyntaxElement), + InsertAll(Position, Vec), /// Represents both a replace single element and a delete element operation. Replace(SyntaxElement, Option), } @@ -162,18 +185,27 @@ enum Change { impl Change { fn target_range(&self) -> TextRange { match self { + Change::Insert(target, _) | Change::InsertAll(target, _) => match &target.repr { + PositionRepr::FirstChild(parent) => TextRange::at( + parent.first_child_or_token().unwrap().text_range().start(), + 0.into(), + ), + PositionRepr::After(child) => TextRange::at(child.text_range().end(), 0.into()), + }, Change::Replace(target, _) => target.text_range(), } } fn target_parent(&self) -> SyntaxNode { match self { + Change::Insert(target, _) | Change::InsertAll(target, _) => target.parent(), Change::Replace(target, _) => target.parent().unwrap(), } } fn change_kind(&self) -> ChangeKind { match self { + Change::Insert(_, _) | Change::InsertAll(_, _) => ChangeKind::Insert, Change::Replace(_, _) => ChangeKind::Replace, } } diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs index 734a26b6003..4f0b30ed6ad 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs @@ -4,8 +4,8 @@ use rustc_hash::{FxHashMap, FxHashSet}; use crate::{ - syntax_editor::{mapping::MissingMapping, Change, ChangeKind}, - ted, SyntaxElement, SyntaxNode, SyntaxNodePtr, + syntax_editor::{mapping::MissingMapping, Change, ChangeKind, Position, PositionRepr}, + SyntaxElement, SyntaxNode, SyntaxNodePtr, }; use super::{SyntaxEdit, SyntaxEditor}; @@ -94,6 +94,7 @@ struct DependentChange { // Add to changed ancestors, if applicable match change { + Change::Insert(_, _) | Change::InsertAll(_, _) => {} Change::Replace(target, _) => { changed_ancestors.push_back(ChangedAncestor::single(target, change_index)) } @@ -106,23 +107,46 @@ struct DependentChange { for index in independent_changes { match &mut changes[index as usize] { - Change::Replace(target, new_node) => { - *target = tree_mutator.make_element_mut(target); - - if let Some(new_node) = new_node { - changed_elements.push(new_node.clone()); - } + Change::Insert(target, _) | Change::InsertAll(target, _) => { + match &mut target.repr { + PositionRepr::FirstChild(parent) => { + *parent = tree_mutator.make_syntax_mut(parent); + } + PositionRepr::After(child) => { + *child = tree_mutator.make_element_mut(child); + } + }; } + Change::Replace(target, _) => { + *target = tree_mutator.make_element_mut(target); + } + } + + // Collect changed elements + match &changes[index as usize] { + Change::Insert(_, element) => changed_elements.push(element.clone()), + Change::InsertAll(_, elements) => changed_elements.extend(elements.iter().cloned()), + Change::Replace(_, Some(element)) => changed_elements.push(element.clone()), + Change::Replace(_, None) => {} } } for DependentChange { parent, child } in dependent_changes.into_iter() { let (input_ancestor, output_ancestor) = match &changes[parent as usize] { - // insert? unreachable + // No change will depend on an insert since changes can only depend on nodes in the root tree + Change::Insert(_, _) | Change::InsertAll(_, _) => unreachable!(), Change::Replace(target, Some(new_target)) => { (to_owning_node(target), to_owning_node(new_target)) } - Change::Replace(_, None) => continue, // silently drop outdated change + // Silently drop outdated change + Change::Replace(_, None) => continue, + }; + + let upmap_target_node = |target: &SyntaxNode| { + match mappings.upmap_child(target, &input_ancestor, &output_ancestor) { + Ok(it) => it, + Err(MissingMapping(current)) => unreachable!("no mappings exist between {current:?} (ancestor of {input_ancestor:?}) and {output_ancestor:?}"), + } }; let upmap_target = |target: &SyntaxElement| { @@ -133,6 +157,14 @@ struct DependentChange { }; match &mut changes[child as usize] { + Change::Insert(target, _) | Change::InsertAll(target, _) => match &mut target.repr { + PositionRepr::FirstChild(parent) => { + *parent = upmap_target_node(parent); + } + PositionRepr::After(child) => { + *child = upmap_target(child); + } + }, Change::Replace(target, _) => { *target = upmap_target(&target); } @@ -142,8 +174,21 @@ struct DependentChange { // Apply changes for change in changes { match change { - Change::Replace(target, None) => ted::remove(target), - Change::Replace(target, Some(new_target)) => ted::replace(target, new_target), + Change::Insert(position, element) => { + let (parent, index) = position.place(); + parent.splice_children(index..index, vec![element]); + } + Change::InsertAll(position, elements) => { + let (parent, index) = position.place(); + parent.splice_children(index..index, elements); + } + Change::Replace(target, None) => { + target.detach(); + } + Change::Replace(target, Some(new_target)) => { + let parent = target.parent().unwrap(); + parent.splice_children(target.index()..target.index() + 1, vec![new_target]); + } } } From f08299f9f8014ed9fb0f13f76afaebdb7cb9d2ab Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 2 Sep 2024 20:45:57 -0400 Subject: [PATCH 06/11] fix insert ranges not being excluded from disjointness --- .../syntax/src/syntax_editor/edit_algo.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs index 4f0b30ed6ad..4d6cb01c01c 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs @@ -1,10 +1,10 @@ use std::{collections::VecDeque, ops::RangeInclusive}; use rowan::TextRange; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashMap; use crate::{ - syntax_editor::{mapping::MissingMapping, Change, ChangeKind, Position, PositionRepr}, + syntax_editor::{mapping::MissingMapping, Change, ChangeKind, PositionRepr}, SyntaxElement, SyntaxNode, SyntaxNodePtr, }; @@ -41,12 +41,17 @@ pub(super) fn apply_edits(editor: SyntaxEditor) -> SyntaxEdit { .then(a.change_kind().cmp(&b.change_kind())) }); - let disjoint_replaces_ranges = changes.iter().zip(changes.iter().skip(1)).all(|(l, r)| { - l.change_kind() == ChangeKind::Replace - && r.change_kind() == ChangeKind::Replace - && (l.target_parent() != r.target_parent() + let disjoint_replaces_ranges = changes + .iter() + .zip(changes.iter().skip(1)) + .filter(|(l, r)| { + // We only care about checking for disjoint replace ranges + l.change_kind() == ChangeKind::Replace && r.change_kind() == ChangeKind::Replace + }) + .all(|(l, r)| { + (l.target_parent() != r.target_parent() || l.target_range().intersect(r.target_range()).is_none()) - }); + }); if stdx::never!( !disjoint_replaces_ranges, From f74ef3aa526f1b293f37b6574d5d24f5532d810f Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 2 Sep 2024 21:34:00 -0400 Subject: [PATCH 07/11] properly sort changes by depth to sort between nodes that have the same start range --- .../syntax/src/syntax_editor/edit_algo.rs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs index 4d6cb01c01c..2adc1f67d14 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs @@ -1,4 +1,4 @@ -use std::{collections::VecDeque, ops::RangeInclusive}; +use std::{cmp::Ordering, collections::VecDeque, ops::RangeInclusive}; use rowan::TextRange; use rustc_hash::FxHashMap; @@ -30,7 +30,12 @@ pub(super) fn apply_edits(editor: SyntaxEditor) -> SyntaxEdit { let SyntaxEditor { root, mut changes, mappings, annotations } = editor; - // Sort changes by range then change kind, so that we can: + let mut node_depths = FxHashMap::::default(); + let mut get_node_depth = |node: SyntaxNode| { + *node_depths.entry(node).or_insert_with_key(|node| node.ancestors().count()) + }; + + // Sort changes by range, then depth, then change kind, so that we can: // - ensure that parent edits are ordered before child edits // - ensure that inserts will be guaranteed to be inserted at the right range // - easily check for disjoint replace ranges @@ -38,6 +43,16 @@ pub(super) fn apply_edits(editor: SyntaxEditor) -> SyntaxEdit { a.target_range() .start() .cmp(&b.target_range().start()) + .then_with(|| { + let a_target = a.target_parent(); + let b_target = b.target_parent(); + + if a_target == b_target { + return Ordering::Equal; + } + + get_node_depth(a_target).cmp(&get_node_depth(b_target)) + }) .then(a.change_kind().cmp(&b.change_kind())) }); @@ -49,8 +64,8 @@ pub(super) fn apply_edits(editor: SyntaxEditor) -> SyntaxEdit { l.change_kind() == ChangeKind::Replace && r.change_kind() == ChangeKind::Replace }) .all(|(l, r)| { - (l.target_parent() != r.target_parent() - || l.target_range().intersect(r.target_range()).is_none()) + get_node_depth(l.target_parent()) != get_node_depth(r.target_parent()) + || l.target_range().intersect(r.target_range()).is_none() }); if stdx::never!( From f03f95f36918c5d64ead3fdafc920915ac204e04 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 2 Sep 2024 21:42:08 -0400 Subject: [PATCH 08/11] support replacing root node --- .../crates/syntax/src/syntax_editor.rs | 197 +++++++++++++++++- .../syntax/src/syntax_editor/edit_algo.rs | 16 +- .../syntax/src/syntax_editor/mapping.rs | 1 - 3 files changed, 200 insertions(+), 14 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs index 4b5134249c8..77a560bfe5c 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs @@ -51,18 +51,28 @@ pub fn merge(&mut self, mut other: SyntaxEditor) { } pub fn insert(&mut self, position: Position, element: impl Element) { + debug_assert!(is_ancestor_or_self(&position.parent(), &self.root)); self.changes.push(Change::Insert(position, element.syntax_element())) } pub fn insert_all(&mut self, position: Position, elements: Vec) { + debug_assert!(is_ancestor_or_self(&position.parent(), &self.root)); self.changes.push(Change::InsertAll(position, elements)) } pub fn delete(&mut self, element: impl Element) { + let element = element.syntax_element(); + debug_assert!(is_ancestor_or_self_of_element(&element, &self.root)); + debug_assert!( + !matches!(&element, SyntaxElement::Node(node) if node == &self.root), + "should not delete root node" + ); self.changes.push(Change::Replace(element.syntax_element(), None)); } pub fn replace(&mut self, old: impl Element, new: impl Element) { + let old = old.syntax_element(); + debug_assert!(is_ancestor_or_self_of_element(&old, &self.root)); self.changes.push(Change::Replace(old.syntax_element(), Some(new.syntax_element()))); } @@ -199,7 +209,10 @@ fn target_range(&self) -> TextRange { fn target_parent(&self) -> SyntaxNode { match self { Change::Insert(target, _) | Change::InsertAll(target, _) => target.parent(), - Change::Replace(target, _) => target.parent().unwrap(), + Change::Replace(SyntaxElement::Node(target), _) => { + target.parent().unwrap_or_else(|| target.clone()) + } + Change::Replace(SyntaxElement::Token(target), _) => target.parent().unwrap(), } } @@ -248,6 +261,15 @@ fn syntax_element(self) -> SyntaxElement { } } +fn is_ancestor_or_self(node: &SyntaxNode, ancestor: &SyntaxNode) -> bool { + node == ancestor || node.ancestors().any(|it| &it == ancestor) +} + +fn is_ancestor_or_self_of_element(node: &SyntaxElement, ancestor: &SyntaxNode) -> bool { + matches!(node, SyntaxElement::Node(node) if node == ancestor) + || node.ancestors().any(|it| &it == ancestor) +} + #[cfg(test)] mod tests { use expect_test::expect; @@ -370,15 +392,11 @@ fn basic_usage() { Some(to_wrap.clone().into()), ); - // should die: editor.replace(to_replace.syntax(), name_ref.syntax()); editor.replace(to_wrap.syntax(), new_block.syntax()); - // editor.replace(to_replace.syntax(), name_ref.syntax()); let edit = editor.finish(); - dbg!(&edit.annotations); - let expect = expect![[r#" _ => { let var_name = 2 + 2; @@ -393,4 +411,173 @@ fn basic_usage() { .flat_map(|(_, elements)| elements) .all(|element| element.ancestors().any(|it| &it == edit.root()))) } + + #[test] + #[should_panic = "some replace change ranges intersect: [Replace(Node(TUPLE_EXPR@5..7), Some(Node(NAME_REF@0..8))), Replace(Node(TUPLE_EXPR@5..7), Some(Node(NAME_REF@0..8)))]"] + fn fail_on_non_disjoint_single_replace() { + let root = make::match_arm([make::wildcard_pat().into()], None, make::expr_tuple([])); + + let to_wrap = root.syntax().descendants().find_map(ast::TupleExpr::cast).unwrap(); + + let mut editor = SyntaxEditor::new(root.syntax().clone()); + + let name_ref = make::name_ref("var_name").clone_for_update(); + + // should die, ranges are not disjoint + editor.replace(to_wrap.syntax(), name_ref.syntax()); + editor.replace(to_wrap.syntax(), name_ref.syntax()); + + let _ = editor.finish(); + } + + #[test] + fn test_insert_independent() { + let root = make::block_expr( + [make::let_stmt( + make::ext::simple_ident_pat(make::name("second")).into(), + None, + Some(make::expr_literal("2").into()), + ) + .into()], + None, + ); + + let second_let = root.syntax().descendants().find_map(ast::LetStmt::cast).unwrap(); + + let mut editor = SyntaxEditor::new(root.syntax().clone()); + + editor.insert( + Position::first_child_of(root.stmt_list().unwrap().syntax()), + make_let_stmt( + None, + make::ext::simple_ident_pat(make::name("first")).into(), + None, + Some(make::expr_literal("1").into()), + ) + .syntax(), + ); + + editor.insert( + Position::after(second_let.syntax()), + make_let_stmt( + None, + make::ext::simple_ident_pat(make::name("third")).into(), + None, + Some(make::expr_literal("3").into()), + ) + .syntax(), + ); + + let edit = editor.finish(); + + let expect = expect![[r#" + let first = 1;{ + let second = 2;let third = 3; + }"#]]; + expect.assert_eq(&edit.root.to_string()); + } + + #[test] + fn test_insert_dependent() { + let root = make::block_expr( + [], + Some( + make::block_expr( + [make::let_stmt( + make::ext::simple_ident_pat(make::name("second")).into(), + None, + Some(make::expr_literal("2").into()), + ) + .into()], + None, + ) + .into(), + ), + ); + + let inner_block = + root.syntax().descendants().flat_map(ast::BlockExpr::cast).nth(1).unwrap(); + let second_let = root.syntax().descendants().find_map(ast::LetStmt::cast).unwrap(); + + let mut editor = SyntaxEditor::new(root.syntax().clone()); + + let new_block_expr = + make_block_expr(Some(&mut editor), [], Some(ast::Expr::BlockExpr(inner_block.clone()))); + + let first_let = make_let_stmt( + Some(&mut editor), + make::ext::simple_ident_pat(make::name("first")).into(), + None, + Some(make::expr_literal("1").into()), + ); + + let third_let = make_let_stmt( + Some(&mut editor), + make::ext::simple_ident_pat(make::name("third")).into(), + None, + Some(make::expr_literal("3").into()), + ); + + editor.insert( + Position::first_child_of(inner_block.stmt_list().unwrap().syntax()), + first_let.syntax(), + ); + editor.insert(Position::after(second_let.syntax()), third_let.syntax()); + editor.replace(inner_block.syntax(), new_block_expr.syntax()); + + let edit = editor.finish(); + + let expect = expect![[r#" + { + { + let first = 1;{ + let second = 2;let third = 3; + } + } + }"#]]; + expect.assert_eq(&edit.root.to_string()); + } + + #[test] + fn test_replace_root_with_dependent() { + let root = make::block_expr( + [make::let_stmt( + make::ext::simple_ident_pat(make::name("second")).into(), + None, + Some(make::expr_literal("2").into()), + ) + .into()], + None, + ); + + let inner_block = root.clone(); + + let mut editor = SyntaxEditor::new(root.syntax().clone()); + + let new_block_expr = + make_block_expr(Some(&mut editor), [], Some(ast::Expr::BlockExpr(inner_block.clone()))); + + let first_let = make_let_stmt( + Some(&mut editor), + make::ext::simple_ident_pat(make::name("first")).into(), + None, + Some(make::expr_literal("1").into()), + ); + + editor.insert( + Position::first_child_of(inner_block.stmt_list().unwrap().syntax()), + first_let.syntax(), + ); + editor.replace(inner_block.syntax(), new_block_expr.syntax()); + + let edit = editor.finish(); + + let expect = expect![[r#" + { + let first = 1;{ + let second = 2; + } + }"#]]; + expect.assert_eq(&edit.root.to_string()); + } } diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs index 2adc1f67d14..2c331fc1f69 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs @@ -192,6 +192,8 @@ struct DependentChange { } // Apply changes + let mut root = tree_mutator.mutable_clone; + for change in changes { match change { Change::Insert(position, element) => { @@ -205,6 +207,9 @@ struct DependentChange { Change::Replace(target, None) => { target.detach(); } + Change::Replace(SyntaxElement::Node(target), Some(new_target)) if &target == &root => { + root = new_target.into_node().expect("root node replacement should be a node"); + } Change::Replace(target, Some(new_target)) => { let parent = target.parent().unwrap(); parent.splice_children(target.index()..target.index() + 1, vec![new_target]); @@ -214,7 +219,7 @@ struct DependentChange { // Propagate annotations let annotations = annotations.into_iter().filter_map(|(element, annotation)| { - match mappings.upmap_element(&element, &tree_mutator.mutable_clone) { + match mappings.upmap_element(&element, &root) { // Needed to follow the new tree to find the resulting element Some(Ok(mapped)) => Some((mapped, annotation)), // Element did not need to be mapped @@ -230,11 +235,7 @@ struct DependentChange { annotation_groups.entry(annotation).or_insert(vec![]).push(element); } - SyntaxEdit { - root: tree_mutator.mutable_clone, - changed_elements, - annotations: annotation_groups, - } + SyntaxEdit { root, changed_elements, annotations: annotation_groups } } fn to_owning_node(element: &SyntaxElement) -> SyntaxNode { @@ -278,7 +279,6 @@ fn affected_range(&self) -> TextRange { } struct TreeMutator { - immutable: SyntaxNode, mutable_clone: SyntaxNode, } @@ -286,7 +286,7 @@ impl TreeMutator { fn new(immutable: &SyntaxNode) -> TreeMutator { let immutable = immutable.clone(); let mutable_clone = immutable.clone_for_update(); - TreeMutator { immutable, mutable_clone } + TreeMutator { mutable_clone } } fn make_element_mut(&self, element: &SyntaxElement) -> SyntaxElement { diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs index f14d0b347d7..b2c677c8696 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs @@ -108,7 +108,6 @@ fn upmap_to_ancestor( input_ancestor: &SyntaxNode, output_ancestor: &SyntaxNode, ) -> Result, MissingMapping> { - eprintln!("mapping ancestor {input_ancestor:#?} to {output_ancestor:#?}"); let mut current = self.upmap_node_single(input_ancestor).unwrap_or_else(|| input_ancestor.clone()); let mut upmap_chain = vec![current.index()]; From f6e05a744dcf47aaa229cac246228268394263d8 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 2 Sep 2024 22:27:14 -0400 Subject: [PATCH 09/11] handle replace_with_many and replace_all --- .../crates/syntax/src/syntax_editor.rs | 47 +++++++++++-- .../syntax/src/syntax_editor/edit_algo.rs | 66 ++++++++++++++++--- 2 files changed, 97 insertions(+), 16 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs index 77a560bfe5c..3a05cc480b2 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs @@ -6,6 +6,7 @@ use std::{ num::NonZeroU32, + ops::RangeInclusive, sync::atomic::{AtomicU32, Ordering}, }; @@ -76,6 +77,26 @@ pub fn replace(&mut self, old: impl Element, new: impl Element) { self.changes.push(Change::Replace(old.syntax_element(), Some(new.syntax_element()))); } + pub fn replace_with_many(&mut self, old: impl Element, new: Vec) { + let old = old.syntax_element(); + debug_assert!(is_ancestor_or_self_of_element(&old, &self.root)); + debug_assert!( + !(matches!(&old, SyntaxElement::Node(node) if node == &self.root) && new.len() > 1), + "cannot replace root node with many elements" + ); + self.changes.push(Change::ReplaceWithMany(old.syntax_element(), new)); + } + + pub fn replace_all(&mut self, range: RangeInclusive, new: Vec) { + if range.start() == range.end() { + self.replace_with_many(range.start(), new); + return; + } + + debug_assert!(is_ancestor_or_self_of_element(range.start(), &self.root)); + self.changes.push(Change::ReplaceAll(range, new)) + } + pub fn finish(self) -> SyntaxEdit { edit_algo::apply_edits(self) } @@ -186,10 +207,17 @@ pub fn last_child_of(node: &(impl Into + Clone)) -> Position { #[derive(Debug)] enum Change { + /// Inserts a single element at the specified position. Insert(Position, SyntaxElement), + /// Inserts many elements in-order at the specified position. InsertAll(Position, Vec), /// Represents both a replace single element and a delete element operation. Replace(SyntaxElement, Option), + /// Replaces a single element with many elements. + ReplaceWithMany(SyntaxElement, Vec), + /// Replaces a range of elements with another list of elements. + /// Range will always have start != end. + ReplaceAll(RangeInclusive, Vec), } impl Change { @@ -202,24 +230,29 @@ fn target_range(&self) -> TextRange { ), PositionRepr::After(child) => TextRange::at(child.text_range().end(), 0.into()), }, - Change::Replace(target, _) => target.text_range(), + Change::Replace(target, _) | Change::ReplaceWithMany(target, _) => target.text_range(), + Change::ReplaceAll(range, _) => { + range.start().text_range().cover(range.end().text_range()) + } } } fn target_parent(&self) -> SyntaxNode { match self { Change::Insert(target, _) | Change::InsertAll(target, _) => target.parent(), - Change::Replace(SyntaxElement::Node(target), _) => { - target.parent().unwrap_or_else(|| target.clone()) - } - Change::Replace(SyntaxElement::Token(target), _) => target.parent().unwrap(), + Change::Replace(target, _) | Change::ReplaceWithMany(target, _) => match target { + SyntaxElement::Node(target) => target.parent().unwrap_or_else(|| target.clone()), + SyntaxElement::Token(target) => target.parent().unwrap(), + }, + Change::ReplaceAll(target, _) => target.start().parent().unwrap(), } } fn change_kind(&self) -> ChangeKind { match self { Change::Insert(_, _) | Change::InsertAll(_, _) => ChangeKind::Insert, - Change::Replace(_, _) => ChangeKind::Replace, + Change::Replace(_, _) | Change::ReplaceWithMany(_, _) => ChangeKind::Replace, + Change::ReplaceAll(_, _) => ChangeKind::ReplaceRange, } } } @@ -227,7 +260,7 @@ fn change_kind(&self) -> ChangeKind { #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] enum ChangeKind { Insert, - // TODO: deal with replace spans + ReplaceRange, Replace, } diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs index 2c331fc1f69..3b92ac1cbd8 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs @@ -61,7 +61,13 @@ pub(super) fn apply_edits(editor: SyntaxEditor) -> SyntaxEdit { .zip(changes.iter().skip(1)) .filter(|(l, r)| { // We only care about checking for disjoint replace ranges - l.change_kind() == ChangeKind::Replace && r.change_kind() == ChangeKind::Replace + matches!( + (l.change_kind(), r.change_kind()), + ( + ChangeKind::Replace | ChangeKind::ReplaceRange, + ChangeKind::Replace | ChangeKind::ReplaceRange + ) + ) }) .all(|(l, r)| { get_node_depth(l.target_parent()) != get_node_depth(r.target_parent()) @@ -97,6 +103,7 @@ struct DependentChange { // Pop off any ancestors that aren't applicable changed_ancestors.drain((index + 1)..); + // FIXME: Resolve changes that depend on a range of elements let ancestor = &changed_ancestors[index]; dependent_changes.push(DependentChange { @@ -115,9 +122,12 @@ struct DependentChange { // Add to changed ancestors, if applicable match change { Change::Insert(_, _) | Change::InsertAll(_, _) => {} - Change::Replace(target, _) => { + Change::Replace(target, _) | Change::ReplaceWithMany(target, _) => { changed_ancestors.push_back(ChangedAncestor::single(target, change_index)) } + Change::ReplaceAll(range, _) => { + changed_ancestors.push_back(ChangedAncestor::multiple(range, change_index)) + } } } @@ -137,9 +147,15 @@ struct DependentChange { } }; } - Change::Replace(target, _) => { + Change::Replace(target, _) | Change::ReplaceWithMany(target, _) => { *target = tree_mutator.make_element_mut(target); } + Change::ReplaceAll(range, _) => { + let start = tree_mutator.make_element_mut(range.start()); + let end = tree_mutator.make_element_mut(range.end()); + + *range = start..=end; + } } // Collect changed elements @@ -148,6 +164,10 @@ struct DependentChange { Change::InsertAll(_, elements) => changed_elements.extend(elements.iter().cloned()), Change::Replace(_, Some(element)) => changed_elements.push(element.clone()), Change::Replace(_, None) => {} + Change::ReplaceWithMany(_, elements) => { + changed_elements.extend(elements.iter().cloned()) + } + Change::ReplaceAll(_, elements) => changed_elements.extend(elements.iter().cloned()), } } @@ -160,6 +180,9 @@ struct DependentChange { } // Silently drop outdated change Change::Replace(_, None) => continue, + Change::ReplaceAll(_, _) | Change::ReplaceWithMany(_, _) => { + unimplemented!("cannot resolve changes that depend on replacing many elements") + } }; let upmap_target_node = |target: &SyntaxNode| { @@ -185,9 +208,12 @@ struct DependentChange { *child = upmap_target(child); } }, - Change::Replace(target, _) => { + Change::Replace(target, _) | Change::ReplaceWithMany(target, _) => { *target = upmap_target(&target); } + Change::ReplaceAll(range, _) => { + *range = upmap_target(range.start())..=upmap_target(range.end()); + } } } @@ -214,6 +240,16 @@ struct DependentChange { let parent = target.parent().unwrap(); parent.splice_children(target.index()..target.index() + 1, vec![new_target]); } + Change::ReplaceWithMany(target, elements) => { + let parent = target.parent().unwrap(); + parent.splice_children(target.index()..target.index() + 1, elements); + } + Change::ReplaceAll(range, elements) => { + let start = range.start().index(); + let end = range.end().index(); + let parent = range.start().parent().unwrap(); + parent.splice_children(start..end + 1, elements); + } } } @@ -252,7 +288,7 @@ struct ChangedAncestor { enum ChangedAncestorKind { Single { node: SyntaxNode }, - Range { changed_nodes: RangeInclusive, in_parent: SyntaxNode }, + Range { _changed_elements: RangeInclusive, in_parent: SyntaxNode }, } impl ChangedAncestor { @@ -267,13 +303,25 @@ fn single(element: &SyntaxElement, change_index: usize) -> Self { Self { kind, change_index } } + fn multiple(range: &RangeInclusive, change_index: usize) -> Self { + Self { + kind: ChangedAncestorKind::Range { + _changed_elements: range.clone(), + in_parent: range.start().parent().unwrap(), + }, + change_index, + } + } + fn affected_range(&self) -> TextRange { match &self.kind { ChangedAncestorKind::Single { node } => node.text_range(), - ChangedAncestorKind::Range { changed_nodes, in_parent: _ } => TextRange::new( - changed_nodes.start().text_range().start(), - changed_nodes.end().text_range().end(), - ), + ChangedAncestorKind::Range { _changed_elements: changed_nodes, in_parent: _ } => { + TextRange::new( + changed_nodes.start().text_range().start(), + changed_nodes.end().text_range().end(), + ) + } } } } From 4e81ca344b6ce53b11309d14bc4ea72e39db5b83 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 2 Sep 2024 22:53:54 -0400 Subject: [PATCH 10/11] misc fixes --- .../crates/syntax/src/syntax_editor.rs | 24 +++++-------------- .../syntax/src/syntax_editor/edit_algo.rs | 14 ++++++----- .../syntax/src/syntax_editor/mapping.rs | 8 +++++-- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs index 3a05cc480b2..139c6518bf7 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs @@ -150,6 +150,12 @@ pub fn new() -> Self { } } +impl Default for SyntaxAnnotation { + fn default() -> Self { + Self::new() + } +} + /// Position describing where to insert elements #[derive(Debug)] pub struct Position { @@ -445,24 +451,6 @@ fn basic_usage() { .all(|element| element.ancestors().any(|it| &it == edit.root()))) } - #[test] - #[should_panic = "some replace change ranges intersect: [Replace(Node(TUPLE_EXPR@5..7), Some(Node(NAME_REF@0..8))), Replace(Node(TUPLE_EXPR@5..7), Some(Node(NAME_REF@0..8)))]"] - fn fail_on_non_disjoint_single_replace() { - let root = make::match_arm([make::wildcard_pat().into()], None, make::expr_tuple([])); - - let to_wrap = root.syntax().descendants().find_map(ast::TupleExpr::cast).unwrap(); - - let mut editor = SyntaxEditor::new(root.syntax().clone()); - - let name_ref = make::name_ref("var_name").clone_for_update(); - - // should die, ranges are not disjoint - editor.replace(to_wrap.syntax(), name_ref.syntax()); - editor.replace(to_wrap.syntax(), name_ref.syntax()); - - let _ = editor.finish(); - } - #[test] fn test_insert_independent() { let root = make::block_expr( diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs index 3b92ac1cbd8..55e8867a46c 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs @@ -1,3 +1,5 @@ +//! Implementation of applying changes to a syntax tree. + use std::{cmp::Ordering, collections::VecDeque, ops::RangeInclusive}; use rowan::TextRange; @@ -209,7 +211,7 @@ struct DependentChange { } }, Change::Replace(target, _) | Change::ReplaceWithMany(target, _) => { - *target = upmap_target(&target); + *target = upmap_target(target); } Change::ReplaceAll(range, _) => { *range = upmap_target(range.start())..=upmap_target(range.end()); @@ -233,7 +235,7 @@ struct DependentChange { Change::Replace(target, None) => { target.detach(); } - Change::Replace(SyntaxElement::Node(target), Some(new_target)) if &target == &root => { + Change::Replace(SyntaxElement::Node(target), Some(new_target)) if target == root => { root = new_target.into_node().expect("root node replacement should be a node"); } Change::Replace(target, Some(new_target)) => { @@ -288,7 +290,7 @@ struct ChangedAncestor { enum ChangedAncestorKind { Single { node: SyntaxNode }, - Range { _changed_elements: RangeInclusive, in_parent: SyntaxNode }, + Range { _changed_elements: RangeInclusive, _in_parent: SyntaxNode }, } impl ChangedAncestor { @@ -307,7 +309,7 @@ fn multiple(range: &RangeInclusive, change_index: usize) -> Self Self { kind: ChangedAncestorKind::Range { _changed_elements: range.clone(), - in_parent: range.start().parent().unwrap(), + _in_parent: range.start().parent().unwrap(), }, change_index, } @@ -316,7 +318,7 @@ fn multiple(range: &RangeInclusive, change_index: usize) -> Self fn affected_range(&self) -> TextRange { match &self.kind { ChangedAncestorKind::Single { node } => node.text_range(), - ChangedAncestorKind::Range { _changed_elements: changed_nodes, in_parent: _ } => { + ChangedAncestorKind::Range { _changed_elements: changed_nodes, _in_parent: _ } => { TextRange::new( changed_nodes.start().text_range().start(), changed_nodes.end().text_range().end(), @@ -339,7 +341,7 @@ fn new(immutable: &SyntaxNode) -> TreeMutator { fn make_element_mut(&self, element: &SyntaxElement) -> SyntaxElement { match element { - SyntaxElement::Node(node) => SyntaxElement::Node(self.make_syntax_mut(&node)), + SyntaxElement::Node(node) => SyntaxElement::Node(self.make_syntax_mut(node)), SyntaxElement::Token(token) => { let parent = self.make_syntax_mut(&token.parent().unwrap()); parent.children_with_tokens().nth(token.index()).unwrap() diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs index b2c677c8696..9bb5e6d9338 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/mapping.rs @@ -1,3 +1,7 @@ +//! Maps syntax elements through disjoint syntax nodes. +//! +//! [`SyntaxMappingBuilder`] should be used to create mappings to add to a [`SyntaxEditor`] + use itertools::Itertools; use rustc_hash::FxHashMap; @@ -168,11 +172,11 @@ pub fn upmap_node( match (input_mapping, input_ancestor) { (Some(input_mapping), _) => { // A mapping exists at the input, follow along the tree - Some(self.upmap_child(&input_mapping, &input_mapping, &output_root)) + Some(self.upmap_child(&input_mapping, &input_mapping, output_root)) } (None, Some(input_ancestor)) => { // A mapping exists at an ancestor, follow along the tree - Some(self.upmap_child(input, &input_ancestor, &output_root)) + Some(self.upmap_child(input, &input_ancestor, output_root)) } (None, None) => { // No mapping exists at all, is the same position in the final tree From a07e54c6ea28ced847f954b5e05e948839f991f2 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Tue, 3 Sep 2024 11:20:23 -0400 Subject: [PATCH 11/11] bundle old root into `SyntaxEdit` result useful for `SourceChangeBuilder` so it can still perform a tree diff without having to store the old root separately --- .../crates/syntax/src/syntax_editor.rs | 25 ++++++++++++------- .../syntax/src/syntax_editor/edit_algo.rs | 17 ++++++++++--- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs index 139c6518bf7..eb114f5e5f1 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor.rs @@ -102,16 +102,23 @@ pub fn finish(self) -> SyntaxEdit { } } +/// Represents a completed [`SyntaxEditor`] operation. pub struct SyntaxEdit { - root: SyntaxNode, + old_root: SyntaxNode, + new_root: SyntaxNode, changed_elements: Vec, annotations: FxHashMap>, } impl SyntaxEdit { - /// Root of the modified syntax tree - pub fn root(&self) -> &SyntaxNode { - &self.root + /// Root of the initial unmodified syntax tree. + pub fn old_root(&self) -> &SyntaxNode { + &self.old_root + } + + /// Root of the modified syntax tree. + pub fn new_root(&self) -> &SyntaxNode { + &self.new_root } /// Which syntax elements in the modified syntax tree were inserted or @@ -441,14 +448,14 @@ fn basic_usage() { let var_name = 2 + 2; (var_name, true) }"#]]; - expect.assert_eq(&edit.root.to_string()); + expect.assert_eq(&edit.new_root.to_string()); assert_eq!(edit.find_annotation(placeholder_snippet).len(), 2); assert!(edit .annotations .iter() .flat_map(|(_, elements)| elements) - .all(|element| element.ancestors().any(|it| &it == edit.root()))) + .all(|element| element.ancestors().any(|it| &it == edit.new_root()))) } #[test] @@ -495,7 +502,7 @@ fn test_insert_independent() { let first = 1;{ let second = 2;let third = 3; }"#]]; - expect.assert_eq(&edit.root.to_string()); + expect.assert_eq(&edit.new_root.to_string()); } #[test] @@ -556,7 +563,7 @@ fn test_insert_dependent() { } } }"#]]; - expect.assert_eq(&edit.root.to_string()); + expect.assert_eq(&edit.new_root.to_string()); } #[test] @@ -599,6 +606,6 @@ fn test_replace_root_with_dependent() { let second = 2; } }"#]]; - expect.assert_eq(&edit.root.to_string()); + expect.assert_eq(&edit.new_root.to_string()); } } diff --git a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs index 55e8867a46c..b769c941105 100644 --- a/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs +++ b/src/tools/rust-analyzer/crates/syntax/src/syntax_editor/edit_algo.rs @@ -81,7 +81,12 @@ pub(super) fn apply_edits(editor: SyntaxEditor) -> SyntaxEdit { "some replace change ranges intersect: {:?}", changes ) { - return SyntaxEdit { root, annotations: Default::default(), changed_elements: vec![] }; + return SyntaxEdit { + old_root: root.clone(), + new_root: root, + annotations: Default::default(), + changed_elements: vec![], + }; } #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] @@ -273,7 +278,12 @@ struct DependentChange { annotation_groups.entry(annotation).or_insert(vec![]).push(element); } - SyntaxEdit { root, changed_elements, annotations: annotation_groups } + SyntaxEdit { + old_root: tree_mutator.immutable, + new_root: root, + changed_elements, + annotations: annotation_groups, + } } fn to_owning_node(element: &SyntaxElement) -> SyntaxNode { @@ -329,6 +339,7 @@ fn affected_range(&self) -> TextRange { } struct TreeMutator { + immutable: SyntaxNode, mutable_clone: SyntaxNode, } @@ -336,7 +347,7 @@ impl TreeMutator { fn new(immutable: &SyntaxNode) -> TreeMutator { let immutable = immutable.clone(); let mutable_clone = immutable.clone_for_update(); - TreeMutator { mutable_clone } + TreeMutator { immutable, mutable_clone } } fn make_element_mut(&self, element: &SyntaxElement) -> SyntaxElement {