internal: remove one more immutable tree

Author: matklad

Cargo.lock

@@ -2,8 +2,9 @@
 use std::cmp::Ordering;
 
 use itertools::{EitherOrBoth, Itertools};
-use syntax::ast::{
-    self, edit::AstNodeEdit, make, AstNode, AttrsOwner, PathSegmentKind, VisibilityOwner,
+use syntax::{
+    ast::{self, make, AstNode, AttrsOwner, PathSegmentKind, VisibilityOwner},
+    ted,
 };
 
 /// What type of merges are allowed.
@@ -65,7 +66,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.
@@ -78,7 +79,8 @@ fn recursive_merge(
         .use_tree_list()
         .into_iter()
         .flat_map(|list| list.use_trees())
-        // we use Option here to early return from this function(this is not the same as a `filter` op)
+        // We use Option here to early return from this function(this is not the
+        // same as a `filter` op).
         .map(|tree| match merge.is_tree_allowed(&tree) {
             true => Some(tree),
             false => None,
@@ -111,8 +113,10 @@ fn recursive_merge(
                     let tree_is_self = |tree: ast::UseTree| {
                         tree.path().as_ref().map(path_is_self).unwrap_or(false)
                     };
-                    // check if only one of the two trees has a tree list, and whether that then contains `self` or not.
-                    // If this is the case we can skip this iteration since the path without the list is already included in the other one via `self`
+                    // Check if only one of the two trees has a tree list, and
+                    // whether that then contains `self` or not. If this is the
+                    // case we can skip this iteration since the path without
+                    // the list is already included in the other one via `self`.
                     let tree_contains_self = |tree: &ast::UseTree| {
                         tree.use_tree_list()
                             .map(|tree_list| tree_list.use_trees().any(tree_is_self))
@@ -127,9 +131,11 @@ fn recursive_merge(
                         _ => (),
                     }
 
-                    // glob imports arent part of the use-tree lists so we need to special handle them here as well
-                    // this special handling is only required for when we merge a module import into a glob import of said module
-                    // see the `merge_self_glob` or `merge_mod_into_glob` tests
+                    // Glob imports aren't part of the use-tree lists so we need
+                    // to special handle them here as well this special handling
+                    // is only required for when we merge a module import into a
+                    // glob import of said module see the `merge_self_glob` or
+                    // `merge_mod_into_glob` tests.
                     if lhs_t.star_token().is_some() || rhs_t.star_token().is_some() {
                         *lhs_t = make::use_tree(
                             make::path_unqualified(make::path_segment_self()),
@@ -165,11 +171,11 @@ fn recursive_merge(
         }
     }
 
-    Some(if let Some(old) = lhs.use_tree_list() {
-        lhs.replace_descendant(old, make::use_tree_list(use_trees)).clone_for_update()
-    } else {
-        lhs.clone()
-    })
+    let lhs = lhs.clone_subtree().clone_for_update();
+    if let Some(old) = lhs.use_tree_list() {
+        ted::replace(old.syntax(), make::use_tree_list(use_trees).syntax().clone_for_update());
+    }
+    ast::UseTree::cast(lhs.syntax().clone_subtree())
 }
 
 /// Traverses both paths until they differ, returning the common prefix of both.

crates/ide_db/src/helpers/merge_imports.rs

@@ -13,7 +13,7 @@ doctest = false
 [dependencies]
 cov-mark = { version = "1.1", features = ["thread-local"] }
 itertools = "0.10.0"
-rowan = "=0.13.0-pre.5"
+rowan = "=0.13.0-pre.6"
 rustc_lexer = { version = "716.0.0", package = "rustc-ap-rustc_lexer" }
 rustc-hash = "1.1.0"
 arrayvec = "0.7"

crates/syntax/Cargo.toml

@@ -337,7 +337,7 @@ enum InsertPos {
 }
 
 #[derive(Default)]
-pub struct SyntaxRewriter<'a> {
+pub(crate) struct SyntaxRewriter<'a> {
     //FIXME: add debug_assertions that all elements are in fact from the same file.
     replacements: FxHashMap<SyntaxElement, Replacement>,
     insertions: IndexMap<InsertPos, Vec<SyntaxElement>>,
@@ -354,13 +354,13 @@ impl fmt::Debug for SyntaxRewriter<'_> {
 }
 
 impl SyntaxRewriter<'_> {
-    pub fn replace<T: Clone + Into<SyntaxElement>>(&mut self, what: &T, with: &T) {
+    pub(crate) fn replace<T: Clone + Into<SyntaxElement>>(&mut self, what: &T, with: &T) {
         let what = what.clone().into();
         let replacement = Replacement::Single(with.clone().into());
         self.replacements.insert(what, replacement);
     }
 
-    pub fn rewrite(&self, node: &SyntaxNode) -> SyntaxNode {
+    pub(crate) fn rewrite(&self, node: &SyntaxNode) -> SyntaxNode {
         let _p = profile::span("rewrite");
 
         if self.replacements.is_empty() && self.insertions.is_empty() {
@@ -370,37 +370,10 @@ impl SyntaxRewriter<'_> {
         with_green(node, green)
     }
 
-    pub fn rewrite_ast<N: AstNode>(self, node: &N) -> N {
+    pub(crate) fn rewrite_ast<N: AstNode>(self, node: &N) -> N {
         N::cast(self.rewrite(node.syntax())).unwrap()
     }
 
-    /// Returns a node that encompasses all replacements to be done by this rewriter.
-    ///
-    /// Passing the returned node to `rewrite` will apply all replacements queued up in `self`.
-    ///
-    /// Returns `None` when there are no replacements.
-    pub fn rewrite_root(&self) -> Option<SyntaxNode> {
-        let _p = profile::span("rewrite_root");
-        fn element_to_node_or_parent(element: &SyntaxElement) -> Option<SyntaxNode> {
-            match element {
-                SyntaxElement::Node(it) => Some(it.clone()),
-                SyntaxElement::Token(it) => it.parent(),
-            }
-        }
-
-        self.replacements
-            .keys()
-            .filter_map(element_to_node_or_parent)
-            .chain(self.insertions.keys().filter_map(|pos| match pos {
-                InsertPos::FirstChildOf(it) => Some(it.clone()),
-                InsertPos::After(it) => element_to_node_or_parent(it),
-            }))
-            // If we only have one replacement/insertion, we must return its parent node, since `rewrite` does
-            // not replace the node passed to it.
-            .map(|it| it.parent().unwrap_or(it))
-            .fold1(|a, b| least_common_ancestor(&a, &b).unwrap())
-    }
-
     fn replacement(&self, element: &SyntaxElement) -> Option<Replacement> {
         self.replacements.get(element).cloned()
     }

crates/syntax/src/algo.rs

@@ -47,6 +47,12 @@ pub trait AstNode {
     {
         Self::cast(self.syntax().clone_for_update()).unwrap()
     }
+    fn clone_subtree(&self) -> Self
+    where
+        Self: Sized,
+    {
+        Self::cast(self.syntax().clone_subtree()).unwrap()
+    }
 }
 
 /// Like `AstNode`, but wraps tokens rather than interior nodes.

crates/syntax/src/ast.rs

@@ -12,7 +12,7 @@
 use itertools::Itertools;
 use stdx::{format_to, never};
 
-use crate::{ast, AstNode, SourceFile, SyntaxKind, SyntaxNode, SyntaxToken};
+use crate::{ast, AstNode, SourceFile, SyntaxKind, SyntaxToken};
 
 /// While the parent module defines basic atomic "constructors", the `ext`
 /// module defines shortcuts for common things.
@@ -601,17 +601,11 @@ fn ast_from_text<N: AstNode>(text: &str) -> N {
             panic!("Failed to make ast node `{}` from text {}", std::any::type_name::<N>(), text)
         }
     };
-    let node = node.syntax().clone();
-    let node = unroot(node);
-    let node = N::cast(node).unwrap();
+    let node = node.clone_subtree();
     assert_eq!(node.syntax().text_range().start(), 0.into());
     node
 }
 
-fn unroot(n: SyntaxNode) -> SyntaxNode {
-    SyntaxNode::new_root(n.green().into())
-}
-
 pub fn token(kind: SyntaxKind) -> SyntaxToken {
     tokens::SOURCE_FILE
         .tree()

crates/syntax/src/ast/make.rs

@@ -951,6 +951,28 @@ match p.current() {
 
 ## Documentation
 
+Style inline code comments as proper sentences.
+Start with a capital letter, end with a dot.
+
+```rust
+// GOOD
+
+// Only simple single segment paths are allowed.
+MergeBehavior::Last => {
+    tree.use_tree_list().is_none() && tree.path().map(path_len) <= Some(1)
+}
+
+// BAD
+
+// only simple single segment paths are allowed
+MergeBehavior::Last => {
+    tree.use_tree_list().is_none() && tree.path().map(path_len) <= Some(1)
+}
+```
+
+**Rationale:** writing a sentence (or maybe even a paragraph) rather just "a comment" creates a more appropriate frame of mind.
+It tricks you into writing down more of the context you keep in your head while coding.
+
 For `.md` and `.adoc` files, prefer a sentence-per-line format, don't wrap lines.
 If the line is too long, you want to split the sentence in two :-)