Lacking standards for documenting classes, interfaces, methods

I think I would like doxygen documentation for all of the above. :)

Should all member variables (public? private? protected?) have a doxygen header?

+1 for documenting them, but I'd like to see an example for the formatting you're proposing here.

If a particular class extends another class and/or implements one or more interfaces, do we still need to have doc headers for the methods that it is overriding/implementing? If so, can we use an abbreviated format, like what we do for hook implementations?

I think we could use an abbreviated format - for the 1st one line sentence, but I'd like to see a more detailed description following it, which we don't require for hook implementations.

Cheers,
Stella

A, B, C, D: Yes, yes, yes yes. Anything without a *complete* docblock is a bug, with a caveat for E below.

E: My concern here is that many many IDEs and documentation parsing programs (real Doxygen, PHPDoc, etc.) will, when they detect a parent class/interface, inherit its documentation. If there is an over-ride docblock, I'm not sure how they handle it.

Before making a decision on E, I'd want to see how such supporting programs handle it and what they prefer. We'd want to at least look at Eclipse and Komodo for IDEs and PHPDocumentor for doc parsers. Then follow their preference.

What about "Implements... " for methods that.. well, implement an interface? We already use that for hooks and it is imho the correct technical description.

API module doesn't do anything with classes either way, so we can define what it will do. (Or just use PHPDoc. :-)) We can probably assume interfaces will work the same as classes.

From an IDE perspective, short description and @return are the most important to have working. (@param less so because we can also type hint in the method signature itself, but it's still important.) Long description we don't need as much.

I'd rather not be always throwing "implements" on everything. For one thing, it's not an implements. :-) Implements is for hooks, and method overrides are not hooks. If we want to start using implements here, then we shouldn't use it for hooks, since they're not the same thing.

Yes, "implement" is the standard OO terminology. I don't think it's necessary here, though, as it is already implicit in the class definition itself (class Foo implements Bar). The only question is which method relates to which interface. For that, we should go with whatever format ties into IDEs and doc parsers best. For overrides, a place to describe why we are overriding a parent class's method (assuming it's not an abstract method, in which case the reason is self-evident) would be useful as well.

And no, hooks and OO interfaces are not at all the same thing. I really really wish people would stop confusing modules with classes, as it's a very misleading and harmful assumption. :-)

An Interface in the OO sense is a very specific syntactic construct. That can unfortunately be confused with an "application programming interface", which is more abstract and can be any defined set of operations to instruct code to do something. English is, sadly, quickly running out of nouns for us to use. Hooks as Drupal uses them are a specific procedural implementation of the observer pattern using tricks of PHP to be really really fast. And then there's magic callbacks, which we call hooks even though they are not. Those generally should be methods of a class instead, but that's a Drupal 8 task to fix.

What do doxygen, PHPDoc, and Eclipse do with the "Overrides", "Implements" docblock headers? Do they pass through anything, or do they override entirely? I do not like those at all, unless they are actually directly supported by the available tools.

Atleast NetBeans doesn't display the interface documentation, it's irrelevant if there is a docblock or not. Though that could probably be changed with the nbdrupal plugin, but that's a different story :)

I will check to see what Eclipse does and report back.

#710774: Ensure all class-related items are parsed is the related issue from API module.

Gah. Sorry, this slipped off my radar.

I've tested both Eclipse and NetBeans, and much to my irritation it looks like neither one does any sort of docblock inheritance for code-assistance. Eclipse's code assistance is particularly awful and half the time or more doesn't trigger at all. NetBeans is much more reliable, but doesn't scan up the inheritance tree. Bugger.

