8000 [ErrorHandler] don't throw deprecations for return-types by default · symfony/symfony@2cb419e · GitHub
[go: up one dir, main page]
Skip to content
Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

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)
0