Add missing data type indicators to @return tags throughout core

[ghost]

According to the documentation standards:

The @return tag is followed by a data type indicator, and then a newline. The following paragraph is considered by the API module to be documentation.

and:

Data types are required to be included as of Backdrop 1.x.

Yet of the 2033 @return tags I found in core, 1123 don't have a data type indicator (over 50%).

Don't worry, I fix!

BTW, this is related to backdrop-ops/docs.backdropcms.org#64 because the fix I made there doesn't take into account @return tags without anything after them. And rather than try to account for those situations, why not just fix those situations?

[ghost]

Have started on this, might take a while though...

Also, some functions return values that could be different data types depending on what the parameters are, and there's no way of knowing ahead of time. For example, variable_get() - we don't know whether it'll return a string, int, bool, etc. because it can vary. I've therefore used the (made-up) convention of putting [varies] as the return data type. Please let me know if there's a better solution for this.

[serundeputy]

@BWPanda

@return array|false
  array of beautiful flower names if blah; otherwise false

is a thing that happens.

also see this initiative: #3213

[oadaeh]

Drupal used to use mixed for those situations, but that has been changed to what Geoff said.
https://www.drupal.org/docs/develop/standards/api-documentation-and-comment-standards#types

[ghost]

Yes, our documentation also says to separate multiple possible return data types with a vertical line, however I'm referring to situations where any data type could be returned. Do I then list all of them? I don't think that's ideal... Hence the [varies].

[klonos]

Don't worry, I fix!

😆

Have started on this, might take a while though...

Thanks in advance for your time/effort @BWPanda 👍

[ghost]

It seems I have bitten off more than I can chew with this issue... So soon I'll post a PR of my work so far and then let others have a crack at the remainder.

For anyone who wants to helps with this, I'll also post a list of the files where there are @return tags without data type indicators. I've been working my way through this list, deleting files from it once I fixed them.

Here's how I've been doing it (but maybe there's a better way):

1. Checkout a copy of Backdrop core
2. Create a new branch for this issue
3. Open the first file listed (see list below)
4. Search for all @return tags without a date type indicator

• My text editor has the ability to search via regex, so I've been using @return\s*\n which searches for all instances of @return followed by 0 or more whitespace characters, followed by a newline

5. Read the description of the @return tag to see what it should be returning (e.g. array, bool, etc.)

• Failing this, look at the code to see what's returned
• Try to notice multiple returns (e.g. array|bool)
• Use [varies] when the data type(s) cannot be determined (see comments above)

6. Fix any other obvious issues (e.g. no blank line above @return tag)
7. Once all @return tags in the file have been fixed, save it and delete it from the list
8. Repeat steps 3-7 ad nauseum
9. When all files in a module have been fixed, make a commit of those files

[ghost]

PR here. I made separate commits for folders/modules to make it easier to review than one huge commit.

And here's the list of remaining files to fix:

• modules/file/file.api.php
• modules/file/file.field.inc
• modules/file/file.module
• modules/filter/filter.admin.inc
• modules/filter/filter.api.php
• modules/filter/filter.module
• modules/filter/js/filter.filtered_html.admin.js
• modules/filter/tests/filter.test
• modules/image/image.api.php
• modules/image/image.effects.inc
• modules/image/image.module
• modules/installer/installer.browser.inc
• modules/installer/installer.manager.inc
• modules/installer/installer.module
• modules/installer/installer.pages.inc
• modules/installer/tests/installer.test
• modules/language/language.module
• modules/layout/layout.api.php
• modules/layout/layout.module
• modules/locale/locale.bulk.inc
• modules/locale/locale.module
• modules/menu/menu.module
• modules/node/node.api.php
• modules/node/node.module
• modules/node/node.pages.inc
• modules/node/node.types.inc
• modules/path/path.admin.inc
• modules/path/path.inc
• modules/path/path.module
• modules/redirect/redirect.module
• modules/search/search.api.php
• modules/search/search.extender.inc
• modules/search/search.module
• modules/simpletest/backdrop_web_test_case_cache.php
• modules/simpletest/backdrop_web_test_case.php
• modules/simpletest/simpletest.module
• modules/simpletest/tests/batch.test
• modules/simpletest/tests/bootstrap.test
• modules/simpletest/tests/cache.test
• modules/simpletest/tests/file_test.module
• modules/simpletest/tests/file.test
• modules/simpletest/tests/form_test.module
• modules/simpletest/tests/form.test
• modules/simpletest/tests/image_test.module
• modules/simpletest/tests/mail.test
• modules/simpletest/tests/menu_test.module
• modules/simpletest/tests/menu.test
• modules/simpletest/tests/path.test
• modules/simpletest/tests/schema.test
• modules/simpletest/tests/session.test
• modules/simpletest/tests/simpletest.test
• modules/simpletest/tests/upgrade/upgrade.test
• modules/system/image.gd.inc
• modules/system/language.api.php
• modules/system/system.admin.inc
• modules/system/system.api.php
• modules/system/system.archiver.inc
• modules/system/system.mail.inc
• modules/system/system.menu.inc
• modules/system/system.module
• modules/system/system.queue.inc
• modules/system/system.theme.inc
• modules/system/tests/system.test
• modules/taxonomy/taxonomy_vocabulary.class.inc
• modules/taxonomy/taxonomy.module
• modules/taxonomy/taxonomy.tokens.inc
• modules/translation/tests/translation.test
• modules/translation/translation.module
• modules/translation/translation.pages.inc
• modules/update/tests/update_test/update_test.module
• modules/update/update.api.php
• modules/update/update.compare.inc
• modules/update/update.fetch.inc
• modules/update/update.install
• modules/update/update.module
• modules/user/user.module
• modules/user/user.pages.inc
• modules/views_ui/views_ui.admin.inc
• modules/views_ui/views_ui.analyze.inc
• modules/views_ui/views_ui.module
• modules/views_ui/wizards/views_ui_base_views_wizard.php
• modules/views/handlers/views_handler_argument.inc
• modules/views/handlers/views_handler_field.inc
• modules/views/handlers/views_handler_filter_in_operator.inc
• modules/views/handlers/views_handler_relationship_groupwise_max.inc
• modules/views/includes/ajax.inc
• modules/views/includes/handlers.inc
• modules/views/includes/plugin.inc
• modules/views/includes/utility.inc
• modules/views/includes/view.inc
• modules/views/plugins/views_plugin_display.inc
• modules/views/plugins/views_plugin_query_default.inc
• modules/views/plugins/views_plugin_style_rss.inc
• modules/views/plugins/views_plugin_style.inc
• modules/views/tests/views_ajax.test
• modules/views/tests/views_ui.test
• modules/views/views.api.php
• modules/views/views.module

[herbdool]

Maybe we can commit multiple PRs for this so we at least make some progress.

Most of it looks good @BWPanda though you'll need to replace "varied" with the actual types.

[ghost]

@herbdool Multiple PRs are fine.

Re 'varies', are you suggesting using array|bool|float|int|null|object|string instead?

[herbdool]

Correct. Though in some cases it might not include object. Such as variable_get I think.

[herbdool]

Though just looked at config_get and it uses "mixed". Does it make it okay or should we correct those as well? https://github.com/backdrop/backdrop/blob/1.x/core/includes/config.inc

[herbdool]

Does someone have a link to the backdrop standard? I can't find it

[ghost]

https://docs.backdropcms.org/doc-standards