ruby / rbs

Type Signature for Ruby
Other
1.96k stars 215 forks source link

Add AST Visitor #1975

Closed Morriar closed 2 months ago

Morriar commented 2 months ago

The Visitor class implements the Visitor pattern for traversing the RBS Abstract Syntax Tree (AST).

It provides methods to visit each type of node in the AST, allowing for custom processing of each node type.

This class is designed to be subclassed, with specific visit methods overridden to implement custom behavior for different node types.

Example usage:

class MyVisitor < RBS::AST::Visitor
  def visit_declaration_class(node)
    puts "Visiting class: #{node.name}"

    super # call `super` to run the default visiting behavior
  end
end

visitor = MyVisitor.new
visitor.visit(ast_node)

I needed something like this and couldn't find anything in the repo other than the EnvironmentWalker that is way to specialized for my purpose.

ParadoxV5 commented 2 months ago

-1. This visitor is a bloat to RBS’s infrastructure.

❌ You need to update all visitors each time a class gets added to or removed from the element hierarchy. ⸺ https://refactoring.guru/design-patterns/visitor

An (not necessarily the) idiomatic design for Ruby is the simple enumerator.

# https://github.com/ruby/rbs/blob/v3.5.2/sig/parser.rbs#L67-L69
_buffer, _directives, declarations = RBS::Parser.parse_signature RBS_STRING
declarations.each do|node|
  case node
  when RBS::AST::Declarations::Class
    puts "Visiting class: #{node.name}"
    # node.each_member / each_decl …
  # …
  end
end

Not to mention that Ruby doesn’t have abstract classes.

Sometimes patterns are also used because of specific inflexibilities of e.g. Java ;-) ⸺ Lapizistik, https://ptb.discord.com/channels/518658712081268738/766466740069859349/1268121025195933801

soutaro commented 2 months ago

I don't think providing a visitor is a bad idea, while I understand technically tree walking can be easily implemented with the pattern matching and recursive calls. I know there are some features that can be easily implemented on the top of visitor, without boring pattern matching enumeration.

One concern I have is requiring #accept methods in all of the AST classes. How about implementing the down-cast logics in the visitor class? I know it's different from the traditional visitor pattern, but it's possible and I'm curious if having the logic in one place makes more sense.

ParadoxV5 commented 2 months ago

How about implementing the down-cast logics in the visitor class?

This means the visitor class itself will be the boring pattern matching enumeration.

Morriar commented 2 months ago

@ParadoxV5

An (not necessarily the) idiomatic design for Ruby is the simple enumerator

As shown by your own example, the simple enumerator doesn't absolve you to write the burdensome visit logic hidden in your # node.each_member / each_decl … comment.

The goal of the visitor is exactly to not have you write this logic yourself and focus on what you want to do with the nodes.

Not to mention that Ruby doesn’t have abstract classes

Thank you for being so pedantic. By abstract I meant the base visitor doesn't provide anything more than the visit logic, it's up to the subclass to do something with each node.

This pattern tremendously simplifies the implementation of any tree walker and keeps things nicely organized in classes and methods rather than a long case..when statement. It's just a better practice.

@soutaro

How about implementing the down-cast logics in the visitor class?

Good idea, I moved the logic in Visitor#visit to avoid the need for an accept method in each node class.

ParadoxV5 commented 2 months ago

This pattern tremendously simplifies the implementation of any tree walker and keeps things nicely organized in classes and methods rather than a long case..when statement.

Fair. Not all tree walkers care about every class and method, but most probably do.