Allow protected methods on API docs

[Blacksmoke16]

Allows protected methods to be displayed on the generated API docs.

Example:

[vladfaust]

Actually useful for me!

[straight-shoota]

I'm not sure if there has ever been a discussion about this. But I'm actually fine with not showing protected methods in the API docs. Until now I have never encountered a case where this would be useful.
The way protected methods are used, at least in the stdlib, is essentially as internal implementation methods, similar to private, just callable from other scopes.
And it makes sense to hide such methods from the docs. When a method is supposed to be public API, make it public.

Obviously, methods that are not supposed to be shown in the docs can be nodoc-ed, while the opposite is not possible. But still, I'm not sure if there is a good reason to show protected methods in the docs.
I'm curious about your use cases for this.

[Blacksmoke16]

I agree that private methods should not be shown in the public API docs since they are mostly used internally and not exposed to the end user. However protected methods should be shown since the end user could override them in their subclasses to change the behavior of that subclass.

My specific use case would be for a logging shard I am working on. As I showed in the screenshot, I have a protected default_formatter method defined in a module, that would return the default formatter to use.

  # The formatter to use, defaults to `Crylog::Formatters::LineFormatter`.
  setter formatter : Crylog::Formatters::LogFormatter? = nil

  # Returns the `Crylog::Formatters::LogFormatter` defined on `self`, or `#default_formatter` if none was set.
  def formatter : Crylog::Formatters::LogFormatter
    (f = @formatter) ? f : default_formatter
  end

  # Can be overridden to use a different formatter on a handler.
  protected def default_formatter : Crylog::Formatters::LogFormatter
    Crylog::Formatters::LineFormatter.new
  end

I then have a public formatter method that returns either the one that set on the handler, or the default one. Displaying the protected default_formatter method in the API docs allows the end user to know that changing the default formatter is possible by simply overriding this method in their custom handler, plus any other handlers inheriting from it, if it was abstract. Example could be having an abstract struct MailHandler that overrides it and sets the default formatter to be like HTMLFormatter.

It makes sense to have it protected as the user should not use it directly, but to just change the behavior of their handler.

[straight-shoota]

the end user could override them in their subclasses to change the behavior of that subclass.

Exactly the same is true for private methods.

Why is your default_formatter method protected at all? For the purpose of overriding it in subclasses, private would be fitting. But when it's part of the public API, it could as well be public. But protected actually seems to make the least sense here.

[vladfaust]

# This module is part of a third-party shard, it has API docs online
module Foo
  # This method must not be called outside including object,
  # but it actually has some logic inside, which could be described
  # in this very comment. But it's not included into API docs
  protected foo
  end
end

# This object is part of my application
class ActualObject
  include Foo

  def call
    # What does `Foo#foo` do? I could have read it from the shard's API docs online, 
    # but it's not there, so I must jump into its source code, what a bummer
    foo 
  end
end

[Blacksmoke16]

Why is your default_formatter method protected at all?

Because I wanted it to be in the middle ground between private and public. I.e. not public so it couldn't be used on its own, but not private so it would show up in the docs (which currently isn't the case). IMO protected implies that it still could be used by an end user, while private implies like "hey this is used internally, override it your own risk".

[wooster0]

What about adding :doc: or something similar that can be applied to protected methods so they will be included? Instead of just including all the protected methods in the documentation. This allows for more customization and maybe :doc: can even be used for private methods then if you want to include them in the documentation for whatever reason.
I'm pretty sure there are more protected methods that should not be in the documentation rather than protected methods that should be in the documentation so maybe this way should be considered.
So people who want this can use :doc: and for the others who don't want this nothing changes.

[straight-shoota]

@Blacksmoke16 By the same argument, private methods should also be documented as they can be used by the end user to override behaviour in implementing types.

In my opinion it makes sense to consider both private and protected methods implementation details. And everything that's relevant to using the API should be public and documented.
That's a simple convention and should be working for most use cases I can think of.

An opt-in flag like :doc: could be useful in some cases, but in general I'd prefer the simplicity of {protected,private} => undocumented, {public} => documented.

[bcardiff]

If protected methods are to be shown, I would put them in a separate section so the user of the types is not bothered with them.

[Blacksmoke16]

