aws/aws-sdk-ruby

Generate RBS

ksss opened this issue ยท 15 comments

Describe the feature

Currently, Ruby code generation is done by build_tools, but I propose that RBS files also be generated during this Ruby code generation. RBS refers to Ruby's standard type files and tools, which can be bundled with the gem itself.
I have already been conducting experimental operations for over a year.
In a repository similar to DefinitelyTyped for RBS, I have been generating RBS from apis/**/*.json, and it has been used within the RBS community.

https://github.com/ruby/gem_rbs_collection/tree/main/generators/aws-sdk-rbs-generator

Use Case

Using LSP (Language Server Protocol) in recent years, we can receive coding assistance as follows:

  • Automatic suggestion of methods
  • Type checking of argument names and method names
  • Usage similar to API documentation

Moreover, by being incorporated into the upstream, it becomes possible to accurately manage the Ruby code and corresponding RBS for each version.

Proposed Solution

I will integrate the existing aws-sdk-code-generator and the aws-sdk-rbs-generator that I implemented, and organize the workflow so that both Ruby code and RBS code are generated and released simultaneously.
Prepare a sig directory in each gem and generate RBS files within this directory. This will not affect the Ruby code at all.

Other Information

Since I have made numerous commits to RBS, I can provide detailed support regarding RBS.

https://github.com/ruby/rbs/graphs/contributors
https://github.com/ruby/gem_rbs_collection/tree/main/generators/aws-sdk-rbs-generator

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

SDK version used

3

Environment details (OS name and version, etc.)

any

Thanks for reaching out! We met with Soutaro at RubyConf and he told us about your project. We would love to get RBS typing into v3 and would appreciate any help doing so. This would certainly be a large project and may take some time. As far as generation goes, v3 code generation uses Mustache templates. Looking at aws-sdk-rbs-generator, I think most of the logic goes away when using the existing aws-sdk-code-generator. I think we should get a simple gem working, like lambda, as a proof of concept. There's no need to be backwards compatible inside aws-sdk-code-generator within reason. You can test builds with rake build:aws-sdk-lambda for example. If you start a feature branch, we would be happy to contribute and review when able.

@mullermp
I apologize for the late reply; I was down with a cold ๐Ÿคง
As you said, I also heard from @soutaro that there are plans to add RBS to aws-sdk-ruby.
Let's start with lambda and gradually expand.
Thanks for the follow-up.

Issue1 We need RBS for aws-sdk-core

There are RBS that should be defined in aws-sdk-core, such as Aws::Errors::ServiceError, Aws::EmptyStructure, and Seahorse::Client::Base.
Firstly, we need to introduce RBS in the aws-sdk-core gem, and gems that want to incorporate RBS will need to change their dependencies to use this core version or later. Is this acceptable?

Yes. That's fine! A minimal set would be appreciated, as we will have to maintain this going forward as a small team. When we are done, we can setup the minimum version constraints between gems and core.

We should also setup steep on CI to check new features are added.

OK. Since aws-sdk-core is outside the scope of automatic generation and has a broad impact, I intend to limit it to only the minimum necessary RBS.

ksss commented

Issue2 CI for RBS files

We should be able to automatically verify the correctness of the output RBS for maintainability.
There are several things we can do to achieve this.

Summary

I recommend implementing the following

  • Run $ rbs validate
  • Run rspec with RBS::Test and modify spec file a little.

Details

There are 4 possible methods for testing RBS.

static dynamic
light $ rbs validate RBS::Test
heavy $ steep check RBS::UnitTest

$ rbs validate

$ rbs validate cli command can statically validate RBS files.
It checks for various RBS inconsistencies as well as syntax.
This can already be implemented as a rake task.

RBS::Test

RBS::Test is a tool included with the rbs gem.
It allows for dynamic checking to see if the types of arguments and return values for a specific class match the actual behavior by running existing rspec tests.

