Missing `ActiveRecord#save` signature

[iMacTia]

I was expecting to find save and save! in the methods generated by https://github.com/Shopify/tapioca/blob/main/lib/tapioca/dsl/compilers/active_record_relations.rb, together with build, create and all the others, but for some reason it's missing.

Is it somewhere else, or was it simply missed?
And if it's missing, would you be open to a PR to add it there?

[paracycle]

1. build, create and others are generated via the constant definition here and processing here.
2. save and save! are not relation methods, they are ActiveRecord::Persistence methods

[paracycle]

Sorry, I meant to add the following but hit the button too quickly:

It could be that you are calling save on a relation and it is working at runtime, since ActiveRecord::Relation delegates missing methods to the base model class. There is no good way to model wholesale method delegation with Sorbet, so Tapioca doesn't even try to do it. Sorbet won't be able to see those method calls even with Tapioca generated RBI files.

If you have a different problem, though, please reopen the issue with more details.

[iMacTia]

Thank you @paracycle, I should have provided a better example.
I have the following method in my application:

sig { returns(MyModel) }
def process
  record = MyModel.find_by(...)
  # make some changes to the record...
  record.save!
end

The signature is obviously wrong, because save! either returns a boolean or raises an error, but srb tc still passes without any issues. My understanding is that this is because save! is somehow considered to return T.untyped, hence whatever type I put as return type, it will work, but this way it makes it impossible to check this kind of methods.

I've been searching for where save! was defined in RBIs and couldn't find it, so I decided to ask.
Is it possible to deal with this scenario correctly? What is missing in this case?

[paracycle]

I've been searching for where save! was defined in RBIs and couldn't find it, so I decided to ask.
Is it possible to deal with this scenario correctly? What is missing in this case?

Ah, ok, the example makes it more clear. Tapioca does not generate a DSL RBI method for save or save! since they are inherited by models from ActiveRecord::Base, which has the method on it (well, a module mixed in to it, but same thing) statically. You can see that the save! method has been generated in the Tapioca codebase as part of the ActiveRecord::Persistence module here.

Since this is a method on a module mixed into the base class, so there is no way to properly generate a signature for these methods, since the return type could depend on the concrete type of the model. Moreover, Tapioca never generates these methods as part of the DSL RBI generation either, since these methods are not generated at runtime, which is what DSL compilers do.

Because, the save! method in the gem RBI does not have a signature, the return type is T.untyped and that's why you are having your problem.

I see two solutions to this:

1. You can add a shim RBI file (i.e. a hand written RBI file) that defines the save! method on MyModel and defines a signature for it, or
2. You can write a DSL compiler that will process all AR model classes and generate save! methods with the correct signature for all of them.

The last option would work, but you could get into conflicting method signature problems and is generally against the spirit of why DSL compilers exist in the first place.

[iMacTia]

Thank you so much @paracycle for the detailed explanation 🙏, this makes much more sense!
I was ready to explore the second solution you highlighted above, but you seem to indicate this is not the correct one.

The only thing I don't understand is how save and save! are any different from build, create and all the other ActiveRecord method that are already correctly generated by tapioca in one of the compilers with the correct signature.
Why is it harder/against the spirit to do the same for save and save! 🤔 ?
I don't want to make any contribution that is not in the right direction/spirit of the project

[KaanOzkan]

The only thing I don't understand is how save and save! are any different from build, create and all the other ActiveRecord method that are already correctly generated by tapioca in one of the compilers with the correct signature.
Why is it harder/against the spirit to do the same for save and save! 🤔 ?

@iMacTia I'm mostly rephrasing @paracycle's response but it may be of help. dsl command's job is to expose metaprogramming to Sorbet. Relations compiler's documentation can paint a more detailed picture. It exposes methods to ActiveRecord::Base by representing the underlying relationships to Sorbet using synthetic entities. Methods such as build aren't available to Sorbet without this compiler due to metaprogramming. They exist in ActiveRecord::Relation which isn't statically mixed into ActiveRecord::Base.

On the other hand save and save! are methods available to Sorbet after a "simpler" gem RBI generation since Sorbet knows that ActiveRecord::Base includes ActiveRecord::Persistence. All tapioca is doing is loading the gems and writing the constant definitions, method definitions and the mixin information to a file (docs). You can check the RBI generated for activerecord and find

class ActiveRecord::Base
  include ::ActiveRecord::Persistence
  ...

You can also find the definitions for save and save! in the same file under the ActiveRecord::Persistence module.
This means we already have gem RBIs generated for these methods unlike build/create.

Now if you write a DSL generator to improve the signature of save for each Model in your application it would conflict with the method definitions in gem RBIs since there will be 2 definitions for the same method. It's also against the spirit since the methods aren't defined by metaprogramming, which was why dsl command was introduced. save/save! are statically available to ActiveRecord::Base and don't need any extra processing unlike build/create.

Also one thing to note is that if you went the DSL compiler route it doesn't have to live inside tapioca. You can write your custom compiler and tapioca will execute it during dsl command.

Hope this clears things up a bit.

[iMacTia]

Thank you @KaanOzkan, now I understand what @paracycle has been trying to tell me, I really didn't get it the first time, sorry!
It makes sense now, and I see why the DSL compiler is a bad idea in this case.

If I look at another case though, I'm not entirely sure this holds (unless again I'm missing something): first! is statically defined in ActiveRecord::Relation::FinderMethods (activerecord//lib/active_record/relation/finder_methods.rb#128) and sorbet seems to know it returns the correct type.
Looking into my project RBI, I can see a generic def first!; end in sorbet/rbi/gems/activerecord@6.1.6.1.rbi, but this is then "overridden" by a properly typed RBI under sorbet/rbi/dsl/my_model.rb.
What's the difference in this case? Shouldn't first! be unknown to sorbet like save/save!?

As to why Sorbet thinks that save/save! return T.untyped, let's see if I understand correctly:

• My model inherits from ActiveRecord, which includes ActiveRecord::Relation.
• sorbet checks the activerecord.rbi file and finds def save; end
• Since there's no explicit signature in the RBI file, sorbet assumes that the return type is T.untyped

If the above is correct, I guess the issue lies with the ActiveRecord RBI file?
What would be the correct fix in this case? Should the community have a shared annotation for ActiveRecord::Relation that tapioca can use?

Sorry for bringing up even more questions 😅 I really want to understand the missing link and potentially contribute so that others can avoid this issue in future.

[KaanOzkan]

Nothing wrong with asking questions 🙂

What's the difference in this case? Shouldn't first! be unknown to sorbet like save/save!?

first! is similar to build. It's defined in FinderMethods as you pointed out which is then included in ActiveRecord::Relation. So similarly to build it only exists in ActiveRecord::Relation which isn't statically mixed into ActiveRecord::Base. The correct type comes from the overriden DSL RBI.

As to why Sorbet thinks that save/save! return T.untyped, let's see if I understand correctly

Yes all of those points are correct.

What would be the correct fix in this case? Should the community have a shared annotation for ActiveRecord::Relation that tapioca can use?

Issue is the metaprogramming makes it hard for us to represent this with an annotation. first! is supposed to return an instance of your model. For example, Foo. We can't write a shareable annotation since Foo is specific to your application. That's why Ufuk suggested using a shim to type it only for your application. Unfortunately it's a tedious process if you have many models but I think it's the best solution.

If the signature wasn't specific to your application you'd be correct, we could use something like rbi-central and share the signature.

I see two solutions to this:
1 - You can add a shim RBI file (i.e. a hand written RBI file) that defines the save! method on MyModel and defines a signature for it, or
2 - You can write a DSL compiler that will process all AR model classes and generate save! methods with the correct signature for all of them.

I'd say go with 1 if it's not a frequently recurring investment, ie. you don't have new models being added every week. Then the initial cost of the shims is worth it which is already simple enough.

Otherwise you can explore the option number 2 and have a custom DSL compiler live inside your application. This would automate the "shim" process however, adds maintenance work.

And final option is being satisfied with T.untyped. A good balance could be adding a shim for one or two models that heavily utilize these methods and keep the rest untouched.

[iMacTia]

first! is similar to build. It's defined in FinderMethods as you pointed out which is then included in ActiveRecord::Relation. So similarly to build it only exists in ActiveRecord::Relation which isn't statically mixed into ActiveRecord::Base. The correct type comes from the overriden DSL RBI.

Thanks again @KaanOzkan, I believe I was getting lost with the includes but thanks to your explanation this now makes sense again.
first!, build and all the others are fine thanks to the DSL compiler, which is also the correct solution there!

I unfortunately misplaced my questions about the correct fix:

What would be the correct fix in this case? Should the community have a shared annotation for ActiveRecord::Relation that tapioca can use?

Here I was referring to save/save!, so I should have mentioned ActiveRecord::Persistence rather than ActiveRecord::Relation, sorry about the confusion!
But I suspect you gave me enough details to answer my own questions now, let's see if I got it:

• ActiveRecord::Persistence is statically mixed, which means we shouldn't be using a DSL compiler
• However, tapioca is unaware of the return type of that function, so it assumes T.untyped
• We know, though, that save returns true/false and save! returns true (or raises an exception), which makes it possible (and easy!) to annotate them

So, I believe the next step is to open a PR against rbi-central to add an annotation for save/save! as the return type is not specific to my application and can easily be generalised.

@paracycle @KaanOzkan does that make sense? If so, I'd be happy to contribute the PR myself 😄!

[KaanOzkan]

@iMacTia I think you're correct. @paracycle mentioned save/save!'s return type could depend on the concrete type of the model. However, it does look like it returns a boolean or an exception unless I'm missing something. If that's the case yes let's annotate and share it in rbi-central. If it's specific to the concrete model type then we could only shim or a DSL compiler to automate it.

[iMacTia]

Thank you @KaanOzkan, I can confirm that save! returns true or raises an error (see doc), while save returns true/false by internally calling create_or_update (save doc, create_or update doc).

I'll open a PR against rbi-central!

One final question 🥁
IF we assume for a moment that save/save! returned the record itself, shouldn't it still be possible to type-check using T.self_type?

[KaanOzkan]

Yes that's correct. My bad earlier, I forgot about that one. I can't think of an edge case that invalidates T.self_type so it's a good replacement to returning record itself given the usage is sensible. Keep in mind though in some more complex cases receiver could be T.untyped and therefore return type would also be T.untyped.

[iMacTia]

Thanks again for coping with all my questions @KaanOzkan 🙏 !

FYI -- I've prepared a PR to fix this, but when I test it locally with my Rails app (manually update the annotation), the new signature is not picked up 😢.
I suspect the problem is that sorbet is assuming the save method is actually processed by some other override (there are so many!) like newrelic or another gem.

I haven't been able to find which method sorbet thinks is being called, but this definitely complicates things 😅