chanzuckerberg / sorbet-rails

A set of tools to make the Sorbet typechecker work with Ruby on Rails seamlessly.
MIT License
638 stars 84 forks source link

`pluck_to_tstruct` assigns plucked values to the wrong tstruct keys depending on the order in which the tstruct props for associations are listed #562

Open gnaratil2017 opened 1 year ago

gnaratil2017 commented 1 year ago

Describe the bug:

When calling pluck_to_tstruct with association mappings, the plucked values do not get assigned to the correct tstruct keys if any of the tstruct props for the associations are listed before the non-association ones.

This happens because the associations always end up at the end of the pluck_keys array, due to the fact that we have to replace the association keys with their corresponding association values from the association mappings param: https://github.com/chanzuckerberg/sorbet-rails/blob/447ecbe4a2586ce8aa9055290d571fe34a55e27f/lib/sorbet-rails/rails_mixins/pluck_to_tstruct.rb#L33

So when the row of plucked keys gets zipped with tstruct_keys, the plucked values do not necessarily get matched up with their corresponding keys, since the association keys in tstruct_keys are in their initial positions rather than at the end of the tstruct_keys array. https://github.com/chanzuckerberg/sorbet-rails/blob/447ecbe4a2586ce8aa9055290d571fe34a55e27f/lib/sorbet-rails/rails_mixins/pluck_to_tstruct.rb#L40

In addition, this means that if the order of the association keys within the associations param to pluck_to_tstruct differs from the order of their corresponding keys in the tstruct class, the plucked association values will also get assigned to the incorrect tstruct keys.

Steps to reproduce:

Call pluck_to_tstruct with a tstruct class that lists the prop for an association mapping (wand_wood_type) before any of the non-association ones:

class WizardWithWandT < T::Struct
    include TStructComparable

    const :name, String
    const :wand_wood_type, String
    const :house, String
end

Wizard.joins(:wand).pluck_to_tstruct(TA[WizardWithWandT].new, associations: { wand_wood_type: "wands.wood_type" })

The plucked value for wands.wood_type gets assigned to house instead of wand_wood_type (which also means the plucked house gets assigned to wand_wood_type)

[
     <WizardWithWandT house="Holly" name="Harry Potter" wand_wood_type="Gryffindor">,
     <WizardWithWandT house="Vine" name="Hermione Granger" wand_wood_type="Gryffindor">,
]

OR

Call pluck_to_tstruct with a tstruct class that lists the props for association mappings in a different order than the keys in the associations param:

class WizardWithWandT < T::Struct
    include TStructComparable

    const :name, String
    const :house, String
    const :wand_wood_type, String
    const :wand_core_type, Integer
end

Wizard.joins(:wand).pluck_to_tstruct(
     TA[WizardWithWandT].new,
     associations: { wand_core_type: "wands.core_type", wand_wood_type: "wands.wood_type" },
)

It incorrectly tries to assign the plucked value for wands.core_type to wand_wood_type, and results in a TypeError:

Failure/Error: tstruct.new(value)

TypeError:
     Parameter 'wand_wood_type': Can't set WizardWithWandT.wand_wood_type to 0 (instance of Integer) - need a String

Expected behavior:

All the plucked values get correctly assigned to their corresponding tstruct keys.

[
     <WizardWithWandT house="Gryffindor" name="Harry Potter" wand_wood_type="Holly">,
     <WizardWithWandT house="Gryffindor" name="Hermione Granger" wand_wood_type="Vine">,
]
[
     <WizardWithWandT house="Gryffindor" name="Harry Potter" wand_core_type=0 wand_wood_type="Holly">, 
     <WizardWithWandT house="Gryffindor" name="Hermione Granger" wand_core_type=0 wand_wood_type="Vine">,
]

Versions: