Conversation
…pe safety for array inputs
- Document array and ArrayObject support with item parameter - Include examples for basic array usage and custom ArrayObject inheritance - Show query data format for arrays - Demonstrate controller method usage with array parameters 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer's GuideThis PR extends the Input attribute system to automatically convert array and ArrayObject parameters into collections of Input-annotated objects by adding item-type handling in InputQuery, updating the Input attribute, documentation, tests, and a demo script. Sequence diagram for array and ArrayObject parameter resolution in InputQuerysequenceDiagram
participant Controller as UserController
participant InputQuery
participant Input as Input attribute
participant UserInput
participant ArrayObject
Controller->>InputQuery: getArguments(method, query)
InputQuery->>Input: read item type (UserInput::class)
alt array parameter
InputQuery->>InputQuery: createArrayOfInputs(paramName, query, UserInput)
loop for each user data
InputQuery->>UserInput: create(UserInput, itemData)
end
InputQuery-->>Controller: array of UserInput
else ArrayObject parameter
InputQuery->>InputQuery: createArrayOfInputs(paramName, query, UserInput)
loop for each user data
InputQuery->>UserInput: create(UserInput, itemData)
end
InputQuery->>ArrayObject: new ArrayObject(array of UserInput)
InputQuery-->>Controller: ArrayObject of UserInput
end
Entity relationship diagram for UserInput and UserCollectionerDiagram
USER_INPUT {
string id
string name
}
USER_COLLECTION ||--o{ USER_INPUT : contains
Class diagram for updated Input attribute and InputQuery handlingclassDiagram
class Input {
+string|null item
+__construct(string|null item = null)
}
class InputQuery {
-createArrayOfInputs(paramName: string, query: array, itemClass: string): array
-resolveParameter(param: ReflectionParameter, query: array): mixed
...
}
class ArrayObject
class UserInput {
+string id
+string name
+__construct(string id, string name)
}
class UserCollection {
+getFirst(): ?UserInput
}
UserCollection --|> ArrayObject
InputQuery --> Input : uses
InputQuery --> ArrayObject : instantiates
InputQuery --> UserInput : creates
InputQuery --> UserCollection : creates
UserCollection --> UserInput : contains
UserInput <.. Input : attribute
Class diagram for custom ArrayObject subclass supportclassDiagram
class UserCollection {
+getFirst(): ?UserInput
}
class ArrayObject
UserCollection --|> ArrayObject
UserCollection --> UserInput : contains
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughArray and collection input support was added to the input mapping system, enabling automatic population of method parameters typed as arrays or ArrayObject subclasses using the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant InputQuery
participant Controller
participant UserInput
Client->>InputQuery: getArguments(reflection, query)
InputQuery->>InputQuery: Detect array/ArrayObject param with #[Input(item: UserInput::class)]
InputQuery->>InputQuery: createArrayOfInputs(paramName, query, UserInput::class)
loop For each element in query[paramName]
InputQuery->>UserInput: create instance from element
end
InputQuery->>Controller: Provide array/ArrayObject of UserInput
Controller->>Client: Method executes with populated collection
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @koriym - I've reviewed your changes - here's some feedback:
- You can consolidate the three separate branches for
array,ArrayObject, and its subclasses into a single code path (e.g. unify logic based on whether the type is iterable) to reduce duplication inresolveParameter. - Instead of silently returning an empty array when the query value isn’t an array, consider throwing or logging an error for invalid data to help callers detect mis-typed input early.
- Update the
@returndocblock oncreateArrayOfInputstoarray<array-key, T>(orarray<int|string, T>) so static analyzers and readers know you preserve the original keys.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You can consolidate the three separate branches for `array`, `ArrayObject`, and its subclasses into a single code path (e.g. unify logic based on whether the type is iterable) to reduce duplication in `resolveParameter`.
- Instead of silently returning an empty array when the query value isn’t an array, consider throwing or logging an error for invalid data to help callers detect mis-typed input early.
- Update the `@return` docblock on `createArrayOfInputs` to `array<array-key, T>` (or `array<int|string, T>`) so static analyzers and readers know you preserve the original keys.
## Individual Comments
### Comment 1
<location> `src/InputQuery.php:315` </location>
<code_context>
+
+ $result = [];
+ /** @var mixed $itemData */
+ foreach ($arrayData as $key => $itemData) {
+ if (is_array($itemData)) {
+ // Query parameters from HTTP requests have string keys
+ /** @psalm-var array<string, mixed> $itemData */
+ /** @phpstan-var array<string, mixed> $itemData */
+ $result[$key] = $this->create($itemClass, $itemData);
+ }
+ }
+
</code_context>
<issue_to_address>
Non-array items in the input array are silently ignored.
This may cause data loss. Please handle or report non-array elements explicitly.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
$result = [];
/** @var mixed $itemData */
foreach ($arrayData as $key => $itemData) {
if (is_array($itemData)) {
// Query parameters from HTTP requests have string keys
/** @psalm-var array<string, mixed> $itemData */
/** @phpstan-var array<string, mixed> $itemData */
$result[$key] = $this->create($itemClass, $itemData);
}
}
=======
$result = [];
/** @var mixed $itemData */
foreach ($arrayData as $key => $itemData) {
if (is_array($itemData)) {
// Query parameters from HTTP requests have string keys
/** @psalm-var array<string, mixed> $itemData */
/** @phpstan-var array<string, mixed> $itemData */
$result[$key] = $this->create($itemClass, $itemData);
} else {
throw new InvalidArgumentException(
sprintf(
'Expected array for item at key "%s", got %s.',
$key,
gettype($itemData)
)
);
}
}
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/InputQuery.php (1)
88-169: Address the high complexity in resolveParameter() method.The method has grown significantly complex with an NPath complexity of 384 (threshold is 200), making it harder to maintain and test. The logic for handling ArrayObject is also duplicated.
Consider extracting the array/collection handling logic into separate methods:
private function resolveParameter(ReflectionParameter $param, array $query): mixed { $inputAttributes = $param->getAttributes(Input::class); $hasInputAttribute = ! empty($inputAttributes); if (! $hasInputAttribute) { return $this->resolveFromDI($param); } $type = $param->getType(); $paramName = $param->getName(); if (! $type instanceof ReflectionNamedType) { return $query[$paramName] ?? $this->getDefaultValue($param); } + // Handle collection types (arrays and ArrayObject) + if ($this->isCollectionType($type)) { + return $this->resolveCollectionParameter($param, $query, $inputAttributes[0]); + } if ($type->isBuiltin()) { - // Check if it's an array type with item specification - if ($type->getName() === 'array') { - $inputAttribute = $inputAttributes[0]->newInstance(); - if ($inputAttribute->item !== null) { - assert(class_exists($inputAttribute->item)); - $itemClass = $inputAttribute->item; - /** @var class-string<T> $itemClass */ - return $this->createArrayOfInputs($paramName, $query, $itemClass); - } - } // Scalar type with #[Input] $value = $query[$paramName] ?? $this->getDefaultValue($param); return $this->convertScalar($value, $type); } - // Check if it's ArrayObject or its subclass with item specification - $className = $type->getName(); - if (class_exists($className) && is_subclass_of($className, ArrayObject::class)) { - $inputAttribute = $inputAttributes[0]->newInstance(); - if ($inputAttribute->item !== null) { - assert(class_exists($inputAttribute->item)); - /** @var class-string<T> $itemClass */ - $itemClass = $inputAttribute->item; - $array = $this->createArrayOfInputs($paramName, $query, $itemClass); - $reflectionClass = new ReflectionClass($className); - return $reflectionClass->newInstance($array); - } - } - - // Check if it's ArrayObject itself with item specification - if ($className === ArrayObject::class) { - $inputAttribute = $inputAttributes[0]->newInstance(); - if ($inputAttribute->item !== null) { - assert(class_exists($inputAttribute->item)); - /** @var class-string<T> $itemClass */ - $itemClass = $inputAttribute->item; - $array = $this->createArrayOfInputs($paramName, $query, $itemClass); - return new ArrayObject($array); - } - } // Object type with #[Input] - create nested // ... existing logic } +private function isCollectionType(ReflectionNamedType $type): bool +{ + $typeName = $type->getName(); + return $typeName === 'array' || + ($typeName === ArrayObject::class) || + (class_exists($typeName) && is_subclass_of($typeName, ArrayObject::class)); +} +private function resolveCollectionParameter(ReflectionParameter $param, array $query, ReflectionAttribute $inputAttribute): mixed +{ + $type = $param->getType(); + $paramName = $param->getName(); + $inputAttr = $inputAttribute->newInstance(); + + if ($inputAttr->item === null) { + return $query[$paramName] ?? $this->getDefaultValue($param); + } + + assert(class_exists($inputAttr->item)); + /** @var class-string<T> $itemClass */ + $itemClass = $inputAttr->item; + $array = $this->createArrayOfInputs($paramName, $query, $itemClass); + + $typeName = $type->getName(); + if ($typeName === 'array') { + return $array; + } + + // ArrayObject or its subclass + if ($typeName === ArrayObject::class) { + return new ArrayObject($array); + } + + $reflectionClass = new ReflectionClass($typeName); + return $reflectionClass->newInstance($array); +}
🧹 Nitpick comments (1)
tests/ArrayInputTest.php (1)
35-35: Consider adding documentation annotations to anonymous class methods.While not critical, adding
@paramand@returnannotations would improve static analysis compliance.$controller = new class { + /** + * @param array<UserInputWithAttribute> $users + * @return array<UserInputWithAttribute> + */ public function listUsers( #[Input(item: UserInputWithAttribute::class)] array $users, ): array { return $users; } };Also applies to: 68-68, 96-96, 116-116, 136-136, 164-164, 192-192
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.claude/settings.local.json(1 hunks)README.md(1 hunks)demo/ArrayDemo.php(1 hunks)src/Attribute/Input.php(1 hunks)src/InputQuery.php(4 hunks)tests/ArrayInputTest.php(1 hunks)tests/Fake/CustomArrayObject.php(1 hunks)tests/Fake/UserInput.php(1 hunks)tests/Fake/UserInputWithAttribute.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
tests/Fake/UserInputWithAttribute.php (2)
src/InputQuery.php (2)
InputQuery(44-327)__construct(46-49)demo/ArrayDemo.php (1)
__construct(14-20)
tests/Fake/CustomArrayObject.php (1)
src/InputQuery.php (1)
InputQuery(44-327)
tests/ArrayInputTest.php (5)
src/InputQuery.php (1)
InputQuery(44-327)tests/Fake/CustomArrayObject.php (2)
CustomArrayObject(9-15)getFirst(11-14)tests/Fake/UserInputWithAttribute.php (1)
UserInputWithAttribute(9-18)demo/ArrayDemo.php (1)
listUsers(26-34)src/InputQueryInterface.php (1)
getArguments(19-19)
demo/ArrayDemo.php (4)
src/InputQuery.php (2)
InputQuery(44-327)__construct(46-49)tests/Fake/UserInputWithAttribute.php (1)
__construct(11-17)tests/ArrayInputTest.php (7)
listUsers(35-40)listUsers(68-73)listUsers(96-101)listUsers(116-121)listUsers(136-141)listUsers(164-169)listUsers(192-197)src/InputQueryInterface.php (1)
getArguments(19-19)
🪛 GitHub Actions: Static Analysis
src/InputQuery.php
[error] 88-88: PHP Mess Detector: The method resolveParameter() has an NPath complexity of 384. The configured NPath complexity threshold is 200.
🪛 GitHub Check: Coding Standards
tests/ArrayInputTest.php
[failure] 164-164:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 164-164:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
[failure] 136-136:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 136-136:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
[failure] 116-116:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 116-116:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
[failure] 96-96:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 96-96:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
[failure] 35-35:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 35-35:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
🔇 Additional comments (17)
.claude/settings.local.json (1)
7-7: LGTM: Necessary permission for new test infrastructure.The addition of PHPUnit execution permission aligns with the new test files introduced in this PR and follows the existing permission pattern.
tests/Fake/UserInput.php (1)
14-15: LGTM: Formatting improvement.The formatting change from inline to multi-line constructor body improves code consistency.
tests/Fake/CustomArrayObject.php (1)
9-15: LGTM: Well-designed test helper class.The
CustomArrayObjectclass provides a clean test helper for verifying custom ArrayObject subclass support. ThegetFirst()method implementation using array access with null coalescing is correct and themixedreturn type is appropriate.src/Attribute/Input.php (1)
12-15: LGTM: Core enhancement for array support.The constructor addition with the optional
itemparameter successfully enables array and ArrayObject support while maintaining backward compatibility. Thestring|nulltype with defaultnullis appropriate for specifying item class names.tests/Fake/UserInputWithAttribute.php (1)
11-17: LGTM: Well-structured test input class.The
UserInputWithAttributeclass follows the established pattern for input classes with proper#[Input]attribute annotations on constructor parameters. The structure is ideal for testing the new array input functionality.README.md (1)
123-189: Excellent documentation of the new array support feature!The new "Array Support" section is comprehensive and well-structured. It clearly explains:
- Usage of the
#[Input(item: ...)]attribute for arrays and ArrayObject collections- Expected query data format with practical examples
- Support for custom ArrayObject subclasses
- Integration with the existing input system
The examples are clear and demonstrate both basic and advanced usage scenarios.
src/InputQuery.php (2)
7-7: Good addition of necessary imports.The new imports for ArrayObject support are appropriate and necessary for the enhanced functionality.
Also applies to: 20-20, 23-23, 30-30
294-325: Well-implemented createArrayOfInputs method.The method correctly handles:
- Missing parameters (returns empty array)
- Non-array values (returns empty array)
- Filters out non-array elements
- Maintains array keys for valid elements
- Proper type annotations for static analysis
The implementation is robust and handles edge cases appropriately.
tests/ArrayInputTest.php (6)
1-24: Excellent test setup and imports.The test class is well-structured with proper setup using dependency injection and all necessary imports.
25-56: Comprehensive test for array input functionality.The test correctly verifies:
- Array parameter resolution with
#[Input(item: ...)]- Proper instantiation of UserInputWithAttribute objects
- Correct data mapping from query to object properties
- Array structure preservation
58-89: Good coverage of ArrayObject functionality.The test validates ArrayObject parameter handling with proper type checking and data verification.
91-109: Excellent edge case coverage.These tests properly handle:
- Empty arrays
- Missing parameters
- Non-array values
The implementation correctly returns empty arrays for these edge cases, which is the expected behavior.
Also applies to: 111-129, 131-149
151-180: Robust handling of mixed array elements.The test demonstrates that the implementation correctly:
- Filters out non-array elements
- Preserves array keys for valid elements
- Only processes valid array elements into objects
This is exactly the right behavior for handling potentially malformed input data.
182-218: Great test for custom ArrayObject subclasses.The test validates:
- Custom ArrayObject subclass instantiation
- Preservation of custom methods (
getFirst())- Proper inheritance behavior
This confirms the implementation works with ArrayObject inheritance as documented.
demo/ArrayDemo.php (3)
12-21: Well-defined input class.The User class correctly demonstrates the pattern of using
#[Input]attributes on constructor parameters for input mapping.
24-45: Excellent demonstration of array and ArrayObject usage.The controller methods clearly show:
- Array parameter handling with
#[Input(item: User::class)]- ArrayObject parameter handling with the same pattern
- Proper iteration and access to typed objects
- Clear output for demonstration purposes
47-70: Effective demonstration of the complete workflow.The demo successfully shows:
- InputQuery setup with dependency injection
- Sample query data structure (matching the documented format)
- Method resolution using reflection
- Argument resolution and method invocation
- Complete end-to-end usage of the new functionality
This serves as an excellent reference for users implementing the feature.
Split complex resolveParameter method into smaller, focused methods: - resolveInputParameter: handles Input attribute processing - resolveBuiltinType: handles array and scalar types - resolveObjectType: handles object types and ArrayObject - resolveArrayObjectType: handles ArrayObject creation logic This reduces NPath complexity from 384 to below the 200 threshold while maintaining the same functionality and test coverage. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…improve error handling
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/ArrayInputTest.php (3)
26-57: Consider adding PHPDoc annotations to satisfy static analysis.The test logic is correct. To address the static analysis warnings, you could add PHPDoc annotations to the anonymous class method:
$controller = new class { + /** + * @param array<int, UserInputWithAttribute> $users + * @return array<int, UserInputWithAttribute> + */ public function listUsers( #[Input(item: UserInputWithAttribute::class)] array $users, ): array { return $users; } };
92-110: Good edge case coverage.Tests the important scenario of empty arrays. Consider adding PHPDoc annotations for consistency with other fixes:
$controller = new class { + /** + * @param array<int, UserInputWithAttribute> $users + * @return array<int, UserInputWithAttribute> + */ public function listUsers( #[Input(item: UserInputWithAttribute::class)] array $users, ): array { return $users; } };
216-238: Consider strengthening the assertion.The test correctly validates the scenario, but the assertion could be more comprehensive:
// Should create a regular ArrayObject with the nested query data $this->assertInstanceOf(ArrayObject::class, $args[0]); + $this->assertCount(1, $args[0]); + $this->assertIsArray($args[0][0]); + $this->assertSame(['id' => '1', 'name' => 'test'], $args[0][0]);This would ensure the ArrayObject contains the raw nested data as expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/InputQuery.php(4 hunks)tests/ArrayInputTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/InputQuery.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/ArrayInputTest.php (4)
src/InputQuery.php (1)
InputQuery(46-375)tests/Fake/CustomArrayObject.php (2)
CustomArrayObject(9-15)getFirst(11-14)tests/Fake/UserInputWithAttribute.php (1)
UserInputWithAttribute(9-18)src/InputQueryInterface.php (1)
getArguments(19-19)
🪛 GitHub Check: Coding Standards
tests/ArrayInputTest.php
[failure] 162-162:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 162-162:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
[failure] 137-137:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 137-137:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
[failure] 117-117:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 117-117:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
[failure] 97-97:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 97-97:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
[failure] 36-36:
Method class@anonymous::listUsers() does not have @return annotation for its traversable return value.
[failure] 36-36:
Method class@anonymous::listUsers() does not have @param annotation for its traversable parameter $users.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Psalm
- GitHub Check: Psalm
🔇 Additional comments (6)
tests/ArrayInputTest.php (6)
16-24: LGTM!Clean test class setup following PHPUnit best practices.
59-90: LGTM!Comprehensive test for ArrayObject parameter handling with proper validation of the resulting collection.
112-130: LGTM!Proper handling of missing array parameters defaulting to empty array.
132-150: LGTM!Good defensive test for handling non-array values gracefully.
152-176: Excellent error handling test.Properly validates that non-array elements in the input array throw a descriptive exception.
178-214: Comprehensive test for ArrayObject inheritance.Excellent coverage of custom ArrayObject subclass functionality, including validation of custom methods.
…and improve documentation
…and improve documentation
Add array and ArrayObject support for Input attribute
1 participant
Summary
Add support for array and ArrayObject parameters to the Input attribute system, allowing automatic conversion of array data to collections of Input objects.
Features
#[Input(item: UserInput::class)]witharraytype parameters#[Input(item: UserInput::class)]withArrayObjecttype parametersExample Usage
Query Data Format
Test Coverage
Documentation
🤖 Generated with Claude Code
Summary by Sourcery
Add support for mapping query arrays and
ArrayObjectinstances into collections ofInput-decorated objects by introducing anitemparameter on theInputattribute and updating the parsing logic accordingly.New Features:
#[Input(item: …)]to produce collections of Input objectsArrayObjectparameters (including subclasses) with#[Input(item: …)]to produce collections of Input objectsEnhancements:
createArrayOfInputshelper and extendresolveParameterto handle arrays andArrayObjectinheritanceInputattribute to accept an optionalitemclass parameter for collection typingDocumentation:
ArrayObjectsupport documentation and usage examplesTests:
ArrayObjectinputs, empty/missing/invalid data cases, and customArrayObjectsubclassesChores:
ArrayObjectusageSummary by CodeRabbit
New Features
itemparameter in the input attribute.Documentation
Tests