So for PHPDoc, we don't need to specify anything. For IDEs, we would want to specify at least the @return, since that's the most important for them (assuming it's typed), and potentially the short description. I really want to avoid just replicating the entire docblock, though. Aside from making the code harder to read, I'm certain that it will quickly get out of sync.

An API parser should be able to derive D from the language itself, without need of docblock assistance. I don't know if api.module does, but I believe phpDocumentor does.

It sounds like A and C are at odds, given that IDEs apparently don't parse up the tree (bastards). However, duplication in the name of B is also at odds with A.

Anyone know how Zend Framework, Symfony, etc. handle this?

@Crell in #20

As far as I can tell from my forays into Cake and Symfony, they have no ways of handling this either. It seems many common tasks in Symfony w/ Doctrine actually end up using magic methods, so there is really no chance of any IDE having any docs at all.

The only place I've seen any sort of magic method issue in Drupal is with Database query extenders, which cannot have a helpful @return value, since the returned class is set at runtime. There could certainly be more examples of this, I am just not aware of them.

I think we should OK #3 to go into the coding standards. I don't think the final version will differ much.

Speaking to Drumm, the classes are being parsed to the detail we need, and it is theoretically possible to build a complex API documentation interface without and additional doxygen style markup.

So then we just need to evaluate the need for @inherits or @overrides. I'm working on api.module over the next couple days and hope I can provide some feedback.

if I understood well:
- a proposal was sent at #3 and seems that there is some consensus to adopt it
- before solving this issue, api.module needs some work as noted in #21 (see: #757568: Add UI pages for classes)
- Doxygen presumably works "as is" for point D in #3 as noted in #23

@inherits and @overrides do not need any parser support. The parser can get that out of the code already; there's no need to introduce redundant information.

The problem with #3 is that it doesn't have any benefit for people while actually writing code. Showing that Foo::bar() overrides Baz::bar() doesn't tell me anything about what bar() actually does or how to use it.

a) There's a difference between "Implements Foo::bar()." for interface implementations and "Extends/Overrides Foo::bar()." for extended classes, no?

b) If the parser can properly figure out relationships automatically, then we shouldn't add explicit documentation. Like for hook implementations, we'd rather have to think about an ideal API UI to display derived documentation (in a separate issue).

oh, I wasn't really aware that it's debated whether to have a docblock at all, sorry.

I'd also say we want them - and more or less the same rules for hook implementations can be applied, i.e. "Implements/Overrides ..." and if the function is complex or needs documentation for implementors, then also use a function description.

Looks good to me.

What about not requiring but suggesting to document what those overriden methods do? Interface implementations usually just, well, implement what's already described in the interface but overriden methods either extend or change existing behavior so that might need additional explanation. Something like:

  /**
   * Overrides PDOStatement::fetchAssoc().
   *
   * Optional explanation comes here.
   */
  public function fetchAssoc() {
    // Call PDOStatement::fetch to fetch the row.
    return $this->fetch(PDO::FETCH_ASSOC);
  }

I thought the original at #3 was good enough to go in as a first iteration, so this is even better!

I like what Berdir suggested of course, no harm in encouraging extra documentation.

The problem with using "Overrides ParentClass::foo()" as the description is that then replaces the real, useful description from the parent class in code assistance in IDEs. So it's really just a distraction for those users.

The only thing I wanted to add is that I find the current way of defining a @throws a bit strange.

   * @throws MyFooUndefinedException if the foo is undefined.

We basically make a sentence of the whole thing, including @throws and the Exception name. This is imho a) not consistent with other things like @params and b) api.module would need to be pretty intelligent to parse that into a useful, complete description sentence.

And when thinking more about it, do we even need to description? If we use an exception in 10 different places, is there any reason to document in 10 times? The reason for throwing the exception should always be the same, if not, then we should use a different exception there IMHO. If you need to know more about the exception, then read the documentation of that exception and api.module could even integrate that into the same page if we really want that.

Just for reference, Java doesn't say anything about a description either: http://java.sun.com/j2se/javadoc/writingdoccomments/#throwstag

Not sure about the Overrides stuff. I'm pretty sure that we can make api.module intelligent enough to integrate the parent/interface documentation but we can't do much about IDE's, which I'm pretty sure will not display the parent documentation unless you work against the parent class/interface and don't care/know about the actually used implementation. This is for example the case with the database layer, you will only see the description of SelectQuery_mysql if you open the file and look at the source code. The IDE and api.module should not care much about that.

I've sent an email to the PHP-General mailing list to see if they have any useful thoughts here.

Also agreed regarding Exceptions.

This is a variant of a post in #807336: Document thrown exceptions in database layer, it was suggested I post it here instead....

You have, at the minimum, two sets of stakeholders here:

(1) Those who are working with the API code and see the documentation as augmenting what they are seeing in the IDE, and
(2) Those who want to use the API and have absolutely no interest in the code of that API.

For example, everybody doing any coding with Drupal is probably going to the PHP Manual from time to time - or in my case over and over again at present :-) PHP is just a bunch of code, an API if you like, that is used to produce Drupal. Nobody is interested in what that code looks like, they just want to use it. That's stakeholder group (2), they will be reading the API pages.

Then there are the other folk, group (1), who want to change Drupal itself, they want to look at the API in an IDE and have it work magic for them because inheritance and overloading makes life miserable.

I think this discussion is dominated by group (1) and the API pages should be targeting group (2).

I believe you need to get this fundamental issue sorted before you will make much progress here.

cheers

@lostchord, apologies if I'm underestimating you, but reading between the lines I think you may be missing the fact that the online API documentation pages (the ones on api.drupal.org, not the handbook pages) are auto-generated from the same Doxygen comments in the code that folks are discussing above. This whole discussion is about enabling the creation of better documentation.

For the automatic information extraction to work right, it's important to establish a standard way of formatting the comments, which is what this issue discussion is about. This is not about API doc *vs* IDE behavior. It's a given that the outcome will benefit those consulting the API documentation. Of course it's nice if it *also* benefits those coding in IDEs.

If I misunderstood your question, maybe you can explain more directly where you feel API doc is getting shortchanged.

@thinkling, I know exactly how the documentation is derived.

If I were going to create better documentation out of the information embedded in the code I would start with what I consider great documentation and then work out what I need to embed in the code to achieve this. What I see here doesn't seem to be really addressing this directly.

Why not generate examples of the documentation, mockups, kick that around until everybody is satisfied and then, and only then, work out how this needs to be represented in the embedded code comments and what needs to be added to the API module to support it.

That's all.

@44/46: Of course is this *discussion* dominated by group 1, members of group 2 don't look into issues like this in the first place. But I don't think that we ignore group 2, the whole point of this issue is to figure out a way that works best for all groups (there are more than just two, just read #40). Also, I disagree with having good documentation first and then standardize on a way how to document that. Because once we have the standard, we can start to improve documentation parallel and in many different issues and incrementally improve the docs. If we do it as you suggest, then all the documentation needs to be re-worked, re-written and updated every time we change the standards.

Doesn't look like Crell's thread on the php mailing list is getting anywhere, there are just lots of people claiming that he got it all wrong ;) (see http://osdir.com/ml/php-general/2010-05/msg00770.html).

To summarize my point of view:

- When implementing an interface, there is no point in repeating the information, because the interface is the same, only the implementation differs, which is not relevant when you use that interface. You only care about the actual implementation when looking at the source code, so imho, documentation about that can simply be done as inline comments. Additionaly, for most if not all cases, we usually have factory functions, so you do not care which implementation you use and work against the interface. So I'd vote for a simple "Implements ... " as we have it in the example.

- When having a child class which overrides a parent class, then there is usually a reason for doing that, so I guess we can reference the parent implementation and also document in the docblock what this implementation is doing differently but not repeat existing information. Just as I suggested in #35. And when the parent class is abstract and defines abstract methods, then we can document them there and use "Implements... " similiar to interfaces.

- Additionally, we should always use the interface/base class for factory functions, take db_select() for example: http://api.drupal.org/api/function/db_select/7. Instead of SelectQuery, we should document SelectQueryInterface as the return type. Advantage: IDE-autocomplete displays the full documentation of the interface, api.module can make it clickable and point directly to the class (instead of clicking to the class and then the interface)

This gives us the following for the different "readers" (taken from #40):
- People writing documentation: A single place to document the API and small copy&paste-able references which do not need to be updated

- Maintainers of documentation: Same as above imho

- Coders looking at class/method/property doc on api.drupal.org: Documentation of the parent/interface documentation is just a click away, implementation details are either displayed too or displayed as inline comments directly inside the code.

- Coders using a full-featured IDE: See the interface/base class documentation as long as they work against them, which they should. I think instances when you work directly with a specific implementation is rare, and for example, there is a drupal plugin for Netbeans (which is outdated and not really maintained, but someone could change that) which we could enhance to automatically pull-in documentation about the parent class/interface when it discovers a Implements/Overrides or directly through class information.

- Coders using a plain-text editor: Probably use a.d.o most of the time, where they find all the documentation at most 1-2 clicks away or when they look at the source code, get a hint at where to look.

Anyone disagrees? :)

Yes, afaik, pretty much every place in Drupal where we use classes has factory functions, which return (to a different degree configurable) implementations against an interface.

An incomplete list (many of these are often not even used directly but only internally, some are missing proper documentation):
- http://api.drupal.org/api/function/drupal_mail_system/7
- http://api.drupal.org/api/function/_cache_get_object/7
- http://api.drupal.org/api/function/file_stream_wrapper_get_instance_by_u...
- http://api.drupal.org/api/function/file_stream_wrapper_get_instance_by_s...
- http://api.drupal.org/api/function/_batch_queue/7
- http://api.drupal.org/api/function/entity_get_controller/7
- http://api.drupal.org/api/function/db_select/7 (and db_query/update/insert/delete/or/and/xor/transaction/....)
- DrupalQueue::get() not yet linkable ;)

