version: enable Sorbet type checking

[SeekingMeaning]

• 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?
• Have you successfully run brew man locally and committed any changes?

---

Notable changes:

• Added a NullVersion singleton class that extends Version
• Changed Version::Token to be an abstract class
• Added version.rbi to work around Sorbet limitation with aliases

There are a lot of T.untyped but this is a start, at least

[BrewTestBot]

Review period will end on 2021-01-15 at 22:32:42 UTC.

[MikeMcQuaid]

Sorbet question: what does this do and why is it needed?

[reitermarkus]

It casts T.nilable(Type) to Type, i.e. ensuring something is non-nil.

[MikeMcQuaid]

What happens if it is nil with or without Sorbet enabled at runtime?

[reitermarkus]

Without, the >= call will fail; with, a TypeError is raised.

Both should never happen since the macOS major version should always be non-nil.

[MikeMcQuaid]

Thanks. Is it possible to somehow move logic like this into where major is set/returned instead?

[reitermarkus]

It's calling Version#major, which definitely can have a nil major version.

[SeekingMeaning]

One option is to add an RBI file that says OS::Mac::Version#major will always return a non-nil Token. We can do this since we have this check in #initialize:

raise MacOSVersionError, version unless /\A1\d+(?:\.\d+){0,2}\Z/.match?(version)

[MikeMcQuaid]

One option is to add an RBI file that says OS::Mac::Version#major will always return a non-nil Token

Something like that seems good 👍🏻

[BrewTestBot]

Review period ended.

[SeekingMeaning]

• Replaced T.untyped with String for Token#create, Version#create, and Version#initialize
  • Replaced ENV["HOMEBREW_LINUX_MINIMUM_GLIBC_VERSION"] with ENV.fetch("...")
• Replaced when nil and else branches in Token#from with return NULL_TOKEN if ... guard clause
• Added #null? to Token class as overridable method
  • The NullToken class already had #null? but now we can add override to its sig
• Changed return types of NullVersion methods:
  • Changed return type of NullVersion#<=> from T.nilable(Integer) to plain Integer since it always returns -1
  • Changed return type of NullVersion#major, #minor, #patch from T.nilable(Token) to plain Token since they always return NULL_TOKEN

[MikeMcQuaid]

Could we restrict this perhaps to Token, String, Integer, nil or NullToken? Would ease some other code, too.

[SeekingMeaning]

I gave this a go, but I came to the conclusion that adding a type to this would require something like Java's generics where a subclass can specify the type of a parent variable. I don't think Sorbet supports this, though

Edit: I'll give this another try and see where I get

[SeekingMeaning]

1. Added a type for the value parameter for #initialize
   • Token → T.nilable(T.any(String, Integer))
   • NullToken has no parameters for #initialize
   • StringToken → String
   • NumericToken → T.any(String, Integer) (converted to Integer by value.to_i)
2. Added overrides for attr_reader :value with the following return types:
   • NullToken → NilClass (The class for plain nil is NilClass)
   • StringToken → String
   • NumericToken → Integer
3. Replaced @value = value with @value = T.let(value, T.untyped), otherwise we get this error for NullToken (and a similar error for StringToken and NumericToken):

version.rb:107: Expected NilClass but found T.nilable(T.any(String, Integer)) for method result type https://srb.help/7005
     107 |    attr_reader :value
                           ^^^^^
  Expected NilClass
    version.rb:107: Method value has return type NilClass
     107 |    attr_reader :value
              ^^^^^^^^^^^^^^^^^^
  Got T.nilable(T.any(String, Integer)) originating from:
    version.rb:59:
    59 |      @value = value
              ^^^^^^

[MikeMcQuaid]

One option is to add an RBI file that says OS::Mac::Version#major will always return a non-nil Token

Something like that seems good 👍🏻