Skip to content

Commit a61bb4a

Browse files
committed
Don't replace parent node when inserting as first child in algo::diff
1 parent c365329 commit a61bb4a

File tree

1 file changed

+89
-36
lines changed

1 file changed

+89
-36
lines changed

crates/syntax/src/algo.rs

Lines changed: 89 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -111,18 +111,28 @@ pub enum InsertPosition<T> {
111111

112112
type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<rustc_hash::FxHasher>>;
113113

114+
#[derive(Debug, Hash, PartialEq, Eq)]
115+
enum TreeDiffInsertPos {
116+
After(SyntaxElement),
117+
AsFirstChild(SyntaxElement),
118+
}
119+
114120
#[derive(Debug)]
115121
pub struct TreeDiff {
116122
replacements: FxHashMap<SyntaxElement, SyntaxElement>,
117123
deletions: Vec<SyntaxElement>,
118124
// the vec as well as the indexmap are both here to preserve order
119-
insertions: FxIndexMap<SyntaxElement, Vec<SyntaxElement>>,
125+
insertions: FxIndexMap<TreeDiffInsertPos, Vec<SyntaxElement>>,
120126
}
121127

122128
impl TreeDiff {
123129
pub fn into_text_edit(&self, builder: &mut TextEditBuilder) {
124130
for (anchor, to) in self.insertions.iter() {
125-
to.iter().for_each(|to| builder.insert(anchor.text_range().end(), to.to_string()));
131+
let offset = match anchor {
132+
TreeDiffInsertPos::After(it) => it.text_range().end(),
133+
TreeDiffInsertPos::AsFirstChild(it) => it.text_range().start(),
134+
};
135+
to.iter().for_each(|to| builder.insert(offset, to.to_string()));
126136
}
127137
for (from, to) in self.replacements.iter() {
128138
builder.replace(from.text_range(), to.to_string())
@@ -188,19 +198,20 @@ pub fn diff(from: &SyntaxNode, to: &SyntaxNode) -> TreeDiff {
188198
let lhs_child = lhs_children.next();
189199
match (lhs_child.clone(), rhs_children.next()) {
190200
(None, None) => break,
191-
(None, Some(element)) => match last_lhs.clone() {
192-
Some(prev) => {
193-
mark::hit!(diff_insert);
194-
diff.insertions.entry(prev).or_insert_with(Vec::new).push(element);
195-
}
196-
// first iteration, this means we got no anchor element to insert after
197-
// therefor replace the parent node instead
198-
None => {
199-
mark::hit!(diff_replace_parent);
200-
diff.replacements.insert(lhs.clone().into(), rhs.clone().into());
201-
break;
202-
}
203-
},
201+
(None, Some(element)) => {
202+
let insert_pos = match last_lhs.clone() {
203+
Some(prev) => {
204+
mark::hit!(diff_insert);
205+
TreeDiffInsertPos::After(prev)
206+
}
207+
// first iteration, insert into out parent as the first child
208+
None => {
209+
mark::hit!(diff_insert_as_first_child);
210+
TreeDiffInsertPos::AsFirstChild(lhs.clone().into())
211+
}
212+
};
213+
diff.insertions.entry(insert_pos).or_insert_with(Vec::new).push(element);
214+
}
204215
(Some(element), None) => {
205216
mark::hit!(diff_delete);
206217
diff.deletions.push(element);
@@ -224,8 +235,15 @@ pub fn diff(from: &SyntaxNode, to: &SyntaxNode) -> TreeDiff {
224235
}
225236
}
226237
let drain = look_ahead_scratch.drain(..);
227-
if let Some(prev) = last_lhs.clone().filter(|_| insert) {
228-
diff.insertions.entry(prev).or_insert_with(Vec::new).extend(drain);
238+
if insert {
239+
let insert_pos = if let Some(prev) = last_lhs.clone().filter(|_| insert) {
240+
TreeDiffInsertPos::After(prev)
241+
} else {
242+
mark::hit!(insert_first_child);
243+
TreeDiffInsertPos::AsFirstChild(lhs.clone().into())
244+
};
245+
246+
diff.insertions.entry(insert_pos).or_insert_with(Vec::new).extend(drain);
229247
rhs_children = rhs_children_clone;
230248
} else {
231249
go(diff, lhs_ele, rhs_ele)
@@ -629,18 +647,19 @@ mod tests {
629647

630648
#[test]
631649
fn replace_parent() {
632-
mark::check!(diff_replace_parent);
650+
mark::check!(diff_insert_as_first_child);
633651
check_diff(
634652
r#""#,
635653
r#"use foo::bar;"#,
636654
expect![[r#"
637655
insertions:
638656
639-
657+
Line 0: AsFirstChild(Node([email protected]))
658+
-> use foo::bar;
640659
641660
replacements:
642661
643-
Line 0: Node([email protected]) -> use foo::bar;
662+
644663
645664
deletions:
646665
@@ -663,7 +682,7 @@ use baz;"#,
663682
expect![[r#"
664683
insertions:
665684
666-
Line 2: Node([email protected])
685+
Line 2: After(Node([email protected]))
667686
-> "\n"
668687
-> use baz;
669688
@@ -691,7 +710,7 @@ use baz;"#,
691710
expect![[r#"
692711
insertions:
693712
694-
Line 2: Token([email protected] "\n")
713+
Line 2: After(Token([email protected] "\n"))
695714
-> use bar;
696715
-> "\n"
697716
@@ -719,7 +738,7 @@ use baz;"#,
719738
expect![[r#"
720739
insertions:
721740
722-
Line 0: Token([email protected] "\n")
741+
Line 0: After(Token([email protected] "\n"))
723742
-> use foo;
724743
-> "\n"
725744
@@ -734,6 +753,36 @@ use baz;"#,
734753
)
735754
}
736755

756+
#[test]
757+
fn first_child_insertion() {
758+
mark::check!(insert_first_child);
759+
check_diff(
760+
r#"fn main() {
761+
stdi
762+
}"#,
763+
r#"use foo::bar;
764+
765+
fn main() {
766+
stdi
767+
}"#,
768+
expect![[r#"
769+
insertions:
770+
771+
Line 0: AsFirstChild(Node([email protected]))
772+
-> use foo::bar;
773+
-> "\n\n "
774+
775+
replacements:
776+
777+
778+
779+
deletions:
780+
781+
782+
"#]],
783+
);
784+
}
785+
737786
#[test]
738787
fn delete_last() {
739788
mark::check!(diff_delete);
@@ -776,7 +825,7 @@ use crate::AstNode;
776825
expect![[r#"
777826
insertions:
778827
779-
Line 1: Node([email protected])
828+
Line 1: After(Node([email protected]))
780829
-> "\n\n"
781830
-> use crate::AstNode;
782831
@@ -842,10 +891,10 @@ use std::ops::{self, RangeInclusive};
842891
expect![[r#"
843892
insertions:
844893
845-
Line 2: Node([email protected])
894+
Line 2: After(Node([email protected]))
846895
-> ::
847896
-> fmt
848-
Line 6: Token([email protected] "\n")
897+
Line 6: After(Token([email protected] "\n"))
849898
-> use std::hash::BuildHasherDefault;
850899
-> "\n"
851900
-> use std::ops::{self, RangeInclusive};
@@ -889,14 +938,14 @@ fn main() {
889938
expect![[r#"
890939
insertions:
891940
892-
Line 3: Node([email protected])
941+
Line 3: After(Node([email protected]))
893942
-> " "
894943
-> match Err(92) {
895944
Ok(it) => it,
896945
_ => return,
897946
}
898947
-> ;
899-
Line 3: Node([email protected])
948+
Line 3: After(Node([email protected]))
900949
-> "\n "
901950
-> foo(x);
902951
@@ -931,14 +980,18 @@ fn main() {
931980
_ => format!("{}", syn),
932981
};
933982

934-
let insertions = diff.insertions.iter().format_with("\n", |(k, v), f| {
935-
f(&format!(
936-
"Line {}: {:?}\n-> {}",
937-
line_number(k),
938-
k,
939-
v.iter().format_with("\n-> ", |v, f| f(&fmt_syntax(v)))
940-
))
941-
});
983+
let insertions =
984+
diff.insertions.iter().format_with("\n", |(k, v), f| -> Result<(), std::fmt::Error> {
985+
f(&format!(
986+
"Line {}: {:?}\n-> {}",
987+
line_number(match k {
988+
super::TreeDiffInsertPos::After(syn) => syn,
989+
super::TreeDiffInsertPos::AsFirstChild(syn) => syn,
990+
}),
991+
k,
992+
v.iter().format_with("\n-> ", |v, f| f(&fmt_syntax(v)))
993+
))
994+
});
942995

943996
let replacements = diff
944997
.replacements

0 commit comments

Comments
 (0)