Refactor DSL compilers

[paracycle]

Motivation

This is a breaking and backwards-incompatible change to how DSL compilers are structured. We are moving from creating a single instance of each compiler for the whole pipeline run to creating a new compiler instance for each constant that is processed. This allows the compiler interface to be cleaner (since there are less things that need to be passed to decorate) and the compiler to hold state if it wants. In particular, the fact that each compiler had to pass constant between its methods was a smell for me.

The general interface for a DSL compiler is changing from:

class Foo < Tapioca::Compiler::Dsl::Base
  sig { override.params(root: RBI::Tree, constant: T.class_of(FooDsl)).void }
  def decorate(root, constant)
  end

  sig { override.returns(T::Enumerable[Module]) }
  def gather_constants
  end
end

to

class Foo < Tapioca::Dsl::Compiler
  ConstantType  = type_member(fixed: T.class_of(FooDsl))

  sig { override.void }
  def decorate
  end

  sig { override.returns(T::Enumerable[Module]) }
  def self.gather_constants
  end
end

Implementation

1. Make the relevant change in the Compiler base class
2. Adapt the compilers to adhere to the new structure
3. Change the DslCompiler helper to adapt to the new structure
4. Change how errors are collected now that compiler instances are created and destroyed dynamically.

Tests

No added tests, refactored existing ones.

[vinistock]

Why do we need the tap here? Wouldn't it be enough to invoke compare_by_identity straight away?

[paracycle]

I think when I originally wrote this, I thought that compare_by_identity didn't return self, so that's why. I can update this.

[vinistock]

Is it intentional that we're including and extending the same module? Is there any way we could consolidate these?

[paracycle]

This is intentional, since we want gather_constants methods to have access to Reflection methods too, but that method is on the singleton class now.

[paracycle]

Not sure if there is a better way to do this, though.

[vinistock]

Yeah, I guess it'll be hard to escape this. Just checking.

[jeffcarbs]

It looks like the PR description has a small typo in the "new" code. I think it should be:

-def gather_constants
+def self.gather_constants

Might be worth updating for posterity since (like I did) some folks may come to this PR to see how to refactor their custom DSL compilers.

[paracycle]

It looks like the PR description has a small typo in the "new" code. I think it should be:

-def gather_constants
+def self.gather_constants

Might be worth updating for posterity since (like I did) some folks may come to this PR to see how to refactor their custom DSL compilers.

Thanks for pointing this out @jeffcarbs, fixed the code in the description.

[ericfirth]

When I did this, I started getting this error in my DSL Generators

sorbet/tapioca/generators/devise_methods.rb:10: Type ConstantType declared by parent Tapioca::Dsl::Compiler must be re-declared in DeviseMethods https://srb.help/5014
    10 |class DeviseMethods < Tapioca::Dsl::Compiler
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    sorbet/rbi/gems/[email protected]:586: ConstantType declared in parent here
     586 |  ConstantType = type_member(upper: Module)

If I added ConstantType = type_member(upper: Module) to the Generator class it worked, but I don't really know what that is, why I have to do it and why its not on the above example. Curious if anyone knows.

Does the code sample need to look like:

class Foo < Tapioca::Dsl::Compiler
  ConstantType = type_member(upper: Module)
  Elem  = type_member(fixed: T.class_of(FooDsl))

  sig { override.void }
  def decorate
  end

  sig { override.returns(T::Enumerable[Module]) }
  def self.gather_constants
  end
end

?

[Morriar]

👋 Hey Eric,

We renamed Elem into ConstantType in #819.

I believe this is enough:

class Foo < Tapioca::Dsl::Compiler
  ConstantType = type_member(fixed: T.class_of(FooDsl))

  sig { override.void }
  def decorate
  end

  sig { override.returns(T::Enumerable[Module]) }
  def self.gather_constants
  end
end

Hope this helps.

[paracycle]

I updated the example in the description to reflect that the type member name is ConstantType.