From 49fd1be15bb37abcca4be1aaa68b725e904472a7 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Sat, 1 Dec 2018 12:50:43 +0200 Subject: [PATCH] Fixed compilation error with inheritance of prototype interfaces Closes: https://github.com/phalcon/zephir/issues/1758 --- CHANGELOG.md | 3 + Library/ClassDefinition.php | 102 ++++++++++++------ Library/Compiler.php | 65 ++++++----- Library/CompilerFile.php | 4 +- unit-tests/Zephir/Test/ProtoDirTest.php | 86 +++++++++++++++ unit-tests/fixtures/protodir/config.json | 53 +++++++++ .../fixtures/protodir/connectionexception.h | 11 ++ .../protodir/protodir/ConnectionException.zep | 10 ++ .../fake/ClientExceptionInterface.php | 7 ++ .../fake/NetworkExceptionInterface.php | 8 ++ 10 files changed, 291 insertions(+), 58 deletions(-) create mode 100644 unit-tests/Zephir/Test/ProtoDirTest.php create mode 100644 unit-tests/fixtures/protodir/config.json create mode 100644 unit-tests/fixtures/protodir/connectionexception.h create mode 100644 unit-tests/fixtures/protodir/protodir/ConnectionException.zep create mode 100644 unit-tests/fixtures/protodir/prototypes/fake/ClientExceptionInterface.php create mode 100644 unit-tests/fixtures/protodir/prototypes/fake/NetworkExceptionInterface.php diff --git a/CHANGELOG.md b/CHANGELOG.md index b9319d6636..295af2c0f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Fixed +- Fixed compilation error with inheritance of prototype interfaces + [#1758](https://github.com/phalcon/zephir/issues/1758) ## [0.11.7] - 2018-11-27 ### Changed diff --git a/Library/ClassDefinition.php b/Library/ClassDefinition.php index 9dab3ce62a..4b1aa556de 100644 --- a/Library/ClassDefinition.php +++ b/Library/ClassDefinition.php @@ -20,7 +20,7 @@ * * Represents a class/interface and their properties and methods. */ -class ClassDefinition +final class ClassDefinition { /** @var string */ protected $namespace; @@ -49,11 +49,11 @@ class ClassDefinition /** @var bool */ protected $external = false; - /** @var ClassDefinition */ + /** @var ClassDefinitionRuntime|ClassDefinition */ protected $extendsClassDefinition; /** @var ClassDefinition[] */ - protected $implementedInterfaceDefinitions; + protected $implementedInterfaceDefinitions = []; /** @var ClassProperty[] */ protected $properties = []; @@ -350,7 +350,7 @@ public function getImplementedInterfaces() /** * Sets the class definition for the extended class. * - * @param $classDefinition + * @param ClassDefinitionRuntime|ClassDefinition $classDefinition */ public function setExtendsClassDefinition($classDefinition) { @@ -360,7 +360,7 @@ public function setExtendsClassDefinition($classDefinition) /** * Returns the class definition related to the extended class. * - * @return ClassDefinition + * @return ClassDefinition|ClassDefinitionRuntime */ public function getExtendsClassDefinition() { @@ -401,18 +401,13 @@ public function getImplementedInterfaceDefinitions() public function getDependencies() { $dependencies = []; - if ($this->extendsClassDefinition) { - $classDefinition = $this->extendsClassDefinition; - if (method_exists($classDefinition, 'increaseDependencyRank')) { - $dependencies[] = $classDefinition; - } + if ($this->extendsClassDefinition && $this->extendsClassDefinition instanceof self) { + $dependencies[] = $this->extendsClassDefinition; } - if ($this->implementedInterfaceDefinitions) { - foreach ($this->implementedInterfaceDefinitions as $interfaceDefinition) { - if (method_exists($interfaceDefinition, 'increaseDependencyRank')) { - $dependencies[] = $interfaceDefinition; - } + foreach ($this->implementedInterfaceDefinitions as $interfaceDefinition) { + if ($interfaceDefinition instanceof self) { + $dependencies[] = $interfaceDefinition; } } @@ -699,10 +694,23 @@ public function hasMethod($methodName) } $extendsClassDefinition = $this->getExtendsClassDefinition(); - if ($extendsClassDefinition instanceof self) { + if ($extendsClassDefinition instanceof ClassDefinitionRuntime) { + try { + $extendsClassDefinition = $this->compiler->getInternalClassDefinition( + $extendsClassDefinition->getName() + ); + } catch (\ReflectionException $e) { + // Do nothing + return false; + } + } + + while ($extendsClassDefinition instanceof self) { if ($extendsClassDefinition->hasMethod($methodName)) { return true; } + + $extendsClassDefinition = $extendsClassDefinition->getExtendsClassDefinition(); } return false; @@ -868,14 +876,32 @@ public function checkInterfaceImplements(self $classDefinition, self $interfaceD { foreach ($interfaceDefinition->getMethods() as $method) { if (!$classDefinition->hasMethod($method->getName())) { - throw new CompilerException('Class '.$classDefinition->getCompleteName().' must implement a method called: "'.$method->getName().'" as requirement of interface: "'.$interfaceDefinition->getCompleteName().'"'); + throw new CompilerException( + sprintf( + 'Class %s must implement a method called: "%s" as requirement of interface: "%s"', + $classDefinition->getCompleteName(), + $method->getName(), + $interfaceDefinition->getCompleteName() + ) + ); } - if ($method->hasParameters()) { - $implementedMethod = $classDefinition->getMethod($method->getName()); - if ($implementedMethod->getNumberOfRequiredParameters() > $method->getNumberOfRequiredParameters() || $implementedMethod->getNumberOfParameters() < $method->getNumberOfParameters()) { - throw new CompilerException('Class '.$classDefinition->getCompleteName().'::'.$method->getName().'() does not have the same number of required parameters in interface: "'.$interfaceDefinition->getCompleteName().'"'); - } + if (!$method->hasParameters()) { + continue; + } + + $implementedMethod = $classDefinition->getMethod($method->getName()); + if ($implementedMethod->getNumberOfRequiredParameters() > $method->getNumberOfRequiredParameters() || + $implementedMethod->getNumberOfParameters() < $method->getNumberOfParameters() + ) { + throw new CompilerException( + sprintf( + 'Method %s::%s() does not have the same number of required parameters in interface: "%s"', + $classDefinition->getCompleteName(), + $method->getName(), + $interfaceDefinition->getCompleteName() + ) + ); } } } @@ -1008,12 +1034,12 @@ public function compile(CompilationContext $compilationContext) */ $compilationContext->classDefinition = $this; - /** + /* * Get the global codePrinter. */ $codePrinter = $compilationContext->codePrinter; - /** + /* * The ZEPHIR_INIT_CLASS defines properties and constants exported by the class. */ $initClassName = $this->getCNamespace().'_'.$this->getName(); @@ -1022,7 +1048,7 @@ public function compile(CompilationContext $compilationContext) $codePrinter->increaseLevel(); - /** + /* * Method entry. */ $methods = &$this->methods; @@ -1052,7 +1078,7 @@ public function compile(CompilationContext $compilationContext) } } - /** + /* * Register the class with extends + interfaces. */ $classExtendsDefinition = null; @@ -1082,8 +1108,6 @@ public function compile(CompilationContext $compilationContext) /* * Compile properties. - * - * @var ClassProperty */ foreach ($this->getProperties() as $property) { $docBlock = $property->getDocBlock(); @@ -1102,8 +1126,6 @@ public function compile(CompilationContext $compilationContext) /* * Compile constants. - * - * @var ClassConstant */ foreach ($this->getConstants() as $constant) { $docBlock = $constant->getDocBlock(); @@ -1142,9 +1164,25 @@ public function compile(CompilationContext $compilationContext) if (!$classEntry) { if ($compiler->isClass($interface)) { - throw new CompilerException('Cannot locate interface '.$interface.' when implementing interfaces on '.$this->getCompleteName().'. '.$interface.' is currently a class', $this->originalNode); + throw new CompilerException( + sprintf( + 'Cannot locate interface %s when implementing interfaces on %s. '. + '%s is currently a class', + $interface, + $this->getCompleteName(), + $interface + ), + $this->originalNode + ); } else { - throw new CompilerException('Cannot locate interface '.$interface.' when implementing interfaces on '.$this->getCompleteName(), $this->originalNode); + throw new CompilerException( + sprintf( + 'Cannot locate interface %s when implementing interfaces on %s', + $interface, + $this->getCompleteName() + ), + $this->originalNode + ); } } diff --git a/Library/Compiler.php b/Library/Compiler.php index 106b56d9eb..83af1e6a2b 100644 --- a/Library/Compiler.php +++ b/Library/Compiler.php @@ -16,6 +16,7 @@ use Zephir\Compiler\CompilerFileFactory; use Zephir\Exception\CompilerException; use Zephir\Exception\IllegalStateException; +use Zephir\Exception\InvalidArgumentException; use Zephir\Exception\NotImplementedException; use Zephir\Exception\ParseException; use Zephir\Exception\RuntimeException; @@ -58,7 +59,7 @@ final class Compiler protected $externalDependencies = []; - /** @var \ReflectionClass[] */ + /** @var ClassDefinition[] */ protected static $internalDefinitions = []; protected static $loadedPrototypes = false; @@ -193,10 +194,11 @@ public function addFunction(FunctionDefinition $func, $statement = null) * @param string $className * @param string $location * + * @return bool + * * @throws CompilerException + * @throws IllegalStateException * @throws ParseException - * - * @return bool */ public function loadExternalClass($className, $location) { @@ -218,6 +220,7 @@ public function loadExternalClass($className, $location) return false; } + /** @var CompilerFile|CompilerFileAnonymous $compilerFile */ $compilerFile = $this->compilerFileFactory->create($className, $filePath); $compilerFile->setIsExternal(true); $compilerFile->preCompile($this); @@ -265,6 +268,10 @@ public function isClass($className) * @param string $className * * @return bool + * + * @throws CompilerException + * @throws IllegalStateException + * @throws ParseException */ public function isInterface($className) { @@ -350,6 +357,8 @@ public function addClassDefinition(CompilerFileAnonymous $file, ClassDefinition * @param string $className * * @return ClassDefinition + * + * @throws \ReflectionException */ public function getInternalClassDefinition($className) { @@ -502,18 +511,21 @@ public function preCompileHeaders() * * @param bool $fromGenerate * - * @throws Exception - * * @return bool + * + * @throws CompilerException + * @throws Exception + * @throws IllegalStateException + * @throws InvalidArgumentException */ public function generate($fromGenerate = false) { - /** + /* * Get global namespace. */ $namespace = $this->checkDirectory(); - /** + /* * Check whether there are external dependencies. */ $externalDependencies = $this->config->get('external-dependencies'); @@ -548,7 +560,7 @@ public function generate($fromGenerate = false) $compileFile->checkDependencies($this); } - /** + /* * Sort the files by dependency ranking. */ $files = []; @@ -566,7 +578,7 @@ public function generate($fromGenerate = false) } $this->files = $files; - /** + /* * Convert C-constants into PHP constants. */ $constantsSources = $this->config->get('constants-sources'); @@ -574,7 +586,7 @@ public function generate($fromGenerate = false) $this->loadConstantsSources($constantsSources); } - /** + /* * Set extension globals. */ $globals = $this->config->get('globals'); @@ -610,8 +622,6 @@ public function generate($fromGenerate = false) if (is_dir($prototypesPath) && is_readable($prototypesPath)) { /* * Load additional extension prototypes. - * - * @var \DirectoryIterator */ foreach (new \DirectoryIterator($prototypesPath) as $file) { if ($file->isDir() || $file->isDot()) { @@ -640,10 +650,12 @@ public function generate($fromGenerate = false) if (!\extension_loaded($prototype)) { $prototypeRealpath = realpath($prototypeDir); if ($prototypeRealpath) { - foreach (new \DirectoryIterator($prototypeRealpath) as $file) { - if (!$file->isDir()) { - require $file->getRealPath(); + foreach (new \RecursiveDirectoryIterator($prototypeRealpath) as $file) { + if ($file->isDir()) { + continue; } + + require $file->getRealPath(); } } } @@ -1912,9 +1924,7 @@ public function generatePackageDependenciesM4($contentM4) * * @param string $filePath * - * @throws CompilerException * @throws IllegalStateException - * @throws ParseException */ protected function preCompile($filePath) { @@ -1943,26 +1953,31 @@ protected function preCompile($filePath) * * @param string $path * - * @throws CompilerException + * @throws IllegalStateException + * @throws InvalidArgumentException */ protected function recursivePreCompile($path) { - if (!\is_string($path)) { - throw new CompilerException('Invalid compilation path'.var_export($path, true)); + if (!\is_dir($path)) { + throw new InvalidArgumentException( + sprintf( + "An invalid path was passed to the compiler. Unable to obtain the '%s%s%s' directory.", + getcwd(), + \DIRECTORY_SEPARATOR, + $path + ) + ); } - /** + /* * Pre compile all files. */ - $files = []; $iterator = new \RecursiveIteratorIterator( new \RecursiveDirectoryIterator($path), \RecursiveIteratorIterator::SELF_FIRST ); - /* - * @var \SplFileInfo - */ + $files = []; foreach ($iterator as $item) { if (!$item->isDir()) { $files[] = $item->getPathname(); diff --git a/Library/CompilerFile.php b/Library/CompilerFile.php index 678801b713..84ea873c10 100644 --- a/Library/CompilerFile.php +++ b/Library/CompilerFile.php @@ -457,8 +457,9 @@ public function preCompileClass(CompilationContext $compilationContext, $namespa * * @param Compiler $compiler * - * @throws ParseException * @throws CompilerException + * @throws IllegalStateException + * @throws ParseException */ public function preCompile(Compiler $compiler) { @@ -698,6 +699,7 @@ public function checkDependencies(Compiler $compiler) } } } else { + // Is $extendedClass is part of the compiled extension if ($compiler->isInterface($extendedClass)) { $extendedDefinition = $compiler->getClassDefinition($extendedClass); $classDefinition->setExtendsClassDefinition($extendedDefinition); diff --git a/unit-tests/Zephir/Test/ProtoDirTest.php b/unit-tests/Zephir/Test/ProtoDirTest.php new file mode 100644 index 0000000000..84a5886483 --- /dev/null +++ b/unit-tests/Zephir/Test/ProtoDirTest.php @@ -0,0 +1,86 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Zephir\Test; + +use Zephir\Compiler; +use Zephir\FileSystem\FileSystemInterface; +use function Zephir\unlink_recursive; + +class ProtoDirTest extends KernelTestCase +{ + /** + * Common directory. + * + * @var string + */ + private $pwd; + + /** + * Store the current directory before to be change. + */ + public function setUp() + { + $this->pwd = getcwd(); + } + + /** + * Restore current directory, and clean config.json. + */ + public function tearDown() + { + if (getcwd() !== $this->pwd) { + $dotZephir = \dirname(\dirname(self::$kernel->getCacheDir())); + if (file_exists($dotZephir)) { + unlink_recursive($dotZephir); + } + + if (file_exists(getcwd().'/ext')) { + unlink_recursive(getcwd().'/ext'); + } + + chdir($this->pwd); + } + } + + /** + * @test + * @issue https://github.com/phalcon/zephir/issues/1758 + */ + public function shouldGenerateValidCodeWithInheritanceOfPrototypeInterfaces() + { + $this->generate('ZendEngine3'); + + $this->assertSame( + implode(PHP_EOL, file('connectionexception.h', FILE_IGNORE_NEW_LINES)), + implode(PHP_EOL, file('ext/protodir/connectionexception.zep.h', FILE_IGNORE_NEW_LINES)), + 'Failed asserting that connectionexception.h and ext/protodir/connectionexception.zep.h are identical.' + ); + } + + protected function generate($backend) + { + chdir(\constant('ZEPHIRPATH').'/unit-tests/fixtures/protodir'); + putenv('ZEPHIR_BACKEND='.$backend); + + self::bootKernel(); + + $container = self::$kernel->getContainer(); + + $compilerFs = $container->get(FileSystemInterface::class); + $compilerFs->setBasePath(self::$kernel->getCacheDir()); + + $this->muteOutput($container); + + $compiler = $container->get(Compiler::class); + $compiler->generate(true); + } +} diff --git a/unit-tests/fixtures/protodir/config.json b/unit-tests/fixtures/protodir/config.json new file mode 100644 index 0000000000..3ea2ca4dbc --- /dev/null +++ b/unit-tests/fixtures/protodir/config.json @@ -0,0 +1,53 @@ +{ + "namespace": "protodir", + "name": "protodir", + "description": "", + "author": "", + "version": "0.0.1", + "verbose": false, + "requires": { + "extensions": [] + }, + "stubs": { + "stubs-run-after-generate": false + }, + "warnings": { + "unused-variable": false, + "unused-variable-external": false, + "possible-wrong-parameter": false, + "possible-wrong-parameter-undefined": false, + "nonexistent-function": false, + "nonexistent-class": false, + "non-valid-isset": false, + "non-array-update": false, + "non-valid-objectupdate": false, + "non-valid-fetch": false, + "invalid-array-index": false, + "non-array-append": false, + "invalid-return-type": false, + "unreachable-code": false, + "nonexistent-constant": false, + "not-supported-magic-constant": false, + "non-valid-decrement": false, + "non-valid-increment": false, + "non-valid-clone": false, + "non-valid-new": false, + "non-array-access": false, + "invalid-reference": false, + "invalid-typeof-comparison": false, + "conditional-initialization": false + }, + "optimizations": { + "static-type-inference": true, + "static-type-inference-second-pass": true, + "local-context-pass": true, + "constant-folding": true, + "static-constant-class-folding": true, + "call-gatherer-pass": true, + "check-invalid-reads": false, + "internal-call-transformation": false + }, + "prototype-dir": { + "fake": "prototypes/fake" + } +} diff --git a/unit-tests/fixtures/protodir/connectionexception.h b/unit-tests/fixtures/protodir/connectionexception.h new file mode 100644 index 0000000000..74707df2f5 --- /dev/null +++ b/unit-tests/fixtures/protodir/connectionexception.h @@ -0,0 +1,11 @@ + +extern zend_class_entry *protodir_connectionexception_ce; + +ZEPHIR_INIT_CLASS(ProtoDir_ConnectionException); + +PHP_METHOD(ProtoDir_ConnectionException, getRequest); + +ZEPHIR_INIT_FUNCS(protodir_connectionexception_method_entry) { + PHP_ME(ProtoDir_ConnectionException, getRequest, NULL, ZEND_ACC_PUBLIC) + PHP_FE_END +}; diff --git a/unit-tests/fixtures/protodir/protodir/ConnectionException.zep b/unit-tests/fixtures/protodir/protodir/ConnectionException.zep new file mode 100644 index 0000000000..085dfc8de0 --- /dev/null +++ b/unit-tests/fixtures/protodir/protodir/ConnectionException.zep @@ -0,0 +1,10 @@ +namespace ProtoDir; + +use Fake\Http\Client\NetworkExceptionInterface; + +class ConnectionException extends \RuntimeException implements NetworkExceptionInterface +{ + public function getRequest() + { + } +} diff --git a/unit-tests/fixtures/protodir/prototypes/fake/ClientExceptionInterface.php b/unit-tests/fixtures/protodir/prototypes/fake/ClientExceptionInterface.php new file mode 100644 index 0000000000..1b5b5b10e5 --- /dev/null +++ b/unit-tests/fixtures/protodir/prototypes/fake/ClientExceptionInterface.php @@ -0,0 +1,7 @@ +