nette / php-generator

🐘 Generates neat PHP code for you. Supports new PHP 8.3 features.
https://doc.nette.org/php-generator
Other
2.11k stars 138 forks source link

Helpers::createObject() creates public property #1

Closed vysinsky closed 10 years ago

vysinsky commented 10 years ago

Hi. Take this code:

class SomeObject
{
    private $someProperty;

    function __construct($someProperty)
    {
        $this->someProperty = $someProperty;
    }
}

var_dump(\Nette\PhpGenerator\Helpers::createObject('SomeObject', ['someProperty' => 'test']));

Expected result:

object(SomeObject)[2]
  private 'someProperty' => string 'test' (length=4)

Actual result:

object(SomeObject)[2]
  private 'someProperty' => null
  public 'someProperty' => string 'test' (length=4)

Its on master. I've found this bug when I wanted create service in DI extension and used:

$builder->addDefinition($this->prefix('someService'))
    ->setClass('SomeService', ['someProperty' => $value]);
mishak87 commented 10 years ago

AFAIK this is expected behaviour. If you had inherited Nette/Object it would scream sooner (undefined property). Private properties are defined differently then public. See the note for serialize function. "\x00$class\x00$property" private and "\x00*\x00$property" protected.

vysinsky commented 10 years ago

No if I extended Nette/Object it did the same. Even if I use setter and 'addSetup' method on definition it does the same.

How can I create a service class with private property set only by constructor? I think its very common use case in Nette Framework.

milo commented 10 years ago

How can I create a service class with private property set only by constructor?

@vysinsky Pass array as a list, without keys. ->setClass('SomeService', ['arg1', 'arg2']) Example

hrach commented 10 years ago

imo it' a bug, ->setClass on DefinitionBuilder should add array_values call.

fprochazka commented 10 years ago

@hrach no it should not. In the earlier versions, it paired to the right constructor argument, if it got broke, it should be fixed.

vysinsky commented 10 years ago

@milo I've used array without keys now. It makes new public property named by numeric array key - it means 0, 1 etc.

milo commented 10 years ago

@vysinsky I'm confused what about this issue is.

You are writing about Nette\PhpGenerator\Helpers::createObject('SomeObject', ['someProperty' => 'test']) behaviour. It is a discutable bug.

After that, you are talking about: "Creating service in a DI compiler extension". When I use code:

class C {
    private $p;

    public function __construct($p) {
        $this->p = $p;
    }
}

class TestExtension extends Nette\DI\CompilerExtension {
    public function loadConfiguration() {
        $builder = $this->getContainerBuilder();
        $builder->addDefinition($this->prefix('someService'))
            ->setClass('C', ['p' => 'value']);
    }
}

in compiled container I get a code:

    /**
     * @return C
     */
    public function createServiceTest__someService()
    {
        $service = new C('value');
        return $service;
    } 

which works well, the PhpGenerator is not used.

vysinsky commented 10 years ago

@milo I am using this in extension:

$builder->addDefinition($this->prefix('modulesFactory'))
            ->setClass(ModulesFactory::CLASSNAME, ['modules' => $modules])
            ->addTag(ModulesExtension::TAG_MODULES_FACTORY);

$modules is array of objects. In generated container code I have this:

/**
     * @return Annotatecms\Crud\ModulesFactory
     */
    public function createServiceCrud__modulesFactory()
    {
        $service = new Annotatecms\Crud\ModulesFactory(array(
            'pages' => Nette\PhpGenerator\Helpers::createObject('AnnotateCms\Modules\ModuleDefinition', array(
                'class' => 'AnnotateCms\\Crud\\CrudModule',
                'url' => 'pages',
                'icon' => NULL,
                'type' => NULL,
                'extra' => array(
                    'entity' => 'App\\Model\\Entities\\Page',
                ),
            )),
            'users' => Nette\PhpGenerator\Helpers::createObject('AnnotateCms\Modules\ModuleDefinition', array(
                'class' => 'AnnotateCms\\Crud\\CrudModule',
                'url' => 'users',
                'icon' => NULL,
                'type' => NULL,
                'extra' => array(
                    'entity' => 'App\\Model\\Entities\\User',
                ),
            )),
        ));
        return $service;
    }

So there is createObject method used. Why does it use createObject method when passing array of already created objects?

I think this issue is still about createObject method behaviour, because it gives weird output.

fprochazka commented 10 years ago

Why would you pass objects to have them serialized and deserialized? Pass array of Statement instances.

$modules = [
    new Nette\DI\Statement('AnnotateCms\Modules\ModuleDefinition', array(
        'AnnotateCms\\Crud\\CrudModule',
        array('App\\Model\\Entities\\Page'),
    )),
    new Nette\DI\Statement('AnnotateCms\Modules\ModuleDefinition', array(
        'AnnotateCms\\Crud\\CrudModule',
        array('App\\Model\\Entities\\User')
    )),
];

$builder->addDefinition($this->prefix('modulesFactory'))
            ->setClass(ModulesFactory::CLASSNAME, ['modules' => $modules])
            ->addTag(ModulesExtension::TAG_MODULES_FACTORY);

this should generate you something like this

new Annotatecms\Crud\ModulesFactory(array(
    new AnnotateCms\Modules\ModuleDefinition('AnnotateCms\\Crud\\CrudModule', array(
        'App\\Model\\Entities\\Page'
    )),
    new AnnotateCms\Modules\ModuleDefinition('AnnotateCms\\Crud\\CrudModule', array(
        'App\\Model\\Entities\\User'
    )),
))

Isn't that much better?

vysinsky commented 10 years ago

@fprochazka yes! It is much better. I did not know about that. Still some things to learn after years using Nette :) Thanks

Regarding createObject method: is it really expected behaviour?

fprochazka commented 10 years ago

@vysinsky I'm pretty sure it's a bug.

milo commented 10 years ago

Imo it is not a bug of createObject() method but an issue of _dump() method here.

The output is tested, I don't understand why is the visibility dropped out.

@dg Has this some reason or is it a unintended behaviour?