WIP: Properly attribute dynamically-generated methods #1984
Conversation
This file contains 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
We used to treat methods that didn't have a source location the same as methods that explicitly were not defined in the current gem. That resulted in Tapioca creating more method definitions than necessary. We would only skip method generation for a method if the constant it was on was an ignored type (i.e. a built-in type), so that we wouldn't keep redefining methods for built-in types. However, for all other types, especially types that come from other gems, we would just keep on generating all the methods regardless of if they were defined by this gem or not. Moreover, the source location check was happening at the wrong location, before unwrapping the method signature. Thus, many methods with signatures would not be generated when the previous problem was fixed, since our code would see them as being defined in Sorbet runtime. The fix is to return a more fine-grained result from `method_in_gem?` which signals yes/no/don't-have-source-location. Based on that we can skip generating don't-have-source-location cases if they are for built-in types and totally ignore the methods that have a source location and are definitely not defined in the current gem. Additionally, if we try to unwrap the method signature and we get an exception, that means the signature block raised an error. If we continue with the method as is, the source location checks would think the method definition does not belong in the gem (since the method is still wrapped), and we would thus skip the method generation. To avoid that, the `signature_for` method is now raising a custom exception to signal that exceptional case, so that we can at least continue generating "a" method definition.
…efinitions that are triggered by one gem but defined in another
egiurleo
changed the base branch from
main
to
uk-method-generation-improvements
paracycle
force-pushed
the
uk-method-generation-improvements
branch
from
6ee7756 to
a11f4ce
Compare
paracycle
force-pushed
the
uk-method-generation-improvements
branch
from
d2329a9 to
7b05810
Compare
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
Implementation
Tests