What about adding :doc: or something similar that can be applied to protected methods so they will be included?

I could go with this. I could see how including ALL protected methods might not be the best idea. This would only document public methods by default, but at least give the ability to document whatever the developer wants if they so choose, which is currently not possible.

If protected methods are to be shown, I would put them in a separate section so the user of the types is not bothered with them.

What about having sub sections under class/instance method sections that are categorized by type. In the order of public => protected => private. For example:

Class Method Summary

Public

.some_public_method

Protected

.some_protected_method

Then of course if there aren't any protected/private those sub sections would just be omitted.

[vladfaust]

🏓

[straight-shoota]

I'm still missing a convincing use case for publicly documented methods with protected (or private) visibility.

[bcardiff]

I would not add a :doc:.

I do see value in adding some documentation so inheritors can know what to do, but for that use case, it will be better IMO to have them in a different section as I pointed out before.

[Blacksmoke16]

@bcardiff Is that assuming that protected/private methods would get documented by default, but just appear in their own section?

The idea around :doc: was to maintain current behavior by NOT generating docs for protected/private methods, but allow the developer to do so if they wish.

[j8r]

A solution is to have protected and private methods/objects hidden by default, then having an option to enable/disable them.

[Blacksmoke16]

Yes, that was the purpose behind :doc: that @r00ster91 suggested.

[straight-shoota]

@bcardiff

I do see value in adding some documentation so inheritors can know what to do

But why not simply make methods relevant to inheritors public? I don't see much sense in having these visibility modifiers except for saying "this method is not relevant to the public API". If we remove that, we should consider whether we need protected and private at all. Having them solely for a restriction of caller scope (like in Java for example) doesn't sound very compelling to me. Especially considering that in Crystal types can easily be reopened.

[j8r]

I mean, on the Web UI @Blacksmoke16 , no code to change. Having a different color scheme for private and protected methods may also be a good idea.

[bcardiff]

I don’t see a need to add :doc:. I would make the default for protected to appear in their own section and add :nodoc: if that is not wanted. So we keep the same behavior for all methods.

The nodoc protected method will be used to highlight that the hierarchy behavior is expected to not be overridden outside the shard. (But it is required to be protected because of designs).

Users of a package will usually do not care about protected method, so I don’t want to clutter the public docs.

[asterite]

Maybe we can add a flag to the doc generator to include protected methods in docs. Then if a library wants to expose this information they do it. Then it doesn't need to necessarily be in a separate section.

[straight-shoota]

This all sounds like much additional complexity. Much more than necessary.

Why not just leave everything as it is?
Is there any downside to simply have every method that should show up in the API docs be public? Or put differently: Is there a reasonable benefit from being able to restrict the visibility to protected or private while still being able to document the method?

[asterite]

I agree with @straight-shoota

[Blacksmoke16]

I can create an issue to move the discussion there.

Or put differently: Is there a reasonable benefit from being able to restrict the visibility to protected or private while still being able to document the method?

I think there is. Whether others agree with me is another matter. The best example would be when dealing with inheritance, where you have abstract parent classes that use protected methods to control some of logic in the child/parent classes without allowing them to be used outside of the class. This change would allow these methods to be documented so people extending from it are aware of them, without having to document it in some readme or in code.

[crysbot]

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/documenting-protected-methods/6765/2

[nobodywasishere]

The output of the documentation generator is not just used for the API documentation website, but can give semantic information about the types/methods available in a program that can be used for other things (such as IDE capabilities). Limiting this to only public methods prevents the autocomplete of private/protected methods within relevant areas of the code.

[nobodywasishere]

Relevant: #14816

[stephannv]

I think we're dealing with two different concepts of public/private:

Public/Private for Users:
Public: The method is intended for the user, so it should have documentation even if it is private/protected.
Private: The method isn't meant for user interaction, so documentation isn't necessary.

Public/Private for Objects:
Public: The method can be called externally, from outside the class.
Private: The method is only accessible within the class itself.

If a method is designed for user interaction, it can have documentation. While hiding private and protected methods by default makes sense, it would be useful to have an option to document them as well when necessary.

When we create shards/libs with modules/abstract classes, we want to add private/protected methods and explain to user how those methods can be used.