Confused over `persisted?` type for `ActiveModel`

[G-Rath]

I've been exploring using RBS with RubyMine, and found an oddity with how the persisted? type is defined for ActiveModel:

Here it is defined as returning a bool, which is what I'd expect:

gem_rbs_collection/gems/activemodel/7.1/activemodel-7.1.rbs

Line 18 in 3a33fa6

def persisted?: () -> bool

However here it is typed as returning false:

gem_rbs_collection/gems/activemodel/7.1/activemodel-generated.rbs

Line 1867 in 3a33fa6

def persisted?: () -> false

It seems that the latter definition "wins" in the eyes of RubyMine causing it flag anything that isn't false:

I personally come from a TypeScript background and have been involved in their version of RBS (DefinitelyTyped), so I'm happy to be involved in improving the types here but since this is the first time I'm really digging into RBS I want to make sure I understand how the types are managed especially these Rails ones that are suffixed with -generated.

Would it be correct to just remove the definition from the -generated file, or is there something else that would need adjusting?

[G-Rath]

Looking back over this, I've realized that the types I've referenced are from different modules (API vs Model) which is probably relevant, though ActiveModel::Model includes ActiveModel::API which does not seem to be reflected here?

Anyway, I'm suspecting that what's happened is a manual generation of the types was attempted by a tool that saw persisted? was implemented to return false and so used that as the type when it should just be bool.

I'm happy to do a PR changing that if my assessment is correct

[ksss]

In this case, I think the application should describe the type of Mx#persisted?.

class Mx
  def persisted?: () -> bool
end

[G-Rath]

@ksss could you elaborate why you think that?

In my experience it's usually not considered a good thing for child classes to widen the return type of methods defined on their parents, as that defeats the point of duck typing, and that's why RubyMine flagged this in the first place

[ksss]

@G-Rath You are right, it might be an incorrect state from a type system perspective.

Here are the reasons why I thought types should be written on the application side:

• Whether a subclass implements a method of the parent class with awareness is left to the implementer.
• Only the application implementer knows the application's behavior.
• The return type of false can sometimes be useful for narrowing.
  * Type narrowing union types via return type of method call soutaro/steep#1497

Here is what I found through my research:

Note that, by default, ActiveModel::API implements persisted? to return false, which is the most common case. You may want to override it in your class to simulate a different scenario:

https://api.rubyonrails.org/classes/ActiveModel/API.html#module-ActiveModel::API-label-Active+Model+API

If it is convenient in many cases, the change can be accepted.
Our criterion is not whether it is correct as a type system, but whether it is practical.

[G-Rath]

Ok cool, so it sounds like you're open to a OR changing this - if that's true, I can do one first thing tomorrow morning