diff --git a/shared/tree-sitter-extractor/src/extractor/mod.rs b/shared/tree-sitter-extractor/src/extractor/mod.rs index 13ee3264133b..06505f9b1fc1 100644 --- a/shared/tree-sitter-extractor/src/extractor/mod.rs +++ b/shared/tree-sitter-extractor/src/extractor/mod.rs @@ -904,7 +904,8 @@ impl<'a> Visitor<'a> { if tp == single_type { return true; } - if let EntryKind::Union { members } = &self.schema.get(single_type).unwrap().kind + if let EntryKind::Union { members, .. } = + &self.schema.get(single_type).unwrap().kind && self.type_matches_set(tp, members) { return true; @@ -926,7 +927,7 @@ impl<'a> Visitor<'a> { return true; } for other in types.iter() { - if let EntryKind::Union { members } = &self.schema.get(other).unwrap().kind + if let EntryKind::Union { members, .. } = &self.schema.get(other).unwrap().kind && self.type_matches_set(tp, members) { return true; diff --git a/shared/tree-sitter-extractor/src/generator/mod.rs b/shared/tree-sitter-extractor/src/generator/mod.rs index ea90a8482894..81bf0955cd6e 100644 --- a/shared/tree-sitter-extractor/src/generator/mod.rs +++ b/shared/tree-sitter-extractor/src/generator/mod.rs @@ -399,7 +399,9 @@ fn convert_nodes( .collect(); for node in nodes.values() { match &node.kind { - node_types::EntryKind::Union { members: n_members } => { + node_types::EntryKind::Union { + members: n_members, .. + } => { // It's a tree-sitter supertype node, for which we create a union // type. let members: Set<&str> = n_members diff --git a/shared/tree-sitter-extractor/src/generator/ql_gen.rs b/shared/tree-sitter-extractor/src/generator/ql_gen.rs index b6f3d45f4b12..282b6726becd 100644 --- a/shared/tree-sitter-extractor/src/generator/ql_gen.rs +++ b/shared/tree-sitter-extractor/src/generator/ql_gen.rs @@ -796,15 +796,15 @@ fn create_field_getters<'a>( fn compute_direct_supertypes( nodes: &node_types::NodeTypeMap, -) -> std::collections::BTreeMap> { +) -> BTreeMap<&node_types::TypeName, BTreeSet<&node_types::TypeName>> { let mut supertypes = std::collections::BTreeMap::new(); - for node in nodes.values() { - if let node_types::EntryKind::Union { members } = &node.kind { + for (supertype, node) in nodes { + if let node_types::EntryKind::Union { members, .. } = &node.kind { for member in members { supertypes - .entry(member.clone()) + .entry(member) .or_insert_with(BTreeSet::new) - .insert(node.ql_class_name.as_str()); + .insert(supertype); } } } @@ -813,12 +813,13 @@ fn compute_direct_supertypes( fn ast_base_types<'a>( type_name: &node_types::TypeName, - direct_supertypes: &std::collections::BTreeMap>, + nodes: &'a node_types::NodeTypeMap, + direct_supertypes: &BTreeMap<&node_types::TypeName, BTreeSet<&node_types::TypeName>>, ) -> BTreeSet> { match direct_supertypes.get(type_name) { Some(supertypes) if !supertypes.is_empty() => supertypes .iter() - .map(|name| ql::Type::Facade(name)) + .map(|name| ql::Type::Facade(nodes.get(*name).unwrap().ql_class_name.as_str())) .collect(), _ => vec![ql::Type::Facade("AstNode")].into_iter().collect(), } @@ -827,72 +828,60 @@ fn ast_base_types<'a>( fn class_supertypes<'a>( type_name: &node_types::TypeName, dbscheme_name: &'a str, - direct_supertypes: &std::collections::BTreeMap>, + nodes: &'a node_types::NodeTypeMap, + direct_supertypes: &BTreeMap<&node_types::TypeName, BTreeSet<&node_types::TypeName>>, ) -> BTreeSet> { - let mut supertypes = ast_base_types(type_name, direct_supertypes); + let mut supertypes = ast_base_types(type_name, nodes, direct_supertypes); supertypes.insert(ql::Type::At(dbscheme_name)); supertypes } -/// Returns whether `a` and `b` have the same signature, i.e. the same name, -/// return type, and formal parameters. Predicates with the same signature can -/// override one another. -fn same_predicate_signature(a: &ql::Predicate, b: &ql::Predicate) -> bool { - a.name == b.name && a.return_type == b.return_type && a.formal_parameters == b.formal_parameters +/// Returns the light-signature of 'pred', for use in override detection. +fn get_light_signature(pred: &ql::Predicate) -> String { + format!("{}/{}", pred.name, pred.formal_parameters.len()) } -/// Computes, for each tree-sitter supertype (union) node, the list of -/// predicates that are guaranteed to be defined identically (in terms of -/// name, return type, and formal parameters, though not necessarily body) by -/// every one of its members. These are the predicates that can be hoisted to -/// an `abstract` predicate on the union's class, with the corresponding -/// predicates on its members becoming `override`s. +/// Computes the light-signatures of the predicates exposed by a node, including +/// those inherited from its direct supertypes. /// -/// The result for a given node is memoized in `cache` (keyed by its QL class -/// name), and also used to answer the query for any other node that +/// The result for a given node is memoized in `cache`, and also used to answer +/// the query for any other node that /// (directly, or transitively through further supertypes) has that node as a /// member. The same cache also serves as the answer to "what does the class /// named X expose?", used by `is_predicate_inherited`. fn compute_exposed_predicates<'a, 'b>( type_name: &'a node_types::TypeName, - nodes: &'a node_types::NodeTypeMap, field_predicates: &BTreeMap<&node_types::TypeName, Vec>>, - cache: &'b mut BTreeMap<&'a str, Vec>>, -) -> &'b Vec> { - let node = nodes.get(type_name); - let class_name = node.map_or(type_name.kind.as_str(), |node| node.ql_class_name.as_str()); - if !cache.contains_key(class_name) { + direct_supertypes: &BTreeMap<&'a node_types::TypeName, BTreeSet<&'a node_types::TypeName>>, + cache: &'b mut BTreeMap<&'a node_types::TypeName, BTreeSet>, +) -> &'b BTreeSet { + if !cache.contains_key(type_name) { // Supertype declarations that recursively refer to themselves are a mistake, but we don't // want to cause infinite recursion, so we insert a temporary sentinel. - cache.insert(class_name, Vec::new()); - let exposed = match node.map(|node| &node.kind) { - Some(node_types::EntryKind::Table { .. }) => { - field_predicates.get(type_name).cloned().unwrap_or_default() - } - Some(node_types::EntryKind::Union { members }) => { - let mut members = members.iter(); - let mut common = match members.next() { - Some(first) => { - compute_exposed_predicates(first, nodes, field_predicates, cache).clone() - } - None => Vec::new(), - }; - for member in members { - let member_predicates = - compute_exposed_predicates(member, nodes, field_predicates, cache); - common.retain(|predicate| { - member_predicates - .iter() - .any(|other| same_predicate_signature(predicate, other)) - }); - } - common + cache.insert(type_name, BTreeSet::new()); + let mut exposed: BTreeSet = field_predicates + .get(type_name) + .into_iter() + .flatten() + .map(get_light_signature) + .collect(); + if let Some(supertypes) = direct_supertypes.get(type_name) { + for supertype in supertypes { + exposed.extend( + compute_exposed_predicates( + supertype, + field_predicates, + direct_supertypes, + cache, + ) + .iter() + .cloned(), + ); } - Some(node_types::EntryKind::Token { .. }) | None => Vec::new(), - }; - cache.insert(class_name, exposed); + } + cache.insert(type_name, exposed); } - cache.get(class_name).unwrap() + cache.get(type_name).unwrap() } /// Returns whether `predicate` (declared, or about to be declared, on the @@ -903,16 +892,15 @@ fn compute_exposed_predicates<'a, 'b>( fn is_predicate_inherited( predicate: &ql::Predicate, type_name: &node_types::TypeName, - direct_supertypes: &BTreeMap>, - exposed_predicates: &BTreeMap<&str, Vec>, + direct_supertypes: &BTreeMap<&node_types::TypeName, BTreeSet<&node_types::TypeName>>, + exposed_predicates: &BTreeMap<&node_types::TypeName, BTreeSet>, ) -> bool { + let light_signature = get_light_signature(predicate); direct_supertypes.get(type_name).is_some_and(|supertypes| { supertypes.iter().any(|supertype| { - exposed_predicates.get(supertype).is_some_and(|predicates| { - predicates - .iter() - .any(|other| same_predicate_signature(predicate, other)) - }) + exposed_predicates + .get(supertype) + .is_some_and(|signatures| signatures.contains(&light_signature)) }) }) } @@ -930,25 +918,24 @@ pub fn convert_nodes(nodes: &node_types::NodeTypeMap) -> Vec> { } } - // First, compute the field-getter predicates (and the expressions used by - // `getAFieldOrChild`) for every table node, without yet knowing whether - // any of them will need to be marked `override`. These are needed both - // to build the final classes below, and to figure out which fields are - // shared identically by all the members of a supertype. + // First, compute field-getter predicates for tables and the explicitly + // declared field predicates for supertypes. let mut field_predicates: BTreeMap<&node_types::TypeName, Vec>> = BTreeMap::new(); let mut get_child_exprs: BTreeMap<&node_types::TypeName, Vec>> = BTreeMap::new(); for (type_name, node) in nodes { - if let node_types::EntryKind::Table { - name: main_table_name, - fields, - } = &node.kind - { - if fields.is_empty() { - panic!("Encountered node '{}' with no fields", type_name.kind); + let (main_table_name, fields, has_storage) = match &node.kind { + node_types::EntryKind::Table { name, fields } => (name.as_str(), fields, true), + node_types::EntryKind::Union { fields, .. } => { + (node.dbscheme_name.as_str(), fields, false) } - + node_types::EntryKind::Token { .. } => continue, + }; + if has_storage && fields.is_empty() { + panic!("Encountered node '{}' with no fields", type_name.kind); + } + if !fields.is_empty() { // Count how many columns there will be in the main table. There // will be one for the id, plus one for each field that's stored // as a column. @@ -969,27 +956,25 @@ pub fn convert_nodes(nodes: &node_types::NodeTypeMap) -> Vec> { nodes, ); predicates.extend(get_preds); - if let Some(get_child_expr) = get_child_expr { + if has_storage && let Some(get_child_expr) = get_child_expr { exprs.push(get_child_expr) } } field_predicates.insert(type_name, predicates); - get_child_exprs.insert(type_name, exprs); + if has_storage { + get_child_exprs.insert(type_name, exprs); + } } } - // Next, for every supertype (union) node, compute the predicates that are - // guaranteed to be defined identically (in name, return type, and formal - // parameters) by every one of its members. Such predicates can be hoisted - // to an `abstract` predicate on the supertype's class, with the - // corresponding predicates on its members becoming `override`s. - let mut exposed_predicates: BTreeMap<&str, Vec>> = BTreeMap::new(); + // Next, collect the predicates explicitly exposed by every supertype. + let mut exposed_predicates: BTreeMap<&node_types::TypeName, BTreeSet> = BTreeMap::new(); for (type_name, node) in nodes { if let node_types::EntryKind::Union { .. } = &node.kind { compute_exposed_predicates( type_name, - nodes, &field_predicates, + &direct_supertypes, &mut exposed_predicates, ); } @@ -1002,7 +987,7 @@ pub fn convert_nodes(nodes: &node_types::NodeTypeMap) -> Vec> { let get_a_primary_ql_class = create_get_a_primary_ql_class(&node.ql_class_name, true); let mut supertypes = - class_supertypes(type_name, &node.dbscheme_name, &direct_supertypes); + class_supertypes(type_name, &node.dbscheme_name, nodes, &direct_supertypes); supertypes.insert(ql::Type::Facade("Token")); classes.push(ql::TopLevel::Class(ql::Class { qldoc: Some(format!("A class representing `{}` tokens.", type_name.kind)), @@ -1017,12 +1002,13 @@ pub fn convert_nodes(nodes: &node_types::NodeTypeMap) -> Vec> { })); } } - node_types::EntryKind::Union { members: _ } => { + node_types::EntryKind::Union { .. } => { // It's a tree-sitter supertype node, so we're wrapping a dbscheme - // union type. Any predicate that's identically defined by every - // member becomes an `abstract` predicate here. - let predicates = exposed_predicates - .get(node.ql_class_name.as_str()) + // union type. Fields declared on the supertype get an empty + // default implementation so members without the field can + // inherit it. + let predicates = field_predicates + .get(type_name) .cloned() .unwrap_or_default() .into_iter() @@ -1035,7 +1021,7 @@ pub fn convert_nodes(nodes: &node_types::NodeTypeMap) -> Vec> { ), is_private: false, is_final: false, - body: None, + body: Some(ql::Expression::Pred("none", vec![])), ..predicate }) .collect(); @@ -1049,6 +1035,7 @@ pub fn convert_nodes(nodes: &node_types::NodeTypeMap) -> Vec> { supertypes: class_supertypes( type_name, &node.dbscheme_name, + nodes, &direct_supertypes, ), characteristic_predicate: None, @@ -1067,6 +1054,7 @@ pub fn convert_nodes(nodes: &node_types::NodeTypeMap) -> Vec> { supertypes: class_supertypes( type_name, &node.dbscheme_name, + nodes, &direct_supertypes, ), characteristic_predicate: None, @@ -1075,8 +1063,7 @@ pub fn convert_nodes(nodes: &node_types::NodeTypeMap) -> Vec> { // A field getter that's identically defined (in signature) by // every member of one of this node's direct supertypes is an - // override of the corresponding `abstract` predicate declared - // there. + // override of the corresponding predicate declared there. main_class.predicates.extend( field_predicates .get(type_name) @@ -1215,3 +1202,156 @@ pub fn create_print_ast_module(nodes: &node_types::NodeTypeMap) -> ql::TopLevel< overlay: None, }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn supertype_exposes_only_declared_fields() { + let node_types = r#"[ + { + "type": "root", + "named": true, + "subtypes": [ + { "type": "container", "named": true } + ], + "fields": { + "ancestor": { + "multiple": false, + "required": true, + "types": [{ "type": "item", "named": true }] + } + } + }, + { + "type": "container", + "named": true, + "subtypes": [ + { "type": "alpha", "named": true }, + { "type": "beta", "named": true } + ], + "fields": { + "item": { + "multiple": true, + "required": false, + "types": [{ "type": "container_item", "named": true }] + } + } + }, + { + "type": "alpha", + "named": true, + "fields": { + "ancestor": { + "multiple": false, + "required": true, + "types": [{ "type": "item", "named": true }] + }, + "hidden": { + "multiple": false, + "required": true, + "types": [{ "type": "item", "named": true }] + }, + "item": { + "multiple": true, + "required": false, + "types": [{ "type": "item", "named": true }] + } + } + }, + { + "type": "beta", + "named": true, + "fields": { + "hidden": { + "multiple": false, + "required": true, + "types": [{ "type": "item", "named": true }] + }, + "item": { + "multiple": true, + "required": false, + "types": [{ "type": "item", "named": true }] + } + } + }, + { "type": "container_item", "named": true, "fields": {} }, + { "type": "item", "named": true, "fields": {} } + ]"#; + let nodes = node_types::read_node_types_str("test", node_types).unwrap(); + let direct_supertypes = compute_direct_supertypes(&nodes); + let alpha = nodes.keys().find(|name| name.kind == "alpha").unwrap(); + let container = direct_supertypes + .get(alpha) + .unwrap() + .iter() + .find(|name| name.kind == "container") + .copied() + .unwrap(); + assert!( + direct_supertypes + .get(container) + .unwrap() + .iter() + .any(|name| name.kind == "root") + ); + + let classes = convert_nodes(&nodes); + + let container = classes + .iter() + .find_map(|top_level| match top_level { + ql::TopLevel::Class(class) if class.name == "Container" => Some(class), + _ => None, + }) + .unwrap(); + assert_eq!( + container + .predicates + .iter() + .map(|predicate| predicate.name) + .collect::>(), + BTreeSet::from(["getAnItem", "getItem"]), + ); + assert!(container.predicates.iter().all(|predicate| { + predicate.body == Some(ql::Expression::Pred("none", vec![])) && !predicate.is_final + })); + + let alpha = classes + .iter() + .find_map(|top_level| match top_level { + ql::TopLevel::Class(class) if class.name == "Alpha" => Some(class), + _ => None, + }) + .unwrap(); + let container_get_item = container + .predicates + .iter() + .find(|predicate| predicate.name == "getItem") + .unwrap(); + let alpha_get_item = alpha + .predicates + .iter() + .find(|predicate| predicate.name == "getItem") + .unwrap(); + assert!(container_get_item.return_type != alpha_get_item.return_type); + assert!(alpha_get_item.overridden); + assert!( + alpha + .predicates + .iter() + .find(|predicate| predicate.name == "getAncestor") + .unwrap() + .overridden + ); + assert!( + !alpha + .predicates + .iter() + .find(|predicate| predicate.name == "getHidden") + .unwrap() + .overridden + ); + } +} diff --git a/shared/tree-sitter-extractor/src/node_types.rs b/shared/tree-sitter-extractor/src/node_types.rs index 2967bc845802..494762d62945 100644 --- a/shared/tree-sitter-extractor/src/node_types.rs +++ b/shared/tree-sitter-extractor/src/node_types.rs @@ -17,9 +17,17 @@ pub struct Entry { #[derive(Debug)] pub enum EntryKind { - Union { members: Set }, - Table { name: String, fields: Vec }, - Token { kind_id: usize }, + Union { + members: Set, + fields: Vec, + }, + Table { + name: String, + fields: Vec, + }, + Token { + kind_id: usize, + }, } #[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] @@ -135,16 +143,39 @@ pub fn convert_nodes(prefix: &str, nodes: &[NodeInfo]) -> NodeTypeMap { if !subtypes.is_empty() { // It's a tree-sitter supertype node, for which we create a union // type. + let type_name = TypeName { + kind: node.kind.clone(), + named: node.named, + }; + let mut fields = Vec::new(); + for (field_name, field_info) in &node.fields { + add_field( + prefix, + &type_name, + Some(field_name.to_string()), + field_info, + &mut fields, + &token_kinds, + ); + } + if let Some(children) = &node.children { + add_field( + prefix, + &type_name, + None, + children, + &mut fields, + &token_kinds, + ); + } entries.insert( - TypeName { - kind: node.kind.clone(), - named: node.named, - }, + type_name, Entry { dbscheme_name, ql_class_name, kind: EntryKind::Union { members: convert_types(subtypes), + fields, }, }, ); diff --git a/shared/yeast-schema/src/node_types_yaml.rs b/shared/yeast-schema/src/node_types_yaml.rs index c2d27793a18c..aa76215779d2 100644 --- a/shared/yeast-schema/src/node_types_yaml.rs +++ b/shared/yeast-schema/src/node_types_yaml.rs @@ -7,6 +7,12 @@ /// _expression: /// - assignment /// - binary +/// callable: +/// subtypes: +/// - function +/// - closure +/// fields: +/// parameter*: parameter /// /// named: /// assignment: @@ -31,13 +37,39 @@ use serde_json::json; #[derive(Deserialize, Default)] struct YamlNodeTypes { #[serde(default)] - supertypes: BTreeMap>, + supertypes: BTreeMap, #[serde(default)] named: BTreeMap>>, #[serde(default)] unnamed: Vec, } +#[derive(Deserialize)] +#[serde(untagged)] +enum YamlSupertype { + Subtypes(Vec), + Detailed { + subtypes: Vec, + #[serde(default)] + fields: BTreeMap, + }, +} + +impl YamlSupertype { + fn subtypes(&self) -> &[TypeRef] { + match self { + Self::Subtypes(subtypes) | Self::Detailed { subtypes, .. } => subtypes, + } + } + + fn fields(&self) -> Option<&BTreeMap> { + match self { + Self::Subtypes(_) => None, + Self::Detailed { fields, .. } => Some(fields), + } + } +} + /// A reference to a node type. Can be: /// - a plain string (resolved by looking up named vs unnamed) /// - a map `{unnamed: "name"}` to force unnamed interpretation @@ -133,6 +165,41 @@ fn resolve_type_ref( json!({"type": kind, "named": named}) } +fn convert_fields( + fields: &BTreeMap, + named_types: &BTreeSet, + unnamed_types: &BTreeSet, +) -> ( + serde_json::Map, + Option, +) { + let mut json_fields = serde_json::Map::new(); + let mut json_children = None; + + for (raw_field_name, type_refs) in fields { + let spec = parse_field_name(raw_field_name); + let types: Vec<_> = type_refs + .clone() + .into_vec() + .iter() + .map(|t| resolve_type_ref(t, named_types, unnamed_types)) + .collect(); + let field_info = json!({ + "multiple": spec.multiple, + "required": spec.required, + "types": types, + }); + + if let Some(name) = spec.name { + json_fields.insert(name, field_info); + } else { + json_children = Some(field_info); + } + } + + (json_fields, json_children) +} + /// Convert YAML string to node-types JSON string. pub fn convert(yaml_input: &str) -> Result { let yaml: YamlNodeTypes = @@ -151,16 +218,26 @@ pub fn convert(yaml_input: &str) -> Result { let mut output = Vec::new(); // 1. Supertypes - for (name, members) in &yaml.supertypes { - let subtypes: Vec<_> = members + for (name, supertype) in &yaml.supertypes { + let subtypes: Vec<_> = supertype + .subtypes() .iter() .map(|m| resolve_type_ref(m, &named_types, &unnamed_types)) .collect(); - output.push(json!({ + let (fields, children) = supertype + .fields() + .map(|fields| convert_fields(fields, &named_types, &unnamed_types)) + .unwrap_or_default(); + let mut entry = json!({ "type": name, "named": true, "subtypes": subtypes, - })); + "fields": fields, + }); + if let Some(children) = children { + entry["children"] = children; + } + output.push(entry); } // 2. Named nodes @@ -186,32 +263,7 @@ pub fn convert(yaml_input: &str) -> Result { Some(m) => m, }; - let mut json_fields = serde_json::Map::new(); - let mut json_children: Option = None; - - for (raw_field_name, type_refs) in fields_map { - let spec = parse_field_name(raw_field_name); - let types: Vec<_> = type_refs - .clone() - .into_vec() - .iter() - .map(|t| resolve_type_ref(t, &named_types, &unnamed_types)) - .collect(); - - // Cloning to make the borrow checker happy - let field_info = json!({ - "multiple": spec.multiple, - "required": spec.required, - "types": types, - }); - - if let Some(name) = spec.name { - json_fields.insert(name, field_info); - } else { - // $children - json_children = Some(field_info); - } - } + let (json_fields, json_children) = convert_fields(fields_map, &named_types, &unnamed_types); let mut entry = json!({ "type": name, @@ -290,10 +342,7 @@ fn record_field_order(schema: &mut crate::schema::Schema, yaml_input: &str) -> R Ok(()) } -fn apply_yaml_to_schema( - yaml: &YamlNodeTypes, - schema: &mut crate::schema::Schema, -) { +fn apply_yaml_to_schema(yaml: &YamlNodeTypes, schema: &mut crate::schema::Schema) { // Register all supertypes as node kinds for name in yaml.supertypes.keys() { schema.register_kind(name); @@ -326,8 +375,9 @@ fn apply_yaml_to_schema( } let unnamed_types: BTreeSet = yaml.unnamed.iter().cloned().collect(); - for (supertype, members) in &yaml.supertypes { - let node_types = members + for (supertype, definition) in &yaml.supertypes { + let node_types = definition + .subtypes() .iter() .map(|m| { let (kind, named) = resolve_type_ref_pair(m, &named_types, &unnamed_types); @@ -355,7 +405,8 @@ fn apply_yaml_to_schema( .into_vec() .into_iter() .map(|type_ref| { - let (kind, named) = resolve_type_ref_pair(&type_ref, &named_types, &unnamed_types); + let (kind, named) = + resolve_type_ref_pair(&type_ref, &named_types, &unnamed_types); crate::schema::NodeType { kind, named } }) .collect::>(); @@ -427,7 +478,14 @@ pub fn convert_from_json(json_input: &str) -> Result { } } - let mut supertypes: BTreeMap> = BTreeMap::new(); + let mut supertypes: BTreeMap< + String, + ( + Vec, + BTreeMap, + Option, + ), + > = BTreeMap::new(); let mut named: BTreeMap>> = BTreeMap::new(); let mut unnamed: Vec = Vec::new(); @@ -438,7 +496,7 @@ pub fn convert_from_json(json_input: &str) -> Result { } if !node.subtypes.is_empty() { - supertypes.insert(node.kind, node.subtypes); + supertypes.insert(node.kind, (node.subtypes, node.fields, node.children)); continue; } @@ -463,11 +521,36 @@ pub fn convert_from_json(json_input: &str) -> Result { // Supertypes if !supertypes.is_empty() { writeln!(out, "supertypes:").unwrap(); - for (name, members) in &supertypes { + for (name, (members, fields, children)) in &supertypes { writeln!(out, " {name}:").unwrap(); - for member in members { - let ref_str = format_type_ref(&member.kind, member.named, &all_named, &all_unnamed); - writeln!(out, " - {ref_str}").unwrap(); + if fields.is_empty() && children.is_none() { + for member in members { + let ref_str = + format_type_ref(&member.kind, member.named, &all_named, &all_unnamed); + writeln!(out, " - {ref_str}").unwrap(); + } + } else { + writeln!(out, " subtypes:").unwrap(); + for member in members { + let ref_str = + format_type_ref(&member.kind, member.named, &all_named, &all_unnamed); + writeln!(out, " - {ref_str}").unwrap(); + } + writeln!(out, " fields:").unwrap(); + for (field_name, info) in fields + .iter() + .map(|(name, info)| (name.as_str(), info)) + .chain(children.iter().map(|info| ("$children", info))) + { + write_yaml_field( + &mut out, + " ", + field_name, + info, + &all_named, + &all_unnamed, + ); + } } } writeln!(out).unwrap(); @@ -484,29 +567,14 @@ pub fn convert_from_json(json_input: &str) -> Result { Some(fields) => { writeln!(out, " {name}:").unwrap(); for (field_name, info) in fields { - let suffix = field_suffix(info.multiple, info.required); - let yaml_name = if field_name == "$children" { - format!("$children{suffix}") - } else { - format!("{field_name}{suffix}") - }; - - let type_refs: Vec = info - .types - .iter() - .map(|t| format_type_ref(&t.kind, t.named, &all_named, &all_unnamed)) - .collect(); - - if type_refs.len() == 1 { - writeln!(out, " {yaml_name}: {}", type_refs[0]).unwrap(); - } else { - let list = type_refs - .iter() - .map(|s| s.as_str()) - .collect::>() - .join(", "); - writeln!(out, " {yaml_name}: [{list}]").unwrap(); - } + write_yaml_field( + &mut out, + " ", + field_name, + info, + &all_named, + &all_unnamed, + ); } } } @@ -525,6 +593,29 @@ pub fn convert_from_json(json_input: &str) -> Result { Ok(out) } +fn write_yaml_field( + out: &mut String, + indent: &str, + field_name: &str, + info: &JsonFieldInfo, + all_named: &BTreeSet, + all_unnamed: &BTreeSet, +) { + let suffix = field_suffix(info.multiple, info.required); + let yaml_name = format!("{field_name}{suffix}"); + let type_refs: Vec = info + .types + .iter() + .map(|t| format_type_ref(&t.kind, t.named, all_named, all_unnamed)) + .collect(); + + if type_refs.len() == 1 { + writeln!(out, "{indent}{yaml_name}: {}", type_refs[0]).unwrap(); + } else { + writeln!(out, "{indent}{yaml_name}: [{}]", type_refs.join(", ")).unwrap(); + } +} + fn field_suffix(multiple: bool, required: bool) -> &'static str { match (multiple, required) { (false, true) => "", @@ -678,6 +769,46 @@ unnamed: assert_eq!(end["named"], false); } + #[test] + fn test_supertype_fields() { + let yaml = r#" +supertypes: + callable: + subtypes: + - function + - closure + fields: + parameter*: parameter + body?: body + +named: + function: + closure: + parameter: + body: +"#; + + let json = convert(yaml).unwrap(); + let nodes: Vec = serde_json::from_str(&json).unwrap(); + let callable = nodes + .iter() + .find(|node| node["type"] == "callable") + .unwrap(); + assert_eq!(callable["subtypes"].as_array().unwrap().len(), 2); + assert_eq!(callable["fields"]["parameter"]["multiple"], true); + assert_eq!(callable["fields"]["parameter"]["required"], false); + assert_eq!(callable["fields"]["body"]["multiple"], false); + assert_eq!(callable["fields"]["body"]["required"], false); + + let round_trip = convert_from_json(&json).unwrap(); + assert!(round_trip.contains(" subtypes:")); + assert!(round_trip.contains(" fields:")); + assert_eq!( + serde_json::from_str::(&convert(&round_trip).unwrap()).unwrap(), + serde_json::from_str::(&json).unwrap(), + ); + } + #[test] fn test_explicit_unnamed_disambiguation() { let yaml = r#" diff --git a/shared/yeast-schema/src/schema.rs b/shared/yeast-schema/src/schema.rs index 6f5d3fe7d0a1..3ac2779703d9 100644 --- a/shared/yeast-schema/src/schema.rs +++ b/shared/yeast-schema/src/schema.rs @@ -277,13 +277,8 @@ impl Schema { .insert((parent_kind.to_string(), field_id), node_types); } - pub fn field_types( - &self, - parent_kind: &str, - field_id: FieldId, - ) -> Option<&Vec> { - self.field_types - .get(&(parent_kind.to_string(), field_id)) + pub fn field_types(&self, parent_kind: &str, field_id: FieldId) -> Option<&Vec> { + self.field_types.get(&(parent_kind.to_string(), field_id)) } /// Record the declared (named) field order for a node kind, as authored in diff --git a/shared/yeast/doc/node-types-yaml.md b/shared/yeast/doc/node-types-yaml.md index b887f5a82bbf..d6ce1212918c 100644 --- a/shared/yeast/doc/node-types-yaml.md +++ b/shared/yeast/doc/node-types-yaml.md @@ -50,7 +50,25 @@ This corresponds to the following JSON: } ``` -Members are resolved as named or unnamed using the +To declare fields that consumers may expose on the supertype, use the detailed +form with `subtypes` and `fields`: + +```yaml +supertypes: + callable: + subtypes: + - function + - closure + fields: + parameter*: parameter + body?: block +``` + +The field syntax and multiplicity suffixes are the same as for named nodes. +These fields describe the common interface of the supertype; they do not add +storage to the supertype or its members. + +Members and field types are resolved as named or unnamed using the [type reference rules](#type-references) described below. ## Named nodes diff --git a/unified/extractor/ast_types.yml b/unified/extractor/ast_types.yml index b293820d9cd8..251f45037199 100644 --- a/unified/extractor/ast_types.yml +++ b/unified/extractor/ast_types.yml @@ -64,13 +64,17 @@ supertypes: - do_while_stmt - labeled_stmt callable: - - top_level - - function_expr - - function_declaration - - constructor_declaration - - destructor_declaration - - accessor_declaration - - initializer_declaration + subtypes: + - top_level + - function_expr + - function_declaration + - constructor_declaration + - destructor_declaration + - accessor_declaration + - initializer_declaration + fields: + parameter*: parameter + body?: block # A member is anything that can appear in the body of a class-like declaration member: - constructor_declaration diff --git a/unified/ql/lib/codeql/unified/internal/Ast.qll b/unified/ql/lib/codeql/unified/internal/Ast.qll index ea296e4d3dd7..9aa32c8e7959 100644 --- a/unified/ql/lib/codeql/unified/internal/Ast.qll +++ b/unified/ql/lib/codeql/unified/internal/Ast.qll @@ -116,12 +116,12 @@ module Unified { final F::Identifier getNameNode() { unified_accessor_declaration_def(this, _, result) } /** Gets the node corresponding to the field `parameter`. */ - final F::Parameter getParameter(int i) { + final override F::Parameter getParameter(int i) { unified_accessor_declaration_parameter(this, i, result) } /** Gets the node corresponding to the field `parameter`. */ - final F::Parameter getAParameter() { result = this.getParameter(_) } + final override F::Parameter getAParameter() { result = this.getParameter(_) } /** Gets the node corresponding to the field `type`. */ final F::Expr getType() { unified_accessor_declaration_type(this, result) } @@ -360,7 +360,13 @@ module Unified { class Callable extends @unified_callable, F::AstNode { /** Gets the node corresponding to the field `body`. */ - abstract F::Block getBody(); + F::Block getBody() { none() } + + /** Gets the node corresponding to the field `parameter`. */ + F::Parameter getParameter(int i) { none() } + + /** Gets the node corresponding to the field `parameter`. */ + F::Parameter getAParameter() { none() } } /** A class representing `catch_clause` nodes. */ @@ -498,12 +504,12 @@ module Unified { final F::Identifier getNameNode() { unified_constructor_declaration_name_node(this, result) } /** Gets the node corresponding to the field `parameter`. */ - final F::Parameter getParameter(int i) { + final override F::Parameter getParameter(int i) { unified_constructor_declaration_parameter(this, i, result) } /** Gets the node corresponding to the field `parameter`. */ - final F::Parameter getAParameter() { result = this.getParameter(_) } + final override F::Parameter getAParameter() { result = this.getParameter(_) } /** Gets a field or child node of this node. */ final override F::AstNode getAFieldOrChild() { @@ -689,12 +695,12 @@ module Unified { final F::Identifier getNameNode() { unified_function_declaration_def(this, result) } /** Gets the node corresponding to the field `parameter`. */ - final F::Parameter getParameter(int i) { + final override F::Parameter getParameter(int i) { unified_function_declaration_parameter(this, i, result) } /** Gets the node corresponding to the field `parameter`. */ - final F::Parameter getAParameter() { result = this.getParameter(_) } + final override F::Parameter getAParameter() { result = this.getParameter(_) } /** Gets the node corresponding to the field `return_type`. */ final F::Expr getReturnType() { unified_function_declaration_return_type(this, result) } @@ -750,10 +756,12 @@ module Unified { final F::Modifier getAModifier() { result = this.getModifier(_) } /** Gets the node corresponding to the field `parameter`. */ - final F::Parameter getParameter(int i) { unified_function_expr_parameter(this, i, result) } + final override F::Parameter getParameter(int i) { + unified_function_expr_parameter(this, i, result) + } /** Gets the node corresponding to the field `parameter`. */ - final F::Parameter getAParameter() { result = this.getParameter(_) } + final override F::Parameter getAParameter() { result = this.getParameter(_) } /** Gets the node corresponding to the field `return_type`. */ final F::Expr getReturnType() { unified_function_expr_return_type(this, result) }