You can find type errors without additional code.
I have already been able to find several type errors by having them working at hand.

However, there are several problems with integrating this method into CI.

Need rbs v3.4.0

I have fixed the problems I found throughout this project with the rbs gem. This code will be released in rbs v3.4.0.
rbs v3.4.0 has been released pre1, but the official version has not yet been released.

Need to give up or small fix

We need to give up a little type correctness or small modify spec file.
Which would you prefer for the aws-sdk-ruby team?
My personal opinion is that I prefer the convenience method and use annotations in exceptional cases.

give up a little type correctness

I got 1155 examples, 2 failures for the rspec of aws-sdk-s3 in my test using RBS::Test.

it 'can be used with a Resource client' do
resource = S3::Resource.new(client: client)
expect(resource.client.config).to eq(api_client.config)
end

it 'can be used with a Resource client' do
resource = S3::Resource.new(client: client)
expect(resource.client.config).to eq(api_client.config)
end

That is, when the Encryption client is used as the Resource class.

If we try to express this behavior precisely with types, we would lose the convenience of types in many cases or have to generate distorted RBS.

small modify spec file

Another idea is to tag test files to skip test cases only when using this RBS::Test.

it '...', rbs_skip: true do
  # ...
end
$ rspec --tag '~rbs_skip'

In total there are 90 spec files that are not auto-generated, and 43 aws-sdk-s3 spec files.
Since we found 2 cases in aws-sdk-s3, we can simply calculate that there are about 5 such cases in total.

$ steep check

There are methods to perform this on the implementation and on the sample code.
However, I do not recommend either for this project.

On the implementation

Applying steep check to the implementation can assist in development and check for correctness in types.
However, there are several issues with this project, which is why I personally do not recommend using it within this project.

The Project is Too Large

Using steep in CI implies the need for high RBS coverage and knowledge of RBS.
This project is large, so it takes time to prepare all types without any omissions.

Auto-Generated Code

Most of the code is auto-generated, so using it for development support offers little benefit.

Keyword Arguments

In many cases, this project uses Hash options instead of keyword arguments.
Therefore, even if RBS uses keyword arguments, the implementation does not, so steep will report an error.
If you stop using keyword arguments in RBS, it will then lose convenience on the user side.

RBS::UnitTest

This is how to write type-specific test code as described below.
https://github.com/ruby/rbs/blob/master/docs/gem.md#testing-your-type-definition

We can ensure the accuracy of RBS against the actual behavior.
However, additional test code needs to be written. Knowledge about types and maintenance costs are required.
Therefore, I don't think it's a suitable option for this project.

My guess is that the S3 tests are likely the most complex/likely to have tests that need to be skipped.

I would lean towards adding an rbs_skip tag and just skipping those tests. Though to be honest looking at those two specific test cases - I don't see significant benefit in them (they're only testing that you can pass in the client - not that it is actually api equivalent, since the Resource client does no validation on what is passed in) - I actually think for these two specific cases, we should just remove those test cases. But I think a general strategy of an rbs_skip tag makes sense.

ksss commented

@alextwoods
Thank you for your response.
Finally, I plan to run $ rbs validate and RBS::Test with rspec on aws-sdk-core and aws-sdk-s3.
I've realized that cases to skip should include exceptional cases in the client method (such as when data becomes nil), so it looks like the number will increase to about 13 cases.

ksss commented

Issue3 Handwritten code type

Some services have handwritten custom code.
Should we add RBS for all of these before release?
Or should we first release RBS for the auto-generated code and gradually add types for the handwritten code? (I think this will take time).

ksss commented

I have created a branch here with the generated results for all services at this time.
Please check it out.

https://github.com/ksss/aws-sdk-ruby/tree/rbs-full-generated

Thank you! We will check this out this or next week, please allow time for vacation.

I think code generation cases are fine for now.

ksss commented

No problem at all. Have a happy New Year ๐Ÿ‰

Merged with #2961

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.