Idea: RuboCop-style plugin API for Solargraph (def_node_matcher, single AST pass)

[lekemula]

Idea: write Solargraph plugins like RuboCop extensions

I've been experimenting with a different shape for Solargraph plugin authoring in lekemula/solargraph-rspec#33, and I'd love to get reactions before pushing it further.

The idea

Make writing a Solargraph plugin feel the same as writing a RuboCop extension:

• Plugins declare AST node patterns with def_node_matcher / def_node_search from rubocop-ast and bind them to handler methods.
• Processors self-register on inherited — no manual wiring.
• A single walker owns AST traversal and a shared context hash that processors can read/write to cooperate without coupling to each other.

Example processor:

class AttributeProcessor < Solargraph::Walker::BaseProcessor
  def_node_matcher :attribute_definition, <<~PATTERN
    (send nil? :attribute (sym $_) (const ... $_))
  PATTERN

  on_node_type :send, :process_attribute

  private

  def process_attribute(node)
    name, type = attribute_definition(node)
    return unless name

    add_method_pin(name: name.to_s, return_type: type.to_s, location: node.loc)
  end
end

Why I think this is worth it

• Readability/maintainability. Patterns like '(block (send _ #let_block_method? ...) ...)' replace brittle manual traversals. Intent is visible at a glance.
• Lower barrier to contribution. The Ruby community already knows the RuboCop extension model. Reusing it makes the project far more inviting to people writing plugins (solargraph-rails, solargraph-rspec, etc.).
• Better AI assistance. Thousands of existing RuboCop rules give LLMs excellent examples to draw on for new patterns.
• Single-pass parsing. Today each plugin walks the AST independently. With one walker dispatching to all registered processors, Solargraph + every plugin would traverse the tree only once.

What this fixes today — real examples from solargraph-rspec main

The status quo is reaching into Parser::AST::Node children by integer index. A few snippets straight from the current main branch:

1. "Get the let method name" — five chained safe-navigations, plus a RuboCop disable

# lib/solargraph/rspec/spec_walker/node_types.rb
def self.let_method_name(block_ast)
  return nil unless a_block?(block_ast)

  block_ast.children[0].children[2]&.children&.[](0)&.to_s # rubocop:disable Style/SafeNavigationChainLength
end

To read this you have to hold the full (block (send _ _ (sym :name)) (args) (body)) shape in your head and remember that children[0].children[2] is the first arg of the inner send, whose first child is the :sym. With a node pattern the same lookup is:

def_node_matcher :let_name, '(block (send _ _ (sym $_) ...) ...)'

2. The walker reinvented (badly) pattern matching

The current Walker::Hook carries its own positional matcher with a special :any token, because plain Parser::AST::Node gives you nothing better:

def match_children(children, args = @args)
  args.each_with_index.all? do |arg, i|
    if arg == :any
      true
    elsif children[i].is_a?(::Parser::AST::Node) && arg.is_a?(Symbol)
      children[i].type == arg
    else
      children[i] == arg
    end
  end
end

That's a (worse) reimplementation of what def_node_matcher already provides for free — and every Solargraph plugin that needs more than "match by type" tends to grow its own variant of it.

3. Magic indices spread across helpers

block_ast.children[0].children[1].to_s  # method name of the receiver send
block_ast.children[0].children[2]       # first argument of the receiver send
block_ast.children[2]                   # block body

Nothing in the source tells a reader which index means what. If the parser gem ever changes a node shape, or you want to support a slightly different call form, every magic index is a separate place to update.

4. Hand-built fake AST nodes for "let → def" transformation

# lib/solargraph/rspec/spec_walker/fake_let_method.rb
::Parser::AST::Node.new(
  :def,
  [
    method_name.to_sym,
    ::Parser::AST::Node.new(:args, []),
    block_ast.children[2]
  ]
)

Fine in isolation, but the children list is a positional contract that only a parser-gem expert can sanity-check. With rubocop-ast builders you'd get a typed node back (DefNode) and the same code reads as (def name (args) body).

None of these are "bad code" in isolation — they're what you end up writing when the only API is "an array of children with magic indices". A shared rubocop-ast-based plugin layer would let every plugin author skip this entire category of code.

The concern: parser → rubocop-ast bridging

Solargraph's parser produces Parser::AST::Node, but rubocop-ast patterns need RuboCop::AST::Node. In the experiment I bridge this with a recursive conversion:

def to_rubocop_ast(node)
  return node unless node.is_a?(::Parser::AST::Node)
  return node if node.is_a?(::RuboCop::AST::Node)

  converted_children = node.children.map { |child| to_rubocop_ast(child) }
  BUILDER.n(node.type, converted_children, node.loc)
end

RuboCop::AST::Node is a subclass of Parser::AST::Node, so this is mostly a re-wrap — but it's still a full recursive pass over every analyzed file, which partly defeats the "parse once" goal above.

I think this could be fixed upstream in rubocop-ast. ProcessedSource already has a pre-parsed escape hatch for Prism (PrismPreparsed). A similar ParserPreparsed-style entry point would let Solargraph hand rubocop-ast its already-built AST with the right builder and skip the conversion entirely. Solargraph already depends on rubocop transitively, so the dependency surface wouldn't change.

Expected changes in Solargraph

Concretely, I'd expect the work in Solargraph itself to be scoped to a parallel reimplementation of lib/solargraph/parser/parser_gem/node_processors — the per-node-type processors that build pins during source mapping — written against rubocop-ast nodes and registered alongside the existing ones, so the two paths can run side by side under a flag while the new pipeline is validated.

Because RuboCop::AST::Node is a subclass of Parser::AST::Node, the surrounding plumbing (chains, region tracking, location info, pin shapes) should keep working unchanged — that's the only part I expect to stay the same. Everything that today reaches into children[i] by index would migrate to typed accessors / node patterns eventually, as wanted.

What I'd like to know

1. Is a RuboCop-style plugin API directionally interesting to maintainers, or does it conflict with how Solargraph wants plugins to evolve?
2. Would you accept patches that switch the internal walker to rubocop-ast nodes, given the conversion cost can be addressed upstream?
3. Any prior discussion on this I should be aware of?

Happy to break this up into smaller proposals if it helps.

---

🤖 Drafted with Claude Code; the idea and concerns are mine.

[lekemula]

@apiology curious for your take on how this would land in solargraph-rails. The walker implementation here was originally borrowed from there, and the manual AST traversal it carried over has been bugging me ever since.

[lekemula]

Update on "The concern: parser → rubocop-ast bridging": turns out the bridging isn't needed.

Solargraph already parses through Prism::Translation::Parser with its own builder (FlawedBuilder < Parser::Builders::Default). Changing its superclass to RuboCop::AST::Builder makes the parser emit RuboCop::AST::Nodes directly: no recursive conversion pass, and no upstream ParserPreparsed-style entry point in rubocop-ast.

Since RuboCop::AST::Node subclasses Parser::AST::Node, the rest of solargraph keeps working: the full spec suite passes unchanged on RuboCop nodes, and the tree shapes are identical (the emit_* flags rubocop-ast's builder sets don't affect Prism's translator). The cost is in parsing: lib/ takes ~0.23s instead of ~0.20s to parse.

I put this together in #1386 (draft), along with the walker and the node processors rewritten on top of node patterns.

---

🤖 Drafted with Claude Code; the findings are verified on the branch.