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

Fix crash on generic methods #762

Merged
merged 4 commits into from
Mar 1, 2025

Conversation

apiology
Copy link
Contributor

@apiology apiology commented Feb 28, 2025

Fix crash on methods declared as independently generic. This doesn't yet support inferring types on methods which are generic based on their parameter, but at least avoids crashing when they are defined:

/Users/broz/src/solargraph/lib/solargraph/complex_type/unique_type.rb:71:in `resolve_generics': undefined method `generics' for an instance of Solargraph::Pin::Method (NoMethodError)

          idx = definitions.generics.index(subtypes.first&.name)
                           ^^^^^^^^^
	from /Users/broz/src/solargraph/lib/solargraph/complex_type.rb:118:in `block in resolve_generics'
	from /Users/broz/src/solargraph/lib/solargraph/complex_type.rb:118:in `map'
	from /Users/broz/src/solargraph/lib/solargraph/complex_type.rb:118:in `resolve_generics'
	from /Users/broz/src/solargraph/lib/solargraph/source/chain.rb:132:in `block in infer_first_defined'
	from /Users/broz/src/solargraph/lib/solargraph/source/chain.rb:123:in `each'
	from /Users/broz/src/solargraph/lib/solargraph/source/chain.rb:123:in `infer_first_defined'
        from /Users/broz/src/solargraph/lib/solargraph/source/chain.rb:85:in `infer'

This doesn't yet support inferring types on methods which are generic
based on their parameter, but at least avoids crashing when they are
defined:

```
/Users/broz/src/solargraph/lib/solargraph/complex_type/unique_type.rb:71:in `resolve_generics': undefined method `generics' for an instance of Solargraph::Pin::Method (NoMethodError)

          idx = definitions.generics.index(subtypes.first&.name)
                           ^^^^^^^^^
	from /Users/broz/src/solargraph/lib/solargraph/complex_type.rb:118:in `block in resolve_generics'
	from /Users/broz/src/solargraph/lib/solargraph/complex_type.rb:118:in `map'
	from /Users/broz/src/solargraph/lib/solargraph/complex_type.rb:118:in `resolve_generics'
	from /Users/broz/src/solargraph/lib/solargraph/source/chain.rb:132:in `block in infer_first_defined'
	from /Users/broz/src/solargraph/lib/solargraph/source/chain.rb:123:in `each'
	from /Users/broz/src/solargraph/lib/solargraph/source/chain.rb:123:in `infer_first_defined'
        from /Users/broz/src/solargraph/lib/solargraph/source/chain.rb:85:in `infer'
```
@lekemula
Copy link
Contributor

Nice catch! Are there any methods in Ruby's Standard Library (rbs signatures) that use standalone generic methods?

@apiology
Copy link
Contributor Author

Easy example:
array = [1,2,3] # type is Array
array.fetch(1, "foo") # type is Union<Integer, String>

A couple of others, also in Array - but other classes do this as well:

  • The singleton method [] - the function that turns [1,2,3] into an Array object.
  • Array#+, which returns Array<Union<A, B>>, if A and B are the types on the left and right hand side

@apiology
Copy link
Contributor Author

I was trying to get this to do something useful with type checking, but it's probably worth populating the generics variable now from RBS in case someone beats me to do it - will push up that commit in a moment

@castwide
Copy link
Owner

castwide commented Mar 1, 2025

This definitely looks like a move in the right direction. I'm happy to merge it now and continue testing it locally. Thanks, @apiology!

@castwide castwide merged commit 56be2ad into castwide:master Mar 1, 2025
8 checks passed
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