Add support for env processor

[VincentLanglet]

Close #199

[ondrejmirtes]

Isn't basically the same switch that's casting those values somewhere in Symfony that we could call?

[VincentLanglet]

There is this array: https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/DependencyInjection/EnvVarProcessor.php#L39 that I can use.

But I would end with a type like string, bool or even 'bool|int|float|string|array'.

I assumed there was something to transform string to StringType and 'bool|int|float|string|array' to the UnionType and you could help me about it ?

[VincentLanglet]

The only method I found which seems to transform something like string to the StringType is the method TypeNodeResolver::resolve, but it would require to pass a TypeNode and a NameScope... In order to have a TypeNode I think it would require me to parse some token and I don't know what to do for the NameScope.

That seems way over complicated. I dunno if you have a simpler solution ? @ondrejmirtes

[ondrejmirtes]

You need to ask for TypeStringResolver through constructor and use that: https://github.com/phpstan/phpstan-src/blob/master/src/PhpDoc/TypeStringResolver.php

[VincentLanglet]

It seems to work, but I end up with

Calling PHPStan\PhpDoc\TypeStringResolver::resolve() is not covered    
         by backward compatibility promise. The method might change in a minor  
         PHPStan version.

Should I add the error to the baseline ?

[ondrejmirtes]

No: phpstan/phpstan-src@2b3ee60

[VincentLanglet]

Thanks ! It works now perfectly ; thanks again for the help :)

[VincentLanglet]

Friendly ping @ondrejmirtes ; is this PR ok for you ? :)

[ondrejmirtes]

@VincentLanglet Sorry, I'm busy with PHPStan 1.0, I'll have a look at this after the release.

[VincentLanglet]

@VincentLanglet Sorry, I'm busy with PHPStan 1.0, I'll have a look at this after the release.

No problem. Well done for the 1.0 release.
I'm currently using 0.12 with my fork of the plugin to use this feature and the phpstan/phpstan-doctrine#218 one. But I'll be happy to try the new 1.0 version.

If you have some time to take a look =)

Thanks

[ondrejmirtes]

@VincentLanglet My top priority right now is PHP 8.1 support (https://phpstan.org/blog/plan-to-support-php-8-1). The main action is now happening in https://github.com/ondrejmirtes/BetterReflection/tree/ng and https://github.com/phpstan/phpstan-src/tree/ng-better-reflection. Once this is finished and released, the operations will return to normal eventually. I now have 187 emails in my inbox worthy of my attention (all related to PHPStan), so please stay tuned.

[VincentLanglet]

Friendly ping @ondrejmirtes,

Happy new year, I know you were pretty busy with the support of php 8.1. I might be wrong but I understood you now finished the support. How long is your todo list now before having some time to take a new look to this PR and phpstan/phpstan-doctrine#218 ?

Thanks a lot for all the work you do.

[ondrejmirtes]

Support for PHP 8.1 is not finished. I now have 213 unread emails in my inbox worth of my attention + still a lot of work to be done on PHP 8.1, like readonly properties, and updated php-8-stubs.

You don't need to ping me, when I get to it, I'll get to it.

[lookyman]

This introduces a hard dependency on symfony/dependency-injection in composer.json. I get the idea about not duplicating Symfony's internal code here, but if we want to do it like that, we need to use something like call_user_func(['\Symfony\Component\DependencyInjection\EnvVarProcessor', 'getProvidedTypes']), and probably even wrap it in some class_exists. Might be easier just to copy that array.

[VincentLanglet]

I got the opposite review here: #200 (comment)

So I'll go with the class_exists solution.

Does

if (
			is_string($value)
			&& preg_match('/%env\((.*)\:.*\)%/U', $value, $matches) === 1
			&& strlen($matches[0]) === strlen($value)
			&& class_exists(EnvVarProcessor::class)
		) {
			$providedTypes = EnvVarProcessor::getProvidedTypes();

			return $this->typeStringResolver->resolve($providedTypes[$matches[1]] ?? 'bool|int|float|string|array');
		}

wouldn't be enough ?

[VincentLanglet]

Support for PHP 8.1 is not finished. I now have 213 unread emails in my inbox worth of my attention + still a lot of work to be done on PHP 8.1, like readonly properties, and updated php-8-stubs.

Hi, I saw this repository got some recent activity back and phpstan 1.4 is released with readonly property support. I'd like to avoid having to update my fork on every new release. Is there something I can do to help this PR to move forward @ondrejmirtes ? Do you need help on something else ?

[ondrejmirtes]

@lookyman has to give a 👍 and I’ll be happy to merge this.

[lookyman]

I will take a final look later this week. I'm sorry, I am currently swamped with other stuff.

[Wirone]

@VincentLanglet I have this errors and I found your MR - thank you very much! Before it's merged I would like to ask if it'll support such scenarios like here:

parameters:
  some.bool: '%env(bool:FOO)%'
  is.still.bool: '%some.bool%'

services:
  Foo\Bar:
    public: true
    arguments:
      $isBool: '%some.bool%'

• will $container->getParameter('is.still.bool') be considered as bool?
• will $isBool in Foo\Bar's constructor be considered as bool?

In general: will type resolved by EnvVarProcessor be propagated to "nested" parameters? We have large legacy app and I'm working on introducing this extension but our DI definition and container usage is... not optimal 😉

[VincentLanglet]

Before it's merged I would like to ask if it'll support such scenarios like here:

It's not a blocker to be merged.

• will $container->getParameter('is.still.bool') be considered as bool?

I would say no, since phpstan is not fully resolving the config.
But you're free to try the fork/or add more tests here https://github.com/phpstan/phpstan-symfony/pull/200/files#diff-1727317d6da5fe452f28690571b9764589dda96fa1f72c31e834a6cfe6c1f591
Also I would recommend to change to $container->getParameter('some.bool')

• will $isBool in Foo\Bar's constructor be considered as bool?

It's irrelevant since phpstan is not validating yaml services configs.

[Wirone]

Of course it's not a blocker, I just wanted to take the opportunity to ask before it's finished 🙂 Thanks for quick answer! Looking forward to get this merged but in the meantime I'll try to use fork and check it out.

[lookyman]

@ondrejmirtes Let's merge this!

[ondrejmirtes]

Thank you!

[Wirone]

I've installed 1.1.4 and just wanted to inform you that all places that previously was reported as errors (where parameters were accessed with $container->getParameter()) now are OK 🥳 Could remove one inline ignore and 2 "fixes" that I did before. Thank you very much once again @VincentLanglet 🙂