do not search for service_container references

[xabbuh]

This fixes #36.

[matthiasnoback]

Excellent work as always :)

[craigh]

this change is now breaking my Symfony-2.8.x application. on composer-update I get these errors:

 [RuntimeException]                                                                                                                                 
  An error occurred when executing the "'cache:clear --no-warmup'" command:                                                                          
  Fatal error: Uncaught exception 'Matthias\SymfonyServiceDefinitionValidator\Exception\InvalidServiceDefinitionsException' with message 'Service d  
  efinition validation errors (3):                                                                                                                   
  - zikula_core.internal.doctrine_init: Argument for type-hint "Symfony\Component\DependencyInjection\ContainerBuilder" points to a service of clas  
  s "Symfony\Component\DependencyInjection\Container"                                                                                                
  - hook_dispatcher.servicefactory: Argument for type-hint "Symfony\Component\DependencyInjection\ContainerBuilder" points to a service of class "S  
  ymfony\Component\DependencyInjection\Container"                                                                                                    
  - doctrine_extensions: Argument for type-hint "Zikula\Bridge\DependencyInjection\ContainerBuilder" points to a service of class "Symfony\Componen  
  t\DependencyInjection\Container"' in /Applications/MAMP/htdocs/core.git/src/vendor/matthiasnoback/symfony-service-definition-validator/Compiler/V  
  alidateServiceDefinitionsPass.php:44                                                                                                               
  Stack trace:                                                                                                                                       
  #0 /Applications/MAMP/htdocs/core.git/src/vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/Compiler/Compiler.php( in /Application  
  s/MAMP/htdocs/core.git/src/vendor/matthiasnoback/symfony-service-definition-validator/Compiler/ValidateServiceDefinitionsPass.php on line 44       
  .

[craigh]

reverting to 1.2.6 removes the errors and the project builds properly.

[matthiasnoback]

@xabbuh Any idea how to fix this? The type of `service_container` might have to be changed to `ContainerInterface`. Not sure if that works though! On Feb 9, 2017 03:37, "Craig Heydenburg" <notifications@github.com> wrote: reverting to 1.2.6 removes the errors and the project builds properly. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#37 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABI0djwAlp0y8HAz0F2jlopMtoz_HJ7tks5ranvggaJpZM4L3--Y> .

[xabbuh]

@matthiasnoback That won't solve the issue either. The ContainerBuilder class is an instance of the ContainerInterface. But the check is performed the other way around. That's why it's currently failing as the Container class is not an instance of ContainerBuilder.

By the way, expecting a ContainerBuilder in general is fragile. However, this PR shouldn't indeed have broken that. I will look into how to solve this.

[craigh]

I will look into how to solve this

thank you @xabbuh

[xabbuh]

@craigh Is there a test in Zikula I could execute to reproduce the issue?

[craigh]

@xabbuh sadly, no. Zikula isn't quite up to spec on proper tests yet.

you can see here: zikula/core@4d6dd49

that I restricted the version. If you revert that and run composer update on the repo, then you get the error I showed above

[sstok]

Thanks for fixing this 👍 I did not provide a pr because I wasn't sure how to fix this 😅

[xabbuh]

As far as I can see there is no way to solve the problem without removing the code that validates the argument type hint (no matter what class we used for the pseudo container definition all would fail when the service expects a ContainerBuilder instance).

@matthiasnoback I think we need to drop the type check for the special service_container to not break BC. What do you think?

[matthiasnoback]

Okay, I would agree with a hard-coded rule to ignore this particular service, though it's not pretty of course ;) Another, maybe future solution would be to raise a warning for this kind of Liskov substitution principle violation. But people wouldn't be able to fix broken external dependencies anyway so it might not be helpful at all :) Thanks for your work @xabbuh! On Feb 12, 2017 20:04, "Christian Flothmann" <notifications@github.com> wrote: As far as I can see there is no way to solve the problem without removing the code that validates the argument type hint (no matter what class we used for the pseudo container definition all would fail when the service expects a ContainerBuilder instance). @matthiasnoback <https://github.com/matthiasnoback> I think we need to drop the type check for the special service_container to not break BC. What do you think?

[xabbuh]

@matthiasnoback I agree that this solution is not pretty. But since we don't know if people actually do compile the container after it has been build, we don't know if it's safe for them to make these assumptions (like, for example, Zikula seems to always use the ContainerBuilder during runtime instead of a previously dumped one).

[matthiasnoback]

@xabbuh Okay, seems fine to me then!

[xabbuh]

@craigh It looks like even if we make the change I suggested above, you will run into the same problems due to the ContainerInterface being registered as a definition in 3.3 (see symfony/symfony#21627).

[xabbuh]

see #39