Skip to content

Conversation

@mpdude
Copy link
Contributor

@mpdude mpdude commented Nov 18, 2025

This adds namespace imports for functions from the Symfony\Component\DependencyInjection\Loader\Configurator namespace on top of the generated files.

Technically, those imports are not necessary at runtime, since the functions can be resolved correctly without.

However, it makes a difference for tools like ComposerRequireChecker that have to rely purely on static analysis (SA).

With SA alone, function imports may be ambiguous (see maglnet/ComposerRequireChecker#193 (comment)).

By adding these imports, we add more static information.

Copilot AI review requested due to automatic review settings November 18, 2025 14:41
Copilot finished reviewing on behalf of mpdude November 18, 2025 14:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds explicit use function imports for Symfony Configurator namespace functions in the generated PHP configuration files to improve static analysis tool support.

  • Adds explicit function imports for Configurator namespace functions to help static analysis tools
  • Includes imports for 9 functions: service, inline_service, service_locator, iterator, expr, abstract_arg, env, service_closure, and closure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mpdude and others added 2 commits November 18, 2025 15:46
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@GromNaN
Copy link
Owner

GromNaN commented Nov 18, 2025

Thanks.

I don't think that's in the spirit of the feature. We're in the Symfony\Component\DependencyInjection\Loader\Configurator namespace so we don't have to import classes and functions. I'll keep it open while I think about it.

mpdude added a commit to webfactory/WebfactoryShortcodeBundle that referenced this pull request Nov 24, 2025
XML-based service configuration has been deprecated since
symfony/symfony#60568 and will no longer be
supported in Symfony 8.0.

See also
https://symfony.com/blog/new-in-symfony-7-4-deprecated-xml-configuration.

In order for ComposerRequireChecker to recognise where the functions
come from, we have to import them explicitly. At the same time,
PHP-CS-Fixer must not remove the import, which is actually superfluous.

X-Ref GromNaN/symfony-config-xml-to-php#29,
maglnet/ComposerRequireChecker#193 (comment)
mpdude added a commit to webfactory/WebfactoryHttpCacheBundle that referenced this pull request Nov 29, 2025
XML-based service configuration has been deprecated since symfony/symfony#60568 and will no longer be supported in Symfony 8.0.

See also https://symfony.com/blog/new-in-symfony-7-4-deprecated-xml-configuration.

In order for ComposerRequireChecker to recognise where the functions come from, we have to import them explicitly. At the same time, PHP-CS-Fixer must not remove the import, which is actually superfluous.

X-Ref GromNaN/symfony-config-xml-to-php#29, maglnet/ComposerRequireChecker#193 (comment)
mpdude added a commit to webfactory/WebfactoryHttpCacheBundle that referenced this pull request Nov 29, 2025
XML-based service configuration has been deprecated since
symfony/symfony#60568 and will no longer be
supported in Symfony 8.0.

See also
https://symfony.com/blog/new-in-symfony-7-4-deprecated-xml-configuration.

In order for ComposerRequireChecker to recognise where the functions
come from, we have to import them explicitly. At the same time,
PHP-CS-Fixer must not remove the import, which is actually superfluous.

X-Ref GromNaN/symfony-config-xml-to-php#29,
maglnet/ComposerRequireChecker#193 (comment)
@GromNaN
Copy link
Owner

GromNaN commented Dec 2, 2025

I'm rejecting as this is not necessary nor expected.

@GromNaN GromNaN closed this Dec 2, 2025
@mpdude mpdude deleted the add-namespace-imports branch December 3, 2025 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants