activerecord: Fix #find does not take string(s) as primary key(s).

[tk0miya]

ActiveRecord supports automatic conversion from String to Integer on #find call.

The API document of ActiveRecord::FinderMethods#find says:

If the primary key is an integer, find by id coerces its arguments
by using to_i.
https://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-find

Unfortunately, it's difficult to add the _ToI type only if the primary key is an integer. So this adds it always.

Is it better to override the signature in rbs_rails side?

[github-actions]

@tk0miya Thanks for your contribution!

Please follow the instructions below for each change.
See also: https://github.com/ruby/gem_rbs_collection/blob/main/docs/CONTRIBUTING.md

Available commands

You can use the following commands by commenting on this PR.

• /merge: Merge this PR if CI passes

---

activerecord

You changed RBS files for an existing gem.
You need to get approval from the reviewers of this gem.

@hibariya, @ksss, @Little-Rubyist, please review this pull request.
If this change is acceptable, please make a review comment including APPROVE from here.

After that, the PR author or the reviewers can merge this PR.
Just comment /merge to merge this PR.

[ksss]

Since it says “If the primary key is an integer”, I don't think it always allows _ToI.
The actual #to_i seems to be called with ActiveModel::Type::Integer.

I would like to know your actual problem.

[tk0miya]

In my case, I passed a primary key typed as String read from CSV.

# image code
CSV.read(filename).each do |row|
  User.find(row[0])
end

[ksss]

@tk0miya Is the following correct?

• CSV.read returns Array[Array[String?]]
• row is Array[String?]
• row[0] is String | nil
• String | nil is not PrimaryKey(String)
• You want this code to be error free.

[tk0miya]

Ah, sorry. I missed your message.

Is the following correct?

Almost right.

More precisely, our production code checks row[0] is not nil. So my sample code is not precise.

# image code
CSV.read(filename).each do |row|
  user_id = row[0]
  next if user_id.nil?

  User.find(user_id)  #=> User.find(String)
end

In my thought, it's not strange that ID is converted to String when we interoperate with other systems. Some communication passes ensure serializing IDs to String. For example, CSV and HTTP query params represent all data as String.

Rails and ActiveRecord support the use of String IDs easier by auto conversion. So I think adding _ToI is familiar with Rails philosophies.

As you said, the conversion works only if the primary key is an integer. So adding it to gem_rbs_collection might not be good. what do you think adding this to rbs_rails? If agreed, I'll post this to the repo later.

[ksss]

If the primary key is a string, PrimaryKey == String should hold. Since String satisfies _ToI, it seems that this change is unnecessary and type checking should pass.

However, there may be some issue that makes this assumption incorrect, which is why I am asking what the problem is.

With the current approach to the changes, I am unlikely to merge.