Skip to content

Commit 663bf59

Browse files
committed
refactor(services): extract derive and field diffing into helper methods
Extract derive comparison logic into `diff_derives()` to reduce duplication across function and struct diffing. Implement `diff_struct()` with full field, derive, visibility, and generic parameter comparison using new helper methods `extract_fields()` and `extract_generics()`.
1 parent 2759457 commit 663bf59

1 file changed

Lines changed: 224 additions & 27 deletions

File tree

src/services/differ.rs

Lines changed: 224 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -64,22 +64,7 @@ impl AstDiffer {
6464
// Compare derive attributes (Rust)
6565
let old_derives = Self::extract_derives(old_node, old_source);
6666
let new_derives = Self::extract_derives(new_node, new_source);
67-
let added: Vec<String> = new_derives
68-
.iter()
69-
.filter(|d| !old_derives.contains(d))
70-
.cloned()
71-
.collect();
72-
if !added.is_empty() {
73-
changes.push(ChangeDetail::DeriveAdded(added));
74-
}
75-
let removed: Vec<String> = old_derives
76-
.iter()
77-
.filter(|d| !new_derives.contains(d))
78-
.cloned()
79-
.collect();
80-
if !removed.is_empty() {
81-
changes.push(ChangeDetail::DeriveRemoved(removed));
82-
}
67+
Self::diff_derives(&old_derives, &new_derives, &mut changes);
8368

8469
// Compare mutability (Rust)
8570
let old_has_mut = Self::has_mut_params(old_node, old_source);
@@ -109,14 +94,187 @@ impl AstDiffer {
10994
changes
11095
}
11196

112-
/// Phase 2 stub (F-004) — struct/enum diffing not implemented yet.
97+
/// Compare old and new versions of a struct, enum, or class definition.
11398
pub fn diff_struct(
114-
_old_node: tree_sitter::Node,
115-
_old_source: &str,
116-
_new_node: tree_sitter::Node,
117-
_new_source: &str,
99+
old_node: tree_sitter::Node,
100+
old_source: &str,
101+
new_node: tree_sitter::Node,
102+
new_source: &str,
118103
) -> Vec<ChangeDetail> {
119-
Vec::new()
104+
let mut changes = Vec::new();
105+
106+
// Compare visibility
107+
let old_vis = Self::extract_visibility(old_node, old_source);
108+
let new_vis = Self::extract_visibility(new_node, new_source);
109+
if old_vis != new_vis {
110+
changes.push(ChangeDetail::VisibilityChanged {
111+
old: old_vis,
112+
new: new_vis,
113+
});
114+
}
115+
116+
// Compare derive attributes (Rust)
117+
let old_derives = Self::extract_derives(old_node, old_source);
118+
let new_derives = Self::extract_derives(new_node, new_source);
119+
Self::diff_derives(&old_derives, &new_derives, &mut changes);
120+
121+
// Compare fields or variants
122+
let old_fields = Self::extract_fields(old_node, old_source);
123+
let new_fields = Self::extract_fields(new_node, new_source);
124+
Self::diff_fields(&old_fields, &new_fields, &mut changes);
125+
126+
// Check for generic parameter changes
127+
let old_generics = Self::extract_generics(old_node, old_source);
128+
let new_generics = Self::extract_generics(new_node, new_source);
129+
if old_generics != new_generics
130+
&& let (Some(o), Some(n)) = (old_generics, new_generics)
131+
{
132+
changes.push(ChangeDetail::GenericChanged { old: o, new: n });
133+
}
134+
135+
changes
136+
}
137+
138+
fn diff_derives(old: &[String], new: &[String], changes: &mut Vec<ChangeDetail>) {
139+
let added: Vec<String> = new.iter().filter(|d| !old.contains(d)).cloned().collect();
140+
if !added.is_empty() {
141+
changes.push(ChangeDetail::DeriveAdded(added));
142+
}
143+
let removed: Vec<String> = old.iter().filter(|d| !new.contains(d)).cloned().collect();
144+
if !removed.is_empty() {
145+
changes.push(ChangeDetail::DeriveRemoved(removed));
146+
}
147+
}
148+
149+
fn extract_fields(node: tree_sitter::Node, source: &str) -> Vec<(String, String)> {
150+
let mut fields = Vec::new();
151+
// Find body-like node that contains field or variant definitions
152+
let body = (0..node.child_count())
153+
.filter_map(|i| {
154+
#[allow(clippy::cast_possible_truncation)]
155+
node.child(i as u32)
156+
})
157+
.find(|c| {
158+
matches!(
159+
c.kind(),
160+
"field_declaration_list"
161+
| "ordered_field_declaration_list"
162+
| "enum_variant_list"
163+
| "enum_member_declaration_list"
164+
| "class_body"
165+
| "interface_body"
166+
| "enum_body"
167+
)
168+
});
169+
170+
if let Some(b) = body {
171+
for i in 0..b.child_count() {
172+
#[allow(clippy::cast_possible_truncation)]
173+
if let Some(child) = b.child(i as u32) {
174+
// Skip symbols/punc
175+
if child.is_extra() || !child.is_named() {
176+
continue;
177+
}
178+
179+
if matches!(
180+
child.kind(),
181+
"field_declaration"
182+
| "public_field_definition"
183+
| "property_declaration"
184+
| "variable_declaration"
185+
) {
186+
let name = child
187+
.child_by_field_name("name")
188+
.or_else(|| child.child_by_field_name("declarations").and_then(|d| d.child(0)).and_then(|n| n.child_by_field_name("name")))
189+
.and_then(|n| n.utf8_text(source.as_bytes()).ok())
190+
.unwrap_or("")
191+
.to_string();
192+
let typ = child
193+
.child_by_field_name("type")
194+
.and_then(|t| t.utf8_text(source.as_bytes()).ok())
195+
.unwrap_or("")
196+
.to_string();
197+
if !name.is_empty() {
198+
fields.push((name, typ));
199+
}
200+
} else if matches!(
201+
child.kind(),
202+
"enum_variant" | "enum_member_declaration" | "enum_constant"
203+
) {
204+
let name = child
205+
.child_by_field_name("name")
206+
.and_then(|n| n.utf8_text(source.as_bytes()).ok())
207+
.unwrap_or("")
208+
.to_string();
209+
if !name.is_empty() {
210+
fields.push((name, String::new()));
211+
}
212+
}
213+
}
214+
}
215+
}
216+
fields
217+
}
218+
219+
fn diff_fields(
220+
old: &[(String, String)],
221+
new: &[(String, String)],
222+
changes: &mut Vec<ChangeDetail>,
223+
) {
224+
let old_names: HashSet<&str> = old.iter().map(|(n, _)| n.as_str()).collect();
225+
let new_names: HashSet<&str> = new.iter().map(|(n, _)| n.as_str()).collect();
226+
227+
// Added fields/variants
228+
for (name, typ) in new {
229+
if !old_names.contains(name.as_str()) && !name.is_empty() {
230+
let desc = if typ.is_empty() {
231+
name.clone()
232+
} else {
233+
format!("{name}: {typ}")
234+
};
235+
changes.push(ChangeDetail::FieldAdded(desc));
236+
}
237+
}
238+
239+
// Removed fields/variants
240+
for (name, typ) in old {
241+
if !new_names.contains(name.as_str()) && !name.is_empty() {
242+
let desc = if typ.is_empty() {
243+
name.clone()
244+
} else {
245+
format!("{name}: {typ}")
246+
};
247+
changes.push(ChangeDetail::FieldRemoved(desc));
248+
}
249+
}
250+
251+
// Type changes for fields that exist in both
252+
for (name, old_typ) in old {
253+
if let Some((_, new_typ)) = new.iter().find(|(n, _)| n == name)
254+
&& old_typ != new_typ
255+
&& !old_typ.is_empty()
256+
&& !new_typ.is_empty()
257+
{
258+
changes.push(ChangeDetail::FieldTypeChanged {
259+
name: name.clone(),
260+
old_type: old_typ.clone(),
261+
new_type: new_typ.clone(),
262+
});
263+
}
264+
}
265+
}
266+
267+
fn extract_generics(node: tree_sitter::Node, source: &str) -> Option<String> {
268+
// Look for type_parameters child (Rust, TS, Java)
269+
(0..node.child_count())
270+
.filter_map(|i| {
271+
#[allow(clippy::cast_possible_truncation)]
272+
node.child(i as u32)
273+
})
274+
.find(|c| matches!(c.kind(), "type_parameters" | "type_parameter_list"))
275+
.and_then(|g| g.utf8_text(source.as_bytes()).ok())
276+
.map(|s| s.trim().to_string())
277+
.filter(|s| !s.is_empty())
120278
}
121279

122280
fn bodies_semantically_equal(old_body: Option<&str>, new_body: Option<&str>) -> bool {
@@ -527,7 +685,49 @@ mod tests {
527685
}
528686

529687
#[test]
530-
fn diff_struct_returns_empty_phase1() {
688+
fn diff_struct_detects_added_field() {
689+
let old_src = "struct Config { timeout: u64 }";
690+
let new_src = "struct Config { timeout: u64, retry: u32 }";
691+
let mut parser = tree_sitter::Parser::new();
692+
parser
693+
.set_language(&tree_sitter_rust::LANGUAGE.into())
694+
.unwrap();
695+
let old_tree = parser.parse(old_src, None).unwrap();
696+
let new_tree = parser.parse(new_src, None).unwrap();
697+
let old_node = old_tree.root_node().child(0).unwrap();
698+
let new_node = new_tree.root_node().child(0).unwrap();
699+
let changes = AstDiffer::diff_struct(old_node, old_src, new_node, new_src);
700+
assert!(
701+
changes
702+
.iter()
703+
.any(|c| matches!(c, ChangeDetail::FieldAdded(f) if f.contains("retry"))),
704+
"should detect added field 'retry': {changes:?}"
705+
);
706+
}
707+
708+
#[test]
709+
fn diff_enum_detects_added_variant() {
710+
let old_src = "enum Color { Red, Green }";
711+
let new_src = "enum Color { Red, Green, Blue }";
712+
let mut parser = tree_sitter::Parser::new();
713+
parser
714+
.set_language(&tree_sitter_rust::LANGUAGE.into())
715+
.unwrap();
716+
let old_tree = parser.parse(old_src, None).unwrap();
717+
let new_tree = parser.parse(new_src, None).unwrap();
718+
let old_node = old_tree.root_node().child(0).unwrap();
719+
let new_node = new_tree.root_node().child(0).unwrap();
720+
let changes = AstDiffer::diff_struct(old_node, old_src, new_node, new_src);
721+
assert!(
722+
changes
723+
.iter()
724+
.any(|c| matches!(c, ChangeDetail::FieldAdded(f) if f == "Blue")),
725+
"should detect added variant 'Blue': {changes:?}"
726+
);
727+
}
728+
729+
#[test]
730+
fn diff_struct_returns_empty_if_no_changes() {
531731
let src = "struct Foo { x: i32 }";
532732
let mut parser = tree_sitter::Parser::new();
533733
parser
@@ -536,10 +736,7 @@ mod tests {
536736
let tree = parser.parse(src, None).unwrap();
537737
let node = tree.root_node().child(0).unwrap();
538738
let changes = AstDiffer::diff_struct(node, src, node, src);
539-
assert!(
540-
changes.is_empty(),
541-
"Phase 1: diff_struct should return empty"
542-
);
739+
assert!(changes.is_empty());
543740
}
544741

545742
#[test]

0 commit comments

Comments
 (0)