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

Incorrect array signature for .new in ActiveRecord model #1981

Open
jcollins-gc opened this issue Aug 7, 2024 · 2 comments
Open

Incorrect array signature for .new in ActiveRecord model #1981

jcollins-gc opened this issue Aug 7, 2024 · 2 comments
Assignees

Comments

@jcollins-gc
Copy link

An incorrect signature is being generated for the .new method in an ActiveRecord model.

Even though the .new method does not support arrays, this signature is nevertheless being generated:

    sig do
      params(
        attributes: T::Array[T.untyped],
        block: T.nilable(T.proc.params(object: ::Payment).void)
      ).returns(T::Array[::Payment])
    end

This is despite arrays not being supported in the .new method:

[1] > Payment.new([])
ArgumentError: When assigning attributes, you must pass a hash as an argument, Array passed.

Suspect commit: https://github.com/Shopify/tapioca/pull/1946/files#diff-ccd1794e49448cc1badb48d2a32af60d5fcd47336d00a484fbf474ebf79b6789R223

@pawelduda
Copy link

pawelduda commented Aug 14, 2024

We have a similar issue, probably also caused by the suspect commit linked in the original post. After regenerating types, sorbet is giving us multiple errors with the same pattern: method <x> does not exist on T::Array[<model>], where the underlying type should obviously resolve to <model> - all type mismatches originate from builder methods of AR (#create, #build). Even after checking out the recent fix commit df6e272

I investigated further:

  # no signature -> returns T.untyped
  def some_params
    { company_id:, data: permitted_params.merge({ scheduling_user_id: current_user.id }) }
  end

  Model.create!(some_params) # revealed type is `T::Array[Model]`

Typing attributes works:

  sig { returns(T::Hash[Symbol, T.untyped]) }
  def some_params
    { company_id:, data: permitted_params.merge({ scheduling_user_id: current_user.id }) }
  end

  Model.create!(some_params) # revealed type is `Model`

Splatting attributes also helps:

  def some_params
    { company_id:, data: permitted_params.merge({ scheduling_user_id: current_user.id }) }
  end

  Model.create!(**some_params) # revealed type is `Model`

@KaanOzkan
Copy link
Contributor

@pawelduda it's a side effect of this feature #1978 combined with Sorbet's prioritization which is outlined in sorbet/sorbet#7744. I suggest typing the arguments supplied.

issyl0 added a commit to issyl0/tapioca that referenced this issue Oct 30, 2024
- This swaps the order of the `T.untyped` and `T::Array[T.untyped]`
  signatures when generating RBI files for ActiveRecord methods.
- Some of the methods that define a sig for `T::Array[T.untyped]`, such
  as `new` [1], don't support arrays as input.
- More common than arrays are non-array inputs for ActiveRecord methods,
  so the non-array signature should come first.

[1] - Shopify#1981
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

No branches or pull requests

4 participants