Add RBS type signatures

[sferik]

No description provided.

[hsbt]

@soutaro @pocke @ksss Could you review this? I consider adding test task for this like same as bundled gems. But I'm not sure what's the current direction of rbs with default gems.

[sferik]

@hsbt I'm working on a commit to add type checking/verification with Steep now.

[ksss]

@sferik
If you wish to enrich the type definitions for net/http, please send a PR to the ruby/rbs repository.
Currently, we manage type definitions in the ruby/rbs repository, but it would also be ideal to manage them within the gem itself.
If you wish to move the net/http type definitions to the gem itself for management, attention should be paid to the coverage of the type definitions.

Which one would you like?

[sferik]

@ksss
I think it makes the most sense to manage the type definitions in this repo. Colocating type definitions with the source code will make it easier to keep them in sync. When a contributor adds a new public method, they can update the code, tests, and type signatures all in a single pull request. If the type definitions live in a separate repository, adding a new public method would require two PRs to two separate repos.

I've added Steep as a development dependency along with a new rake task to validate the RBS file against the code, as well as a GitHub workflow that will type check new PRs.

I understand the concern about type coverage. The signatures in this PR were generated with typeprof and rbs prototype rb and then refined by hand. I'd be happy to improve coverage further if there are specific gaps you'd like addressed.

[ksss]

I agree with the argument that type definitions should be included in gems. In that case, the following should be considered:

• Please respect and inherit past assets. Please consider https://github.com/ruby/rbs/blob/master/stdlib/net-http/0/net-http.rbs.
• RBS documentation is missing. Comments need to be synchronized with Ruby files. Consider using the rbs annotate command. You can refer to ruby/bigdecimal and others for examples.
• Test code for type definitions is missing. steep check has strong implications for the internal implementation of net/http and does not test the public interface. net/http is currently a default gem, and test code for type definitions that can be executed in the ruby/ruby repository is needed. Test code should be defined by referring to https://github.com/ruby/rbs/blob/master/test/stdlib/Net_HTTP_test.rb. Also, consider that these tests should not access external URLs.
• Organize dependencies between libraries. Dependencies of default gems should be described using manifest.yaml, as shown in https://github.com/ruby/rbs/blob/master/stdlib/net-http/0/manifest.yaml.

[sferik]

Thank you for this valuable feedback. I am working on fixes now.

[sferik]

@ksss
I was not aware of the existing net-http.rbs in the rbs repo, so I started over by moving that file (and the manifest.yaml) into the sig/ directory.

I then added steep to the Gemfile, a steep task to the Rakefile, and a typecheck.yml GitHub action to run rake steep in CI.

I then ran rake steep locally and systematically fixed the type errors. This required some changes to the lib files (generally making the code more explicit/less dynamic) and some changes to the type signatures in sig/net-http.rbs. In cases where I needed to add type signatures for "internal use only" classes or other libraries, I added separate sig/*-impl.rbs files, which I have excluded from the gemspec. I was able to make rake steep pass without adding any # steep:ignore comments.

Finally, I added local RBS interface tests. Do you think I should also keep Steep or remove it?

Please let me know if I got anything wrong or missed anything. Thanks again for your feedback!

[sferik]

I also added rbs:test and rbs:annotate rake tasks, mirroring the ones in the bigdecimal Rakefile. I ran rbs:annotate and committed the changes to sync the RDoc documentation to the RBS file.

Please also let me know if you'd like me to squash these down into a single commit.

[ksss]

I have no maintenance privileges whatsoever for this repository. I don't even have the authority to approve this PR. I will state my personal opinion based on this premise.

I think simply copying net-http.rbs was an excellent decision. That's where you should start. However, you should start with as complete a copy as possible. I don't know what the differences are, so I can't review it.

I think we should first implement the introduction of RBS, and then as the next step, tackle changes to the Ruby code. These seem like separate issues. I think the PRs should be split.

Steep is a development support tool. If you want to use Steep for developing this repository, then I think it should be introduced, but otherwise, I think it should be left to the developers.

The idea of *-impl.rbs is great, but for the same reason as Steep, I don't think it should be included in this PR.

[sferik]

Closing this PR in favor of:

• Add RBS signatures from ruby/rbs repo #279 (add-rbs-signatures)
  • 0421be4 Add existing RBS file and manifest from ruby/rbs repo
  • 9c60307 Add local RBS interface tests
• Fix RBS type errors #280 (fix-rbs-type-errors, based on sferik:add-rbs-signatures)
  • 5ca31eb Fix type errors in net/http source files for RBS compatibility (renamed — removed Steep mention)
  • 2e1210e Update RBS signatures and fix type errors
  • f761545 Sync net/http RBS docs with RDoc
• Add Steep type checking #281 (add-steep-type-checking, based on fix-rbs-type-errors)
  • f624fce Add Steep for type checking

Feel free to review and merge each of these independently. I hope this helps move this issue forward.