Solargraph fails to determine hash value types (IntelliSense)

[SnowSzn]

Hi,

Recently I realized that hash types cannot be determined when accesing a value, even if the hash is documented with the @type YARD tag.

No errors are raised but, as a result, Solargraph fails to show information like methods and autocomplete when the intellisense contextual menu is used.

Take the example below:

class Foo
  #
  # Returns foo
  #
  def foo
    "foo"
  end
end

# @type [Hash<Integer, String>]
hash_a = {1 => "a", 2 => "b", 3 => "c"}
# @type [Hash<Integer, Foo>]
hash_b = {1 => Foo.new, 2 => Foo.new, 3 => Foo.new}

value_a = hash_a[1]
value_b = hash_b[1]
p value_a
p value_b

Unless I'm wrong I remember using the YARD tag @type for hashes and it worked in previous versions

Maybe worth to mention that arrays do not have this problem, the type is properly resolved and everything works as expected.

I'm using the latest solargraph version 0.55.0 and VSCode version 1.100.3

[apiology]

This works:

# @type [Hash{Integer => Foo}]

The YARD docs describe common conventions as:

Hashes can be specified either via the parametrized type discussed above, in the form Hash<KeyType, ValueType>, or using the hash specific syntax: Hash{KeyTypes=>ValueTypes}. In the latter case, KeyTypes or ValueTypes can also be a list of types separated by commas.

So absent good reason, we should match that behavior, support both, and add regression tests.

Incidentally, YARD's own example parser gets this wrong as well: https://yardoc.org/types

[apiology]

(out of the scope of this issue, but I'd like to have solargraph probe the types from literal hashes as well so that you could avoid explicit annotations)

[SnowSzn]

This works:

# @type [Hash{Integer => Foo}]

Thank you, I completely missed this type syntax for hashes, it works now!

The YARD docs describe common conventions as:

Hashes can be specified either via the parametrized type discussed above, in the form Hash<KeyType, ValueType>, or using the hash specific syntax: Hash{KeyTypes=>ValueTypes}. In the latter case, KeyTypes or ValueTypes can also be a list of types separated by commas.

So absent good reason, we should match that behavior, support both, and add regression tests.

Incidentally, YARD's own example parser gets this wrong as well: https://yardoc.org/types

If possible, that would be very appreciated, my codebase has parametrized type syntax all over the place and that would save me having to change all hash types comments

(out of the scope of this issue, but I'd like to have solargraph probe the types from literal hashes as well so that you could avoid explicit annotations)

Sounds good, that would reduce the need have @type annotations for hashes.

Not demanding this but I'm genuinely curious, how possible would it be for Solargraph to determine the type of a hash based on what is inserted?

For example:

class Foo
  # @return id [Integer]
  attr_reader :id

  #
  # Foo
  #
  # @param id [Integer]
  #
  def initialize(id)
    @id = id
  end
end

class Bar
  #
  # Bar
  #
  def initialize
    # Generic type hash
    @hash = {}
  end

  #
  # Stores a foo object
  #
  # @param foo [Foo] Foo object
  #
  def store(foo)
    # Assume the key->value pair (Integer->Foo) as a possible type for this hash
    # among the rest of types the hash may have in this class context
    @hash.store(foo.id, foo)
  end
end

[lekemula]

Not demanding this but I'm genuinely curious, how possible would it be for Solargraph to determine the type of a hash based on what is inserted?

@SnowSzn I don't think that is possible right now, because solargraph AFAIK does not track variable usage to determine the types.

If you already know the foo param's type ahead of time, you'd need to type the @hash by yourself.

If you'd like to reference Bar class as generic, though, what you can do is use @generic tag like this:

class Foo
  # @return id [Integer]
  attr_reader :id

  #
  # Foo
  #
  # @param id [Integer]
  #
  def initialize(id)
    @id = id
  end

  def my_foo
  end
end

# @generic Index
# @generic T
class Bar
  def initialize
    # @type @hash [Hash<generic<index>, generic<T>>]
    @hash = {}
  end

  #
  # Stores a foo object
  #
  # @param foo [generic<T>]
  # @return [void]
  def store(foo)
    @hash.store(foo.id, foo)
  end

  # @param id [generic<Index>]
  # @return [generic<T>]
  def get(id)
    @hash[id]
  end
end

# @generic T
# @param foo [generic<T>]
# @return [Bar<Integer, generic<T>>]
def store(foo)
  bar = Bar.new
  bar.store(foo)
  bar
end


store(Foo.new(1)).get(1).my_foo
               # ^^^ returns [Foo]

# or

# @type my_store [Bar<Integer, Foo>]
my_store = Bar.new
my_store.store(Foo.new(1))
my_store.get(1).my_foo
       # ^^^ returns [Foo]

Hope this helps somehow.

[apiology]

Not demanding this but I'm genuinely curious, how possible would it be for Solargraph to determine the type of a hash based on what is inserted?

Ah, sorry, my comment was just about literal hashes, where we can base the type assumptions on the initial values:

hash_a = {1 => "a", 2 => "b", 3 => "c"} # Integer => String
hash_b = {1 => Foo.new, 2 => Foo.new, 3 => Foo.new} # Integer => Foo

We do this today with Arrays:

Spec: https://github.com/castwide/solargraph/blob/b036fbebb9fc87089ea15c96565443e075a98751/spec/source/source_chainer_spec.rb#L273-L284

Implementation: https://github.com/castwide/solargraph/blob/master/lib/solargraph/source/chain/array.rb#L24-L28

If the user gets a type error later on, and the code is correct (imagine inserting a Bar as a value into hash_b, they're no worse than they originally were - they can just go add a @type annotation to provide a broader type like Hash{Integer => Object} or Hash{Integer => Foo,Bar}.

[SnowSzn]

Not demanding this but I'm genuinely curious, how possible would it be for Solargraph to determine the type of a hash based on what is inserted?

@SnowSzn I don't think that is possible right now, because solargraph AFAIK does not track variable usage to determine the types.

If you already know the foo param's type ahead of time, you'd need to type the @hash by yourself.

If you'd like to reference Bar class as generic, though, what you can do is use @generic tag like this:

Hope this helps somehow.

Not demanding this but I'm genuinely curious, how possible would it be for Solargraph to determine the type of a hash based on what is inserted?

Ah, sorry, my comment was just about literal hashes, where we can base the type assumptions on the initial values

We do this today with Arrays

If the user gets a type error later on, and the code is correct (imagine inserting a Bar as a value into hash_b, they're no worse than they originally were - they can just go add a @type annotation to provide a broader type like Hash{Integer => Object} or Hash{Integer => Foo,Bar}.

@apiology @lekemula Thanks for your insight! That was helpful, using @type is good enough for my codebase since the type is always the same and known, appreciate it.

I will leave the issue open in case you want to add support for parametrized type tags for hashes (Hash<Integer, Foo>), however feel free to close it otherwise!