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

Proof of concept: Create a cop to validate gem version annotations in RBI files #199

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

egiurleo
Copy link
Contributor

@egiurleo egiurleo commented Feb 2, 2024

Created as part of hackdays, along with Shopify/rbi#180 and Shopify/tapioca#1585

While adding gem version annotations to RBI files would allow developers to write more comprehensive RBIs for their gems, it has has the potential to create RBI files that are cluttered, disorganized, or incorrect.

This would be the first of a few cops meant to keep versioned RBIs clean and accurate.

This cop checks that every version included in a "@Version" annotation fits the format specified by the RBI library.

@egiurleo egiurleo requested a review from a team as a code owner February 2, 2024 21:43
While adding gem version annotations to RBI files would allow developers
to write more comprehensive RBIs for their gems, it has has the
potential to create RBI files that are cluttered, disorganized, or
incorrect.

This would be the first of a few cops meant to keep versioned RBIs clean
and accurate.

This cop checks that every version included in a "@Version" annotation
fits the format specified by the RBI library.
@egiurleo egiurleo force-pushed the emily/valid-rbi-version-annotations branch from 029c92d to 436eddc Compare February 2, 2024 21:46
@egiurleo egiurleo removed request for a team, KaanOzkan and vinistock February 2, 2024 21:47
return false unless VALID_OPERATORS.include?(operator)

begin
Gem::Version.new(version)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wdyt of using the underlying method instead? https://github.com/rubygems/rubygems/blob/40784bb244f7471fa938a421e2773ec2c7a45405/lib/rubygems/version.rb#L173C12-L177.

It won't rely on exception throwing but it might be more brittle to future changes. Although I doubt the interface will go through frequent changes.


See the rubygems.org documentation for more information on how to format gem
versions: https://guides.rubygems.org/patterns/#pessimistic-version-constraint
Enabled: true
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure it safe to enable by default since shims or generated gem RBIs may have surprising comments.

Don't we intend Tapioca to strip the version tagging when pulling annotations? I think we also talked about the possibility to merge annotations directly into the generated gem RBIs right?

RUBY
end

it "registers an offense when gem version is not formatted correctly" do
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a test for an empty @version without an operator and one for @version foo.

Copy link

github-actions bot commented Mar 8, 2024

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Mar 8, 2024
@egiurleo egiurleo marked this pull request as draft March 11, 2024 17:56
@github-actions github-actions bot removed the stale label Mar 12, 2024
Copy link

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Apr 11, 2024
@egiurleo egiurleo removed the stale label Apr 11, 2024
Copy link

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label May 12, 2024
@egiurleo egiurleo removed the stale label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants