Shopify / tapioca

The swiss army knife of RBI generation
MIT License
701 stars 115 forks source link

Fix duplicate `all` signature in Active Record relations DSL compiler #1940

Closed bravehager closed 3 weeks ago

bravehager commented 3 weeks ago

Motivation

I think https://github.com/rails/rails/commit/fd5bd98b34774ec8ccb0fc649d5325b9ac9a875e introduced a subtle change to ActiveRecord::QueryMethods which resulted in duplicate type signatures for GeneratedAssociationRelationMethods.

  module GeneratedAssociationRelationMethods
    sig { returns(PrivateAssociationRelation) }
    def all; end

    sig { params(args: T.untyped, blk: T.untyped).returns(PrivateAssociationRelation) }
    def all(*args, &blk); end

    ...

Implementation

Preserve the custom definition and remove :all from query methods.

Tests

The tests for this DSL compiler appear to be failing on Rails Edge with a couple issues:

RAILS_VERSION=main bundle && RAILS_VERSION=main bin/test spec/tapioca/dsl/compilers/active_record_relations_spec.rb

...

it generates proper relation classes and modules                FAIL (1.43s)
        --- expected
        +++ actual
        @@ -194,6 +194,9 @@
             def all; end

             sig { params(args: T.untyped, blk: T.untyped).returns(PrivateAssociationRelation) }
        +    def all(*args, &blk); end
        +
        +    sig { params(args: T.untyped, blk: T.untyped).returns(PrivateAssociationRelation) }
             def and(*args, &blk); end

             sig { params(args: T.untyped, blk: T.untyped).returns(PrivateAssociationRelation) }
        @@ -234,19 +237,7 @@

             sig { params(args: T.untyped, blk: T.untyped).returns(PrivateAssociationRelation) }
             def includes(*args, &blk); end
        -
        -    sig { params(attributes: Hash, returning: T.nilable(T.any(T::Array[Symbol], FalseClass)), unique_by: T.nilable(T.any(T::Array[Symbol], Symbol))).returns(ActiveRecord::Result) }
        -    def insert(attributes, returning: nil, unique_by: nil); end
        -
        -    sig { params(attributes: Hash, returning: T.nilable(T.any(T::Array[Symbol], FalseClass))).returns(ActiveRecord::Result) }
        -    def insert!(attributes, returning: nil); end
        -
        -    sig { params(attributes: T::Array[Hash], returning: T.nilable(T.any(T::Array[Symbol], FalseClass)), unique_by: T.nilable(T.any(T::Array[Symbol], Symbol))).returns(ActiveRecord::Result) }
        -    def insert_all(attributes, returning: nil, unique_by: nil); end

        -    sig { params(attributes: T::Array[Hash], returning: T.nilable(T.any(T::Array[Symbol], FalseClass))).returns(ActiveRecord::Result) }
        -    def insert_all!(attributes, returning: nil); end
        -
             sig { params(args: T.untyped, blk: T.untyped).returns(PrivateAssociationRelation) }
             def invert_where(*args, &blk); end

...

This PR at least addresses the duplicate type signature which will cause static type-checking issues.