Skip to content

Conversation

@dduugg
Copy link
Contributor

@dduugg dduugg commented Dec 16, 2025

sorbet-runtime does not work on sigs defined on module_function methods, when called as class methods: sorbet/sorbet#8531

tapioca also fails to include type information for the class method versions when generating gem shims: Shopify/tapioca#2000

AFAICT, the instance version of methods created with module_function are not used, so this PR proposes using the class definitions only. (This ofc may not be true for dependents, so this is very much a breaking change.)

There are some other light changes for compatibility, which i'll comment on below.

# or nil if it can't find anything
sig { params(klass: T.nilable(T.any(T::Class[T.anything], Module))).returns(T.nilable(String)) }
def path_from_klass(klass)
sig { params(klass: T.nilable(T.any(T::Class[T.anything], T::Module[T.anything]))).returns(T.nilable(String)) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module is generic in recent sorbet releases, and required at typed: strict: sorbet/sorbet#9580

@@ -1,4 +1,4 @@
# typed: false
# typed: strict
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an easy type promotion 📈

let(:codes_team) { instance_double(CodeTeams::Team) }

before do
allow(codes_team).to receive(:is_a?).with(CodeTeams::Team).and_return(true)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed bc runtime typechecking is enabled as a result of removing module_function. (See sorbet/sorbet#8531 linked in the PR summary.)

spec.add_dependency 'code_teams', '~> 1.0'
spec.add_dependency 'packs-specification'
spec.add_dependency 'sorbet-runtime', '>= 0.5.11249'
spec.add_dependency 'sorbet-runtime', '>= 0.6.12763'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more recent release is necessary for T::Module to resolve (see comment below).

@dduugg dduugg marked this pull request as ready for review December 16, 2025 20:59
@dduugg dduugg force-pushed the dug/no-module-function branch from 4fc00a6 to 67bb6ea Compare December 16, 2025 21:04
@dduugg dduugg force-pushed the dug/no-module-function branch from 67bb6ea to 07c898b Compare December 16, 2025 21:04
@martinemde martinemde merged commit 82bebc5 into rubyatscale:main Dec 17, 2025
9 checks passed
@github-project-automation github-project-automation bot moved this from Triage to Done in Modularity Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants