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 c51592c

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
feature #28329 [Debug] Trigger a deprecation for new parameters not defined in sub classes (GuilhemN)
This PR was merged into the 4.2-dev branch. Discussion ---------- [Debug] Trigger a deprecation for new parameters not defined in sub classes | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28316 | License | MIT | Doc PR | - I'm not sure the way #28316 is implemented is the best so here is an alternative. Instead of counting on a call from the child method, it uses the `DebugClassLoader` and `@param` annotations. If a `@param` annotation is used on a parent but is then not actually implemented in the child class, a deprecation will be thrown. Example: ```php class ClassWithAnnotatedParameters { /** * @param string $foo This is a foo parameter. */ public function fooMethod(string $foo) { } /** * @param string $bar parameter not implemented yet */ public function barMethod(/** string $bar = null */) { } /** * @param Quz $quz parameter not implemented yet */ public function quzMethod(/** Quz $quz = null */) { } } ``` ```php class SubClassWithAnnotatedParameters extends ClassWithAnnotatedParameters { public function fooMethod(string $foo) { } public function barMethod($bar = null) { } public function quzMethod() { } } ``` A deprecation will be triggered because ``SubClassWithAnnotatedParameters::quzMethod()`` which doesn't definee `$quz`. Commits ------- 1f5d8b6 [Debug] Trigger a deprecation for new parameters not defined in sub classes
2 parents 1fc66ff + 1f5d8b6 commit c51592c
Copy full SHA for c51592c

8 files changed

+174
-14
lines changed

‎src/Symfony/Bridge/Monolog/Logger.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Monolog/Logger.php
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ class Logger extends BaseLogger implements DebugLoggerInterface, ResetInterface
2323
{
2424
/**
2525
* {@inheritdoc}
26+
*
27+
* @param Request|null $request
2628
*/
2729
public function getLogs(/* Request $request = null */)
2830
{
@@ -39,6 +41,8 @@ public function getLogs(/* Request $request = null */)
3941

4042
/**
4143
* {@inheritdoc}
44+
*
45+
* @param Request|null $request
4246
*/
4347
public function countErrors(/* Request $request = null */)
4448
{

‎src/Symfony/Bridge/Monolog/Processor/DebugProcessor.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Monolog/Processor/DebugProcessor.php
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ public function __invoke(array $record)
5858

5959
/**
6060
* {@inheritdoc}
61+
*
62+
* @param Request|null $request
6163
*/
6264
public function getLogs(/* Request $request = null */)
6365
{
@@ -78,6 +80,8 @@ public function getLogs(/* Request $request = null */)
7880

7981
/**
8082
* {@inheritdoc}
83+
*
84+
* @param Request|null $request
8185
*/
8286
public function countErrors(/* Request $request = null */)
8387
{

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Debug/DebugClassLoader.php
+63-14Lines changed: 63 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Debug;
1313

14+
use PHPUnit\Framework\MockObject\Matcher\StatelessInvocation;
15+
1416
/**
1517
* Autoloader checking if the class is really defined in the file found.
1618
*
@@ -21,6 +23,7 @@
2123
* @author Fabien Potencier <fabien@symfony.com>
2224
* @author Christophe Coevoet <stof@notk.org>
2325
* @author Nicolas Grekas <p@tchwork.com>
26+
* @author Guilhem Niot <guilhem.niot@gmail.com>
2427
*/
2528
class DebugClassLoader
2629
{
@@ -34,6 +37,7 @@ class DebugClassLoader
3437
private static $deprecated = array();
3538
private static $internal = array();
3639
private static $internalMethods = array();
40+
private static $annotatedParameters = array();
3741
private static $darwinCache = array('/' => array('/', array()));
3842

3943
public function __construct(callable $classLoader)
@@ -200,9 +204,12 @@ private function checkClass($class, $file = null)
200204
}
201205
}
202206

207+
$parent = \get_parent_class($class);
203208
$parentAndTraits = \class_uses($name, false);
204-
if ($parent = \get_parent_class($class)) {
205-
$parentAndTraits[] = $parent;
209+
$parentAndOwnInterfaces = $this->getOwnInterfaces($name, $parent);
210+
if ($parent) {
211+
$parentAndTraits[$parent] = $parent;
212+
$parentAndOwnInterfaces[$parent] = $parent;
206213

207214
if (!isset(self::$checkedClasses[$parent])) {
208215
$this->checkClass($parent);
@@ -214,7 +221,7 @@ private function checkClass($class, $file = null)
214221
}
215222

216223
// Detect if the parent is annotated
217-
foreach ($parentAndTraits + $this->getOwnInterfaces($name, $parent) as $use) {
224+
foreach ($parentAndTraits + $parentAndOwnInterfaces as $use) {
218225
if (!isset(self::$checkedClasses[$use])) {
219226
$this->checkClass($use);
220227
}
@@ -229,11 +236,17 @@ private function checkClass($class, $file = null)
229236
}
230237
}
231238

232-
// Inherit @final and @internal annotations for methods
239+
// Inherit @final, @internal and @param annotations for methods
233240
self::$finalMethods[$name] = array();
234241
self::$internalMethods[$name] = array();
235-
foreach ($parentAndTraits as $use) {
236-
foreach (array('finalMethods', 'internalMethods') as $property) {
242+
self::$annotatedParameters[$name] = array();
243+
$map = array(
244+
'finalMethods' => $parentAndTraits,
245+
'internalMethods' => $parentAndTraits,
246+
'annotatedParameters' => $parentAndOwnInterfaces, // We don't parse traits params
247+
);
248+
foreach ($map as $property => $uses) {
249+
foreach ($uses as $use) {
237250
if (isset(self::${$property}[$use])) {
238251
self::${$property}[$name] = self::${$property}[$name] ? self::${$property}[$use] + self::${$property}[$name] : self::${$property}[$use];
239252
}
@@ -258,20 +271,56 @@ private function checkClass($class, $file = null)
258271
}
259272
}
260273

261-
// Method from a trait
262-
if ($method->getFilename() !== $refl->getFileName()) {
274+
// To read method annotations
275+
$doc = $method->getDocComment();
276+
277+
if (isset(self::$annotatedParameters[$name][$method->name])) {
278+
$definedParameters = array();
279+
foreach ($method->getParameters() as $parameter) {
280+
$definedParameters[$parameter->name] = true;
281+
}
282+
283+
foreach (self::$annotatedParameters[$name][$method->name] as $parameterName => $deprecation) {
284+
if (!isset($definedParameters[$parameterName]) && !($doc && preg_match("/\\n\\s+\\* @param (.*?)(?<= )\\\${$parameterName}\\b/", $doc))) {
285+
@trigger_error(sprintf($deprecation, $name), E_USER_DEPRECATED);
286+
}
287+
}
288+
}
289+
290+
if (!$doc) {
263291
continue;
264292
}
265293

266-
// Detect method annotations
267-
if (false === $doc = $method->getDocComment()) {
294+
$finalOrInternal = false;
295+
296+
// Skip methods from traits
297+
if ($method->getFilename() === $refl->getFileName()) {
298+
foreach (array('final', 'internal') as $annotation) {
299+
if (false !== \strpos($doc, $annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
300+
$message = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
301+
self::${$annotation.'Methods'}[$name][$method->name] = array($name, $message);
302+
$finalOrInternal = true;
303+
}
304+
}
305+
}
306+
307+
if ($finalOrInternal || $method->isConstructor() || false === \strpos($doc, '@param') || StatelessInvocation::class === $name) {
268308
continue;
269309
}
270310

271-
foreach (array('final', 'internal') as $annotation) {
272-
if (false !== \strpos($doc, $annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
273-
$message = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
274-
self::${$annotation.'Methods'}[$name][$method->name] = array($name, $message);
311+
if (!preg_match_all('#\n\s+\* @param (.*?)(?<= )\$([a-zA-Z0-9_\x7f-\xff]++)#', $doc, $matches, PREG_SET_ORDER)) {
312+
continue;
313+
}
314+
if (!isset(self::$annotatedParameters[$name][$method->name])) {
315+
$definedParameters = array();
316+
foreach ($method->getParameters() as $parameter) {
317+
$definedParameters[$parameter->name] = true;
318+
}
319+
}
320+
foreach ($matches as list(, $parameterType, $parameterName)) {
321+
if (!isset($definedParameters[$parameterName])) {
322+
$parameterType = trim($parameterType);
323+
self::$annotatedParameters[$name][$method->name][$parameterName] = sprintf('The "%%s::%s()" method will require a new "%s$%s" argument in the next major version of its parent class "%s", not defining it is deprecated.', $method->name, $parameterType ? $parameterType.' ' : '', $parameterName, $method->class);
275324
}
276325
}
277326
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,24 @@ class_exists('Test\\'.__NAMESPACE__.'\\ExtendsInternals', true);
272272
'The "Symfony\Component\Debug\Tests\Fixtures\InternalTrait2::internalMethod()" method is considered internal. It may change without further notice. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".',
273273
));
274274
}
275+
276+
public function testExtendedMethodDefinesNewParameters()
277+
{
278+
$deprecations = array();
279+
set_error_handler(function ($type, $msg) use (&$deprecations) { $deprecations[] = $msg; });
280+
$e = error_reporting(E_USER_DEPRECATED);
281+
282+
class_exists(__NAMESPACE__.'\\Fixtures\SubClassWithAnnotatedParameters', true);
283+
284+
error_reporting($e);
285+
restore_error_handler();
286+
287+
$this->assertSame(array(
288+
'The "Symfony\Component\Debug\Tests\Fixtures\SubClassWithAnnotatedParameters::quzMethod()" method will require a new "Quz $quz" argument in the next major version of its parent class "Symfony\Component\Debug\Tests\Fixtures\ClassWithAnnotatedParameters", not defining it is deprecated.',
289+
'The "Symfony\Component\Debug\Tests\Fixtures\SubClassWithAnnotatedParameters::whereAmI()" method will require a new "bool $matrix" argument in the next major version of its parent class "Symfony\Component\Debug\Tests\Fixtures\InterfaceWithAnnotatedParameters", not defining it is deprecated.',
290+
'The "Symfony\Component\Debug\Tests\Fixtures\SubClassWithAnnotatedParameters::isSymfony()" method will require a new "true $yes" argument in the next major version of its parent class "Symfony\Component\Debug\Tests\Fixtures\ClassWithAnnotatedParameters", not defining it is deprecated.',
291+
), $deprecations);
292+
}
275293
}
276294

277295
class ClassLoader

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

Copy file name to clipboard
+34Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace Symfony\Component\Debug\Tests\Fixtures;
4+
5+
class ClassWithAnnotatedParameters
6+
{
7+
/**
8+
* @param string $foo this is a foo parameter
9+
*/
10+
public function fooMethod(string $foo)
11+
{
12+
}
13+
14+
/**
15+
* @param string $bar parameter not implemented yet
16+
*/
17+
public function barMethod(/* string $bar = null */)
18+
{
19+
}
20+
21+
/**
22+
* @param Quz $quz parameter not implemented yet
23+
*/
24+
public function quzMethod(/* Quz $quz = null */)
25+
{
26+
}
27+
28+
/**
29+
* @param true $yes
30+
*/
31+
public function isSymfony()
32+
{
33+
}
34+
}

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

Copy file name to clipboard
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
namespace Symfony\Component\Debug\Tests\Fixtures;
4+
5+
/**
6+
* Ensures a deprecation is triggered when a new parameter is not declared in child classes.
7+
*/
8+
interface InterfaceWithAnnotatedParameters
9+
{
10+
/**
11+
* @param bool $matrix
12+
*/
13+
public function whereAmI();
14+
}

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

Copy file name to clipboard
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
namespace Symfony\Component\Debug\Tests\Fixtures;
4+
5+
class SubClassWithAnnotatedParameters extends ClassWithAnnotatedParameters implements InterfaceWithAnnotatedParameters
6+
{
7+
use TraitWithAnnotatedParameters;
8+
9+
public function fooMethod(string $foo)
10+
{
11+
}
12+
13+
public function barMethod($bar = null)
14+
{
15+
}
16+
17+
public function quzMethod()
18+
{
19+
}
20+
21+
public function whereAmI()
22+
{
23+
}
24+
}

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

Copy file name to clipboard
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
namespace Symfony\Component\Debug\Tests\Fixtures;
4+
5+
trait TraitWithAnnotatedParameters
6+
{
7+
/**
8+
* `@param` annotations in traits are not parsed.
9+
*/
10+
public function isSymfony()
11+
{
12+
}
13+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.