Better support for generics via Class @generic tags

[lekemula]

Closes: #681

This PR adds type inference for generics/parameterized params for @return and @yieldparam tags.

I went with @param instead of adding a custom tag for simplicity reasons. The tag is already parsed by YARDoc. Also, I think it's better to have fewer tags for developers to memorize.

Does it make sense to you? Let me know if you have another opinion here.

Screenshots

Next steps

• Adjust documentation accordingly based on inferred types - right now raw yardoc [param<...>] is returned.

  Example

  
  
  

• Deprecate @return_single_parameter and @yieldparam_single_parameter(?)

[lekemula]

Interesting diff :D

[lekemula]

@castwide, in regards of:

Deprecate @return_single_parameter and @yieldparam_single_parameter(?)

Here you can find the draft PRs:

• @yieldparam_single_parameter - Draft PR - Remove @yieldparam_single_parameter  lekemula/solargraph#1
  • It looks like the new implementation seems to play nicely with RBS maps already 🔥
• @return_value_parameter & @return_single_parameter Draft PR - Remove (unused) @return_value_parameter & @return_single_parameter lekemula/solargraph#2
  • These tags seem to be not in use already. Does it make sense to remove them, or do you still see any of them being used in the future?

[castwide]

Thanks, @lekemula! I've been thinking about better ways to handle parametrized type inference, and your solution is very close to how I pictured it.

Good to see that it also works with multiple parameters, e.g., when a Hash-like object needs key and value parameters:

# @param Key
# @param Value
class Map
  # @yieldparam [param<Key>]
  # @yieldparam [param<Value>]
  def each(&block); end
end

# @return [Map<String, Integer>]
def map_method
  Map.new
end

map_method.each do |key, value|
  key._ # Completion for String
  value._ # Completion for Integer
end

YARD doesn't complain about @param tags attached to class definitions, but it's a use case that's not covered in the specification. We might want to get some input from @lsegal to make sure we're not introducing undesirable behavior in things like generated docs.

I can think of a couple ways to extend the syntax around this feature, but we can probably leave those to future enhancements.

As for the @return_single_parameter and @yieldparam_single_parameter tags, both are definitely candidates for deprecation. They started as a quick and dirty way to infer yieldparams for Enumerable methods back before RBS existed.

[castwide]

I opened a YARD issue to ask about the param syntax and he made a strong argument for using a custom tag instead of repurposing the @param tag. His suggestion of @generic seems reasonable. There's no problem with using param<T> in types.

My example, modified:

# @generic [Key, Value]
class Map
  # @yieldparam [param<Key>]
  # @yieldparam [param<Value>]
  def each(&block); end
end

# @return [Map<String, Integer>]
def map_method
  Map.new
end

map_method.each do |key, value|
  key._ # Completion for String
  value._ # Completion for Integer
end

What do you think, @lekemula?

[castwide]

It also occurred to me that we can bypass the need for a class tag if we can reference the type parameters by index:

class Map
  # @yieldparam [param<0>]
  # @yieldparam [param<1>]
  def each(&block); end
end

[lekemula]

@castwide Thanks for starting the discussion there! I also like the @generic tag suggestion as a more explicit way to convey its purpose.

(which would clearly indicate to users that the class involves generics)

My main concern, however, is ensuring that @generic tags defined in third-party gems get indexed properly. If we introduce this new custom tag in solargraph, wouldn’t that mean the yard gems command wouldn’t parse it? If so, is there an easy way to work around this?

Regarding the "indexed parameters" approach, I have mixed feelings. On one hand, it removes the need to manually add extra tags to the class. On the other hand, it feels somewhat obscure to me. In the age of AI, typing feels less and less like a burden - especially for trivial annotations like YARD - not that it ever really mattered to me.

I also think we might lose the "indication" that @lsegal mentions in his feedback. However, one could argue that this is more of yard's responsibility rather than the LSP's.

Additionally, this approach seems somewhat incompatible with RBS. As far as I know, we’d need to map "named generics" to "indexed generics/params," which could introduce additional complexity. If those mapped RBS indexed generics also appear in method documentation (in place of param<Elem> in the example below), it could add further confusion for developers.

.

TL;DR: I’m open to both approaches, but I prefer the @generic tag - ideally with a way to ensure it works for third-party gems as well. That said, I trust your expertise more than mine, so I’m happy to go any of your suggestion.

[castwide]

Good points about named vs. indexed parameters. Let's go with the @generic tag. I might revisit indexed parameters later, but right now it's just a maybe-nice-to-have.

I've been planning to publish a separate yard-solargraph extension. Other gems would be able to include it in their YARD configurations so Solargraph-specific tags would be included in generated yardocs. Currently I think the only two tags we would need to include are @generic and @yieldreceiver (see lsegal/yard#1491). Barring native YARD support, an extension should work well enough.

For now, we can add @generic to lib/yard-solargraph.rb, and I'll split it into a separate extension afterwards. (I already have projects where I'm eager to have @yieldreceiver work correctly, so I plan to do it ASAP.)

If you want any help updating the PR, just let me know.

[lsegal]

My main concern, however, is ensuring that @generic tags defined in third-party gems get indexed properly. If we introduce this new custom tag in solargraph, wouldn’t that mean the yard gems command wouldn’t parse it? If so, is there an easy way to work around this?

I'm not exactly sure how solargraph works, but if you're in charge of running yard gems, YARD.parse or equivalent, you can define your own YARD tags. In the case of generic it would be simply making sure you had the following somewhere in your runtime:

YARD::Tags::Library.define_tag("Generic", :generic, :with_types)

In the case of running with yard gems, it might require you to run the command in a slightly different way (maybe yard gems -e /path/to/the/file/with/the/above/ruby/code.rb or use --plugin).

This would allow you to parse these tags regardless of if library has done so (though they probably would want to in order to generate their own docs).

[lekemula]

This was needed for the unit test to pass.

[lekemula]

Thanks @lsegal, for your helpful information! 🙏

In the case of running with yard gems, it might require you to run the command in a slightly different way (maybe yard gems -e /path/to/the/file/with/the/above/ruby/code.rb or use --plugin).

As far as I know, Solargraph recommends running yard gems directly. However, we should be able to create a wrapper or shim around yard gems based on the suggestion above. I'm unsure, though, how we could maintain the convenience of yard config --gem-install-yri.

@castwide will you be able to look into this, or should I give it a try on a separate PR?

[castwide]

Thanks, @lekemula! This is awesome.

Good call on renaming Namespace#parameters. I think #generics fits well.

Along the same lines, do you think we should use generic instead of param to clarify the relationship between the two?

# @generic T
class Example
  # @yieldparam [generic<T>]
  def evaluate; end
end

Regarding yard gems: I think the first step is to release the yard-solargraph extension so gem authors can opt into the additional tags. That should help us determine what else, if anything, we need to do to support them.

[castwide]

💯

I've also started work on the yard-solargraph plugin.

[lekemula]

I've also started work on the yard-solargraph plugin.

Appreciate that @castwide and thanks for the great feedback. That will be really helpful in allowing third-party gems to provide a better developer experience for their users—hoping it gets adopted by those already using YARD comments!