-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Rails generators #605
Conversation
57adc0e
to
c81faa4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I resolved the style nits and left the other two threads open with comments. Thanks!
c81faa4
to
e288250
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of including the big, messy diff, I included only the pieces that I needed
Feel free to include the big messy diff for railtie.rbi
, since that's what tapioca outputted.
However, since this gem RBI update PR hasn't touched railtie
I'm thinking that you didn't supply the --doc
option? If that's the case please supply that and commit the RBI as a whole.
e288250
to
5eb1d91
Compare
Rails generators are application code, though not part of your main Rails application. The files are not automatically loaded when starting your app because Railties loads them when you run the generate command. However, since it's part of your application, it's helpful to be able to enable strong typing within those files. Without knowing about the Thor-based DSL, you're left writing shims yourself. Having support within Tapioca means that you won't have to manage this manually any more.
5eb1d91
to
026e310
Compare
50d9ca4
to
026e310
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not in love with the BUILT_IN_MATCHER
gymnastics, but I guess this is the best way of excluding internal classes, for now.
Thanks for working on this and putting this together. Much appreciated.
Motivation
Rails generators are application code, though not part of your main Rails application. The files are not automatically loaded when starting your app because Railties loads them when you run the generate command.
However, since it's part of your application, it's helpful to be able to enable strong typing within those files. Without knowing about the Thor-based DSL, you're left writing shims yourself.
Having support within Tapioca means that you won't have to manage this manually any more.
Implementation
I load the two most likely candidate files. There are others that we could load, for example:
rails/generators/active_model
rails/generators/erb
rails/generators/generated_attribute
I didn't want to get into the rabbit hole of trying to load all of them because that would be brittle between Rails versions. Do you think that's the correct choice?
Also, in order to avoid generating RBIs for built-in classes, I opted for pattern matching on the class name. That doesn't feel great, but I wasn't sure what would be a better approach. There are two main base classes,
Rails::Generators::NamedBase
andRails::Generators::Base
, but there are also some in gems other thanrailties
. Do you have suggestions on a better solution?I had to add some RBIs forI rewrote the whole file.railties
for type-checking since I use them in the tests. It appears that a prior version of Tapioca (or maybe Parlour?) wrote the existing file and that it used to include comments. Instead of including the big, messy diff, I included only the pieces that I needed. This leaves the file looking inconsistent because there are chunks in the new style and the rest is in the old. Is that okay, or should I rewrite the whole file?Tests
Yep, there are tests for corner cases that I could think of.