search

ruby / rbs

Type Signature for Ruby
Other
1.91k stars 201 forks source link

use optional record keys for several API options #1900

Open HoneyryderChuck opened 2 weeks ago

ParadoxV5 commented 2 weeks ago

DRAFT:

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests#draft-pull-requests

HoneyryderChuck commented 6 days ago

@soutaro it's ready for review 🙏

soutaro commented 6 days ago

Thank you for your work! 🎉 I'm totally good for using optional records for return values.

However, using the records instead of Hash for options may result in reporting more type errors.

opts = {}
# Set up opts
CGI.new(opts)        # Type error because it's a Hash, not a record

What do you think about this? I'm thinking of if having additional rules for Hash -> record conversion in Steep would help.

HoneyryderChuck commented 6 days ago

I'll run some tests locally to see if this impacts rbs runtime type check significantly. Nevertheless, I think there's great value in being precise on the options you accept in a hash as a function argument, particularly in examples such as ssl context, or URI build functions. That's definitely something steep could be a bit smarter about 👍