Repository navigation
Conversation
The RBI parser records a bare `private` or `protected` as a standalone node and leaves the following methods public. The merge in `exported_rbi_tree` drops those standalone nodes, so methods declared only in an exported RBI were generated as public. Flatten each exported file's visibilities onto its methods before merging.
This branch has not been deployed
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 a gem exports RBIs, methods declared under a bare
privateorprotectedsection come out public in the generated gem RBI if tapioca doesn't also see them at runtime.The RBI parser records a bare
privateas a standaloneRBI::Privatenode and leaves the following methods with public visibility.RBI::Rewriters::Mergeonly carries over scopes and indexable nodes, soGemfile::GemSpec#exported_rbi_treedrops those visibility nodes when it merges the exported files.In this repo's own bundle,
rubydexhits this since it is a native extension and many of its methods only come from its exported RBI. Today we generate publicself.newon six classes whose exported RBI makes it private (Rubydex::Declaration,Rubydex::Definition,Rubydex::Document, and others), public helpers inRubydex::Linter::Helpers::PathHelpers, and a publicRubydex::Linter::CustomRule#add_diagnosticthat the gem declaresprotected.I ran into this while reviewing #2743, whose private method filter can't see these methods.
Implementation
Call
flatten_visibilities!on each parsed exported file before merging it, so every method carries its own visibility, which is what tapioca's pipeline produces and what the formatter'snest_non_public_membersexpects at print time.I kept this in tapioca rather than changing
RBI::Rewriters::Merge: bare visibility is positional, so the merge can't keep it meaningfully once nodes are reordered, andrbialready providesflatten_visibilities!for consumers that need per-method visibility.Method#compatible_with?doesn't compare visibility, so this can't introduce new merge conflicts.Tests
Extended the existing "must generate a gem RBI with the ones exported from the gem by default" spec with a method declared only under a bare
privatein an exported RBI. It fails without the change (the method is generated as public) and passes with it. Regeneratingrbi,spoom,prism, andrubydexwith and without the change only differs forrubydex, as described above.