DateTimePlus doesn't track errors properly

+++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
@@ -268,10 +268,11 @@ public function __construct($time = 'now', $timezone = NULL, $settings = []) {
@@ -442,7 +443,7 @@ protected function prepareFormat($format) {

@@ -442,7 +443,7 @@ protected function prepareFormat($format) {
   public function checkErrors() {
     $errors = \DateTime::getLastErrors();
     if (!empty($errors['errors'])) {
-      $this->errors += $errors['errors'];
...
     }

We should not just use array_values() here. += merge arrays depending of keys. In case $errors = ['error 1', 'error 2']; and $errors['errors'] = ['error 2', 'error 3'];, $this->errors += array_values($errors['errors']); will still result to ['error 1', 'error 2']

Should use something like.

$this->errors = array_merge($this->errors, array_values($errors['errors']));

Tests should reproduce this case (+ 1 patch with only test which should fail)

1. +++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
   @@ -268,10 +268,11 @@ public function __construct($time = 'now', $timezone = NULL, $settings = []) {
   -          $this->errors[] = $test['errors'];
   +          $this->errors = array_merge($this->errors, array_values($test['errors']));
   
   @@ -442,7 +443,7 @@ protected function prepareFormat($format) {
   -      $this->errors += $errors['errors'];
   +      $this->errors = array_merge($this->errors, array_values($errors['errors']));
   

   This is the bugfix. Makes sense.

2. +++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
   @@ -670,4 +670,45 @@ public function providerTestInvalidDateDiff() {
   +   * @return array
   

   Nit: string[]

Looks good. Nice catch, thanks mpdonadio

Miss to check one step in summary

+++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
@@ -670,4 +670,45 @@ public function providerTestInvalidDateDiff() {
+    // Make sure all of the errors are unique strings.
+    $errors = $date->getErrors();
+    $this->assertEquals(FALSE, in_array(FALSE, array_map('is_string', $errors)));
+    $this->assertEquals(count($errors), count(array_unique($errors)));

Why are we not just asserting what $date->getErrors() returns? This seems a bit fragile.

Also the return of $date->getErrors() for ['YYYY-MM-DD'] is:

Array
(
    [0] => The timezone could not be found in the database
    [1] => Unexpected character
    [3] => Double timezone specification
)

The array indexing looks weird.

Thanks!

1. +++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
   @@ -670,4 +670,96 @@ public function providerTestInvalidDateDiff() {
   +   *   A array of errors messages.
   

   Nit: s/errors messages/error messages/

2. +++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
   @@ -670,4 +670,96 @@ public function providerTestInvalidDateDiff() {
   +    // Make sure the parsed date/time has errors.
   +    $this->assertEquals(TRUE, $date->hasErrors());
   

   Nit: this comment seems pointless.

3. +++ b/core/tests/Drupal/Tests/Component/Datetime/DateTimePlusTest.php
   @@ -670,4 +670,96 @@ public function providerTestInvalidDateDiff() {
   +    // Make sure all of the errors are unique strings.
   +    $this->assertEquals($errors, $date->getErrors());
   

   This comment seems wrong.

Exactly :)

Back to RTBC!

+++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
@@ -268,10 +268,11 @@ public function __construct($time = 'now', $timezone = NULL, $settings = []) {
-          $this->errors[] = $test['errors'];
+          $this->errors = array_merge($this->errors, array_values($test['errors']));

This looks weird. Why do we need to array_merge() after $this->errors = [];?

Can't we just do $this->errors = $test['errors'];?

@mpdonadio,

Changes look good and patch applies cleanly. Changing to RTBC.

Committed ade0589 and pushed to 8.4.x. Thanks!

I'm pretty certain we should put this in 8.3.x - as we're in RC going to get a +1 from another committer before cherry-picking.

+1 on cherry-picking back to 8.3.x.

Committed 52e84ae and pushed to 8.3.x. Thanks!