chanzuckerberg / sorbet-rails

A set of tools to make the Sorbet typechecker work with Ruby on Rails seamlessly.
MIT License
638 stars 81 forks source link

Add cursed methods to Rails models #106

Open connorshea opened 5 years ago

connorshea commented 5 years ago

These are all a mess and I'd prefer not to implement them, but I wanted to at least document them here, since they're taking up a lot of my hidden.rbi.

Is there a way to remove these from hidden.rbi without adding a bunch of useless methods to these classes?

Association Callback Methods

For every attribute on a model that has a relation with any other model, ActiveRecord/ActiveModel will have 12 methods. For example, let's say you add a relation on a Post that assigns a User as an author. Rails would create these methods:

# before add
def before_add_for_author; end
def before_add_for_author?; end
def before_add_for_author=(val); end

# before remove
def before_remove_for_author; end
def before_remove_for_author?; end
def before_remove_for_author=(val); end

# after add
def after_add_for_author; end
def after_add_for_author?; end
def after_add_for_author=(val); end

# after remove
def after_remove_for_author; end
def after_remove_for_author?; end
def after_remove_for_author=(val); end

Defined here: https://github.com/rails/rails/blob/5-2-stable/activerecord/lib/active_record/associations/builder/collection_association.rb#L32

These have barely any usage on GitHub, so I'm honestly not sure why they haven't been deprecated, but they do technically exist.

Dirty Methods

These only found their way into my hidden.rbi with Rails 6, but it seems like they've been there since at least 5.2? I'm not really sure why Sorbet couldn't find them before.

In 6.0 they're defined here: https://github.com/rails/rails/blob/v6.0.0.rc2/activemodel/lib/active_model/dirty.rb#L124-L126

If you declare an attribute name, you get these:

def name_change(*args); end
def name_changed?(*args); end
def name_previous_change(*args); end
def name_previously_changed?(*args); end
def name_was(*args); end
def name_will_change!(*args); end
def restore_name!(*args); end

There are also a few defined here: https://github.com/rails/rails/blob/v6.0.0.rc2/activerecord/lib/active_record/attribute_methods/dirty.rb#L19-L26

def saved_change_to_name?(*args); end
def saved_change_to_name(*args); end
def name_before_last_save(*args); end
def will_save_change_to_name?(*args); end
def name_change_to_be_saved(*args); end
def name_in_database(*args); end

And two here: https://github.com/rails/rails/blob/v6.0.0.rc2/activerecord/lib/active_record/attribute_methods/before_type_cast.rb#L32-L33

def name_before_type_cast(*args); end
def name_came_from_user?(*args); end

So that's 15 methods added per attribute (and this isn't counting the setter, getter, or asker), fun!

connorshea commented 5 years ago

Here's another fun one:

https://github.com/rails/rails/blob/96289cfb9b6aeb8f1a917f892148fd47f2f2049a/activerecord/lib/active_record/autosave_association.rb#L180

connorshea commented 5 years ago

And there are also _ids getters and setters:

https://github.com/rails/rails/blob/b343beba03722672b9bb827f8ce29c7c1c216406/activerecord/lib/active_record/associations/builder/collection_association.rb#L55

module Game::GeneratedAssociationMethods
  def game_genre_ids(); end

  def game_genre_ids=(ids); end

  def game_platform_ids(); end

  def game_platform_ids=(ids); end

  def game_publisher_ids(); end

  def game_publisher_ids=(ids); end

  def game_purchase_ids(); end

  def game_purchase_ids=(ids); end
end
connorshea commented 5 years ago

And reload_:

module Game::GeneratedAssociationMethods
  def reload_cover_attachment(); end

  def reload_cover_blob(); end

  def reload_pg_search_document(); end

  def reload_series(); end
end

https://github.com/rails/rails/blob/fc35da76e93f8a5d5ace595b4819e19cc0512edd/activerecord/lib/active_record/associations/builder/singular_association.rb#L19

