8000 bug #29920 [Debug][DebugClassLoader] Match more cases for final, depr… · symfony/symfony@68d84d1 · GitHub
[go: up one dir, main page]
Skip to content

Commit 68d84d1

Browse files
nicolas-grekasnicolas-grekas
committed
bug #29920 [Debug][DebugClassLoader] Match more cases for final, deprecated and internal classes / methods extends (fancyweb)
This PR was merged into the 3.4 branch. Discussion ---------- [Debug][DebugClassLoader] Match more cases for final, deprecated and internal classes / methods extends | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Currently, when there is no comment for a tag and another tag after, the detection does not work. Example : ```php /** * @Final * * @author John */ class A { } ``` AFAIK, those tags must not be in a specific order. That's why we should try to support more cases because we might miss things to report. Also I do not understand why the regex is not the same for the classes and methods detection. I fixed that too. I added a lot of cases in the "extends from final class" test and an easy way to add more when needed. Adding them everywhere might be overkill and useless. WDYT ? I'm considering this as bug fix. Commits ------- c3b670a [Debug][DebugClassLoader] Match more cases for final, deprecated and internal classes / methods extends
2 parents ccf6223 + c3b670a commit 68d84d1

File tree

4 files changed

+110
-28
lines changed

4 files changed

+110
-28
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,8 @@ public function checkAnnotations(\ReflectionClass $refl, $class)
233233
// Detect annotations on the class
234234
if (false !== $doc = $refl->getDocComment()) {
235235
foreach (['final', 'deprecated', 'internal'] as $annotation) {
236-
if (false !== \strpos($doc, $annotation) && preg_match('#\n \* @'.$annotation.'(?:( .+?)\.?)?\r?\n \*(?: @|/$)#s', $doc, $notice)) {
237-
self::${$annotation}[$class] = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
236+
if (false !== \strpos($doc, $annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
237+
self::${$annotation}[$class] = isset($notice[1]) ? preg_replace('#\.?\r?\n( \*)? *(?= |\r?\n|$)#', '', $notice[1]) : '';
238238
}
239239
}
240240
}
@@ -308,7 +308,7 @@ public function checkAnnotations(\ReflectionClass $refl, $class)
308308

309309
foreach (['final', 'internal'] as $annotation) {
310310
if (false !== \strpos($doc, $annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
311-
$message = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
311+
$message = isset($notice[1]) ? preg_replace('#\.?\r?\n( \*)? *(?= |\r?\n|$)#', '', $notice[1]) : '';
312312
self::${$annotation.'Methods'}[$class][$method->name] = [$class, $message];
313313
}
314314
}

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

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -290,24 +290,31 @@ class_exists('Test\\'.__NAMESPACE__.'\\Float', true);
290290

291291
public function testExtendedFinalClass()
292292
{
293-
set_error_handler(function () { return false; });
294-
$e = error_reporting(0);
295-
trigger_error('', E_USER_NOTICE);
293+
$deprecations = [];
294+
set_error_handler(function ($type, $msg) use (&$deprecations) { $deprecations[] = $msg; });
295+
$e = error_reporting(E_USER_DEPRECATED);
296296

297-
class_exists('Test\\'.__NAMESPACE__.'\\ExtendsFinalClass', true);
297+
require __DIR__.'/Fixtures/FinalClasses.php';
298+
299+
$i = 1;
300+
while(class_exists($finalClass = __NAMESPACE__.'\\Fixtures\\FinalClass'.$i++, false)) {
301+
spl_autoload_call($finalClass);
302+
class_exists('Test\\'.__NAMESPACE__.'\\Extends'.substr($finalClass, strrpos($finalClass, '\\') + 1), true);
303+
}
298304

299305
error_reporting($e);
300306
restore_error_handler();
301307

302-
$lastError = error_get_last();
303-
unset($lastError['file'], $lastError['line']);
304-
305-
$xError = [
306-
'type' => E_USER_DEPRECATED,
307-
'message' => 'The "Symfony\Component\Debug\Tests\Fixtures\FinalClass" class is considered final since version 3.3. It may change without further notice as of its next major version. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsFinalClass".',
308-
];
309-
310-
$this->assertSame($xError, $lastError);
308+
$this->assertSame([
309+
'The "Symfony\Component\Debug\Tests\Fixtures\FinalClass1" class is considered final since version 3.3. It may change without further notice as of its next major version. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsFinalClass1".',
310+
'The "Symfony\Component\Debug\Tests\Fixtures\FinalClass2" class is considered final. It may change without further notice as of its next major version. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsFinalClass2".',
311+
'The "Symfony\Component\Debug\Tests\Fixtures\FinalClass3" class is considered final comment with @@@ and ***. It may change without further notice as of its next major version. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsFinalClass3".',
312+
'The "Symfony\Component\Debug\Tests\Fixtures\FinalClass4" class is considered final. It may change without further notice as of its next major version. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsFinalClass4".',
313+
'The "Symfony\Component\Debug\Tests\Fixtures\FinalClass5" class is considered final multiline comment. It may change without further notice as of its next major version. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsFinalClass5".',
314+
'The "Symfony\Component\Debug\Tests\Fixtures\FinalClass6" class is considered final. It may change without further notice as of its next major version. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsFinalClass6".',
315+
'The "Symfony\Component\Debug\Tests\Fixtures\FinalClass7" class is considered final another multiline comment... It may change without further notice as of its next major version. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsFinalClass7".',
316+
'The "Symfony\Component\Debug\Tests\Fixtures\FinalClass8" class is considered final. It may change without further notice as of its next major version. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsFinalClass8".',
317+
], $deprecations);
311318
}
312319

313320
public function testExtendedFinalMethod()
@@ -417,8 +424,9 @@ public function findFile($class)
417424
eval('namespace Test\\'.__NAMESPACE__.'; class NonDeprecatedInterfaceClass implements \\'.__NAMESPACE__.'\Fixtures\NonDeprecatedInterface {}');
418425
} elseif ('Test\\'.__NAMESPACE__.'\Float' === $class) {
419426
eval('namespace Test\\'.__NAMESPACE__.'; class Float {}');
420-
} elseif ('Test\\'.__NAMESPACE__.'\ExtendsFinalClass' === $class) {
421-
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsFinalClass extends \\'.__NAMESPACE__.'\Fixtures\FinalClass {}');
427+
} elseif (0 === strpos($class, 'Test\\'.__NAMESPACE__.'\ExtendsFinalClass')) {
428+
$classShortName = substr($class, strrpos($class, '\\') + 1);
429+
eval('namespace Test\\'.__NAMESPACE__.'; class '.$classShortName.' extends \\'.__NAMESPACE__.'\Fixtures\\'.substr($classShortName, 7).' {}');
422430
} elseif ('Test\\'.__NAMESPACE__.'\ExtendsAnnotatedClass' === $class) {
423431
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsAnnotatedClass extends \\'.__NAMESPACE__.'\Fixtures\AnnotatedClass {
424432
public function deprecatedMethod() { }

‎src/Symfony/Component/Debug/Tests/Fixtures/FinalClass.php

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎src/Symfony/Component/Debug/Tests/Fixtures/FinalClasses.php

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
namespace Symfony\Component\Debug\Tests\Fixtures;
4+
5+
/**
6+
* @final since version 3.3.
7+
*/
8+
class FinalClass1
9+
{
10+
// simple comment
11+
}
12+
13+
/**
14+
* @final
15+
*/
16+
class FinalClass2
17+
{
18+
// no comment
19+
}
20+
21+
/**
22+
* @final comment with @@@ and ***
23+
*
24+
* @author John Doe
25+
*/
26+
class FinalClass3
27+
{
28+
// with comment and a tag after
29+
}
30+
31+
/**
32+
* @final
33+
* @author John Doe
34+
*/
35+
class FinalClass4
36+
{
37+
// without comment and a tag after
38+
}
39+
40+
/**
41+
* @author John Doe
42+
*
43+
*
44+
* @final multiline
45+
* comment
46+
*/
47+
class FinalClass5
48+
{
49+
// with comment and a tag before
50+
}
51+
52+
/**
53+
* @author John Doe
54+
*
55+
* @final
56+
*/
57+
class FinalClass6
58+
{
59+
// without comment and a tag before
60+
}
61+
62+
/**
63+
* @author John Doe
64+
*
65+
* @final another
66+
*
67+
* multiline comment...
68+
*
69+
* @return string
70+
*/
71+
class FinalClass7
72+
{
73+
// with comment and a tag before and after
74+
}
75+
76+
/**
77+
* @author John Doe
78+
* @final
79+
* @return string
80+
*/
81+
class FinalClass8
82+
{
83+
// without comment and a tag before and after
84+
}

0 commit comments

Comments
 (0)
0