No factory function (not counting PHP classes):
- DatabaseTasks, but these are only used in the installer
- FileTransferLocal (used for example in http://api.drupal.org/api/function/update_manager_update_ready_form_subm..., this will (hopefully) not be used by the typical coder :))
- A few more in simpletest/database internals, nothing you are going to touch on a regular basis)

@Berdir:

Of course is this *discussion* dominated by group 1, members of group 2 don't look into issues like this in the first place.

I would consider myself a member of group 2 at present and I'm very interested in this issue because I think the API documentation could do with a bit of improvement. So my interest is selfish and focused (at present) on group 2 requirements. I would suggest that this would probably be the case with most users starting using Drupal with some ideas around coding their own modules. Is this a group you want to nurture?

Apart from different types of users you also have different presentation methods. Print you ignore. The API pages don't have the "Printer friendly version' option and the "everything in a few clicks" approach isn't always appropriate in this case, you may want to consider inline content that can be expanded instead.

Is this sort of issue in scope here?

cheers

Imho it is out of scope.

This issue is not about the quality of the documentation, not even the content at all. It is about defining a common standard of *how* (and not *what*) to write documentation for classes so that all the different readers (be it humans or IDE's or parsers) are able to "understand" it and to ensure a consistency, so that you know when you for example see a "Implements SomeClass::someMethod()" where you have to look.

If you want to improve documentation then check out http://drupal.org/contribute/documentation and start writing ;)

#52

I'm not sure you can contemplate a "How" without some pretty complete concept of "What". You previously mentioned standards and what a pain it is when you change them. You bet! Real painful. Which is why you probably want to make sure you understand all the What's so that you can be sure your How's will not need to be changed the following morning.

In terms of improving documentation I think that the Documentation Team is probably a better starting place for canvasing the fundamental issues that exist with the documentation today.

So I have reached a couple of important conclusions here.

1) PHP-General is mostly for trolling.
2) IDE authors are morons.
3) PHP sucks.

These likely don't come as a surprise to anyone, but I figured I'd mention it anyway. *sigh*

The only useful suggestion to come out of that thread was this:
http://osdir.com/ml/php-general/2010-05/msg00797.html

Which points to this bit of PHPDoc:
http://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutoria...

Of course, that is for inheriting the longdesk, which is of all the parts of the docblock the one we care about the least here. Arguably we could add another tag of our own to do something else, but IDEs still wouldn't read that.

I think Berdir's suggestion to handle IDEs with "well that's just one more reason to always always always use an interface" is the best we're going to be able to do. There's simply nothing else that I have been able to find that any of the major IDEs will do.

I don't like the "Implements Foo::bar()" syntax, for a couple of reasons. One, we already use "Implements" for hooks in a few thousand places. Rightly or wrongly, that's going to be confusing since hooks are not interfaces. Two, I can see that being harder to parse.

Since we have our own parser though, and if we ever fully move to PHPDoc (as I believe we should) it is extensible, we can define our own documentation tag for that. Something like

