feat: 支持插件生成模型文件到插件目录下 #479
Conversation
dosubot
bot
added
the
size:L
WalkthroughThe changes introduce a new command class Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 3
🧹 Outside diff range and nitpick comments (1)
app/Command/PluginGenModelCommand.php (1)
47-170: Ensure consistent language in error messagesThe error messages are a mix of Chinese and English (e.g., '插件名称不能为空', 'Invalid plugin config'). For better consistency and maintainability, consider using the same language throughout all error messages.
| if (empty($columns)) { | ||
| $this->output?->error( | ||
| sprintf('Query columns empty, maybe is table `%s` does not exist.You can check it in database.', $table) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Handle the case when columns are empty by exiting the method or throwing an exception
When no columns are found in the table, you output an error message but continue execution. If the intention is to stop processing when no columns are found, consider returning early or throwing an exception to prevent unnecessary execution.
Apply this diff to return early after the error message:
if (empty($columns)) {
$this->output?->error(
sprintf('Query columns empty, maybe table `%s` does not exist. You can check it in the database.', $table)
);
+ return;
}📝 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.
| if (empty($columns)) { | |
| $this->output?->error( | |
| sprintf('Query columns empty, maybe is table `%s` does not exist.You can check it in database.', $table) | |
| ); | |
| } | |
| if (empty($columns)) { | |
| $this->output?->error( | |
| sprintf('Query columns empty, maybe is table `%s` does not exist.You can check it in database.', $table) | |
| ); | |
| return; | |
| } |
| if (! is_file($jsonPath)) { | ||
| $this->output?->error('插件缺少配置文件'); | ||
| exit(); | ||
| } |
There was a problem hiding this comment.
Avoid using exit() inside methods; use exceptions or return statements instead
Consistently using exit() within methods can lead to abrupt termination of the script. It's better practice to throw an exception or handle errors in a way that can be managed by the calling code.
Apply this diff to replace exit() with throwing an exception:
if (! is_file($jsonPath)) {
$this->output?->error('插件缺少配置文件');
- exit();
+ throw new RuntimeException('插件缺少配置文件');
}Committable suggestion skipped: line range outside the PR's diff.
| if (! $this->input->getOption('plugin')) { | ||
| $this->output?->error('插件名称不能为空'); | ||
| exit(); | ||
| } |
There was a problem hiding this comment.
Avoid using exit() inside methods; use exceptions or return statements instead
Using exit() within a method can abruptly terminate the entire script, which is generally discouraged in object-oriented programming. It's better to throw an exception or return early to allow the application to handle the error gracefully.
Apply this diff to replace exit() with throwing an exception:
if (! $this->input->getOption('plugin')) {
$this->output?->error('插件名称不能为空');
- exit();
+ throw new RuntimeException('插件名称不能为空');
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (4)
app/Command/PluginGenModelCommand.php (4)
131-135: Add type hinting to method parameters for clarity and type safetyThe method
getPluginModelClassdoes not specify the type of the$classparameter. Adding a type declaration improves code readability and helps prevent type-related errors.Modify the method signature:
- private function getPluginModelClass($class): string + private function getPluginModelClass(string $class): string
153-154: Correct the DocBlock formatting for proper documentation parsingThe closing
*/of the DocBlock is misplaced, which can cause issues with documentation tools and IDE parsing.Adjust the DocBlock:
* @return string - * @noinspection PhpSameParameterValueInspection*/ + * @noinspection PhpSameParameterValueInspection + */
60-61: Improve error message grammar for better comprehensionThe error message contains grammatical errors that could confuse users. Refining the message enhances clarity.
Revise the error message:
sprintf('Query columns empty, maybe is table `%s` does not exist.You can check it in database.', $table) +sprintf('Query columns are empty. Maybe the table `%s` does not exist. You can check it in the database.', $table)
167-181: Simplify and ensure correctness in path manipulation logicThe method
getPluginNamespacehas complex logic that could be simplified for better readability and maintenance. Ensuring the correctness of path handling is crucial, especially when dealing with file systems.Consider refactoring the method to make the path manipulation clearer and more robust. For example:
-$ext = pathinfo($path, PATHINFO_EXTENSION); -if ($ext !== '') { - $path = substr($path, 0, -(strlen($ext) + 1)); -} else { - $path = trim($path, '/') . '/'; -} -return str_replace('/', '\\', $path); +$path = rtrim(str_replace('/', '\\', $path), '\\') . '\\'; +return $path;
| foreach ($psrArray as $value => $key) { | ||
| if ($key === 'src') { | ||
| $this->pluginModelNamespace = $value . 'Model'; | ||
| } | ||
| } |
There was a problem hiding this comment.
Correct the foreach loop variable names to properly extract namespace and path
In the foreach loop, the variables $value and $key are reversed, leading to incorrect assignment of the namespace. The correct structure should have $key as the namespace and $value as the path.
Update the loop:
-foreach ($psrArray as $value => $key) {
- if ($key === 'src') {
- $this->pluginModelNamespace = $value . 'Model';
+foreach ($psrArray as $namespace => $path) {
+ if ($path === 'src') {
+ $this->pluginModelNamespace = $namespace . 'Model';
}
}📝 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.
| foreach ($psrArray as $value => $key) { | |
| if ($key === 'src') { | |
| $this->pluginModelNamespace = $value . 'Model'; | |
| } | |
| } | |
| foreach ($psrArray as $namespace => $path) { | |
| if ($path === 'src') { | |
| $this->pluginModelNamespace = $namespace . 'Model'; | |
| } | |
| } |
| $this->output?->error('插件缺少配置文件'); | ||
| exit(); |
There was a problem hiding this comment.
Replace exit() with an exception for consistent error handling
Similar to previous instances, using exit() is not recommended. Throwing an exception ensures the error can be caught and managed appropriately.
Change the code to:
if (! is_file($jsonPath)) {
$this->output?->error('插件缺少配置文件');
- exit();
+ throw new RuntimeException('插件缺少配置文件');
}Committable suggestion skipped: line range outside the PR's diff.
| if (! $this->input->getOption('plugin')) { | ||
| $this->output?->error('插件名称不能为空'); | ||
| exit(); | ||
| } |
There was a problem hiding this comment.
Avoid using exit(); throw an exception for better error handling
Using exit() within methods is discouraged as it abruptly ends script execution and can lead to unexpected behavior. Instead, throwing an exception allows for proper error handling and keeps the application flow manageable.
Consider modifying the code as follows:
if (! $this->input->getOption('plugin')) {
$this->output?->error('插件名称不能为空');
- exit();
+ throw new RuntimeException('插件名称不能为空');
}📝 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.
| if (! $this->input->getOption('plugin')) { | |
| $this->output?->error('插件名称不能为空'); | |
| exit(); | |
| } | |
| if (! $this->input->getOption('plugin')) { | |
| $this->output?->error('插件名称不能为空'); | |
| throw new RuntimeException('插件名称不能为空'); | |
| } |
| if (empty($columns)) { | ||
| $this->output?->error( | ||
| sprintf('Query columns empty, maybe is table `%s` does not exist.You can check it in database.', $table) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add return or throw an exception after error message to prevent unintended execution
After displaying the error message when columns are empty, the method continues execution, which may cause further errors. You should halt execution by returning early or throwing an exception.
Suggested fix:
if (empty($columns)) {
$this->output?->error(
sprintf('Query columns empty. Maybe the table `%s` does not exist. You can check it in the database.', $table)
);
+ return;
}📝 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.
| if (empty($columns)) { | |
| $this->output?->error( | |
| sprintf('Query columns empty, maybe is table `%s` does not exist.You can check it in database.', $table) | |
| ); | |
| } | |
| if (empty($columns)) { | |
| $this->output?->error( | |
| sprintf('Query columns empty, maybe is table `%s` does not exist.You can check it in database.', $table) | |
| ); | |
| return; | |
| } |
| $jsonArray = json_decode(file_get_contents($jsonPath), true); | ||
| $psrArray = $jsonArray['composer']['psr-4']; | ||
|
|
There was a problem hiding this comment.
Validate JSON structure before accessing nested keys to prevent errors
Accessing $jsonArray['composer']['psr-4'] without checking if these keys exist can lead to undefined index errors. Ensure the JSON structure contains the expected keys.
Implement validation:
+$composerConfig = $jsonArray['composer'] ?? null;
+if (!$composerConfig || !isset($composerConfig['psr-4'])) {
+ throw new RuntimeException('Invalid composer configuration in plugin JSON.');
+}
-$psrArray = $jsonArray['composer']['psr-4'];
+$psrArray = $composerConfig['psr-4'];📝 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.
| $jsonArray = json_decode(file_get_contents($jsonPath), true); | |
| $psrArray = $jsonArray['composer']['psr-4']; | |
| $jsonArray = json_decode(file_get_contents($jsonPath), true); | |
| $composerConfig = $jsonArray['composer'] ?? null; | |
| if (!$composerConfig || !isset($composerConfig['psr-4'])) { | |
| throw new RuntimeException('Invalid composer configuration in plugin JSON.'); | |
| } | |
| $psrArray = $composerConfig['psr-4']; | |
|
Summary by CodeRabbit
New Features
Bug Fixes