Dynamic parameter does not work in static parameter

[ondrejmirtes]

Version: 3.1.2

Bug Description

The documentation says (https://doc.nette.org/en/application/bootstrap#toc-dynamic-parameters):

We can also add dynamic parameters to the container, their different values, unlike static parameters, will not cause the generation of new DI containers. Environment variables could be easily made available using dynamic parameters. We can access them via %env.variable% in configuration files.

When I try this in practice, the parameter value is null instead of the expected value of expanded env variable.

Steps To Reproduce

I created a minimal project that shows this problem: https://github.com/ondrejmirtes/nette-di-dynamic

The script test.php:

<?php

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

$configurator = new Nette\Bootstrap\Configurator;
$configurator->setTempDirectory(__DIR__ . '/temp');
$configurator->addConfig(__DIR__ . '/test.neon');
$configurator->addDynamicParameters([
    'env' => getenv(),
]);
$container = $configurator->createContainer();

var_dump($container->parameters['test']);

The neon file test.neon:

parameters:
    test: %env.NETTE_TEST%/foo

How to run:

export NETTE_TEST=foo
php test.php

Expected Behavior

string(7) "foo/foo"

Actual Behaviour

NULL

[mabar]

You can reference dynamic parameters anywhere except static parameters, the same way static parameters are referenced. What you are trying to do cannot work (and should throw an exception?)

[ondrejmirtes]

In theory it should work, it's just that the compiled DIC needs to look different.

[mabar]

I see your point, you just confused me with the title. Reference to a dynamic parameter in another parameter may work, esentially making the referecing parameter dynamic instead of being static

[dg]

I'm looking at it and it's working properly. The documentation doesn't say that the values of dynamic variables are found in the $container->parameters array. This is an internal variable that is public only for historical reasons. But it might be possible to add a getParameter() method to make dynamic parameters available.

[ondrejmirtes]

I did a few more tests: https://github.com/ondrejmirtes/nette-di-dynamic/commit/475447bfa25f0ff477e749ab5ad74e2de680658d

The result of the script is now:

NULL
string(7) "foo/foo"
string(7) "foo/foo"

So when used as a service argument, it works as expected. The compiled container looks like this:

($this->parameters['env']['NETTE_TEST']) . '/foo'

It would be awesome if the parameters array could look like this too.

[dg]

In other words, you want that instead of lazy evaluation, there should be immediately evaluation at the beginning. That might work, of course, but it's a trade-off.

[dg]

I'm thinking there might be a compromise. Parameters like %env.NETTE_TEST%/foo can be resolved right away. Parameters that call methods like @service::foo() can continue to resolve lazy.

I tried implement it in 3.2-dev

[ondrejmirtes]

Looks like 3.2-dev includes a lot of BC breaks (removes current features) which it should not so unfortunately I can't test it: https://github.com/nette/di/commits/v3.2

Would be great if 3.2 still supported PHP 7.2+ which is what PHPStan supports. Although I can work around that if needed.

[dg]

It could probably be put into version 3.1.

Btw what problematic BC breaks did you encounter?

[ondrejmirtes]

1) Minor versions should never contain any BC breaks. 2) nette/di is used by PHPStan users through custom .neon files so any changes to supported NEON syntax can break PHPStan for someone. I think that's the case of this commit https://github.com/nette/di/commit/3b3c1adff31093b008d10df8fae172e8603a0345 This is also a BC break for generated factories from what I understand https://github.com/nette/di/commit/5c36951b3c115039918bcbe16868367032adae64.

When upgrading to nette/di 3.1.3 I also had to silence E_USER_DEPRECATED with this commit https://github.com/phpstan/phpstan-src/commit/c43ac096f511c922af22cf6024208ca611d30a22 when creating the DI container because I don't want to bother PHPStan users with these deprecations: https://github.com/phpstan/phpstan-src/commit/c43ac096f511c922af22cf6024208ca611d30a22 (in the future I'd probably transform the neon array on user's behalf before passing it to nette/di).

[dg]

If you encounter any problems, you'd better write me and not mute E_USER_DEPRECATED.

I don't consider it as a BC break to remove things that have been throwing out E_USER_DEPRECATED since the previous version, two years old. But if you don't play the game, of course it can be a problem.

[ondrejmirtes]

Yeah, personally I wouldn't use any deprecated things in my own applications and fix them immediately, but PHPStan exposes nette/di to its users through the neon config format, and I would have to provide some easy migration path so that my users are not bothered with these deprecations.

[dg]

And which ones in particular did they encounter, do you remember?

[ondrejmirtes]

@dg I haven't released a version that wouldn't silence E_USER_DEPRECATED from nette/di so I didn't really get that feedback, but I think the most common one is the deprecated class key for services, as can be seen very frequently in 3rd party packages like https://github.com/shipmonk-rnd/phpstan-rules/blob/master/rules.neon.

[ondrejmirtes]

Thank you! I'm gonna try it right away.

[dg]

Hmm, that's strange, class should work normally...

[ondrejmirtes]

%env% in parameters now works as expected! 🎉 Thank you very much!

As for class in services: If I un-silence E_USER_DEPRECATED, I get many of these errors:

PHP Deprecated:  Service '9': option 'class' should be changed to 'type'. in vendor/nette/di/src/DI/Extensions/DefinitionSchema.php on line 130

It still works, but I don't want to put this burden on PHPStan users.

[dg]

How is it used?

- class:: ClassName                # should work
- class:: ClassName(arg, arg)      # historically this has worked, by mistake

[ondrejmirtes]

I just debugged it and entries like this are reported as deprecated:

    -
        class: PHPStan\Php\PhpVersion
        factory: @PHPStan\Php\PhpVersionFactory::create

    -
        class: PHPStan\Php\PhpVersionFactory
        factory: @PHPStan\Php\PhpVersionFactoryFactory::create

Which is okay but I still don't want to force PHPStan users to rewrite them :)

[dg]

I'll try to find a solution...

[mabar]

@ondrejmirtes Wouldn't be a way to resolve this in the long run to allow deprecation notifications to be turned on? So that PHPStan users would not be bothered, but the extension developers could make the necessary changes. I believe most users don't use any things from di or neon that may change.

[ondrejmirtes]

TBH in the long run I want a different PHPStan-specific config format, probably based on objects that work with IDE autocompletion and can be validated with PHPStan analysis 😊

[dg]

@ondrejmirtes see 8fd9b4a885017cd33ba6e3fad0b74ac4650c9b09

[dg]

It will work a little differently, instead of $container->parameters['test'] will work $container->getParameter('test').

[kipanshi]

This new patch version 3.1.4 broke codeception tests in our app based on REMP CRM, broke environment config:

database:
    default:
        dsn: @environmentConfig::getDsn()
        user: @environmentConfig::get('CRM_DB_USER')
        password: @environmentConfig::get('CRM_DB_PASS')
        options:
            lazy: yes

And getting error [Exception] DB prepare error in command: 'phinx:migrate' | | Nette\DI\InvalidConfigurationException: The item 'database › default › dsn' expects to be Nette\Schema\DynamicParameter or string, object Nette\DI\Definitions\Statement given. in /var/www/crm/vendor/nette/di/src/DI/Compiler.php:293

Worked fine in the version 3.1.3.

[kipanshi]

I think it's best to open a new issue for it.

[dg]

@kipanshi fixed