-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Optimized the PHPDoc for hyperf/database.
#7511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
types 这个目录也是单测?都放到 tests 目录里,你这个目录下我看都是 database 的单测,那就放到 database/tests 目录下面 |
已調整~ |
|
@limingxinleo 再麻煩有空時 review 一下了 |
huangdijia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Collection是不是也要跟随调整?
|
|
||
| /** | ||
| * @template TValue | ||
| * @template TValue of object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of object or of Model?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be object, because this trait used by not only Model\Builder but Query\Builder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR aims to improve the type annotations in the Hyperf database component by adding comprehensive PHPDoc type hints and creating extensive type tests. The changes enhance static analysis capabilities and provide better IDE support for developers working with the database ORM.
- Improved PHPDoc type annotations throughout database component classes for better static analysis
- Added comprehensive type test files to verify type correctness and coverage
- Enhanced generic type support for Model relationships, Collections, and Builders
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scout/src/SearchableScope.php | Added PHPStan ignore comment for type issue |
| src/database/tests/types/ | Added comprehensive type test files for Query Builder, Model relations, Collections, and other components |
| src/database/src/ | Updated PHPDoc annotations with generic types and improved return type specifications |
| phpstan.types.neon.dist | Added PHPStan configuration for type testing |
| composer.json | Added new script for running type analysis |
| CHANGELOG-3.1.md | Documented the PHPDoc optimization |
| .travis/run.check.sh | Added type analysis to CI checks |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| * | ||
| * @param array|BaseCollection|Model $ids | ||
| * @return array | ||
| * @return array{attached: array, detached: array, updated: array} |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array shape documentation could be more specific. Consider using array<mixed, mixed> or defining the key-value types more precisely for the attached, detached, and updated arrays to provide better type safety.
| if (is_string($relation)) { | ||
| $relation = $this->getRelationWithoutConstraints($relation); | ||
| } |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This guard clause should be extracted into a helper method since it's duplicated logic that appears to convert string relation names to relation instances. Consider creating a resolveRelation method to reduce code duplication.
| * @param mixed $key | ||
| * @param TFindDefault $default | ||
| * @return static<TKey|TModel>|TFindDefault|TModel | ||
| * @return ($key is array ? static : TFindDefault|TModel) |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The conditional return type using ($key is array ? static : TFindDefault|TModel) is complex. Consider breaking this into separate @return annotations for different parameter types to improve readability and maintainability of the documentation.
| * Chunk the results of a query by comparing numeric IDs. | ||
| * | ||
| * @param int $count | ||
| * @param callable(Collection<int, object>, int): mixed $callback |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The callback parameter documentation uses generic object type. Since this is in the Query Builder context, consider using a more specific type like stdClass or documenting that the object represents a database row result.
| * @param callable(Collection<int, object>, int): mixed $callback | |
| * @param callable(Collection<int, \stdClass>, int): mixed $callback |
|
Collection 或許之後再調整比較好?這個 PR 先專注在 Database |
|
@huangdijia , 再麻煩有空時 review 一下,感謝~ |
|
辛苦了,上次我以为我合并了。。 |
hyperf/database.
hyperf/database.hyperf/database.
原先 database component 中,PHPDoc 所標示的型別不少有錯誤或是不完整。
這個 PR 針對其做了 PHPDoc 的修正,並且補上了測試,讓型別標示更正確及完整。