Feature/command rework #173
Conversation
|
Warning Rate limit exceeded@GautierDele has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis update refactors and modernizes the console command infrastructure for generating REST-related classes in a Laravel-based application. It replaces legacy command classes and trait-based stub resolution with new "Make" command classes, updates stub templates to use dynamic placeholders, and revises test coverage to match the new command structure. Stub publishing is now formally registered, and test files are reorganized and expanded for the new commands. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Artisan
participant MakeCommand
participant FileSystem
User->>Artisan: Run rest:controller/resource/etc.
Artisan->>MakeCommand: Instantiate command
MakeCommand->>MakeCommand: Resolve stub path
MakeCommand->>FileSystem: Generate class file from stub
FileSystem-->>MakeCommand: File created
MakeCommand-->>Artisan: Command complete
Artisan-->>User: Output result
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (2)
tests/Feature/Commands/BaseResourceMakeCommandTest.php (1)
11-11: Consider using explicit error handling instead of suppressing errors.Same issue as in
ActionMakeCommandTest- the@unlinksuppresses all errors. Consider checking file existence before deletion for better error visibility.tests/Feature/Commands/BaseControllerMakeCommandTest.php (1)
9-22: 🛠️ Refactor suggestionDuplicate test pattern - consider shared test infrastructure.
This is the third test file with an identical structure. The duplication across
ActionMakeCommandTest,BaseResourceMakeCommandTest, and this test suggests the need for a shared testing approach.
🧹 Nitpick comments (21)
src/Console/Commands/ActionMakeCommand.php (1)
49-54: Consider improving the resolveStubPath method implementation.The stub path resolution logic is functional but could be improved for better readability and error handling.
Consider this more explicit implementation:
protected function resolveStubPath($stub): string { - return file_exists($customPath = $this->laravel->basePath(trim($stub, '/'))) - ? $customPath - : __DIR__.$stub; + $customPath = $this->laravel->basePath(trim($stub, '/')); + + if (file_exists($customPath)) { + return $customPath; + } + + return __DIR__.$stub; }This improves readability and makes the logic flow clearer.
tests/Feature/Commands/ActionMakeCommandTest.php (2)
11-11: Consider using more explicit error handling instead of suppressing errors.The
@unlinksuppresses all errors, which could hide legitimate filesystem issues. Consider checking if the file exists before attempting to delete it.- @unlink(app_path('Rest/Actions/TestAction.php')); + $filePath = app_path('Rest/Actions/TestAction.php'); + if (file_exists($filePath)) { + unlink($filePath); + }
21-21: Add error handling for cleanup operations.The cleanup unlink operation could fail and leave test artifacts. Consider wrapping in a try-catch or using Laravel's File facade for better error handling.
- unlink(app_path('Rest/Actions/TestAction.php')); + $filePath = app_path('Rest/Actions/TestAction.php'); + if (file_exists($filePath)) { + unlink($filePath); + }tests/Feature/Commands/BaseResourceMakeCommandTest.php (1)
9-22: Consider creating a shared test helper to reduce code duplication.This test method follows an identical pattern to
ActionMakeCommandTest::test_make_action_command. Consider extracting the common logic into a base test class or helper method.protected function assertCommandGeneratesFile(string $command, array $arguments, string $expectedFilePath, string $expectedContent): void { if (file_exists($expectedFilePath)) { unlink($expectedFilePath); } $this->artisan($command, $arguments) ->assertOk() ->run(); $this->assertFileExists($expectedFilePath); $this->assertStringContainsString($expectedContent, file_get_contents($expectedFilePath)); if (file_exists($expectedFilePath)) { unlink($expectedFilePath); } }src/Console/Commands/BaseResourceMakeCommand.php (1)
70-73: Consider making the namespace configurable for flexibility.While the hardcoded namespace
Rest\Resourcesis appropriate for most use cases, consider making it configurable through a config file if this package needs to support different namespace structures.protected function getDefaultNamespace($rootNamespace): string { $namespace = config('rest.namespaces.resources', 'Rest\Resources'); return $rootNamespace.'\\'.$namespace; }tests/Feature/Commands/ResourceMakeCommandTest.php (1)
9-23: Solid test coverage with opportunity for improvement.The test covers the essential functionality well. Consider these enhancements for better maintainability:
public function test_make_resource_command() { - @unlink(app_path('Rest/Resources/TestResource.php')); + $filePath = app_path('Rest/Resources/TestResource.php'); + @unlink($filePath); $this ->artisan('rest:resource', ['name' => 'TestResource']) ->assertOk() ->run(); - $this->assertFileExists(app_path('Rest/Resources/TestResource.php')); - $this->assertStringContainsString('class TestResource extends Resource', file_get_contents(app_path('Rest/Resources/TestResource.php'))); - $this->assertStringContainsString('public static $model = \Model::class;', file_get_contents(app_path('Rest/Resources/TestResource.php'))); + $this->assertFileExists($filePath); + $content = file_get_contents($filePath); + $this->assertStringContainsString('class TestResource extends Resource', $content); + $this->assertStringContainsString('public static $model = \Model::class;', $content); - unlink(app_path('Rest/Resources/TestResource.php')); + unlink($filePath); }src/Console/Commands/ResponseMakeCommand.php (1)
1-67: Good implementation with minor consistency improvement needed.The command follows the established Make command pattern correctly. One small inconsistency: the
resolveStubPathmethod parameter lacks a type hint.For consistency with other Make commands like
BaseControllerMakeCommand, add a type hint:-protected function resolveStubPath($stub): string +protected function resolveStubPath(string $stub): stringtests/Feature/Commands/ControllerMakeCommandTest.php (1)
11-11: Consider using tearDown method for cleanup.The manual file cleanup using
@unlink()before each test could be better handled with atearDown()method to ensure cleanup happens regardless of test success or failure.+ protected function tearDown(): void + { + @unlink(app_path('Rest/Controllers/TestController.php')); + parent::tearDown(); + }Then remove the manual
@unlink()calls at the beginning of each test.Also applies to: 27-27
src/Console/Commands/ResourceMakeCommand.php (6)
71-71: Add missing type hint for consistency.The
$stubparameter should have a type hint to match the coding standards and for better IDE support.- protected function resolveStubPath($stub) + protected function resolveStubPath(string $stub): string
93-102: Consider the default model handling logic.When no model option is provided, setting
namespacedModelto'Model'might not be the intended behavior. This could result in an invalid import or class reference in the generated stub if the stub expects a fully qualified class name.Consider either:
- Making the model option required when the stub needs it
- Providing a more meaningful default value
- Adjusting the stub to handle the case when no model is specified
$replace = array_merge( $replace, [ - '{{ namespacedModel }}' => 'Model', + '{{ namespacedModel }}' => '// No model specified', ] );
155-158: Enhance user experience with better prompt handling.The current implementation always shows the model selection prompt when no options are provided. Consider making this more intuitive by checking if the model option specifically wasn't provided.
$model = select( - 'What model should this resource relies to? (Optional)', + 'What model should this resource rely on? (Optional)', $this->possibleModels(), + required: false );Note: Also fixed the grammatical error "relies to" → "rely on".
156-156: Fix grammatical error in user prompt.The prompt text contains a grammatical error: "relies to" should be "rely on".
- 'What model should this resource relies to? (Optional)', + 'What model should this resource rely on? (Optional)',
71-71: Add missing type hint for method parameter.The
$stubparameter should have a string type hint for consistency with other methods and better code documentation.- protected function resolveStubPath($stub) + protected function resolveStubPath(string $stub): string
87-115: Consider early return to reduce nesting.The buildClass method works correctly but could be more readable with early returns.
protected function buildClass($name): string { $rootNamespace = $this->rootNamespace(); $replace = []; - if ($this->option('model')) { - $replace = $this->buildModelReplacements($replace); - } else { - $replace = array_merge( - $replace, - [ - '{{ namespacedModel }}' => 'Model', - ] - ); - } + $replace = $this->option('model') + ? $this->buildModelReplacements($replace) + : array_merge($replace, ['{{ namespacedModel }}' => 'Model']); $baseResourceExists = file_exists($this->getPath("{$rootNamespace}Rest\Resources\Resource")); - if (!$baseResourceExists) { - $replace["use {$rootNamespace}Rest\Resources\Resource;\n"] = "use Lomkit\\Rest\\Http\\Resource;\n"; - } + if (!$baseResourceExists) { + $replace["use {$rootNamespace}Rest\Resources\Resource;\n"] = "use Lomkit\\Rest\\Http\\Resource;\n"; + } return str_replace( array_keys($replace), array_values($replace), parent::buildClass($name) ); }src/Console/Commands/ControllerMakeCommand.php (7)
74-74: Add missing type hint for consistency.Same issue as in ResourceMakeCommand - the
$stubparameter should have a type hint.- protected function resolveStubPath($stub) + protected function resolveStubPath(string $stub): string
160-160: Fix grammatical error and improve clarity.The prompt message has a grammatical error and doesn't clearly indicate this is for a controller.
- 'What resource should this resource relies to? (Optional)', + 'What resource should this controller rely on? (Optional)',
183-183: Fix incorrect documentation.The comment mentions "model names" but this method returns resource names.
- * Get a list of possible model names. + * Get a list of possible resource names.
213-213: Fix parameter name for consistency.The parameter is named
$modelbut should be$resourcesince this method handles resources, not models.- * @param string $model + * @param string $resource * * @throws \InvalidArgumentException * * @return string */ - protected function parseResource($model): string + protected function parseResource(string $resource): string { - if (preg_match('([^A-Za-z0-9_/\\\\])', $model)) { + if (preg_match('([^A-Za-z0-9_/\\\\])', $resource)) { throw new InvalidArgumentException('Resource name contains invalid characters.'); } - return $this->qualifyResource($model); + return $this->qualifyResource($resource); }
160-160: Fix confusing and grammatically incorrect prompt text.The prompt has multiple issues: grammatical error with "relies to" and confusing repetition of "resource".
- 'What resource should this resource relies to? (Optional)', + 'What resource should this controller use? (Optional)',
213-213: Fix inconsistent parameter naming.The parameter is named
$modelbut this method handles resources, not models. This is confusing and inconsistent with the method's purpose.- protected function parseResource($model): string + protected function parseResource($resource): string { - if (preg_match('([^A-Za-z0-9_/\\\\])', $model)) { + if (preg_match('([^A-Za-z0-9_/\\\\])', $resource)) { throw new InvalidArgumentException('Resource name contains invalid characters.'); } - return $this->qualifyResource($model); + return $this->qualifyResource($resource); }
74-74: Add missing type hint for method parameter.The
$stubparameter should have a string type hint for consistency and better code documentation.- protected function resolveStubPath($stub) + protected function resolveStubPath(string $stub): string
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
src/Console/Commands/ActionCommand.php(0 hunks)src/Console/Commands/ActionMakeCommand.php(1 hunks)src/Console/Commands/BaseControllerCommand.php(0 hunks)src/Console/Commands/BaseControllerMakeCommand.php(1 hunks)src/Console/Commands/BaseResourceCommand.php(0 hunks)src/Console/Commands/BaseResourceMakeCommand.php(1 hunks)src/Console/Commands/ControllerCommand.php(0 hunks)src/Console/Commands/ControllerMakeCommand.php(1 hunks)src/Console/Commands/Documentation/DocumentationCommand.php(1 hunks)src/Console/Commands/Documentation/DocumentationServiceProviderMakeCommand.php(1 hunks)src/Console/Commands/DocumentationProviderCommand.php(0 hunks)src/Console/Commands/InstructionCommand.php(0 hunks)src/Console/Commands/InstructionMakeCommand.php(1 hunks)src/Console/Commands/QuickStartCommand.php(2 hunks)src/Console/Commands/ResourceCommand.php(0 hunks)src/Console/Commands/ResourceMakeCommand.php(1 hunks)src/Console/Commands/ResponseCommand.php(0 hunks)src/Console/Commands/ResponseMakeCommand.php(1 hunks)src/Console/ResolvesStubPath.php(0 hunks)src/Console/stubs/base-controller.stub(1 hunks)src/Console/stubs/base-resource.stub(1 hunks)src/Console/stubs/controller.stub(1 hunks)src/Console/stubs/resource.stub(1 hunks)src/Console/stubs/rest-documentation-service-provider.stub(1 hunks)src/Console/stubs/user-controller.stub(0 hunks)src/Console/stubs/user-resource.stub(0 hunks)src/RestServiceProvider.php(5 hunks)tests/Feature/Commands/ActionCommandTest.php(0 hunks)tests/Feature/Commands/ActionMakeCommandTest.php(1 hunks)tests/Feature/Commands/BaseControllerCommandTest.php(0 hunks)tests/Feature/Commands/BaseControllerMakeCommandTest.php(1 hunks)tests/Feature/Commands/BaseResourceCommandTest.php(0 hunks)tests/Feature/Commands/BaseResourceMakeCommandTest.php(1 hunks)tests/Feature/Commands/ControllerCommandTest.php(0 hunks)tests/Feature/Commands/ControllerMakeCommandTest.php(1 hunks)tests/Feature/Commands/DocumentationCommandTest.php(1 hunks)tests/Feature/Commands/InstructionCommandTest.php(0 hunks)tests/Feature/Commands/QuickStartCommandTest.php(1 hunks)tests/Feature/Commands/ResourceCommandTest.php(0 hunks)tests/Feature/Commands/ResourceMakeCommandTest.php(1 hunks)tests/Feature/Commands/ResponseCommandTest.php(0 hunks)tests/Unit/LaravelScoutTest.php(1 hunks)tests/Unit/QueryBuilderTest.php(1 hunks)tests/Unit/RoutesTest.php(1 hunks)tests/Unit/StubsTest.php(1 hunks)
💤 Files with no reviewable changes (18)
- src/Console/Commands/BaseResourceCommand.php
- src/Console/stubs/user-controller.stub
- tests/Feature/Commands/ActionCommandTest.php
- tests/Feature/Commands/InstructionCommandTest.php
- tests/Feature/Commands/ResponseCommandTest.php
- tests/Feature/Commands/ResourceCommandTest.php
- src/Console/Commands/BaseControllerCommand.php
- tests/Feature/Commands/BaseControllerCommandTest.php
- src/Console/ResolvesStubPath.php
- tests/Feature/Commands/BaseResourceCommandTest.php
- tests/Feature/Commands/ControllerCommandTest.php
- src/Console/Commands/DocumentationProviderCommand.php
- src/Console/Commands/ResponseCommand.php
- src/Console/Commands/ActionCommand.php
- src/Console/Commands/ResourceCommand.php
- src/Console/stubs/user-resource.stub
- src/Console/Commands/InstructionCommand.php
- src/Console/Commands/ControllerCommand.php
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/Console/Commands/ResponseMakeCommand.php (6)
src/Console/Commands/ActionMakeCommand.php (4)
AsCommand(8-67)getStub(37-40)resolveStubPath(49-54)getDefaultNamespace(63-66)src/Console/Commands/BaseResourceMakeCommand.php (4)
AsCommand(8-74)getStub(44-47)resolveStubPath(56-61)getDefaultNamespace(70-73)src/Console/Commands/BaseControllerMakeCommand.php (4)
AsCommand(8-74)getStub(44-47)resolveStubPath(56-61)getDefaultNamespace(70-73)src/Console/Commands/InstructionMakeCommand.php (4)
AsCommand(8-67)getStub(37-40)resolveStubPath(49-54)getDefaultNamespace(63-66)src/Console/Commands/ControllerMakeCommand.php (4)
AsCommand(17-249)getStub(46-49)resolveStubPath(74-79)getDefaultNamespace(205-208)src/Console/Commands/ResourceMakeCommand.php (4)
AsCommand(14-206)getStub(43-46)resolveStubPath(71-76)getDefaultNamespace(184-187)
src/Console/Commands/QuickStartCommand.php (8)
src/Console/Commands/ActionMakeCommand.php (1)
resolveStubPath(49-54)src/Console/Commands/BaseResourceMakeCommand.php (1)
resolveStubPath(56-61)src/Console/Commands/Documentation/DocumentationServiceProviderMakeCommand.php (1)
resolveStubPath(62-67)src/Console/Commands/BaseControllerMakeCommand.php (1)
resolveStubPath(56-61)src/Console/Commands/ResponseMakeCommand.php (1)
resolveStubPath(49-54)src/Console/Commands/InstructionMakeCommand.php (1)
resolveStubPath(49-54)src/Console/Commands/ControllerMakeCommand.php (1)
resolveStubPath(74-79)src/Console/Commands/ResourceMakeCommand.php (1)
resolveStubPath(71-76)
src/Console/Commands/BaseControllerMakeCommand.php (7)
src/Console/Commands/ActionMakeCommand.php (4)
AsCommand(8-67)getStub(37-40)resolveStubPath(49-54)getDefaultNamespace(63-66)src/Console/Commands/BaseResourceMakeCommand.php (4)
AsCommand(8-74)getStub(44-47)resolveStubPath(56-61)getDefaultNamespace(70-73)src/Console/Commands/ResponseMakeCommand.php (4)
AsCommand(8-67)getStub(37-40)resolveStubPath(49-54)getDefaultNamespace(63-66)src/Console/Commands/InstructionMakeCommand.php (4)
AsCommand(8-67)getStub(37-40)resolveStubPath(49-54)getDefaultNamespace(63-66)src/Console/Commands/ControllerMakeCommand.php (4)
AsCommand(17-249)getStub(46-49)resolveStubPath(74-79)getDefaultNamespace(205-208)src/Console/Commands/Documentation/DocumentationCommand.php (1)
getStub(69-72)src/Console/Commands/QuickStartCommand.php (1)
resolveStubPath(158-163)
src/Console/Commands/ResourceMakeCommand.php (3)
src/Console/Commands/BaseResourceMakeCommand.php (4)
AsCommand(8-74)getStub(44-47)resolveStubPath(56-61)getDefaultNamespace(70-73)src/Console/Commands/ControllerMakeCommand.php (8)
AsCommand(17-249)getStub(46-49)resolveStubPath(74-79)handle(58-65)buildClass(90-118)afterPromptingForMissingArguments(154-168)getOptions(175-180)getDefaultNamespace(205-208)src/Console/Commands/QuickStartCommand.php (2)
resolveStubPath(158-163)handle(30-45)
🪛 GitHub Actions: tests
tests/Feature/Commands/DocumentationCommandTest.php
[error] 13-13: PHPUnit test failure: Failed asserting that file "./.phpunit.cache/openapi.json" exists.
🔇 Additional comments (40)
tests/Unit/QueryBuilderTest.php (2)
3-3: LGTM! Proper namespace standardization.The namespace addition aligns with the test suite reorganization and improves code structure.
11-11: LGTM! Explicit import improves code clarity.Adding the explicit Mockery import makes dependencies clear and follows good practices.
tests/Unit/RoutesTest.php (1)
3-3: LGTM! Consistent namespace standardization.The namespace addition aligns with the overall test suite reorganization effort.
tests/Unit/LaravelScoutTest.php (2)
3-3: LGTM! Proper namespace standardization.The namespace addition follows the established pattern for unit test organization.
12-12: LGTM! Explicit import improves code clarity.Making the Mockery dependency explicit is a good practice that improves code readability.
src/Console/Commands/InstructionMakeCommand.php (3)
8-8: LGTM! Modern command registration approach.Using PHP 8 attributes for command registration is the recommended modern approach in Laravel.
37-54: LGTM! Robust stub resolution implementation.The stub resolution logic correctly implements the fallback pattern:
- First checks for custom stubs in the Laravel base path
- Falls back to package default stubs
This allows users to override stubs while maintaining safe defaults.
63-66: LGTM! Appropriate default namespace configuration.The default namespace
Rest\Instructionsfollows logical conventions for organizing REST API instruction classes within the application structure.src/Console/Commands/Documentation/DocumentationCommand.php (1)
3-3: Namespace updated toDocumentationsubnamespace
The new namespace correctly reflects the command’s relocation into theDocumentationfolder, aligning with PSR-4 structure and the PR’s reorganization.src/Console/stubs/base-controller.stub (1)
7-7: Use templated placeholder for base controller class
Switching from a hardcodedControllerto{{ class }}makes the stub generic and consistent with the new generator command’s dynamic class naming.src/Console/stubs/rest-documentation-service-provider.stub (1)
11-11: Make service provider class name dynamic
Replacing the fixedRestDocumentationServiceProviderwith{{ class }}ensures the stub aligns with the newMakeCommandnaming conventions and supports overrideable stubs.src/Console/stubs/base-resource.stub (1)
7-7: Abstract resource stub now uses{{ class }}placeholder
This change generalizes the stub for any resource name and keeps inheritance fromRestResourceintact.src/Console/stubs/controller.stub (1)
5-7: Dynamic import and class placeholder for controllers
Using{{ rootNamespace }}in the import and{{ class }}for the class declaration ensures generated controllers extend the correct base controller under the configured root namespace.tests/Feature/Commands/QuickStartCommandTest.php (1)
64-65: LGTM: Method chaining formatting improved.The method chaining formatting is cleaner and more readable.
src/Console/stubs/resource.stub (3)
5-5: Good: Dynamic namespace placeholder improves flexibility.Using
{{ rootNamespace }}makes the stub more flexible and reusable across different application structures.
7-7: Consistent class extension change.Extending
Resourcedirectly instead of the aliasedRestResourceis cleaner and consistent with the import change.
1-79:✅ Verification successful
Verify the impact of removing the defaultLimit property.
According to the AI summary, the
defaultLimitproperty was removed from this stub. This could affect existing functionality if resources rely on this default pagination limit.Please verify if the removal of the
defaultLimitproperty affects existing functionality:
🏁 Script executed:
#!/bin/bash # Description: Check for references to defaultLimit in the codebase # Expected: Should help determine if this property is still needed echo "Searching for defaultLimit references:" rg "defaultLimit" --type php -A 3 -B 3 echo -e "\nSearching for pagination limit usage:" rg "limit.*50|50.*limit" --type php -A 2 -B 2 echo -e "\nChecking base Resource class for default limits:" ast-grep --pattern 'class Resource { $$$ defaultLimit $$$ }'Length of output: 5013
defaultLimit Removal Verified as Non-Breaking
The stub no longer declares
$defaultLimit, but the baseResourceclass still provides:
src/Http/Resource.php:public int $defaultLimit = 50;src/Concerns/Resource/Paginable.phpandsrc/Query/ScoutBuilder.phpboth fallback to$this->defaultLimit ?? 50- Tests in
tests/Support/Rest/Resources/ModelWithDefaultLimitResource.phpexplicitly override$defaultLimitSince every resource inherits the default from the base class and no references were broken, removing the stub’s
defaultLimitis safe.tests/Unit/StubsTest.php (1)
10-27: Excellent test coverage for stub registration.This test effectively verifies that all expected stub files are properly registered for publishing. The approach using the
PublishingStubsevent is correct and follows Laravel conventions.Key strengths:
- Comprehensive coverage of all stub files
- Correct use of
realpath()for absolute path comparison- Clear assertion structure
- Follows Laravel testing patterns
src/Console/Commands/ActionMakeCommand.php (3)
8-8: Good use of PHP 8 attributes for command registration.Using the
AsCommandattribute is a modern approach that improves discoverability and follows current Laravel best practices.
63-66: Appropriate default namespace for actions.The default namespace
Rest\Actionsfollows the package's naming conventions and provides good organization for generated action classes.
1-67: Well-implemented command class following Laravel conventions.This command class is well-structured and follows Laravel's
GeneratorCommandpatterns effectively. Key strengths:
- Extends the appropriate base class (
GeneratorCommand)- Uses modern PHP 8 attributes
- Implements required methods correctly
- Provides flexibility with custom stub paths
- Follows consistent naming conventions
tests/Feature/Commands/ActionMakeCommandTest.php (1)
18-19:❓ Verification inconclusive
Consider verifying directory existence before testing file creation.
The test assumes the target directory exists. Consider ensuring the directory structure is available or the command handles missing directories gracefully.
🏁 Script executed:
#!/bin/bash # Description: Check if the rest:action command creates directories when they don't exist # Search for directory creation logic in the ActionMakeCommand or GeneratorCommand rg -A 10 -B 5 "makeDirectory|ensureDirectoryExists|mkdir"Length of output: 1447
🏁 Script executed:
#!/bin/bash # Locate the ActionMakeCommand file (excluding tests) FILE=$(rg -l "class ActionMakeCommand" | grep -v "Test.php" | head -n 1) echo "Found file: $FILE" echo "---- First 200 lines of $FILE ----" sed -n '1,200p' "$FILE" echo "" echo "---- Search for makeDirectory calls in $FILE ----" grep -n "makeDirectory" "$FILE"Length of output: 2140
Ensure the
rest:actioncommand creates its target directoryWe weren’t able to locate a
makeDirectorycall inActionMakeCommanditself, nor confirm in your Laravel version’sGeneratorCommandwhether directories are auto-created. Please verify and/or address one of the following:
- Confirm that
Illuminate\Console\GeneratorCommand::handle()invokesmakeDirectory(dirname($path))before writing the stub in your framework version.- If it does not, add a call to
$this->makeDirectory($path);(or equivalent) inActionMakeCommand::handle()so theRest/Actionsfolder is created.- Alternatively, adjust your test to explicitly prepare or assert the
app/Rest/Actionsdirectory (e.g.$this->assertDirectoryExists(...)or create it via Filesystem) before checking for the file.src/Console/Commands/BaseResourceMakeCommand.php (3)
8-10: Good use of Laravel 9+ features.Excellent use of the
AsCommandattribute for command registration, which is a modern Laravel practice. The command properly extendsGeneratorCommandfor file generation functionality.
23-23: Appropriate use of hidden flag for base command.Setting the command as hidden is a good practice for base/infrastructure commands that shouldn't appear in the regular artisan command list.
56-61: Robust stub path resolution implementation.The stub path resolution properly checks for custom paths first before falling back to the default location. This allows for customization while maintaining sensible defaults.
src/Console/Commands/QuickStartCommand.php (2)
33-36: Good improvement: More explicit command parameters.The updated command calls with explicit
--model=Userand--resource=UserResourceoptions make the intent clearer and ensure proper linking between the generated resource and controller.
158-163:✅ Verification successful
Verify the unique stub path handling.
The
resolveStubPathimplementation here differs from other Make commands - it strips'rest/'from the stub path. Other commands in the codebase use the stub path directly without modification.Let me verify if this path modification is intentional:
🏁 Script executed:
#!/bin/bash # Description: Check if QuickStartCommand has different stub directory structure # Expected: QuickStartCommand might have legacy stub paths that need modification # Look for stub usage patterns in QuickStartCommand vs other commands echo "=== QuickStartCommand stub usage ===" rg -A 3 -B 3 "resolveStubPath.*rest/" src/Console/Commands/QuickStartCommand.php echo "=== Other MakeCommand stub usage patterns ===" rg -A 3 -B 3 "resolveStubPath" src/Console/Commands/*MakeCommand.php echo "=== Check if rest/ prefixed stubs exist ===" find . -name "*.stub" -path "*/rest/*" | head -10Length of output: 7910
QuickStartCommand’s stub path adjustment is intentional
After inspecting the codebase:
- QuickStartCommand’s
getStub()references a stub understubs/rest/..., but no actualrest/directory exists.- Its override of
resolveStubPathstrips therest/prefix so the fallback correctly points to__DIR__.'/../stubs/quickstart.stub'.- All other Make commands reference stubs directly (no
rest/), so they don’t need this special logic.No change is required here.
src/Console/Commands/BaseControllerMakeCommand.php (1)
1-74: Well-implemented Make command following established patterns.This new command implementation is excellent:
- Follows the consistent Make command pattern established in the codebase
- Appropriately marked as
$hidden = truesince it's a foundational command- Uses proper type hints and documentation
- Implements the standard stub resolution logic
The command aligns perfectly with other Make commands like
BaseResourceMakeCommand,ActionMakeCommand, etc.tests/Feature/Commands/ResourceMakeCommandTest.php (1)
25-39: Test method follows good patterns.The test with model option correctly verifies that the
--modelparameter properly updates the generated resource class to reference the specified model class.src/Console/Commands/Documentation/DocumentationServiceProviderMakeCommand.php (3)
38-41: LGTM! Clean stub resolution implementation.The stub resolution follows Laravel generator conventions properly, using the relative path approach.
48-53: Good argument configuration with sensible default.The command provides a sensible default name (
RestDocumentationServiceProvider) while allowing customization. The argument structure follows Laravel conventions.
62-67:✅ Verification successful
Verify the stub path resolution logic.
The custom path resolution logic checks for stubs in the Laravel base path first, then falls back to the package directory. Ensure this aligns with the stub registration in the service provider.
🏁 Script executed:
#!/bin/bash # Verify that the stub file exists in the expected location fd -t f "rest-documentation-service-provider.stub" --exec ls -laLength of output: 174
🏁 Script executed:
#!/bin/bash # Locate and preview the DocumentationServiceProviderMakeCommand file FILE=$(fd -t f "DocumentationServiceProviderMakeCommand.php") echo "File located at: $FILE" echo "----- File content (first 200 lines) -----" sed -n '1,200p' "$FILE"Length of output: 2457
Stub path resolution validated
The stub filerest-documentation-service-provider.stubexists atsrc/Console/stubs/, and__DIR__.'/../../stubs/rest-documentation-service-provider.stub'correctly resolves to that location when falling back. No changes are required here.tests/Feature/Commands/DocumentationCommandTest.php (1)
18-28: Good test structure for the documentation service provider command.The test properly handles file cleanup, verifies file creation, and checks the generated content. The implementation follows good testing practices.
src/RestServiceProvider.php (4)
122-131: Excellent stub registration implementation.The stub registration using Laravel's
PublishingStubsevent is well-implemented. It properly registers all stub files with meaningful names for the publishing mechanism.
103-112: Good command registration with new classes.The command registration has been properly updated to use the new "Make" command classes, maintaining consistency with the refactoring effort.
85-85: Good addition of explicit return types.Adding explicit return types to methods improves code clarity and type safety. This is a good practice for modern PHP development.
Also applies to: 99-99, 139-139, 153-153
5-18:✅ Verification successful
Verify all new command classes exist.
The imports have been updated to use new "Make" command classes. Ensure all these command classes have been implemented and are available.
🏁 Script executed:
#!/bin/bash # Verify all imported command classes exist fd -t f "ActionMakeCommand.php|BaseControllerMakeCommand.php|BaseResourceMakeCommand.php|ControllerMakeCommand.php|ResourceMakeCommand.php|ResponseMakeCommand.php|InstructionMakeCommand.php|DocumentationServiceProviderMakeCommand.php" src/Console/Commands/Length of output: 665
🏁 Script executed:
#!/bin/bash # Display registerStubs and registerCommands methods along with event listener in RestServiceProvider rg -n -C5 "registerStubs" src/RestServiceProvider.php rg -n -C5 "registerCommands" src/RestServiceProvider.php # Search for stub directory in the repository fd -t d stubLength of output: 1577
🏁 Script executed:
#!/bin/bash # List all commands registered in registerCommands sed -n '99,140p' src/RestServiceProvider.php # List all stub files to confirm registration ls -1 src/Console/stubsLength of output: 2084
🏁 Script executed:
#!/bin/bash # Verify QuickStartCommand and DocumentationCommand exist fd -t f "QuickStartCommand.php|DocumentationCommand.php" src/Console/Commands/Length of output: 183
All command classes and stubs verified – no issues found
All of the newly imported
*MakeCommandand documentation commands are present undersrc/Console/Commands(including theDocumentationsubfolder), and theregisterCommands()array matches these classes exactly.
All stub files listed inregisterStubs()exist insrc/Console/stubs, and thePublishingStubslistener is correctly wired to publish each stub.No further changes are needed here.
src/Console/Commands/ResourceMakeCommand.php (2)
55-62: LGTM! Well-implemented base resource creation.The handle method correctly ensures a base resource exists before generating the specific resource. The silent call prevents unnecessary output clutter.
124-139: LGTM! Comprehensive model placeholder replacements.The method provides all necessary placeholder replacements for different naming conventions (camelCase, PascalCase, etc.) which ensures compatibility with various stub templates.
src/Console/Commands/ControllerMakeCommand.php (1)
58-65: LGTM! Proper base controller setup.The handle method correctly ensures a base controller exists before generating the specific controller, following the same pattern as the ResourceMakeCommand.
|
|
||
| $this->assertFileExists(app_path('Rest/Resources/UserResource.php')); | ||
| $this->assertFileExists(app_path('Rest/Controllers/UsersController.php')); | ||
|
|
||
| $resourceContent = File::get(app_path('Rest/Resources/UserResource.php')); | ||
| $controllerContent = File::get(app_path('Rest/Controllers/UsersController.php')); | ||
|
|
||
| $this->assertStringContainsString('\App\Models\User::class', $resourceContent); | ||
| $this->assertStringContainsString('\App\User::class', $resourceContent); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the User model namespace change is consistent across the codebase.
The test now expects the generated resource to reference \App\User::class instead of \App\Models\User::class. This suggests a significant change in the default User model namespace assumption.
Please verify that this namespace change is consistent throughout the codebase and doesn't break existing applications that use App\Models\User:
Also applies to: 77-77
🏁 Script executed:
#!/bin/bash
# Description: Check for any remaining references to App\Models\User in the codebase
# Expected: Should find minimal or no references if the change is complete
echo "Searching for App\\Models\\User references:"
rg "App\\\\Models\\\\User" --type php
echo -e "\nSearching for App\\Models\\User references:"
rg "App\\Models\\User" --type php
echo -e "\nSearching for new App\\User references:"
rg "App\\\\User" --type php
echo -e "\nChecking QuickStartCommand implementation:"
rg -A 10 -B 5 "class QuickStartCommand" --type phpLength of output: 3287
Update QuickStartCommand to use the new User namespace
QuickStartCommand is still generating stubs that reference App\Models\User::class, while your tests now expect \App\User::class. This mismatches the namespace change and will break code generation.
Please update in
- src/Console/Commands/QuickStartCommand.php
Change the str_replace call from:
str_replace(
'App\Models\Model::class',
'App\Models\User::class',
file_get_contents($resource)
)to:
str_replace(
'App\Models\Model::class',
'\App\User::class',
file_get_contents($resource)
)This will align the generated code with your updated tests and the new default User model namespace.
🤖 Prompt for AI Agents
In tests/Feature/Commands/QuickStartCommandTest.php at line 73 and also line 77,
the test expects the User model namespace to be '\App\User::class' instead of
'App\Models\User::class'. To fix this inconsistency, update the
QuickStartCommand implementation in src/Console/Commands/QuickStartCommand.php
by changing the str_replace call to replace 'App\Models\Model::class' with
'\App\User::class' instead of 'App\Models\User::class'. This ensures the
generated stubs match the new User namespace expected by the tests.
There was a problem hiding this comment.
No, User model should be in models folder
There was a problem hiding this comment.
🧩 Analysis chain
@GautierDele I understand you want the User model to remain in the App\Models folder. Let me examine the current QuickStartCommand implementation to understand how it handles the User model namespace detection.
🏁 Script executed:
#!/bin/bash
# Examine the QuickStartCommand implementation to understand how it handles User model namespace
echo "=== QuickStartCommand implementation ==="
cat src/Console/Commands/QuickStartCommand.php
echo -e "\n=== Check if there's logic for detecting Models directory ==="
rg -A 5 -B 5 "Models" src/Console/Commands/QuickStartCommand.php
echo -e "\n=== Check ResourceMakeCommand for model namespace logic ==="
rg -A 10 -B 5 "getModelNamespace\|Models" src/Console/Commands/ResourceMakeCommand.php
Length of output: 6278
I can see the issue now! Looking at the QuickStartCommand implementation, there's logic that specifically checks for the User model location:
protected function updateUserModelNamespace()
{
$resource = app_path('Rest/Resources/UserResource.php');
if (file_exists(app_path('Models/User.php'))) {
file_put_contents(
$resource,
str_replace('App\Models\Model::class', 'App\Models\User::class', file_get_contents($resource))
);
}
// ... rest of the method
}The command is designed to:
- Check if
app/Models/User.phpexists - If it does exist: Replace
App\Models\Model::classwithApp\Models\User::class - If it doesn't exist: Leave the default stub content unchanged
The test change removed the setup that creates the Models directory and User.php file, but then expects \App\User::class. This seems incorrect.
Let me check what the default stub content contains when no Models/User.php exists:
The test should be updated to match your preference. Since you want the User model in the Models folder, the test should:
- Keep the setup that creates
app/Models/User.php - Expect
App\Models\User::classin the assertions
This would align with both:
- Your preference for User model to be in the Models folder
- The existing QuickStartCommand logic that detects and uses
App\Models\User::classwhen the file exists
🏁 Script executed:
#!/bin/bash
# Check the resource stub content to see what the default model reference is
echo "=== Resource stub content ==="
cat src/Console/stubs/resource.stub | grep -A 5 -B 5 "model\|Model"
echo -e "\n=== Check if there are any references to default model in stubs ==="
rg -i "model" src/Console/stubs/resource.stub
Length of output: 890
Perfect! Now I can see the issue clearly. The resource stub uses \{{ namespacedModel }}::class as a placeholder. Let me check how the ResourceMakeCommand processes the --model parameter to understand what gets generated initially:
Based on what I can see so far:
- QuickStartCommand calls
rest:resourcewith--model => 'User' - ResourceMakeCommand should process this and replace
{{ namespacedModel }}in the stub - QuickStartCommand then has additional logic to check if
Models/User.phpexists and update the namespace accordingly
The test change seems incorrect because it removed the setup that creates the Models directory structure, but you want User models to be in the Models folder. The test should be reverted to:
- Create the Models directory and User.php file in setup
- Expect
App\Models\User::classin the assertions
This would match your preference and the existing command logic.
🏁 Script executed:
#!/bin/bash
# Check how ResourceMakeCommand handles the model parameter and namespacedModel replacement
echo "=== ResourceMakeCommand model handling ==="
rg -A 10 -B 5 "namespacedModel\|parseModel" src/Console/Commands/ResourceMakeCommand.php
echo -e "\n=== Check for model namespace logic in ResourceMakeCommand ==="
rg -A 15 -B 5 "getModelNamespace\|Model.*namespace" src/Console/Commands/ResourceMakeCommand.php
Length of output: 425
🏁 Script executed:
#!/bin/bash
# Check the full ResourceMakeCommand implementation to understand model handling
echo "=== Full ResourceMakeCommand implementation ==="
cat src/Console/Commands/ResourceMakeCommand.php
Length of output: 5523
Now I understand the full flow! Let me check what Laravel's qualifyModel() method does (which ResourceMakeCommand inherits):
Based on this analysis, the test change is actually correct for the scenario where no Models directory exists. However, you want User models to be in the Models folder.
The real issue is that the test should be testing the intended behavior where Models directory exists, not the fallback behavior.
Recommendation: Revert the test to create the Models directory and expect App\Models\User::class, which matches your preference for User models to be in the Models folder.
🏁 Script executed:
#!/bin/bash
# Check what the qualifyModel method does in Laravel's GeneratorCommand
echo "=== Check if there's a Models directory logic in Laravel ==="
rg -A 10 -B 5 "qualifyModel" src/Console/Commands/ || echo "No qualifyModel found in local files"
echo -e "\n=== Let's simulate what happens when QuickStartCommand runs ==="
echo "1. QuickStartCommand calls: rest:resource UserResource --model=User"
echo "2. ResourceMakeCommand.parseModel('User') calls this.qualifyModel('User')"
echo "3. Laravel's qualifyModel() checks if app/Models directory exists"
echo "4. If Models directory exists: returns 'App\\Models\\User'"
echo "5. If Models directory doesn't exist: returns 'App\\User'"
echo "6. This gets substituted into the stub as {{ namespacedModel }}"
echo "7. QuickStartCommand.updateUserModelNamespace() runs afterward"
echo -e "\n=== Let's see what the test is actually testing ==="
echo "The test removed the Models directory setup, so:"
echo "- qualifyModel('User') would return 'App\\User' (no Models folder)"
echo "- Stub gets generated with 'App\\User::class'"
echo "- updateUserModelNamespace() checks for Models/User.php (doesn't exist)"
echo "- No replacement happens"
echo "- Result: ResourceContent contains 'App\\User::class'"
Length of output: 2722
Perfect! Now I have the complete picture. Here's what's happening:
Current Flow Analysis
- QuickStartCommand calls
rest:resource UserResource --model=User - ResourceMakeCommand uses Laravel's inherited
qualifyModel('User')method - Laravel's qualifyModel() checks if
app/Modelsdirectory exists:- ✅ If Models directory exists: returns
'App\Models\User' - ❌ If Models directory doesn't exist: returns
'App\User'(fallback)
- ✅ If Models directory exists: returns
- This value gets substituted into the resource stub as
{{ namespacedModel }} - QuickStartCommand.updateUserModelNamespace() runs afterward as a safety check
The Test Issue
The test change removed the Models directory setup, so it's now testing the fallback behavior (no Models directory) rather than the intended behavior (Models directory exists).
Since you want User models to be in the Models folder, the test should be reverted to:
public function test_quick_start_command_updates_user_model_namespace()
{
// Simulate the existence of App\Models\User (RESTORE THIS)
File::makeDirectory(app_path('Models'), 0755, true);
File::put(app_path('Models/User.php'), '<?php namespace App\Models; class User {}');
$this->artisan('rest:quick-start')->assertExitCode(0);
$this->assertFileExists(app_path('Rest/Resources/UserResource.php'));
$this->assertFileExists(app_path('Rest/Controllers/UsersController.php'));
$resourceContent = File::get(app_path('Rest/Resources/UserResource.php'));
$controllerContent = File::get(app_path('Rest/Controllers/UsersController.php'));
// RESTORE: Expect Models namespace
$this->assertStringContainsString('\App\Models\User::class', $resourceContent);
$this->assertStringContainsString('\App\Rest\Resources\UserResource::class', $controllerContent);
$this->assertStringContainsString('public static $model = \App\Models\User::class;', $resourceContent);
}This will test the intended behavior where User models are properly placed in the Models folder, which aligns with your preference and Laravel's modern conventions.
🧬 Code Graph Analysis Results
Possibly relevant snippets from tests/Feature/TestCase.php (lines 10-26)
class TestCase extends BaseTestCase
{
use InteractsWithAuthorization;
use InteractsWithResource;
protected function setUp(): void
{
parent::setUp();
$this->withAuthenticatedUser();
}
protected function resolveAuthFactoryClass()
{
return UserFactory::class;
}
}This snippet shows the base test case class used by QuickStartCommandTest. It includes setup logic that authenticates a user before tests run and references a UserFactory class, which may relate to the User model location or namespace.
[no other relevant code snippets]
✏️ Learnings added
Learnt from: GautierDele
PR: Lomkit/laravel-rest-api#173
File: tests/Feature/Commands/QuickStartCommandTest.php:73-73
Timestamp: 2025-06-02T17:46:22.528Z
Learning: User GautierDele prefers User models to be in the App\Models folder, not App\ root. The QuickStartCommand should generate resources that reference App\Models\User::class when the Models directory exists.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| $this->assertStringContainsString('class TestController extends Controller', file_get_contents(app_path('Rest/Controllers/TestController.php'))); | ||
| $this->assertStringContainsString('public static $resource = \Resource::class;', file_get_contents(app_path('Rest/Controllers/TestController.php'))); |
There was a problem hiding this comment.
Fix inconsistent resource class reference.
There's an inconsistency in the resource class assertion. Line 20 expects \Resource::class but line 36 expects \App\Rest\Resources\TestResource::class. The first test should likely expect a more specific or consistent format.
- $this->assertStringContainsString('public static $resource = \Resource::class;', file_get_contents(app_path('Rest/Controllers/TestController.php')));
+ $this->assertStringContainsString('public static $resource = Resource::class;', file_get_contents(app_path('Rest/Controllers/TestController.php')));Or verify what the actual generated content should be by checking the stub template.
📝 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.
| $this->assertStringContainsString('class TestController extends Controller', file_get_contents(app_path('Rest/Controllers/TestController.php'))); | |
| $this->assertStringContainsString('public static $resource = \Resource::class;', file_get_contents(app_path('Rest/Controllers/TestController.php'))); | |
| $this->assertStringContainsString('class TestController extends Controller', file_get_contents(app_path('Rest/Controllers/TestController.php'))); | |
| $this->assertStringContainsString('public static $resource = Resource::class;', file_get_contents(app_path('Rest/Controllers/TestController.php'))); |
🤖 Prompt for AI Agents
In tests/Feature/Commands/ControllerMakeCommandTest.php around lines 19 to 20,
the assertion for the resource class reference is inconsistent with later
assertions and likely incorrect. Update the expected string in the assertion on
line 20 to match the fully qualified resource class name, such as
'\App\Rest\Resources\TestResource::class', ensuring consistency with the actual
generated controller content and other tests. Verify the correct expected value
by reviewing the stub template used for generation.
There was a problem hiding this comment.
@GautierDele Thank you for the clarification! I understand now that the different resource class references are intentional and represent the expected behavior based on whether the --resource option is provided or not.
🐰
☁️
( ..)
o(")(")
✏️ Learnings added
Learnt from: GautierDele
PR: Lomkit/laravel-rest-api#173
File: tests/Feature/Commands/ControllerMakeCommandTest.php:19-0
Timestamp: 2025-06-02T17:45:40.445Z
Learning: In Laravel REST API controller generation tests, the resource class assertions are expected to be different:
- Without --resource option: expects `\Resource::class` (generic/default)
- With --resource option: expects fully qualified class name like `\App\Rest\Resources\TestResource::class`
This reflects the actual behavior of the command generator and should not be flagged as inconsistent.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| protected function getOptions(): array | ||
| { | ||
| return [ | ||
| ['resource', null, InputOption::VALUE_NONE, 'The resource associated with this resource'], |
There was a problem hiding this comment.
Fix option configuration and description.
Two issues: the option should accept a value, and the description incorrectly says "resource associated with this resource".
- ['resource', null, InputOption::VALUE_NONE, 'The resource associated with this resource'],
+ ['resource', null, InputOption::VALUE_OPTIONAL, 'The resource associated with this controller'],🤖 Prompt for AI Agents
In src/Console/Commands/ControllerMakeCommand.php at line 178, the 'resource'
option is incorrectly configured to not accept a value and has a misleading
description. Change the option to accept a value by replacing
InputOption::VALUE_NONE with InputOption::VALUE_REQUIRED, and update the
description to clearly explain what the resource option represents, avoiding the
redundant phrase "resource associated with this resource."
Fix incorrect option definition.
The resource option is defined with InputOption::VALUE_NONE which means it cannot accept a value, but the code expects to receive a resource name as the option value.
- ['resource', null, InputOption::VALUE_NONE, 'The resource associated with this resource'],
+ ['resource', null, InputOption::VALUE_OPTIONAL, 'The resource associated with this controller'],Also note the description has been corrected from "resource associated with this resource" to "resource associated with this controller".
🤖 Prompt for AI Agents
In src/Console/Commands/ControllerMakeCommand.php at line 178, the 'resource'
option is incorrectly defined with InputOption::VALUE_NONE, which means it does
not accept a value, but the code expects a resource name as its value. Change
the option definition to use InputOption::VALUE_REQUIRED to allow passing a
resource name. Also, update the option description to "resource associated with
this controller" for clarity.
| { | ||
| $resourceClass = $this->parseResource($this->option('resource')); | ||
|
|
||
| $replace = []; |
There was a problem hiding this comment.
Critical bug: Replace array is being reset.
The $replace array is being reset to an empty array, which completely ignores the input parameter. This will cause any existing replacements to be lost.
- $replace = [];Remove this line entirely since the $replace parameter should be used and built upon.
📝 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.
| $replace = []; |
🤖 Prompt for AI Agents
In src/Console/Commands/ControllerMakeCommand.php at line 131, the $replace
array is being reset to an empty array, which discards any input replacements
passed to the method. Remove the line that sets $replace to an empty array so
that the method uses and builds upon the existing $replace parameter as
intended.
Remove redundant variable assignment.
The $replace = []; assignment overwrites the method parameter, making the parameter meaningless.
protected function buildResourceReplacements(array $replace): array
{
$resourceClass = $this->parseResource($this->option('resource'));
- $replace = [];
return array_merge($replace, [📝 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.
| $replace = []; |
🤖 Prompt for AI Agents
In src/Console/Commands/ControllerMakeCommand.php at line 131, remove the
redundant assignment of $replace to an empty array since it overwrites the
method parameter and makes it meaningless. Instead, use the $replace parameter
as passed to the method without reassigning it.
| $resourcePath = app_path('Rest/Resources'); | ||
|
|
||
| return (new Collection(Finder::create()->files()->depth(0)->in($resourcePath))) | ||
| ->map(fn ($file) => $file->getBasename('.php')) | ||
| ->sort() | ||
| ->values() | ||
| ->all(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing directory.
The possibleResources() method assumes the resources directory exists. If it doesn't exist, the Finder will throw an exception.
protected function possibleResources(): array
{
$resourcePath = app_path('Rest/Resources');
+
+ if (!is_dir($resourcePath)) {
+ return [];
+ }
return (new Collection(Finder::create()->files()->depth(0)->in($resourcePath)))
->map(fn ($file) => $file->getBasename('.php'))
->sort()
->values()
->all();
}📝 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.
| $resourcePath = app_path('Rest/Resources'); | |
| return (new Collection(Finder::create()->files()->depth(0)->in($resourcePath))) | |
| ->map(fn ($file) => $file->getBasename('.php')) | |
| ->sort() | |
| ->values() | |
| ->all(); | |
| protected function possibleResources(): array | |
| { | |
| $resourcePath = app_path('Rest/Resources'); | |
| if (!is_dir($resourcePath)) { | |
| return []; | |
| } | |
| return (new Collection(Finder::create()->files()->depth(0)->in($resourcePath))) | |
| ->map(fn ($file) => $file->getBasename('.php')) | |
| ->sort() | |
| ->values() | |
| ->all(); | |
| } |
🤖 Prompt for AI Agents
In src/Console/Commands/ControllerMakeCommand.php around lines 189 to 195, the
code assumes the resources directory exists, but if it is missing, Finder throws
an exception. Add a check to verify if the $resourcePath directory exists before
using Finder. If it does not exist, return an empty array or handle the case
gracefully to prevent exceptions.
| $resourcePath = app_path('Rest/Resources'); | ||
|
|
||
| return (new Collection(Finder::create()->files()->depth(0)->in($resourcePath))) | ||
| ->map(fn ($file) => $file->getBasename('.php')) | ||
| ->sort() | ||
| ->values() | ||
| ->all(); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Handle missing directory gracefully.
The possibleResources() method assumes the app_path('Rest/Resources') directory exists, which could cause an exception if it doesn't.
Consider adding a directory existence check:
protected function possibleResources(): array
{
$resourcePath = app_path('Rest/Resources');
+ if (!is_dir($resourcePath)) {
+ return [];
+ }
+
return (new Collection(Finder::create()->files()->depth(0)->in($resourcePath)))
->map(fn ($file) => $file->getBasename('.php'))
->sort()
->values()
->all();
}🏁 Script executed:
#!/bin/bash
# Description: Check if the Finder class handles missing directories gracefully
# Search for Finder usage patterns in Laravel core to understand expected behavior
rg -A 5 -B 5 "Finder::create.*in.*" --type phpLength of output: 901
Check for missing Rest/Resources directory before using Finder
The possibleResources() method will throw an exception if the app/Rest/Resources folder doesn’t exist. Let’s guard against that and return an empty list when the directory is missing.
• File: src/Console/Commands/ControllerMakeCommand.php
• Method: possibleResources()
Suggested diff:
protected function possibleResources(): array
{
$resourcePath = app_path('Rest/Resources');
+ if (! is_dir($resourcePath)) {
+ return [];
+ }
+
return (new Collection(Finder::create()->files()->depth(0)->in($resourcePath)))
->map(fn ($file) => $file->getBasename('.php'))
->sort()
->values()
->all();
}📝 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.
| $resourcePath = app_path('Rest/Resources'); | |
| return (new Collection(Finder::create()->files()->depth(0)->in($resourcePath))) | |
| ->map(fn ($file) => $file->getBasename('.php')) | |
| ->sort() | |
| ->values() | |
| ->all(); | |
| } | |
| protected function possibleResources(): array | |
| { | |
| $resourcePath = app_path('Rest/Resources'); | |
| if (! is_dir($resourcePath)) { | |
| return []; | |
| } | |
| return (new Collection(Finder::create()->files()->depth(0)->in($resourcePath))) | |
| ->map(fn ($file) => $file->getBasename('.php')) | |
| ->sort() | |
| ->values() | |
| ->all(); | |
| } |
🤖 Prompt for AI Agents
In src/Console/Commands/ControllerMakeCommand.php around lines 189 to 196, the
possibleResources() method uses Finder on the app_path('Rest/Resources')
directory without checking if it exists, which can cause an exception if the
directory is missing. Fix this by adding a check to see if the directory exists
before using Finder; if it does not exist, return an empty array immediately to
avoid errors.
Summary by CodeRabbit
New Features
*MakeCommandclasses for generating actions, controllers, resources, instructions, responses, and documentation service providers.Bug Fixes
Tests
Refactor
Documentation