What if the formatter was just a Prism visitor?

[joeldrapper]

Hey Kevin, you’ve probably already considered this, but I was just reading through this project and wondering if it might be simpler and/or easier to maintain if the formatter was just a Prism visitor.

1. You wouldn't need to maintain a separate set of AST nodes in addition to Prism’s nodes.
   1. Edit (Correction): It looks like you're actually patching Prism’s existing AST nodes rather than defining new ones, so maybe this isn’t such a strong point.
      1. Edit again: I still think there's an advantage to not having to monkey-patch a core Ruby library and also to having a continuous formatting context in the visitor as opposed to separate format methods. The whole thing, or any part of it, is swappable without having to patch further and add more conditionals.
2. It would be easy for consumers to subclass with overrides and customizations.
3. You could easily pass the buffer/indentation level (or some context object holding both of these) back and forth between specialised formatters. This would be particularly helpful for transpilers / pre-processors.

I wrote a tiny prototype that can just about format modules and classes, though I don’t know for sure if this pattern will scale to cover everything.

I’ve also added some primitive formatting tools like indentation, but I think you could easily bring over tools like seplist, etc. and maybe build those into an abstract Prism visitor formatter class or module mixin.

class Formatter < Prism::BasicVisitor
  def initialize(buffer = +"", indentation_level = 0)
    @buffer = buffer
    @indentation_level = indentation_level
  end

  def self.format(source)
    new.tap { |v| v.visit(Prism.parse(source).value) }.buffer
  end

  attr_accessor :buffer

  def visit_program_node(node)
    visit_all(node.child_nodes)
  end

  def visit_statements_node(node)
    visit_all(node.child_nodes)
  end

  def visit_module_node(node)
    push "module "
    visit(node.constant_path)

    if node.body
      block {
        visit node.body
      }
    else
      new_line
    end

    push "end"
  end

  def visit_constant_read_node(node)
    push node.name
  end

  def visit_constant_path_node(node)
    visit node.parent
    push "::"
    visit node.child
  end

  def visit_class_node(node)
    push "class "
    visit node.constant_path

    if node.superclass
      push " < "
      visit node.superclass
    end

    if node.body
      block {
        visit node.body
      }
    else
      new_line
    end

    push "end"
  end

  def visit_def_node(node)
    push "def "
    push node.name

    visit node.parameters

    if node.body
      block { visit node.body }
    else
      new_line
    end

    push "end"
  end

  def visit_parameters_node(node)
    push "("
    visit_all node.child_nodes
    push ")"
  end

  def visit_required_parameter_node(node)
    push node.name
  end

  def visit_local_variable_read_node(node)
    push node.name
  end

  private

  def block
    indent {
      new_line
      yield
    }
    new_line
  end

  def indent
    @indentation_level += 1
    yield
    @indentation_level -= 1
  end

  def new_line
    @buffer << "\n" << ("  " * @indentation_level)
  end

  def push(str)
    @buffer << str.to_s
  end
end

In a different project, I found having the option of passing a block to visit_each, to be yielded between each item was really useful.

def visit_each(nodes)
  total = nodes.size
  i = 0
  while i < total
    visit(nodes[I])
    i += 1
    yield if (i < total) && block_given?
  end
end

This worked like seplist, e.g.

visit_each(node.child_nodes) { push ", " }

[kddnewton]

Yeah for sure, I could see the value in that. I've written it this way just to get it working, but my plan was to experiment with making it a visitor once the tests pass. I'm also wondering if I should effectively inline prettier_print. Once Syntax Tree is back to just being a formatter, the name of the game is exclusively going to be formatting speed, so whichever option is fastest is going to win.

I'm going to close this for now, because we're still a ways off, but I'll reopen once I'm ready to experiment with it.

[joeldrapper]

Sounds great @kddnewton! I’m very excited about this project. Thanks for all your work.

[kddnewton]

@joeldrapper great call on this. I ended up going with a visitor after your suggestion, and I really like how it turned out. Thanks!