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.

Sign up for GitHub

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

Jump to bottom

Create a cop to validate gem version annotations in RBI files #199

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Create a cop to validate gem version annotations in RBI files #199

merged 1 commit into from
Jul 17, 2024

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
@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
@github-actions GitHub Actions
Copy link
Contributor

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
@github-actions GitHub Actions
Copy link
Contributor

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
@github-actions GitHub Actions
Copy link
Contributor

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
@github-actions GitHub Actions
Copy link
Contributor

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 Jun 13, 2024
@egiurleo egiurleo removed the stale label Jun 13, 2024
@egiurleo egiurleo force-pushed the emily/valid-rbi-version-annotations branch from 436eddc to 477c2b0 Compare June 27, 2024 15:05
@egiurleo egiurleo changed the title Proof of concept: Create a cop to validate gem version annotations in RBI files Create a cop to validate gem version annotations in RBI files Jun 27, 2024
@egiurleo egiurleo marked this pull request as ready for review June 27, 2024 15:06
@egiurleo egiurleo requested review from Morriar and KaanOzkan June 27, 2024 15:06
@egiurleo egiurleo force-pushed the emily/valid-rbi-version-annotations branch from 477c2b0 to f0424ea Compare June 27, 2024 15:07
Morriar
Morriar reviewed Jun 27, 2024
View reviewed changes
Copy link
Contributor

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

Should we make sure the annotation is attached to a class/method/attr node? Or we don't care about this yet?

config/default.yml Outdated Show resolved Hide resolved
lib/rubocop/cop/sorbet/rbi_versioning/valid_gem_version_annotations.rb Outdated Show resolved Hide resolved
lib/rubocop/cop/sorbet/rbi_versioning/valid_gem_version_annotations.rb Outdated Show resolved Hide resolved
@egiurleo egiurleo force-pushed the emily/valid-rbi-version-annotations branch from f0424ea to b9fa1da Compare June 27, 2024 17:36
@egiurleo egiurleo force-pushed the emily/valid-rbi-version-annotations branch from b9fa1da to eaeba24 Compare June 27, 2024 17:38
@egiurleo
Copy link
Contributor Author

@Morriar Great question -- how do you generally think about what goes in the same Rubocop rule vs different rules? Like I could imagine that being in a rule called "NoOrphanedVersionAnnotations" or something like that?

@Morriar
Copy link
Contributor

Morriar commented Jun 28, 2024

@egiurleo I think it would be better in the same cop but we can merge this now and iterate in another PR 👍

KaanOzkan
KaanOzkan reviewed Jul 2, 2024
View reviewed changes
lib/rubocop/cop/sorbet/rbi_versioning/valid_gem_version_annotations.rb

if gem_versions.empty?
message = format(MSG, versions: "empty version")
add_offense(comment, message: message)
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 assuming this cop will be enabled in rbi-central. Have we considered having the default case not error to make the onboarding easier (both here and in rbi logic)? Or do we want to manually add versions to existing annotations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh this is only if someone writes # @version but then doesn't actually provide any version numbers! Any code without any comment or a different comment will be left alone :)

@egiurleo
Create a cop to validate gem version annotations in RBI files
3cffa24 
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 eaeba24 to 3cffa24 Compare July 17, 2024 20:16
@egiurleo egiurleo merged commit 5cba866 into main Jul 17, 2024
9 checks passed
@egiurleo egiurleo deleted the emily/valid-rbi-version-annotations branch July 17, 2024 20:29
@Morriar Morriar added the feature Add a new feature label Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KaanOzkan KaanOzkan KaanOzkan left review comments

@andyw8 andyw8 andyw8 left review comments

@Morriar Morriar Morriar approved these changes

Assignees
No one assigned
Labels
feature Add a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@egiurleo @Morriar @andyw8 @KaanOzkan