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

Change two recently added classes to strict typing #415

Merged
merged 1 commit into from Dec 29, 2021

Conversation

yob
Copy link
Owner

@yob yob commented Dec 29, 2021

New code may as well go all in on type hints. I'll be interested to see if this helps with maintainability over time.

Thanks to this answer on stackoverflow for pointing me in the right direction: https://stackoverflow.com/questions/70515191/how-can-i-resolve-sorbet-error-use-of-undeclared-variable/70519194#70519194

New code may as well go all in on type hints. I'll be interested to see
if this helps with maintainability over time.
@yob yob merged commit 82a4b96 into main Dec 29, 2021
@yob yob deleted the strict-typed-new-classes branch December 29, 2021 12:40
@yob
Copy link
Owner Author

yob commented Dec 29, 2021

I wonder if I could fail the build if a new ruby file added since sorbet was introduced was missing strict typing? Maybe something based on this git command that lists files added since the relevant commit:

$ git diff --numstat --diff-filter=A feb9203 main -- lib/
16      0       lib/pdf/reader/bounding_rectangle_runs_filter.rb
25      0       lib/pdf/reader/point.rb
113     0       lib/pdf/reader/rectangle.rb

@@ -1110,7 +1110,10 @@ module PDF
y: Numeric,
).void
end
def initialize(x, y); end
def initialize(x, y)
@x = T.let(0, Numeric)

Choose a reason for hiding this comment

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

You don't need the actual value of the variable in RBI files, you should be able to just do:

@x = T.let(T.unsafe(nil), Numeric)

instead. Here T.unsafe(nil) creates a dummy value that is T.untyped and T.let declares the type of it.

This way, your internal implementation details will not leak into RBI files, you can just declare the types instead.

Copy link
Owner Author

Choose a reason for hiding this comment

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

cheers, updated in #416

yob added a commit that referenced this pull request Dec 29, 2021
These type hints were added in #415 while changing some files to `typed:
strict`. It felt weird providing an initial value that I knew wasn't
used, and in a follow up comment on the PR it was suggested that I can
use this form to be clear that I'm annotating the ivar type without
setting a value.

`T.unsafe(nil)` also feels a bit weird. So much syntax for so little
value. It'd be nice if sorbet had a way to only specificy the ivar type
in an RBI files, without the value argument at all 🤷‍♂️
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