Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 126 additions & 1 deletion rust/ruby-rbs/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,16 +185,135 @@ fn write_visit_trait(file: &mut File, config: &Config) -> Result<(), Box<dyn std
writeln!(file, "}}")?;
writeln!(file)?;

// Map C field types (e.g. `rbs_type_name`) to the corresponding
// visitor method name (e.g. `type_name` -> `visit_type_name_node`).
let visitor_method_names: std::collections::HashMap<String, String> = config
.nodes
.iter()
.map(|node| {
let c_type = convert_name(&node.name, CIdentifier::Type);
let c_type = c_type.strip_suffix("_t").unwrap_or(&c_type).to_string();
let method = convert_name(node.variant_name(), CIdentifier::Method);
(c_type, method)
})
.collect();

let is_visitable = |c_type: &str| -> bool {
matches!(c_type, "rbs_node" | "rbs_node_list" | "rbs_hash")
|| visitor_method_names.contains_key(c_type)
};

for node in &config.nodes {
let node_variant_name = node.variant_name();
let method_name = convert_name(node_variant_name, CIdentifier::Method);

writeln!(file, "#[allow(unused_variables)]")?; // TODO: Remove this once all nodes that need visitor are implemented
let has_visitable_fields = node
.fields
.iter()
.flatten()
.any(|field| is_visitable(&field.c_type));

if !has_visitable_fields {
// If there's nothing to visit in this node, write the empty method with
// underscored parameters, and skip to the next iteration
writeln!(
file,
"pub fn visit_{method_name}_node<V: Visit + ?Sized>(_visitor: &mut V, _node: &{node_variant_name}Node) {{}}"
)?;

continue;
}

writeln!(
file,
"pub fn visit_{}_node<V: Visit + ?Sized>(visitor: &mut V, node: &{}Node) {{",
method_name, node_variant_name
)?;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to remove the allow(unused_variables) above.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check to verify if there are visitable fields. If there are, we no longer generate this allow attribute.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of prefixing the parameters with an underscore instead of turning off the lint? Something like this:

if !has_visitable_fields {
  // If there's nothing to visit in this
  // node, write the empty method with
  // the parameters using an underscore
  // and skip to the next iteration
  writeln!(
    file,
    "pub fn visit_{}_node<V: Visit + ?Sized>(_visitor: &mut V, _node: &{}Node) {{}}",
    method_name, node_variant_name
  )?;

  continue;
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I like that. I remember you mentioning it and it totally slipped my mind. I'll change it.

if let Some(fields) = &node.fields {
for field in fields {
let field_method_name = if field.name == "type" {
"type_"
} else {
field.name.as_str()
};

match field.c_type.as_str() {
"rbs_node" => {
if field.optional {
writeln!(
file,
" if let Some(item) = node.{field_method_name}() {{"
)?;
writeln!(file, " visitor.visit(&item);")?;
writeln!(file, " }}")?;
} else {
writeln!(file, " visitor.visit(&node.{field_method_name}());")?;
}
}

"rbs_node_list" => {
if field.optional {
writeln!(
file,
" if let Some(list) = node.{field_method_name}() {{"
)?;
writeln!(file, " for item in list.iter() {{")?;
writeln!(file, " visitor.visit(&item);")?;
writeln!(file, " }}")?;
writeln!(file, " }}")?;
} else {
writeln!(file, " for item in node.{field_method_name}().iter() {{")?;
writeln!(file, " visitor.visit(&item);")?;
writeln!(file, " }}")?;
}
}

"rbs_hash" => {
if field.optional {
writeln!(
file,
" if let Some(hash) = node.{field_method_name}() {{"
)?;
writeln!(file, " for (key, value) in hash.iter() {{")?;
writeln!(file, " visitor.visit(&key);")?;
writeln!(file, " visitor.visit(&value);")?;
writeln!(file, " }}")?;
writeln!(file, " }}")?;
} else {
writeln!(
file,
" for (key, value) in node.{field_method_name}().iter() {{"
)?;
writeln!(file, " visitor.visit(&key);")?;
writeln!(file, " visitor.visit(&value);")?;
writeln!(file, " }}")?;
}
}

_ => {
if let Some(visit_method_name) = visitor_method_names.get(&field.c_type) {
if field.optional {
writeln!(
file,
" if let Some(item) = node.{field_method_name}() {{"
)?;
writeln!(
file,
" visitor.visit_{visit_method_name}_node(&item);"
)?;
writeln!(file, " }}")?;
} else {
writeln!(
file,
" visitor.visit_{visit_method_name}_node(&node.{field_method_name}());"
)?;
}
}
}
}
}
}
writeln!(file, "}}")?;
writeln!(file)?;
}
Expand Down Expand Up @@ -226,6 +345,12 @@ fn generate(config: &Config) -> Result<(), Box<dyn Error>> {
writeln!(file, "}}\n")?;

writeln!(file, "impl {} {{", node.rust_name)?;
writeln!(file, " /// Converts this node to a generic node.")?;
writeln!(file, " #[must_use]")?;
writeln!(file, " pub fn as_node(self) -> Node {{")?;
writeln!(file, " Node::{}(self)", node.variant_name())?;
writeln!(file, " }}")?;

if let Some(fields) = &node.fields {
for field in fields {
match field.c_type.as_str() {
Expand Down
109 changes: 109 additions & 0 deletions rust/ruby-rbs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,4 +326,113 @@ mod tests {
panic!("Expected TypeAlias with RecordType");
}
}

#[test]
fn visitor_test() {
struct Visitor {
visited: Vec<String>,
}

impl Visit for Visitor {
fn visit_bool_type_node(&mut self, node: &BoolTypeNode) {
self.visited.push("type:bool".to_string());

crate::visit_bool_type_node(self, node);
}

fn visit_class_node(&mut self, node: &ClassNode) {
self.visited.push(format!(
"class:{}",
String::from_utf8(node.name().name().name().to_vec()).unwrap()
));

crate::visit_class_node(self, node);
}

fn visit_class_instance_type_node(&mut self, node: &ClassInstanceTypeNode) {
self.visited.push(format!(
"type:{}",
String::from_utf8(node.name().name().name().to_vec()).unwrap()
));

crate::visit_class_instance_type_node(self, node);
}

fn visit_class_super_node(&mut self, node: &ClassSuperNode) {
self.visited.push(format!(
"super:{}",
String::from_utf8(node.name().name().name().to_vec()).unwrap()
));

crate::visit_class_super_node(self, node);
}

fn visit_function_type_node(&mut self, node: &FunctionTypeNode) {
let count = node.required_positionals().iter().count();
self.visited
.push(format!("function:required_positionals:{count}"));

crate::visit_function_type_node(self, node);
}

fn visit_method_definition_node(&mut self, node: &MethodDefinitionNode) {
self.visited.push(format!(
"method:{}",
String::from_utf8(node.name().name().to_vec()).unwrap()
));

crate::visit_method_definition_node(self, node);
}

fn visit_record_type_node(&mut self, node: &RecordTypeNode) {
self.visited.push("record".to_string());

crate::visit_record_type_node(self, node);
}

fn visit_symbol_node(&mut self, node: &SymbolNode) {
self.visited.push(format!(
"symbol:{}",
String::from_utf8(node.name().to_vec()).unwrap()
));

crate::visit_symbol_node(self, node);
}
}

let rbs_code = r#"
class Foo < Bar
def process: ({ name: String, age: Integer }, bool) -> void
end
"#;

let signature = parse(rbs_code.as_bytes()).unwrap();

let mut visitor = Visitor {
visited: Vec::new(),
};

visitor.visit(&signature.as_node());

assert_eq!(
vec![
"class:Foo",
"symbol:Foo",
"super:Bar",
"symbol:Bar",
"method:process",
"symbol:process",
"function:required_positionals:2",
"record",
"symbol:name",
"type:String",
"symbol:String",
"symbol:age",
"type:Integer",
"symbol:Integer",
"type:bool",
],
visitor.visited
);
}
}
Loading