Only set next/prev on potentially changed syntax nodes

Otherwise we end up running dijkstra on nodes at the end that we've
already marked as unchanged, and erroneously claim they're novel.

Fixes #112
better_inline 0.17.0
Wilfred Hughes 2022-01-25 22:26:40 +07:00
parent 440c94ce3c
commit e123cad49e
6 changed files with 44 additions and 28 deletions

@ -4,6 +4,9 @@
Improved performance when all file changes are close together.
Fixed a bug where syntax after the last changed item was incorrectly
marked as added.
### Display
Added syntax highlighting for unchanged comments, strings and types.

@ -95,7 +95,7 @@ mod tests {
use crate::{
graph::Edge::*,
positions::SingleLineSpan,
syntax::{init_info, AtomKind, ChangeKind},
syntax::{init_all_info, AtomKind, ChangeKind},
};
use itertools::Itertools;
@ -124,7 +124,7 @@ mod tests {
let lhs = Syntax::new_atom(&arena, pos_helper(0), "foo", AtomKind::Normal);
// Same content as LHS.
let rhs = Syntax::new_atom(&arena, pos_helper(0), "foo", AtomKind::Normal);
init_info(&[lhs], &[rhs]);
init_all_info(&[lhs], &[rhs]);
let start = Vertex {
lhs_syntax: Some(lhs),
@ -167,7 +167,7 @@ mod tests {
"]",
pos_helper(2),
)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -211,7 +211,7 @@ mod tests {
"]",
pos_helper(3),
)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -259,7 +259,7 @@ mod tests {
"}",
pos_helper(4),
)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -299,7 +299,7 @@ mod tests {
"foo",
AtomKind::Normal,
)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -339,7 +339,7 @@ mod tests {
)];
let rhs = vec![];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -379,7 +379,7 @@ mod tests {
];
let rhs = vec![];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -464,7 +464,7 @@ mod tests {
"]",
pos_helper(100),
)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -503,7 +503,7 @@ mod tests {
"the quick brown cat",
AtomKind::Comment,
)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -537,7 +537,7 @@ mod tests {
"foo bar",
AtomKind::Comment,
)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -579,7 +579,7 @@ mod tests {
"the quick brown fox.",
AtomKind::Comment,
)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let start = Vertex {
lhs_syntax: lhs.get(0).copied(),
@ -604,7 +604,7 @@ mod tests {
let arena = Arena::new();
let lhs = Syntax::new_atom(&arena, pos_helper(1), "foo", AtomKind::Normal);
let rhs = Syntax::new_atom(&arena, pos_helper(1), "foo", AtomKind::Normal);
init_info(&[lhs], &[rhs]);
init_all_info(&[lhs], &[rhs]);
mark_syntax(Some(lhs), Some(rhs));
assert_eq!(lhs.change(), Some(ChangeKind::Unchanged(rhs)));
@ -616,7 +616,7 @@ mod tests {
let arena = Arena::new();
let lhs = Syntax::new_atom(&arena, pos_helper(1), "foo", AtomKind::Normal);
let rhs = Syntax::new_atom(&arena, pos_helper(1), "bar", AtomKind::Normal);
init_info(&[lhs], &[rhs]);
init_all_info(&[lhs], &[rhs]);
mark_syntax(Some(lhs), Some(rhs));
assert_eq!(lhs.change(), Some(ChangeKind::Novel));

@ -48,6 +48,7 @@ use clap::{crate_version, App, AppSettings, Arg};
use sliders::fix_all_sliders;
use std::{env, path::Path};
use summary::DiffResult;
use syntax::init_next_prev;
use typed_arena::Arena;
use unchanged::skip_unchanged_at_ends;
use walkdir::WalkDir;
@ -57,7 +58,7 @@ use crate::{
files::{is_probably_binary, read_or_die},
line_parser as lp,
lines::MaxLine,
syntax::{change_positions, init_info},
syntax::{change_positions, init_all_info},
tree_sitter_parser as tsp,
};
@ -221,7 +222,7 @@ fn main() {
let ts_lang = tsp::from_language(lang);
let arena = Arena::new();
let ast = tsp::parse(&arena, &src, &ts_lang);
init_info(&ast, &[]);
init_all_info(&ast, &[]);
println!("{:#?}", ast);
}
None => {
@ -306,9 +307,12 @@ fn diff_file_content(display_path: &str, lhs_bytes: &[u8], rhs_bytes: &[u8]) ->
let lhs = tsp::parse(&arena, &lhs_src, &ts_lang);
let rhs = tsp::parse(&arena, &rhs_src, &ts_lang);
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
let (possibly_changed_lhs, possibly_changed_rhs) = skip_unchanged_at_ends(&lhs, &rhs);
init_next_prev(&possibly_changed_lhs);
init_next_prev(&possibly_changed_rhs);
mark_syntax(
possibly_changed_lhs.get(0).copied(),
possibly_changed_rhs.get(0).copied(),

@ -336,7 +336,7 @@ impl<'a> Syntax<'a> {
#[cfg(test)]
mod tests {
use super::*;
use crate::syntax::{init_info, AtomKind};
use crate::syntax::{init_all_info, AtomKind};
use pretty_assertions::assert_eq;
use typed_arena::Arena;
@ -375,7 +375,7 @@ mod tests {
}];
let rhs = [Syntax::new_atom(&arena, pos, "a", AtomKind::Comment)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
lhs[0].set_change(Unchanged(rhs[0]));
lhs[1].set_change(Novel);
@ -422,7 +422,7 @@ mod tests {
}];
let rhs = [Syntax::new_atom(&arena, pos, "a", AtomKind::Comment)];
init_info(&lhs, &rhs);
init_all_info(&lhs, &rhs);
lhs[0].set_change(Novel);
lhs[1].set_change(Novel);

@ -323,6 +323,12 @@ impl<'a> Syntax<'a> {
}
/// Initialise all the fields in `SyntaxInfo`.
pub fn init_all_info<'a>(lhs_roots: &[&'a Syntax<'a>], rhs_roots: &[&'a Syntax<'a>]) {
init_info(lhs_roots, rhs_roots);
init_next_prev(lhs_roots);
init_next_prev(rhs_roots);
}
pub fn init_info<'a>(lhs_roots: &[&'a Syntax<'a>], rhs_roots: &[&'a Syntax<'a>]) {
let mut id = 0;
init_info_single(lhs_roots, &mut id);
@ -392,12 +398,15 @@ fn set_content_id(nodes: &[&Syntax], existing: &mut HashMap<ContentKey, u32>) {
}
}
fn init_info_single<'a>(roots: &[&'a Syntax<'a>], next_id: &mut u32) {
pub fn init_next_prev<'a>(roots: &[&'a Syntax<'a>]) {
set_next(roots, None);
set_prev(roots, None);
set_prev_is_contiguous(roots);
}
fn init_info_single<'a>(roots: &[&'a Syntax<'a>], next_id: &mut u32) {
set_parent(roots, None);
set_num_ancestors(roots, 0);
set_prev_is_contiguous(roots);
set_unique_id(roots, next_id)
}
@ -832,7 +841,7 @@ mod tests {
let comment = Syntax::new_atom(&arena, pos.clone(), "foo", AtomKind::Comment);
let atom = Syntax::new_atom(&arena, pos, "foo", AtomKind::Normal);
init_info(&[comment], &[atom]);
init_all_info(&[comment], &[atom]);
assert_ne!(comment, atom);
}
@ -865,7 +874,7 @@ mod tests {
let x = Syntax::new_atom(&arena, pos.clone(), "foo\nbar", AtomKind::Comment);
let y = Syntax::new_atom(&arena, pos, "foo\n bar", AtomKind::Comment);
init_info(&[x], &[y]);
init_all_info(&[x], &[y]);
assert_eq!(x, y);
}
@ -899,7 +908,7 @@ mod tests {
content: "foo".into(),
kind: AtomKind::Normal,
};
init_info(&[&lhs], &[&rhs]);
init_all_info(&[&lhs], &[&rhs]);
assert_eq!(lhs, rhs);
}

@ -52,7 +52,7 @@ mod tests {
use super::*;
use crate::{
guess_language,
syntax::init_info,
syntax::init_all_info,
tree_sitter_parser::{from_language, parse},
};
use typed_arena::Arena;
@ -64,7 +64,7 @@ mod tests {
let lhs_nodes = parse(&arena, "unchanged A B", &config);
let rhs_nodes = parse(&arena, "unchanged X", &config);
init_info(&lhs_nodes, &rhs_nodes);
init_all_info(&lhs_nodes, &rhs_nodes);
let (lhs_after_skip, rhs_after_skip) = skip_unchanged_at_ends(&lhs_nodes, &rhs_nodes);
@ -88,7 +88,7 @@ mod tests {
let lhs_nodes = parse(&arena, "A B unchanged", &config);
let rhs_nodes = parse(&arena, "X unchanged", &config);
init_info(&lhs_nodes, &rhs_nodes);
init_all_info(&lhs_nodes, &rhs_nodes);
let (lhs_after_skip, rhs_after_skip) = skip_unchanged_at_ends(&lhs_nodes, &rhs_nodes);