From 5c726426341d5cfe7d51b56275147390386062a3 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Mon, 18 Nov 2019 14:47:48 -0800 Subject: [PATCH 1/6] A test demonstrating the issue with named aliases --- cli/src/generate/node_types.rs | 89 ++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/cli/src/generate/node_types.rs b/cli/src/generate/node_types.rs index af90b900..1db65511 100644 --- a/cli/src/generate/node_types.rs +++ b/cli/src/generate/node_types.rs @@ -1169,6 +1169,95 @@ mod tests { ); } + #[test] + fn test_node_types_with_named_aliases() { + let node_types = get_node_types(InputGrammar { + name: String::new(), + extra_symbols: Vec::new(), + external_tokens: Vec::new(), + expected_conflicts: Vec::new(), + variables_to_inline: Vec::new(), + word_token: None, + supertype_symbols: vec![], + variables: vec![ + Variable { + name: "expression".to_string(), + kind: VariableType::Named, + rule: Rule::choice(vec![Rule::named("yield"), Rule::named("argument_list")]), + }, + Variable { + name: "yield".to_string(), + kind: VariableType::Named, + rule: Rule::Seq(vec![Rule::string("YIELD")]), + }, + Variable { + name: "argument_list".to_string(), + kind: VariableType::Named, + rule: Rule::choice(vec![ + Rule::named("x"), + Rule::alias(Rule::named("b"), "expression".to_string(), true), + ]), + }, + Variable { + name: "b".to_string(), + kind: VariableType::Named, + rule: Rule::choice(vec![Rule::seq(vec![Rule::string("B")]), Rule::named("c")]), + }, + Variable { + name: "c".to_string(), + kind: VariableType::Named, + rule: Rule::seq(vec![Rule::string("C")]), + }, + Variable { + name: "x".to_string(), + kind: VariableType::Named, + rule: Rule::seq(vec![Rule::string("X")]), + }, + ], + }); + + assert_eq!( + node_types.iter().map(|n| &n.kind).collect::>(), + &[ + "argument_list", + "c", + "expression", + "x", + "yield", + "B", + "C", + "X", + "YIELD" + ] + ); + assert_eq!( + node_types[2], + NodeInfoJSON { + kind: "expression".to_string(), + named: true, + subtypes: None, + children: Some(FieldInfoJSON { + multiple: false, + required: true, + types: vec![ + NodeTypeJSON { + kind: "argument_list".to_string(), + named: true, + }, + NodeTypeJSON { + kind: "c".to_string(), + named: true, + }, + NodeTypeJSON { + kind: "yield".to_string(), + named: true, + }, + ] + }), + fields: Some(BTreeMap::new()), + } + ); + } #[test] fn test_node_types_with_tokens_aliased_to_match_rules() { let node_types = get_node_types(InputGrammar { From c346ce4a5e9412ce7db9a2765fb808976ca56fc3 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Mon, 18 Nov 2019 14:48:24 -0800 Subject: [PATCH 2/6] Try not to loose existing children --- cli/src/generate/node_types.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/cli/src/generate/node_types.rs b/cli/src/generate/node_types.rs index 1db65511..b76df3a4 100644 --- a/cli/src/generate/node_types.rs +++ b/cli/src/generate/node_types.rs @@ -615,12 +615,20 @@ pub(crate) fn generate_node_types_json( .iter() .map(child_type_to_node_type) .collect::>(); + let mut multiple = info.children_without_fields.quantity.multiple; + let mut required = info.children_without_fields.quantity.required; + if let Some(children) = &mut node_type_json.children { + println!("children: {:?}", children); + children_types.append(&mut children.types); + multiple |= children.multiple; + required |= children.required; + } if children_types.len() > 0 { children_types.sort_unstable(); children_types.dedup(); node_type_json.children = Some(FieldInfoJSON { - multiple: info.children_without_fields.quantity.multiple, - required: info.children_without_fields.quantity.required, + multiple: multiple, + required: required, types: children_types, }); } From e2325102d14e28f982208919f21d3f779459167c Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Tue, 19 Nov 2019 09:03:35 -0800 Subject: [PATCH 3/6] No printing --- cli/src/generate/node_types.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/src/generate/node_types.rs b/cli/src/generate/node_types.rs index b76df3a4..af18a856 100644 --- a/cli/src/generate/node_types.rs +++ b/cli/src/generate/node_types.rs @@ -618,7 +618,6 @@ pub(crate) fn generate_node_types_json( let mut multiple = info.children_without_fields.quantity.multiple; let mut required = info.children_without_fields.quantity.required; if let Some(children) = &mut node_type_json.children { - println!("children: {:?}", children); children_types.append(&mut children.types); multiple |= children.multiple; required |= children.required; From 5489bc4dc59eacb98e43ef8d98d8a97f8f462a90 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 19 Nov 2019 11:53:47 -0800 Subject: [PATCH 4/6] Fix small issues with merging node types * Merge the `required` field with an 'and', not an 'or' * Merge field info in addition to children info --- cli/src/generate/node_types.rs | 36 +++++++++++++++++----------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/cli/src/generate/node_types.rs b/cli/src/generate/node_types.rs index af18a856..ca63b2ed 100644 --- a/cli/src/generate/node_types.rs +++ b/cli/src/generate/node_types.rs @@ -567,6 +567,8 @@ pub(crate) fn generate_node_types_json( } else if variable.kind.is_visible() && !syntax_grammar.variables_to_inline.contains(&symbol) { + // If a rule is aliased under multiple names, then its information + // contributes to multiple entries in the final JSON. for alias in aliases_by_symbol .get(&Symbol::non_terminal(i)) .unwrap_or(&HashSet::new()) @@ -581,17 +583,20 @@ pub(crate) fn generate_node_types_json( is_named = variable.kind == VariableType::Named; } + // There may already be an entry with this name, because multiple + // rules may be aliased with the *same* name. let node_type_json = node_types_json .entry(kind.clone()) .or_insert_with(|| NodeInfoJSON { kind: kind.clone(), named: is_named, - fields: None, + fields: Some(BTreeMap::new()), children: None, subtypes: None, }); - let mut fields_json = BTreeMap::new(); + + let fields_json = node_type_json.fields.as_mut().unwrap(); for (field, field_info) in info.fields.iter() { let field_info_json = fields_json.entry(field.clone()).or_insert(FieldInfoJSON { @@ -599,7 +604,6 @@ pub(crate) fn generate_node_types_json( required: true, types: Vec::new(), }); - field_info_json.multiple |= field_info.quantity.multiple; field_info_json.required &= field_info.quantity.required; field_info_json @@ -608,28 +612,24 @@ pub(crate) fn generate_node_types_json( field_info_json.types.sort_unstable(); field_info_json.types.dedup(); } - node_type_json.fields = Some(fields_json); + let mut children_types = info .children_without_fields .types .iter() .map(child_type_to_node_type) .collect::>(); - let mut multiple = info.children_without_fields.quantity.multiple; - let mut required = info.children_without_fields.quantity.required; - if let Some(children) = &mut node_type_json.children { - children_types.append(&mut children.types); - multiple |= children.multiple; - required |= children.required; - } if children_types.len() > 0 { - children_types.sort_unstable(); - children_types.dedup(); - node_type_json.children = Some(FieldInfoJSON { - multiple: multiple, - required: required, - types: children_types, + let mut children_json = node_type_json.children.get_or_insert(FieldInfoJSON { + multiple: false, + required: true, + types: Vec::new(), }); + children_json.types.append(&mut children_types); + children_json.types.sort_unstable(); + children_json.types.dedup(); + children_json.multiple |= info.children_without_fields.quantity.multiple; + children_json.required &= info.children_without_fields.quantity.required; } } } @@ -1245,7 +1245,7 @@ mod tests { subtypes: None, children: Some(FieldInfoJSON { multiple: false, - required: true, + required: false, types: vec![ NodeTypeJSON { kind: "argument_list".to_string(), From 1d6343466439b301ed2be49a82cea2687b5b450a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 19 Nov 2019 11:57:21 -0800 Subject: [PATCH 5/6] Refactor node-type merging --- cli/src/generate/node_types.rs | 64 ++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/cli/src/generate/node_types.rs b/cli/src/generate/node_types.rs index ca63b2ed..1560a068 100644 --- a/cli/src/generate/node_types.rs +++ b/cli/src/generate/node_types.rs @@ -59,6 +59,16 @@ pub struct ChildQuantity { multiple: bool, } +impl Default for FieldInfoJSON { + fn default() -> Self { + FieldInfoJSON { + multiple: false, + required: true, + types: Vec::new(), + } + } +} + impl Default for ChildQuantity { fn default() -> Self { Self::zero() @@ -517,6 +527,15 @@ pub(crate) fn generate_node_types_json( } }; + let populate_field_info_json = |json: &mut FieldInfoJSON, info: &FieldInfo| { + json.multiple |= info.quantity.multiple; + json.required &= info.quantity.required; + json.types + .extend(info.types.iter().map(child_type_to_node_type)); + json.types.sort_unstable(); + json.types.dedup(); + }; + let mut aliases_by_symbol = HashMap::new(); for (symbol, alias) in simple_aliases { aliases_by_symbol.insert(*symbol, { @@ -584,7 +603,7 @@ pub(crate) fn generate_node_types_json( } // There may already be an entry with this name, because multiple - // rules may be aliased with the *same* name. + // rules may be aliased with the same name. let node_type_json = node_types_json .entry(kind.clone()) @@ -598,38 +617,21 @@ pub(crate) fn generate_node_types_json( let fields_json = node_type_json.fields.as_mut().unwrap(); for (field, field_info) in info.fields.iter() { - let field_info_json = - fields_json.entry(field.clone()).or_insert(FieldInfoJSON { - multiple: false, - required: true, - types: Vec::new(), - }); - field_info_json.multiple |= field_info.quantity.multiple; - field_info_json.required &= field_info.quantity.required; - field_info_json - .types - .extend(field_info.types.iter().map(child_type_to_node_type)); - field_info_json.types.sort_unstable(); - field_info_json.types.dedup(); + populate_field_info_json( + &mut fields_json + .entry(field.clone()) + .or_insert(FieldInfoJSON::default()), + field_info, + ); } - let mut children_types = info - .children_without_fields - .types - .iter() - .map(child_type_to_node_type) - .collect::>(); - if children_types.len() > 0 { - let mut children_json = node_type_json.children.get_or_insert(FieldInfoJSON { - multiple: false, - required: true, - types: Vec::new(), - }); - children_json.types.append(&mut children_types); - children_json.types.sort_unstable(); - children_json.types.dedup(); - children_json.multiple |= info.children_without_fields.quantity.multiple; - children_json.required &= info.children_without_fields.quantity.required; + if info.children_without_fields.types.len() > 0 { + populate_field_info_json( + &mut node_type_json + .children + .get_or_insert(FieldInfoJSON::default()), + &info.children_without_fields, + ); } } } From 5a979d1457fc6f689600d59f87d169a25a8f59bd Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 19 Nov 2019 14:31:56 -0800 Subject: [PATCH 6/6] node-types: Add test for field merging with aliases Co-Authored-By: Timothy Clem --- cli/src/generate/node_types.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/cli/src/generate/node_types.rs b/cli/src/generate/node_types.rs index 1560a068..cd1f9637 100644 --- a/cli/src/generate/node_types.rs +++ b/cli/src/generate/node_types.rs @@ -1210,7 +1210,10 @@ mod tests { Variable { name: "b".to_string(), kind: VariableType::Named, - rule: Rule::choice(vec![Rule::seq(vec![Rule::string("B")]), Rule::named("c")]), + rule: Rule::choice(vec![ + Rule::field("f".to_string(), Rule::string("B")), + Rule::named("c"), + ]), }, Variable { name: "c".to_string(), @@ -1263,7 +1266,21 @@ mod tests { }, ] }), - fields: Some(BTreeMap::new()), + fields: Some( + vec![( + "f".to_string(), + FieldInfoJSON { + required: false, + multiple: false, + types: vec![NodeTypeJSON { + named: false, + kind: "B".to_string(), + }] + } + )] + .into_iter() + .collect() + ), } ); }