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

Run Sorbet type-checking with typed: true #2561

Merged
merged 10 commits into from
Mar 6, 2024
Merged

Conversation

paracycle
Copy link
Collaborator

This PR makes the Sorbet type-checking task work with typed: true for all files by default, except for a few files that we revert back to typed: false. We also suppress a handful of classes of errors that are hard to solve without inline type assertions and are not essential errors for what we are trying to do here.

I've also had to make some code changes to get type-checking to work at this level. The major refactor was to move all template related methods and classes under the Prism::Template namespace, so that there is no name collision with types under the Prism namespace.

@paracycle paracycle requested a review from kddnewton March 6, 2024 19:56
@paracycle paracycle force-pushed the uk-typecheck-with-typed-true branch from c5fa311 to b9de1b2 Compare March 6, 2024 20:24
This change makes the Sorbet typechecking step use `typed: true` by
default for all files except the ones we explicitly typed override back
to `typed: false`.

We also extend the Sorbet config to suppress a few classes of errors
that are not essential and would be hard to fix without inline type annotations.
For example, use `.fetch` or `.dig` instead of `[]`, and use `===` instead of `is_a?` for checking types of objects.
@paracycle paracycle force-pushed the uk-typecheck-with-typed-true branch from b9de1b2 to f0e06d8 Compare March 6, 2024 21:24
@kddnewton kddnewton merged commit 532e4cc into main Mar 6, 2024
53 of 55 checks passed
@kddnewton kddnewton deleted the uk-typecheck-with-typed-true branch March 6, 2024 21:37
Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

Nice!

@@ -1799,7 +1799,7 @@ def visit_block(call, block)

# Visit a heredoc that can be either a string or an xstring.
def visit_heredoc(node)
children = []
children = Array.new
Copy link
Member

Choose a reason for hiding this comment

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

Is this change necessary?

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.

3 participants