Draft: Deprecate ToStringTrait #3465827
Closes #3465827
Merge request reports
Activity
2 2 3 3 namespace Drupal\Component\Utility; 4 4 5 @trigger_error('The ' . __NAMESPACE__ . '\ToStringTrait is deprecated in drupal:11.1.0 and is removed from drupal:12.0.0. Use to implement the __toString() method instead. See https://www.drupal.org/node/3466031', E_USER_DEPRECATED); 6 5 7 /** 6 8 * Wraps __toString in a trait to avoid some fatal errors. 9 * 10 * @deprecated in drupal:11.1.0 and is removed from drupal:12.0.0. Use to 11 * implement the __toString() method instead. wording of the second sentence can be improved a bit, not sure if the coding standard mandates "Use", but just "Implement the __toString() method directly, exception handling is no longer required" should be sufficient IMHO. Possibly with more details in the change record that explains that this is no longer needed since PHP 7.4
changed this line in version 3 of the diff
59 59 $string = 'May I have an exception?'; 60 60 $text = $this->getMockBuilder(TranslatableMarkup::class) 61 61 ->setConstructorArgs([$string, [], [], $translation]) 62 ->onlyMethods(['_die']) 63 62 ->getMock(); 64 $text 65 ->expects($this->once()) 66 ->method('_die') 67 ->willReturn(''); strange that this fails on the return type assertion, I would expect it throws an exception?
We could drop the error handler and just expect an exception, but I'm wondering if we shouldn't just drop the whole test method. Because there is no code left anymore that we are testing with it, we'd just assert that our mock callback does indeed throw an exception. So I'd just remove the whole method and the errorHandler() method too.
added 16 commits
-
51d64d1c...5ac4f96a - 15 commits from branch
project:11.x
- 5cf7cc26 - Deprecate ToStringTrait #3465827
-
51d64d1c...5ac4f96a - 15 commits from branch
added 136 commits
-
5cf7cc26...68589170 - 135 commits from branch
project:11.x
- 2e9b5e93 - Deprecate ToStringTrait #3465827
-
5cf7cc26...68589170 - 135 commits from branch
73 throw new \Exception('Yes you may.'); 74 }); 75 76 // We set a custom error handler because of https://github.com/sebastianbergmann/phpunit/issues/487 77 set_error_handler([$this, 'errorHandler']); 78 // We want this to trigger an error. 79 (string) $text; 80 restore_error_handler(); 33 ->method('render') 34 ->willThrowException($exception); 81 35 82 $this->assertEquals(E_USER_ERROR, $this->lastErrorNumber); 83 $this->assertMatchesRegularExpression('/Exception thrown while calling __toString on a .*MockObject_TranslatableMarkup_.* object in .*TranslatableMarkupTest.php on line [0-9]+: Yes you may./', $this->lastErrorMessage); 36 $this->expectException(\Exception::class); 37 $this->expectExceptionMessage($exception->getMessage()); 38 $this->assertTrue('' === (string) $text); added 9 commits
-
2e9b5e93...a9b0fa6d - 8 commits from branch
project:11.x
- b5001318 - Deprecate ToStringTrait #3465827
-
2e9b5e93...a9b0fa6d - 8 commits from branch
added 1 commit
381 381 throw $e; 382 382 } 383 383 else { 384 trigger_error($e, E_USER_ERROR); 384 $this->logger->error($e->getMessage(), [ 385 'exception' => $e, 386 'backtrace' => $e->getTraceAsString(), 387 ]); - Comment on lines +384 to +387
Yes, it needs explanation why core allows to recover from this kind of error, the
continue;
after logger points that this error is supposed to return execution back!But I lack of idea why and this kind of logging has test coverage that watchdog entry has
@message
for some reason, asked Wim in Slack
615 621 throw $e; 616 622 } 617 623 else { 618 trigger_error($e, E_USER_ERROR); 624 $this->logger->error($e->getMessage(), [ 625 '@message' => $e->getMessage(),