From 46c4f71bf095f229fa540551a3d903535b779fc1 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 22 Sep 2018 12:34:13 +0200 Subject: [PATCH] [Debug] Fix false-positive "MicroKernelTrait::loadRoutes()" method is considered internal" --- .../Component/Debug/DebugClassLoader.php | 385 ++++++++++-------- .../Debug/Tests/DebugClassLoaderTest.php | 20 +- .../Fixtures/TraitWithInternalMethod.php | 13 + 3 files changed, 237 insertions(+), 181 deletions(-) create mode 100644 src/Symfony/Component/Debug/Tests/Fixtures/TraitWithInternalMethod.php diff --git a/src/Symfony/Component/Debug/DebugClassLoader.php b/src/Symfony/Component/Debug/DebugClassLoader.php index 32cbd31607..c9fa76cfe7 100644 --- a/src/Symfony/Component/Debug/DebugClassLoader.php +++ b/src/Symfony/Component/Debug/DebugClassLoader.php @@ -129,8 +129,6 @@ class DebugClassLoader * * @param string $class The name of the class * - * @return bool|null True, if loaded - * * @throws \RuntimeException */ public function loadClass($class) @@ -184,198 +182,227 @@ class DebugClassLoader throw new \RuntimeException(sprintf('Case mismatch between loaded and declared class names: "%s" vs "%s".', $class, $name)); } - // Don't trigger deprecations for classes in the same vendor - if (2 > $len = 1 + (\strpos($name, '\\') ?: \strpos($name, '_'))) { - $len = 0; - $ns = ''; - } else { - $ns = \substr($name, 0, $len); - } - - // Detect annotations on the class - if (false !== $doc = $refl->getDocComment()) { - foreach (array('final', 'deprecated', 'internal') as $annotation) { - if (false !== \strpos($doc, $annotation) && preg_match('#\n \* @'.$annotation.'(?:( .+?)\.?)?\r?\n \*(?: @|/$)#s', $doc, $notice)) { - self::${$annotation}[$name] = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : ''; - } - } - } - - $parentAndTraits = \class_uses($name, false); - if ($parent = \get_parent_class($class)) { - $parentAndTraits[] = $parent; - - if (!isset(self::$checkedClasses[$parent])) { - $this->checkClass($parent); - } - - if (isset(self::$final[$parent])) { - @trigger_error(sprintf('The "%s" class is considered final%s. It may change without further notice as of its next major version. You should not extend it from "%s".', $parent, self::$final[$parent], $name), E_USER_DEPRECATED); - } - } - - // Detect if the parent is annotated - foreach ($parentAndTraits + $this->getOwnInterfaces($name, $parent) as $use) { - if (!isset(self::$checkedClasses[$use])) { - $this->checkClass($use); - } - if (isset(self::$deprecated[$use]) && \strncmp($ns, $use, $len)) { - $type = class_exists($name, false) ? 'class' : (interface_exists($name, false) ? 'interface' : 'trait'); - $verb = class_exists($use, false) || interface_exists($name, false) ? 'extends' : (interface_exists($use, false) ? 'implements' : 'uses'); - - @trigger_error(sprintf('The "%s" %s %s "%s" that is deprecated%s.', $name, $type, $verb, $use, self::$deprecated[$use]), E_USER_DEPRECATED); - } - if (isset(self::$internal[$use]) && \strncmp($ns, $use, $len)) { - @trigger_error(sprintf('The "%s" %s is considered internal%s. It may change without further notice. You should not use it from "%s".', $use, class_exists($use, false) ? 'class' : (interface_exists($use, false) ? 'interface' : 'trait'), self::$internal[$use], $name), E_USER_DEPRECATED); - } - } - - // Inherit @final and @internal annotations for methods - self::$finalMethods[$name] = array(); - self::$internalMethods[$name] = array(); - foreach ($parentAndTraits as $use) { - foreach (array('finalMethods', 'internalMethods') as $property) { - if (isset(self::${$property}[$use])) { - self::${$property}[$name] = self::${$property}[$name] ? self::${$property}[$use] + self::${$property}[$name] : self::${$property}[$use]; - } - } - } - - $isClass = \class_exists($name, false); - foreach ($refl->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) as $method) { - if ($method->class !== $name) { - continue; - } - - if ($isClass && $parent && isset(self::$finalMethods[$parent][$method->name])) { - list($declaringClass, $message) = self::$finalMethods[$parent][$method->name]; - @trigger_error(sprintf('The "%s::%s()" method is considered final%s. It may change without further notice as of its next major version. You should not extend it from "%s".', $declaringClass, $method->name, $message, $name), E_USER_DEPRECATED); - } - - if (isset(self::$internalMethods[$name][$method->name])) { - list($declaringClass, $message) = self::$internalMethods[$name][$method->name]; - if (\strncmp($ns, $declaringClass, $len)) { - @trigger_error(sprintf('The "%s::%s()" method is considered internal%s. It may change without further notice. You should not extend it from "%s".', $declaringClass, $method->name, $message, $name), E_USER_DEPRECATED); - } - } - - // Method from a trait - if ($method->getFilename() !== $refl->getFileName()) { - continue; - } - - // Detect method annotations - if (false === $doc = $method->getDocComment()) { - continue; - } - - foreach (array('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]) : ''; - self::${$annotation.'Methods'}[$name][$method->name] = array($name, $message); - } - } - } + $deprecations = $this->checkAnnotations($refl, $name); if (isset(self::$php7Reserved[\strtolower($refl->getShortName())])) { - @trigger_error(sprintf('The "%s" class uses the reserved name "%s", it will break on PHP 7 and higher', $name, $refl->getShortName()), E_USER_DEPRECATED); + $deprecations[] = sprintf('The "%s" class uses the reserved name "%s", it will break on PHP 7 and higher', $name, $refl->getShortName()); + } + + foreach ($deprecations as $message) { + @trigger_error($message, E_USER_DEPRECATED); } } - if ($file) { - if (!$exists) { - if (false !== strpos($class, '/')) { - throw new \RuntimeException(sprintf('Trying to autoload a class with an invalid name "%s". Be careful that the namespace separator is "\" in PHP, not "/".', $class)); - } + if (!$file) { + return; + } - throw new \RuntimeException(sprintf('The autoloader expected class "%s" to be defined in file "%s". The file was found but the class was not in it, the class name or namespace probably has a typo.', $class, $file)); + if (!$exists) { + if (false !== strpos($class, '/')) { + throw new \RuntimeException(sprintf('Trying to autoload a class with an invalid name "%s". Be careful that the namespace separator is "\" in PHP, not "/".', $class)); } - if (self::$caseCheck) { - $real = explode('\\', $class.strrchr($file, '.')); - $tail = explode(\DIRECTORY_SEPARATOR, str_replace('/', \DIRECTORY_SEPARATOR, $file)); - $i = \count($tail) - 1; - $j = \count($real) - 1; + throw new \RuntimeException(sprintf('The autoloader expected class "%s" to be defined in file "%s". The file was found but the class was not in it, the class name or namespace probably has a typo.', $class, $file)); + } - while (isset($tail[$i], $real[$j]) && $tail[$i] === $real[$j]) { - --$i; - --$j; - } + if (self::$caseCheck && $message = $this->checkCase($refl, $file, $class)) { + throw new \RuntimeException(sprintf('Case mismatch between class and real file names: "%s" vs "%s" in "%s".', $message[0], $message[1], $message[2])); + } + } - array_splice($tail, 0, $i + 1); - } - if (self::$caseCheck && $tail) { - $tail = \DIRECTORY_SEPARATOR.implode(\DIRECTORY_SEPARATOR, $tail); - $tailLen = \strlen($tail); - $real = $refl->getFileName(); + public function checkAnnotations(\ReflectionClass $refl, $class) + { + $deprecations = array(); - if (2 === self::$caseCheck) { - // realpath() on MacOSX doesn't normalize the case of characters + // Don't trigger deprecations for classes in the same vendor + if (2 > $len = 1 + (\strpos($class, '\\') ?: \strpos($class, '_'))) { + $len = 0; + $ns = ''; + } else { + $ns = \substr($class, 0, $len); + } - $i = 1 + strrpos($real, '/'); - $file = substr($real, $i); - $real = substr($real, 0, $i); - - if (isset(self::$darwinCache[$real])) { - $kDir = $real; - } else { - $kDir = strtolower($real); - - if (isset(self::$darwinCache[$kDir])) { - $real = self::$darwinCache[$kDir][0]; - } else { - $dir = getcwd(); - chdir($real); - $real = getcwd().'/'; - chdir($dir); - - $dir = $real; - $k = $kDir; - $i = \strlen($dir) - 1; - while (!isset(self::$darwinCache[$k])) { - self::$darwinCache[$k] = array($dir, array()); - self::$darwinCache[$dir] = &self::$darwinCache[$k]; - - while ('/' !== $dir[--$i]) { - } - $k = substr($k, 0, ++$i); - $dir = substr($dir, 0, $i--); - } - } - } - - $dirFiles = self::$darwinCache[$kDir][1]; - - if (isset($dirFiles[$file])) { - $kFile = $file; - } else { - $kFile = strtolower($file); - - if (!isset($dirFiles[$kFile])) { - foreach (scandir($real, 2) as $f) { - if ('.' !== $f[0]) { - $dirFiles[$f] = $f; - if ($f === $file) { - $kFile = $k = $file; - } elseif ($f !== $k = strtolower($f)) { - $dirFiles[$k] = $f; - } - } - } - self::$darwinCache[$kDir][1] = $dirFiles; - } - } - - $real .= $dirFiles[$kFile]; - } - - if (0 === substr_compare($real, $tail, -$tailLen, $tailLen, true) - && 0 !== substr_compare($real, $tail, -$tailLen, $tailLen, false) - ) { - throw new \RuntimeException(sprintf('Case mismatch between class and real file names: "%s" vs "%s" in "%s".', substr($tail, -$tailLen + 1), substr($real, -$tailLen + 1), substr($real, 0, -$tailLen + 1))); + // Detect annotations on the class + if (false !== $doc = $refl->getDocComment()) { + foreach (array('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]) : ''; } } } + + $parent = \get_parent_class($class); + $parentAndOwnInterfaces = $this->getOwnInterfaces($class, $parent); + if ($parent) { + $parentAndOwnInterfaces[$parent] = $parent; + + if (!isset(self::$checkedClasses[$parent])) { + $this->checkClass($parent); + } + + if (isset(self::$final[$parent])) { + $deprecations[] = sprintf('The "%s" class is considered final%s. It may change without further notice as of its next major version. You should not extend it from "%s".', $parent, self::$final[$parent], $class); + } + } + + // Detect if the parent is annotated + foreach ($parentAndOwnInterfaces + \class_uses($class, false) as $use) { + if (!isset(self::$checkedClasses[$use])) { + $this->checkClass($use); + } + if (isset(self::$deprecated[$use]) && \strncmp($ns, $use, $len)) { + $type = class_exists($class, false) ? 'class' : (interface_exists($class, false) ? 'interface' : 'trait'); + $verb = class_exists($use, false) || interface_exists($class, false) ? 'extends' : (interface_exists($use, false) ? 'implements' : 'uses'); + + $deprecations[] = sprintf('The "%s" %s %s "%s" that is deprecated%s.', $class, $type, $verb, $use, self::$deprecated[$use]); + } + if (isset(self::$internal[$use]) && \strncmp($ns, $use, $len)) { + $deprecations[] = sprintf('The "%s" %s is considered internal%s. It may change without further notice. You should not use it from "%s".', $use, class_exists($use, false) ? 'class' : (interface_exists($use, false) ? 'interface' : 'trait'), self::$internal[$use], $class); + } + } + + if (\trait_exists($class)) { + return $deprecations; + } + + // Inherit @final and @internal annotations for methods + self::$finalMethods[$class] = array(); + self::$internalMethods[$class] = array(); + foreach ($parentAndOwnInterfaces as $use) { + foreach (array('finalMethods', 'internalMethods') as $property) { + if (isset(self::${$property}[$use])) { + self::${$property}[$class] = self::${$property}[$class] ? self::${$property}[$use] + self::${$property}[$class] : self::${$property}[$use]; + } + } + } + + foreach ($refl->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) as $method) { + if ($method->class !== $class) { + continue; + } + + if ($parent && isset(self::$finalMethods[$parent][$method->name])) { + list($declaringClass, $message) = self::$finalMethods[$parent][$method->name]; + $deprecations[] = sprintf('The "%s::%s()" method is considered final%s. It may change without further notice as of its next major version. You should not extend it from "%s".', $declaringClass, $method->name, $message, $class); + } + + if (isset(self::$internalMethods[$class][$method->name])) { + list($declaringClass, $message) = self::$internalMethods[$class][$method->name]; + if (\strncmp($ns, $declaringClass, $len)) { + $deprecations[] = sprintf('The "%s::%s()" method is considered internal%s. It may change without further notice. You should not extend it from "%s".', $declaringClass, $method->name, $message, $class); + } + } + + // Detect method annotations + if (false === $doc = $method->getDocComment()) { + continue; + } + + foreach (array('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]) : ''; + self::${$annotation.'Methods'}[$class][$method->name] = array($class, $message); + } + } + } + + return $deprecations; + } + + public function checkCase(\ReflectionClass $refl, $file, $class) + { + $real = explode('\\', $class.strrchr($file, '.')); + $tail = explode(\DIRECTORY_SEPARATOR, str_replace('/', \DIRECTORY_SEPARATOR, $file)); + + $i = \count($tail) - 1; + $j = \count($real) - 1; + + while (isset($tail[$i], $real[$j]) && $tail[$i] === $real[$j]) { + --$i; + --$j; + } + + array_splice($tail, 0, $i + 1); + + if (!$tail) { + return; + } + + $tail = \DIRECTORY_SEPARATOR.implode(\DIRECTORY_SEPARATOR, $tail); + $tailLen = \strlen($tail); + $real = $refl->getFileName(); + + if (2 === self::$caseCheck) { + $real = $this->darwinRealpath($real); + } + + if (0 === substr_compare($real, $tail, -$tailLen, $tailLen, true) + && 0 !== substr_compare($real, $tail, -$tailLen, $tailLen, false) + ) { + return array(substr($tail, -$tailLen + 1), substr($real, -$tailLen + 1), substr($real, 0, -$tailLen + 1)); + } + } + + /** + * `realpath` on MacOSX doesn't normalize the case of characters. + */ + private function darwinRealpath($real) + { + $i = 1 + strrpos($real, '/'); + $file = substr($real, $i); + $real = substr($real, 0, $i); + + if (isset(self::$darwinCache[$real])) { + $kDir = $real; + } else { + $kDir = strtolower($real); + + if (isset(self::$darwinCache[$kDir])) { + $real = self::$darwinCache[$kDir][0]; + } else { + $dir = getcwd(); + chdir($real); + $real = getcwd().'/'; + chdir($dir); + + $dir = $real; + $k = $kDir; + $i = \strlen($dir) - 1; + while (!isset(self::$darwinCache[$k])) { + self::$darwinCache[$k] = array($dir, array()); + self::$darwinCache[$dir] = &self::$darwinCache[$k]; + + while ('/' !== $dir[--$i]) { + } + $k = substr($k, 0, ++$i); + $dir = substr($dir, 0, $i--); + } + } + } + + $dirFiles = self::$darwinCache[$kDir][1]; + + if (isset($dirFiles[$file])) { + return $real .= $dirFiles[$file]; + } + + $kFile = strtolower($file); + + if (!isset($dirFiles[$kFile])) { + foreach (scandir($real, 2) as $f) { + if ('.' !== $f[0]) { + $dirFiles[$f] = $f; + if ($f === $file) { + $kFile = $k = $file; + } elseif ($f !== $k = strtolower($f)) { + $dirFiles[$k] = $f; + } + } + } + self::$darwinCache[$kDir][1] = $dirFiles; + } + + return $real .= $dirFiles[$kFile]; } /** diff --git a/src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php b/src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php index cd89c06e58..e9ce535ee7 100644 --- a/src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php +++ b/src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php @@ -358,12 +358,26 @@ class DebugClassLoaderTest extends TestCase restore_error_handler(); $this->assertSame($deprecations, array( - 'The "Symfony\Component\Debug\Tests\Fixtures\InternalClass" class is considered internal since version 3.4. It may change without further notice. You should not use it from "Test\Symfony\Component\Debug\Tests\ExtendsInternalsParent".', 'The "Symfony\Component\Debug\Tests\Fixtures\InternalInterface" interface is considered internal. It may change without further notice. You should not use it from "Test\Symfony\Component\Debug\Tests\ExtendsInternalsParent".', + 'The "Symfony\Component\Debug\Tests\Fixtures\InternalClass" class is considered internal since version 3.4. It may change without further notice. You should not use it from "Test\Symfony\Component\Debug\Tests\ExtendsInternalsParent".', 'The "Symfony\Component\Debug\Tests\Fixtures\InternalTrait" trait is considered internal. It may change without further notice. You should not use it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".', - 'The "Symfony\Component\Debug\Tests\Fixtures\InternalTrait2::internalMethod()" method is considered internal since version 3.4. It may change without further notice. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".', + 'The "Symfony\Component\Debug\Tests\Fixtures\InternalClass::internalMethod()" method is considered internal since version 3.4. It may change without further notice. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".', )); } + + public function testUseTraitWithInternalMethod() + { + $deprecations = array(); + set_error_handler(function ($type, $msg) use (&$deprecations) { $deprecations[] = $msg; }); + $e = error_reporting(E_USER_DEPRECATED); + + class_exists('Test\\'.__NAMESPACE__.'\\UseTraitWithInternalMethod', true); + + error_reporting($e); + restore_error_handler(); + + $this->assertSame(array(), $deprecations); + } } class ClassLoader @@ -417,6 +431,8 @@ class ClassLoader }'); } elseif ('Test\\'.__NAMESPACE__.'\ExtendsInternalsParent' === $class) { eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsInternalsParent extends \\'.__NAMESPACE__.'\Fixtures\InternalClass implements \\'.__NAMESPACE__.'\Fixtures\InternalInterface { }'); + } elseif ('Test\\'.__NAMESPACE__.'\UseTraitWithInternalMethod' === $class) { + eval('namespace Test\\'.__NAMESPACE__.'; class UseTraitWithInternalMethod { use \\'.__NAMESPACE__.'\Fixtures\TraitWithInternalMethod; }'); } } } diff --git a/src/Symfony/Component/Debug/Tests/Fixtures/TraitWithInternalMethod.php b/src/Symfony/Component/Debug/Tests/Fixtures/TraitWithInternalMethod.php new file mode 100644 index 0000000000..4c3dae37ab --- /dev/null +++ b/src/Symfony/Component/Debug/Tests/Fixtures/TraitWithInternalMethod.php @@ -0,0 +1,13 @@ +