Skip to content

[XML2PHP] UiBundle#18776

Merged
TheMilek merged 1 commit intoSylius:2.3from
crydotsnake:xml2php/ui
Feb 4, 2026
Merged

[XML2PHP] UiBundle#18776
TheMilek merged 1 commit intoSylius:2.3from
crydotsnake:xml2php/ui

Conversation

@crydotsnake
Copy link
Copy Markdown
Member

@crydotsnake crydotsnake commented Jan 31, 2026

Q A
Branch? 2.3
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets partially #18490
License MIT

Summary by CodeRabbit

  • Chores
    • Migrated bundle service configuration from XML to a consolidated PHP-based format; public behavior unchanged.
    • Re-registered controllers, form types, Twig extensions, error renderer and UX template integration through the new PHP configs to preserve functionality.
    • Updated dependency whitelist for requirement checks.

@crydotsnake crydotsnake requested review from a team as code owners January 31, 2026 13:26
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 31, 2026

📝 Walkthrough

Walkthrough

Replaces XML-based Symfony DI in UiBundle with PHP configurators: adds PHP service files, removes corresponding XML files, updates the bundle extension to use PhpFileLoader, and updates the composer-require-checker whitelist to include tagged_iterator.

Changes

Cohort / File(s) Summary
Dependency Validation
composer-require-checker.json
Added tagged_iterator to symbol-whitelist and reordered entries.
Bundle Extension & Root DI
src/Sylius/Bundle/UiBundle/DependencyInjection/SyliusUiExtension.php, src/Sylius/Bundle/UiBundle/Resources/config/services.php, src/Sylius/Bundle/UiBundle/Resources/config/services.xml
Switched loader from XmlFileLoaderPhpFileLoader; removed services.xml and added services.php which imports per-area PHP config files.
Controller Service
src/Sylius/Bundle/UiBundle/Resources/config/services/controller.php, src/Sylius/Bundle/UiBundle/Resources/config/services/controller.xml
Moved sylius.controller.security service registration from XML to PHP with same class, args, and public visibility; XML removed.
Form Service
src/Sylius/Bundle/UiBundle/Resources/config/services/form.php, src/Sylius/Bundle/UiBundle/Resources/config/services/form.xml
Moved sylius.form.type.security_login service to PHP and preserved form.type tag; XML removed.
Twig Extensions
src/Sylius/Bundle/UiBundle/Resources/config/services/twig.php, src/Sylius/Bundle/UiBundle/Resources/config/services/twig.xml
Migrated twig extension services (percentage, merge_recursive, redirect_path) to PHP with same tags/args; XML removed.
Twig Error Renderer
src/Sylius/Bundle/UiBundle/Resources/config/services/twig/error_renderer.php, src/Sylius/Bundle/UiBundle/Resources/config/services/twig/error_renderer.xml
Replaced XML decorator sylius.twig.error_renderer with PHP configurator preserving decoration target, priority, and arguments.
Twig UX Component
src/Sylius/Bundle/UiBundle/Resources/config/services/twig/ux.php, src/Sylius/Bundle/UiBundle/Resources/config/services/twig/ux.xml
Replaced UX component template finder decorator XML with PHP equivalent (decorates ux.twig_component.component_template_finder, same args); XML removed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

Maintenance, DX, Admin

Suggested reviewers

  • TheMilek
  • Rafikooo

Poem

🐰 I hopped from XML to tidy PHP,
Imported services, neat as can be,
Decorators snug and tags aligned,
The container hums, all wired and kind,
A rabbit cheers — configs now free.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '[XML2PHP] UiBundle' directly summarizes the main change: converting XML configuration files to PHP in the UiBundle, which is the primary focus of all modifications across multiple service configuration files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 31, 2026

❌ Preview Environment deleted from Bunnyshell

Available commands:

  • 🚀 /bns:deploy to redeploy the environment


