lsegal / yard

YARD is a Ruby Documentation tool. The Y stands for "Yay!"
http://yardoc.org
MIT License
1.94k stars 397 forks source link

Question: parse docstring for classes defined with `@!parse` directive? #1475

Closed pirj closed 1 year ago

pirj commented 1 year ago

Would it make sense to consume the docstring from code and use it for the generated code defined with @!parse?

This can be helpful to make copies of classes, e.g. when renaming, changing namespaces, renaming of Sidekiq jobs with a fallback etc, dynamically define classes in meta way etc.

Steps to reproduce

Real-life example

This works, but the documentation has to be double-commented:

      module Capybara
        # @!parse
        #   # Checks for usage of deprecated style methods.
        #   #
        #   class MatchStyle < ::RuboCop::Cop::Base; end
        MatchStyle = ::RuboCop::Cop::Capybara::MatchStyle
      end

This doesn't work (produces no class doc for Capybara::MatchStyle):

      module Capybara
        # Checks for usage of deprecated style methods.
        #
        # @!parse
        #   class MatchStyle < ::RuboCop::Cop::Base; end
        MatchStyle = ::RuboCop::Cop::Capybara::MatchStyle
      end

I've researched this down to ParseDirective, and the handler is YARD::Handlers::Ruby::ConstantHandler, and the docstring isn't making its way to YARD::Registry.all(:class).

The Ruby style guide recommended way of defining single-line classes says to define classes normally, not with MyClass = Class.new(BaseClass), but the latter notation is a valid Ruby construct and deserves an option to be properly documented, too.

Change @!parse or introduce @!class?

There is a @!method directive and @!attribute that make it easy to define methods/attributes in unusual ways.

Should @!parse pick up and use the accumulated docstring? Or would it make sense to introduce @!class?

Actual Output

""

Expected Output

The docstring is picked from what comes before the @!parse.

Environment details:

lsegal commented 1 year ago

Apologies for the late reply on this:

I generally don't think it would make sense to grab the docstring from the example you've provided. I think there are cases where this makes sense, but there are alternative cases where it does not.

The bigger issue here is that because of the latter statement (there are cases where this would be wrong to do), this means that flipping this behavior would be a breaking change in those situations, and thus, it would be really difficult to make this change safely.

pirj commented 1 year ago

No objections to closing this, keeping in mind there is an alternative way of achieving the same result. Subjectively, it is less intuitive. Having this ticket in the history will eventually show if it is even worth documenting, or it is an extremely rare corner case. Thanks!