Flatten namespaces when outputting RBI files

[dorner]

Addresses #11.

Rather than making this an option, I opted to change the behavior entirely. This is definitely a breaking change in terms of the output, but it should still be technically backwards compatible in that all output should result in the same objects. The main issue is that Tapioca (which is now the recommended way of handling RBI files) will rewrite any ingested RBI files into this format, and it can complain about things which this format doesn't (since constants can be referenced within the namespace). 

Flattening the output basically removes context and ambiguity and ensures the output is consistent.

Eager to hear feedback!

Here's a Slack thread from the Sorbet Slack for context:

Daniel Orner  3 days ago

Another question - not sure if this is expected behavior or not.
In my generated RBI, I have the following (eliding unnecessary lines):

module Deimos::ActiveRecordConsume::BatchConsumption
  sig { params(records: T::Array[Message]).returns(ActiveRecord::Relation) }
  def deleted_query(records); end
end

Then later on:

class Deimos::Message
...
end

When I run srb tc, I get the following error:

 Unable to resolve constant Message

It seems like Sorbet is not able to figure out to go up the namespace tree until it finds the actual namespace of the Message class the way Ruby itself does?

Daniel Orner  3 days ago
Moving Deimos::Message up above the other doesn’t seem to make a difference btw

jez  3 days ago

sorbet is accurately modeling ruby here

❯ cat foo.rb

module Deimos
  class Message
  end

  module ActiveRecordConsume
    module BatchConsumption
    end
  end
end

module Deimos::ActiveRecordConsume::BatchConsumption
  p(Message)
end

❯ ruby foo.rb
Traceback (most recent call last):
        1: from foo.rb:12:in `<main>'
foo.rb:13:in `<module:BatchConsumption>': uninitialized constant Deimos::ActiveRecordConsume::BatchConsumption::Message (NameError)
Did you mean?  Deimos::Message

Daniel Orner  3 days ago
Huh! I could have sworn I tested this out and it worked 

Daniel Orner  3 days ago
Thank you for the prompt assistance yet again 

jez  3 days ago

it will only work in ruby and sorbet if you define your module like

module Deimos
  module ActiveRecordConsume
    module BatchConsumption
      # ...

np

Daniel Orner  3 days ago
ohhhhhh that’s what I was missing

Daniel Orner  3 days ago
poifect, thanks

Daniel Orner  3 days ago

FYI - looks like the generated rbi that I packaged with the gem did it the “right” way (nesting the module) but the one generated by Tapioca seems to remove nesting and flatten everything. This means that I can run srb tc on the generated RBI and it looks fine, but when I include the gem in another project it fails. 

Daniel Orner  3 days ago

Yeah. Had to spend a bunch of time with sord making sure that every reference was fully namespaced. Even within the same module, e.g.

module Deimos
  class BatchRecordList
      sig { params(records: T::Array[BatchRecord]).void }
  def initialize(records); end
  end
end

module Deimos
  class BatchRecord
  end
end

still couldn’t find it since everything is splatted out by Tapioca.

Ufuk Kayserilioglu  3 days ago

as I mentioned in the other thread, the right thing to do is to fully qualify all constant references, like how Tapioca does. Sord should be able to do that based on the runtime information. Relying on relative constant resolution is brittle since the introduction of a completely unrelated constant can end up messing the whole resolution.

Daniel Orner  2 days ago

That’s a good point! Might see if I can take some time to update the Sord output to do this (actually I think it’s Parlour that does).