Cleanup AssertContentTrait::xpath()

+++ b/core/modules/simpletest/src/AssertContentTrait.php
@@ -220,7 +220,7 @@ protected function buildXPathQuery($xpath, array $args = array()) {
+   * @return \SimpleXMLElement[]|bool
    *   The return value of the xpath search. For details on the xpath string
    *   format and return values see the SimpleXML documentation,
    *   http://php.net/manual/function.simplexml-element-xpath.php.

The let's adjust also the description of the return value, telling that we might return also FALSE.

If we already touch the function, we might want to cleanup also the function?

@@ -234,9 +234,7 @@
       // to an empty array to allow foreach(...) constructions.
       return $result ? $result : array();
     }
-    else {
-      return FALSE;
+    return FALSE;
-    }
   }

1. +++ b/core/modules/simpletest/src/AssertContentTrait.php
   @@ -220,23 +220,21 @@ protected function buildXPathQuery($xpath, array $args = array()) {
   +   * @return \SimpleXMLElement[]
       *   The return value of the xpath search. For details on the xpath string
       *   format and return values see the SimpleXML documentation,
       *   http://php.net/manual/function.simplexml-element-xpath.php.
   

   The first concern was not addressed. The help txt needs to tell also about the FALSE return.

2. +++ b/core/modules/simpletest/src/AssertContentTrait.php
   @@ -220,23 +220,21 @@ protected function buildXPathQuery($xpath, array $args = array()) {
   +      return $result ? : [];
   

   "? :" should be "?:" (no space between them).

+++ b/core/modules/simpletest/src/AssertContentTrait.php
@@ -220,23 +220,21 @@ protected function buildXPathQuery($xpath, array $args = array()) {
+    return [];

Oh, no. We cannot change this from FALSE to empty array. This may break a lot of tests. We still need FALSE there. We are not changing the interface here, we just cleanup docs, and additionally the code.

Despite the comment the function still returns FALSE in some cases. That makes it inconsistent. Why not fix it at once?

That comment is inside if(), so it doesn't cover the case of FALSE.

Sorry, FALSE is different than []. First is telling that a we have a failure (aka "we weren't able to parse"), the 2nd tells that a successful search occurred but there are no results. Totally different cases.

The review is against 2587229-boolean.patch.

1. +++ b/core/modules/simpletest/src/AssertContentTrait.php
   @@ -220,23 +220,23 @@ protected function buildXPathQuery($xpath, array $args = array()) {
   +   * @return \SimpleXMLElement[]|boolean
   

   The data type should be declared as bool (not boolean). See https://www.drupal.org/coding-standards/docs#types.

2. +++ b/core/modules/simpletest/src/AssertContentTrait.php
   @@ -220,23 +220,23 @@ protected function buildXPathQuery($xpath, array $args = array()) {
   +   *   The return value of the xpath search or FALSE on failure.
   +   *   For details on the xpath string format and return values see
   +   *   the SimpleXML documentation.
   

   The wrapping is incorrect. The second phrase should continue right when the first ends. The existing wrapping is correct. Don't break lines after phrase ends, but only on 80 chars.

Looks good. Thank you!

nice small cleanup

This is rc eligible as it just changes tests.

Based on @xjm's post release triage document, committed 01387f8 and pushed to 8.0.x and 8.1.x. Thanks!