Add type restrictions to io

[Vici37]

This is the output of compiling cr-source-typer and running it with the below incantation:

CRYSTAL_PATH="./src" ./typer spec/std_spec.cr \
  --error-trace --exclude src/crystal/ \
  --stats --progress \
  --union-size-threshold 2 \
  --ignore-private-defs \
  --ignore-protected-defs \
  src/io

This is related to #15682 .

I also reran the above command but without the --union-size-threshold 2, and updated several long union types to IO by itself.

[yxhuvud]

This look very odd to me. Especially as pos is noreturn. Where does this union type come from?

[Vici37]

I see Memory#pos return Int32 and Buffered#pos return Int64. The side effect being this wrapping def indirectly inherits the return types of those methods 🤷

Maybe better to omit the return type restriction in this case?

[yxhuvud]

Either that, or declare it to be typeof(pos), if that is possible 🤔

[yxhuvud]

These combination of input and return types should not be correct. The return type of an assignment should be the value assigned. Does system_blocking= do anything weird?

[straight-shoota]

system_blocking= may leak the return value of fcntl.
We should probably return value (c.f. #10083).

For this PR, I'd suggest to just set the return type restriction to Nil.

[Vici37]

Per #10083, I could also update this method to return Bool and put value at the end?

Edit: Scratch that, I'll just do what you suggest in this case, and let that issue take care of this :)

[straight-shoota]

Yes, it's best to keep this PR focused on adding type restrictions, without changing anything else.

[ysbaddaden]

Thought (non-blocking): we should refactor to use an enum instead of a symbol. That would validate the value at comptime, instead of delaying to runtime, and it would be backward compatible, since symbols are automatically transformed into enums.

class IO
  struct EncodingOptions
    enum Invalid
      Skip
    end

    getter name : String
    getter invalid : Invalid?

    def initialize(@name : String, @invalid : Invalid?)
    end
  end

[ysbaddaden]

There are still a few details!

[ysbaddaden]

Question: Why restrict the types to Int32 for the instance method, but not the class method above?

Also, the type should probably be Int. There's no telling what the LibC definitions are using, and the Crystal::System methods are untyped anyway.

[ysbaddaden]

Issue: The File result type is wrong. We only happen to use File in specs, but we return other that is actually an IO::FileDescriptor (or a descendant such as File). Since type annotations don't coerce the value, the method will still return a File or an IO::FileDescriptor (verified).

Suggested change

	def reopen(other : IO::FileDescriptor) : File
	def reopen(other : IO::FileDescriptor) : IO::FileDescriptor

[Vici37]

Dang, great catch!

[ysbaddaden]

Nitpick: maybe Bytes?

Suggested change

	def read_utf8(io : IO, slice : Slice(UInt8)) : Int32
	def read_utf8(io : IO, slice : Bytes) : Int32