Yard not detecting boolean undocumented methods

mensfeld commented 7 years ago

YARD does not return errors upon undocumented methods with question mark at the end

Steps to reproduce

Create file like that:

class Test
  def test?
  end

  def test
  end
end

Then run yard:

 yard stats --list-undoc --private --debug ./*
[debug]: Parsing ["./doc", "./test.rb"] with `ruby` parser
[debug]: Parsing ./test.rb
[debug]: Serializing to .yardoc/objects/root.dat
Files:           1
Modules:         0 (    0 undocumented)
Classes:         1 (    1 undocumented)
Constants:       0 (    0 undocumented)
Attributes:      0 (    0 undocumented)
Methods:         2 (    1 undocumented)
 33.33% documented

Undocumented Objects:

(in file: test.rb)
Test
Test#test

Expected Output

yard stats --list-undoc --private --debug ./*
[debug]: Parsing ["./doc", "./test.rb"] with `ruby` parser
[debug]: Parsing ./test.rb
[debug]: Serializing to .yardoc/objects/root.dat
Files:           1
Modules:         0 (    0 undocumented)
Classes:         1 (    1 undocumented)
Constants:       0 (    0 undocumented)
Attributes:      0 (    0 undocumented)
Methods:         2 (    2 undocumented)
0% documented

Undocumented Objects:

(in file: test.rb)
Test
Test#test
Test#test?

Environment details:

OS: Ubuntu 14.04
Ruby version (ruby -v): ruby 2.4.0p0 (2016-12-24 revision 57164) [x86_64-linux]
YARD version (yard -v): yard 0.9.9
Relevant software dependency/versions: no deps

nijikon commented 7 years ago

It looks like this is broken like forever. I tested up to 0.8.7.6 and it does not work on any.

lsegal commented 7 years ago

YARD generates boilerplate documentation for these, which is why they don't show up. We could fix this by tweaking the way those boilerplate docs are identified.

mensfeld commented 7 years ago

@lsegal that's what I thought but IMHO it shouldn't work that way or at least should be configurable. I know this breaks the "?" pattern, but those methods can return monads/futures/promises and not always "direct" booleans.

My case is kinda specific since I track documentation in my projects and monitor it's quality - default behavior makes it a bit harder, since it does not mark those methods as undocumented.

nijikon commented 7 years ago

Any update on this?

nijikon commented 7 years ago

@lsegal ping?

ghost commented 7 years ago

I know this breaks the "?" pattern, but those methods can return monads/futures/promises and not always "direct" booleans.

Although it is common for methods that end with a '?' to return a boolean value, this is not enforced by the ruby parser itself. I do sometimes use methods with a trailing '?' that do not return boolean either.

The documentation should reflect that fact based on the ruby parser itself. There are a lot of ... conventions that are sometimes awkward - I don't mean the '?' convention but more like the content of the rubocop style guide.

lsegal commented 7 years ago

@nijikon @shevegen @mensfeld

The general answer here is that patches would be accepted to fix this, but they do technically have documentation. If the documentation generated by YARD is wrong, the workaround here is to simply document these methods.

If the issue is being able to identify these methods, you can use different queries or heuristics to display undocumented booleans in your project:

$ yard list --query 'o.name.to_s.end_with?("?") && o.docstring == ""'

mensfeld commented 7 years ago

but they do technically have documentation Well technically then don't. IMHO (and I hope @nijikon and @shevegen agree with me on that) assigning a theoretical documentation based on theoretical concept (that is not one and only format standard for ruby) is just wrong.

The command you presented can be indeed useful and than you for that one but I still believe that this problem should be patched as it also impacts the consistency of yard API and warnings. If we go with the "?" why not go further and do the same with "!" and "get" etc assigning a theoretical doc for those two as well (! informing that it modifies the object on which it runs and for "get" that it returns something, etc).

It's also worth mentioning that it also not only breaks the consistency but can be misleading as a non documented "?" method will have a boolean return documentation. This can be extremely confusing for people new to the code that just need to get a grasp of a public semi-documented api based on the yard output.

lsegal commented 7 years ago

assigning a theoretical documentation based on theoretical concept (that is not one and only format standard for ruby) is just wrong.

YARD has been doing this for the last 8+ years with no complaints prior to this issue, so this assertion may be somewhat overstated.

If we go with the "?" why not go further and do the same with "!" and "get_" etc assigning a theoretical doc for those two as well ...

"!" isn't a great example because that doesn't influence any type information (ignoring the fact that the "!" convention is not as strict)-- "get_" is also a bad example, since that's not a convention in Ruby, and probably an anti-pattern (you should use "foo", not "get_foo"). That said, your overall point is well taken, and YARD does infer documentation for a number of other methods, actually:

In other words, you are very likely already making use of the same type of documentation inference.

It's also worth mentioning that it also not only breaks the consistency but can be misleading as a non documented "?" method will have a boolean return documentation.

I don't really buy this-- having return information for a method that is reliably boolean is only misleading if you left methods in your code undocumented and decided not to follow conventions. That should be a very small edge case, and the workaround here is to simply add documentation to your method.

As mentioned: patches would be accepted to improve the way synthesized documentation works alongside the stats calculation commands, but as it stands, this behavior is working as intended. I am going to mark this as closed since there is no work being done to address this directly, but if you want to open a PR to improve stats, that can be looked at.

PS. Another workaround could be to write a plugin to remove these tags as you see fit. The code would be something like:

# myplugin.rb
YARD::SourceParser.after_parse_list do
  YARD::Registry.all(:method).each do |meth|
    if meth.tag(:return).types == ['Boolean'] && meth.docstring == ''
      meth.docstring = '' # reset the docstring
    end
  end
end

And you would run it with yard doc -e ./path/to/myplugin.rb

lsegal / yard