fix(foxy): Update CI workflows and apply automated refactors. #115
Conversation
terabytesoftw
commented
Jan 23, 2026
terabytesoftw
commented
| Q | A |
|---|---|
| Is bugfix? | ✔️ |
| New feature? | ❌ |
| Breaks BC? | ❌ |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughConsolidates and extends CI workflows, upgrades QA tooling, refactors JSON encoding/formatting, expands and renames asset-manager APIs, tightens dependency immutability/visibility, restructures fallback/solver wiring, and modernizes PHPUnit tests and fixtures across the repository. Changes
Sequence Diagram(s)sequenceDiagram
participant Composer as Composer (hook)
participant Solver as Solver
participant AssetMgr as AssetManager
participant ProcExec as ProcessExecutor
participant FS as Filesystem
participant Fallback as ComposerFallback
Composer->>Solver: solve(packages, assetDir)
Solver->>AssetMgr: prepare assets / getAssets
AssetMgr->>ProcExec: execute(buildCommand(action))
ProcExec-->>AssetMgr: exitCode, output
alt exitCode == 0
AssetMgr->>FS: write package.json and lockfile
FS-->>AssetMgr: success
AssetMgr-->>Solver: return run result
else failure
AssetMgr->>Fallback: restore()
Fallback->>FS: restore lockfile or remove vendor
Fallback-->>Solver: restored / error
end
Solver-->>Composer: dispatch PostSolveEvent(result)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #115 +/- ##
===========================================
Coverage 100.00% 100.00%
+ Complexity 348 347 -1
===========================================
Files 26 26
Lines 803 799 -4
===========================================
- Hits 803 799 -4 ☔ View full report in Codecov by Sentry. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/Json/JsonFormatter.php (1)
103-137: Minor: Callback return type annotation is overly broad.The callback's return type
string|arrayon line 118 is misleading.preg_replace_callbackexpects the callback to return astring, and this callback always returns a string (either the converted result or'').♻️ Suggested fix
- static function (mixed $match): string|array { + static function (array $match): string {src/Fallback/ComposerFallback.php (1)
105-118: Add default[]togetLockValue('packages-dev').The second parameter of
Locker::setLockData()expectsarray $devPackages, notnull. WhengetLockValue('packages-dev')is called without a default, it returnsnullif the key doesn't exist, violating the type contract. All other parameters in this call already provide defaults—only this one is missing.Fix
$this->composer->getLocker()->setLockData( $this->getLockValue('packages', []), - $this->getLockValue('packages-dev'), + $this->getLockValue('packages-dev', []), $this->getLockValue('platform', []),src/Solver/Solver.php (1)
1-3: Address PHP-CS-Fixer issues flagged by pipeline.The GitHub Actions ECS workflow reports fixable formatting issues. Run
vendor/bin/ecs check --fixto resolve them before merging.
🤖 Fix all issues with AI agents
In `@composer.json`:
- Around line 69-78: The sync-metadata script in composer.json currently pulls
files from the main branch of the external repo; change each URL in the
"sync-metadata" array to reference a specific commit SHA or release tag (e.g.
replace .../template/main/... with .../template/<commit-or-tag>/...) so the
downloaded .editorconfig, .gitattributes, .gitignore, ecs.php, infection.json5,
phpstan.neon, phpunit.xml.dist and rector.php are pinned to a known immutable
revision and won’t unexpectedly change when upstream main is updated.
In `@ecs.php`:
- Around line 20-25: The NullableTypeDeclarationFixer is being configured via
withConfiguredRule(NullableTypeDeclarationFixer::class, ['syntax' => 'union'])
but the same rule is listed in the skip array, so the configuration is ignored;
fix by either removing the withConfiguredRule(...) block if you intend to keep
the rule skipped, or remove NullableTypeDeclarationFixer::class from the skip
array (the skip list) so the configured rule takes effect—update whichever of
withConfiguredRule or the skip array (referencing
NullableTypeDeclarationFixer::class and the skip entries) matches the intended
behavior.
In `@src/Asset/BunManager.php`:
- Around line 26-45: Extract the duplicated Platform::isWindows() ? 'bun.exe' :
'bun' into a private helper (e.g., private function resolveBunBinary(): string)
and update getInstallCommand, getUpdateCommand, and getVersionCommand to call
resolveBunBinary() instead of repeating the ternary; keep existing calls to
buildCommand(...) unchanged so only the binary resolution is centralized.
In `@src/Config/Config.php`:
- Line 171: Fix the formatting in the assignment expression in Config.php by
removing the extra space before array_key_exists so the expression "$value =
null === $default && array_key_exists($key, $this->defaults)" becomes properly
spaced; update the assignment in the method containing that line (the Config
class assignment to $value) so there is a single space before array_key_exists
to satisfy ECS formatting rules.
- Around line 195-198: The isInteger method currently uses
ctype_digit(trim($value, '-')) which incorrectly treats inputs like "--123" or
"123-" as valid; update Config::isInteger to perform strict validation of
optional single leading sign followed by digits (e.g., use a regex like
^[+-]?\d+$ or filter_var with FILTER_VALIDATE_INT) so only proper integers
(including single leading + or -) return true and malformed strings return
false; locate and replace the isInteger(string $value): bool implementation
accordingly.
In `@src/Converter/SemverUtil.php`:
- Around line 99-103: The code trims $end using strlen($type) after normalizing
the stability token, which can be longer than the original match (e.g., "a" →
"alpha") and causes the numeric suffix to be dropped; change the substr trimming
to use the raw matched length (strlen($matches[0])) instead of strlen($type) so
the numeric portion remains, keeping the preg_match and self::normalizeStability
usage intact in SemverUtil.
In `@src/FoxyEvents.php`:
- Around line 7-8: Remove the unused import statement "use Event;" from this
file because there is no Event class referenced and the `@Event` annotations in
docblocks are string literals; update the top of the file to delete the "use
Event;" line so there are no unused imports remaining (no other code changes
required).
In `@src/Json/JsonFile.php`:
- Around line 68-78: The write() method in JsonFile should preserve the parsed
indent and always reset the static encode state even if parent::write() throws;
change JsonFile::write to capture the parsed indent (use $this->getArrayKeys()
and the parsed indent value rather than immediately assigning
JsonFormatter::DEFAULT_INDENT), set self::$encodeArrayKeys and
self::$encodeIndent before calling parent::write, and wrap the call in a
try/finally so that self::$encodeArrayKeys and self::$encodeIndent are restored
in the finally block; reference the symbols JsonFile::write, getArrayKeys(),
parent::write(), self::$encodeArrayKeys, self::$encodeIndent, and
JsonFormatter::DEFAULT_INDENT when making the change.
- Around line 25-30: The encode() method in JsonFile::encode is ignoring the
$indent argument (it always uses self::$encodeIndent), so update the call sites
to honor the passed $indent: pass $indent into JsonFormatter::format instead of
self::$encodeIndent, and if parent::encode supports an indent parameter,
propagate $indent to parent::encode(...) as well; this preserves the public API
and ensures callers can control indentation (refer to JsonFile::encode,
self::$encodeIndent, $indent, parent::encode, and JsonFormatter::format).
In `@src/Solver/Solver.php`:
- Line 19: The file imports the global RuntimeException but the project uses a
custom Foxy\Exception\RuntimeException; update the import so Solver uses the
project exception instead of the global one and ensure any throw statements in
the Solver class (the throw at/around the current exception usage) instantiate
Foxy\Exception\RuntimeException; locate the Solver class and the throw site (the
existing throw around line 73) and replace the global exception reference with
the Foxy\Exception\RuntimeException symbol.
In `@tests/Asset/AssetManager.php`:
- Line 46: Add an import for Symfony\Component\Filesystem\Filesystem (e.g. `use
Symfony\Component\Filesystem\Filesystem as SymfonyFilesystem;`) and update the
property/type hints that currently use the fully qualified name (the protected
property `$sfs` and any other occurrences such as at line with reference to
Filesystem) to use the imported alias `SymfonyFilesystem` for consistency with
other imports; ensure all occurrences of
`\Symfony\Component\Filesystem\Filesystem` in this file (including the `$sfs`
property and its usages) are replaced with the new imported `SymfonyFilesystem`
type.
In `@tests/Config/ConfigTest.php`:
- Around line 141-149: The closure handling log messages never compares the
incoming $message to the expected strings, so $globalLogComposer and
$globalLogConfig are always set; update the static function ($message) use
($globalPath, &$globalLogComposer, &$globalLogConfig) to compare $message
against the formatted strings (e.g., compare $message === sprintf('Loading Foxy
config in file %s/composer.json', $globalPath) and similarly for config.json, or
use strpos/str_contains if partial match is intended) and only set
$globalLogComposer/$globalLogConfig to true when the comparison matches.
In `@tests/Fallback/AssetFallbackTest.php`:
- Around line 68-96: The test testRestoreThrowsWhenRemoveFails uses
fully-qualified \RuntimeException inline; change to a named import for
consistency (e.g., add "use RuntimeException;" at top) and update occurrences in
the method (testRestoreThrowsWhenRemoveFails assertions and expects) to
reference the imported RuntimeException symbol instead of the FQCN so imports
match project style.
- Around line 158-189: Add a top-level use statement for
Symfony\Component\Filesystem\Filesystem (optionally aliased) and replace the
fully-qualified instantiation in setUp() (new
\Symfony\Component\Filesystem\Filesystem()) with the imported class name so
$this->sfs = new Filesystem() (or new SymfonyFilesystem() if aliased), ensuring
tearDown() references remain correct for $this->sfs; this keeps imports
consistent and avoids inline FQN usage.
In `@tests/Fixtures/Asset/StubAssetManager.php`:
- Around line 7-23: Run the ECS fixer and apply the automated changes
(vendor/bin/ecs check --fix) to resolve the reported coding-standard issues in
this file; if anything remains, manually fix the StubAssetManager constructor
signature and imports: remove the trailing comma after the last constructor
parameter ($fs) in __construct, remove any unused use statements or
reorder/group them per project CS, ensure spacing/braces inside the class match
PSR-12 (no extra blank lines in the empty constructor body) and add a final
newline—target the StubAssetManager class and its __construct method when making
these edits.
In `@tests/Fixtures/Util/ThrowingProcessExecutorMock.php`:
- Around line 8-14: Replace the use of PHP's built-in RuntimeException with the
project's custom exception by importing and throwing
Foxy\Exception\RuntimeException in the ThrowingProcessExecutorMock::execute
override (which extends ProcessExecutor); keep the method body as a single throw
and, if static analysis flags unused parameters, add a small comment or
parameter annotations to silence false positives rather than changing logic.
In `@tests/FoxyTest.php`:
- Around line 122-123: The if-block checking PHP_VERSION_ID (< 80500) in
tests/FoxyTest.php is empty and should be fixed; either remove the entire empty
conditional (if (PHP_VERSION_ID < 80500) { }) or implement the intended
backward-compatibility logic that was meant to run for older PHP versions—locate
the empty conditional in tests/FoxyTest.php and either delete it or insert the
missing compatibility code (tests/setup, mocks, or alternative assertions) so
the test suite behavior is explicit.
- Around line 134-135: The empty conditional "if (PHP_VERSION_ID < 80500) { }"
in tests/FoxyTest.php should be fixed: either remove the entire if-block if no
special handling is needed, or implement the intended behavior (for example call
$this->markTestSkipped('Requires PHP >= 8.5') or add the compatibility fallback
code) so the block is not empty; search for the PHP_VERSION_ID < 80500 check in
class FoxyTest and apply the chosen fix consistently.
- Around line 166-172: Replace the meaningless self::assertTrue(true) in
testDeactivate and testUninstall with a clear intent: either call
$this->expectNotToPerformAssertions(); at the start of each test or add a short
inline comment stating these tests are intentionally no-ops to assert no
exceptions from Foxy::deactivate and Foxy::uninstall; locate the assertions in
the testDeactivate and testUninstall methods and update accordingly.
In `@tests/Util/PackageUtilTest.php`:
- Around line 76-79: The two assertEquals calls in PackageUtilTest currently
pass actual then expected; swap their argument order so the expected value
variables ($expectedPackages and $expectedDevPackages) are the first argument
and the actual values ($lockDataLoaded['packages'] and
$lockDataLoaded['packages-dev']) are the second. Update the assertions in
tests/Util/PackageUtilTest.php that reference $lockDataLoaded['packages'] and
$lockDataLoaded['packages-dev'] accordingly to use
assertEquals($expectedPackages, $lockDataLoaded['packages']) and
assertEquals($expectedDevPackages, $lockDataLoaded['packages-dev']).
- Around line 50-51: The test asserts use assertEquals with parameters reversed;
change the call to assertEquals so the first argument is $expectedAliases and
the second is $convertedAliases['aliases'] (i.e., swap the expected and actual
arguments) while keeping the existing assertArrayHasKey('aliases',
$convertedAliases) check and references to $convertedAliases and
$expectedAliases intact.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (81)
.editorconfig.gitattributes.github/workflows/dependency-check.yml.github/workflows/ecs.yml.github/workflows/linter.yml.github/workflows/mutation.yml.github/workflows/static.yml.gitignoreCHANGELOG.mdcomposer.jsonecs.phpinfection.json5phpstan-baseline.neonphpstan.neonphpunit.xml.distrector.phpruntime/.gitignoresrc/Asset/AbstractAssetManager.phpsrc/Asset/AssetManagerFinder.phpsrc/Asset/AssetManagerInterface.phpsrc/Asset/AssetPackage.phpsrc/Asset/AssetPackageInterface.phpsrc/Asset/BunManager.phpsrc/Asset/NpmManager.phpsrc/Asset/PnpmManager.phpsrc/Asset/YarnManager.phpsrc/Config/Config.phpsrc/Config/ConfigBuilder.phpsrc/Converter/SemverConverter.phpsrc/Converter/SemverUtil.phpsrc/Converter/VersionConverterInterface.phpsrc/Event/AbstractSolveEvent.phpsrc/Event/GetAssetsEvent.phpsrc/Event/PostSolveEvent.phpsrc/Event/PreSolveEvent.phpsrc/Exception/ExceptionInterface.phpsrc/Exception/RuntimeException.phpsrc/Fallback/AssetFallback.phpsrc/Fallback/ComposerFallback.phpsrc/Fallback/FallbackInterface.phpsrc/Foxy.phpsrc/FoxyEvents.phpsrc/Json/JsonFile.phpsrc/Json/JsonFormatter.phpsrc/Solver/Solver.phpsrc/Solver/SolverInterface.phpsrc/Util/AssetUtil.phpsrc/Util/ComposerUtil.phpsrc/Util/ConsoleUtil.phpsrc/Util/LockerUtil.phpsrc/Util/PackageUtil.phptests/Asset/AssetManager.phptests/Asset/AssetManagerFinderTest.phptests/Asset/AssetPackageTest.phptests/Asset/BunAssetManagerTest.phptests/Asset/NpmAssetManagerTest.phptests/Asset/PnpmAssetManagerTest.phptests/Asset/YarnAssetManagerTest.phptests/Asset/YarnNextAssetManagerTest.phptests/Config/ConfigTest.phptests/Converter/SemverConverterTest.phptests/Event/GetAssetsEventTest.phptests/Event/PostSolveEventTest.phptests/Event/PreSolveEventTest.phptests/Event/SolveEvent.phptests/Fallback/AssetFallbackTest.phptests/Fallback/ComposerFallbackTest.phptests/Fixtures/Asset/StubAssetManager.phptests/Fixtures/Util/AbstractProcessExecutorMock.phptests/Fixtures/Util/ProcessExecutorMock.phptests/Fixtures/Util/ProcessExecutorMockTest.phptests/Fixtures/Util/ThrowingProcessExecutorMock.phptests/FoxyTest.phptests/Json/JsonFileTest.phptests/Json/JsonFormatterTest.phptests/Solver/SolverTest.phptests/Support/InternalMockerExtension.phptests/Util/AssetUtilTest.phptests/Util/ComposerUtilTest.phptests/Util/ConsoleUtilTest.phptests/Util/PackageUtilTest.php
💤 Files with no reviewable changes (7)
- tests/Event/PreSolveEventTest.php
- src/Event/PreSolveEvent.php
- src/Util/ComposerUtil.php
- src/Converter/VersionConverterInterface.php
- src/Exception/RuntimeException.php
- src/Exception/ExceptionInterface.php
- src/Solver/SolverInterface.php
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
📚 Learning: 2026-01-22T11:42:32.878Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 111
File: tests/Fallback/AssetFallbackTest.php:98-98
Timestamp: 2026-01-22T11:42:32.878Z
Learning: When using xepozz/internal-mocker in PHP tests, the library's namespaced wrapper records and matches function calls with all default parameters filled in. To ensure proper matching, always specify every parameter in MockerState::addCondition(), including defaults. For example, a call to file_get_contents($path) is observed as file_get_contents($path, false, null, 0, null). Use the same complete argument list (including default values) when adding conditions for mocks in tests like tests/Fallback/AssetFallbackTest.php.
Applied to files:
tests/Event/PostSolveEventTest.phptests/Support/InternalMockerExtension.phptests/Asset/PnpmAssetManagerTest.phptests/Util/ComposerUtilTest.phptests/Asset/AssetPackageTest.phptests/Fixtures/Util/ProcessExecutorMock.phptests/Fixtures/Util/ThrowingProcessExecutorMock.phptests/Asset/NpmAssetManagerTest.phptests/Event/GetAssetsEventTest.phptests/Util/PackageUtilTest.phptests/Converter/SemverConverterTest.phptests/Solver/SolverTest.phptests/Config/ConfigTest.phptests/Fallback/AssetFallbackTest.phptests/Asset/AssetManager.phptests/Fallback/ComposerFallbackTest.phptests/Util/AssetUtilTest.phptests/Json/JsonFormatterTest.phptests/Fixtures/Util/ProcessExecutorMockTest.phptests/Fixtures/Util/AbstractProcessExecutorMock.phptests/Util/ConsoleUtilTest.phptests/Event/SolveEvent.phptests/Fixtures/Asset/StubAssetManager.phptests/FoxyTest.phptests/Asset/BunAssetManagerTest.phptests/Asset/AssetManagerFinderTest.phptests/Json/JsonFileTest.phptests/Asset/YarnAssetManagerTest.phptests/Asset/YarnNextAssetManagerTest.php
📚 Learning: 2025-09-12T10:52:52.955Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 55
File: .github/workflows/reusable/linter.yml:65-79
Timestamp: 2025-09-12T10:52:52.955Z
Learning: For Super Linter workflows, always recommend using the official super-linter/super-linter action instead of docker run commands, as the action properly handles GitHub context variables and annotations automatically.
Applied to files:
.github/workflows/linter.yml
📚 Learning: 2025-08-18T15:43:30.996Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: composer.json:13-16
Timestamp: 2025-08-18T15:43:30.996Z
Learning: Testing helper libraries (packages with "testing" or "support" in keywords/description that provide test utilities) should include PHPUnit and other testing frameworks in the main "require" section rather than "require-dev", as consuming packages need these dependencies to use the testing utilities.
Applied to files:
tests/Asset/AssetPackageTest.phptests/Util/PackageUtilTest.phptests/Util/AssetUtilTest.phpcomposer.jsontests/Util/ConsoleUtilTest.phptests/Json/JsonFileTest.php
📚 Learning: 2025-09-29T14:58:04.095Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Applied to files:
.github/workflows/dependency-check.ymlcomposer.jsonsrc/Asset/NpmManager.phptests/Asset/YarnAssetManagerTest.phptests/Asset/YarnNextAssetManagerTest.php
📚 Learning: 2025-08-18T16:13:31.606Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: phpstan.neon:1-3
Timestamp: 2025-08-18T16:13:31.606Z
Learning: In the php-forge/support project, PHPStan is intended to be used as a PHAR installation rather than as a Composer dependency, even though the composer.json was updated to include phpstan/phpstan as a dependency.
Applied to files:
composer.json
🧬 Code graph analysis (35)
tests/Event/PostSolveEventTest.php (2)
src/Event/PostSolveEvent.php (1)
getRunResult(27-30)tests/Event/PreSolveEventTest.php (1)
PreSolveEventTest(25-31)
src/Event/PostSolveEvent.php (2)
src/Event/AbstractSolveEvent.php (1)
__construct(19-22)src/Event/GetAssetsEvent.php (1)
__construct(19-22)
tests/Asset/PnpmAssetManagerTest.php (4)
tests/Asset/AssetManager.php (5)
getValidInstallCommand(50-50)getValidLockPackageName(52-52)getValidName(54-54)getValidUpdateCommand(56-56)getValidVersionCommand(58-58)tests/Asset/NpmAssetManagerTest.php (5)
getValidInstallCommand(16-19)getValidLockPackageName(21-24)getValidName(26-29)getValidUpdateCommand(31-34)getValidVersionCommand(36-39)tests/Asset/YarnAssetManagerTest.php (5)
getValidInstallCommand(33-36)getValidLockPackageName(38-41)getValidName(43-46)getValidUpdateCommand(48-51)getValidVersionCommand(53-56)tests/Asset/YarnNextAssetManagerTest.php (5)
getValidInstallCommand(31-34)getValidLockPackageName(36-39)getValidName(41-44)getValidUpdateCommand(46-49)getValidVersionCommand(51-54)
tests/Util/ComposerUtilTest.php (3)
src/Foxy.php (1)
Foxy(24-199)src/Exception/RuntimeException.php (1)
RuntimeException(7-9)src/Util/ComposerUtil.php (1)
ComposerUtil(11-38)
tests/Asset/AssetPackageTest.php (1)
src/Asset/AssetPackageInterface.php (6)
addNewDependencies(16-16)getPackage(28-28)getInstalledDependencies(23-23)removeUnusedDependencies(35-35)setPackage(42-42)write(47-47)
src/FoxyEvents.php (2)
src/Event/PreSolveEvent.php (1)
PreSolveEvent(24-36)tests/Event/PreSolveEventTest.php (1)
PreSolveEventTest(25-31)
tests/Fixtures/Util/ProcessExecutorMock.php (2)
tests/Fixtures/Util/AbstractProcessExecutorMock.php (2)
AbstractProcessExecutorMock(11-107)doExecute(30-39)tests/Fixtures/Util/ThrowingProcessExecutorMock.php (1)
execute(12-15)
tests/Fixtures/Util/ThrowingProcessExecutorMock.php (2)
src/Exception/RuntimeException.php (2)
RuntimeException(7-9)RuntimeException(21-23)tests/Fixtures/Util/ProcessExecutorMock.php (1)
execute(9-12)
src/Asset/AssetPackage.php (1)
src/Asset/AssetPackageInterface.php (4)
getInstalledDependencies(23-23)getPackage(28-28)setPackage(42-42)write(47-47)
tests/Event/GetAssetsEventTest.php (1)
src/Event/GetAssetsEvent.php (3)
hasAsset(56-59)addAsset(36-41)getAssets(46-49)
tests/Util/PackageUtilTest.php (1)
src/Util/PackageUtil.php (2)
PackageUtil(10-93)loadLockPackages(86-92)
tests/Converter/SemverConverterTest.php (2)
src/Converter/SemverConverter.php (2)
SemverConverter(14-30)convertVersion(16-29)src/Converter/VersionConverterInterface.php (1)
convertVersion(16-16)
src/Asset/YarnManager.php (5)
src/Asset/AssetManagerInterface.php (2)
getLockPackageName(24-24)getName(29-29)src/Asset/BunManager.php (3)
getLockPackageName(11-14)getName(16-19)getVersionCommand(40-45)src/Asset/NpmManager.php (3)
getLockPackageName(9-12)getName(14-17)getVersionCommand(36-39)src/Asset/PnpmManager.php (3)
getLockPackageName(9-12)getName(14-17)getVersionCommand(34-37)src/Asset/AbstractAssetManager.php (3)
getVersionCommand(59-59)buildCommand(235-246)getVersion(293-303)
src/Config/Config.php (2)
src/Asset/AbstractAssetManager.php (1)
__construct(35-44)src/Fallback/AssetFallback.php (1)
__construct(19-26)
tests/Solver/SolverTest.php (4)
src/Solver/Solver.php (3)
Solver(26-136)setUpdatable(41-46)solve(51-75)src/Asset/AbstractAssetManager.php (2)
setUpdatable(182-187)getPackageName(81-84)src/Asset/AssetManagerInterface.php (2)
setUpdatable(78-78)getPackageName(34-34)src/Solver/SolverInterface.php (2)
setUpdatable(17-17)solve(25-25)
tests/Config/ConfigTest.php (3)
src/Config/Config.php (3)
Config(21-209)getArray(65-70)get(39-57)src/Config/ConfigBuilder.php (1)
ConfigBuilder(16-96)src/Exception/RuntimeException.php (1)
RuntimeException(7-9)
tests/Fallback/AssetFallbackTest.php (3)
src/Fallback/AssetFallback.php (3)
AssetFallback(13-69)save(55-68)restore(28-53)src/Fallback/FallbackInterface.php (2)
save(17-17)restore(12-12)src/Exception/RuntimeException.php (1)
RuntimeException(7-9)
src/Converter/SemverConverter.php (1)
src/Converter/VersionConverterInterface.php (1)
convertVersion(16-16)
src/Event/AbstractSolveEvent.php (2)
src/Event/GetAssetsEvent.php (1)
__construct(19-22)src/Event/PreSolveEvent.php (3)
__construct(18-21)PreSolveEvent(24-36)__construct(32-35)
src/Solver/Solver.php (2)
src/Foxy.php (1)
Foxy(24-199)src/Exception/RuntimeException.php (1)
RuntimeException(7-9)
src/Json/JsonFile.php (1)
src/Json/JsonFormatter.php (2)
JsonFormatter(25-162)format(45-60)
tests/Fallback/ComposerFallbackTest.php (3)
src/Fallback/ComposerFallback.php (3)
ComposerFallback(20-162)save(66-81)restore(47-64)src/Util/LockerUtil.php (2)
LockerUtil(12-26)getLocker(17-25)src/Fallback/FallbackInterface.php (2)
save(17-17)restore(12-12)
tests/Json/JsonFormatterTest.php (1)
src/Json/JsonFormatter.php (4)
JsonFormatter(25-162)format(45-60)getArrayKeys(69-74)getIndent(81-91)
tests/Util/ConsoleUtilTest.php (1)
src/Util/ConsoleUtil.php (3)
ConsoleUtil(17-78)getInput(24-38)getPreferredInstallOptions(48-77)
tests/Event/SolveEvent.php (2)
src/Event/AbstractSolveEvent.php (3)
AbstractSolveEvent(10-41)getAssetDir(27-30)getPackages(37-40)src/Event/PreSolveEvent.php (1)
PreSolveEvent(24-36)
tests/Fixtures/Asset/StubAssetManager.php (2)
src/Asset/AssetManagerInterface.php (10)
addDependencies(19-19)getLockPackageName(24-24)getName(29-29)hasLockFile(39-39)isAvailable(44-44)isInstalled(49-49)run(64-64)setFallback(71-71)setUpdatable(78-78)validate(86-86)src/Solver/SolverInterface.php (1)
setUpdatable(17-17)
tests/FoxyTest.php (3)
src/Foxy.php (6)
Foxy(24-199)activate(79-99)init(119-130)initOnInstall(137-144)getSubscribedEvents(106-114)solveAssets(161-165)src/Exception/RuntimeException.php (1)
RuntimeException(7-9)src/Fallback/AssetFallback.php (1)
AssetFallback(13-69)
src/Asset/NpmManager.php (5)
src/Asset/AssetManagerInterface.php (2)
getLockPackageName(24-24)getName(29-29)src/Asset/BunManager.php (3)
getLockPackageName(11-14)getName(16-19)getVersionCommand(40-45)src/Asset/PnpmManager.php (3)
getLockPackageName(9-12)getName(14-17)getVersionCommand(34-37)src/Asset/YarnManager.php (3)
getLockPackageName(11-14)getName(16-19)getVersionCommand(49-52)src/Asset/AbstractAssetManager.php (3)
actionWhenComposerDependenciesAreAlreadyInstalled(223-226)getVersionCommand(59-59)buildCommand(235-246)
tests/Asset/BunAssetManagerTest.php (4)
tests/Asset/AssetManager.php (5)
getValidInstallCommand(50-50)getValidLockPackageName(52-52)getValidName(54-54)getValidUpdateCommand(56-56)getValidVersionCommand(58-58)tests/Asset/NpmAssetManagerTest.php (5)
getValidInstallCommand(16-19)getValidLockPackageName(21-24)getValidName(26-29)getValidUpdateCommand(31-34)getValidVersionCommand(36-39)tests/Asset/YarnAssetManagerTest.php (5)
getValidInstallCommand(33-36)getValidLockPackageName(38-41)getValidName(43-46)getValidUpdateCommand(48-51)getValidVersionCommand(53-56)tests/Asset/YarnNextAssetManagerTest.php (5)
getValidInstallCommand(31-34)getValidLockPackageName(36-39)getValidName(41-44)getValidUpdateCommand(46-49)getValidVersionCommand(51-54)
tests/Asset/AssetManagerFinderTest.php (2)
src/Asset/AssetManagerFinder.php (2)
AssetManagerFinder(11-79)findManager(43-54)src/Exception/RuntimeException.php (1)
RuntimeException(7-9)
tests/Json/JsonFileTest.php (3)
src/Exception/RuntimeException.php (1)
RuntimeException(7-9)src/Json/JsonFile.php (5)
JsonFile(9-99)getArrayKeys(37-44)getIndent(49-56)read(58-66)write(68-78)src/Json/JsonFormatter.php (2)
getArrayKeys(69-74)getIndent(81-91)
src/Asset/BunManager.php (5)
src/Asset/AssetManagerInterface.php (3)
getLockPackageName(24-24)getName(29-29)isInstalled(49-49)src/Asset/NpmManager.php (5)
getLockPackageName(9-12)getName(14-17)getInstallCommand(26-29)getUpdateCommand(31-34)getVersionCommand(36-39)src/Asset/PnpmManager.php (6)
getLockPackageName(9-12)getName(14-17)isInstalled(19-22)getInstallCommand(24-27)getUpdateCommand(29-32)getVersionCommand(34-37)src/Asset/YarnManager.php (6)
getLockPackageName(11-14)getName(16-19)isInstalled(21-24)getInstallCommand(37-40)getUpdateCommand(42-47)getVersionCommand(49-52)src/Asset/AbstractAssetManager.php (6)
isInstalled(96-99)getLockFilePath(248-251)getInstallCommand(49-49)buildCommand(235-246)getUpdateCommand(54-54)getVersionCommand(59-59)
src/Asset/AbstractAssetManager.php (3)
src/Converter/SemverConverter.php (2)
SemverConverter(14-30)convertVersion(16-29)src/Asset/NpmManager.php (1)
getLockPackageName(9-12)src/Converter/VersionConverterInterface.php (1)
convertVersion(16-16)
src/Foxy.php (5)
src/Asset/BunManager.php (2)
BunManager(9-46)getName(16-19)src/Asset/NpmManager.php (2)
NpmManager(7-40)getName(14-17)src/Asset/PnpmManager.php (2)
PnpmManager(7-38)getName(14-17)src/Asset/YarnManager.php (2)
YarnManager(9-71)getName(16-19)src/Asset/AssetManagerInterface.php (1)
getName(29-29)
src/Asset/AssetManagerInterface.php (1)
src/Asset/AbstractAssetManager.php (6)
addDependencies(61-74)isAvailable(91-94)run(111-173)setFallback(175-180)setUpdatable(182-187)validate(189-214)
🪛 GitHub Actions: ecs
tests/Support/InternalMockerExtension.php
[error] 1-1: PHP-CS-Fixer reported fixable issues detected in this file. Run 'vendor/bin/ecs check --fix' to apply changes.
src/Config/Config.php
[error] 1-1: PHP-CS-Fixer reported fixable issues detected in this file. Run 'vendor/bin/ecs check --fix' to apply changes.
src/Solver/Solver.php
[error] 1-1: PHP-CS-Fixer reported fixable issues detected in this file. Run 'vendor/bin/ecs check --fix' to apply changes.
tests/Fixtures/Asset/StubAssetManager.php
[error] 1-1: PHP-CS-Fixer reported fixable issues detected in this file. Run 'vendor/bin/ecs check --fix' to apply changes.
🪛 PHPMD (2.15.0)
tests/Fixtures/Util/ThrowingProcessExecutorMock.php
12-12: Avoid unused parameters such as '$command'. (undefined)
(UnusedFormalParameter)
12-12: Avoid unused parameters such as '$output'. (undefined)
(UnusedFormalParameter)
12-12: Avoid unused parameters such as '$cwd'. (undefined)
(UnusedFormalParameter)
tests/Util/PackageUtilTest.php
74-74: Avoid using static access to class '\Foxy\Util\PackageUtil' in method 'testLoadLockPackages'. (undefined)
(StaticAccess)
84-84: Avoid using static access to class '\Foxy\Util\PackageUtil' in method 'testLoadLockPackagesWithoutPackages'. (undefined)
(StaticAccess)
src/Fallback/ComposerFallback.php
22-22: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
35-35: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
38-38: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
58-63: The method restore uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
88-88: Avoid using static access to class '\Composer\Installer' in method 'getInstaller'. (undefined)
(StaticAccess)
tests/Solver/SolverTest.php
37-37: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
38-38: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
39-39: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
89-94: The method testSolve uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
96-96: Avoid excessively long variable names like $requirePackageFilename. Keep variable name length under 20. (undefined)
(LongVariable)
141-141: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
143-146: The method setUp uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
tests/Config/ConfigTest.php
28-28: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
71-71: Avoid using static access to class '\Foxy\Config\ConfigBuilder' in method 'testGetArrayConfig'. (undefined)
(StaticAccess)
141-141: Avoid unused parameters such as '$message'. (undefined)
(UnusedFormalParameter)
192-192: Avoid using static access to class '\Foxy\Config\ConfigBuilder' in method 'testGetEnvConfigWithInvalidJson'. (undefined)
(StaticAccess)
193-193: Avoid variables with short names like $ex. Configured minimum length is 3. (undefined)
(ShortVariable)
tests/Fallback/AssetFallbackTest.php
26-26: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
27-27: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
63-65: The method testRestore uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
79-79: Missing class import via use statement (line '79', column '38'). (undefined)
(MissingImport)
111-111: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testRestoreThrowsWhenWriteFails'. (undefined)
(StaticAccess)
150-150: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testSaveThrowsWhenFileCannotBeRead'. (undefined)
(StaticAccess)
167-167: Missing class import via use statement (line '167', column '26'). (undefined)
(MissingImport)
src/Json/JsonFormatter.php
49-49: The method format has a boolean flag argument $formatJson, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
src/Solver/Solver.php
36-36: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
122-122: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'getMockPackagePath'. (undefined)
(StaticAccess)
tests/Asset/AssetManager.php
38-38: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
40-40: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
281-281: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testHasLockFileWithoutRootPackageDirAndGetcwdFailure'. (undefined)
(StaticAccess)
294-294: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testHasLockFileWithRelativeRootPackageDirAndGetcwdFailure'. (undefined)
(StaticAccess)
307-307: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testHasLockFileWithRootPackageDirAsRoot'. (undefined)
(StaticAccess)
379-381: The method testRunForInstallCommand uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
424-424: Avoid using static access to class '\Composer\Util\ProcessExecutor' in method 'testRunRestoresTimeoutWhenExecutorThrows'. (undefined)
(StaticAccess)
427-427: Avoid using static access to class '\Composer\Util\ProcessExecutor' in method 'testRunRestoresTimeoutWhenExecutorThrows'. (undefined)
(StaticAccess)
443-443: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testRunWithChdirFailure'. (undefined)
(StaticAccess)
472-472: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testRunWithChdirRestoreFailure'. (undefined)
(StaticAccess)
473-473: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testRunWithChdirRestoreFailure'. (undefined)
(StaticAccess)
663-663: Missing class import via use statement (line '663', column '26'). (undefined)
(MissingImport)
src/Json/JsonFile.php
25-25: Avoid unused parameters such as '$indent'. (undefined)
(UnusedFormalParameter)
29-29: Avoid using static access to class 'Foxy\Json\JsonFormatter' in method 'encode'. (undefined)
(StaticAccess)
tests/Fallback/ComposerFallbackTest.php
35-35: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
38-38: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
83-83: Avoid variables with short names like $ed. Configured minimum length is 3. (undefined)
(ShortVariable)
87-87: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
91-91: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
96-96: Avoid using static access to class '\Foxy\Util\LockerUtil' in method 'testRestore'. (undefined)
(StaticAccess)
114-117: The method testRestore uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
141-141: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
145-145: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
175-175: Missing class import via use statement (line '175', column '26'). (undefined)
(MissingImport)
tests/Util/AssetUtilTest.php
127-136: The method testFormatPackage uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
140-140: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testFormatPackage'. (undefined)
(StaticAccess)
150-150: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testGetName'. (undefined)
(StaticAccess)
158-158: The method testGetPathWithExtraActivation has a boolean flag argument $fileExists, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
184-186: The method testGetPathWithExtraActivation uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
188-188: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testGetPathWithExtraActivation'. (undefined)
(StaticAccess)
210-210: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testGetPathWithoutRequiredFoxy'. (undefined)
(StaticAccess)
223-223: The method testGetPathWithRequiredFoxy has a boolean flag argument $fileExists, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
241-243: The method testGetPathWithRequiredFoxy uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
251-253: The method testGetPathWithRequiredFoxy uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
255-255: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testGetPathWithRequiredFoxy'. (undefined)
(StaticAccess)
285-285: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testGetPathWithRootPackageDir'. (undefined)
(StaticAccess)
293-293: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testHasNoPluginDependency'. (undefined)
(StaticAccess)
300-306: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testHasPluginDependency'. (undefined)
(StaticAccess)
329-329: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testIsProjectActivation'. (undefined)
(StaticAccess)
350-350: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'testIsProjectActivationWithWildcardPattern'. (undefined)
(StaticAccess)
tests/Json/JsonFormatterTest.php
39-39: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testFormat'. (undefined)
(StaticAccess)
40-40: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testFormat'. (undefined)
(StaticAccess)
40-40: Avoid using static access to class '\Foxy\Json\JsonFormatter' in method 'testFormat'. (undefined)
(StaticAccess)
50-50: Avoid using static access to class '\Foxy\Json\JsonFormatter' in method 'testFormatWithEmptyContent'. (undefined)
(StaticAccess)
67-67: Avoid using static access to class '\Foxy\Json\JsonFormatter' in method 'testGetArrayKeys'. (undefined)
(StaticAccess)
78-78: Avoid using static access to class '\Foxy\Json\JsonFormatter' in method 'testGetArrayKeysWithoutSpacesBeforeArray'. (undefined)
(StaticAccess)
93-93: Avoid using static access to class '\Foxy\Json\JsonFormatter' in method 'testGetIndent'. (undefined)
(StaticAccess)
113-113: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testUnescapeSlashes'. (undefined)
(StaticAccess)
114-114: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testUnescapeSlashes'. (undefined)
(StaticAccess)
114-114: Avoid using static access to class '\Foxy\Json\JsonFormatter' in method 'testUnescapeSlashes'. (undefined)
(StaticAccess)
134-134: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testUnescapeUnicode'. (undefined)
(StaticAccess)
135-135: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testUnescapeUnicode'. (undefined)
(StaticAccess)
135-135: Avoid using static access to class '\Foxy\Json\JsonFormatter' in method 'testUnescapeUnicode'. (undefined)
(StaticAccess)
tests/Fixtures/Util/ProcessExecutorMockTest.php
18-18: Avoid using undefined variables such as '$output' which will lead to PHP notices. (undefined)
(UndefinedVariable)
19-19: Avoid using undefined variables such as '$output2' which will lead to PHP notices. (undefined)
(UndefinedVariable)
37-37: Avoid using undefined variables such as '$output' which will lead to PHP notices. (undefined)
(UndefinedVariable)
38-38: Avoid using undefined variables such as '$output2' which will lead to PHP notices. (undefined)
(UndefinedVariable)
45-45: Avoid using undefined variables such as '$output' which will lead to PHP notices. (undefined)
(UndefinedVariable)
59-59: Avoid using undefined variables such as '$output' which will lead to PHP notices. (undefined)
(UndefinedVariable)
tests/Util/ConsoleUtilTest.php
32-32: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
34-34: Avoid using static access to class '\Foxy\Util\ConsoleUtil' in method 'testGetInput'. (undefined)
(StaticAccess)
40-40: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
42-42: Avoid using static access to class '\Foxy\Util\ConsoleUtil' in method 'testGetInputWithoutValidInput'. (undefined)
(StaticAccess)
66-66: Avoid using static access to class '\Foxy\Util\ConsoleUtil' in method 'testGetPreferredInstallOptions'. (undefined)
(StaticAccess)
src/Fallback/AssetFallback.php
15-15: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
20-20: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
23-23: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
tests/Fixtures/Asset/StubAssetManager.php
18-18: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
18-18: Avoid unused parameters such as '$io'. (undefined)
(UnusedFormalParameter)
19-19: Avoid unused parameters such as '$config'. (undefined)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$executor'. (undefined)
(UnusedFormalParameter)
21-21: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
21-21: Avoid unused parameters such as '$fs'. (undefined)
(UnusedFormalParameter)
25-25: Avoid unused parameters such as '$rootPackage'. (undefined)
(UnusedFormalParameter)
25-25: Avoid unused parameters such as '$dependencies'. (undefined)
(UnusedFormalParameter)
75-75: Avoid unused parameters such as '$fallback'. (undefined)
(UnusedFormalParameter)
80-80: Avoid unused parameters such as '$updatable'. (undefined)
(UnusedFormalParameter)
src/Config/ConfigBuilder.php
54-54: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
tests/FoxyTest.php
34-34: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
68-68: Avoid excessively long variable names like $assetFallbackProperty. Keep variable name length under 20. (undefined)
(LongVariable)
176-176: Avoid using static access to class '\Foxy\Foxy' in method 'testGetSubscribedEvents'. (undefined)
(StaticAccess)
223-223: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
230-230: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
tests/Asset/BunAssetManagerTest.php
19-19: Avoid using static access to class '\Composer\Util\Platform' in method 'getValidInstallCommand'. (undefined)
(StaticAccess)
34-34: Avoid using static access to class '\Composer\Util\Platform' in method 'getValidUpdateCommand'. (undefined)
(StaticAccess)
39-39: Avoid using static access to class '\Composer\Util\Platform' in method 'getValidVersionCommand'. (undefined)
(StaticAccess)
src/Converter/SemverUtil.php
177-177: Avoid using static access to class '\Composer\Package\Version\VersionParser' in method 'normalizeStability'. (undefined)
(StaticAccess)
tests/Asset/AssetManagerFinderTest.php
15-15: Avoid variables with short names like $am. Configured minimum length is 3. (undefined)
(ShortVariable)
33-33: Avoid variables with short names like $am. Configured minimum length is 3. (undefined)
(ShortVariable)
54-54: Avoid variables with short names like $am. Configured minimum length is 3. (undefined)
(ShortVariable)
70-70: Avoid variables with short names like $am. Configured minimum length is 3. (undefined)
(ShortVariable)
81-81: Avoid variables with short names like $am. Configured minimum length is 3. (undefined)
(ShortVariable)
tests/Json/JsonFileTest.php
35-35: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testGetArrayKeysThrowsWhenFileCannotBeRead'. (undefined)
(StaticAccess)
141-141: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testWriteForcesFourSpacesIndentWithExistingTwoSpaceFile'. (undefined)
(StaticAccess)
142-142: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testWriteForcesFourSpacesIndentWithExistingTwoSpaceFile'. (undefined)
(StaticAccess)
219-219: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testWriteWithExistingFile'. (undefined)
(StaticAccess)
220-220: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testWriteWithExistingFile'. (undefined)
(StaticAccess)
248-248: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testWriteWithoutFile'. (undefined)
(StaticAccess)
249-249: Avoid using static access to class '\PHPForge\Support\LineEndingNormalizer' in method 'testWriteWithoutFile'. (undefined)
(StaticAccess)
src/Asset/BunManager.php
28-28: Avoid using static access to class '\Composer\Util\Platform' in method 'getInstallCommand'. (undefined)
(StaticAccess)
35-35: Avoid using static access to class '\Composer\Util\Platform' in method 'getUpdateCommand'. (undefined)
(StaticAccess)
42-42: Avoid using static access to class '\Composer\Util\Platform' in method 'getVersionCommand'. (undefined)
(StaticAccess)
src/Asset/AbstractAssetManager.php
36-36: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
39-39: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
68-68: Avoid excessively long variable names like $alreadyInstalledDependencies. Keep variable name length under 20. (undefined)
(LongVariable)
238-238: Avoid using static access to class '\Composer\Util\Platform' in method 'buildCommand'. (undefined)
(StaticAccess)
297-297: Avoid using undefined variables such as '$version' which will lead to PHP notices. (undefined)
(UndefinedVariable)
298-298: Avoid using undefined variables such as '$version' which will lead to PHP notices. (undefined)
(UndefinedVariable)
src/Foxy.php
109-109: Avoid using static access to class '\Foxy\Util\ComposerUtil' in method 'getSubscribedEvents'. (undefined)
(StaticAccess)
167-167: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
167-167: Avoid unused parameters such as '$composer'. (undefined)
(UnusedFormalParameter)
167-167: Avoid unused parameters such as '$io'. (undefined)
(UnusedFormalParameter)
183-183: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
186-186: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: phpstan / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.5-windows-2022
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.2-windows-2022
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Json/JsonFormatter.php (1)
93-133: Add guard for scalar JSON values inarray_walk_recursivecalls.When
json_decode($json, true)is called on scalar JSON (e.g.,"foo",123,null), it returns the scalar value directly rather than an array, causingarray_walk_recursiveto throw aTypeError. Add anis_array()check before line 109 to handle scalar JSON safely. Theext-mbstringdependency is already required incomposer.json:16.Suggested fix
$array = json_decode($json, true, 512, JSON_THROW_ON_ERROR); + if (!is_array($array)) { + return json_encode($array, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES); + }
🤖 Fix all issues with AI agents
In `@composer.json`:
- Line 24: composer.json currently allows "infection/infection": "^0.27|^0.32"
which lets Composer pick 0.32.x (requires PHP ^8.2); update the constraint in
composer.json to restrict infection to the 0.27.x series (e.g. "^0.27") so it
remains compatible with the project's PHP ^8.1 requirement, or alternatively
update the project's "php" constraint to ^8.2 if you intend to use infection
0.32.x.
In `@src/Config/Config.php`:
- Around line 70-78: convertBoolean currently compares the raw $value against
lowercase truthy strings and therefore fails for uppercase/mixed inputs; modify
convertBoolean (the private method convertBoolean) to normalize the input (e.g.,
call strtolower or mb_strtolower on $value) before running in_array so it
matches the same normalization used in isBoolean, ensuring values like "TRUE",
"Yes", "Y", etc. are treated as true.
In `@src/Json/JsonFormatter.php`:
- Around line 33-57: In JsonFormatter::format, avoid using str_replace(' ',
...) which corrupts JSON string values; instead replace only leading indentation
from formatInternal output when $indent !== 4 by applying a line-start scoped
replacement (e.g., use a multiline regex to replace only occurrences of four
spaces at beginning of lines, repeated as needed to map default indent to the
requested $indent) and keep the rest of the method flow intact; update tests to
include a JSON string value containing four consecutive spaces to assert
internal string spacing is preserved when $indent differs from the default.
♻️ Duplicate comments (3)
tests/Fixtures/Asset/StubAssetManager.php (1)
15-22: Suppress PHPMD warnings for fixture-only unused parameters.PHPMD flags unused/short parameters on Lines 18-21 and Lines 74-79; if PHPMD is enforced, this will fail CI. Consider suppressing these warnings for this stub fixture (or storing the args in properties).
🧹 Suggested suppression at class level
-final class StubAssetManager implements AssetManagerInterface +/** + * `@SuppressWarnings`(PHPMD.UnusedFormalParameter) + * `@SuppressWarnings`(PHPMD.ShortVariable) + */ +final class StubAssetManager implements AssetManagerInterfaceAlso applies to: 74-82
src/Config/Config.php (1)
193-196:isInteger()accepts malformed values.The trim-based check treats values like
"--123"or"123-"as valid integers. A stricter check prevents invalid env values from being converted.🛠️ Proposed fix
private function isInteger(string $value): bool { - return ctype_digit(trim($value, '-')); + return (bool) preg_match('/^-?\d+$/', $value); }src/Solver/Solver.php (1)
12-24: UseFoxy\Exception\RuntimeExceptionfor solver failures.Throwing the global
RuntimeExceptionbypasses the project’sExceptionInterface, which can break callers that catch it.🔧 Proposed fix
-use RuntimeException; +use Foxy\Exception\RuntimeException;Also applies to: 72-72
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.styleci.ymlcomposer.jsonsrc/Config/Config.phpsrc/Exception/ExceptionInterface.phpsrc/Exception/RuntimeException.phpsrc/Json/JsonFormatter.phpsrc/Solver/Solver.phptests/Fixtures/Asset/StubAssetManager.phptests/Support/InternalMockerExtension.php
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
📚 Learning: 2025-08-18T16:13:31.606Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: phpstan.neon:1-3
Timestamp: 2025-08-18T16:13:31.606Z
Learning: In the php-forge/support project, PHPStan is intended to be used as a PHAR installation rather than as a Composer dependency, even though the composer.json was updated to include phpstan/phpstan as a dependency.
Applied to files:
composer.json
📚 Learning: 2026-01-23T11:09:08.789Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: composer.json:68-77
Timestamp: 2026-01-23T11:09:08.789Z
Learning: When a composer.json in php-forge repositories references external template repos in sync-metadata scripts, prefer using the main branch reference rather than pinning to specific commit SHAs or tags. This reduces maintenance when upstream templates change, but be aware of potential drift; document rationale and review changes periodically.
Applied to files:
composer.json
📚 Learning: 2025-09-28T15:12:48.345Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
Applied to files:
composer.json
📚 Learning: 2025-09-29T14:58:04.095Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Applied to files:
composer.json
📚 Learning: 2025-08-18T20:13:33.518Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Applied to files:
composer.json
📚 Learning: 2025-09-29T14:57:51.387Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: actions/php-setup/action.yml:12-15
Timestamp: 2025-09-29T14:57:51.387Z
Learning: The user prefers `update` as the default composer command in centralized workflows to ensure latest dependencies are always installed, rather than `install` which would be more reproducible.
Applied to files:
composer.json
📚 Learning: 2025-08-18T15:43:30.996Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: composer.json:13-16
Timestamp: 2025-08-18T15:43:30.996Z
Learning: Testing helper libraries (packages with "testing" or "support" in keywords/description that provide test utilities) should include PHPUnit and other testing frameworks in the main "require" section rather than "require-dev", as consuming packages need these dependencies to use the testing utilities.
Applied to files:
composer.json
📚 Learning: 2026-01-22T11:42:32.878Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 111
File: tests/Fallback/AssetFallbackTest.php:98-98
Timestamp: 2026-01-22T11:42:32.878Z
Learning: When using xepozz/internal-mocker in PHP tests, the library's namespaced wrapper records and matches function calls with all default parameters filled in. To ensure proper matching, always specify every parameter in MockerState::addCondition(), including defaults. For example, a call to file_get_contents($path) is observed as file_get_contents($path, false, null, 0, null). Use the same complete argument list (including default values) when adding conditions for mocks in tests like tests/Fallback/AssetFallbackTest.php.
Applied to files:
tests/Support/InternalMockerExtension.phptests/Fixtures/Asset/StubAssetManager.php
🧬 Code graph analysis (6)
src/Exception/ExceptionInterface.php (1)
src/Fallback/FallbackInterface.php (2)
restore(31-31)FallbackInterface(21-32)
src/Solver/Solver.php (2)
src/Exception/RuntimeException.php (1)
RuntimeException(7-7)src/Event/AbstractSolveEvent.php (2)
AbstractSolveEvent(24-55)__construct(33-36)
tests/Fixtures/Asset/StubAssetManager.php (2)
src/Asset/AbstractAssetManager.php (9)
__construct(35-44)addDependencies(61-74)isAvailable(91-94)isInstalled(96-99)run(111-173)setFallback(175-180)setUpdatable(182-187)validate(189-214)AbstractAssetManager(43-325)src/Asset/AssetManagerInterface.php (10)
addDependencies(19-19)getLockPackageName(24-24)getName(29-29)isAvailable(44-44)isInstalled(49-49)run(64-64)setFallback(71-71)setUpdatable(78-78)validate(86-86)AssetManagerInterface(25-101)
src/Config/Config.php (2)
src/Config/ConfigBuilder.php (2)
ConfigBuilder(25-99)getConfigBase(47-57)src/Foxy.php (1)
Foxy(47-213)
src/Exception/RuntimeException.php (1)
src/Fallback/AssetFallback.php (1)
AssetFallback(26-81)
src/Json/JsonFormatter.php (1)
src/Json/JsonFile.php (2)
JsonFile(23-106)encode(81-86)
🪛 GitHub Actions: build
composer.json
[error] 1-1: Composer update failed: Your requirements could not be resolved to an installable set of packages. PHP 8.1.34 does not satisfy the required PHP ^8.2 (infection/infection and related dependencies conflict with json-schema versions). Consider upgrading PHP to 8.2+ or adjusting package constraints (e.g., use --with-all-dependencies). The command that failed: 'composer update --prefer-dist --no-interaction --no-progress --optimize-autoloader --ansi'.
🪛 PHPMD (2.15.0)
src/Solver/Solver.php
36-36: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
121-121: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'getMockPackagePath'. (undefined)
(StaticAccess)
tests/Support/InternalMockerExtension.php
15-15: Avoid unused parameters such as '$configuration'. (undefined)
(UnusedFormalParameter)
15-15: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
21-21: Avoid using static access to class 'Foxy\Tests\Support\InternalMockerExtension' in method 'bootstrap'. (undefined)
(StaticAccess)
tests/Fixtures/Asset/StubAssetManager.php
18-18: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
18-18: Avoid unused parameters such as '$io'. (undefined)
(UnusedFormalParameter)
19-19: Avoid unused parameters such as '$config'. (undefined)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$executor'. (undefined)
(UnusedFormalParameter)
21-21: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
21-21: Avoid unused parameters such as '$fs'. (undefined)
(UnusedFormalParameter)
24-24: Avoid unused parameters such as '$rootPackage'. (undefined)
(UnusedFormalParameter)
24-24: Avoid unused parameters such as '$dependencies'. (undefined)
(UnusedFormalParameter)
74-74: Avoid unused parameters such as '$fallback'. (undefined)
(UnusedFormalParameter)
79-79: Avoid unused parameters such as '$updatable'. (undefined)
(UnusedFormalParameter)
src/Json/JsonFormatter.php
49-49: The method format has a boolean flag argument $formatJson, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
🔇 Additional comments (27)
tests/Support/InternalMockerExtension.php (2)
7-11: Grouped imports look clean and consistent.
No concerns.
18-25: Anonymous class syntax update is safe.
No concerns..styleci.yml (2)
20-23: LGTM!The newly enabled rules (
empty_loop_body_braces,integer_literal_case) align with modern PHP best practices and enforce consistent formatting.
83-87: LGTM!Disabling specific PSR-12 rules (
new_with_parentheses,psr12_braces,psr12_class_definition) while using thepsr12preset is a valid approach for customizing coding standards while retaining most PSR-12 conventions.composer.json (3)
23-33: LGTM on the tooling modernization.The updated dev dependencies introduce a solid modern PHP tooling stack:
- PHPStan with strict rules for static analysis
- Rector for automated refactoring
- Updated ECS for coding standards
This aligns with the PR objective of modernizing CI workflows.
40-43: LGTM!The
allow-pluginsconfiguration correctly permits the infection and phpstan extension installers to execute during Composer operations.
61-78: LGTM on script additions.The new scripts provide a comprehensive development workflow:
mutation/mutation-staticfor mutation testingrectorfor automated refactoringstaticfor PHPStan analysissync-metadatafor configuration synchronizationtestsfor running PHPUnitNote: The
sync-metadatascripts use themainbranch reference for the external template repository, which aligns with your established preference across php-forge repositories. Based on learnings, this is intentional.tests/Fixtures/Asset/StubAssetManager.php (3)
9-10: No review notes for this import change.
29-37: Stubbed accessors align with the updated interface.Constant returns for name/lock/package/availability/installation/run/validate are consistent for a fixture stub.
Also applies to: 49-57, 69-72, 84-84
24-27: No action required. Verification confirms thataddDependencies()is never invoked onStubAssetManagerin the test suite. The stub is exclusively used inFoxyTest.phpfor activation testing, which only calls methods likegetPackageName()andgetName(). The unconditionalRuntimeExceptionwith the message "Not used in this test" is an intentional, defensive pattern that clearly documents the method's status.src/Json/JsonFormatter.php (4)
7-23: Import consolidation looks good.No issues with the explicit function imports.
27-31: DEFAULT_INDENT constant looks good.
81-88: Indent detection update looks good.
143-152: Replacement loop looks good.src/Config/Config.php (8)
9-19: LGTM.
29-29: LGTM.
37-55: LGTM.
57-68: LGTM.
111-138: LGTM.
140-174: LGTM.
176-186: LGTM.
198-206: LGTM.src/Exception/ExceptionInterface.php (1)
7-7: LGTM.src/Exception/RuntimeException.php (1)
7-7: LGTM.src/Solver/Solver.php (3)
87-106: LGTM.
119-134: LGTM.
33-38: AllSolverinstantiations correctly pass theFilesystemargument. Three call sites verified: tests/Solver/SolverTest.php (lines 111, 164) and src/Foxy.php (line 96) all include the required parameter in the correct position.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Json/JsonFormatter.php (1)
107-130: Handle scalar JSON values informatInternal()to preventTypeError.
json_decode($json, true)returns scalars (int, string, bool, null) for valid scalar JSON like"text"or123, not just arrays. Passing scalars toarray_walk_recursive()throwsTypeErrorbecause it expects only arrays. The method must guard against scalar results or handle them separately.Suggested fix
$array = json_decode($json, true, 512, JSON_THROW_ON_ERROR); + $normalize = static function (string $value) use ($unescapeUnicode, $unescapeSlashes): string { + if ($unescapeUnicode) { + $value = preg_replace_callback( + '/\\\\u([0-9a-fA-F]{4})/', + static function (array $match): string { + $result = mb_convert_encoding(pack('H*', $match[1]), 'UTF-8', 'UCS-2BE'); + return $result !== false ? $result : ''; + }, + $value, + ) ?? $value; + } + if ($unescapeSlashes) { + $value = str_replace('\\/', '/', $value); + } + return $value; + }; + + if (is_array($array)) { + if ($unescapeUnicode) { + array_walk_recursive($array, static function (mixed &$item) use ($normalize): void { + if (is_string($item)) { + $item = $normalize($item); + } + }); + } + + if ($unescapeSlashes) { + array_walk_recursive($array, static function (mixed &$item) use ($normalize): void { + if (is_string($item)) { + $item = $normalize($item); + } + }); + } + } elseif (is_string($array)) { + $array = $normalize($array); + }
🤖 Fix all issues with AI agents
In `@src/Config/Config.php`:
- Around line 63-68: getArray currently ignores its $default when resolving
manager-specific values; change the call in getArray to pass $default into get
(i.e. call $this->get($key, $default)) so manager-specific defaults are
considered, then keep the existing ternary check (return is_array($value) ?
$value : $default) to ensure you still return an array fallback.
- Around line 75-78: convertBoolean currently checks the raw string against
['true','1','yes','y'] causing values like "TRUE" to be mis-converted; update
convertBoolean to normalize case first (e.g. use strtolower or mb_strtolower on
$value) before checking the list or reuse the same normalization logic as
isBoolean so that values like "TRUE", "Yes", "Y" are correctly converted to true
in convertBoolean.
In `@src/Json/JsonFormatter.php`:
- Around line 55-57: The current global str_replace(' ', str_repeat(' ',
$indent), $json) corrupts quoted strings; instead, only rewrite leading
indentation per line: replace occurrences of leading groups of four spaces using
a line-start regex (e.g. match '/^(?: {4})+/m') and in a preg_replace_callback
compute the number of 4-space groups and replace them with that count multiplied
by $indent (use $json and $indent variables and remove the global str_replace).
This preserves spaces inside quoted values while correctly adapting indentation.
In `@src/Solver/Solver.php`:
- Around line 119-126: In getMockPackagePath(PackageInterface $package, string
$assetDir, string $filename) detect and handle failures from mkdir() and copy():
check the boolean result of mkdir($packagePath, 0777, true) and of
copy($filename, $newFilename), and if either fails throw a descriptive
RuntimeException (include $package->getName(), $packagePath and $filename) so
execution stops instead of proceeding to JSON processing; additionally, if copy
fails after mkdir succeeded, remove any partially created file and, if
appropriate, remove the created directory to avoid partial state.
In `@tests/Fixtures/Asset/StubAssetManager.php`:
- Around line 24-27: The addDependencies method in StubAssetManager currently
throws RuntimeException; change it to return a minimal AssetPackageInterface
stub instead so tests invoking Solver won't fail: in
StubAssetManager::addDependencies(RootPackageInterface $rootPackage, array
$dependencies): AssetPackageInterface, construct and return a lightweight
AssetPackageInterface implementation (e.g., a simple stub/anonymous class or
reuse an existing test stub) that supplies safe default values for required
methods (name, version, etc.) rather than throwing.
♻️ Duplicate comments (3)
tests/Fixtures/Asset/StubAssetManager.php (1)
17-22: Suppress PHPMD unused-parameter/short-variable warnings in the fixturePHPMD flags these required-but-unused parameters; consider a fixture-level suppression to keep CI clean while preserving interface signatures.
🔧 Suggested suppression (fixture-level)
-final class StubAssetManager implements AssetManagerInterface +/** + * `@SuppressWarnings`(PHPMD.UnusedFormalParameter) + * `@SuppressWarnings`(PHPMD.ShortVariable) + */ +final class StubAssetManager implements AssetManagerInterfaceAlso applies to: 24-26, 74-81
src/Config/Config.php (1)
193-195: Integer detection accepts malformed values like"123-"or"--123".
If strict validation is intended, a regex orFILTER_VALIDATE_INTwould be safer.src/Solver/Solver.php (1)
12-19: Use the projectFoxy\Exception\RuntimeExceptionfor consistency.
The current import uses the global class and can break exception handling consistency across the library.🔧 Proposed fix
-use RuntimeException; +use Foxy\Exception\RuntimeException;
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.styleci.ymlcomposer.jsonsrc/Config/Config.phpsrc/Exception/ExceptionInterface.phpsrc/Exception/RuntimeException.phpsrc/Json/JsonFormatter.phpsrc/Solver/Solver.phptests/Fixtures/Asset/StubAssetManager.phptests/Support/InternalMockerExtension.php
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
📚 Learning: 2025-08-18T16:13:31.606Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: phpstan.neon:1-3
Timestamp: 2025-08-18T16:13:31.606Z
Learning: In the php-forge/support project, PHPStan is intended to be used as a PHAR installation rather than as a Composer dependency, even though the composer.json was updated to include phpstan/phpstan as a dependency.
Applied to files:
composer.json
📚 Learning: 2025-09-29T14:58:04.095Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Applied to files:
composer.json
📚 Learning: 2026-01-23T11:09:08.789Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: composer.json:68-77
Timestamp: 2026-01-23T11:09:08.789Z
Learning: When a composer.json in php-forge repositories references external template repos in sync-metadata scripts, prefer using the main branch reference rather than pinning to specific commit SHAs or tags. This reduces maintenance when upstream templates change, but be aware of potential drift; document rationale and review changes periodically.
Applied to files:
composer.json
📚 Learning: 2025-09-28T15:12:48.345Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
Applied to files:
composer.json
📚 Learning: 2025-08-18T20:13:33.518Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Applied to files:
composer.json
📚 Learning: 2025-09-29T14:57:51.387Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: actions/php-setup/action.yml:12-15
Timestamp: 2025-09-29T14:57:51.387Z
Learning: The user prefers `update` as the default composer command in centralized workflows to ensure latest dependencies are always installed, rather than `install` which would be more reproducible.
Applied to files:
composer.json
📚 Learning: 2025-08-18T15:43:30.996Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: composer.json:13-16
Timestamp: 2025-08-18T15:43:30.996Z
Learning: Testing helper libraries (packages with "testing" or "support" in keywords/description that provide test utilities) should include PHPUnit and other testing frameworks in the main "require" section rather than "require-dev", as consuming packages need these dependencies to use the testing utilities.
Applied to files:
composer.json
📚 Learning: 2026-01-22T11:42:32.878Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 111
File: tests/Fallback/AssetFallbackTest.php:98-98
Timestamp: 2026-01-22T11:42:32.878Z
Learning: When using xepozz/internal-mocker in PHP tests, the library's namespaced wrapper records and matches function calls with all default parameters filled in. To ensure proper matching, always specify every parameter in MockerState::addCondition(), including defaults. For example, a call to file_get_contents($path) is observed as file_get_contents($path, false, null, 0, null). Use the same complete argument list (including default values) when adding conditions for mocks in tests like tests/Fallback/AssetFallbackTest.php.
Applied to files:
tests/Support/InternalMockerExtension.phptests/Fixtures/Asset/StubAssetManager.php
🧬 Code graph analysis (2)
src/Solver/Solver.php (4)
src/Event/GetAssetsEvent.php (2)
GetAssetsEvent(10-60)getAssets(46-49)src/FoxyEvents.php (1)
FoxyEvents(9-33)src/Util/AssetUtil.php (2)
AssetUtil(26-225)getName(58-61)src/Exception/RuntimeException.php (1)
RuntimeException(7-7)
tests/Fixtures/Asset/StubAssetManager.php (1)
src/Asset/AssetManagerInterface.php (9)
addDependencies(19-19)getLockPackageName(24-24)getName(29-29)isAvailable(44-44)isInstalled(49-49)run(64-64)setFallback(71-71)setUpdatable(78-78)validate(86-86)
🪛 GitHub Actions: build
composer.json
[error] 1-1: Command failed: composer update --prefer-dist --no-interaction --no-progress --optimize-autoloader --ansi. Dependency resolution failed: Root composer.json requires composer/composer ^2.8 and infection/infection ^0.27|^0.32, but PHP 8.1.34 is incompatible with infection/infection 0.32.x which requires PHP 8.2+. Also conflicting versions of justinrainbow/json-schema (5.x vs 6.x). Consider upgrading PHP to 8.2+ or aligning package versions, or run with --with-all-dependencies to allow upgrades.
🪛 PHPMD (2.15.0)
tests/Support/InternalMockerExtension.php
15-15: Avoid unused parameters such as '$configuration'. (undefined)
(UnusedFormalParameter)
15-15: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
21-21: Avoid using static access to class 'Foxy\Tests\Support\InternalMockerExtension' in method 'bootstrap'. (undefined)
(StaticAccess)
src/Solver/Solver.php
36-36: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
121-121: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'getMockPackagePath'. (undefined)
(StaticAccess)
tests/Fixtures/Asset/StubAssetManager.php
18-18: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
18-18: Avoid unused parameters such as '$io'. (undefined)
(UnusedFormalParameter)
19-19: Avoid unused parameters such as '$config'. (undefined)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$executor'. (undefined)
(UnusedFormalParameter)
21-21: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
21-21: Avoid unused parameters such as '$fs'. (undefined)
(UnusedFormalParameter)
24-24: Avoid unused parameters such as '$rootPackage'. (undefined)
(UnusedFormalParameter)
24-24: Avoid unused parameters such as '$dependencies'. (undefined)
(UnusedFormalParameter)
74-74: Avoid unused parameters such as '$fallback'. (undefined)
(UnusedFormalParameter)
79-79: Avoid unused parameters such as '$updatable'. (undefined)
(UnusedFormalParameter)
src/Json/JsonFormatter.php
49-49: The method format has a boolean flag argument $formatJson, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
🔇 Additional comments (20)
src/Exception/ExceptionInterface.php (1)
7-7: LGTM — concise interface declaration.
No behavioral impact from the docblock removal and inline declaration.src/Exception/RuntimeException.php (1)
7-7: LGTM — no behavior change.
Inline empty body keeps API intact and is valid PHP.tests/Support/InternalMockerExtension.php (3)
7-11: LGTM! Clean grouped import syntax.The grouped use statements consolidate related imports nicely, improving readability while maintaining the same functionality.
18-29: LGTM! Concise anonymous class syntax.Removing the empty parentheses from anonymous class instantiations is valid PHP syntax when no constructor arguments are needed, resulting in cleaner code.
15-31: Static analysis hints are false positives.The PHPMD warnings can be safely ignored:
- Unused parameters:
$configurationand$parametersare required by theExtensioninterface contract and cannot be removed.- Static access: Using
InternalMockerExtension::load()is intentional—the anonymous class cannot access$thisof the enclosing class, making static access the correct pattern here.src/Config/Config.php (7)
29-29: Constructor promotion to readonly looks good.
39-54: Env cache + manager resolution flow looks solid.
96-138: Env value conversion pipeline looks consistent.
147-159: Manager-specific selection logic looks good.
167-174: Default value resolution is clear and consistent.
181-186: Boolean detection logic is fine.
203-206: JSON detection heuristic is OK for env overrides.src/Solver/Solver.php (3)
33-38: Constructor promotion to private readonly dependencies looks good.
47-74: Solve flow is clear and consistent with the event lifecycle.
76-106: Asset discovery and event augmentation look solid..styleci.yml (2)
20-23: LGTM: stricter formatting rules are clear and consistent.
85-87: LGTM: PSR-12 customizations are consistent with a tailored style profile.composer.json (3)
39-43: LGTM: allow-plugins is correctly scoped to required installers.
62-78: LGTM: vendor/bin script wiring is consistent and reduces toolchain drift.
24-32: Verify PHP 8.1 support requirement before constraining infection to 0.27 only.The
infection/infection^0.32 constraint is incompatible with PHP 8.1 (0.32.x requires PHP ^8.2), but this only matters if PHP 8.1 must be supported. The current CI workflows default to PHP 8.5, where infection 0.32 works fine. No json-schema conflict exists (6.6.4 is compatible with PHP 8.1).If PHP 8.1 support is intentional, constrain infection to ^0.27 or bump the PHP requirement to ^8.2. Otherwise, the current configuration is compatible with CI as tested.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@ecs.php`:
- Around line 26-48: OrderedClassElementsFixer configuration is missing the
method_public_abstract entry so public abstract methods are not ordered
consistently; update the 'order' array passed to
OrderedClassElementsFixer::class to insert 'method_public_abstract' immediately
before 'method_protected_abstract' (keeping 'method_protected_abstract',
'method_public', 'method_protected', 'method_private' following) so public
abstract methods are handled correctly.
In `@tests/Fallback/ComposerFallbackTest.php`:
- Around line 149-156: The composer.lock creation in the test uses
json_encode(...) without JSON_THROW_ON_ERROR, causing inconsistent error
handling compared to testRestore; update the call that writes composer.lock
(inside the test that currently does file_put_contents($this->cwd .
'/composer.lock', json_encode(['content-hash' => 'HASH_VALUE']))) to use
json_encode(['content-hash' => 'HASH_VALUE'], JSON_THROW_ON_ERROR) so encoding
failures throw exceptions consistently (no other behavior change needed).
- Around line 169-174: The PHPDoc incorrectly annotates `$this` instead of the
property; update the test to remove or correct the docblock for the mock
creation: delete the `/** `@var` Installer|MockObject $this */` annotation before
the mock setup or change it to `/** `@var` Installer|MockObject $this->installer
*/` — preferably remove it entirely because the property `$this->installer` is
already typed as `Installer|MockObject|null` and the explicit doc is redundant;
keep the mock creation using
`getMockBuilder(Installer::class)->disableOriginalConstructor()->onlyMethods(['run'])->getMock()`
intact.
- Line 40: Add an import for Symfony\Component\Filesystem\Filesystem at the top
of the ComposerFallbackTest file and update the typed property and any other
occurrences using the fully-qualified name to the imported class; specifically
add "use Symfony\Component\Filesystem\Filesystem;" and change the property
declaration "private \Symfony\Component\Filesystem\Filesystem|null $sfs = null;"
(and the other occurrence referencing the FQN, e.g. in methods around the $sfs
usage) to use the imported Filesystem type (or, if that name conflicts with
Composer's Filesystem, import without alias and rename the property $sfs to
avoid collision).
♻️ Duplicate comments (5)
tests/Fallback/AssetFallbackTest.php (1)
158-173: LGTM!Well-structured test setup with proper isolation:
- Unique temp directory per test prevents interference
- CWD management ensures relative paths resolve correctly
- Mock injection allows controlled testing of Filesystem behavior
src/Config/Config.php (3)
63-68: Pass$defaultintoget()to preserve manager-specific defaults.The
getArray()method ignores its$defaultparameter during manager resolution, which can causemanager-*keys to return incorrect values instead of the manager-specific default.💡 Proposed fix
- $value = $this->get($key); + $value = $this->get($key, $default);
75-78: Normalize case before boolean conversion.
isBoolean()lowercases input before checking, butconvertBoolean()doesn't. This causes"TRUE","Yes","Y"to be detected as boolean but incorrectly converted tofalse.🐛 Proposed fix
private function convertBoolean(string $value): bool { + $value = strtolower($value); + return in_array($value, ['true', '1', 'yes', 'y'], true); }
193-196: Consider stricter integer validation.The current implementation
ctype_digit(trim($value, '-'))accepts malformed inputs like"--123"or"123-"as valid integers. If strict validation is needed, consider using regex.♻️ Alternative implementation
private function isInteger(string $value): bool { - return ctype_digit(trim($value, '-')); + return (bool) preg_match('/^-?\d+$/', $value); }src/Solver/Solver.php (1)
119-134: Handlemkdir/copyfailures explicitly to avoid partial state.Both
mkdir()andcopy()can fail due to permissions, race conditions, or missing source files. Currently, failures silently fall through to JSON processing, potentially causing cryptic errors or partial state.🐛 Proposed fix
- mkdir($packagePath, 0777, true); - copy($filename, $newFilename); + if (!is_dir($packagePath) && !mkdir($packagePath, 0777, true) && !is_dir($packagePath)) { + throw new RuntimeException("Unable to create asset directory: {$packagePath}"); + } + if (!copy($filename, $newFilename)) { + throw new RuntimeException("Unable to copy asset file from {$filename} to {$newFilename}"); + }Note: You'll need to add
is_dirto the function imports:use function is_dir;
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
ecs.phpsrc/Config/Config.phpsrc/FoxyEvents.phpsrc/Solver/Solver.phptests/Fallback/AssetFallbackTest.phptests/Fallback/ComposerFallbackTest.phptests/Util/PackageUtilTest.php
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: User terabytesoftw prefers not to use aliases when importing classes in PHP. They are comfortable with either fully-qualified class names or direct imports without aliases.
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
📚 Learning: 2025-08-18T15:43:30.996Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: composer.json:13-16
Timestamp: 2025-08-18T15:43:30.996Z
Learning: Testing helper libraries (packages with "testing" or "support" in keywords/description that provide test utilities) should include PHPUnit and other testing frameworks in the main "require" section rather than "require-dev", as consuming packages need these dependencies to use the testing utilities.
Applied to files:
tests/Util/PackageUtilTest.php
📚 Learning: 2026-01-22T11:42:32.878Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 111
File: tests/Fallback/AssetFallbackTest.php:98-98
Timestamp: 2026-01-22T11:42:32.878Z
Learning: When using xepozz/internal-mocker in PHP tests, the library's namespaced wrapper records and matches function calls with all default parameters filled in. To ensure proper matching, always specify every parameter in MockerState::addCondition(), including defaults. For example, a call to file_get_contents($path) is observed as file_get_contents($path, false, null, 0, null). Use the same complete argument list (including default values) when adding conditions for mocks in tests like tests/Fallback/AssetFallbackTest.php.
Applied to files:
tests/Util/PackageUtilTest.phptests/Fallback/AssetFallbackTest.phptests/Fallback/ComposerFallbackTest.php
📚 Learning: 2026-01-23T11:22:00.118Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: In PHP tests (e.g., tests/Fallback/AssetFallbackTest.php) avoid using import aliases. Prefer either importing the full namespace without an alias or using fully-qualified class names directly. For example, avoid statements like 'use Some\Long\Namespace as Alias;' and either 'use Some\\Long\\Namespace;' or reference 'Some\\Long\\Namespace' in code. This guideline applies to PHP files under tests and improves readability and refactor safety.
Applied to files:
tests/Util/PackageUtilTest.phptests/Fallback/AssetFallbackTest.phptests/Fallback/ComposerFallbackTest.php
📚 Learning: 2025-08-18T20:13:33.518Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Applied to files:
tests/Fallback/AssetFallbackTest.php
📚 Learning: 2025-09-28T15:12:48.345Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
Applied to files:
tests/Fallback/AssetFallbackTest.php
📚 Learning: 2026-01-23T11:22:00.118Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: User terabytesoftw prefers not to use aliases when importing classes in PHP. They are comfortable with either fully-qualified class names or direct imports without aliases.
Applied to files:
ecs.php
🧬 Code graph analysis (4)
tests/Util/PackageUtilTest.php (1)
src/Util/PackageUtil.php (3)
PackageUtil(10-93)loadLockPackages(86-92)PackageUtil(24-107)
src/Solver/Solver.php (1)
src/Exception/RuntimeException.php (1)
RuntimeException(7-7)
tests/Fallback/AssetFallbackTest.php (3)
src/Fallback/AssetFallback.php (4)
AssetFallback(13-69)save(55-68)restore(28-53)AssetFallback(26-81)src/Fallback/FallbackInterface.php (2)
save(17-17)restore(12-12)src/Exception/RuntimeException.php (1)
RuntimeException(7-7)
tests/Fallback/ComposerFallbackTest.php (3)
src/Fallback/ComposerFallback.php (3)
ComposerFallback(20-162)save(66-81)restore(47-64)src/Util/LockerUtil.php (2)
LockerUtil(12-26)getLocker(17-25)src/Fallback/FallbackInterface.php (2)
save(17-17)restore(12-12)
🪛 PHPMD (2.15.0)
tests/Util/PackageUtilTest.php
74-74: Avoid using static access to class '\Foxy\Util\PackageUtil' in method 'testLoadLockPackages'. (undefined)
(StaticAccess)
84-84: Avoid using static access to class '\Foxy\Util\PackageUtil' in method 'testLoadLockPackagesWithoutPackages'. (undefined)
(StaticAccess)
src/Solver/Solver.php
36-36: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
121-121: Avoid using static access to class '\Foxy\Util\AssetUtil' in method 'getMockPackagePath'. (undefined)
(StaticAccess)
tests/Fallback/AssetFallbackTest.php
26-26: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
27-27: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
63-65: The method testRestore uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
111-111: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testRestoreThrowsWhenWriteFails'. (undefined)
(StaticAccess)
150-150: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testSaveThrowsWhenFileCannotBeRead'. (undefined)
(StaticAccess)
167-167: Missing class import via use statement (line '167', column '26'). (undefined)
(MissingImport)
tests/Fallback/ComposerFallbackTest.php
35-35: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
38-38: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
83-83: Avoid variables with short names like $ed. Configured minimum length is 3. (undefined)
(ShortVariable)
87-87: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
91-91: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
96-96: Avoid using static access to class '\Foxy\Util\LockerUtil' in method 'testRestore'. (undefined)
(StaticAccess)
114-117: The method testRestore uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
141-141: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
145-145: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
175-175: Missing class import via use statement (line '175', column '26'). (undefined)
(MissingImport)
🔇 Additional comments (33)
src/FoxyEvents.php (1)
25-30: LGTM — constant reorder is safe.No functional change introduced; the event value and annotation remain intact.
tests/Util/PackageUtilTest.php (4)
9-11: Clean TestCase usage.Import and inheritance are clear and consistent with PHPUnit conventions.
15-52: Alias conversion test looks solid.The expected alias map and key assertion align with
convertLockAliasoutput shape.
54-80: Good coverage for loadLockPackages.Validates both packages and packages-dev loading paths with concrete fixtures.
82-84: Nice edge-case check.Empty lock data returning an empty array is a sensible guard.
ecs.php (4)
5-11: LGTM!The import statements are well-organized using PHP's grouped import syntax. This aligns with your preference for direct imports without aliases.
13-25: LGTM! Previous issue resolved.The NullableTypeDeclarationFixer is now properly configured without being contradictorily skipped. The
'syntax' => 'union'setting will enforce PHP 8+ union type nullable syntax (string|nullinstead of?string).
49-72: LGTM!Clean configuration for import ordering, PHPDoc type ordering, and PHPUnit static method call style. The settings follow sensible defaults:
- Imports ordered by type (class, function, const) then alphabetically
- Null types positioned last in PHPDoc
- PHPUnit static methods called via
self::
73-94: LGTM!The path configuration correctly scopes ECS to
srcandtestsdirectories with.phpextension only. The prepared sets (cleanCode,comments,docblocks,namespaces,strict) and PHP-CS-Fixer PER-CS 3.0 set provide a comprehensive baseline. The explicit rules for unused imports, trait ordering, and single quotes complement the sets appropriately.tests/Fallback/ComposerFallbackTest.php (5)
1-28: LGTM!The imports are well-organized, follow PHPUnit 9 conventions, and align with the preference of avoiding aliases.
42-50: LGTM!The data providers are correctly structured and provide good test coverage for both empty and non-empty package scenarios, as well as lock file presence variations.
57-121: LGTM!The test method comprehensively validates the restore functionality with proper mock setup and branching assertions based on the packages data. The use of
JSON_THROW_ON_ERRORon line 74 ensures proper error handling during JSON encoding.
126-134: LGTM!Good addition to verify the early-return behavior when
fallback-composeris disabled. The test correctly asserts that no write operations occur.
190-207: LGTM!The teardown properly restores the working directory, cleans up the temporary test directory, and nullifies all properties to prevent state leakage between tests.
tests/Fallback/AssetFallbackTest.php (9)
21-29: LGTM!Class structure and property declarations are appropriate for a PHPUnit test fixture. The union types with
nullcorrectly support the setup/teardown lifecycle pattern.
31-39: LGTM!Data providers are correctly declared as static methods (required by PHPUnit 10.x) and provide appropriate test cases for both file-exists and no-file scenarios.
41-66: LGTM!Test logic correctly validates both branches:
- With package file: verifies content is restored after the save/restore cycle
- Without package file: verifies no file is created when none existed originally
The else clause appropriately separates the distinct assertions for each branch.
68-96: LGTM!Excellent exception chaining verification. The try-catch pattern correctly tests both:
- The outer
RuntimeExceptionwith the expected message- The
$previousexception preserving the original error contextThe test properly validates the implementation's error wrapping behavior.
98-117: LGTM!Test correctly uses
MockerState::addConditionwith all default parameters specified forfile_put_contents, which is required by the xepozz/internal-mocker library. The test properly validates the write failure exception path. Based on learnings, the complete argument list['package.json', $content, 0, null]correctly matches how the library observes function calls.
119-129: LGTM!Test correctly verifies the early-return behavior when
fallback-assetconfiguration is disabled, ensuring no file operations or I/O writes occur.
131-141: LGTM!Test correctly verifies the fluent interface pattern (
save()returnsself) for both file-exists and no-file scenarios.
143-156: LGTM!Test correctly uses
MockerState::addConditionwith all default parameters forfile_get_contents(['package.json', false, null, 0, null]), properly testing the read failure exception path during save operations. Based on learnings, this complete argument specification ensures proper mock matching.
175-189: LGTM!Proper teardown sequence:
- Restores CWD before removing temp directory (critical order)
- Cleans up temp directory using Symfony Filesystem
- Nulls all properties to prevent test pollution
src/Config/Config.php (6)
9-19: LGTM!Clean explicit function imports following modern PHP best practices.
29-29: LGTM!Constructor uses PHP 8 promoted properties with
private readonly, providing immutability and cleaner syntax.
140-159: LGTM!The manager-based value resolution logic correctly handles:
- Keys prefixed with
manager-when values are arrays- Falls back to default when manager key doesn't exist in the value array
- Returns non-array values unchanged
167-174: LGTM!Default value resolution correctly integrates with manager-based defaults. The formatting issue (double space) has been addressed.
181-186: LGTM!
isBoolean()correctly normalizes case before checking against the list of boolean-like values.
198-206: LGTM!Simple heuristic for detecting JSON strings is appropriate for environment variable parsing.
src/Solver/Solver.php (4)
12-24: LGTM!Imports are now consistent:
Foxy\Exception\RuntimeExceptioncorrectly replaces the built-inRuntimeException- Function imports (
basename,copy,mkdir,rtrim) are explicit and clean
33-38: LGTM!Constructor uses promoted properties with
private readonlyfor immutability. The$fsvariable name, while short, is a widely recognized abbreviation forFilesystemand is clear in context.
47-73: LGTM!The
solvemethod now correctly usesFoxy\Exception\RuntimeExceptionwith proper@throwsannotation. The exception handling aligns with the codebase's custom exception hierarchy.
87-106: LGTM!Visibility change from
protectedtoprivateis appropriate since this is an internal helper method not intended for subclass extension.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…on-schema requirement.
…ces in test files.
… ComposerFallbackTest.php.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.github/workflows/mutation.yml:
- Around line 19-26: The mutation job currently requires
STRYKER_DASHBOARD_API_KEY which breaks on forked PRs because secrets aren’t
passed; update the job (mutation) to guard execution for non-forked runs by
adding an if condition such as ensuring either the event isn’t a pull_request or
that github.event.pull_request.head.repo.fork == false so the reusable workflow
isn’t invoked for forked PRs; keep the existing secrets mapping
(STRYKER_DASHBOARD_API_KEY) as-is for non-fork runs.
In `@composer.json`:
- Around line 52-55: Remove "infection/extension-installer" from the
"allow-plugins" block in composer.json if the package "infection/infection" is
not a direct dependency (it only appears in "suggest"); locate the
"allow-plugins" section and delete the "infection/extension-installer": true
entry, or alternatively keep it only if you intentionally install infection
later—if you retain it, add a brief comment explaining why it must remain.
- Line 41: The composer.json entry for the package
"symplify/easy-coding-standard" is using a loose ^13.0 constraint that may
resolve to releases requiring PHP >=8.3; update the dependency to a
PHP-compatible option by either pinning "symplify/easy-coding-standard" to a
known compatible version (e.g., 13.0.0 or 13.0.4) in composer.json or,
alternatively, raise the project's PHP platform requirement from ^8.1 to ^8.3 so
the ^13.0 constraint is safe; pick one approach and ensure composer.lock is
regenerated (run composer update) after the change.
In `@phpstan-baseline.neon`:
- Around line 105-175: The baseline contains suppressions for varTag.nativeType
and constructor.unusedParameter that should be fixed in tests rather than
ignored: update PHPDoc mock annotations in tests/FoxyTest.php,
tests/Solver/SolverTest.php, tests/Util/AssetUtilTest.php, and
tests/Util/ConsoleUtilTest.php to use intersection types like
Interface&PHPUnit\Framework\MockObject\MockObject (e.g.,
Composer\Package\PackageInterface&MockObject) so PHPStan recognizes native mock
types, and adjust the StubAssetManager constructor in
tests/Fixtures/Asset/StubAssetManager.php to match the intended signature
(remove or use $config, $executor, $fs, $io parameters) so
constructor.unusedParameter violations disappear; after these edits remove the
corresponding entries (if.condNotBoolean and varTag.nativeType /
constructor.unusedParameter) from the phpstan-baseline.neon.
- Around line 4-104: The phpstan baseline is suppressing 17 real issues in
production code across multiple classes; create a remediation plan that either
fixes them or documents a timeboxed cleanup with a tracked issue ID: 1) Audit
and fix code in AbstractAssetManager (replace empty() uses with strict
comparisons), Config, AssetFallback, ComposerFallback, Foxy, JsonFormatter,
Solver, and ConsoleUtil to ensure boolean expressions are properly typed
(if/&&/||/! use booleans), replace short ternary instances with null coalesce or
long ternary, and eliminate the static access to the private
SemverUtil::cleanWildcard by making the method public/static as appropriate or
calling it via an allowed instance; 2) Remove the corresponding suppressions
from phpstan-baseline.neon once fixed; 3) If immediate fixes aren’t feasible,
create a tracked issue (e.g., ISSUE-1234) with a clear timebox and per-file
owner, list each symbol to change (AbstractAssetManager, Config,
SemverUtil::cleanWildcard, AssetFallback, ComposerFallback, Foxy, JsonFormatter,
Solver, ConsoleUtil), and commit the baseline update referencing that issue and
the deadline.
♻️ Duplicate comments (3)
tests/Fixtures/Asset/StubAssetManager.php (1)
24-27: Return a minimal stub instead of throwing inaddDependencies()This concern was already raised: production code in
Solver.phpcallsaddDependencies()on the asset manager. Throwing an exception here will break tests if any test path exercises Solver with this fixture. For consistency with other stub methods, consider returning a minimalAssetPackageInterfacestub.tests/Fallback/ComposerFallbackTest.php (1)
149-156: Inconsistentjson_encodeerror handling.Line 152 uses
json_encode()withoutJSON_THROW_ON_ERROR, whiletestRestore(line 74) includes it. For consistency, add the flag here as well.♻️ Suggested fix
if ($withLockFile) { - file_put_contents($this->cwd . '/composer.lock', json_encode(['content-hash' => 'HASH_VALUE'])); + file_put_contents($this->cwd . '/composer.lock', json_encode(['content-hash' => 'HASH_VALUE'], JSON_THROW_ON_ERROR)); }composer.json (1)
80-89: External configuration sync acknowledged.Based on learnings, you prefer using the
mainbranch reference for sync-metadata scripts rather than pinning to specific commits. This approach is consistent across your repositories.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
.github/linters/actionlint.yml.github/workflows/mutation.ymlcomposer.jsonphpstan-baseline.neonresources/doc/config.mdresources/doc/events.mdresources/doc/faqs.mdresources/doc/index.mdresources/doc/usage.mdtests/Fallback/ComposerFallbackTest.phptests/Fixtures/Asset/StubAssetManager.phptests/Fixtures/Util/ThrowingProcessExecutorMock.phptests/Fixtures/package/global/theme/foo/bar/package.json
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: User terabytesoftw prefers not to use aliases when importing classes in PHP. They are comfortable with either fully-qualified class names or direct imports without aliases.
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
📚 Learning: 2026-01-22T11:42:32.878Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 111
File: tests/Fallback/AssetFallbackTest.php:98-98
Timestamp: 2026-01-22T11:42:32.878Z
Learning: When using xepozz/internal-mocker in PHP tests, the library's namespaced wrapper records and matches function calls with all default parameters filled in. To ensure proper matching, always specify every parameter in MockerState::addCondition(), including defaults. For example, a call to file_get_contents($path) is observed as file_get_contents($path, false, null, 0, null). Use the same complete argument list (including default values) when adding conditions for mocks in tests like tests/Fallback/AssetFallbackTest.php.
Applied to files:
tests/Fixtures/Util/ThrowingProcessExecutorMock.phptests/Fixtures/Asset/StubAssetManager.phptests/Fallback/ComposerFallbackTest.php
📚 Learning: 2026-01-23T11:22:00.118Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: In PHP tests (e.g., tests/Fallback/AssetFallbackTest.php) avoid using import aliases. Prefer either importing the full namespace without an alias or using fully-qualified class names directly. For example, avoid statements like 'use Some\Long\Namespace as Alias;' and either 'use Some\\Long\\Namespace;' or reference 'Some\\Long\\Namespace' in code. This guideline applies to PHP files under tests and improves readability and refactor safety.
Applied to files:
tests/Fixtures/Util/ThrowingProcessExecutorMock.phptests/Fixtures/Asset/StubAssetManager.phptests/Fallback/ComposerFallbackTest.php
📚 Learning: 2025-08-18T16:13:31.606Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: phpstan.neon:1-3
Timestamp: 2025-08-18T16:13:31.606Z
Learning: In the php-forge/support project, PHPStan is intended to be used as a PHAR installation rather than as a Composer dependency, even though the composer.json was updated to include phpstan/phpstan as a dependency.
Applied to files:
resources/doc/usage.mdcomposer.json
📚 Learning: 2025-08-18T15:43:30.996Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: composer.json:13-16
Timestamp: 2025-08-18T15:43:30.996Z
Learning: Testing helper libraries (packages with "testing" or "support" in keywords/description that provide test utilities) should include PHPUnit and other testing frameworks in the main "require" section rather than "require-dev", as consuming packages need these dependencies to use the testing utilities.
Applied to files:
resources/doc/usage.mdcomposer.json
📚 Learning: 2025-09-29T14:58:04.095Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Applied to files:
composer.json
📚 Learning: 2026-01-23T11:09:08.789Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: composer.json:68-77
Timestamp: 2026-01-23T11:09:08.789Z
Learning: When a composer.json in php-forge repositories references external template repos in sync-metadata scripts, prefer using the main branch reference rather than pinning to specific commit SHAs or tags. This reduces maintenance when upstream templates change, but be aware of potential drift; document rationale and review changes periodically.
Applied to files:
composer.json
📚 Learning: 2025-09-28T15:12:48.345Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
Applied to files:
composer.json
📚 Learning: 2025-09-29T14:57:51.387Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: actions/php-setup/action.yml:12-15
Timestamp: 2025-09-29T14:57:51.387Z
Learning: The user prefers `update` as the default composer command in centralized workflows to ensure latest dependencies are always installed, rather than `install` which would be more reproducible.
Applied to files:
composer.json
📚 Learning: 2025-08-18T20:13:33.518Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Applied to files:
composer.json
🧬 Code graph analysis (3)
tests/Fixtures/Util/ThrowingProcessExecutorMock.php (2)
src/Exception/RuntimeException.php (1)
RuntimeException(7-7)tests/Fixtures/Util/ProcessExecutorMock.php (1)
execute(9-12)
tests/Fixtures/Asset/StubAssetManager.php (2)
src/Exception/RuntimeException.php (1)
RuntimeException(7-7)src/Asset/AssetManagerInterface.php (9)
addDependencies(19-19)getLockPackageName(24-24)getName(29-29)isAvailable(44-44)isInstalled(49-49)run(64-64)setFallback(71-71)setUpdatable(78-78)validate(86-86)
tests/Fallback/ComposerFallbackTest.php (3)
src/Fallback/ComposerFallback.php (3)
ComposerFallback(20-162)save(66-81)restore(47-64)src/Util/LockerUtil.php (2)
LockerUtil(12-26)getLocker(17-25)src/Fallback/FallbackInterface.php (2)
save(17-17)restore(12-12)
🪛 PHPMD (2.15.0)
tests/Fixtures/Util/ThrowingProcessExecutorMock.php
12-12: Avoid unused parameters such as '$command'. (undefined)
(UnusedFormalParameter)
12-12: Avoid unused parameters such as '$output'. (undefined)
(UnusedFormalParameter)
12-12: Avoid unused parameters such as '$cwd'. (undefined)
(UnusedFormalParameter)
tests/Fixtures/Asset/StubAssetManager.php
18-18: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
18-18: Avoid unused parameters such as '$io'. (undefined)
(UnusedFormalParameter)
19-19: Avoid unused parameters such as '$config'. (undefined)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$executor'. (undefined)
(UnusedFormalParameter)
21-21: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
21-21: Avoid unused parameters such as '$fs'. (undefined)
(UnusedFormalParameter)
24-24: Avoid unused parameters such as '$rootPackage'. (undefined)
(UnusedFormalParameter)
24-24: Avoid unused parameters such as '$dependencies'. (undefined)
(UnusedFormalParameter)
74-74: Avoid unused parameters such as '$fallback'. (undefined)
(UnusedFormalParameter)
79-79: Avoid unused parameters such as '$updatable'. (undefined)
(UnusedFormalParameter)
tests/Fallback/ComposerFallbackTest.php
35-35: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
38-38: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
83-83: Avoid variables with short names like $ed. Configured minimum length is 3. (undefined)
(ShortVariable)
87-87: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
91-91: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
96-96: Avoid using static access to class '\Foxy\Util\LockerUtil' in method 'testRestore'. (undefined)
(StaticAccess)
114-117: The method testRestore uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
141-141: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
145-145: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
174-174: Missing class import via use statement (line '174', column '26'). (undefined)
(MissingImport)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: phpunit / PHP 8.5-windows-2022
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: phpstan / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
🔇 Additional comments (22)
tests/Fixtures/Util/ThrowingProcessExecutorMock.php (1)
8-14: LGTM! Good alignment with the project's exception hierarchy.The change to use
Foxy\Exception\RuntimeExceptioninstead of PHP's built-in exception maintains consistency with the codebase's exception handling patterns. This addresses the previous review feedback appropriately.Regarding the PHPMD warnings about unused parameters: these are false positives since this mock intentionally overrides
ProcessExecutor::execute()to always throw—the parameters must be declared to match the parent signature but are necessarily unused.tests/Fixtures/Asset/StubAssetManager.php (5)
9-14: LGTM!The grouped imports follow PSR-12 conventions and the
RuntimeExceptionimport is properly scoped to the Foxy exception namespace. Based on learnings, no aliases are used, which aligns with user preferences.
17-22: LGTM!The constructor signature matches the expected factory contract. Unused parameters and short variable names like
$ioand$fsare acceptable for a stub class and are conventional in the Composer ecosystem. The PHPMD warnings are false positives in this context.
29-37: LGTM!Both
getLockPackageName()andgetName()return sensible stub values that satisfy theAssetManagerInterfacecontract.
49-57: LGTM!The boolean stub values are reasonable defaults: available for use but nothing installed.
69-84: LGTM!The lifecycle methods return appropriate stub values:
run()returns success (0), fluent setters return$this, andvalidate()is a no-op. These implementations correctly satisfy theAssetManagerInterfacecontract for testing purposes.tests/Fallback/ComposerFallbackTest.php (6)
29-41: LGTM on class structure and properties.The test class structure follows PHPUnit conventions with properly typed nullable properties. The short variable names like
$fsand$ioare consistent with Composer's own naming conventions.
42-50: LGTM on data providers.Both
getRestoreData()andgetSaveData()are correctly structured aspublic staticmethods returning properly formatted arrays for PHPUnit parameterized tests.
57-121: LGTM ontestRestoremethod.The test comprehensively covers both scenarios (with and without packages) and properly exercises the
save()andrestore()lifecycle. Mock expectations are appropriately set usingself::for consistency, and the branching logic correctly validates different fallback behaviors.
126-134: LGTM ontestRestoreWithDisableOption.This test correctly validates that when
fallback-composeris disabled, therestore()method returns early without writing any output. The minimal constructor parameters are appropriate here since the disabled path doesn't require the filesystem or installer dependencies.
158-187: LGTM onsetUpmethod.The setup properly initializes an isolated temporary workspace with a unique directory name, creates all necessary mocks, and correctly constructs the
ComposerFallbackinstance with full dependencies. Thechdir()ensures file operations work relative to the test workspace.
189-206: LGTM ontearDownmethod.The cleanup follows the correct sequence: restoring the original working directory before removing the temporary workspace, ensuring no issues with deleting the current directory. Explicit property nullification provides clear state cleanup between tests.
resources/doc/events.md (1)
1-1: LGTM!The ATX-style heading (
# Events) is a good formatting improvement for consistency across the documentation files.resources/doc/config.md (2)
1-1: LGTM!The ATX-style heading and consistent blank line formatting improve documentation readability.
401-406: Good addition of behavioral documentation.These notes about
package.jsonhandling (4-space indentation, preserved nested empty arrays, path resolution) provide valuable clarity for users configuringroot-package-json-dir.resources/doc/index.md (1)
1-1: LGTM!Consistent ATX-style heading formatting applied across documentation.
resources/doc/usage.md (1)
73-80: Good addition of behavioral guarantees.This section clearly documents important runtime behaviors that users should know about. The explicit mention of
RuntimeExceptionfor I/O failures and working directory restoration guarantees are helpful for error handling and debugging.resources/doc/faqs.md (1)
1-3: LGTM!Consistent ATX-style headings applied throughout the FAQ documentation.
composer.json (2)
44-46: Good resolution of infection/infection compatibility issue.Moving
infection/infectionto thesuggestsection instead ofrequire-devresolves the PHP version incompatibility issue flagged in previous reviews. Users on PHP 8.2+ can optionally install it while PHP 8.1 users won't have compatibility problems.
73-90: LGTM on script modernization.The scripts have been properly updated to use
./vendor/bin/paths consistently, which is the recommended approach for invoking Composer-installed tools. The mutation, static analysis, and rector scripts are well-configured..github/linters/actionlint.yml (1)
1-6: Upgrade actionlint to v1.7.10+ to resolve anchor/alias false positives.The "alias node mapping node" errors in the
.github/linters/actionlint.ymlconfig are generated by older actionlint versions that did not properly parse YAML anchors and aliases. Modern actionlint (v1.7.10+, released Dec 30, 2025) and GitHub Actions (as of Sep 18, 2025) both support anchors and aliases natively. Rather than suppressing these errors, upgrade actionlint and remove the ignore rules—anchors are a documented best practice for deduplicating workflow fragments likepathsandbranchesacrosspushandpull_requesttriggers.Likely an incorrect or invalid review comment.
.github/workflows/mutation.yml (1)
21-24: Confirmbefore-hookis a supported input for the reusable workflow.The external repository
yii2-framework/actionscould not be located via public sources, so the accepted inputs for itsinfection.ymlworkflow cannot be verified automatically. If the input names do not match the workflow's specification, this job will fail. Please verify the accepted inputs inyii2-framework/actions/.github/workflows/infection.ymland align accordingly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
resources/doc/faqs.md (5)
11-14: Polish grammar for clarity.The sentence structure is awkward and contains subject-verb agreement errors ("libraries that using assets", "list of asset dependencies that using by your PHP library", "ask him"). Consider rewriting for clarity and neutrality.
💡 Suggested edit
-but when you create PHP libraries that using assets, there is no way to automatically add asset dependencies, -and most importantly, no validation of versions can be done automatically. You must tell the developers -the list of asset dependencies that using by your PHP library, and you must ask him to add manually the asset -dependencies to its asset manager of his project. +but when you create PHP libraries that use assets, there is no way to automatically add asset dependencies, +and, most importantly, no version validation can be done automatically. You must tell developers +which asset dependencies your PHP library uses, and you must ask them to add those dependencies manually +to the asset manager of their project.
16-19: Fix verb agreement and remove awkward comma."another solution exist" and "less possible, to use" are grammatical issues.
💡 Suggested edit
-However, another solution exist - what many projects propose - you must add the assets in the folder of the +However, another solution exists—what many projects propose—is to add the assets in the folder of the ... -less possible, to use correctly all tools such as Babel, Scss, Less, etc ... +less possible to correctly use tools such as Babel, Scss, Less, etc.
39-41: Typo: "depreciated" vs "deprecated"."Bower has been depreciated" should be "deprecated."
💡 Suggested edit
-Now, Bower has been depreciated, NPM has a true lock file (since 5.x), as well as the possibility +Now, Bower has been deprecated, NPM has a true lock file (since 5.x), as well as the possibility
49-52: Grammar/tense issues in conclusion."there is not a backward compatibility" and "Plugin was become Foxy" read incorrectly.
💡 Suggested edit
-To conclude, given that there is not a backward compatibility, and that it is impossible to have a version +To conclude, given that there is no backward compatibility, and that it is impossible to have a version ... -Plugin was become Foxy. +Plugin became Foxy.
67-69: Fix subject-verb agreement."For more details, the plugin work in this order:" should be "works."
💡 Suggested edit
-For more details, the plugin work in this order: +For more details, the plugin works in this order:
🤖 Fix all issues with AI agents
In @.github/linters/.editorconfig-checker.json:
- Around line 1-7: Fix the inconsistent indentation in the
.editorconfig-checker.json by aligning the closing bracket for the "Exclude"
array to use 4-space indentation (matching the rest of the file); update the
trailing closing bracket and the final brace so they align with the opening
braces around the "Exclude" property, ensuring the JSON object formatting is
consistent.
In `@resources/doc/faqs.md`:
- Line 3: Update the ungrammatical Markdown heading "## What version required of
Composer?" to a correct form such as "## What version of Composer is required?"
(or "## Which Composer version is required?") by editing the heading text in the
file; ensure only the heading is changed and surrounding content remains
untouched.
- Around line 73-75: Revise the two bullet points for clarity and consistent
imperative phrasing: replace "Retains only PHP dependencies with the `foxy/foxy`
dependency in the `require` or `require-dev` section of the `composer.json` file
and with the presence of the `package.json` file" with a concise imperative like
"Keep only PHP packages that list `foxy/foxy` in the `require` or `require-dev`
section of composer.json and that are accompanied by a package.json file", and
rewrite "Checking the lock file of asset manager" as an imperative such as
"Check the asset manager lock file" so both steps read clearly and consistently.
- Around line 102-107: Update the FAQ paragraph under the "Why Foxy does nothing
with the '--dry-run' option?" heading to use present tense and the standard
"cannot" spelling: change "chose" to "chooses" and all occurrences of "can not"
to "cannot" so the sentence reads e.g. "Foxy can work with Composer's
`--dry-run` option, but chooses to do nothing" and "Foxy cannot update the
`package.json` file".
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.github/CODE_OF_CONDUCT.md.github/FUNDING.yml.github/ISSUE_TEMPLATE.md.github/PULL_REQUEST_TEMPLATE.md.github/linters/.editorconfig-checker.jsonCHANGELOG.mdREADME.mdresources/doc/faqs.mdtests/Support/InternalMockerExtension.php
💤 Files with no reviewable changes (4)
- .github/FUNDING.yml
- .github/ISSUE_TEMPLATE.md
- .github/CODE_OF_CONDUCT.md
- .github/PULL_REQUEST_TEMPLATE.md
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: User terabytesoftw prefers not to use aliases when importing classes in PHP. They are comfortable with either fully-qualified class names or direct imports without aliases.
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
📚 Learning: 2026-01-22T11:42:32.878Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 111
File: tests/Fallback/AssetFallbackTest.php:98-98
Timestamp: 2026-01-22T11:42:32.878Z
Learning: When using xepozz/internal-mocker in PHP tests, the library's namespaced wrapper records and matches function calls with all default parameters filled in. To ensure proper matching, always specify every parameter in MockerState::addCondition(), including defaults. For example, a call to file_get_contents($path) is observed as file_get_contents($path, false, null, 0, null). Use the same complete argument list (including default values) when adding conditions for mocks in tests like tests/Fallback/AssetFallbackTest.php.
Applied to files:
tests/Support/InternalMockerExtension.php
📚 Learning: 2026-01-23T11:22:00.118Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: In PHP tests (e.g., tests/Fallback/AssetFallbackTest.php) avoid using import aliases. Prefer either importing the full namespace without an alias or using fully-qualified class names directly. For example, avoid statements like 'use Some\Long\Namespace as Alias;' and either 'use Some\\Long\\Namespace;' or reference 'Some\\Long\\Namespace' in code. This guideline applies to PHP files under tests and improves readability and refactor safety.
Applied to files:
tests/Support/InternalMockerExtension.php
🧬 Code graph analysis (1)
.github/linters/.editorconfig-checker.json (1)
tests/Json/JsonFileTest.php (1)
JsonFileTest(29-241)
🪛 PHPMD (2.15.0)
tests/Support/InternalMockerExtension.php
16-16: Avoid unused parameters such as '$configuration'. (undefined)
(UnusedFormalParameter)
16-16: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
22-22: Avoid using static access to class 'Foxy\Tests\Support\InternalMockerExtension' in method 'bootstrap'. (undefined)
(StaticAccess)
28-28: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'bootstrap'. (undefined)
(StaticAccess)
32-32: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
34-34: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'bootstrap'. (undefined)
(StaticAccess)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: phpstan / PHP 8.5-ubuntu-latest
- GitHub Check: easy-coding-standard / PHP 8.5-ubuntu-latest
- GitHub Check: composer-require-checker / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.5-windows-2022
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
🔇 Additional comments (4)
CHANGELOG.md (1)
17-17: Changelog entry looks correct.
Matches the PR scope and numbering.tests/Support/InternalMockerExtension.php (3)
7-12: LGTM!The grouped import syntax improves readability and keeps related classes organized by namespace. The new
FinishedandFinishedSubscriberimports are properly added to support the new event handling.
19-30: LGTM!Removing the empty parentheses from anonymous class instantiations is a valid simplification—parentheses are only required when passing constructor arguments.
31-36: LGTM! Good addition for test isolation.Adding the
FinishedSubscriberto reset mock state after each test completes ensures proper cleanup and test isolation. Combined with the existingPreparationStartedSubscriberreset, this provides robust state management—resetting both before setup and after completion.The static analysis warning about the unused
$eventparameter is a false positive; the parameter is required by theFinishedSubscriberinterface contract.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@ecs.php`:
- Around line 81-95: The call to withPhpCsFixerSets(...) uses the wrong named
argument perCS30; update that argument name to perCS3x0 to match
ECS13/PHP-CS-Fixer naming conventions—locate the withPhpCsFixerSets invocation
and replace perCS30: true with perCS3x0: true so the intended PHP-CS-Fixer set
is enabled.
In `@resources/doc/faqs.md`:
- Line 43: Replace the phrase "more and more required" with a stronger adverb
such as "increasingly required" in the sentence containing "Node.js filling its
gaps, and becoming more and more required, a plugin could finally perform the
reverse operation," (search for the exact phrase "more and more required") to
make the wording more concise and technically clear.
♻️ Duplicate comments (2)
resources/doc/faqs.md (2)
104-104: Complete the grammar fix: "can not" → "cannot".Line 104 still contains "npm can not" which should be "cannot" for standard English usage. The previous review comment addressed "Foxy can not" but missed this second occurrence.
📝 Proposed fix
-are not installed, updated or deleted, Foxy cannot update the `package.json` file, and so, npm can not +are not installed, updated or deleted, Foxy cannot update the `package.json` file, and so, npm cannot check the new constraints, if any. To sum up, this amounts to running the commandsBased on learnings, as per the static analysis tools and previous review feedback.
73-74: Maintain consistent verb form in the list.Line 73 uses "Retains" (third person present tense) while all other items in the numbered list use gerund forms (Validation, Saving, Installing, Retrieving, Comparing, Creating, etc.). For consistency, this should match the pattern.
📝 Proposed fix
-5. Retains only PHP dependencies that include `foxy/foxy` dependency in `require` or `require-dev` and that contain a +5. Retaining only PHP dependencies that include `foxy/foxy` dependency in `require` or `require-dev` and that contain a `package.json` file
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/linters/.editorconfig-checker.jsonecs.phpresources/doc/faqs.mdtests/Fallback/ComposerFallbackTest.php
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: User terabytesoftw prefers not to use aliases when importing classes in PHP. They are comfortable with either fully-qualified class names or direct imports without aliases.
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
📚 Learning: 2026-01-23T11:22:00.118Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: User terabytesoftw prefers not to use aliases when importing classes in PHP. They are comfortable with either fully-qualified class names or direct imports without aliases.
Applied to files:
ecs.php
📚 Learning: 2026-01-23T11:22:00.118Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:00.118Z
Learning: In PHP tests (e.g., tests/Fallback/AssetFallbackTest.php) avoid using import aliases. Prefer either importing the full namespace without an alias or using fully-qualified class names directly. For example, avoid statements like 'use Some\Long\Namespace as Alias;' and either 'use Some\\Long\\Namespace;' or reference 'Some\\Long\\Namespace' in code. This guideline applies to PHP files under tests and improves readability and refactor safety.
Applied to files:
tests/Fallback/ComposerFallbackTest.php
📚 Learning: 2025-08-18T20:13:33.518Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Applied to files:
tests/Fallback/ComposerFallbackTest.php
📚 Learning: 2025-09-28T15:12:48.345Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
Applied to files:
tests/Fallback/ComposerFallbackTest.php
📚 Learning: 2026-01-23T11:09:08.789Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: composer.json:68-77
Timestamp: 2026-01-23T11:09:08.789Z
Learning: In the php-forge organization repositories, terabytesoftw prefers using the main branch reference in composer.json sync-metadata scripts when downloading configuration files from external template repositories, rather than pinning to specific commit SHAs or tags, even though pinning provides more stability and prevents unexpected changes.
Applied to files:
tests/Fallback/ComposerFallbackTest.php
📚 Learning: 2026-01-22T11:42:32.878Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 111
File: tests/Fallback/AssetFallbackTest.php:98-98
Timestamp: 2026-01-22T11:42:32.878Z
Learning: When using xepozz/internal-mocker in PHP tests, the library's namespaced wrapper records and matches function calls with all default parameters filled in. To ensure proper matching, always specify every parameter in MockerState::addCondition(), including defaults. For example, a call to file_get_contents($path) is observed as file_get_contents($path, false, null, 0, null). Use the same complete argument list (including default values) when adding conditions for mocks in tests like tests/Fallback/AssetFallbackTest.php.
Applied to files:
tests/Fallback/ComposerFallbackTest.php
🧬 Code graph analysis (2)
.github/linters/.editorconfig-checker.json (1)
tests/Json/JsonFileTest.php (1)
JsonFileTest(29-241)
tests/Fallback/ComposerFallbackTest.php (2)
src/Fallback/ComposerFallback.php (3)
ComposerFallback(20-162)save(66-81)restore(47-64)src/Fallback/FallbackInterface.php (2)
save(17-17)restore(12-12)
🪛 LanguageTool
resources/doc/faqs.md
[style] ~43-~43: Consider using a different adverb to strengthen your wording.
Context: ... Node.js filling its gaps, and becoming more and more required, a plugin could finally perfor...
(MORE_AND_MORE)
[style] ~104-~104: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...te the package.json file, and so, npm can not check the new constraints, if any. To s...
(CAN_NOT_PREMIUM)
🪛 PHPMD (2.15.0)
tests/Fallback/ComposerFallbackTest.php
35-35: Avoid variables with short names like $fs. Configured minimum length is 3. (undefined)
(ShortVariable)
38-38: Avoid variables with short names like $io. Configured minimum length is 3. (undefined)
(ShortVariable)
83-83: Avoid variables with short names like $ed. Configured minimum length is 3. (undefined)
(ShortVariable)
87-87: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
91-91: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
96-96: Avoid using static access to class '\Foxy\Util\LockerUtil' in method 'testRestore'. (undefined)
(StaticAccess)
114-117: The method testRestore uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
143-143: Avoid variables with short names like $rm. Configured minimum length is 3. (undefined)
(ShortVariable)
147-147: Avoid variables with short names like $im. Configured minimum length is 3. (undefined)
(ShortVariable)
179-179: Missing class import via use statement (line '179', column '26'). (undefined)
(MissingImport)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: phpstan / PHP 8.5-ubuntu-latest
- GitHub Check: composer-require-checker / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.5-windows-2022
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: composer-require-checker / PHP 8.5-ubuntu-latest
🔇 Additional comments (14)
tests/Fallback/ComposerFallbackTest.php (7)
23-27: LGTM!Good use of explicit function and constant imports. This makes dependencies clear and aligns with PHP best practices for explicit namespace usage.
42-50: LGTM!Data providers are well-structured and cover the key test scenarios: empty vs. populated packages for restore, and with/without lock file for save operations.
57-121: LGTM!The test method is well-structured with:
- Consistent use of
JSON_THROW_ON_ERRORfor JSON encoding- Modern PHPUnit assertion patterns (
self::any(),self::once(), etc.)- Clear branching logic that tests both the remove-vendor and run-installer paths based on package data
141-161: LGTM!The test method properly:
- Uses
JSON_THROW_ON_ERRORfor consistent error handling (addressing the previous review feedback)- Covers both scenarios via the data provider (with and without lock file)
- Validates the fluent interface by asserting the return type
163-192: LGTM!The
setUpmethod follows PHPUnit best practices with:
- Proper isolation using a unique temp directory
- Complete mock initialization for all dependencies
- Working directory management for test isolation
194-211: LGTM!Proper cleanup implementation:
- Restores working directory before removing temp files (correct order)
- Removes temp directory to prevent disk space accumulation
- Nullifies all properties to prevent test state leakage
126-134: No issues found. TheComposerFallbackconstructor properly supports instantiation with 4 arguments because the last two parameters ($fsand$installer) have default values ofnull. The test design is correct and not brittle..github/linters/.editorconfig-checker.json (1)
1-8: LGTM! Configuration is well-structured.The exclusion list is appropriate for files that should skip editorconfig validation (baseline files, documentation, and JSON test fixtures), and the indentation issue from the previous review has been resolved.
ecs.php (6)
5-11: LGTM!Clean grouped import syntax following PHP 8.0+ conventions. All imported fixers are utilized in the configuration.
13-19: LGTM!Proper fluent API initialization with valid
ClassDefinitionFixerconfiguration.
20-25: Previous issue resolved.The contradictory configuration (configured then skipped) has been properly fixed. The
NullableTypeDeclarationFixerwith'syntax' => 'union'will now be applied as intended.
26-49: Previous issue resolved - comprehensive class element ordering.The
method_public_abstractentry (line 41) has been added as previously suggested. The ordering configuration now properly handles all visibility levels for abstract methods.
50-67: LGTM!Standard import ordering (class → function → const) and sensible PHPDoc type ordering with null placed last in union types.
68-80: LGTM!PHPUnit static method call enforcement using
self::is consistent with static assertion patterns. Path configuration correctly scopes linting tosrcandtestsdirectories.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
