mergiraf/mergiraf
mergiraf/mergiraf
9
213
Fork 45
Code Issues 50 Pull requests 6 Releases 19 Activity Actions

fix(typescript): Bad handling of semicolons in commutative merging of class bodies #585

Merged
wetneb merged 4 commits from tsx_separators into main 2025-09-11 23:01:54 +02:00
Conversation 7 Commits 4 Files changed 8 +72 -62
8 changed files with 72 additions and 62 deletions
Download patch file Download diff file Expand all files Collapse all files

2
doc/src/adding-a-language.md
View file

@ -215,8 +215,6 @@ CommutativeParent::new("declaration_list", "{", "\n\n", "}")
]),
```
Note that the separator for a children group and the separator for the commutative parent can only differ in leading and trailing whitespace.
## Add signatures
One piece of knowledge we have not encoded yet is the fact that `using` statements should be unique: there is no point in importing the same thing twice. This is specified using so-called signatures, which associate keys to the children of commutative parents. Those keys are then required to be unique among the children of a particular commutative parent. This mechanism can be used to define such keys for a lot of other elements. For instance, class fields are keyed by their name only, given that field names should be unique in a given class, regardless of their type. Keys can also be generated for methods, which not only includes their name but also the types of the arguments the function takes, as [C# supports method overloading](https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/member-overloading).

4
examples/typescript/working/attributes/Base.ts Normal file
View file

@ -0,0 +1,4 @@
export class Grid {
userAgent: string;
apiKey: string;
}

6
examples/typescript/working/attributes/Expected.ts Normal file
View file

@ -0,0 +1,6 @@
export class Grid {
userAgent: string;
apiKey: string;
strategy: string | null;
mission = new Mission(this);
}

5
examples/typescript/working/attributes/Left.ts Normal file
View file

@ -0,0 +1,5 @@
export class Grid {
userAgent: string;
apiKey: string;
strategy: string | null;
}

5
examples/typescript/working/attributes/Right.ts Normal file
View file

@ -0,0 +1,5 @@
export class Grid {
userAgent: string;
apiKey: string;
mission = new Mission(this);
}

63
src/lang_profile.rs
View file

@ -3,7 +3,7 @@ use std::{collections::HashSet, ffi::OsStr, fmt::Display, hash::Hash, path::Path
use itertools::Itertools;
use tree_sitter::Language;
use crate::{signature::SignatureDefinition, supported_langs::SUPPORTED_LANGUAGES};
use crate::{ast::AstNode, signature::SignatureDefinition, supported_langs::SUPPORTED_LANGUAGES};
/// Language-dependent settings to influence how merging is done.
/// All those settings are declarative (except for the tree-sitter parser, which is
@ -326,20 +326,6 @@ impl CommutativeParent {
/// Restrict a commutative parent to some children groups, possibly with their own separators
pub(crate) fn restricted_to(self, children_groups: Vec<ChildrenGroup>) -> Self {
#[cfg(debug_assertions)]
{
for children_group in &children_groups {
if let Some(specific_separator) = children_group.separator {
assert_eq!(
specific_separator.trim(),
self.separator.trim(),
"Children group separator '{specific_separator:?}' inconsistent with parent separator '{:?}' in commutative parent '{:?}'",
self.separator,
self.parent_type
);
}
}
}
Self {
children_groups,
..self
@ -354,22 +340,43 @@ impl CommutativeParent {
/// Can children with the supplied types commute together?
/// If so, return the separator to use when inserting two nodes
/// in the same place.
/// TODO: return None if any of the node_types is extra,
/// see https://codeberg.org/mergiraf/mergiraf/issues/467
pub(crate) fn child_separator(&self, node_types: &HashSet<&str>) -> Option<&'static str> {
if self.children_groups.is_empty() {
pub(crate) fn child_separator<'a>(
&self,
base_nodes: &[&'a AstNode<'a>],
left_nodes: &[&'a AstNode<'a>],
right_nodes: &[&'a AstNode<'a>],
) -> Option<&'static str> {
let trimmed_left_delim = self.left_delim.unwrap_or_default().trim();
let trimmed_right_delim = self.right_delim.unwrap_or_default().trim();
if (base_nodes.iter())
.chain(left_nodes)
.chain(right_nodes)
.any(|node| node.is_extra)
{
// Extra nodes can't commute
None
} else if self.children_groups.is_empty() {
// If there are no children groups to restrict commutativity to,
// any children can commute and the default separator is used
Some(self.separator)
} else {
// Otherwise, children can only commute if their types all belong
// to the same group, in which case the separator is either that of
// that specific group, or the default one for the commutative parent
// as a fall-back.
self.children_groups
.iter()
.find(|group| group.node_types.is_superset(node_types))
.map(|group| group.separator.unwrap_or(self.separator))
// Otherwise, children belong to a given group if both the grammar kinds of the content nodes
// and the contents of the separator nodes are accepted by the group.
self.children_groups.iter().find_map(|group| {
let group_separator = group.separator.unwrap_or(self.separator);
(base_nodes.iter())
.chain(left_nodes)
.chain(right_nodes)
.all(|node| {
let trimmed = node.source.trim();
group.node_types.contains(node.kind)
|| trimmed == group_separator.trim()
|| trimmed == trimmed_right_delim
|| trimmed == trimmed_left_delim
})
.then_some(group_separator)
})
}
}
@ -406,8 +413,6 @@ pub struct ChildrenGroup {
pub node_types: HashSet<&'static str>,
/// An optional separator specific to this children group,
/// better suited than the one from the commutative parent.
/// It must only differ from the separator of the parent up to
/// whitespace (their trimmed versions should be equal).
pub separator: Option<&'static str>,
}

10
src/supported_langs.rs
View file

@ -16,7 +16,15 @@ pub static SUPPORTED_LANGUAGES: LazyLock<Vec<LangProfile>> = LazyLock::new(|| {
.restricted_to_groups(&[&["import_statement"]]),
CommutativeParent::new("named_imports", "{", ", ", "}"),
CommutativeParent::new("object", "{", ", ", "}"),
CommutativeParent::new("class_body", " {\n", "\n\n", "\n}\n"),
CommutativeParent::new("class_body", " {\n", "\n\n", "\n}\n").restricted_to(vec![
ChildrenGroup::with_separator(&["public_field_definition", "index_signature"], ";\n"),
ChildrenGroup::new(&["class_static_block"]),
ChildrenGroup::new(&[
"abstract_method_signature",
"method_definition",
"method_signature",
]),
]),
CommutativeParent::new("interface_body", " {\n", ";\n", "\n}\n"),
CommutativeParent::new("object_type", " {\n", ";\n", "\n}\n"),
CommutativeParent::new("enum_body", " {\n", ",\n", "\n}\n"),

39
src/tree_builder.rs
View file

@ -605,12 +605,17 @@ impl<'a, 'b> TreeBuilder<'a, 'b> {
) -> Result<Vec<MergedTree<'a>>, String> {
let pad = visiting_state.indentation();
trace!("{pad}commutatively_merge_lists");
let trimmed_sep = commutative_parent.trimmed_separator();
let trimmed_left_delim = commutative_parent.left_delim.unwrap_or_default().trim();
let trimmed_right_delim = commutative_parent.right_delim.unwrap_or_default().trim();
// TODO improve handling of comments? comments added by the right side should ideally be placed sensibly
// first, map each list via class mapping to make each element comparable
// check that all the nodes involved are allowed to commute in this context
let raw_separator = commutative_parent
.child_separator(base, left, right)
.ok_or("The children are not allowed to commute")?;
let trimmed_sep = raw_separator.trim();
let trimmed_left_delim = commutative_parent.left_delim.unwrap_or_default().trim();
let trimmed_right_delim = commutative_parent.right_delim.unwrap_or_default().trim();
// map each list via class mapping to make each element comparable
let base_leaders: HashSet<_> = self
.keep_content_only(
base,
@ -639,32 +644,6 @@ impl<'a, 'b> TreeBuilder<'a, 'b> {
)
.collect();
// check that all the nodes involved are allowed to commute in this context
let child_types: HashSet<&str> = (base_leaders.iter())
.chain(left_leaders.iter())
.chain(right_leaders.iter())
.map(|leader| {
if self
.class_mapping
.representatives(leader)
.iter()
.any(|repr| repr.node.is_extra)
{
Err(
"One of the children is an extra node, cannot merge commutatively"
.to_string(),
)
} else {
Ok(leader.kind())
}
})
.try_collect()?;
let Some(raw_separator) = commutative_parent.child_separator(&child_types) else {
return Err("The children are not allowed to commute".to_string());
};
// NOTE: trimmed_sep is still consistent with raw_separator per the assumption that the two
// kinds of separators are equal up to leading and trailing whitespace
let left_added: HashSet<_> = left_leaders
.iter()
.filter(|x| !base_leaders.contains(x))