As well as build_/create_:

module Game::GeneratedAssociationMethods
  def build_cover_attachment(*args, &block); end

  def build_pg_search_document(*args, &block); end

  def build_series(*args, &block); end

  def create_cover_attachment(*args, &block); end

  def create_cover_attachment!(*args, &block); end

  def create_pg_search_document(*args, &block); end

  def create_pg_search_document!(*args, &block); end

  def create_series(*args, &block); end

  def create_series!(*args, &block); end
end

https://github.com/rails/rails/blob/fc35da76e93f8a5d5ace595b4819e19cc0512edd/activerecord/lib/active_record/associations/builder/singular_association.rb#L28-L38

hdoan741 commented 5 years ago

Since they aren't really used, there is little value to generate them, except to clean up hidden.rbi file. If you'd like to clean these up properly, feel free to contribute a plugin that generate the methods, but we don't add the plugin to the default list.

connorshea commented 4 years ago

I've created a working plugin (although everything is essentially untyped): https://github.com/connorshea/VideoGameList/blob/9c9f1d0d1fb2394103fad05b77295007c0a7c2ae/lib/cursed_rbi_plugin.rb

It removed a little over 5k lines out of my 35k line hidden.rbi :)

@manhhung741 do you still want this? :) Should I rename it from 'CursedRbiPlugin'? And how should the user enable its usage?

hdoan741 commented 4 years ago

Yes. I think it's useful to have it as a plugin, for completeness. We don't have to make it a default plugin, but can add a line in Readme to tell people they can enable it.

On Tue, Oct 8, 2019 at 6:11 PM Connor Shea notifications@github.com wrote:

I've created a working plugin (although everything is essentially untyped): https://github.com/connorshea/VideoGameList/blob/9c9f1d0d1fb2394103fad05b77295007c0a7c2ae/lib/cursed_rbi_plugin.rb

It removed a little over 5k lines out of my 35k line hidden.rbi :)

@manhhung741 https://github.com/manhhung741 do you still want this? :) Should I rename it from 'CursedRbiPlugin'? And how should the user enable its usage?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/chanzuckerberg/sorbet-rails/issues/106?email_source=notifications&email_token=AAFH4APWDDBF4APYGUOZTELQNUVSLA5CNFSM4IIZV3Z2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAWERJA#issuecomment-539773092, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFH4AL7NWBGOTRRWIAIK4DQNUVSLANCNFSM4IIZV3ZQ .

hdoan741 commented 4 years ago

We should rename it :P IDK what's a good name though, something like AllOtherUselessMethodThatRailsAddedPlugin lol (j/k)

On Tue, Oct 8, 2019 at 6:20 PM Harry Doan hdoan@chanzuckerberg.com wrote:

Yes. I think it's useful to have it as a plugin, for completeness. We don't have to make it a default plugin, but can add a line in Readme to tell people they can enable it.

On Tue, Oct 8, 2019 at 6:11 PM Connor Shea notifications@github.com wrote:

I've created a working plugin (although everything is essentially untyped): https://github.com/connorshea/VideoGameList/blob/9c9f1d0d1fb2394103fad05b77295007c0a7c2ae/lib/cursed_rbi_plugin.rb

It removed a little over 5k lines out of my 35k line hidden.rbi :)

@manhhung741 https://github.com/manhhung741 do you still want this? :) Should I rename it from 'CursedRbiPlugin'? And how should the user enable its usage?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/chanzuckerberg/sorbet-rails/issues/106?email_source=notifications&email_token=AAFH4APWDDBF4APYGUOZTELQNUVSLA5CNFSM4IIZV3Z2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAWERJA#issuecomment-539773092, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFH4AL7NWBGOTRRWIAIK4DQNUVSLANCNFSM4IIZV3ZQ .

connorshea commented 4 years ago

Something to do with metaprogramming, probably. Maybe MetaprogrammingGarageSalePlugin? :P