fix(support): Refactor project structure and update dependencies. #11
Conversation
WalkthroughRepository-wide maintenance: license change to BSD-3-Clause, CI/workflow refactors and re-pinning, migration from Psalm to PHPStan, added Rector and PHPStan configs, modernized PHPUnit config, expanded Assert reflection utilities and tests, various ignore/export updates and updated tooling configs. Changes
Sequence Diagram(s)sequenceDiagram
actor Developer
participant Test as Test Code
participant Assert as PHPForge\Support\Assert
Developer->>Test: call equalsWithoutLE(expected, actual)
Test->>Assert: equalsWithoutLE(...)
Assert->>Assert: normalize CRLF -> LF
Assert->>Assert: assertEquals(normalized)
Assert-->>Test: assertion result
sequenceDiagram
actor Developer
participant Test as Test Code
participant Assert as PHPForge\Support\Assert
participant Reflection as Reflection API
participant Obj as Target Object
Developer->>Test: call inaccessibleParentProperty(obj, ParentClass, name)
Test->>Assert: inaccessibleParentProperty(...)
Assert->>Reflection: ReflectionProperty(ParentClass, name)
Reflection-->>Assert: makeAccessible / getValue
Assert-->>Test: return property value
Developer->>Test: call invokeParentMethod(obj, ParentClass, method, args)
Test->>Assert: invokeParentMethod(...)
Assert->>Reflection: ReflectionMethod(ParentClass, method)
Reflection-->>Assert: makeAccessible / invoke(args)
Assert-->>Test: return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 21
🔭 Outside diff range comments (2)
README.md (2)
17-18: Infection badge URL references a different repositoryThe Stryker badge points to yii2-extensions/asset-bootstrap5, not this repo. This likely shows an incorrect badge.
- <img src="https://img.shields.io/endpoint?style=flat&url=https%3A%2F%2Fbadge-api.stryker-mutator.io%2Fgithub.com%2Fyii2-extensions%2Fasset-bootstrap5%2Fmain" alt="Infection"> + <img src="https://img.shields.io/endpoint?style=flat&url=https%3A%2F%2Fbadge-api.stryker-mutator.io%2Fgithub.com%2Fphp-forge%2Fsupport%2Fmain" alt="Infection">
191-196: License mismatch: README says MIT but repository moved to BSD-3-ClauseThis discrepancy can cause legal confusion. Update README to reflect BSD-3-Clause consistently with LICENSE.md and other docs.
## License @@ -[](LICENSE.md) +[](LICENSE.md) -The MIT License. Please see [License File](LICENSE.md) for more information. +BSD 3-Clause License. Please see the [License File](LICENSE.md) for more information.
🧹 Nitpick comments (35)
.github/workflows/mutation.yml (1)
22-24: Re: .github/workflows/mutation.yml –phpstaninput verified; optional CI hardening
- ✅ The reusable workflow at
php-forge/actions/.github/workflows/infection.yml@v1declares a boolean inputphpstan, sophpstan: truewill be picked up correctly.- 🔧 Optional CI hygiene: at the top of your workflow (
.github/workflows/mutation.yml), add minimal permissions and concurrency to prevent sprawl:permissions: contents: read concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true.github/workflows/static.yml (1)
21-22: Rename job id for clarity (psalm -> phpstan).The job runs PHPStan but is still named “psalm”. Renaming avoids confusion in the Actions UI and future maintenance.
Apply this diff:
jobs: - psalm: + phpstan: uses: php-forge/actions/.github/workflows/phpstan.yml@v1.github/workflows/build.yml (1)
22-22: Optional hardening: pin reusable workflow to a commit SHA.Tagging at v1 is reasonable; if you want supply-chain hardening, consider pinning to a specific commit SHA in addition to the major tag.
Example:
-uses: php-forge/actions/.github/workflows/phpunit.yml@v1 +uses: php-forge/actions/.github/workflows/phpunit.yml@v1 +# Optionally mirror with a pinned SHA for extra safety: +# uses: php-forge/actions/.github/workflows/phpunit.yml@<commit-sha>phpstan.neon (1)
11-11: Consider a repo-local cache dir to avoid polluting project root.tmpDir under runtime at the root works but is easy to commit accidentally. Using a hidden or build-only path (e.g., .phpstan or var/phpstan) keeps things tidy.
- tmpDir: %currentWorkingDirectory%/runtime + tmpDir: %currentWorkingDirectory%/.phpstanIf you keep runtime/, please ensure it’s gitignored.
.gitattributes (1)
24-36: Exclude phpstan.neon and rector.php from release archivesTo keep your distribution tarballs lean, add the missing static-analysis/tooling configs to
.gitattributes:/.psalm.xml export-ignore +/phpstan.neon export-ignore +/rector.php export-ignore(Note:
composer-require-checker.jsonisn’t in the repo—add itsexport-ignoreentry if you introduce that file later.)copilot/class-documentation.md (6)
30-38: Fix markdownlint MD058: add blank lines around the table.Tables should be surrounded by blank lines to satisfy markdownlint and improve readability.
-### Common Tags -| Tag | Purpose | -|---------------|------------------------------------------| -| `@param` | Describes each input parameter | -| `@return` | Explains the returned value | -| `@throws` | Lists exceptions thrown | -| `@template` | Declares generic templates | -| `@phpstan-var`| Specifies complex property types | +### Common Tags + +| Tag | Purpose | +|----------------|------------------------------------------| +| `@param` | Describes each input parameter | +| `@return` | Explains the returned value | +| `@throws` | Lists exceptions thrown | +| `@template` | Declares generic templates | +| `@phpstan-var` | Specifies complex property types | +
50-51: Make headings and labels consistent with the “English only” guideline.Several headings use Spanish while the guide mandates English. Unify for consistency.
-### Ejemplo para Clases Regulares +### Example for Regular Classes @@ -### Ejemplo Abstract class +### Example for Abstract Classes @@ -### Ejemplo para Interfaces +### Example for Interfaces @@ -### Ejemplo para Excepciones +### Example for Exceptions @@ -### Ejemplo para Tests +### Example for Tests @@ -// ❌ Incorrecto +// ❌ Incorrect @@ -// ✅ Correcto +// ✅ CorrectAlso applies to: 87-90, 123-126, 152-156, 243-247, 403-415
218-221: Correct message text typo in HOST_NOT_MATCHED example.There’s an extra “of” making the sentence ungrammatical.
- * Format: "The passed host '%s' of doesn't match the regexp '%s'" + * Format: "The passed host '%s' does not match the regexp '%s'" @@ - case HOST_NOT_MATCHED = 'The passed host \'%s\' of does not match the regexp \'%s\''; + case HOST_NOT_MATCHED = 'The passed host \'%s\' does not match the regexp \'%s\'';
224-236: Align method description with actual behavior (interpolates arguments).The doc says it returns the raw string without interpolation, but the implementation uses sprintf with arguments.
- * Returns the formatted message string for the error case. - * - * Retrieves the raw message string associated with this error case without parameter interpolation. + * Returns the formatted message string for the error case. + * + * Retrieves the message template associated with this case and interpolates the provided arguments. @@ - * @return string Error message string with interpolated arguments. + * @return string Error message string with interpolated arguments. @@ - * throw new RouterNotFoundException(Message::ROUTE_NOT_FOUND->getMessage('/invalid/path')); + * throw new RouteNotFoundException(Message::ROUTE_NOT_FOUND->getMessage('/invalid/path'));
288-294: Fix namespace typo: PHPPress vs PHPress.Maintain a consistent project name to avoid confusion in generated docs and IDE tooling.
- * Creates a new instance of the {@see \PHPress\Router\RouteCollection} class. + * Creates a new instance of the {@see \PHPPress\Router\RouteCollection} class.
381-385: Tighten phrasing in throws example tail.The trailing “matching …” fragment reads as a broken sentence.
-@throws WidgetStackException if the stack is empty, or if `end()` is called without a `begin()` call. -matching {@see AbstractBlock::begin()}. +@throws WidgetStackException if the stack is empty, or if `end()` is called without a matching {@see AbstractBlock::begin()} call.copilot/copilot-instructions.md (5)
18-20: Use cross-platform Composer scripts or forward-slash paths for tooling commands.Backslashes will not work on Unix shells. Prefer Composer scripts (recommended) or forward slashes:
- - Run static analysis: `vendor\bin\phpstan analyse` - - Run code style: `vendor\bin\ecs check` - - Run tests: `vendor\bin\phpunit` + - Run static analysis: `composer phpstan` + - Run code style: `composer ecs` + - Run tests: `composer test`If Composer scripts are not defined, use forward slashes:
- - Run static analysis: `vendor\bin\phpstan analyse` - - Run code style: `vendor\bin\ecs check` - - Run tests: `vendor\bin\phpunit` + - Run static analysis: `vendor/bin/phpstan analyse` + - Run code style: `vendor/bin/ecs check` + - Run tests: `vendor/bin/phpunit`
21-24: If 100% coverage is required, ensure CI enforces it.Set an explicit coverage minimum in CI (e.g., via coverage thresholds or Codecov status checks) so the requirement is actionable and enforced for PRs.
If you want, I can propose a GitHub Actions step and a Codecov status-check configuration to gate merges on coverage.
26-31: Document the rationale for “string|null” over “?string” to avoid stylistic churn.Both are valid in PHP 8.2+. Since this deviates from many teams’ defaults, add a short rationale here and link to the exact ECS/PHP-CS-Fixer rules that enforce it to prevent bikeshedding.
10-14: Verify exception package paths and add links.The doc references
src/emitter/exception/andsrc/http/exception/. Make sure these paths exist here; if not, update to the Support library’s structure. Also consider turning them into clickable relative links for faster discovery.
44-49: Use direct same-folder relative links to reduce fragility.These files live in the same
copilot/dir. Prefer./file.mdto avoid unnecessary../copilot/hops:- [copilot/general-instructions.md](../copilot/general-instructions.md) - [copilot/code-style.md](../copilot/code-style.md) - [copilot/class-documentation.md](../copilot/class-documentation.md) - [copilot/unit-test.md](../copilot/unit-test.md) - [copilot/static-analysis.md](../copilot/static-analysis.md) + [general-instructions.md](./general-instructions.md) + [code-style.md](./code-style.md) + [class-documentation.md](./class-documentation.md) + [unit-test.md](./unit-test.md) + [static-analysis.md](./static-analysis.md).gitignore (1)
33-35: Ignore PHPUnit cache directory used by phpunit.xml.dist.Your phpunit.xml.dist writes cache to
runtime/.phpunit.cache. Ignoring the parent avoids polluting the working tree:# phpunit cache .phpunit.result.cache + +# phpunit cache directory +/runtime/Confirm you don’t intend to version any files under
runtime/. If you do, narrow the ignore to/runtime/.phpunit.cache/instead.copilot/project-documentation.md (2)
47-55: Fix the backtick rendering instruction.The current escaping is awkward. Make it explicit and readable:
-Use backticks \``\` for inline code like: +Use backticks (`) for inline code, for example:
100-103: Hyphenate the compound adjective “Rate-limiting.”-- Rate limiting policies +- Rate-limiting policiesphpunit.xml.dist (1)
4-4: Consider using the versioned remote schema URL for better IDE validation portability.Referencing the schema in
vendor/depends on having dependencies installed locally. Using the official URL improves portability:- xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd" + xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.5/phpunit.xsd"Adjust the version to match the installed PHPUnit major/minor.
copilot/static-analysis.md (2)
29-36: Align PHPDoc generics with the declared property type.The example says
Collection<int, Link>but the property is typedarray. Usearray<int, Link>for coherence:- * @var Collection<int, Link> CSS link tags for rendering. + * @var array<int, Link> CSS link tags for rendering.
86-88: Prefer Composer scripts for cross-platform PHPStan invocations.This avoids path separator issues and centralizes options:
-vendor/bin/phpstan analyse --generate-baseline +composer phpstan -- --generate-baselineIf a
phpstanComposer script is not defined, consider adding one in composer.json.I can propose the composer.json scripts block if you want.
copilot/code-style.md (2)
8-13: Table of contents says PHP 8.1 but the section is PHP 8.2.Minor inconsistency; update the ToC to match the actual section.
-1. PHP 8.1 Feature Usage. +1. PHP 8.2 Feature Usage.
88-95: Namespace/import examples still use PHPPress.Keep examples consistent with the repository’s vendor/namespace to reduce cognitive load.
-namespace PHPPress\Component; +namespace PHPForge\Support\Component; -use PHPPress\Event\EventDispatcher; +use PHPForge\Support\Event\EventDispatcher; use function array_map; use function sprintf;ecs.php (1)
51-56:VisibilityRequiredFixerconfigured with empty elements; consider removing or enabling explicitly.With
'elements' => [], the fixer effectively does nothing. Either drop it to simplify config or configure desired elements.- ->withConfiguredRule( - VisibilityRequiredFixer::class, - [ - 'elements' => [], - ], - ) + // Enable to enforce visibility declarations: + // ->withConfiguredRule( + // VisibilityRequiredFixer::class, + // [ + // 'elements' => ['const', 'property', 'method'], + // ], + // )docs/testing.md (1)
15-20: Polish phrasing for imperative sentences; keep style consistentShort, punchy imperative phrasing reads better for tooling instructions. Suggest tightening the language as below.
-## Easy coding standard - -The code is checked with [Easy Coding Standard](https://github.com/easy-coding-standard/easy-coding-standard) and -[PHP CS Fixer](https://github.com/PHP-CS-Fixer/PHP-CS-Fixer). To run it. +## Easy coding standard + +Use [Easy Coding Standard](https://github.com/easy-coding-standard/easy-coding-standard) and +[PHP CS Fixer](https://github.com/PHP-CS-Fixer/PHP-CS-Fixer). Run: @@ -Mutation testing is checked with [Infection](https://infection.github.io/). To run it. +Mutation testing is performed with [Infection](https://infection.github.io/). Run: @@ -With PHPStan analysis, it will also check for static analysis issues during mutation testing. +With PHPStan enabled, mutation testing will also check static-analysis issues. @@ -The code is statically analyzed with [PHPStan](https://phpstan.org/). To run static analysis. +Static analysis is performed with [PHPStan](https://phpstan.org/). Run: @@ -## Unit Tests - -The code is tested with [PHPUnit](https://phpunit.de/). To run tests. +## Unit tests + +Unit tests are run with [PHPUnit](https://phpunit.de/). Run:Also applies to: 24-26, 30-35, 38-42, 44-50
copilot/general-instructions.md (1)
78-80: Add a language to fenced code block (markdownlint MD040)Specify a language to satisfy markdownlint and improve rendering.
- ``` + ```text type(scope): description. ```README.md (2)
20-21: Badge alt text still mentions Psalm; project moved to PHPStanAlign badge alt text with the current tooling to avoid confusion.
- <img src="https://github.com/php-forge/support/actions/workflows/static.yml/badge.svg" alt="Psalm"> + <img src="https://github.com/php-forge/support/actions/workflows/static.yml/badge.svg" alt="PHPStan"> @@ - <img src="https://shepherd.dev/github/php-forge/support/coverage.svg" alt="Psalm Coverage"> + <img src="https://shepherd.dev/github/php-forge/support/coverage.svg" alt="Static Analysis Coverage">Also applies to: 23-23
54-61: Install as dev dependency by defaultThis package is test/support tooling; recommend installing with --dev to match the manual example below.
-```bash -composer require --prefer-dist php-forge/support -``` +```bash +composer require --dev --prefer-dist php-forge/support +```src/Assert.php (2)
55-61: Normalize lone carriage returns tooHandling only CRLF misses files with legacy Mac line endings. Normalize both "\r\n" and lone "\r".
- $expected = str_replace("\r\n", "\n", $expected); - $actual = str_replace("\r\n", "\n", $actual); + $expected = str_replace(["\r\n", "\r"], "\n", $expected); + $actual = str_replace(["\r\n", "\r"], "\n", $actual);
201-226: Avoid error suppression; prefer explicit checks when removing filesThe @ operator hides real failures. Consider explicit checks and throwing on failure to simplify debugging of flaky tests. Also consider skipping symlinks to avoid recursion outside the intended test directory.
- $handle = @opendir($basePath); + $handle = opendir($basePath); @@ - if (is_dir($path)) { + if (is_link($path)) { + // Skip symlinks + continue; + } elseif (is_dir($path)) { self::removeFilesFromDirectory($path); - @rmdir($path); + if (!rmdir($path) && is_dir($path)) { + throw new RuntimeException("Unable to remove directory: $path"); + } } else { - @unlink($path); + if (file_exists($path) && !unlink($path)) { + throw new RuntimeException("Unable to remove file: $path"); + } }tests/AssertTest.php (4)
34-41: Add a cross-OS assertion: verify normalization across different line endings.This test currently passes identical CRLF strings. To fully exercise equalsWithoutLE, add a case where expected has LF and actual has CRLF (or vice-versa) to prove normalization works.
Example additional test method you can add:
public function testEqualsWithoutLEReturnsTrueWhenLineEndingsDiffer(): void { Assert::equalsWithoutLE( "foo\nbar", "foo\r\nbar", "Should return 'true' when strings differ only by line endings." ); }
61-66: Make the test resilient by ensuring the base runtime directory exists.If the repository doesn’t already contain a top-level runtime directory, mkdir("{$dir}/subdir") will fail. Ensure $dir exists and create subdir recursively to avoid warnings and flakiness.
Apply this diff:
- $dir = dirname(__DIR__) . '/runtime'; + $dir = dirname(__DIR__) . '/runtime'; + if (!is_dir($dir)) { + mkdir($dir, 0777, true); + } - - mkdir("{$dir}/subdir"); + mkdir("{$dir}/subdir", 0777, true);Optional: For better test isolation, consider using a per-test temp directory under DIR (tests/runtime) rather than a repo-level runtime path.
96-101: Fix the assertion message to reference the correct method.The message mentions 'inaccessibleParentMethod' but the invoked method is 'inaccessibleMethod'.
Apply this diff:
- "Should return 'value' when invoking the inaccessible method 'inaccessibleParentMethod' on 'TestClass' " . + "Should return 'value' when invoking the inaccessible method 'inaccessibleMethod' on 'TestClass' " .
22-28: Add negative tests for reflection error paths to match the docblock claims.Docblock mentions “Ensuring correct exception handling for invalid operations,” but there are no tests asserting failures for missing properties/methods. Adding these will harden the contract of the helpers.
Sample tests you could add:
public function testInaccessiblePropertyThrowsForUnknownProperty(): void { $this->expectException(\ReflectionException::class); Assert::inaccessibleProperty(new TestClass(), 'unknown'); } public function testInvokeMethodThrowsForUnknownMethod(): void { $this->expectException(\ReflectionException::class); Assert::invokeMethod(new TestClass(), 'unknownMethod'); } public function testInaccessibleParentPropertyThrowsForUnknownProperty(): void { $this->expectException(\ReflectionException::class); Assert::inaccessibleParentProperty(new TestClass(), TestBaseClass::class, 'unknownParentProp'); } public function testInvokeParentMethodThrowsForUnknownMethod(): void { $this->expectException(\ReflectionException::class); Assert::invokeParentMethod(new TestClass(), TestBaseClass::class, 'unknownParentMethod'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (29)
.gitattributes(1 hunks).github/workflows/build.yml(1 hunks).github/workflows/dependency-check.yml(1 hunks).github/workflows/mutation.yml(1 hunks).github/workflows/static.yml(1 hunks).gitignore(1 hunks)LICENSE(0 hunks)LICENSE.md(1 hunks)README.md(1 hunks)code_coverage/.gitignore(1 hunks)composer.json(2 hunks)copilot/class-documentation.md(1 hunks)copilot/code-style.md(1 hunks)copilot/copilot-instructions.md(1 hunks)copilot/general-instructions.md(1 hunks)copilot/project-documentation.md(1 hunks)copilot/static-analysis.md(1 hunks)copilot/unit-test.md(1 hunks)docs/testing.md(1 hunks)ecs.php(1 hunks)phpstan.neon(1 hunks)phpunit.xml.dist(1 hunks)psalm.xml(0 hunks)rector.php(1 hunks)runtime/.gitignore(1 hunks)src/Assert.php(3 hunks)tests/AssertTest.php(1 hunks)tests/Stub/TestBaseClass.php(1 hunks)tests/Stub/TestClass.php(1 hunks)
💤 Files with no reviewable changes (2)
- LICENSE
- psalm.xml
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/Stub/TestClass.php (1)
tests/Stub/TestBaseClass.php (1)
TestBaseClass(15-23)
tests/AssertTest.php (3)
tests/Stub/TestBaseClass.php (1)
TestBaseClass(15-23)tests/Stub/TestClass.php (1)
TestClass(15-23)src/Assert.php (9)
Assert(40-290)equalsWithoutLE(55-61)inaccessibleParentProperty(82-90)removeFilesFromDirectory(201-226)inaccessibleProperty(110-120)invokeMethod(140-150)invokeParentMethod(172-186)setInaccessibleParentProperty(246-260)setInaccessibleProperty(276-289)
🪛 LanguageTool
LICENSE.md
[grammar] ~10-~10: There might be a mistake here.
Context: ...conditions and the following disclaimer. * Redistributions in binary form must repr...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...aterials provided with the distribution. * Neither the name of PHP Forge (Terabytes...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...GHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLU...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...IGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIA...
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...RISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBI...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...VISED OF THE POSSIBILITY OF SUCH DAMAGE.
(QB_NEW_EN)
copilot/static-analysis.md
[grammar] ~123-~123: There might be a mistake here.
Context: ...e Safety - Undefined methods/properties. - Invalid argument types. - Incompatible r...
(QB_NEW_EN)
[grammar] ~147-~147: There might be a mistake here.
Context: ...lized properties. - Readonly violations. - Visibility checks. ## Best Practices #...
(QB_NEW_EN)
copilot/general-instructions.md
[grammar] ~4-~4: There might be a mistake here.
Context: ...and code-style.md to provide a unified foundation for team workflows, Copilot g...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...butor consistency. ## Table of Contents 1. Communication 2. Language & Version 3. C...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: .... ## Table of Contents 1. Communication 2. Language & Version 3. Code Organization ...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...s 1. Communication 2. Language & Version 3. Code Organization 4. Quality Assurance 5...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ... Language & Version 3. Code Organization 4. Quality Assurance 5. Documentation Stand...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: .... Code Organization 4. Quality Assurance 5. Documentation Standards 6. Contribution ...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...ity Assurance 5. Documentation Standards 6. Contribution Workflow 7. Reference Docum...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...ation Standards 6. Contribution Workflow 7. Reference Documents ## Communication -...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ... as: - Constructor property promotion. - Property hooks. - Arrow functions. -...
(QB_NEW_EN)
[grammar] ~60-~60: There might be a mistake here.
Context: ...des validation for: > - Static analysis. > - Code style. > - Unit tests passing. ## ...
(QB_NEW_EN)
copilot/project-documentation.md
[grammar] ~4-~4: There might be a mistake here.
Context: ...imized for clarity, maintainability, and compatibility with GitHub Copilot. ## T...
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...d forms (for example, "it's", "you're"). - Use sentence-style capitalization. -...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...rmat dates as "March 26, 2024". - Avoid: - Sentence fragments - Synonyms for a si...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ..., 2024". - Avoid: - Sentence fragments - Synonyms for a single concept - Cultur...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...gments - Synonyms for a single concept - Culturally sensitive terms - Long modi...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...e concept - Culturally sensitive terms - Long modifier chains - Overly complex ...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...sensitive terms - Long modifier chains - Overly complex words ### Formatting Gui...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...# Technical Writing ### Code References Use backticks `\ for inline code like...
(QB_NEW_EN)
[grammar] ~86-~86: There might be a mistake here.
Context: ...## Documentation Types ### README Files - Project overview and purpose - List of k...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...DME Files - Project overview and purpose - List of key features - Quick start guide...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...rview and purpose - List of key features - Quick start guide - Installation steps -...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ...List of key features - Quick start guide - Installation steps - Usage examples - De...
(QB_NEW_EN)
[grammar] ~90-~90: There might be a mistake here.
Context: ...- Quick start guide - Installation steps - Usage examples - Dependencies - License ...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ...de - Installation steps - Usage examples - Dependencies - License information ### ...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...on steps - Usage examples - Dependencies - License information ### API Documentati...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...cense information ### API Documentation - Purpose and scope - Authentication mecha...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...## API Documentation - Purpose and scope - Authentication mechanism - Request and r...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...ose and scope - Authentication mechanism - Request and response format - Error stru...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ... mechanism - Request and response format - Error structures and codes - Rate limiti...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ...onse format - Error structures and codes - Rate limiting policies - Usage examples ...
(QB_NEW_EN)
[uncategorized] ~100-~100: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...e format - Error structures and codes - Rate limiting policies - Usage examples per endpoint ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~100-~100: There might be a mistake here.
Context: ...tures and codes - Rate limiting policies - Usage examples per endpoint - Reference ...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...g policies - Usage examples per endpoint - Reference to external SDKs or libraries ...
(QB_NEW_EN)
[grammar] ~104-~104: There might be a mistake here.
Context: ...xternal SDKs or libraries ### Tutorials - Clear learning objectives - Prerequisite...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ...## Tutorials - Clear learning objectives - Prerequisites list - Step-by-step instru...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...learning objectives - Prerequisites list - Step-by-step instructions - Complete cod...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...uisites list - Step-by-step instructions - Complete code examples - Screenshots or ...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...ep instructions - Complete code examples - Screenshots or expected output - Trouble...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...xamples - Screenshots or expected output - Troubleshooting tips - References to dee...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...r expected output - Troubleshooting tips - References to deeper docs ### Changelog...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...ecture Docs ### Component Documentation - Component purpose and context - Internal...
(QB_NEW_EN)
[grammar] ~128-~128: There might be a mistake here.
Context: ...entation - Component purpose and context - Internal and external dependencies - Con...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...ext - Internal and external dependencies - Configuration options and defaults - Usa...
(QB_NEW_EN)
[grammar] ~130-~130: There might be a mistake here.
Context: ...ies - Configuration options and defaults - Usage patterns and lifecycle - Event tri...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ... defaults - Usage patterns and lifecycle - Event triggers and listeners - Extensibi...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...lifecycle - Event triggers and listeners - Extensibility points (hooks/plugins) - P...
(QB_NEW_EN)
[grammar] ~133-~133: There might be a mistake here.
Context: ...s - Extensibility points (hooks/plugins) - Performance or scaling considerations #...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...g considerations ### Integration Guides - System/environment requirements - Setup ...
(QB_NEW_EN)
[grammar] ~137-~137: There might be a mistake here.
Context: ...Guides - System/environment requirements - Setup and configuration - Supported plat...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: ...t requirements - Setup and configuration - Supported platforms or stacks - Common s...
(QB_NEW_EN)
[grammar] ~139-~139: There might be a mistake here.
Context: ...guration - Supported platforms or stacks - Common scenarios and walkthroughs - Erro...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ...acks - Common scenarios and walkthroughs - Error handling and logging - Security be...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ...alkthroughs - Error handling and logging - Security best practices - Links to examp...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...ng and logging - Security best practices - Links to example projects or templates ...
(QB_NEW_EN)
[grammar] ~154-~154: There might be a mistake here.
Context: ...angelogs or history. ### Quality Checks - Spellcheck and grammar review - Validate...
(QB_NEW_EN)
[grammar] ~155-~155: There might be a mistake here.
Context: ...y Checks - Spellcheck and grammar review - Validate links and anchors - Run code sn...
(QB_NEW_EN)
[grammar] ~156-~156: There might be a mistake here.
Context: ...mmar review - Validate links and anchors - Run code snippets if executable - Refres...
(QB_NEW_EN)
[grammar] ~157-~157: There might be a mistake here.
Context: ...nchors - Run code snippets if executable - Refresh outdated screenshots - Test conf...
(QB_NEW_EN)
[grammar] ~158-~158: There might be a mistake here.
Context: ...xecutable - Refresh outdated screenshots - Test configuration instructions - Use co...
(QB_NEW_EN)
[grammar] ~159-~159: There might be a mistake here.
Context: ...nshots - Test configuration instructions - Use consistent formatting across files ...
(QB_NEW_EN)
[grammar] ~162-~162: There might be a mistake here.
Context: ... files ### File and Folder Organization - Match doc folder structure to source tre...
(QB_NEW_EN)
[grammar] ~169-~169: There might be a mistake here.
Context: ...tive links ### Feedback and Improvement - Monitor issue trackers and feedback tool...
(QB_NEW_EN)
copilot/class-documentation.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...s project, also optimized for tools like GitHub Copilot. ## Table of Contents 1....
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...e code actually does**. ### Common Tags | Tag | Purpose ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...rpose | |---------------|-----------------------...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...---------------------------------------| | @param | Describes each input p...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...scribes each input parameter | | @return | Explains the returned ...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...plains the returned value | | @throws | Lists exceptions throw...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...sts exceptions thrown | | @template | Declares generic templ...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...clares generic templates | | @phpstan-var| Specifies complex prop...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...ation ### Ejemplo para Clases Regulares path: /core/Src/Router/Route.php ```php...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...ss Route ``` ### Ejemplo Abstract class path: /core/Src/Web/View/Base/AbstractVi...
(QB_NEW_EN)
[grammar] ~123-~123: There might be a mistake here.
Context: ...mponent ``` ### Ejemplo para Interfaces path: /core/Src/View/ViewContextInterfac...
(QB_NEW_EN)
[grammar] ~152-~152: There might be a mistake here.
Context: ...erface ``` ### Ejemplo para Excepciones path: /core/Src/Router/Exception/RouteNo...
(QB_NEW_EN)
[grammar] ~243-~243: There might be a mistake here.
Context: ... } } ``` ### Ejemplo para Tests path: /core/tests/Router/RouteTest.php ...
(QB_NEW_EN)
[grammar] ~316-~316: There might be a mistake here.
Context: ...n Route[] */ ``` ### Protected Methods (Ensure that PHPDoc is as detailed as fo...
(QB_NEW_EN)
[grammar] ~361-~361: There might be a mistake here.
Context: ...error occurs...” — be clear and precise. - Use the most specific exception availabl...
(QB_NEW_EN)
copilot/unit-test.md
[grammar] ~17-~17: There might be a mistake here.
Context: ...he production class and end with Test. - Internal Documentation: Document each ...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...t Method Naming ### Recommended Pattern Use the format `test<Co...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...ovider`). ### Documentation and Example Include complete documentation in each d...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...e and purpose of the data. Example: path: /core/tests/Provider/Router/RouteP...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ...s RouteProvider ``` Usage in Tests: path: /core/tests/Router/RouteTest.php ...
(QB_NEW_EN)
[grammar] ~156-~156: There might be a mistake here.
Context: ...terns.', ); ``` ### Exceptions in Tests - Document in the PHPDoc the conditions un...
(QB_NEW_EN)
[grammar] ~159-~159: There might be a mistake here.
Context: ... including clear examples. Example: path: /core/tests/Router/RouteTest.php ...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ...ate suggestions aligned with the design.
(QB_NEW_EN)
copilot/copilot-instructions.md
[grammar] ~16-~16: There might be a mistake here.
Context: ...ange-Act-Assert. ## Developer Workflows - Build/Static Analysis: - Run static ...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...per Workflows - Build/Static Analysis: - Run static analysis: `vendor\bin\phpstan...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ception messages. ## Integration Points - PSR-7/PSR-15: Integrates with any PSR-...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...es with any PSR-7 compatible HTTP stack. - Yii2: Extends and adapts Yii2 core req...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ... documentation standards. ## References - [copilot/general-instructions.md](../copi...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ...ences - copilot/general-instructions.md - [copilot/code-style.md](../copilot/code-s...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...nstructions.md) - copilot/code-style.md - [copilot/class-documentation.md](../copil...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...le.md) - copilot/class-documentation.md - [copilot/unit-test.md](../copilot/unit-te...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...ocumentation.md) - copilot/unit-test.md - [copilot/static-analysis.md](../copilot/s...
(QB_NEW_EN)
copilot/code-style.md
[grammar] ~4-~4: There might be a mistake here.
Context: ...to improve readability, maintainability, and Copilot-assisted development. ## Ta...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...e ### Typed Properties with Union Types Use explicit union types when needed: ``...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ... ``` ### Constructor Property Promotion Prefer concise constructor definitions: ...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...) => $x < 0); ### Match Expressions Use match for explicit value checks:...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ...(true)` constructs. ### Named Arguments Use for disambiguation only: ```php // G...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...llback = strtolower(...); ### Enums Document each case clearly:php enum ...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...s. - PSR-12 compliant. - Files must end with newline. ### Namespace & Imports ```ph...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...ages. - Use ?? operator for fallbacks. - Document @throws in PHPDoc. ### Array...
(QB_NEW_EN)
[grammar] ~151-~151: There might be a mistake here.
Context: ...alse) { return null; } ``` - Braces on same line. - One space after control ke...
(QB_NEW_EN)
[grammar] ~151-~151: There might be a mistake here.
Context: ...eturn null; } ``` - Braces on same line. - One space after control keywords. - Use ...
(QB_NEW_EN)
[grammar] ~156-~156: There might be a mistake here.
Context: ...ractices - Composition over inheritance. - Follow SOLID principles. - Immutable obj...
(QB_NEW_EN)
README.md
[grammar] ~33-~33: There might be a mistake here.
Context: ...ate/protected properties via reflection. - Invoke inaccessible methods for comprehe...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...om Windows/Unix line ending differences. - Normalize line endings for consistent st...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ... cleanup for isolated test environments. - Safe removal operations preserving Git t...
(QB_NEW_EN)
[grammar] ~185-~185: There might be a mistake here.
Context: ...ting guidance, see: - 🧪 Testing Guide ## Our social networks [
🪛 markdownlint-cli2 (0.17.2)
copilot/general-instructions.md
78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
copilot/class-documentation.md
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (18)
code_coverage/.gitignore (1)
1-2: LGTM: Ignore coverage artifacts while keeping the placeholder tracked.The per-directory .gitignore pattern is correct and prevents accidental commits of coverage outputs while ensuring the directory stays in Git.
runtime/.gitignore (1)
1-2: LGTM: Ignore runtime artifacts while preserving the folder.Good use of a per-directory .gitignore to keep transient runtime files out of Git while ensuring the directory exists.
.github/workflows/build.yml (1)
22-22: No action needed: default PHP/OS matrix aligns with composer.jsonThe project’s composer.json requires PHP ^8.1 (i.e. ≥ 8.1 < 9.0), and the reusable workflow defaults to PHP 8.1–8.4 and Ubuntu/Windows. These defaults already cover your supported range, so there’s no need to pass an explicit matrix.
.github/workflows/dependency-check.yml (1)
22-22: Please verify composer-require-checker defaults
The workflow at.github/workflows/dependency-check.yml(line 22) currently invokes:uses: php-forge/actions/.github/workflows/composer-require-checker.yml@v1Without any
with:inputs, this action will fall back to its own defaults for PHP versions, OS runner, and Composer flags. Before merging, please review the source ofphp-forge/actions/.github/workflows/composer-require-checker.yml@v1to confirm:
- Which PHP versions it tests (e.g. a matrix vs. a single version)
- The default
runs-onOS (typicallyubuntu-latest)- Any default Composer commands or flags (
args) it passesIf those defaults don’t match your project requirements, override them by adding a
with:block under this step.phpstan.neon (1)
13-21: Verify strict PHPStan configuration keys against pinned versionsWe’ve pinned
phpstan/phpstanto^2.1andphpstan/phpstan-strict-rulesto^2.0.3, and you’ve enabled these advanced checks:
checkImplicitMixedcheckBenevolentUnionTypescheckUninitializedPropertiescheckMissingCallableSignaturecheckTooWideReturnTypesInProtectedAndPublicMethodsreportAnyTypeWideningInVarTagreportPossiblyNonexistentConstantArrayOffsetreportPossiblyNonexistentGeneralArrayOffsetPlease cross-check each key against:
• PHPStan core config reference: https://phpstan.org/config-reference
• PHPStan Strict Rules docs: https://github.com/phpstan/phpstan-strict-rules#configurationUnsupported options will cause PHPStan to error at startup.
.gitignore (2)
1-2: AI summary inconsistency: .idea is still ignored here.The high-level summary says
.ideawas removed from ignores, but Line 2 still ignores it. Either update the summary or the ignore list for consistency.
21-22: Ignoring composer.lock at repo root is appropriate for a library.For libraries, not committing
composer.lockis the standard practice. LGTM.phpunit.xml.dist (2)
6-7: Modernized, strict config looks good.Cache dir, execution order, and fail-on-risky/warning are solid defaults. The testsuite and source include are clean and conventional.
Also applies to: 13-23
6-7: Ensure the cache directory is ignored or ephemeral.
cacheDirectory="runtime/.phpunit.cache"implies write artifacts underruntime/. Pair with an ignore (see .gitignore suggestion) to avoid untracked files in dev environments.ecs.php (1)
14-79: Nice migration to fluent ECS config.The switch to
ECSConfig::configure()with configured rules, prepared sets, and explicit paths looks clean and maintainable. The rule set composition is clear.copilot/unit-test.md (1)
120-128: Attribute usage example: verify PHPUnit version supports DataProviderExternal.If the project is on PHPUnit 10+, DataProviderExternal is fine; otherwise provide alternative (traditional @dataProvider or static method).
Please confirm the PHPUnit major version in composer.json. If < 10, replace the attribute with a compatible data provider approach.
rector.php (2)
5-28: Solid, minimal Rector config.Parallelization, name imports, scoped paths, and a focused code-quality rule are sensible defaults.
17-23: Align Rector PHP target with PHP 8.2Composer.json’s “php”: "^8.1" already allows PHP 8.2, and you’re on rector/rector ^2.1 (which should include the PHP_82 sets). Please verify your installed rector version supports these constants (and, if not, bump to a release that does), then update your Rector config in rector.php (around lines 17–23):
- Rector\Set\ValueObject\SetList::PHP_81, - Rector\Set\ValueObject\LevelSetList::UP_TO_PHP_81, + Rector\Set\ValueObject\SetList::PHP_82, + Rector\Set\ValueObject\LevelSetList::UP_TO_PHP_82,tests/Stub/TestClass.php (1)
15-23: LGTM: concise stub for visibility/inheritance tests.Private property and protected method are appropriate for exercising reflection helpers in tests.
tests/Stub/TestBaseClass.php (1)
15-23: LGTM: solid stub for inheritance testsAbstract base with a private parent property and a protected method is appropriate for the reflection scenarios exercised in tests. Naming is clear and strict_types is enabled.
copilot/general-instructions.md (1)
42-48: Directory convention uses '/Src' (capital S); confirm alignment with PSR-4 mappingMost PHP projects and Composer PSR-4 mappings conventionally use 'src/'. If this repo uses 'src', adjust the example to reduce friction.
Would you like me to update the example block to 'src' if composer.json maps to 'src/'?
README.md (1)
48-51: No PHP version mismatch between README and composer.jsonThe project’s
composer.jsonspecifies"php": "^8.1"and the README correctly lists “PHP 8.1 or higher.” No update is needed.Likely an incorrect or invalid review comment.
tests/AssertTest.php (1)
14-31: Good expansion of test coverage and clear docblock summary.The scenarios covered (inaccessible members, parent interactions, and filesystem ops) are well-scoped and provide meaningful coverage for the new reflection helpers and utilities.
…nd string equality.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
README.md (3)
48-55: Align install guidance: recommend dev dependency for test utilities.The Composer command installs as a regular dependency, while the manual method places it under require-dev. For a testing/support package, recommending a dev install avoids shipping it to production.
Suggested adjustments:
-Install the extension. +Install the extension as a development dependency. -```bash -composer require --prefer-dist php-forge/support +```bash +composer require --dev --prefer-dist php-forge/support:^0.1-Add to your
composer.json.
+Add to yourcomposer.json(manual approach).{ "require-dev": { - "php-forge/support": "^0.1" + "php-forge/support": "^0.1" } }Also applies to: 61-66 --- `106-109`: **Clarify PHPUnit context in examples using `$this->assertSame`.** As written, these snippets assume execution inside a `PHPUnit\Framework\TestCase`. Add a brief note to reduce confusion when readers paste into a standalone script. Minimal inline clarification: ```diff -// Access private properties for testing -$value = Assert::inaccessibleProperty($object, 'secretValue'); -$this->assertSame('hidden', $value); +// Access private properties for testing +$value = Assert::inaccessibleProperty($object, 'secretValue'); +// In a PHPUnit TestCase context: +$this->assertSame('hidden', $value);-// Test protected method behavior -$result = Assert::invokeMethod($object, 'calculate', [5, 3]); -$this->assertSame(8, $result); +// Test protected method behavior +$result = Assert::invokeMethod($object, 'calculate', [5, 3]); +// In a PHPUnit TestCase context: +$this->assertSame(8, $result);-$newValue = Assert::inaccessibleProperty($object, 'config'); -$this->assertSame('test-mode', $newValue); +$newValue = Assert::inaccessibleProperty($object, 'config'); +// In a PHPUnit TestCase context: +$this->assertSame('test-mode', $newValue);-$this->assertFileDoesNotExist($testDir . '/test.txt'); -$this->assertDirectoryDoesNotExist($testDir . '/subdir'); +// In a PHPUnit TestCase context: +$this->assertFileDoesNotExist($testDir . '/test.txt'); +$this->assertDirectoryDoesNotExist($testDir . '/subdir');Alternatively, switch to native
assert()for standalone examples.Also applies to: 127-130, 148-150, 171-173
85-91: Tiny copy edit suggestion for consistency.Nit: Consider mirroring the section title in the comment to reinforce intent.
-// Normalize line endings for consistent comparisons +// Normalize line endings for consistent, cross-platform comparisons
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~27-~27: There might be a mistake here.
Context: ...ate/protected properties via reflection. - Invoke inaccessible methods for comprehe...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...om Windows/Unix line ending differences. - Normalize line endings for consistent st...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ... cleanup for isolated test environments. - Safe removal operations preserving Git t...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ...ting guidance, see: - 🧪 Testing Guide ## Quality code [
[grammar] ~184-~184: There might be a mistake here.
Context: ...ackagist.org/packages/php-forge/support) [
[grammar] ~185-~185: There might be a mistake here.
Context: ...https://codecov.io/gh/php-forge/support) [
🔇 Additional comments (1)
README.md (1)
183-188: Update README Badges Based on Active IntegrationsStyleCI is still configured (
.styleci.ymlpresent), and docs/testing.md exists for testing guidance. However, no Codecov upload step was found in CI workflows—remove the Codecov badge to avoid confusion.Please update
README.mdaccordingly:• Remove the Codecov badge (line 183)
• Keep the StyleCI badge (line 187)
• Keep other valid badges (packagist, downloads, PHPStan)Revised snippet:
[](https://github.com/php-forge/support/releases) [](https://packagist.org/packages/php-forge/support) [](https://github.com/php-forge/support/actions/workflows/static.yml) [](https://github.styleci.io/repos/661073468?branch=main)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
README.md (2)
17-18: Infection badge image still points to the wrong repository.The badge image URL references
yii2-extensions/asset-bootstrap5instead ofphp-forge/support. The link target is correct; only the image endpoint needs fixing.Apply this diff to correct the endpoint:
- <img src="https://img.shields.io/endpoint?style=flat&url=https%3A%2F%2Fbadge-api.stryker-mutator.io%2Fgithub.com%2Fyii2-extensions%2Fasset-bootstrap5%2Fmain" alt="Infection"> + <img src="https://img.shields.io/endpoint?style=flat&url=https%3A%2F%2Fbadge-api.stryker-mutator.io%2Fgithub.com%2Fphp-forge%2Fsupport%2Fmain" alt="Infection">
190-192: License text still says MIT; project intends BSD-3-Clause.Update the user-facing text to avoid compliance confusion.
Apply this diff:
-[](LICENSE.md) - -The MIT License. Please see [License File](LICENSE.md) for more information. +[](LICENSE.md) + +This project is licensed under the BSD-3-Clause License. Please see [License File](LICENSE.md) for more information.
🧹 Nitpick comments (3)
README.md (3)
24-37: Tighten feature bullets for clarity (minor wording).Small phrasing tweaks improve readability and address the grammar hints without changing meaning.
Apply this diff:
-✅ **Advanced Reflection Utilities** -- Access and modify private/protected properties via reflection. -- Invoke inaccessible methods for comprehensive testing coverage. +✅ **Advanced Reflection Utilities** +- Access and modify private and protected properties via reflection. +- Invoke inaccessible methods to expand testing coverage. -✅ **Cross-Platform String Assertions** -- Eliminate false negatives from Windows/Unix line ending differences. -- Normalize line endings for consistent string comparisons across platforms. +✅ **Cross-Platform String Assertions** +- Eliminate false positives/negatives caused by Windows vs. Unix line-ending differences. +- Normalize line endings for consistent string comparisons across platforms. -✅ **File System Test Management** -- Recursive file and directory cleanup for isolated test environments. -- Safe removal operations preserving Git tracking files. +✅ **File System Test Management** +- Recursively clean files and directories for isolated test environments. +- Safe removal that preserves Git-tracking files (e.g., .gitignore, .gitkeep).
48-55: Align install command with manual section (dev dependency).The manual section installs under require-dev, but the Composer command installs as a regular dependency. If this package is intended for testing/utilities, recommend installing as a dev dependency to keep prod footprint clean.
Apply this diff:
-```bash -composer require --prefer-dist php-forge/support -``` +```bash +composer require --dev --prefer-dist php-forge/support +```Alternatively, keep the current command and change the JSON example to use "require" for consistency.
134-147: Consider using a temp directory in the cleanup example (optional).Pointing to a project-relative
runtimeis fine, but examples usingsys_get_temp_dir()reduce accidental data loss when copied verbatim.Example alternative:
$testDir = sys_get_temp_dir() . '/support-tests'; Assert::removeFilesFromDirectory($testDir);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~27-~27: There might be a mistake here.
Context: ...ate/protected properties via reflection. - Invoke inaccessible methods for comprehe...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...om Windows/Unix line ending differences. - Normalize line endings for consistent st...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ... cleanup for isolated test environments. - Safe removal operations preserving Git t...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: ...ting guidance, see: - 🧪 Testing Guide ## Quality code [
[grammar] ~179-~179: There might be a mistake here.
Context: ...ackagist.org/packages/php-forge/support) [
[grammar] ~180-~180: There might be a mistake here.
Context: ...https://codecov.io/gh/php-forge/support) [
🔇 Additional comments (10)
README.md (10)
10-12: PHP 8.1+ badge update looks good.Consistent with the PR’s min-version bump. No issues spotted.
19-21: Static analysis badge addition looks correct.The workflow path and badge target align with GitHub Actions conventions for
static.yml.
68-72: Installation flow reads well.No issues; the update command is appropriate for the manual path.
76-93: Reflection example is coherent with new Assert API.The usage of
inaccessiblePropertywith an object target and property name matches the documented API.
95-110: Line-ending normalization example looks correct.
equalsWithoutLEusage and messaging are clear.
112-132: Protected method invocation sample is accurate.
invokeMethod($object, 'calculate', [5, 3])aligns with reflection helper semantics.
149-168: Property set/get example reads well.Clear demonstration of
setInaccessiblePropertyfollowed by verification.
176-183: Badges section looks consistent with the new tooling.PHPStan, StyleCI, and release/download badges point to plausible endpoints.
170-175: Documentation link verified
docs/testing.mdexists in the repository, so the link in README.md will not 404.
42-45: PHP requirement in composer.json matches README. composer.json specifies^8.1, which aligns with the README’s “PHP 8.1 or higher” requirement—no changes needed.
|
@coderabbitai review |
…n of testing capabilities.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
README.md (10)
24-37: Minor copy/consistency tweaks in Features section.Small wording/consistency improvements for readability. Consider applying:
## Features ✅ **Advanced Reflection Utilities** -- Access and modify private and protected properties via reflection. -- Invoke inaccessible methods to expand testing coverage. +- Access and modify private and protected properties via reflection. +- Invoke inaccessible methods to expand test coverage. ✅ **Cross-Platform String Assertions** -- Eliminate false positives/negatives caused by Windows vs. Unix line-ending differences. -- Normalize line endings for consistent string comparisons across platforms. +- Eliminate false positives/negatives caused by Windows vs. Unix line‑ending differences. +- Normalize line endings for consistent string comparisons across platforms. ✅ **File System Test Management** -- Recursively clean files and directories for isolated test environments. -- Safe removal that preserves Git-tracking files (for example, '.gitignore', '.gitkeep'). +- Recursively clean files and directories for isolated test environments. +- Safe removal that preserves Git‑tracked sentinel files (e.g., .gitignore, .gitkeep).
95-95: Pluralize section title for clarity.Recommend minor wording fix.
-### Equals without line ending +### Equals without line endings
92-92: Prefer explicit Assert in examples to avoid “self” context confusion.Docs snippets aren’t inside a PHPUnit test class, so using Assert::assertSame improves clarity.
-self::assertSame('hidden', $value); +Assert::assertSame('hidden', $value);
131-131: Prefer explicit Assert in examples to avoid “self” context confusion.-self::assertSame(8, $result); +Assert::assertSame(8, $result);
167-167: Prefer explicit Assert in examples to avoid “self” context confusion.-self::assertSame('test-mode', $newValue); +Assert::assertSame('test-mode', $newValue);
134-134: Article usage in section title.-### Remove files from directory +### Remove files from a directory
149-149: Article usage in section title.-### Set inaccessible property +### Set an inaccessible property
112-132: Add example for parent-level reflection helpers.Since parent-level utilities were added (e.g., invokeParentMethod, inaccessibleParentProperty, setInaccessibleParentProperty), consider adding a short example adjacent to this section to showcase those APIs. I can draft it if helpful.
Example to add (outside current lines):
<?php declare(strict_types=1); use PHPForge\Support\Assert; class BaseClass { protected function sum(int $a, int $b): int { return $a + $b; } private string $mode = 'base'; } class ChildClass extends BaseClass { private string $mode = 'child'; } $child = new ChildClass(); // invoke protected method defined on parent $result = Assert::invokeParentMethod($child, 'sum', [2, 3]); Assert::assertSame(5, $result); // read parent’s private property $parentMode = Assert::inaccessibleParentProperty($child, 'mode'); Assert::assertSame('base', $parentMode); // set parent’s private property Assert::setInaccessibleParentProperty($child, 'mode', 'test'); Assert::assertSame('test', Assert::inaccessibleParentProperty($child, 'mode'));
178-182: Codecov badge token: consider removing if repository is public.For public repos, Codecov badges typically don’t require a token. Exposing tokens unnecessarily isn’t ideal. If this repo is public and uploads are authenticated via CI secrets, consider dropping the token from the badge URL.
190-190: Add explicit license statement for clarity.Badge is good, but an explicit sentence helps compliance readers.
[](LICENSE.md) + +This project is licensed under the BSD-3-Clause License. See the [License File](LICENSE.md) for details.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
README.md(1 hunks)composer.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- composer.json
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~27-~27: There might be a mistake here.
Context: ...and protected properties via reflection. - Invoke inaccessible methods to expand te...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...indows vs. Unix line-ending differences. - Normalize line endings for consistent st...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...ectories for isolated test environments. - Safe removal that preserves Git-tracking...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: ...ting guidance, see: - 🧪 Testing Guide ## Quality code [
[grammar] ~179-~179: There might be a mistake here.
Context: ...ackagist.org/packages/php-forge/support) [
[grammar] ~180-~180: There might be a mistake here.
Context: ...https://codecov.io/gh/php-forge/support) [
🔇 Additional comments (5)
README.md (5)
5-5: Good, concise headline.The new headline is clear and sets the scope well.
9-12: PHP version badge looks correct.Targeting PHP >= 8.1 and linking to the 8.1 release page is consistent with the stated requirements.
16-21: Badges: Infection endpoint and Static Analysis workflow are correct.
- Infection badge now points to this repository.
- Static analysis badge references the expected workflow.
178-182: StyleCI badge remains relevant
The.styleci.ymlfile is still present at the repo root, confirming StyleCI is in use. You can keep the badge.
48-66: Composer package availability confirmed
- The package php-forge/support is published on Packagist (latest tagged version 0.1.0, released 2024-01-21).
- The Composer constraint
^0.1(resolves to ≥ 0.1.0 < 0.2.0) will successfully install v0.1.0.- Note: the package requires PHP ^8.1—ensure your project’s PHP version meets this requirement.
No changes to the README installation instructions are necessary.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CHANGELOG.md (3)
4-5: Fix markdownlint MD003: use setext header style for the new version headingYour repo appears to prefer setext-style headings. Convert the newly added atx header to setext to satisfy MD003 and keep style consistent with the linter.
Apply this diff:
-## 0.1.1 August 18, 2025 - +0.1.1 August 18, 2025 +---------------------
6-6: Link the PR and add terminal punctuation for consistencyMake the issue/PR reference clickable and end the line with a period to match “Initial release.” below.
-- Bug #11: Refactor project structure and update dependencies (@terabytesoftw) +- Bug [#11](https://github.com/php-forge/support/pull/11): Refactor project structure and update dependencies (@terabytesoftw).
8-10: Optional: unify heading styles across all entriesLine 8 still uses an atx header. If MD003 is enforced globally, converting older entries to setext will prevent future lints and keep the file consistent. Happy to provide a full-file patch if you want to standardize all headings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
4-4: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
2 participants
Summary by CodeRabbit