Add CI workflows for PHPUnit and coding standards#5
Conversation
Add continuous-integration.yml and coding-standards.yml calling the ray-di/.github reusable workflows (@v1), following the Ray.Aop setup. - PHPUnit: PHP 8.2, 8.3, 8.4 (old stable) and 8.5 (current stable) - Coding Standards: phpcs on PHP 8.3
📝 WalkthroughWalkthroughAdds PHPCS-based coding standards and reusable CI workflows, removes the PHP-CS-Fixer configuration, and modernizes PHP source signatures, imports, reflection handling, tests, PHPStan configuration, and Composer scripts. ChangesStandards and source modernization
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
composer.json: - Add description, config.sort-packages, scripts-descriptions - Drop friendsofphp/php-cs-fixer (v2 incompatible with PHP 8.5) - Bump squizlabs/php_codesniffer to ^4.0, add slevomat/coding-standard, doctrine/coding-standard, and dealerdirect/phpcodesniffer-composer-installer - Use bare commands (phpunit, phpcs, phpcbf); cs checks src tests, cs-fix runs phpcbf on src tests phpcs.xml: copied from Ray.Aop, edited for this project (name, schema path, project-specific excludes and patterns). cs-fix results: - phpcbf auto-fixed 181 violations across 16 files - Added :string return type to all __toString() methods - Removed empty if block in src/Arrow.php - Renamed test__invoke* to testInvoke* (PSR1 CamelCaps) Cleanup: remove orphaned .php_cs.dist, gitignore .phpcs-cache. Verified: composer cs passes (20 files), composer test passes (5 tests, 13 assertions).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/coding-standards.yml (1)
1-12: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueRestrict default token permissions in GitHub Actions workflows.
Both workflows rely on default token permissions, which can be overly broad. Consider explicitly setting top-level permissions to adhere to the principle of least privilege.
.github/workflows/coding-standards.yml#L1-L12: addpermissions: contents: readat the top level..github/workflows/continuous-integration.yml#L1-L13: addpermissions: contents: readat the top level.🤖 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 @.github/workflows/coding-standards.yml around lines 1 - 12, Restrict GitHub Actions token permissions to read-only repository contents by adding top-level permissions with contents: read in .github/workflows/coding-standards.yml (lines 1-12) and .github/workflows/continuous-integration.yml (lines 1-13), alongside the workflow name and triggers.Source: Linters/SAST tools
src/DependencyId.php (1)
12-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix typo in parameter name.
The parameter name
$interaceis misspelled. Consider renaming it to$interfacefor better readability.💡 Proposed refactor
- public function __invoke(string $interace, string $name): string + public function __invoke(string $interface, string $name): string { - if (class_exists($interace)) { - return (new ClassId())($interace); + if (class_exists($interface)) { + return (new ClassId())($interface);🤖 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 `@src/DependencyId.php` around lines 12 - 14, Rename the misspelled $interace parameter to $interface in __invoke, and update all references within the method while preserving its behavior.src/ObjectGrapher.php (1)
132-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the already extracted
$argumentsvariable.The reflection call on line 134 is redundant because the exact same value was already assigned to
$argumentson line 132. You can safely reuse the variable here.💡 Proposed refactor
// constructor injection $arguments = ($this->prop)($dependency->arguments, 'arguments'); $port = sprintf('p_%s_construct', ($this->snakeName)($dependency->class)); - $setters = ($this->prop)($dependency->arguments, 'arguments') === [] ? [] : ['construct' => $port]; + $setters = $arguments === [] ? [] : ['construct' => $port]; $this->drawInjectionGraph($arguments, $dependency->classId, $port);🤖 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 `@src/ObjectGrapher.php` around lines 132 - 135, In the dependency graph setup, update the `$setters` assignment in `ObjectGrapher` to reuse the already extracted `$arguments` variable instead of calling `$this->prop` again. Preserve the existing empty-arguments check and setter behavior.
🤖 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.
Nitpick comments:
In @.github/workflows/coding-standards.yml:
- Around line 1-12: Restrict GitHub Actions token permissions to read-only
repository contents by adding top-level permissions with contents: read in
.github/workflows/coding-standards.yml (lines 1-12) and
.github/workflows/continuous-integration.yml (lines 1-13), alongside the
workflow name and triggers.
In `@src/DependencyId.php`:
- Around line 12-14: Rename the misspelled $interace parameter to $interface in
__invoke, and update all references within the method while preserving its
behavior.
In `@src/ObjectGrapher.php`:
- Around line 132-135: In the dependency graph setup, update the `$setters`
assignment in `ObjectGrapher` to reuse the already extracted `$arguments`
variable instead of calling `$this->prop` again. Preserve the existing
empty-arguments check and setter behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 905a67ca-c8ee-4cf8-a48e-3d6362068cc3
📒 Files selected for processing (24)
.github/workflows/coding-standards.yml.github/workflows/continuous-integration.yml.gitignore.php_cs.distcomposer.jsonphpcs.xmlsrc/Arrow.phpsrc/ArrowInterface.phpsrc/ClassId.phpsrc/ClassNode.phpsrc/DependencyId.phpsrc/Graph.phpsrc/InstanceNode.phpsrc/InterfaceNode.phpsrc/MyDependency.phpsrc/NodeInterface.phpsrc/ObjectGrapher.phpsrc/Prop.phpsrc/ProviderNode.phpsrc/SnakeName.phpsrc/ToClass.phpsrc/ToProvider.phptests/ObjectVisualGrapherTest.phptests/bin/package.php
💤 Files with no reviewable changes (1)
- .php_cs.dist
- Upgrade phpstan/phpstan from ^0.12.5 to ^2.0; the 0.12 phar emitted PHP 8.5 deprecation noise from its own bundled Symfony console and hoa compiler and was effectively unusable on modern PHP. - Populate phpstan.neon (was empty): level 8, phpVersion 80200, paths: [src]. Level 8 chosen over max because Prop::__invoke reads Ray.Di internal objects via reflection and returns mixed; the mixed assignments flagged at max stem from untyped upstream Ray.Di properties, not fixable here without ignoring/casting. - Remove redundant assert( instanceof Argument) in ObjectGrapher::drawInjectionGraph(); the @param array<Argument> PHPDoc already narrows the type, flagged by PHPStan 2.x as function.alreadyNarrowedType / instanceof.alwaysTrue. - composer tests: phpstan now reads level/paths from phpstan.neon. - Add .github/workflows/static-analysis.yml running PHPStan in CI (cs2pr-annotated), on PHP 8.3. Verified: composer cs (20/20), composer test (5 tests), phpstan (level 8) all pass.
… plugin CI failed on --prefer-lowest: bear/package ^1.9.6 resolves bear/query-repository 1.6.0, whose QueryRepositoryModule binds Doctrine\Common\Cache\Cache, but doctrine/cache is not installed -> Ray\Di\Exception\NotFound in testInvokeBearPackage. bear/package cannot be bumped to sidestep this: every bear/package >= 1.10.10 self-requires ray/object-visual-grapher ^1.0, which is circular for this repo's own dev install (root is dev-master). So pin bear/query-repository ^1.13 instead -- 1.13.0 dropped the Doctrine Cache binding and the doctrine/* requires. Tests pass at both --prefer-lowest and highest. Remove dealerdirect/phpcodesniffer-composer-installer from require-dev (still pulled transitively by doctrine/coding-standard and slevomat/coding-standard) and disable its plugin execution via allow-plugins=false; register the external standards via <config name=installed_paths> in phpcs.xml instead. Drop the unused doctrine/annotations ^1.8 direct dev dependency. Verified: composer cs (20/20), phpstan (level 8, no errors), phpunit (5 tests) pass at both highest and --prefer-lowest.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/static-analysis.yml:
- Around line 8-16: Add read-only repository permissions to the phpstan job and
disable credential persistence in its actions/checkout step. Set the job’s
permissions to contents: read and add persist-credentials: false under the
existing checkout configuration.
In `@phpstan.neon`:
- Around line 4-5: Update the PHPStan paths configuration to include both src
and tests, preserving static analysis coverage for test files while retaining
the existing source analysis scope.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b99d697-9b27-4d46-9e4d-edc85ff29d2e
📒 Files selected for processing (5)
.github/workflows/static-analysis.ymlcomposer.jsonphpcs.xmlphpstan.neonsrc/ObjectGrapher.php
💤 Files with no reviewable changes (1)
- src/ObjectGrapher.php
🚧 Files skipped from review as they are similar to previous changes (2)
- phpcs.xml
- composer.json
| jobs: | ||
| phpstan: | ||
| name: PHPStan | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .github/workflows/static-analysis.yml ==\n'
wc -l .github/workflows/static-analysis.yml
cat -n .github/workflows/static-analysis.yml
printf '\n== workflow file references ==\n'
rg -n "permissions:|persist-credentials|actions/checkout|pull_request|phpstan|composer|dependency" .github/workflows -SRepository: ray-di/Ray.ObjectGrapher
Length of output: 3014
🌐 Web query:
GitHub Actions default GITHUB_TOKEN permissions pull_request public repository actions/checkout persist-credentials default v4
💡 Result:
For new repositories created in personal accounts, the default permissions for the GITHUB_TOKEN are restricted to read-only access for the contents and packages scopes [1]. For new repositories owned by organizations, the default permissions are inherited from the organization's settings, which, for newly created organizations, default to read-only [1][2]. Users can manually configure these defaults to be more permissive in the repository or organization settings [1]. Regarding the actions/checkout (v4) action: - The persist-credentials input, which determines whether to configure the token or SSH key with the local git configuration, defaults to true [3][4]. - When persist-credentials is true, the action persists the authentication token in the local git config, enabling subsequent steps to run authenticated git commands [5][6]. This token is removed during post-job cleanup [5][6]. For pull requests, the GITHUB_TOKEN is subject to specific constraints: - Pull requests from public forks always receive a read-only token, regardless of the configured repository or workflow permissions [7][8]. - Workflows triggered by pull requests have access to the GITHUB_TOKEN, but you should adhere to the principle of least privilege by explicitly defining the necessary permissions in your workflow using the permissions key [7][9]. Any permissions omitted from the permissions key are set to none [7].
Citations:
- 1: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository
- 2: https://github.blog/changelog/2023-02-02-github-actions-updating-the-default-github_token-permissions-to-read-only/
- 3: https://github.com/actions/checkout/blob/v4/action.yml
- 4: https://github.com/actions/checkout/blob/v4.1.1/action.yml
- 5: https://github.com/actions/checkout/blob/v4/README.md
- 6: https://github.com/actions/checkout
- 7: https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/
- 8: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows
- 9: https://docs.github.com/actions/reference/authentication-in-a-workflow
Tighten this job’s token access. Add permissions: contents: read and set persist-credentials: false on actions/checkout; checkout persists the token in git config by default, and this job runs repository-controlled tooling.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 13-16: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/static-analysis.yml around lines 8 - 16, Add read-only
repository permissions to the phpstan job and disable credential persistence in
its actions/checkout step. Set the job’s permissions to contents: read and add
persist-credentials: false under the existing checkout configuration.
Source: Linters/SAST tools
| paths: | ||
| - src No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(phpstan.*\.neon|\.github/workflows/.*\.ya?ml|composer\.json|package\.json)$' || true
echo
echo "== phpstan references =="
rg -n --hidden --glob '!vendor' --glob '!node_modules' 'phpstan|PHPStan' .
echo
echo "== phpstan.neon =="
if [ -f phpstan.neon ]; then cat -n phpstan.neon; fi
echo
echo "== phpstan-related workflow snippets =="
for f in $(git ls-files '.github/workflows/*' 2>/dev/null); do
if rg -n 'phpstan|PHPStan' "$f" >/dev/null; then
echo "--- $f ---"
rg -n 'phpstan|PHPStan|tests|src' "$f"
fi
doneRepository: ray-di/Ray.ObjectGrapher
Length of output: 1669
Keep tests in PHPStan’s analysis scope. phpstan.neon:4-5 now limits PHPStan to src, and the static-analysis job relies on that config, so test files are no longer analyzed. Add tests back unless dropping test coverage is intentional.
Proposed fix
paths:
- src
+ - tests📝 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.
| paths: | |
| - src | |
| paths: | |
| - src | |
| - tests |
🤖 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 `@phpstan.neon` around lines 4 - 5, Update the PHPStan paths configuration to
include both src and tests, preserving static analysis coverage for test files
while retaining the existing source analysis scope.
1 participant
Summary
Add CI following the
~/git/Ray.Aopsetup, using theray-di/.githubreusable workflows (@v1)..github/workflows/continuous-integration.yml— PHPUnit on PHP8.2,8.3,8.4(old stable) and8.5(current stable), matchingcomposer.json(php: ^8.2)..github/workflows/coding-standards.yml—phpcson PHP8.3.Notes
scriptinput omitted (nodemo/run.phpequivalent in this repo).とりあえずphpunit + cs).update-copyright-years-in-license-file.ymlis left untouched.Summary by CodeRabbit
Chores
Refactor
__toString(): string,: void) and normalized related syntax.Tests