Skip to content

Commit

Permalink
bug #29920 [Debug][DebugClassLoader] Match more cases for final, depr…
Browse files Browse the repository at this point in the history
…ecated 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
  • Loading branch information
nicolas-grekas committed Jan 24, 2019
2 parents ccf6223 + c3b670a commit 68d84d1
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 28 deletions.
6 changes: 3 additions & 3 deletions src/Symfony/Component/Debug/DebugClassLoader.php
Expand Up @@ -233,8 +233,8 @@ public function checkAnnotations(\ReflectionClass $refl, $class)
// Detect annotations on the class
if (false !== $doc = $refl->getDocComment()) {
foreach (['final', 'deprecated', 'internal'] as $annotation) {
if (false !== \strpos($doc, $annotation) && preg_match('#\n \* @'.$annotation.'(?:( .+?)\.?)?\r?\n \*(?: @|/$)#s', $doc, $notice)) {
self::${$annotation}[$class] = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
if (false !== \strpos($doc, $annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
self::${$annotation}[$class] = isset($notice[1]) ? preg_replace('#\.?\r?\n( \*)? *(?= |\r?\n|$)#', '', $notice[1]) : '';
}
}
}
Expand Down Expand Up @@ -308,7 +308,7 @@ public function checkAnnotations(\ReflectionClass $refl, $class)

foreach (['final', 'internal'] as $annotation) {
if (false !== \strpos($doc, $annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
$message = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
$message = isset($notice[1]) ? preg_replace('#\.?\r?\n( \*)? *(?= |\r?\n|$)#', '', $notice[1]) : '';
self::${$annotation.'Methods'}[$class][$method->name] = [$class, $message];
}
}
Expand Down
38 changes: 23 additions & 15 deletions src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php
Expand Up @@ -290,24 +290,31 @@ class_exists('Test\\'.__NAMESPACE__.'\\Float', true);

public function testExtendedFinalClass()
{
set_error_handler(function () { return false; });
$e = error_reporting(0);
trigger_error('', E_USER_NOTICE);
$deprecations = [];
set_error_handler(function ($type, $msg) use (&$deprecations) { $deprecations[] = $msg; });
$e = error_reporting(E_USER_DEPRECATED);

class_exists('Test\\'.__NAMESPACE__.'\\ExtendsFinalClass', true);
require __DIR__.'/Fixtures/FinalClasses.php';

$i = 1;
while(class_exists($finalClass = __NAMESPACE__.'\\Fixtures\\FinalClass'.$i++, false)) {
spl_autoload_call($finalClass);
class_exists('Test\\'.__NAMESPACE__.'\\Extends'.substr($finalClass, strrpos($finalClass, '\\') + 1), true);
}

error_reporting($e);
restore_error_handler();

$lastError = error_get_last();
unset($lastError['file'], $lastError['line']);

$xError = [
'type' => E_USER_DEPRECATED,
'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".',
];

$this->assertSame($xError, $lastError);
$this->assertSame([
'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".',
'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".',
'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".',
'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".',
'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".',
'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".',
'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".',
'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".',
], $deprecations);
}

public function testExtendedFinalMethod()
Expand Down Expand Up @@ -417,8 +424,9 @@ public function findFile($class)
eval('namespace Test\\'.__NAMESPACE__.'; class NonDeprecatedInterfaceClass implements \\'.__NAMESPACE__.'\Fixtures\NonDeprecatedInterface {}');
} elseif ('Test\\'.__NAMESPACE__.'\Float' === $class) {
eval('namespace Test\\'.__NAMESPACE__.'; class Float {}');
} elseif ('Test\\'.__NAMESPACE__.'\ExtendsFinalClass' === $class) {
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsFinalClass extends \\'.__NAMESPACE__.'\Fixtures\FinalClass {}');
} elseif (0 === strpos($class, 'Test\\'.__NAMESPACE__.'\ExtendsFinalClass')) {
$classShortName = substr($class, strrpos($class, '\\') + 1);
eval('namespace Test\\'.__NAMESPACE__.'; class '.$classShortName.' extends \\'.__NAMESPACE__.'\Fixtures\\'.substr($classShortName, 7).' {}');
} elseif ('Test\\'.__NAMESPACE__.'\ExtendsAnnotatedClass' === $class) {
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsAnnotatedClass extends \\'.__NAMESPACE__.'\Fixtures\AnnotatedClass {
public function deprecatedMethod() { }
Expand Down
10 changes: 0 additions & 10 deletions src/Symfony/Component/Debug/Tests/Fixtures/FinalClass.php

This file was deleted.

84 changes: 84 additions & 0 deletions src/Symfony/Component/Debug/Tests/Fixtures/FinalClasses.php
@@ -0,0 +1,84 @@
<?php

namespace Symfony\Component\Debug\Tests\Fixtures;

/**
* @final since version 3.3.
*/
class FinalClass1
{
// simple comment
}

/**
* @final
*/
class FinalClass2
{
// no comment
}

/**
* @final comment with @@@ and ***
*
* @author John Doe
*/
class FinalClass3
{
// with comment and a tag after
}

/**
* @final
* @author John Doe
*/
class FinalClass4
{
// without comment and a tag after
}

/**
* @author John Doe
*
*
* @final multiline
* comment
*/
class FinalClass5
{
// with comment and a tag before
}

/**
* @author John Doe
*
* @final
*/
class FinalClass6
{
// without comment and a tag before
}

/**
* @author John Doe
*
* @final another
*
* multiline comment...
*
* @return string
*/
class FinalClass7
{
// with comment and a tag before and after
}

/**
* @author John Doe
* @final
* @return string
*/
class FinalClass8
{
// without comment and a tag before and after
}

0 comments on commit 68d84d1

Please sign in to comment.