Conversation
📝 WalkthroughWalkthroughConverted ShopBundle DependencyInjection from XML to PHP: replaced XmlFileLoader with PhpFileLoader, moved many service/config files from .xml to .php, added Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
❌ Preview Environment deleted from BunnyshellAvailable commands:
|
37c67f9 to
37f6ac0
Compare
|
|
||
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); | ||
| $parameters = $container->parameters(); |
There was a problem hiding this comment.
| $parameters = $container->parameters(); |
same here and in other places where not used
| $services = $container->services(); | ||
| $parameters = $container->parameters(); | ||
| $container->import('sylius_admin/**/*.php'); |
There was a problem hiding this comment.
| $services = $container->services(); | |
| $parameters = $container->parameters(); | |
| $container->import('sylius_admin/**/*.php'); | |
| $container->import('sylius_admin/**/*.php'); |
| $services = $container->services(); | ||
| $parameters = $container->parameters(); | ||
| $container->import('services/**.php'); | ||
| $container->import('services/order_pay/**/*.php'); | ||
| $container->import('services/twig/**/*.php'); |
There was a problem hiding this comment.
| $services = $container->services(); | |
| $parameters = $container->parameters(); | |
| $container->import('services/**.php'); | |
| $container->import('services/order_pay/**/*.php'); | |
| $container->import('services/twig/**/*.php'); | |
| $container->import('services/**.php'); | |
| $container->import('services/order_pay/**/*.php'); | |
| $container->import('services/twig/**/*.php'); | |
| $services = $container->services(); |
37f6ac0 to
583610b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services.php`:
- Line 47: The alias for CartStorageInterface is not public so autowiring fails;
update the service alias created via
$services->alias(CartStorageInterface::class,
'sylius_shop.storage.cart_session') to be public by calling ->public() on the
alias so that type-hinted dependencies (e.g., in UserImpersonatedListener,
ShopUserLogoutHandler, SessionAndChannelBasedCartContext) can be autowired to
the 'sylius_shop.storage.cart_session' service.
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/controllers.php`:
- Around line 16-17: The closure returned by the factory is missing an explicit
return type; update the anonymous static function signature "static function
(ContainerConfigurator $container)" to declare a void return type (i.e., add ":
void") so the configurator closure explicitly returns void in accordance with
the project's type-declaration guideline.
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/offline/providers.php`:
- Around line 18-19: The exported configurator closure currently has no return
type; update the static closure declaration in this file from "static function
(ContainerConfigurator $container) {" to include an explicit void return type as
"static function (ContainerConfigurator $container): void {" so the closure
signature (ContainerConfigurator $container) matches the project's
type-declaration guidelines for the configurator.
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/providers.php`:
- Around line 16-17: The closure returned by the factory (the static function
accepting ContainerConfigurator $container) needs an explicit void return type;
update the declaration of the returned closure (the "static function
(ContainerConfigurator $container)") to declare ": void" so it becomes "static
function (ContainerConfigurator $container): void" to satisfy the
type-declaration guideline.
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/resolvers.php`:
- Around line 19-20: The returned static closure "static function
(ContainerConfigurator $container)" in the resolver registration should declare
an explicit void return type; update the closure signature to "static function
(ContainerConfigurator $container): void" so the closure's return type is
explicit and complies with the type-declaration guideline (locate the closure
that returns the ContainerConfigurator and add ": void" to its signature).
🧹 Nitpick comments (4)
src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/account.php (1)
21-24: Consider sortinguseimports alphabetically.The component class imports are not in alphabetical order.
ChangePasswordFormComponentshould come beforeDefaultFormComponent.♻️ Suggested reordering
-use Sylius\Bundle\ShopBundle\Twig\Component\Account\Address\DefaultFormComponent; -use Sylius\Bundle\ShopBundle\Twig\Component\Account\Address\FormComponent; use Sylius\Bundle\ShopBundle\Twig\Component\Account\ChangePasswordFormComponent; +use Sylius\Bundle\ShopBundle\Twig\Component\Account\Address\DefaultFormComponent; +use Sylius\Bundle\ShopBundle\Twig\Component\Account\Address\FormComponent; use Sylius\Bundle\UiBundle\Twig\Component\ResourceFormComponent;src/Sylius/Bundle/ShopBundle/Resources/config/services/integrations/sylius_admin/form.php (1)
20-20: Remove unused variable.The
$parametersvariable is defined but never used in this file.♻️ Suggested fix
return static function (ContainerConfigurator $container) { $services = $container->services(); - $parameters = $container->parameters(); $services->set('sylius_shop.form.extension.type.channel', ChannelTypeExtension::class)src/Sylius/Bundle/ShopBundle/Resources/config/services/context.php (1)
21-25: Consider making this service public if ThemeBundle requires direct container access.The service is referenced in
SyliusShopExtension::prependSyliusThemeBundle()as a configuration value passed to thesylius_themeextension. If ThemeBundle directly fetches this service from the container at runtime, it should be marked as public.♻️ Suggested change
$services->set('sylius_shop.theme.context.channel_based', ChannelBasedThemeContext::class) ->args([ service('sylius.context.channel'), service('sylius.repository.theme'), - ]); + ]) + ->public();src/Sylius/Bundle/ShopBundle/Resources/config/services/menu.php (1)
21-26: Sort tag attribute keys alphabetically.This improves consistency with coding guidelines.
♻️ Suggested change
- ->tag('knp_menu.menu_builder', ['method' => 'createMenu', 'alias' => 'sylius_shop.account']); + ->tag('knp_menu.menu_builder', ['alias' => 'sylius_shop.account', 'method' => 'createMenu']);As per coding guidelines, order PHP array keys alphabetically where applicable.
| service('sylius.repository.order'), | ||
| ]); | ||
|
|
||
| $services->alias(CartStorageInterface::class, 'sylius_shop.storage.cart_session'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and inspect the original XML services configuration
if [ -f "src/Sylius/Bundle/ShopBundle/Resources/config/services.xml" ]; then
echo "=== Current XML file exists ==="
cat "src/Sylius/Bundle/ShopBundle/Resources/config/services.xml" | head -100
else
echo "XML file not found in working directory"
fi
# Check git history
echo -e "\n=== Checking git log for services.xml ==="
git log --oneline -- "src/Sylius/Bundle/ShopBundle/Resources/config/services.xml" 2>/dev/null | head -5Repository: Sylius/Sylius
Length of output: 139
🏁 Script executed:
#!/bin/bash
# Search for CartStorageInterface usage in the codebase
echo "=== Searching for CartStorageInterface usage ==="
rg "CartStorageInterface" --type php -B 2 -A 2
echo -e "\n=== Checking if services.xml exists in ShopBundle ==="
find . -path "*/ShopBundle/Resources/config/*" -name "*.xml" -o -name "*.php" | head -20
echo -e "\n=== Listing config files in ShopBundle ==="
ls -la "src/Sylius/Bundle/ShopBundle/Resources/config/" 2>/dev/null || echo "Directory not found"
</parameter>Repository: Sylius/Sylius
Length of output: 16995
🏁 Script executed:
#!/bin/bash
# Find the CartSessionStorage service definition in the PHP config
echo "=== Checking CartSessionStorage service definition ==="
cat -n "src/Sylius/Bundle/ShopBundle/Resources/config/services.php" | grep -A 10 -B 2 "CartSessionStorage"
# Check if there's a default autowire setting
echo -e "\n=== Checking for default autowire in services.php ==="
cat -n "src/Sylius/Bundle/ShopBundle/Resources/config/services.php" | head -50
# Search for how services are typically defined in CoreBundle for reference
echo -e "\n=== Looking for CartSessionStorage in CoreBundle config ==="
rg "CartSessionStorage" --type php -A 3 -B 1 | grep -A 5 -B 5 "set\|alias\|public"Repository: Sylius/Sylius
Length of output: 6198
🏁 Script executed:
#!/bin/bash
# Search for SessionCartSubscriber and other dependents in config
echo "=== Checking if SessionCartSubscriber is explicitly configured ==="
rg "SessionCartSubscriber" --type php | grep -i "set\|config\|service"
echo -e "\n=== Checking CoreBundle config for CartSessionStorage ==="
find . -path "*/CoreBundle/Resources/config/*" -name "services.php" | head -1 | xargs cat 2>/dev/null | head -100
echo -e "\n=== Checking if there's a base services config with autowiring defaults ==="
cat "config/services.yaml" 2>/dev/null || echo "Not found in config/services.yaml"
echo -e "\n=== Checking Kernel config ==="
grep -A 20 "autoconfigure\|autowire" "src/Kernel.php" 2>/dev/null | head -30Repository: Sylius/Sylius
Length of output: 590
🏁 Script executed:
#!/bin/bash
# Check the listeners.php file to see how SessionCartSubscriber is configured
echo "=== Checking listeners.php configuration ==="
cat "src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.php"
echo -e "\n=== Check if there's default autowiring in the container ==="
grep -r "autowire.*true\|autoconfigure.*true" src/Sylius/Bundle/ShopBundle/DependencyInjection/ 2>/dev/null | head -10Repository: Sylius/Sylius
Length of output: 6659
Mark the alias as public to enable autowiring for type-hinted dependencies.
The CartStorageInterface alias points to a private service and does not have an explicit ->public() declaration. This prevents autowiring from resolving CartStorageInterface in type-hinted constructor parameters, which is required by classes like UserImpersonatedListener, ShopUserLogoutHandler, and SessionAndChannelBasedCartContext.
♻️ Suggested change
- $services->alias(CartStorageInterface::class, 'sylius_shop.storage.cart_session');
+ $services->alias(CartStorageInterface::class, 'sylius_shop.storage.cart_session')
+ ->public();🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services.php` at line 47, The
alias for CartStorageInterface is not public so autowiring fails; update the
service alias created via $services->alias(CartStorageInterface::class,
'sylius_shop.storage.cart_session') to be public by calling ->public() on the
alias so that type-hinted dependencies (e.g., in UserImpersonatedListener,
ShopUserLogoutHandler, SessionAndChannelBasedCartContext) can be autowired to
the 'sylius_shop.storage.cart_session' service.
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); |
There was a problem hiding this comment.
Add explicit : void return type on the configurator closure.
The closure doesn’t return a value; declare it as void to satisfy the type-declaration guideline.
✅ Suggested fix
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {As per coding guidelines, add type declarations for all properties, arguments, and return values in PHP code.
📝 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.
| return static function (ContainerConfigurator $container) { | |
| $services = $container->services(); | |
| return static function (ContainerConfigurator $container): void { | |
| $services = $container->services(); |
🤖 Prompt for AI Agents
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/controllers.php`
around lines 16 - 17, The closure returned by the factory is missing an explicit
return type; update the anonymous static function signature "static function
(ContainerConfigurator $container)" to declare a void return type (i.e., add ":
void") so the configurator closure explicitly returns void in accordance with
the project's type-declaration guideline.
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); |
There was a problem hiding this comment.
Add explicit : void return type on the configurator closure.
The closure doesn’t return a value; declare it as void to satisfy the type-declaration guideline.
✅ Suggested fix
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {As per coding guidelines, add type declarations for all properties, arguments, and return values in PHP code.
📝 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.
| return static function (ContainerConfigurator $container) { | |
| $services = $container->services(); | |
| return static function (ContainerConfigurator $container): void { | |
| $services = $container->services(); |
🤖 Prompt for AI Agents
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/offline/providers.php`
around lines 18 - 19, The exported configurator closure currently has no return
type; update the static closure declaration in this file from "static function
(ContainerConfigurator $container) {" to include an explicit void return type as
"static function (ContainerConfigurator $container): void {" so the closure
signature (ContainerConfigurator $container) matches the project's
type-declaration guidelines for the configurator.
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); |
There was a problem hiding this comment.
Add explicit : void return type on the configurator closure.
The closure doesn’t return a value; declare it as void to satisfy the type-declaration guideline.
✅ Suggested fix
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {As per coding guidelines, add type declarations for all properties, arguments, and return values in PHP code.
🤖 Prompt for AI Agents
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/providers.php`
around lines 16 - 17, The closure returned by the factory (the static function
accepting ContainerConfigurator $container) needs an explicit void return type;
update the declaration of the returned closure (the "static function
(ContainerConfigurator $container)") to declare ": void" so it becomes "static
function (ContainerConfigurator $container): void" to satisfy the
type-declaration guideline.
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); |
There was a problem hiding this comment.
Add explicit : void return type on the configurator closure.
The closure doesn’t return a value; declare it as void to satisfy the type-declaration guideline.
✅ Suggested fix
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {As per coding guidelines, add type declarations for all properties, arguments, and return values in PHP code.
📝 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.
| return static function (ContainerConfigurator $container) { | |
| $services = $container->services(); | |
| return static function (ContainerConfigurator $container): void { | |
| $services = $container->services(); |
🤖 Prompt for AI Agents
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/resolvers.php`
around lines 19 - 20, The returned static closure "static function
(ContainerConfigurator $container)" in the resolver registration should declare
an explicit void return type; update the closure signature to "static function
(ContainerConfigurator $container): void" so the closure's return type is
explicit and complies with the type-declaration guideline (locate the closure
that returns the ContainerConfigurator and add ": void" to its signature).
583610b to
4e8c74f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.php`:
- Around line 57-59: The kernel.event_subscriber tag for the service
sylius_shop.event_subscriber.shop_customer_account_sub_section_cache_control is
incorrectly given an explicit ['event' => 'kernel.response'] attribute even
though ShopCustomerAccountSubSectionCacheControlSubscriber implements
EventSubscriberInterface and declares its events via getSubscribedEvents();
remove the redundant event attribute so the service definition becomes
->tag('kernel.event_subscriber') to match the pattern used by
SessionCartSubscriber and other subscribers.
🧹 Nitpick comments (1)
src/Sylius/Bundle/ShopBundle/Resources/config/services/menu.php (1)
21-26: Sort tag option keys alphabetically.Please swap the order to keep array keys consistent.
♻️ Proposed tweak
- ->tag('knp_menu.menu_builder', ['method' => 'createMenu', 'alias' => 'sylius_shop.account']); + ->tag('knp_menu.menu_builder', ['alias' => 'sylius_shop.account', 'method' => 'createMenu']);As per coding guidelines: Order PHP array keys alphabetically where applicable.
| $services->set('sylius_shop.event_subscriber.shop_customer_account_sub_section_cache_control', ShopCustomerAccountSubSectionCacheControlSubscriber::class) | ||
| ->args([service('sylius.section_resolver.uri_based')]) | ||
| ->tag('kernel.event_subscriber', ['event' => 'kernel.response']); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the full listeners.php file structure around the mentioned lines
head -70 src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.php | tail -20Repository: Sylius/Sylius
Length of output: 1319
🏁 Script executed:
# Find and examine the ShopCustomerAccountSubSectionCacheControlSubscriber class
find . -name "ShopCustomerAccountSubSectionCacheControlSubscriber.php" -type fRepository: Sylius/Sylius
Length of output: 157
🏁 Script executed:
# Search for all event_subscriber tags in the listeners.php config file
rg -n "kernel.event_subscriber" src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.php -A 1 -B 1Repository: Sylius/Sylius
Length of output: 272
🏁 Script executed:
# Check if there are any other subscriber tags with event attributes
rg "kernel.event_subscriber.*\[.*event" src/Sylius/Bundle/ShopBundle/Resources/config/services/ -rRepository: Sylius/Sylius
Length of output: 470
🏁 Script executed:
cat -n ./src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.phpRepository: Sylius/Sylius
Length of output: 2054
🏁 Script executed:
# Also check the SessionCartSubscriber class mentioned at line 102 for comparison
grep -n "class SessionCartSubscriber" src/Sylius/Bundle/ShopBundle/EventListener/*.phpRepository: Sylius/Sylius
Length of output: 198
🏁 Script executed:
# Let's see the full SessionCartSubscriber to compare implementation
cat src/Sylius/Bundle/ShopBundle/EventListener/SessionCartSubscriber.phpRepository: Sylius/Sylius
Length of output: 1830
🏁 Script executed:
# Let's verify Symfony conventions by checking if there are other event_subscriber tags with event attributes
rg -n "kernel\.event_subscriber" src/Sylius/Bundle/ShopBundle/Resources/config/services/ -A 1Repository: Sylius/Sylius
Length of output: 467
🏁 Script executed:
# Check across other Sylius bundles for consistency
rg -n "kernel\.event_subscriber.*\[" src/Sylius/Bundle/ -A 1 | head -40Repository: Sylius/Sylius
Length of output: 275
Remove unnecessary event attribute from kernel.event_subscriber tag.
The kernel.event_subscriber tag includes ['event' => 'kernel.response'], which is redundant since ShopCustomerAccountSubSectionCacheControlSubscriber implements EventSubscriberInterface and defines subscribed events via getSubscribedEvents(). This is also inconsistent with SessionCartSubscriber at line 102, which correctly uses ->tag('kernel.event_subscriber') without the event attribute. Across all Sylius bundles, this is the only kernel.event_subscriber tag with an event attribute.
🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.php` around
lines 57 - 59, The kernel.event_subscriber tag for the service
sylius_shop.event_subscriber.shop_customer_account_sub_section_cache_control is
incorrectly given an explicit ['event' => 'kernel.response'] attribute even
though ShopCustomerAccountSubSectionCacheControlSubscriber implements
EventSubscriberInterface and declares its events via getSubscribedEvents();
remove the redundant event attribute so the service definition becomes
->tag('kernel.event_subscriber') to match the pattern used by
SessionCartSubscriber and other subscribers.
4e8c74f to
56b0288
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services/modifier.php`:
- Around line 18-23: The closure returned by the file-level factory is missing
an explicit return type; change the signature of the returned static function
(the anonymous closure that accepts ContainerConfigurator $container and
currently returns nothing) to declare a void return type by adding ": void" to
the closure signature so it conforms to the project's return-type guideline
(look for the "return static function (ContainerConfigurator $container)"
construct and update it).
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/resolvers.php`:
- Line 21: Remove the dead assignment to $parameters; the variable $parameters
is set via $container->parameters() but never used, so delete that line in
resolvers.php (or replace its usage with a direct call to
$container->parameters() where actually needed). Ensure no other code references
$parameters after removal and run tests to confirm nothing relies on that unused
variable.
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/account.php`:
- Around line 26-27: The closure passed in this file is missing a return type;
update the closure signature that accepts ContainerConfigurator $container (the
anonymous function returning nothing where $services = $container->services() is
called) to declare a void return type (i.e. change to static function
(ContainerConfigurator $container): void { ... }) so it follows Symfony/PHP
guidelines for side-effect-only configurators and remains compatible with the
project PHP version.
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/common.php`:
- Around line 19-20: The returned static configurator closure lacks an explicit
return type; update the closure signature for the returned function (the "static
function (ContainerConfigurator $container)" closure) to declare a void return
type so it becomes "static function (ContainerConfigurator $container): void" to
comply with the project's type declaration rules and Symfony DI conventions.
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/product_review.php`:
- Around line 28-30: The Twig component tag key uses a dot separator instead of
the colon, preventing matching; update the tag on the service
sylius_shop.twig.component.product_review.count (CountComponent::class) so the
'sylius.twig_component' tag uses the key 'sylius_shop:product_review.count'
(colon between namespace and component) instead of
'sylius_shop.product_review.count'.
🧹 Nitpick comments (3)
src/Sylius/Bundle/ShopBundle/Resources/config/services/integrations/locale/url.php (1)
19-21: Remove unused$parametersvariable.The
$parametersvariable is assigned on line 21 but never used in this configuration file.♻️ Proposed fix
return static function (ContainerConfigurator $container) { $services = $container->services(); - $parameters = $container->parameters(); $services->set('sylius_shop.locale_switcher', UrlBasedLocaleSwitcher::class)src/Sylius/Bundle/ShopBundle/Resources/config/services.php (2)
23-23: Add an explicit: voidreturn type for the configurator closure.
Keeps return typing consistent with the project guidelines.As per coding guidelines, Add type declarations for all properties, arguments, and return values in PHP code.
Proposed diff
-return static function (ContainerConfigurator $container) { +return static function (ContainerConfigurator $container): void {
49-50: Alphabetize tag attribute keys for consistency.
This keeps config arrays predictable.As per coding guidelines, Order PHP array keys alphabetically where applicable.
Proposed diff
- $services->set('sylius_shop.grid_filter.string', StringFilter::class) - ->tag('sylius.grid_filter', ['type' => 'shop_string', 'form_type' => StringFilterType::class]); + $services->set('sylius_shop.grid_filter.string', StringFilter::class) + ->tag('sylius.grid_filter', ['form_type' => StringFilterType::class, 'type' => 'shop_string']);
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); | ||
|
|
||
| $services->set('sylius_shop.modifier.default_address_form_values', DefaultAddressFormValuesModifier::class) | ||
| ->tag('sylius_shop.modifier.address_form_values'); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit void return type for the config closure.
The guidelines require return type declarations in PHP code; this closure should declare : void.
🛠️ Proposed change
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {
$services = $container->services();
$services->set('sylius_shop.modifier.default_address_form_values', DefaultAddressFormValuesModifier::class)
->tag('sylius_shop.modifier.address_form_values');
};🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services/modifier.php` around
lines 18 - 23, The closure returned by the file-level factory is missing an
explicit return type; change the signature of the returned static function (the
anonymous closure that accepts ContainerConfigurator $container and currently
returns nothing) to declare a void return type by adding ": void" to the closure
signature so it conforms to the project's return-type guideline (look for the
"return static function (ContainerConfigurator $container)" construct and update
it).
|
|
||
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); | ||
| $parameters = $container->parameters(); |
There was a problem hiding this comment.
Remove unused $parameters variable.
The $parameters variable is assigned but never used in this file. Remove this dead code.
Suggested fix
return static function (ContainerConfigurator $container): void {
$services = $container->services();
- $parameters = $container->parameters();
$services->set('sylius_shop.resolver.order_pay.payment_to_pay', PaymentToPayResolver::class)📝 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.
| $parameters = $container->parameters(); | |
| return static function (ContainerConfigurator $container): void { | |
| $services = $container->services(); | |
| $services->set('sylius_shop.resolver.order_pay.payment_to_pay', PaymentToPayResolver::class) |
🤖 Prompt for AI Agents
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/resolvers.php`
at line 21, Remove the dead assignment to $parameters; the variable $parameters
is set via $container->parameters() but never used, so delete that line in
resolvers.php (or replace its usage with a direct call to
$container->parameters() where actually needed). Ensure no other code references
$parameters after removal and run tests to confirm nothing relies on that unused
variable.
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Symfony DependencyInjection PHP config configurator closure return type void
💡 Result:
In Symfony’s PHP-DI “config files” (e.g. config/services_test.php, config/packages/*.php), the file typically returns a closure that Symfony will execute to mutate the container. That closure is not supposed to return a value, so its natural return type is void.
Symfony’s docs explicitly show this pattern, including an example where Symfony can autowire the current environment into the closure and the closure is declared : void:
return function (ContainerConfigurator $containerConfigurator, string $env): void {
// ...
};[1]
So:
: voidis the correct/idiomatic return type because the closure is executed for side effects (service definitions), not for producing a value. [1]- You should not
return $containerConfigurator/return $services/ etc. from that closure; just configure and end. [2]
Sources:
[1] Symfony Docs – Service Container (PHP closures in config, $env injection, : void) (symfony.com)
[2] Symfony Docs – Service Container (returned config closure examples) (symfony.com)
Citations:
- 1: https://symfony.com/doc/current/service_container.html
- 2: https://symfony.com/doc/current/service_container.html
🏁 Script executed:
# Search for similar configurator patterns in the codebase
fd -e php -path '*/config/services*' | head -20 | xargs grep -l "static function.*ContainerConfigurator" | head -5Repository: Sylius/Sylius
Length of output: 225
🏁 Script executed:
# Check the specific file and nearby config files for patterns
fd -e php -path '*/config/services*' -path '*/twig/component*' -o -path '*/config/services*' | head -10Repository: Sylius/Sylius
Length of output: 225
🏁 Script executed:
# Search for PHP DI config files and check for return type patterns
find . -name "*.php" -path "*/config/services*" | head -20 | xargs grep -A 1 "return.*function.*ContainerConfigurator" | head -30Repository: Sylius/Sylius
Length of output: 2518
Add a void return type to the configurator closure.
This closure is missing a return type; the guidelines require return type declarations. The closure executes for side effects only (configuring the container), so : void is the idiomatic Symfony return type per official documentation.
♻️ 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.
| return static function (ContainerConfigurator $container) { | |
| $services = $container->services(); | |
| return static function (ContainerConfigurator $container): void { | |
| $services = $container->services(); |
🤖 Prompt for AI Agents
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/account.php`
around lines 26 - 27, The closure passed in this file is missing a return type;
update the closure signature that accepts ContainerConfigurator $container (the
anonymous function returning nothing where $services = $container->services() is
called) to declare a void return type (i.e. change to static function
(ContainerConfigurator $container): void { ... }) so it follows Symfony/PHP
guidelines for side-effect-only configurators and remains compatible with the
project PHP version.
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual file
cat -n src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/common.php | head -30Repository: Sylius/Sylius
Length of output: 1181
🌐 Web query:
Symfony DependencyInjection PHP configuration configurator closure return type void
💡 Result:
In Symfony’s PHP-DI config files (e.g. config/services.php, config/packages/*.php), the file must return a callable, but the configurator closure itself should not return anything—its return type is effectively void.
Recommended signature:
<?php
// config/services.php
namespace Symfony\Component\DependencyInjection\Loader\Configurator;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
return static function (ContainerConfigurator $container): void {
$services = $container->services();
// define services...
};Symfony’s docs show this pattern (including : void), and also show that you can add extra injected args like $env while still returning void. [1] (See also a common return static function (...): void {} example in practice. [2])
Note: You’ll also see examples without : void; that works because Symfony ignores any return value from the closure, but : void is the clearest/correct intent. [1]
Sources:
[1] Symfony Docs – Service Container (PHP closure config examples, including : void and $env injection)
[2] SymfonyCasts – bundle services.php using return static function (ContainerConfigurator $container): void
Add a void return type to the configurator closure.
This closure is missing a return type declaration. Per the coding guidelines requiring type declarations for all methods and return values, add : void to indicate the closure does not return a value. This also aligns with Symfony's PHP DI configuration conventions.
♻️ 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.
| return static function (ContainerConfigurator $container) { | |
| $services = $container->services(); | |
| return static function (ContainerConfigurator $container): void { | |
| $services = $container->services(); |
🤖 Prompt for AI Agents
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/common.php`
around lines 19 - 20, The returned static configurator closure lacks an explicit
return type; update the closure signature for the returned function (the "static
function (ContainerConfigurator $container)" closure) to declare a void return
type so it becomes "static function (ContainerConfigurator $container): void" to
comply with the project's type declaration rules and Symfony DI conventions.
src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/product_review.php
Show resolved
Hide resolved
56b0288 to
6cf89fb
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services.php`:
- Around line 23-28: The exported anonymous function in
src/Sylius/Bundle/ShopBundle/Resources/config/services.php lacks an explicit
return type; update the closure signature "return static function
(ContainerConfigurator $container)" to include the void return type (i.e. add ":
void") so it becomes "return static function (ContainerConfigurator $container):
void" to satisfy the coding guidelines and modern PHP conventions; ensure the
change is applied to the closure that performs imports and that your project PHP
version supports void return types.
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services/controller.php`:
- Around line 21-22: The configurator closure currently has no return type; add
an explicit void return type to the returned static closure so its signature
reads static function (ContainerConfigurator $container): void { to reflect that
it does not return a value and improve type clarity for ContainerConfigurator
usage.
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.php`:
- Around line 31-32: The returned anonymous configurator closure currently has
no return type; update the closure signature for the returned value (the "static
function (ContainerConfigurator $container)" closure) to declare a void return
type by adding ": void" to the closure signature so the configurator explicitly
returns nothing.
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/product_review.php`:
- Around line 21-22: The configurator closure returned by "return static
function (ContainerConfigurator $container)" lacks an explicit return type;
change its signature to include ": void" so it becomes "return static function
(ContainerConfigurator $container): void" to reflect that the closure does not
return a value and satisfy the type hinting expectations for the container
configuration (the services variable inside remains unchanged).
🧹 Nitpick comments (5)
src/Sylius/Bundle/ShopBundle/Resources/config/services/order_pay/handlers.php (1)
18-18: Remove unused$parametersvariable.The
$parametersvariable is declared but never used in this file.🧹 Proposed fix
return static function (ContainerConfigurator $container) { $services = $container->services(); - $parameters = $container->parameters(); $services->set('sylius_shop.handler.order_pay.payment_state_flash')src/Sylius/Bundle/ShopBundle/Resources/config/services/form.php (1)
28-28: Add: voidreturn type to the configurator closure.The closure is executed for side effects only (configuring the container) and should declare a void return type per Symfony best practices and coding guidelines requiring return type declarations.
♻️ Proposed fix
-return static function (ContainerConfigurator $container) { +return static function (ContainerConfigurator $container): void {src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/product.php (1)
25-26: Add: voidreturn type to the configurator closure.The closure should declare a void return type per coding guidelines requiring return type declarations.
♻️ Proposed fix
-return static function (ContainerConfigurator $container) { +return static function (ContainerConfigurator $container): void {src/Sylius/Bundle/ShopBundle/Resources/config/services/integrations/locale/storage.php (1)
22-24: Add: voidreturn type and remove unused variable.The closure should declare a void return type. Additionally,
$parameterson line 24 is declared but never used.♻️ Proposed fix
-return static function (ContainerConfigurator $container) { +return static function (ContainerConfigurator $container): void { $services = $container->services(); - $parameters = $container->parameters();src/Sylius/Bundle/ShopBundle/Resources/config/services/integrations/locale/url.php (1)
19-21: Add: voidreturn type and remove unused variable.The closure should declare a void return type. Additionally,
$parameterson line 21 is declared but never used.♻️ Proposed fix
-return static function (ContainerConfigurator $container) { +return static function (ContainerConfigurator $container): void { $services = $container->services(); - $parameters = $container->parameters();
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); | ||
| $container->import('services/**.php'); | ||
| $container->import('services/order_pay/**/*.php'); | ||
| $container->import('services/twig/**/*.php'); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check how other services.php config closures declare return types
rg -n "return static function \(ContainerConfigurator \$container\)" -g 'services.php' -A 1Repository: Sylius/Sylius
Length of output: 39
🏁 Script executed:
# Also search for variations with return types already declared
rg -n "return static function \(ContainerConfigurator \$container\)" -A 1 -g '**/*.php' | head -50Repository: Sylius/Sylius
Length of output: 39
🏁 Script executed:
# Check the specific file mentioned in the review
cat -n src/Sylius/Bundle/ShopBundle/Resources/config/services.php | head -35Repository: Sylius/Sylius
Length of output: 1627
🏁 Script executed:
# Find all services.php config files
fd -name 'services.php' -type f | head -20Repository: Sylius/Sylius
Length of output: 287
🏁 Script executed:
# Check patterns in services.php files without strict regex escaping
rg 'return static function' -g 'services.php' -A 2 | head -50Repository: Sylius/Sylius
Length of output: 4126
🏁 Script executed:
# Look for any return type declarations in services.php files
rg 'return static function.*\).*:' -g 'services.php'Repository: Sylius/Sylius
Length of output: 39
Add explicit void return type to the config closure.
Line 23 closure lacks a return type declaration. This aligns with the coding guidelines requiring type declarations for all return values and follows modern PHP 8.2+ conventions.
Proposed change
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services.php` around lines 23 -
28, The exported anonymous function in
src/Sylius/Bundle/ShopBundle/Resources/config/services.php lacks an explicit
return type; update the closure signature "return static function
(ContainerConfigurator $container)" to include the void return type (i.e. add ":
void") so it becomes "return static function (ContainerConfigurator $container):
void" to satisfy the coding guidelines and modern PHP conventions; ensure the
change is applied to the closure that performs imports and that your project PHP
version supports void return types.
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/Sylius/Bundle/ShopBundle/Resources/config/services/controller.php | head -60Repository: Sylius/Sylius
Length of output: 2324
Add explicit : void return type on the configurator closure.
The closure doesn't return a value.
♻️ Proposed change
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services/controller.php` around
lines 21 - 22, The configurator closure currently has no return type; add an
explicit void return type to the returned static closure so its signature reads
static function (ContainerConfigurator $container): void { to reflect that it
does not return a value and improve type clarity for ContainerConfigurator
usage.
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.php | head -140Repository: Sylius/Sylius
Length of output: 7448
Add explicit : void return type on the configurator closure.
The closure doesn't return a value.
♻️ Proposed change
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {As per coding guidelines, add type declarations for all properties, arguments, and return values in PHP code.
🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.php` around
lines 31 - 32, The returned anonymous configurator closure currently has no
return type; update the closure signature for the returned value (the "static
function (ContainerConfigurator $container)" closure) to declare a void return
type by adding ": void" to the closure signature so the configurator explicitly
returns nothing.
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "product_review.php" | head -20Repository: Sylius/Sylius
Length of output: 147
🏁 Script executed:
cat -n ./src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/product_review.phpRepository: Sylius/Sylius
Length of output: 1916
Add explicit : void return type on the configurator closure.
The closure doesn't return a value.
♻️ Proposed change
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {🤖 Prompt for AI Agents
In
`@src/Sylius/Bundle/ShopBundle/Resources/config/services/twig/component/product_review.php`
around lines 21 - 22, The configurator closure returned by "return static
function (ContainerConfigurator $container)" lacks an explicit return type;
change its signature to include ": void" so it becomes "return static function
(ContainerConfigurator $container): void" to reflect that the closure does not
return a value and satisfy the type hinting expectations for the container
configuration (the services variable inside remains unchanged).
6cf89fb to
16e6f6c
Compare
Summary by CodeRabbit
Chores
New Features