Filter prepended-into-ancestors modules in ActionControllerHelpers DSL compiler#2597
Open
atraver-shopify wants to merge 1 commit intomainfrom
Open
Filter prepended-into-ancestors modules in ActionControllerHelpers DSL compiler#2597atraver-shopify wants to merge 1 commit intomainfrom
atraver-shopify wants to merge 1 commit intomainfrom
Conversation
When a module is prepended into another ancestor in the helpers chain (e.g. `DEBUGGER__::TrapInterceptor` prepended into `::Kernel` by debug.gem, or `ActiveSupport::CoreExt::ERBUtil*` prepended into `::ERB::Util` by ActiveSupport), it appears in `mod.ancestors` even though the user never explicitly included it. Emitting `include ::X` for those modules pollutes generated RBIs and, in the DEBUGGER case, causes Sorbet to fail with "Unable to resolve constant DEBUGGER__" because debug.gem is not typechecked. This shows up in practice when the Ruby LSP Tapioca add-on regenerates DSL RBIs inside a ruby-lsp process that has `debug/session` loaded -- the chain is visible in the LSP but not in plain-shell invocations of `bin/tapioca dsl`, which is very confusing to diagnose. Fix: in `gather_includes`, compute the set of modules that appear in any ancestor's prepend chain and exclude them from the output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
When
debug/sessionis loaded in the Ruby process that runs theActionControllerHelpersDSL compiler, generated controller RBIs gain a spuriousinclude ::DEBUGGER__::TrapInterceptorline. Sorbet then fails with:This is a direct consequence of how
debug.geminstalls itself:That
prependinsertsTrapInterceptorbefore::Kernelin every module's ancestor chain that traverses::Kernel. TheActionControllerHelperscompiler walks_helpers.ancestorsand emits aninclude ::Xfor each entry, so the prepended module ends up in the generated RBI even though no user wroteinclude ::DEBUGGER__::TrapInterceptor.In practice this is hit whenever the Tapioca Ruby LSP add-on (
lib/ruby_lsp/tapioca/addon.rb) triggers in-process DSL regeneration inside aruby-lspprocess that hasdebug/sessionloaded. It's especially confusing because the same DSL command run from a plain shell produces a clean RBI — the bad output depends on what's loaded into the host process.The same mechanism produces quieter leaks for any module prepended into something the user's helper chain touches. For example, ActiveSupport does:
Those two modules show up in committed RBIs as
include ::ActiveSupport::CoreExt::ERBUtilandinclude ::ActiveSupport::CoreExt::ERBUtilPrivatein any Rails app whose helpers chain reaches::ERB::Util. They're semantically wrong — no user wroteincludefor them — but because the referenced modules are typechecked, nothing has flagged the lines so far.Implementation
In
Tapioca::Dsl::Compilers::ActionControllerHelpers#gather_includes, walk the target module's ancestors and precompute the set of modules that appear in another ancestor's prepend chain (i.e. appear before that ancestor in its own.ancestors). Reject those from the emittedincludelist.This matches the intent of the compiler: it is translating explicit
helper X/include Xcalls intoinclude ::XRBI lines. Prepended modules weren't included by the user and should not appear.Manual reproduction
On
main, in any Rails app where at least one helper resolves::Kernelin its ancestor chain:With this patch the bad include is absent and
srb tcis clean.Tests
Added a new case in
spec/tapioca/dsl/compilers/action_controller_helpers_spec.rbthat:debug/session, mirroring the in-editor scenario where the Ruby LSP Tapioca add-on runs in a process with the debugger loaded.include Kernel(the realistic case where Kernel ends up in theHelperMethodsancestor chain).DEBUGGER__.The test fails on
mainand passes with this change.bin/test,bin/style, andbin/typecheckare all clean.Note for downstream users
Existing committed DSL RBIs in Rails apps may currently contain
include ::ActiveSupport::CoreExt::ERBUtil,include ::ActiveSupport::CoreExt::ERBUtilPrivate, or other prepend-side-effect includes accumulated over multiple Rails/ActiveSupport versions. After this change,bin/tapioca dsl --verifywill flag those lines as stale. The resolution is a one-timebin/tapioca dsl+ commit to clean them up. No functional impact — those lines were always semantically wrong; they just weren't producing Sorbet errors because the referenced modules were themselves typechecked.