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

Custom decorator handler vs alias #1047

Closed zverok closed 7 years ago

zverok commented 7 years ago

Steps to reproduce

File to document (t.rb):

class Test
  memoize def payment_group # <= here is call to Memoist as method decorator
  end

  def bar
  end

  alias subject payment_group # <= here would be warning
end

Handler to process memoize decorator (as advised in https://github.com/lsegal/yard/issues/994):

class MemoizeHandler < YARD::Handlers::Ruby::Base
  include YARD::Handlers::Ruby::DecoratorHandlerMethods
  handles method_call(:memoize)
  namespace_only

  process do
    case statement.type
    when :fcall, :command
      process_decorator
    end
  end
end

Actual Output

While processing the t.rb, YARD is suddenly produces a warning:

[warn]: Unknown tag @decorator in file `lib/t.rb` near line 8

(all methods and aliases are documented, though.)

Expected Output

No warning, probably :)

Environment details:

I have read the Contributing Guide.

lsegal commented 7 years ago

That warning seems unrelated. Are you using some @decorator tag? It's likely YARD was not processing that line before and started to after parsing the block, creating the warning. If you can link to the full lib/t.rb, that would be helpful.

zverok commented 7 years ago

Are you using some @decorator tag? ... If you can link to the full lib/t.rb, that would be helpful.

The issue demonstrates a full, reproducible example. You can check it yourself, with those .yardopts:

--load=./test_handler.rb

Just check it. Looks like deeply inside process_decorator something is erroneously added as decorator tag. I'll try to debug tomorrow.

zverok commented 7 years ago

I believe (through testing), that @decorator tag is added by this handler processing code: https://github.com/lsegal/yard/blob/master/lib/yard/handlers/ruby/decorator_handler_methods.rb#L111

Yet currently I can't understand clearly what it meant to do.

zverok commented 7 years ago

Fixing MemoizeHandler as follow solves the issue. Though, I have absolutely no idea what's going on, still.

class MemoizeHandler < YARD::Handlers::Ruby::Base
  include YARD::Handlers::Ruby::DecoratorHandlerMethods
  handles method_call(:memoize)
  namespace_only

  process do
    case statement.type
    when :fcall, :command
      process_decorator do |m, *arg|
        m.docstring.delete_tags('decorator') # The "Fix"
      end
    end
  end
end

Probably, @amclain, being author of decorators commit, could guess?..

amclain commented 7 years ago

I believe (through testing), that @decorator tag is added by this handler processing code: https://github.com/lsegal/yard/blob/master/lib/yard/handlers/ruby/decorator_handler_methods.rb#L111

Yet currently I can't understand clearly what it meant to do.

@decorator was designed to label decorated methods in the generated docs so it's more clear that they're being modified (and possibly add info about how they're being modified). I don't think YARD is using this yet, but it's possible to use it on a project-by-project basis with a custom extension.

As far as the bug goes, it looks like it may be trying to tag the alias statement incorrectly. Looks like I didn't add a test for aliasing a decorated method. :zipper_mouth_face: @zverok is this something you want to fix since you've done the work to find the bug, or do you want me to fix it?

zverok commented 7 years ago

@amclain To be honest, for me it is still hard to get how to fix it properly :( Plus I am super overwhelmed with primary work tasks, plus I've found the "quick-fix" (shown above) which is enough for my current problem. So, I'd be happy to fix, but not sure I can do it in some near time.

lsegal commented 7 years ago

Can we remove the @decorator creation in the handler? That would be the easiest fix. I'm not sure that there's a good reason to have this (custom plugins can do their own tag addition).

amclain commented 7 years ago

Sure, I'll make a PR to remove the tag.