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

Make bare Class an error #6983

Merged
merged 8 commits into from
May 17, 2023
Merged

Make bare Class an error #6983

merged 8 commits into from
May 17, 2023

Conversation

jez
Copy link
Collaborator

@jez jez commented May 6, 2023

Note to users

If you need to silence this error temporarily for a migration or
compatibility reasons, you can pass this command line flag:

--suppress-error-code=5046

This will suppress all bare stdlib generic errors, so things like
Array and Hash will also not be reported as an error. Thus using
this flag should be considered only a temporary migration mechanism.

Other temporary migration techniques:

  • Downgrade any files with these errors to # typed: true, and
    re-upgrade them to # typed: strict once all bare Class errors have
    been fixed in the file.

  • Downgrade Sorbet back to the version before this error started to be
    reported. That version treats T::Class as valid type syntax, but
    does not report an error for bare Class usage.

Motivation

This closes a follow-up action item from #6781.

Test plan

See included automated tests.

Review by commit to see the changes to the tests.

@jez jez requested a review from a team as a code owner May 6, 2023 21:26
@jez jez requested review from neilparikh and removed request for a team May 6, 2023 21:26
@jez
Copy link
Collaborator Author

jez commented May 6, 2023

@jez jez mentioned this pull request May 6, 2023
@jez jez force-pushed the jez-t-class-error branch 2 times, most recently from f4deb7e to 5910569 Compare May 9, 2023 21:24
@jez jez force-pushed the jez-t-class-error branch 4 times, most recently from 8af1421 to 9488501 Compare May 11, 2023 23:04
Base automatically changed from jez-t-class to master May 11, 2023 23:45
@jez jez force-pushed the jez-t-class-error branch 2 times, most recently from b573bbe to 5c9b638 Compare May 12, 2023 01:37
jez added 6 commits May 17, 2023 10:14
If you need to silence this error temporarily for a migration or
compatibility reasons, you can pass this command line flag:

    --suppress-error-code=5046

This will suppress *all* bare stdlib generic errors, so things like
`Array` and `Hash` will also not be reported as an error. Thus using
this flag should be considered only a temporary migration mechanism.

Other temporary migration techniques:

- Downgrade any files with these errors to `# typed: true`, and
  re-upgrade them to `# typed: strict` once all bare `Class` errors have
  been fixed in the file.

- Downgrade Sorbet back to the version before this error started to be
  reported. That version treats `T::Class` as valid type syntax, but
  does not report an error for bare `Class` usage.
@jez
Copy link
Collaborator Author

jez commented May 17, 2023

We have a policy of testing changes to Sorbet against Stripe's codebase before
merging them. I've kicked off a test run for the current PR. When the build
finishes, I'll share with you whether or how it failed. Thanks!

Stripe employees can see the build results here:

https://go/builds/bui_NuhREj4kpTW4r4
https://go/builds/bui_NuhR1vtGd8Jxlv
https://go/builds/bui_NuhRfQ0yK7Lf31

Copy link
Collaborator

@neilparikh neilparikh left a comment

Choose a reason for hiding this comment

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

Is the migration plan for the all the Stripe errors to autocorrect everything to T::Class[T.anything]?

@jez
Copy link
Collaborator Author

jez commented May 17, 2023

Is the migration plan for the all the Stripe errors to autocorrect everything to T::Class[T.anything]?

Yep.

@jez jez merged commit c3ba8c0 into master May 17, 2023
15 checks passed
@jez jez deleted the jez-t-class-error branch May 17, 2023 23:33
egiurleo added a commit to Shopify/ruby-lsp that referenced this pull request May 22, 2023
Sorbet introduced a change to raise an error when `Class` is used as a
type without generic type parameters: sorbet/sorbet#6983

This commit converts two instances of the `Class` type to
`T::Class[T.anything]`
egiurleo added a commit to Shopify/tapioca that referenced this pull request May 24, 2023
Sorbet has made using Class without specifying a generic type argument
an error (sorbet/sorbet#6983), so this commit
fixes the existing instances of this pattern in Tapioca.
egiurleo added a commit to Shopify/tapioca that referenced this pull request May 25, 2023
Sorbet has made `Class` a generic type, so using `Class` on its own in
type signatures now raises an exception
(sorbet/sorbet#6983).
andyw8 pushed a commit to Shopify/tapioca that referenced this pull request Jun 6, 2023
Sorbet has made `Class` a generic type, so using `Class` on its own in
type signatures now raises an exception
(sorbet/sorbet#6983).
ilyailya pushed a commit that referenced this pull request Sep 6, 2023
* Make bare `Class` an error

If you need to silence this error temporarily for a migration or
compatibility reasons, you can pass this command line flag:

    --suppress-error-code=5046

This will suppress *all* bare stdlib generic errors, so things like
`Array` and `Hash` will also not be reported as an error. Thus using
this flag should be considered only a temporary migration mechanism.

Other temporary migration techniques:

- Downgrade any files with these errors to `# typed: true`, and
  re-upgrade them to `# typed: strict` once all bare `Class` errors have
  been fixed in the file.

- Downgrade Sorbet back to the version before this error started to be
  reported. That version treats `T::Class` as valid type syntax, but
  does not report an error for bare `Class` usage.

* Fix the bare `Class` in our payload

* Fix existing tests

* Capture master behavior for new test

* Show test's new behavior

* Also fix ClassNew rewriter

* Fix test

* Update cli exp
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

2 participants