Skip to content

Navigation Menu

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 68d84d1

Browse filesBrowse 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
Copy full SHA for 68d84d1

File tree

Expand file treeCollapse file tree

4 files changed

+110
-28
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+110
-28
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Debug/DebugClassLoader.php
+3-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php
+23-15Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Debug/Tests/Fixtures/FinalClass.php
-10Lines changed: 0 additions & 10 deletions
This file was deleted.

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

Copy file name to clipboard
+84Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.