Add missing type hinting to Simpletest module docblocks (excluding test base classes)

The blocking issue is marked closed.

I would be willing to review or work on this.

Patch in #2 wouldn't apply, but the changes are reflected here, as well.

Try the API docs, .module file, and unit tests for starters.

a reroll, mostly removing stuff from the patch cause it's been taken care of already

MOAR.

Note that I kind of generated this in the process: #2403007: Drupal\simpletest\WebTestBase::drupalHead doesn't store results despite documentation..

This patch fixes these files:

$ git diff 8.0.x --name-only
core/modules/simpletest/simpletest.api.php
core/modules/simpletest/simpletest.module
core/modules/simpletest/src/TestBase.php
core/modules/simpletest/src/Tests/SimpleTestTest.php
core/modules/simpletest/src/WebTestBase.php
core/modules/simpletest/tests/src/Unit/PhpUnitErrorTest.php
core/modules/simpletest/tests/src/Unit/TestBaseTest.php

Note that I used a phpcs/NetBeans toolchain to find the errors, and you can use it to review the patch, like this: http://youtu.be/H6pWz_6XoiQ

This might be a little out of scope but it solves the issue and improves the documentation.

OK, so it doesn't solve the problem. Reverting back to the original code, changing only the docblock.

Reroll attached. There were conflicts in WebTestBase.php and simpletest.module, both because a few types in head were being hinted like this:

@param \Drupal\Core\Url|string $path

I switched them to just:

@param string $path

But I'm not sure that's right. I also fixed a typo from the patch in #16 that had "arrau" instead of array for a type hint for function drupalGet().

The patch looks good.

+++ b/core/modules/simpletest/src/WebTestBase.php
@@ -1481,15 +1490,15 @@ protected function isInChildSite() {
-   * @param \Drupal\Core\Url|string $path
+   * @param string $path

@@ -2449,21 +2460,21 @@ protected function drupalGetMails($filter = array()) {
-   * @param \Drupal\Core\Url|string $path
+   * @param string $path

In both instances the parameter can be an URL. So can you remove these changes.

Thanks for the feedback @daffie, I've added those back in and rerolled.

There was one conflict in /core/modules/simpletest/tests/src/Unit/TestBaseTest.php, looks like a method had been renamed while making our fix obsolete.

Looks good to me. Thanks mrjmd for the reroll.
This issue is all about documention of tests. So allowed as a beta change.

Still applies.

+++ b/core/modules/simpletest/simpletest.api.php
@@ -13,7 +13,7 @@
+ * @param array $groups

+++ b/core/modules/simpletest/simpletest.module
@@ -115,7 +114,7 @@ function _simpletest_format_summary_line($summary) {
+ * @param array $test_list

@@ -166,9 +165,9 @@ function simpletest_run_tests($test_list) {
+ * @param array $unescaped_test_classnames

@@ -363,6 +362,21 @@ function _simpletest_batch_operation($test_list_init, $test_id, &$context) {
+ * @param array $results
...
+ * @param array $operations

+++ b/core/modules/simpletest/src/TestBase.php
@@ -363,20 +363,20 @@ protected function storeAssertion(array $assertion) {
+   * @param array $caller

@@ -830,21 +830,21 @@ protected function fail($message = NULL, $group = 'Other') {
+   * @param array $caller

@@ -1551,11 +1551,11 @@ protected function getRandomGenerator() {
+   * @param array $parameters

+++ b/core/modules/simpletest/src/WebTestBase.php
@@ -1280,15 +1282,15 @@ protected function curlInitialize() {
+   * @param array $curl_options

@@ -1483,13 +1492,13 @@ protected function isInChildSite() {
+   * @param array $options
...
+   * @param array $headers

@@ -1562,7 +1570,7 @@ protected function drupalGetAJAX($path, array $options = array(), array $headers
+   * @param array $edit

@@ -1589,7 +1597,7 @@ protected function drupalGetAJAX($path, array $options = array(), array $headers
+   * @param array $submit

@@ -1614,19 +1622,19 @@ protected function drupalGetAJAX($path, array $options = array(), array $headers
+   * @param array $options
...
+   * @param array $headers

@@ -1726,36 +1734,36 @@ protected function drupalPostForm($path, $edit, $submit, array $options = array(
+   * @param array $edit

@@ -2082,16 +2090,18 @@ protected function checkForMetaRefresh() {
+   * @param array $options
...
+   * @param array $headers

@@ -2115,17 +2125,18 @@ protected function drupalHead($path, array $options = array(), array $headers =
+   * @param array $post
...
+   * @param array $edit
...
+   * @param array $form

@@ -2423,11 +2434,11 @@ protected function drupalGetHeader($name, $all_requests = FALSE) {
+   * @param array $filter

@@ -2451,19 +2462,19 @@ protected function drupalGetMails($filter = array()) {
+   * @param array $options

@@ -2662,11 +2677,12 @@ protected function verboseEmail($count = 1) {
+   * @param array $override_server_vars

Shouldn't we be type-hinting in the function too? Eg:

protected function prepareRequestForGenerator($clean_urls = TRUE, array $override_server_vars = array()) {

Also:

+++ b/core/modules/simpletest/simpletest.module
@@ -1,4 +1,8 @@
 <?php
+/**

Needs a new line before the file doc.

I've added typehints to the function names everywhere phpcs pointed them out, and added the new line before the file doc. There are still a lot of other phpcs warnings in simpletest but I didn't want to go out of scope for this issue.

Some of the issues for this meta include type hinting in the function signatures, some don't.

My understanding is that ones which do not can be documentation issues, whereas ones which do are code changes. A maintainer could please talk about such a distinction on the meta if it matters.

Re-rolling the patch#28

All docblock @param and @return filled in.

Running the phpcs gist in the issue summary only yields one false-positive CS error. (It's complaining about the lack of description in a @param, but the type hinting is there.)

Turns out we can get documentation into 8.0.x.

Needed a re-roll, but I couldn't get that to work easily, so I used git apply --reject and re-did the other work. This only affected WebTestBase.php.

phpcs reports only one false-positive of a @param without a comment.

There's another issue for base test classes, which are arguably more important: #1805346: Add missing type hinting to base test classes in Simpletest module docblocks

The simpletest module is no longer in core, moving to the SimpleTest project.

It doesn't seem to make sense to have this as a child of a core issue, so removing parent.