feat: add snippet generator (WIP) - #89
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@classes/local/generators/snippets/base.php`:
- Around line 45-47: Update the constructor to validate the result of
component::resolve_component_from_path() before assigning it to the typed
$component property. If resolution returns null, throw an appropriate exception;
otherwise preserve the existing assignment behavior.
- Around line 114-118: Update namespace extraction around $classesdir and
$dirpath so the classes directory itself resolves to an empty relative
namespace, while nested directories retain their relative path. Normalize the
directory-boundary comparison before removing the prefix, and ensure paths such
as classes_extra are not treated as descendants; preserve the existing separator
conversion and component namespace construction.
In `@classes/local/generators/snippets/task.php`:
- Line 48: Update the call in the task generator to invoke the inherited
protected instance method php_file_with_namespaced_class() through $this rather
than self::, while preserving the existing destructuring assignment.
- Around line 60-66: Update the generated task class attribute setup in the
snippet generator so the execute method’s Override attribute is emitted as
global \Override rather than a namespace-relative Override; change only the
addAttribute call associated with $execute and leave the getname generation
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dd7675ac-c43b-466a-81f2-dd940a1519a4
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
classes/local/generators/snippets/base.phpclasses/local/generators/snippets/task.phpcomposer.json
| public function __construct(string $filepath) { | ||
| $this->filepath = utils::get_path_relative_to_moodle_root($filepath); | ||
| $this->component = component::resolve_component_from_path($this->filepath); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Handle unresolved components to prevent type errors.
The static analysis tool correctly flags that $component does not accept null. If component::resolve_component_from_path() fails to resolve a component, assigning null to a typed string property will trigger a runtime TypeError. Throw an exception to safely halt execution instead.
🛡️ Proposed fix
public function __construct(string $filepath) {
$this->filepath = utils::get_path_relative_to_moodle_root($filepath);
- $this->component = component::resolve_component_from_path($this->filepath);
+ $component = component::resolve_component_from_path($this->filepath);
+ if ($component === null) {
+ throw new \InvalidArgumentException("Could not resolve component for path: $this->filepath");
+ }
+ $this->component = $component;
$year = date("Y");📝 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.
| public function __construct(string $filepath) { | |
| $this->filepath = utils::get_path_relative_to_moodle_root($filepath); | |
| $this->component = component::resolve_component_from_path($this->filepath); | |
| public function __construct(string $filepath) { | |
| $this->filepath = utils::get_path_relative_to_moodle_root($filepath); | |
| $component = component::resolve_component_from_path($this->filepath); | |
| if ($component === null) { | |
| throw new \InvalidArgumentException("Could not resolve component for path: $this->filepath"); | |
| } | |
| $this->component = $component; | |
| $year = date("Y"); |
🧰 Tools
🪛 GitHub Check: test (8.3, MOODLE_405_STABLE, mariadb)
[notice] 47-47: phpstan/assign.propertyType
Property local_devkit\local\generators\snippets\base::$component (string) does not accept string|null.
🪛 GitHub Check: test (8.3, MOODLE_500_STABLE, mariadb)
[notice] 47-47: phpstan/assign.propertyType
Property local_devkit\local\generators\snippets\base::$component (string) does not accept string|null.
🪛 GitHub Check: test (8.3, MOODLE_501_STABLE, mariadb)
[notice] 47-47: phpstan/assign.propertyType
Property local_devkit\local\generators\snippets\base::$component (string) does not accept string|null.
🪛 GitHub Check: test (8.3, MOODLE_502_STABLE, mariadb)
[notice] 47-47: phpstan/assign.propertyType
Property local_devkit\local\generators\snippets\base::$component (string) does not accept string|null.
🪛 GitHub Check: test (8.4, MOODLE_500_STABLE, mariadb)
[notice] 47-47: phpstan/assign.propertyType
Property local_devkit\local\generators\snippets\base::$component (string) does not accept string|null.
🪛 GitHub Check: test (8.4, MOODLE_501_STABLE, mariadb)
[notice] 47-47: phpstan/assign.propertyType
Property local_devkit\local\generators\snippets\base::$component (string) does not accept string|null.
🪛 GitHub Check: test (8.4, MOODLE_502_STABLE, mariadb)
[notice] 47-47: phpstan/assign.propertyType
Property local_devkit\local\generators\snippets\base::$component (string) does not accept string|null.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@classes/local/generators/snippets/base.php` around lines 45 - 47, Update the
constructor to validate the result of component::resolve_component_from_path()
before assigning it to the typed $component property. If resolution returns
null, throw an appropriate exception; otherwise preserve the existing assignment
behavior.
Source: Linters/SAST tools
| $classesdir = "$componentpath/classes/"; | ||
| $namespace = str_replace($classesdir, '', $dirpath); | ||
| $namespace = str_replace('/', '\\', $namespace); | ||
| $namespace = "$this->component\\$namespace"; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix namespace resolution for files placed directly in the classes directory.
Because $classesdir includes a trailing slash, str_replace will fail to match if the target file is placed directly inside the classes directory (e.g., $dirpath is .../classes without a trailing slash), resulting in a broken, unreplaced namespace. Additionally, it is safer to enforce a strict boundary check to prevent false matches (like classes_extra).
🐛 Proposed fix for robust namespace extraction
- $classesdir = "$componentpath/classes/";
- $namespace = str_replace($classesdir, '', $dirpath);
- $namespace = str_replace('/', '\\', $namespace);
- $namespace = "$this->component\\$namespace";
+ $classesdir = "$componentpath/classes";
+ if ($dirpath !== $classesdir && !str_starts_with($dirpath, $classesdir . '/')) {
+ throw new \InvalidArgumentException("Generated file must be within the component's classes directory.");
+ }
+ $subnamespace = substr($dirpath, strlen($classesdir));
+ $subnamespace = trim(str_replace('/', '\\', $subnamespace), '\\');
+ $namespace = $this->component . ($subnamespace !== '' ? "\\$subnamespace" : '');📝 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.
| $classesdir = "$componentpath/classes/"; | |
| $namespace = str_replace($classesdir, '', $dirpath); | |
| $namespace = str_replace('/', '\\', $namespace); | |
| $namespace = "$this->component\\$namespace"; | |
| $classesdir = "$componentpath/classes"; | |
| if ($dirpath !== $classesdir && !str_starts_with($dirpath, $classesdir . '/')) { | |
| throw new \InvalidArgumentException("Generated file must be within the component's classes directory."); | |
| } | |
| $subnamespace = substr($dirpath, strlen($classesdir)); | |
| $subnamespace = trim(str_replace('/', '\\', $subnamespace), '\\'); | |
| $namespace = $this->component . ($subnamespace !== '' ? "\\$subnamespace" : ''); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@classes/local/generators/snippets/base.php` around lines 114 - 118, Update
namespace extraction around $classesdir and $dirpath so the classes directory
itself resolves to an empty relative namespace, while nested directories retain
their relative path. Normalize the directory-boundary comparison before removing
the prefix, and ensure paths such as classes_extra are not treated as
descendants; preserve the existing separator conversion and component namespace
construction.
| #[\Override] | ||
| public function generate(): string { | ||
| $this->category = 'task'; | ||
| [$file, $namespace, $class] = self::php_file_with_namespaced_class(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that the method in the base class is non-static.
ast-grep run --pattern 'protected function php_file_with_namespaced_class(): array { $$$ }' --lang php classes/local/generators/snippets/base.phpRepository: moodlicious/moodle-local_devkit
Length of output: 527
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n-- task.php --\n'
sed -n '1,120p' classes/local/generators/snippets/task.php | cat -n
printf '\n-- base.php outline --\n'
ast-grep outline classes/local/generators/snippets/base.php --view expanded
printf '\n-- base.php method search --\n'
rg -n "php_file_with_namespaced_class|function .*static" classes/local/generators/snippets/base.phpRepository: moodlicious/moodle-local_devkit
Length of output: 3691
Call this helper on the instance, not statically classes/local/generators/snippets/task.php:48
php_file_with_namespaced_class() is a protected instance method in base, so self::php_file_with_namespaced_class() will fatally error in PHP 8. Use $this->php_file_with_namespaced_class() instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@classes/local/generators/snippets/task.php` at line 48, Update the call in
the task generator to invoke the inherited protected instance method
php_file_with_namespaced_class() through $this rather than self::, while
preserving the existing destructuring assignment.
Use str_starts_with + substr instead of str_replace so files like classes/test.php are handled correctly alongside classes/test/test.php.
1 participant
This PR is a draft — the implementation is not complete and is subject to change.
Summary
Adds a code snippet generator system for scaffolding common Moodle boilerplate, starting with task classes.
Motivation
Creating Moodle tasks requires repetitive boilerplate (class structure,
get_name(),execute(), docblocks). This generator automates that, similar to Laravel'smake:commands.Planned usage
Auto-formatting
Generated files are automatically run through the
formatcommand after creation (Pint + PHPCBF for PHP), so the output is immediately compliant with the project's coding standards — no manual formatting needed.Example output
Generated scheduled task
Files
classes/local/generators/snippets/base.php— Abstract base class using Nette PHP Generator for creating PHP files with namespaces, classes, and Moodle-standard docblocksclasses/local/generators/snippets/task.php— Task generator supportingscheduledandadhoctypesDependencies
nette/php-generator— PHP code generation libraryTODO
make:task)thirdpartylibs.xmlwith nette/php-generator entry