return static function (ContainerConfigurator $container) {
$services = $container->services();
$parameters = $container->parameters();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$parameters = $container->parameters();


return static function (ContainerConfigurator $container) {
$services = $container->services();
$parameters = $container->parameters();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$parameters = $container->parameters();

Comment on lines +20 to +23
$parameters = $container->parameters();

$services->defaults()
->public();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$parameters = $container->parameters();
$services->defaults()
->public();

service('twig'),
service('security.authorization_checker'),
service('router'),
]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
]);
])
->public();


return static function (ContainerConfigurator $container) {
$services = $container->services();
$parameters = $container->parameters();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$parameters = $container->parameters();

Comment on lines +22 to +23
$services->set('sylius.form.type.security_login', SecurityLoginType::class)
->tag('form.type');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$services->set('sylius.form.type.security_login', SecurityLoginType::class)
->tag('form.type');
$services->set('sylius.form.type.security_login', SecurityLoginType::class)->tag('form.type');


return static function (ContainerConfigurator $container) {
$services = $container->services();
$parameters = $container->parameters();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$parameters = $container->parameters();

Comment on lines +17 to +22
$services = $container->services();
$parameters = $container->parameters();
$container->import('services/controller.php');
$container->import('services/form.php');
$container->import('services/twig.php');
$container->import('services/twig/**/*.php');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$services = $container->services();
$parameters = $container->parameters();
$container->import('services/controller.php');
$container->import('services/form.php');
$container->import('services/twig.php');
$container->import('services/twig/**/*.php');
$container->import('services/controller.php');
$container->import('services/form.php');
$container->import('services/twig.php');
$container->import('services/twig/**/*.php');

@crydotsnake crydotsnake force-pushed the xml2php/ui branch 3 times, most recently from 96b9e46 to ce2d703 Compare February 4, 2026 09:55
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/Sylius/Bundle/UiBundle/Resources/config/services.php`:
- Around line 16-21: The configurator closure currently lacks an explicit return
type; update the anonymous function declaration that takes ContainerConfigurator
$container (the closure in services.php) to declare a void return type by adding
": void" to the function signature so the closure becomes "function
(ContainerConfigurator $container): void { ... }", keeping all existing imports
(services/controller.php, services/form.php, services/twig.php,
services/twig/**/*.php) unchanged.

In
`@src/Sylius/Bundle/UiBundle/Resources/config/services/twig/error_renderer.php`:
- Around line 18-28: The closure passed to the service configuration currently
lacks a return type; update the anonymous function signature that takes
ContainerConfigurator $container (the configurator closure creating $services
and setting 'sylius.twig.error_renderer' with TwigErrorRenderer::class and
decorating 'twig.error_renderer.html') to declare a void return type by adding
": void" to the function signature so it complies with the coding guidelines for
Symfony service configurators.

Comment on lines +16 to +21
return static function (ContainerConfigurator $container) {
$container->import('services/controller.php');
$container->import('services/form.php');
$container->import('services/twig.php');
$container->import('services/twig/**/*.php');
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check if file exists and examine the relevant lines
if [ -f "src/Sylius/Bundle/UiBundle/Resources/config/services.php" ]; then
  echo "=== File exists ==="
  cat -n "src/Sylius/Bundle/UiBundle/Resources/config/services.php" | head -30
else
  echo "File does not exist"
fi

Repository: Sylius/Sylius

Length of output: 790


🌐 Web query:

Symfony ContainerConfigurator closure return type void PHP 8.2

💡 Result:

In Symfony’s PHP service config files (e.g. config/services.php, config/packages/*.php), the file should return a closure that Symfony will execute to configure the container. That closure doesn’t need to return anything, so on PHP 8.2 you can (and in newer docs, often do) declare it as : void. [1]

<?php
// config/services.php (or config/packages/something.php)
namespace Symfony\Component\DependencyInjection\Loader\Configurator;

return static function (ContainerConfigurator $container): void {
    $services = $container->services();
    // configure services...
};

If you want the current environment injected, add string $env (and still : void): [1]

return static function (ContainerConfigurator $container, string $env): void {
    // ...
};

Key point: the file returns the closure; the closure itself should return nothing (void), i.e., don’t return $container from inside it. [1]

Sources: [1] Symfony Docs – Service Container (PHP closure config + : void example).


Add an explicit void return type to the configurator closure.

The closure must declare its return type per coding guidelines. Symfony DI configuration closures return nothing and should explicitly declare : void.

♻️ Proposed change
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {
🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/UiBundle/Resources/config/services.php` around lines 16 -
21, The configurator closure currently lacks an explicit return type; update the
anonymous function declaration that takes ContainerConfigurator $container (the
closure in services.php) to declare a void return type by adding ": void" to the
function signature so the closure becomes "function (ContainerConfigurator
$container): void { ... }", keeping all existing imports
(services/controller.php, services/form.php, services/twig.php,
services/twig/**/*.php) unchanged.

Comment on lines +18 to +28
return static function (ContainerConfigurator $container) {
$services = $container->services();

$services->set('sylius.twig.error_renderer', TwigErrorRenderer::class)
->decorate('twig.error_renderer.html', null, 64)
->args([
service('.inner'),
service('twig'),
tagged_iterator('sylius.twig.error_template_finder'),
'%kernel.debug%',
]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Symfony ContainerConfigurator closure return type void PHP 8.2

💡 Result:

In Symfony’s PHP service config files (e.g. config/services.php), the file returns a closure, but that closure itself should return void (it configures the container; it doesn’t return config data). Symfony’s docs show the closure with an explicit : void return type, which is fully compatible with PHP 8.2. [1]

<?php

namespace Symfony\Component\DependencyInjection\Loader\Configurator;

use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $containerConfigurator): void {
    // configure services/parameters here
};

If you also want the current environment injected, add the $env argument (still : void): [1]

return static function (ContainerConfigurator $containerConfigurator, string $env): void {
    // ...
};

Sources: [1] Symfony Docs – Service Container (PHP configurator examples showing : void).


🏁 Script executed:

fd 'error_renderer.php' src/Sylius/Bundle/UiBundle/Resources/config/services/twig/ --type f

Repository: Sylius/Sylius

Length of output: 133


🏁 Script executed:

cat -n src/Sylius/Bundle/UiBundle/Resources/config/services/twig/error_renderer.php

Repository: Sylius/Sylius

Length of output: 1059


Add an explicit void return type to the configurator closure.

The closure on line 18 is missing the return type declaration required by the coding guidelines. Symfony's service configurator closures should declare : void since they configure the container without returning data.

♻️ Proposed change
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return static function (ContainerConfigurator $container) {
$services = $container->services();
$services->set('sylius.twig.error_renderer', TwigErrorRenderer::class)
->decorate('twig.error_renderer.html', null, 64)
->args([
service('.inner'),
service('twig'),
tagged_iterator('sylius.twig.error_template_finder'),
'%kernel.debug%',
]);
return static function (ContainerConfigurator $container): void {
$services = $container->services();
$services->set('sylius.twig.error_renderer', TwigErrorRenderer::class)
->decorate('twig.error_renderer.html', null, 64)
->args([
service('.inner'),
service('twig'),
tagged_iterator('sylius.twig.error_template_finder'),
'%kernel.debug%',
]);
🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/UiBundle/Resources/config/services/twig/error_renderer.php`
around lines 18 - 28, The closure passed to the service configuration currently
lacks a return type; update the anonymous function signature that takes
ContainerConfigurator $container (the configurator closure creating $services
and setting 'sylius.twig.error_renderer' with TwigErrorRenderer::class and
decorating 'twig.error_renderer.html') to declare a void return type by adding
": void" to the function signature so it complies with the coding guidelines for
Symfony service configurators.

@TheMilek TheMilek merged commit 3f15a43 into Sylius:2.3 Feb 4, 2026
36 checks passed
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