Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed

Conversation

bravehager
Copy link
Contributor

Motivation

I think rails/rails@fd5bd98 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.

@bravehager bravehager requested a review from a team as a code owner June 24, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant