Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null objects do not extend their original class #300

Closed
trickleup opened this issue Mar 13, 2016 · 5 comments
Closed

Null objects do not extend their original class #300

trickleup opened this issue Mar 13, 2016 · 5 comments
Assignees
Labels
Milestone

Comments

@trickleup
Copy link

It seems to me that classes with PHP7 return types fail.

PHP Fatal error:  Declaration of ProxyManagerGeneratedProxy\__PM__\MyClass::queryUsers() must be compatible with MyClass::queryUsers(): array in /Users/johan/Projects/sparkcentral/auth-context/vendor/ocramius/proxy-manager/src/ProxyManager/GeneratorStrategy/EvaluatingGeneratorStrategy.php(63) : eval()'d code on line 3

Is there a workaround/fix for this issue?

@trickleup
Copy link
Author

Also, the same seems to be the case with scalar type hints in method declarations.

@Ocramius
Copy link
Owner

@trickleup ProxyManager v2 fully supports return type hints: can you check what version you have installed?

@Ocramius Ocramius added the bug label Mar 13, 2016
@michaelmoussa
Copy link
Contributor

@Ocramius I'm having (what I think is) the same issue, only with the Null Object Proxy. See example case below:

$ cat test.php
<?php

declare (strict_types = 1);

require __DIR__ . '/vendor/autoload.php';

echo \ProxyManager\Version::getVersion() . "\n";

class Foo
{
}

function bar() : Foo
{
    return (new \ProxyManager\Factory\NullObjectFactory())->createProxy(Foo::class);
}

bar();
$ php test.php
2.0.1@6c89b7bd6039d8047b1473e2074cb56baa4bc15d
PHP Fatal error:  Uncaught TypeError: Return value of bar() must be an instance of Foo, instance of ProxyManagerGeneratedProxy\__PM__\Foo\Generatede07f73e523c2fb4772e096fa7615d794 returned in /Users/mmoussa/Code/pm-test/test.php:15

The generated code looks like this:

namespace ProxyManagerGeneratedProxy\__PM__\Foo;

class Generatede07f73e523c2fb4772e096fa7615d794 implements \ProxyManager\Proxy\NullObjectInterface
{

    private static $signaturee07f73e523c2fb4772e096fa7615d794 = 'YTozOntzOjk6ImNsYXNzTmFtZSI7czozOiJGb28iO3M6NzoiZmFjdG9yeSI7czozODoiUHJveHlNYW5hZ2VyXEZhY3RvcnlcTnVsbE9iamVjdEZhY3RvcnkiO3M6MTk6InByb3h5TWFuYWdlclZlcnNpb24iO3M6NDY6IjIuMC4xQDZjODliN2JkNjAzOWQ4MDQ3YjE0NzNlMjA3NGNiNTZiYWE0YmMxNWQiO30=';

    /**
     * Constructor for null object initialization
     */
    public static function staticProxyConstructor()
    {
        static $reflection;

        $reflection = $reflection ?: $reflection = new \ReflectionClass(__CLASS__);
        $instance = (new \ReflectionClass(get_class()))->newInstanceWithoutConstructor();

        return $instance;
    }
}

Since Generatede07f73e523c2fb4772e096fa7615d794 doesn't extend Foo, it will obviously fail the return type check.

If I add this: $classGenerator->setExtendedClass($originalClass->getName()); after this line of code, then my test case works, but then a few of the datasets cause this test to fail.

Am I on the right track at all? Or does the above give you enough information to resolve this fairly quickly? I'm not terribly familiar with ProxyManager's internals, so I'd rather check with you before digging any deeper.

@Ocramius
Copy link
Owner

I suggest writing a functional test case first: see tests/language-feature-scripts

Also, please remember to use absolute references for files, as the branches keep moving, and the diffs change (and your links become invalid)

EDIT: I updated your links

@Ocramius
Copy link
Owner

Corrected via #301. Rewording title though, as the return types were actually respected, but the inheritance wasn't.

@Ocramius Ocramius changed the title PHP7 return type compatibility Null objects do not extend their original class May 27, 2016
@Ocramius Ocramius added this to the 2.0.2 milestone May 27, 2016
@Ocramius Ocramius self-assigned this May 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants