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

Add parameter and return type restrictions to all stdlib public methods #15334

Open
nobodywasishere opened this issue Jan 9, 2025 · 5 comments

Comments

@nobodywasishere
Copy link
Contributor

Discussion

I believe it could be really beneficial if all stdlib public methods (ones intended to be used outside of the compiler itself and not implementation details) had parameter and return type restrictions. There are a few reasons for this:

This can be enforced using the new Typing/* rules that will be added in ameba 1.7.0 as part of #14608.

For certain methods such as Value#==(other) where it can take any type, I propose we use underscores to indicate to the user that this is intended to take any type:

struct Value
  # Current code
  def ==(other)
    false
  end

  # Proposed change for parameters that are intended to take any type
  def ==(other : _)
    false
  end
end

Related:

@ysbaddaden
Copy link
Contributor

No need for an underscore type: there's already forall though I believe it's job is mostly to explicit that a type is shared between the args and/or the return type.

def ==(other : F) forall F
end

I think it will mostly fill the stdlib will pointless types, and it will prove nightmarish to get right (see the attempts to type some modules). Either apps will break or we'll have to fallback to forall everywhere which will only impact readability with no benefit in my opinion; there's no point to specify any types everywhere since that's the default.

It might also give the false impression that Crystal is a fully typed language when it definitely isn't: it's a templated language with inferred loose types. Type signatures are mostly constraints, the compiler is allowed to change at will, as long as the constraint stands: it's free to reduce Int32 | Int64 | Nil to Int32 for example.

That being said, please prove me wrong!

@straight-shoota
Copy link
Member

straight-shoota commented Jan 10, 2025

This would be a continuation of #10575, which already added in lots of type restrictions where they make sense.
@ysbaddaden I understand your concerns, and I fully agree that we shouldn't do this when it's pointless or adds unnecessary restrictions.
However, there is a huge value in adding type restrictions when possible in order to document the interface.

Return types in particular should usually be pretty well defined. The method implementation choses them and there's no chance to accidentally get a different type. If we change the type in the future, it would be intentional and we'd express that intention in a change to the return type restriction as well (although it would likely also be a breaking change).

Parameter types are often more fuzzy due to duck typing. But still I'm sure we can often enough add some type restrictions. This requires careful handling and manual control, of course.

@ysbaddaden
Copy link
Contributor

We can type more. My concern is towards typing everything, which makes little sense as explained above.

@nobodywasishere
Copy link
Contributor Author

My concern is towards typing everything, which makes little sense as explained above.

That is fair. If there are some things that cannot be typed without breaking changes, I don't think there's anything we can do for that.

I guess this issue is more along the lines that existing APIs that can be typed, and any new APIs that are added to the stdlib should have type restrictions.

there's no point to specify any types everywhere since that's the default.

That is also fair. For me the difference is some methods are intended to take certain types, and others (like Object#==) are intended to take anything, and explicitly putting _ or forall helps differentiate the former from the latter. If there isn't a type restriction, I often need to dig into the source code to understand it before I can use it. I'm fine with not having the any types everywhere though if people think it's not beneficial.

@nobodywasishere
Copy link
Contributor Author

One idea I'm playing around with is to look into adding non-restrictive type restrictions (more akin to type hints) using unions with underscore. From a tooling and documentation perspective, having some idea of what is acceptable is better than none. For example:

def hello(names) : Nil
  # I have no idea the type of `names` and cannot do any semantic analysis
  # without resolving the types of all the calls

  puts "Hello, #{names.join(", ")}!"

  names.each do |name|
    # Same with `name`
  end
end

hello(%w(George Sam Jeff)) # => "Hello, George, Sam, Jeff!"
hello([1, 2, 3])           # => "Hello, 1, 2, 3!"
def hello(names : Array(String) | _) : Nil
  # I immediately know that `names` is an `Array`-like thing and 
  # can do some (imprecise) semantic analysis

  puts "Hello, #{names.join(", ")}!"

  names.each do |name|
    # With some semantic analysis using just the top-level semantic and this method,
    # I can figure out the type of `name` as a `String`-like thing and provide
    # autocomplete and jump-to-definition accordingly, while not requiring
    # the caller to follow the exact restriction.
  end
end

hello(%w(George Sam Jeff)) # => "Hello, George, Sam, Jeff!"

# Doesn't follow exact type hint but is allowed
hello([1, 2, 3])           # => "Hello, 1, 2, 3!"

Maybe a shorthand similar to String* for Pointer(String) and String? for String | Nil could be added to ease with writing these, where String~ could be expanded to String | _:

def hello(names : Array(String)~) : Nil
  puts "Hello, #{names.join(", ")}!"
end

hello(%w(George Sam Jeff)) # => "Hello, George, Sam, Jeff!"
hello([1, 2, 3])           # => "Hello, 1, 2, 3!"

Plus when reading documentation, it could help the reader have some idea about what a method would take without needing to dive into the source code.

I don't know how this would impact type resolving / method overloading though, so that would require more research. Feedback is welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants