Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support inline RBI versioning #180

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Support inline RBI versioning #180

merged 1 commit into from
Feb 9, 2024

Conversation

egiurleo
Copy link
Contributor

This is a proof of concept for RBI versioning, which @Morriar, @bitwise-aiden, and I had been working on for fun a while back.

Problem

This proof of concept aims to solve the problem of versioning RBIs. Up until now, there is no default way to write RBI for different versions of the same gem. This can lead to confusing issues for Sorbet users where RBIs don't match the version of a gem they're using, and they have to spend time figuring out whether their gem is out of date, whether the RBI is wrong, or if it's even both! Having a way to specify RBI for multiple versions of the same gem would allow people to adopt and use Sorbet without having to go through this hassle.

Approach

To implement this behavior, we decided to create a system of versioning RBI within the same file using @version annotations within comments. This is an example of using such an annotation:

class Foo

  # @version > 0.3.0
  def bar
  end

  # @version <= 0.3.0
  def bar(arg1)
  end
end

In this example, the class Foo has a method bar that takes one argument in versions 0.3.0 and earlier, but that argument is removed in later versions.

There are a few benefits to this approach:

  • Limits duplication by preventing multiple files being created for the same gem RBI
  • Less duplication means that relevant annotations are easier to find and read all at once
  • When parsing the RBI files, parts that are not relevant to the current gem version can be removed

There are also some drawbacks, namely that RBI files could potentially become cluttered if many gem versions are supported. We plan to address this using some automated linting (see "Future Work" section), and in practice we don't believe that most gem authors actually support that many versions at a time. Another potential drawback is that parsing out irrelevant parts of the RBI can slow down parsing, but in practice, we see that most RBI files are relatively small and that speed is not currently a big issue; however, this can be revisited if performance becomes a bigger priority.

For more information about the other approaches we considered, see this document.

Implementation

This change was implemented by creating a new rewriter called FilterVersions. This takes the current RBI tree as well as a gem version to filter on. For each node in the RBI, the rewriter determines if the node satisfies the gem version by examining any @version annotations in the comments and relying on logic from Ruby's Gem::Requirement class to perform comparisons.

RBIs with no versions remain unaffected.

Future Work

  1. Documentation
    If the team is supportive of this approach, I'll write up documentation for this behavior before merging the PR.

  2. Linting
    While we believe the inline versioning approach makes the most sense for RBI, it still leaves the possibility of developers writing disorganized RBIs (e.g. different versions of the same method are far away from each other) or invalid RBI files (e.g. writing versions that are somehow incompatible with each other). To address these issues, we would implement some sort of linting that prevents developers from falling into these mistakes so that this feature is as easy to use as possible.

@egiurleo egiurleo requested a review from a team as a code owner May 19, 2023 18:13
@egiurleo egiurleo requested review from Morriar and st0012 May 19, 2023 18:13
return true unless is_a?(NodeWithComments)

requirements = version_requirements
requirements.empty? || requirements.any? { |req| req.satisfied_by?(version) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: any node with no requirements is treated as satisfying the requirements.


def test_filter_versions_and
rbi = <<~RBI
# @version > 0.3.0, < 1.0.0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this implementation supports and/or logic. The line above is an example of "and" -- the version must be greater than 0.3.0 AND less than 1.0.0. Line 218 is an example of "or" logic.

Copy link
Contributor

@andyw8 andyw8 May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it's not quite as flexible, I wonder if only permitting the same syntax as a .gemspec would be feasible, so there's not a new thing to learn.

The downside is that in some situations it could be very verbose, e.g. instead of:

 # @version < 0.3.0
 # @version > 1.0.0

You would need to explicitly exclude versions, e.g.:

 != 0.4.0, != 0.5.0, != 0.6.0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if only permitting the same syntax as a .gemspec would be feasible

Could you please elaborate on this a bit more @andyw8? Are you referring to the version as defined here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean the syntax for adding for adding a dependency, e.g. https://guides.rubygems.org/specification-reference/#add_runtime_dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this comment! As far as I know, the syntax I'm using here is pretty much identical to the gemspec syntax, at least per the semantic versioning guide on RubyGems. Can you let me know if there's a particular difference you've spotted?

@st0012 st0012 removed their request for review January 16, 2024 08:34
This commit adds the notion of versioning to the RBI gem.

1. Users can specify versions for parts of their RBI files using
`@version` annotations in comments above any class, module, or method
definition.

2. The `FilterVersions` rewriter will take an RBI file, as well as a
version number, and rewrite the RBI to only include the portions that
are relevant for a specific gem version.

3. RBI without any version annotations will continue to behave as expected.

Co-authored-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Co-authored-by: Aiden Storey <aiden.storey@shopify.com>
@egiurleo egiurleo changed the title Proof of concept: Support inline RBI versioning Support inline RBI versioning Feb 9, 2024
@egiurleo egiurleo merged commit 15448ad into main Feb 9, 2024
9 checks passed
@egiurleo egiurleo deleted the emily-alex/rbi-versioning branch February 9, 2024 15:09
@st0012 st0012 added the enhancement New feature or request label Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants