Should we support jobs with specific arguments?

[jaredbeck]

Describe the bug:

When hidden.rbi contains something like this,

class DeleteBooksJob
  def perform(*args, &blk); end
end

(I'll refer to these as "non-specific" arguments.)

And jobs/delete_books_job.rb looks like this,

  sig {
    params(
      acting_user_id: Integer,
      book_ids: T::Array[Integer]
    )
  }
  def perform(acting_user_id, book_ids)

(I'll refer to these as "specific" arguments.)

then srb produces an error

be srb t
sorbet/rbi/hidden-definitions/hidden.rbi:8786: Method DeleteBooksJob#perform redefined without matching argument count. Expected: 2, got: 1 https://srb.help/4010
    8786 |  def perform(*args, &blk); end
            ^^^^^^^^^^^^^^^^^^^^^^^^
    app/jobs/delete_books_job.rb:13: Previous definition
    13 |  def perform(acting_user_id, book_ids)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Errors: 1

My question is:

Should we support jobs with specific arguments, according to my definition of "specific" given above?

Steps to reproduce:

rails new sorbet_rails_issue
bin/rails g model books title:string
bin/rails db:migrate
bin/rails g job delete_books
# install sorbet, then sorbet-rails according to standard instructions

Edit jobs/delete_books_job.rb as follows:

# typed: false

class DeleteBooksJob < ApplicationJob
  extend T::Sig
  queue_as :default

  sig {
    params(
      acting_user_id: Integer,
      book_ids: T::Array[Integer]
    ).void
  }
  def perform(acting_user_id, book_ids)
    # Do something later
  end
end

Notice the typed: false.

Run srb, see error:

sorbet/rbi/hidden-definitions/hidden.rbi:8786: Method DeleteBooksJob#perform redefined without matching argument count. Expected: 2, got: 1 https://srb.help/4010
    8786 |  def perform(*args, &blk); end
            ^^^^^^^^^^^^^^^^^^^^^^^^
    app/jobs/delete_books_job.rb:13: Previous definition
    13 |  def perform(acting_user_id, book_ids)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expected behavior:

srb should not produce error.

Versions:

• Ruby: 2.6.5
• Rails: 6.0.1
• Sorbet: 0.4.5133
• Sorbet-Rails: 0.5.8.1

[jaredbeck]

My question is: Should we support jobs with specific arguments, according to my definition of "specific" given above?

If the answer is no, that's fine, but then it seems job files will not be able to have a sigil higher than ignore.

[hdoan741]

I think if you re-run srb rbi hidden-definitions, it might remove the "non-specific" sig generated by sorbet.
Rerunning hidden-definitions task may be slow :P You can consider removing the sig manually.

Those are sorbet-specific behavior. For sorbet-rails, I think we can generate perform_later and perform_now class methods based on the manually written signature, much like how we generate delivery method for mailers. If you're interested in doing that, I can give you more direction on how to do it. It should be simple because sorbet-rails already has a library method that extracts sorbet signature from a method

[hdoan741]

Rerunning hidden-definitions task may be slow :P You can consider removing the sig manually.

I mean removing the sig from hidden-definitions.rbi file. It's just an auto-generated file of dynamic methods that sorbet detected in the codebase.

https://sorbet.org/docs/rbi#the-hidden-definition-rbi

[jaredbeck]

I think if you re-run srb rbi hidden-definitions, it might remove the "non-specific" sig generated by sorbet. .. Those are sorbet-specific behavior.

Aha! It does.

So, the lesson is: after I change a sigil from ignore to false (or greater) I should re-run hidden-definitions. That makes sense. When a file is ignored it is not required, thus anything defined in it is "hidden". Thanks, Harry.

I will make a PR to add this tip to the sigil page of the sorbet docs.

For sorbet-rails, I think we can generate perform_later and perform_now class methods based on the manually written signature, much like how we generate delivery method for mailers. .. If you're interested in doing that, I can give you more direction ..

Sure, sounds like fun. I'll open a new issue to discuss.