Conversation
📝 WalkthroughWalkthroughSwaps XML DI configs for PHP in ProductBundle: Extension now uses PhpFileLoader and loads .php service fragments (services, form, validators, integrations). Adds many service/alias/tag declarations as PHP files and deletes corresponding XML files. Adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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:
|
4f229c4 to
40e3b7e
Compare
| $parameters = $container->parameters(); | ||
|
|
||
| $services->defaults() | ||
| ->public(); |
There was a problem hiding this comment.
| $parameters = $container->parameters(); | |
| $services->defaults() | |
| ->public(); | |
| $services->defaults()->public(); |
| $services = $container->services(); | ||
| $parameters = $container->parameters(); | ||
| $container->import('services/**/*.php'); | ||
|
|
||
| $services->defaults() | ||
| ->public(); |
There was a problem hiding this comment.
| $services = $container->services(); | |
| $parameters = $container->parameters(); | |
| $container->import('services/**/*.php'); | |
| $services->defaults() | |
| ->public(); | |
| $container->import('services/**/*.php'); | |
| $services = $container->services(); | |
| $services->defaults()->public(); |
40e3b7e to
9059b2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Sylius/Bundle/ProductBundle/Resources/config/services.php`:
- Line 35: The closure calling $container->import('services/**/*.php') should be
updated to include an explicit return type declaration (add ": void" to the
closure signature) and the glob should be narrowed to avoid re-importing
integration files (change the pattern from "services/**/*.php" to
"services/*.php"); update the closure that contains the $container->import call
(the anonymous function in this services.php file) to have a void return type
and replace the glob pattern so it only imports top-level service files and
prevents duplicates already loaded by SyliusProductExtension.
🧹 Nitpick comments (1)
src/Sylius/Bundle/ProductBundle/Resources/config/services/validators.php (1)
20-20: Add: voidreturn type to the config closure.This keeps the DI config aligned with the typing rule for PHP code.
♻️ Suggested 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.
|
|
||
| return static function (ContainerConfigurator $container) { | ||
| $services = $container->services(); | ||
| $container->import('services/**/*.php'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the services.php file to understand context
head -n 50 src/Sylius/Bundle/ProductBundle/Resources/config/services.phpRepository: Sylius/Sylius
Length of output: 2179
🏁 Script executed:
# List the directory structure under services/
find src/Sylius/Bundle/ProductBundle/Resources/config/services -type f -name "*.php" | sortRepository: Sylius/Sylius
Length of output: 284
🏁 Script executed:
# Find the SyliusProductExtension class to see how it loads integrations
find . -name "*ProductExtension*" -o -name "*Extension.php" | grep -i productRepository: Sylius/Sylius
Length of output: 1527
🏁 Script executed:
# Examine SyliusProductExtension to see how it loads integrations
cat -n src/Sylius/Bundle/ProductBundle/DependencyInjection/SyliusProductExtension.phpRepository: Sylius/Sylius
Length of output: 5565
🏁 Script executed:
# Check if there's any mention of integration loading patterns
rg -n "services/integrations" src/Sylius/Bundle/ProductBundle/Repository: Sylius/Sylius
Length of output: 221
🏁 Script executed:
# Check other bundle configurations to see if they use narrower globs
rg -A2 'import\(.*services' src/Sylius/Bundle/*/Resources/config/ | head -40Repository: Sylius/Sylius
Length of output: 314
🏁 Script executed:
# Check if there's documentation or other patterns for services directory structure
rg -n 'services/\*\*' src/Sylius/Bundle/*/Resources/config/Repository: Sylius/Sylius
Length of output: 603
Add return type declaration to closure and narrow glob import to avoid duplicate loading.
The closure at line 35 lacks an explicit return type (: void per PHP 8.2+ guidelines). Additionally, services/**/*.php will re-import services/integrations/%s.php that is already explicitly loaded by SyliusProductExtension at line 43 before services.php is loaded. Narrow the glob to top-level files only:
Suggested changes
-return static function (ContainerConfigurator $container) {
+return static function (ContainerConfigurator $container): void {
$services = $container->services();
- $container->import('services/**/*.php');
+ $container->import('services/*.php');🤖 Prompt for AI Agents
In `@src/Sylius/Bundle/ProductBundle/Resources/config/services.php` at line 35,
The closure calling $container->import('services/**/*.php') should be updated to
include an explicit return type declaration (add ": void" to the closure
signature) and the glob should be narrowed to avoid re-importing integration
files (change the pattern from "services/**/*.php" to "services/*.php"); update
the closure that contains the $container->import call (the anonymous function in
this services.php file) to have a void return type and replace the glob pattern
so it only imports top-level service files and prevents duplicates already
loaded by SyliusProductExtension.
7305fd0 to
f3da284
Compare
f3da284 to
0d11607
Compare
Summary by CodeRabbit
New Features
Chores