class Foo extends Bar implements Baz {

/**
* Full docblock here.
*/
function a() { }

/**
* @implement Baz
*/
function b($x, $y) { }

/**
* @override Bar
*/
function c($x, $y) { }
}

And then api.module can just detect @implement and @override and pull in documentation as necessary. (Knowing which method to pull from is trivial at that point, since it has to be the one with the same name.)

lostchord: This sort of documentation is already well standardized. PHPDoc and Doxygen and JavaDoc are all very widely used documentation systems by thousands of projects. All we're trying to do in this thread is fill in a few gaps in the existing standards for the technology. Drupal is already a fairly well documented system, so we already know the "What". This is just edge cases for the How.

The only reason we'd have to revisit the entirety of How from the ground up would be if we wanted to throw out the entire Doxygen/PHPDoc concept and start from scratch, which is seriously not in scope. :-) This is a well-understood problem space already.

@Crell

This sort of documentation is already well standardized.

But you have chosen to implement your own subset of Doxygen.

Drupal is already a fairly well documented system

The documentation is improving, well documented is drawing a long bow.

Documentation that relies on developers is, has always been, may well continue to be, an issue. There is a lack of focus there because, particularly in the OSS environment, there is limited incentive to get it right. If I don't do the documentation right I can write a book, sell support,...whatever.

Embedding documentation in the source code is a two edged sword. Who is responsible for it? What goes in the source code and what goes in the Handbooks, is the same process used to control content on each and ensure synchronisation... are these broader issues being considered? Are the necessary controls placed around source code completely appropriate where documentation is concerned particularly if you want to encourage community participation in documentation (which you have to do if you want to improve).

Documentation tends to be the poor citizen in this environment because everybody involved (well nearly everybody) is doing it as a hobby. They have 'no skin in the game'.

YMMV :-)

cheers

lostchord: What you're debating is extremely off topic for this thread. You're still talking grand plans and content changes to documentation. The *only* thing relevant in this thread is how we want to document inheritance and interface usage within the currently existing inline documentation framework. Anything else is off topic and should be moved elsewhere. Please do not derail this issue.

+1 - and then set aspilicious on the prowl to enforce it all.

I agree with the proposed standards :D...

Great job jhodgdon!

Looks good to me too!

I don't really get Crell's reason in #54 regarding not using Implements because we already use it for hooks. Using a @tag in the first line looks strange to me and is a) totally against all other coding standards we have (first sentence stuff) and b) we do not need a tag because api.module can detect implemented/overriden methods simply by parsing the class definition (class ChildClass extends ParentClass implements AnInterface). The "Implements/Overrides .. " text is imho just for a human reader.

#54 on @implement and @override: this is just repeating what we know from code. The dev version of API module actually already shows if class members override or inherit from their parent classes, on class pages. I think I broke it recently, but http://api.scratch.drupal.org/api/Drupal/includes--database--sqlite--dat... shows the general idea. Documentation should not repeat what we already know from the structure of the code.

http://api.scratch.drupal.org/ is the staging site for the new stuff. Things from #57 should already be fully implemented. Please file issues for what is wrong.

I checked the http://api.scratch.drupal.org/ site and while awesome, can't we will in the missing class documentation from the interface?

jhogdon, I think what chx is referring to is what I outlined in #710774-3: Ensure all class-related items are parsed. I'm pretty sure that this should be filed against API module.

For instance the documentation from http://api.scratch.drupal.org/api/Drupal/includes--database--query.inc/f... should be shown on http://api.scratch.drupal.org/api/Drupal/includes--database--query.inc/f....

#65 is done. http://drupal.org/cvs?commit=382648

Nitpick on #57, since you asked:

- [EDIT: ADDED] Make sure when documenting function return values to use the most general class/interface possible. E.g., document the return value of db_select() to be a SelectQuery, not a particular class that implements SelectQuery.

s/SelectQuery/SelectQueryInterface/ -- if I remember latest discussions elsewhere correctly.

I still think the double-use of "Implements" will be confusing (we switched to that for hooks, too, a while back), but not so much that I am going to block it.

PHP sucks, this is the best we can do for now, I guess. The rest is issues for api.module until we can switch to phpDocumentor.