Correct the @param and @return statements on dbtng

I started to work on it, I did the files in /core/lib/Drupal/Core/Database
Can you give me a feedback please ? If it's good, I'll continue with the sub-folders.

Here is the previous patch, splitted into one-per-file patches. Hope I didn't miss one. I fixed the Contains word.
@TODO is now /core/lib/Drupal/Core/Database subfolders and database.inc

We can't really have one patch per file. How about splitting into one patch per "thing fixed"? Like, one patch that fixes @param, one that fixes @return (or these two could be combined), and one that adds \ before namespaced class names?

Ok, I continued to work on it, I'll try to finish and post proper patches tomorrow.

As promised, here is my work

Note that each uploaded patch blocks a test bot for 30m-1h30, depending on the bot. This issue blocked (and still is) the whole test system for hours.

Either limit the number of patches you upload to (very) few or use the -do-not-test.patch suffix, as you're only making documentation changes that can't break tests.

Berdir: I will not commit docs-only patches if they haven't been run through the test bot, just in case there is a stray character that wasn't noticed that then breaks HEAD.

That said, we generally only want one patch per issue. It's just very difficult to review #7 and keep track of "I'm reviewing this patch and it needs this work, while this other patch is ready" -- how do you set the issue status in that case?

So. We either need it all combined into one patch, or this issue needs to be broken up. And hopefully somone will then review the patch(es).

StephaneQ, the lack of IDE integration for the database layer (due to broken @return doxygen) is a nightmare for me and I am totally shocked someone got this far so very fast. Thanks much!

I think this could be rolled into one patch. The one failed patch doesn't matter much, it's just a bot fluke, I checked it.

If you roll this together, I will RTBC it, if there's anything left out, we can pick up the slack later but this work really badly needs to find its way in.

One more thing: all the Implements whateverinterface::method is just {@inheritdoc} now.

(I want to yell from the rooftops: yay, finally we are fixing the DBTNG documentation!)

According to IRC discussion, we limit this issue to @return and @param. Here's an updated patch.

Oops, forget the previous one, I did a mistake and @param fixes are missing

Yay!

Added fixes for database.inc

*even better*

Thanks!

I'm not sure what you decided the scope of this issue is, but there are some additional @param/@return problems that are not fixed in this patch, and that affect lines in this patch:

a) @return statements need descriptions:

+ * @return \Drupal\Core\Database\Query\Condition
  */
 function db_or() {

This needs an additional line describing what is returned, which can be similar to the return statements for db_select() and the others. Same for db_and() and a few others.

b) There should be a blank line between @param and @return

  * @param $conjunction
  *   The conjunction to use for query conditions (AND, OR or XOR).
- * @return Condition
+ * @return \Drupal\Core\Database\Query\Condition
  */
 function db_condition($conjunction) {

This is I think the only place in the patch with this problem.

Feel free to tell me "out of scope", but I think it would be nice if an issue saying "correct the @param and @return statements" as its title made them completely correct. ;)

I think it's in the scope jhodgdon, here's an updated patch. Thanks

Looks fine to me, back to RTBC, don't think the bot is going to disagree :)

Thanks! Great work! Go StephaneQ!!!!

Committed to 8.x.

I made another issue for @see, var and throws statements, as I did them here
#2064261: Correct the @var, @throws and @see statements on dbtng

This patch collided a bit with #1533096: Make includes directory, files starting with D-G pass Coder Review

But I've rerolled that one so it works again.

One thing I noticed while working on the patch above was that there's no type declarations for @param or @return

Would it be of any use if I made a patch to fix that? It's out of scope for the patch above, but i'm happy to do it if it's of any use

RE #23... What are you talking about? I believe all the @param/@return statements do have types on them. That was the whole point of the patch above, to make the types actually be correct and with the correct namespaces. ??

@jhodgdon, as far as i know - the @return's now have the correct types, but the @param's only have their name, not their type

eg

* @param $query
* The prepared statement query to run..
* @param $args
* An array of values t...
* @param $options
* An array of options....

should be:
* @param string $query
* The prepared statement query to run..
* @param array $args
* An array of values t...
* @param array $options
* An array of options....

I think there's some debate about whether to add them, but certainly it's helpful if you use an ide

Ah. The patch cleaned up a bunch of *incorrect* params that were using the wrong class names or lacked namespaces. But I guess it didn't go through and add missing types.

That would be a separate issue. Much of core suffers from the same problem and there was at one time an initiative to add them to all of core, but it's a HUGE job to write the patches (requires some care!), review the patches (more care!) and commit the patches (I personally never commit patches that I haven't also given a final correctness review to, so more care!). The effort required is therefore large, and the benefits... well, probably not as large? Anyway, search the issue queue and you can probably find the meta-issue and get it restarted if that is what you want to do. I personally think that fixing incorrect docs is a higher priority.