Skip to content

Commit 2cb419e

Browse files
nicolas-grekasnicolas-grekas
committed
[ErrorHandler] don't throw deprecations for return-types by default
1 parent 373469b commit 2cb419e

File tree

4 files changed

+39
-19
lines changed

4 files changed

+39
-19
lines changed

‎.github/patch-types.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
<?php
22

33
if (false === getenv('SYMFONY_PATCH_TYPE_DECLARATIONS')) {
4-
putenv('SYMFONY_PATCH_TYPE_DECLARATIONS=force=1&php71-compat=0');
4+
echo "Please define the SYMFONY_PATCH_TYPE_DECLARATIONS env var when running this script.\n";
5+
exit(1);
56
}
67

78
require __DIR__.'/../.phpunit/phpunit-8.3-0/vendor/autoload.php';

‎.travis.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ env:
2222
- MESSENGER_AMQP_DSN=amqp://localhost/%2f/messages
2323
- MESSENGER_REDIS_DSN=redis://127.0.0.1:7001/messages
2424
- SYMFONY_PHPUNIT_DISABLE_RESULT_CACHE=1
25+
- SYMFONY_PATCH_TYPE_DECLARATIONS=force=0
2526

2627
matrix:
2728
include:
@@ -298,6 +299,7 @@ install:
298299
ln -sd $(realpath src/Symfony/Contracts) vendor/symfony/contracts
299300
sed -i 's/"\*\*\/Tests\/"//' composer.json
300301
composer install --optimize-autoloader
302+
export SYMFONY_PATCH_TYPE_DECLARATIONS=force=object
301303
php .github/patch-types.php
302304
php .github/patch-types.php # ensure the script is idempotent
303305
PHPUNIT_X="$PHPUNIT_X,legacy"

‎src/Symfony/Component/ErrorHandler/DebugClassLoader.php

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,19 @@
2424
* and will throw an exception if a file is found but does
2525
* not declare the class.
2626
*
27+
* It can also patch classes to turn docblocks into actual return types.
28+
* This behavior is controlled by the SYMFONY_PATCH_TYPE_DECLARATIONS env var,
29+
* which is a url-encoded array with the follow parameters:
30+
* - force: any value enables deprecation notices - can be any of:
31+
* - "docblock" to patch only docblock annotations
32+
* - "object" to turn union types to the "object" type when possible (not recommended)
33+
* - "1"/"0" to enable/disable patching of return types
34+
* - php: the target version of PHP - e.g. "7.1" doesn't generate "object" types
35+
*
36+
* Note that patching doesn't care about any coding style so you'd better to run
37+
* php-cs-fixer after, with rules "phpdoc_trim_consecutive_blank_line_separation"
38+
* and "no_superfluous_phpdoc_tags" enabled typically.
39+
*
2740
* @author Fabien Potencier <fabien@symfony.com>
2841
* @author Christophe Coevoet <stof@notk.org>
2942
* @author Nicolas Grekas <p@tchwork.com>
@@ -141,6 +154,7 @@ class DebugClassLoader
141154
private $isFinder;
142155
private $loaded = [];
143156
private $patchTypes;
157+
144158
private static $caseCheck;
145159
private static $checkedClasses = [];
146160
private static $final = [];
@@ -160,6 +174,10 @@ public function __construct(callable $classLoader)
160174
$this->classLoader = $classLoader;
161175
$this->isFinder = \is_array($classLoader) && method_exists($classLoader[0], 'findFile');
162176
parse_str(getenv('SYMFONY_PATCH_TYPE_DECLARATIONS') ?: '', $this->patchTypes);
177+
$this->patchTypes += [
178+
'force' => null,
179+
'php' => null,
180+
];
163181

164182
if (!isset(self::$caseCheck)) {
165183
$file = file_exists(__FILE__) ? __FILE__ : rtrim(realpath('.'), \DIRECTORY_SEPARATOR);
@@ -551,15 +569,14 @@ public function checkAnnotations(\ReflectionClass $refl, string $class): array
551569
}
552570
}
553571

554-
$forcePatchTypes = $this->patchTypes['force'] ?? null;
572+
$forcePatchTypes = $this->patchTypes['force'];
555573

556-
if ($canAddReturnType = null !== $forcePatchTypes && false === strpos($method->getFileName(), \DIRECTORY_SEPARATOR.'vendor'.\DIRECTORY_SEPARATOR)) {
574+
if ($canAddReturnType = $forcePatchTypes && false === strpos($method->getFileName(), \DIRECTORY_SEPARATOR.'vendor'.\DIRECTORY_SEPARATOR)) {
557575
if ('void' !== (self::MAGIC_METHODS[$method->name] ?? 'void')) {
558576
$this->patchTypes['force'] = $forcePatchTypes ?: 'docblock';
559577
}
560578

561-
$canAddReturnType = ($this->patchTypes['force'] ?? false)
562-
|| false !== strpos($refl->getFileName(), \DIRECTORY_SEPARATOR.'Tests'.\DIRECTORY_SEPARATOR)
579+
$canAddReturnType = false !== strpos($refl->getFileName(), \DIRECTORY_SEPARATOR.'Tests'.\DIRECTORY_SEPARATOR)
563580
|| $refl->isFinal()
564581
|| $method->isFinal()
565582
|| $method->isPrivate()
@@ -570,20 +587,20 @@ public function checkAnnotations(\ReflectionClass $refl, string $class): array
570587
}
571588

572589
if (null !== ($returnType = self::$returnTypes[$class][$method->name] ?? self::MAGIC_METHODS[$method->name] ?? null) && !$method->hasReturnType() && !($doc && preg_match('/\n\s+\* @return +(\S+)/', $doc))) {
573-
if ('void' === $returnType) {
590+
list($normalizedType, $returnType, $declaringClass, $declaringFile) = \is_string($returnType) ? [$returnType, $returnType, '', ''] : $returnType;
591+
592+
if ('void' === $normalizedType) {
574593
$canAddReturnType = false;
575594
}
576595

577-
list($normalizedType, $returnType, $declaringClass, $declaringFile) = \is_string($returnType) ? [$returnType, $returnType, '', ''] : $returnType;
578-
579-
if ($canAddReturnType && 'docblock' !== ($this->patchTypes['force'] ?? false)) {
596+
if ($canAddReturnType && 'docblock' !== $this->patchTypes['force']) {
580597
$this->patchMethod($method, $returnType, $declaringFile, $normalizedType);
581598
}
582599

583600
if (strncmp($ns, $declaringClass, $len)) {
584-
if ($canAddReturnType && 'docblock' === ($this->patchTypes['force'] ?? false) && false === strpos($method->getFileName(), \DIRECTORY_SEPARATOR.'vendor'.\DIRECTORY_SEPARATOR)) {
601+
if ($canAddReturnType && 'docblock' === $this->patchTypes['force'] && false === strpos($method->getFileName(), \DIRECTORY_SEPARATOR.'vendor'.\DIRECTORY_SEPARATOR)) {
585602
$this->patchMethod($method, $returnType, $declaringFile, $normalizedType);
586-
} elseif ('' !== $declaringClass) {
603+
} elseif ('' !== $declaringClass && null !== $this->patchTypes['force']) {
587604
$deprecations[] = sprintf('Method "%s::%s()" will return "%s" as of its next major version. Doing the same in child class "%s" will be required when upgrading.', $declaringClass, $method->name, $normalizedType, $className);
588605
}
589606
}
@@ -820,7 +837,7 @@ private function setReturnType(string $types, \ReflectionMethod $method, ?string
820837
} elseif ($n !== $normalizedType || !preg_match('/^\\\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)*+$/', $n)) {
821838
if ($iterable) {
822839
$normalizedType = $returnType = 'iterable';
823-
} elseif ($object) {
840+
} elseif ($object && 'object' === $this->patchTypes['force']) {
824841
$normalizedType = $returnType = 'object';
825842
} else {
826843
// ignore multi-types return declarations
@@ -947,7 +964,7 @@ private function patchMethod(\ReflectionMethod $method, string $returnType, stri
947964
}
948965
}
949966

950-
if ('docblock' === ($this->patchTypes['force'] ?? null) || ('object' === $normalizedType && ($this->patchTypes['php71-compat'] ?? false))) {
967+
if ('docblock' === $this->patchTypes['force'] || ('object' === $normalizedType && '7.1' === $this->patchTypes['php'])) {
951968
$returnType = implode('|', $returnType);
952969

953970
if ($method->getDocComment()) {
@@ -1015,7 +1032,7 @@ private static function getUseStatements(string $file): array
10151032

10161033
private function fixReturnStatements(\ReflectionMethod $method, string $returnType)
10171034
{
1018-
if (($this->patchTypes['php71-compat'] ?? false) && 'object' === ltrim($returnType, '?') && 'docblock' !== ($this->patchTypes['force'] ?? null)) {
1035+
if ('7.1' === $this->patchTypes['php'] && 'object' === ltrim($returnType, '?') && 'docblock' !== $this->patchTypes['force']) {
10191036
return;
10201037
}
10211038

@@ -1026,7 +1043,7 @@ private function fixReturnStatements(\ReflectionMethod $method, string $returnTy
10261043
$fixedCode = $code = file($file);
10271044
$i = (self::$fileOffsets[$file] ?? 0) + $method->getStartLine();
10281045

1029-
if ('?' !== $returnType && 'docblock' !== ($this->patchTypes['force'] ?? null)) {
1046+
if ('?' !== $returnType && 'docblock' !== $this->patchTypes['force']) {
10301047
$fixedCode[$i - 1] = preg_replace('/\)(;?\n)/', "): $returnType\\1", $code[$i - 1]);
10311048
}
10321049

‎src/Symfony/Component/ErrorHandler/Tests/DebugClassLoaderTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,15 @@
1616

1717
class DebugClassLoaderTest extends TestCase
1818
{
19-
/**
20-
* @var int Error reporting level before running tests
21-
*/
19+
private $patchTypes;
2220
private $errorReporting;
23-
2421
private $loader;
2522

2623
protected function setUp(): void
2724
{
25+
$this->patchTypes = getenv('SYMFONY_PATCH_TYPE_DECLARATIONS');
2826
$this->errorReporting = error_reporting(E_ALL);
27+
putenv('SYMFONY_PATCH_TYPE_DECLARATIONS=force=0');
2928
$this->loader = [new DebugClassLoader([new ClassLoader(), 'loadClass']), 'loadClass'];
3029
spl_autoload_register($this->loader, true, true);
3130
}
@@ -34,6 +33,7 @@ protected function tearDown(): void
3433
{
3534
spl_autoload_unregister($this->loader);
3635
error_reporting($this->errorReporting);
36+
putenv('SYMFONY_PATCH_TYPE_DECLARATIONS'.(false !== $this->patchTypes ? '='.$this->patchTypes : ''));
3737
}
3838

3939
/**

0 commit comments

Comments
 (0)