support `module_function def`

[jesse-shopify]

Summary

spoom srb sigs translate converts Sorbet sig blocks into inline RBS comments (#:). When a sig sits above a module_function def, the translation produces code Sorbet rejects: the method stops being a module function, disappears from the module's singleton, and every caller fails to typecheck.

Playground links below carry ?arg=--enable-experimental-rbs-comments&arg=--parser=prism. Both flags are required — without them Sorbet ignores every #: comment and reports a misleading 7017 ("does not have a sig") instead of the real error.

Reproduction

Before — typechecks clean, Config.fetch("a") is String:

# typed: strict
module Config
  extend T::Sig

  sig { params(key: String).returns(String) }
  module_function def fetch(key)
    key
  end
end

Config.fetch("a")

▶ Run in Sorbet playground — clean

Run bundle exec spoom srb sigs translate --from rbi --to rbs, which yields:

  #: (String key) -> String
  module_function def fetch(key)

▶ Run in Sorbet playground — 3505 + 7003

srb tc now reports:

Bad argument to `module_function`: must be a symbol, string, method definition, or nothing  https://srb.help/3505
Method `fetch` does not exist on `T.class_of(Config)`                                       https://srb.help/7003

Expected

Either leave module_function def untouched, or emit a form Sorbet accepts. This variant is verified to typecheck green, with Config.fetch("a") resolving to String:

module_function

#: (String key) -> String
def fetch(key)

▶ Run in Sorbet playground — Revealed type: String

Impact

This was the highest-impact translator failure of three found while translating Shopify's Ruby monorepo (19,197 files rewritten): two module_function def sites produced 11 of the run's 14 errors — 2 at the definitions, 9 at callsites spread across unrelated files, one of them a test for a different component.

The failure is non-local. One broken definition emits one error in its own file and one error per callsite, wherever those live. A reviewer reading the diff of the translated file sees nothing wrong with it.

Environment

spoom 1.8.7, sorbet / sorbet-static 0.6.13386, rbs 4.1.1, prism 1.9.0, Ruby 4.0. Also confirmed on sorbet 0.6.13351.

Related

#777 — "Cannot translate rbi to rbs if there is a keyword before def", closed by #779. That is the same construct in the rbs→rbi direction, and that fix taught the translator to handle a keyword before def — which is precisely what breaks here going the other way. Worth reading alongside.

#1012 — a different translator bug with the same shape of resolution: detect the construct spoom can't translate faithfully and report it, rather than emitting something that changes meaning.

Workaround

Replace module_function def foo with a bare module_function plus a plain def foo. Semantics are identical for a module where every method is a module function.

[Morriar]

I think the better fix is in Sorbet: sorbet/sorbet#10568

[Morriar]

sorbet/sorbet#10568 was merged 👍