Debugging notes for required params after optional params error #5403
Conversation
I spent a bunch of time today debugging an error I was seeing on a Rails 7 app, where we were getting this error and it was pointing here: https://github.com/rails/rails/blob/2459c20afb508c987347f52148210d874a9af4fa/activerecord/lib/active_record/persistence.rb#L405 Eventually, with the help of @jeffcarbs and @paracycle I tracked it down to a `def self.update!` in an Active Record model. This method had a `sig { void }` but it seems like for some reason the names matching still triggered this error. I'm not sure how to fix that or if it's even a bug, but I thought some debugging notes might help the next person to come across it.
|
This passes statically but raises at runtime: # typed: true
require "sorbet-runtime"
module A
def foo(x = 1, y)
end
end
class B
extend T::Sig
extend A
sig { params(z: String).void }
def self.foo(z)
end
end
B.foo("hi")As does this # typed: true
require "sorbet-runtime"
class A
def self.foo(x = 1, y)
end
end
class B < A
extend T::Sig
sig { params(z: String).void }
def self.foo(z)
end
end
B.foo("hi")Would you consider that a bug? |
|
@jez not sure if you saw ^ but let me know if you'd like me to add that to https://github.com/sorbet/sorbet/tree/master/gems/sorbet-runtime/test somewhere |
|
@ghiculescu I think this is the call that is ultimately causing that error. In your first example, This is arguably a bug, as in, theoretically, it can be better handled by Sorbet runtime, wrt to super methods. |
|
Yes, I think you're right. It sounds like maybe we want to add some more scenarios to this path? |
I spent a bunch of time today debugging an error I was seeing on a Rails 7 app, where we were getting this error and it was pointing here: https://github.com/rails/rails/blob/2459c20afb508c987347f52148210d874a9af4fa/activerecord/lib/active_record/persistence.rb#L405
Eventually, with the help of @jeffcarbs and @paracycle I tracked it down to a
def self.update!in an Active Record model. This method had asig { void }but it seems like for some reason the names matching still triggered this error.I'm not sure how to fix that or if it's even a bug, but I thought some debugging notes might help the next person to come across it. If it is considered a bug, happy to provide more info to help with replication.