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

rubocop: Set TargetRubyVersion to 3.1 & disable all cops that fail #16341

Merged
merged 1 commit into from Dec 15, 2023

Conversation

issyl0
Copy link
Member

@issyl0 issyl0 commented Dec 14, 2023

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

- As discussed in
  Homebrew#16337 (comment),
  then we won't have the chicken/egg problem of fixing cops for syntax
  that's unsupported on Ruby 2.6 _before_ we set the TargetRubyVersion
  to 3.1, but if we set it too early then all the taps will fail.
@issyl0 issyl0 requested a review from Bo98 December 14, 2023 23:23
@@ -10,13 +10,10 @@ class PATH

delegate each: :@paths

# FIXME: Enable cop again when https://github.com/sorbet/sorbet/issues/3532 is fixed.
# rubocop:disable Style/MutableConstant
Copy link
Member

Choose a reason for hiding this comment

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

I think these comments still need to stay around but perhaps in a different format otherwise I worry they will get "lost" when changing Style/MutableConstant back. @issyl0 can you do this in a follow-up PR? Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Given this T.type_alias thing is unfixable on our side, it might be worth creating a cop that injects into Style/MutableConstant and allows T.type_alias.

Something similar to how https://github.com/Shopify/rubocop-sorbet/blob/main/lib/rubocop/cop/sorbet/mutable_constant_sorbet_aware_behaviour.rb works for T.let. Unlike T.type_alias, T.let can be frozen, just not in a way RuboCop understands. What we'd do differently for T.type_alias is we'd probably do a return if t_type_alias in on_assignment rather than passthrough.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks @issyl0!

@MikeMcQuaid MikeMcQuaid merged commit dc3daf7 into Homebrew:master Dec 15, 2023
24 checks passed
@issyl0 issyl0 deleted the rubocop-ruby-3.1 branch December 15, 2023 08:55
@Bo98 Bo98 mentioned this pull request Dec 15, 2023
11 tasks
@github-actions github-actions bot added the outdated PR was locked due to age label Jan 15, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants