Add missing parameter and return documentation for Gettext

This patch still applies (but not cleanly) and everything that is being changed in it looks solid.

1. +++ b/core/modules/locale/src/LocaleDefaultConfigStorage.php
   @@ -56,6 +56,8 @@ class LocaleDefaultConfigStorage {
   +   * @param $install_profile
   

   @param string $install_profile

2. +++ b/core/modules/locale/src/LocaleDefaultConfigStorage.php
   @@ -98,7 +100,7 @@ public function read($name) {
   -   *   List of configuration in install storage and current languages.
   +   *   List of configuration names.
   
   @@ -142,7 +144,7 @@ public function getComponentNames($type, array $list) {
   -   *   The list of configuration names that match predefined languages.
   +   *   List of configuration names.
   

   I don't really understand how this improves the documentation.

3. +++ b/core/modules/locale/src/PoDatabaseReader.php
   @@ -71,6 +74,11 @@ public function getOptions() {
   +   * @see self::options
   

   Whilst there are other examples in core this doesn't work in PHPStorm or on https://api.drupal.org/api/drupal/core%21modules%21book%21src%21Form%21B...

   Should be a fully qualified class name here.

4. +++ b/core/modules/locale/src/StringDatabaseStorage.php
   @@ -314,8 +314,8 @@ protected function dbStringKeys($string) {
   -   * @return \Drupal\locale\StringInterface[]
   -   *   Array of objects of the class requested.
   +   * @return array
   +   *   Array of objects of the requested class.
   

   This looks like a regression too. In fact we probably ought to assert that class implements StringInterface since we rely on it.

The changes look solid, and I agree that the assertion is something that we should do in a followup. Can you open a followup for that @Sutharsan?

+++ b/core/modules/locale/src/StringDatabaseStorage.php
@@ -533,6 +533,13 @@ protected function dbDelete($table, $keys) {
+   * @return \Drupal\Core\Database\StatementInterface|int|null

This needs a comment, other than that, looks good to me

LGTM.

Thanks for your work on this.

Typically, we shouldn't fix "all the problems" in one module. We should fix the individual kinds of errors, across all of core, and then enable the phpcs rule for that kind of error to avoid the rule regressing. This patch includes a grab-bag of the following kinds of coding standards issues:

• Missing @param
• Missing @throws
• Missing @return
• Missing blank lines
• Incorrect data types in docblock typehints
• Adding the s to the one-summary verb
• Grammatical errors

Now, writing new @param and @return documentation requires humans reading and understanding the code, so it makes sense to add them together in one patch for a given API. However, it makes less sense to mix in rewrites of existing parameter documentation, whitespace fixes, etc. I have to open all the files in this patch to review the changes (in order to verify that the added documentation is correct), so mixing in other sorts of changes and other APIs increases the reviewer burden a bit.

For more information, see: https://www.drupal.org/core/scope

Alright, now that I got the obligatory scope discussion out of the way:

1. +++ b/core/modules/locale/src/Gettext.php
   @@ -37,6 +37,10 @@ class Gettext {
   +   *   Thrown when the header is missing, malformed or the translations can not
   +   *   be written to the database.
   

   This isn't quite a sentence. I'd suggest:

   Thrown when the header is missing or malformed, or when the translation cannot be written to the database.

   (Note that "cannot" is one word in our content standards.)

2. +++ b/core/modules/locale/src/PoDatabaseReader.php
   @@ -64,6 +64,9 @@ public function setLangcode($langcode) {
   +   * @return array
   +   *   The read options.
   
   @@ -71,6 +74,9 @@ public function getOptions() {
   +   * @param array $options
   +   *   The options to set.
   

   Things are rarely just array; we can usually be more specific. They are usually string[], bool[], array[], etc. Even mixed[] gives more info. What kind of array is it?

   Also "the read options" and "The options to set" don't really explain what "options" are. It needs a reference to wherever the structure of these options is defined.

3. +++ b/core/modules/locale/src/PoDatabaseReader.php
   @@ -100,6 +103,9 @@ public function setHeader(PoHeader $header) {
   +   * @return \Drupal\locale\StringStorageInterface[]
   +   *   String storage objects.
   

   "String storage objects" isn't adding information beyond the typehint. What string storage objects?

4. +++ b/core/modules/locale/src/PoDatabaseReader.php
   @@ -148,6 +154,9 @@ private function loadStrings() {
   +   *   String storage object.
   

   I'm guessing this means the string storage object for the given language and options? Let's say that. Otherwise it's confusing .

5. +++ b/core/modules/locale/src/PoDatabaseWriter.php
   @@ -78,6 +78,9 @@ public function setLangcode($langcode) {
   +   * @return array
   
   @@ -102,6 +105,9 @@ public function setReport($report = []) {
   +   * @return array
   

   What shape of array?

6. +++ b/core/modules/locale/src/PoDatabaseWriter.php
   @@ -78,6 +78,9 @@ public function setLangcode($langcode) {
   +   *   The write result report.
   

   What is a write result report? Where is its structure defined? What references should I look at to know about it?

7. +++ b/core/modules/locale/src/PoDatabaseWriter.php
   @@ -102,6 +105,9 @@ public function setReport($report = []) {
   +   *   The writer options.
   

   What are writer options?

8. +++ b/core/modules/locale/src/StringDatabaseStorage.php
   @@ -315,7 +315,7 @@ protected function dbStringKeys($string) {
   -   *   Array of objects of the class requested.
   +   *   Array of objects of the requested class.
   

   Either is correct, I think.

9. +++ b/core/modules/locale/src/StringStorageInterface.php
   @@ -24,8 +24,8 @@
   +   *   The source strings.
   

   Maybe "The matching source strings"? "The source strings" isn't very helpful.

Thanks!

Also, setting to 8.8.x, since docs fixes are eligible for backport to the production branch.