Fix PHPStan blind spots and runtime edge cases in Arrayy; add audit and tests#175
Fix PHPStan blind spots and runtime edge cases in Arrayy; add audit and tests#175voku wants to merge 6 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis change audits PHPStan ignores, adds dynamic return-type configuration and fixtures, tightens annotations and generic typing, adjusts JSON mapping and Arrayy traversal behavior, and expands tests for scalar paths, replacements, mixed data, access shapes, and callable inference. ChangesPHPStan audit and runtime corrections
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Tick the box to add this pull request to the merge queue (same as
|
|
coverage: 92.676% (+0.3%) from 92.38% — codex/integriere-phpstan-annotations into master |
3 similar comments
|
coverage: 92.676% (+0.3%) from 92.38% — codex/integriere-phpstan-annotations into master |
|
coverage: 92.676% (+0.3%) from 92.38% — codex/integriere-phpstan-annotations into master |
|
coverage: 92.676% (+0.3%) from 92.38% — codex/integriere-phpstan-annotations into master |
|
coverage: 91.157% (-1.2%) from 92.38% — codex/integriere-phpstan-annotations into master |
|
coverage: 92.676% (+0.3%) from 92.38% — codex/integriere-phpstan-annotations into master |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Mapper/Json.php`:
- Around line 82-83: Update the normalization before the foreach in the JSON
mapping method to preserve \Traversable inputs by iterating them directly, while
continuing to use get_object_vars() for non-Traversable objects and arrays. Add
a regression test covering an iterator such as ArrayIterator or Generator and
verify its yielded entries are mapped.
In `@tests/ArrayyTest.php`:
- Around line 216-229: Update cleanProvider() to branch its expected result for
the negative-key dataset based on PHP_VERSION_ID: retain the PHP 8.3+
expectation, while PHP 8.0–8.2 must expect [-8 => -9, 0 => 1]. Keep all other
provider cases unchanged and preserve compatibility with the declared PHP >=8.0
matrix.
In `@tests/PHPStan/AnalyseTest.php`:
- Around line 34-36: The PHPStan regression test must continue verifying the
inferred generic types instead of suppressing analysis errors. Update the
assertions around the foreach using $chunk and the corresponding checks at lines
69-78 to restore equivalent assertType coverage for $chunk, int, and \stdClass,
and remove the method.nonObject suppression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cb6009c8-a23b-4103-a88b-24c7c459968a
📒 Files selected for processing (26)
build/docs/phpstan-ignore-audit.mdphpstan.neonsrc/Arrayy.phpsrc/Collection/AbstractCollection.phpsrc/Create.phpsrc/Mapper/Json.phpsrc/PHPStan/MetaDynamicStaticMethodReturnTypeExtension.phpsrc/Type/DetectFirstValueTypeCollection.phpsrc/TypeCheck/TypeCheckCallback.phpsrc/TypeCheck/TypeCheckPhpDoc.phptests/Account.phptests/ArrayyTest.phptests/BasicArrayTest.phptests/Collection/BoolTypeTest.phptests/Collection/CollectionTest.phptests/Collection/StringTypeTest.phptests/Collection/TypeTypeTest.phptests/Collection/TypesTest.phptests/JsonMapperCoverageTest.phptests/JsonMapperTest.phptests/MetaPhpStanIntegrationTest.phptests/ModelA.phptests/PHPStan/AnalyseTest.phptests/PHPStan/CallableGenericInferenceTest.phptests/PHPStan/phpstan-fixtures.neontests/TypeCheckCoreCoverageTest.php
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/PHPStan/GetDynamicMethodReturnTypeExtension.php`:
- Around line 46-60: Restrict nested path inference in
GetDynamicMethodReturnTypeExtension to calls using the default "." path
separator, or otherwise obtain and honor the configured separator before
splitting paths. Document this default-separator restriction in README.md lines
123-133 and build/docs/base.md lines 122-132; no other behavior should change.
In `@tests/PHPStan/AccessShapeProfile.php`:
- Around line 14-17: Update the offset parameter annotation on
AccessShapeProfile::offsetGet() from only 'name' to the full 'avatar'|'name'
union declared by the profile, while preserving its string return type and
existing missing-value exception behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c11c3951-ebec-47e1-a787-2f811a4937a9
📒 Files selected for processing (8)
README.mdbuild/docs/base.mdphpstan.neonsrc/PHPStan/GetDynamicMethodReturnTypeExtension.phptests/PHPStan/AccessShapeProfile.phptests/PHPStan/AccessShapeUser.phptests/PHPStan/AccessWaysTest.phptests/PHPStan/phpstan-fixtures.neon
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/PHPStan/phpstan-fixtures.neon
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Mapper/Json.php (1)
82-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the nested ternary into an independent conditional.
Line 84 is behaviorally correct, but separating the
Traversable, object, and iterable cases will improve readability and address the reported SonarCloud warning.Proposed refactor
- $jsonValues = $json instanceof \Traversable - ? $json - : (\is_object($json) ? \get_object_vars($json) : $json); + if ($json instanceof \Traversable) { + $jsonValues = $json; + } elseif (\is_object($json)) { + $jsonValues = \get_object_vars($json); + } else { + $jsonValues = $json; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Mapper/Json.php` around lines 82 - 85, In the JSON value preparation before the foreach in the mapper, replace the nested ternary with separate conditional branches for Traversable values, objects, and other iterable inputs. Preserve the existing behavior and resulting $jsonValues for each case while eliminating the SonarCloud readability warning.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Mapper/Json.php`:
- Around line 82-85: In the JSON value preparation before the foreach in the
mapper, replace the nested ternary with separate conditional branches for
Traversable values, objects, and other iterable inputs. Preserve the existing
behavior and resulting $jsonValues for each case while eliminating the
SonarCloud readability warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d947ad8-2e48-4bb4-8bd3-28559254f7c4
📒 Files selected for processing (13)
README.mdbuild/docs/base.mdsrc/Arrayy.phpsrc/Mapper/Json.phpsrc/PHPStan/DefaultDotNotationTypeInterface.phpsrc/PHPStan/GetDynamicMethodReturnTypeExtension.phptests/ArrayyTest.phptests/JsonMapperCoverageTest.phptests/PHPStan/AccessShapeProfile.phptests/PHPStan/AccessShapeUser.phptests/PHPStan/AccessWaysTest.phptests/PHPStan/AnalyseTest.phptests/PHPStan/CustomSeparatorAccessUser.php
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/PHPStan/AccessShapeProfile.php
- tests/PHPStan/AccessWaysTest.php
- tests/PHPStan/AccessShapeUser.php
- tests/JsonMapperCoverageTest.php
- src/PHPStan/GetDynamicMethodReturnTypeExtension.php
- tests/ArrayyTest.php
- src/Arrayy.php
Apply fixes from StyleCI
|
3 participants
Motivation
@phpstan-ignore-next-lineannotations.array_combine()behavior on PHP 8, float keys, and JSON mapper typing so runtime behavior matches documented contracts.Description
build/docs/phpstan-ignore-audit.mdand enable an analysis exclude list inphpstan.neon, plus addtests/PHPStan/phpstan-fixtures.neonto centralize fixture config.Arrayyimplementation: guardarray_key_exists()calls withis_array()for dot-path checks, normalize float keys toint, stop descending into scalar intermediate values incallAtPath(), and add many targeted/* @phpstan-ignore-next-line ... */annotations where the API intentionally re-parameterizes generics.array_combine()is only called when counts match (returning an empty collection otherwise), and normalize other behaviors (JSON mapper inputs, traversal, generator conversions) to have explicit runtime-safe types.Mapper/Json.php, fix type parsing utilities and factory/creation generics acrossCreate.php,MetaDynamicStaticMethodReturnTypeExtension.php, and severalType*classes.tests/PHPStan/CallableGenericInferenceTest.php, adjust many test phpdoc annotations toarray<mixed>, add targeted phpstan suppressions in tests, and add explicit unit tests validating dot-notation stopping at scalar, mismatched replacement-size behavior, andwhere()support for arrays and objects.Testing
vendor/bin/phpunit(the repository test harness), which executed updated tests including new PHPStan-related coverage tests, and all tests succeeded.MetaPhpStanIntegrationTest/ fixture analysis usingtests/PHPStan/phpstan-fixtures.neon), and the phpstan-driven tests passed under the updated configuration.array_combine()mismatch cases, JSON mapper edge cases, and callable/generic inference, and these tests also passed.Codex Task
This change is
Summary by CodeRabbit