From 425ab7a846803b721603eea3f80847c4bf3da460 Mon Sep 17 00:00:00 2001 From: amy bones Date: Fri, 30 May 2025 13:02:36 -0700 Subject: [PATCH] Add phpDoc to PhabricatorCursorPagedPolicyAwareQuery Summary: Adding class templates to `PhabricatorPolicyAwareQuery` and `PhabricatorCursorPagedPolicyAwareQuery` to make `execute()` have the right return signature, when the query classes are also annotated. Otherwise, the return type is inferred as `PhabricatorPolicyInterface` as before. Test Plan: Add some annotations to query classes and see that `$query->execute()` infers the more specific return type. Diviner still works after ./bin/diviner generate. For example this page still exist: /book/dev/class/PhabricatorCursorPagedPolicyAwareQuery/ And the method newResultObject() is now documented with the return type R|null. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D26037 --- ...PhabricatorCursorPagedPolicyAwareQuery.php | 6 ++++ .../policy/PhabricatorPolicyAwareQuery.php | 28 ++++++++++--------- src/view/control/AphrontCursorPagerView.php | 5 ++++ 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 5b7d4f9742..8d4244ba06 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -12,6 +12,9 @@ * @task order Result Ordering * @task edgelogic Working with Edge Logic * @task spaces Working with Spaces + * + * @template R of PhabricatorPolicyInterface + * @extends PhabricatorPolicyAwareQuery */ abstract class PhabricatorCursorPagedPolicyAwareQuery extends PhabricatorPolicyAwareQuery { @@ -439,6 +442,9 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery return null; } + /** + * @return R|null + */ public function newResultObject() { return null; } diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php index 3daaef4a4b..a0c24cdc07 100644 --- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php @@ -25,6 +25,8 @@ * @task config Query Configuration * @task exec Executing Queries * @task policyimpl Policy Query Implementation + * + * @template R of PhabricatorPolicyInterface */ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { @@ -170,7 +172,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * example, the user is trying to view or edit an object which exists but * which they do not have permission to see) a policy exception is thrown. * - * @return mixed Single result, or null. + * @return R|null Single result, or null. * @task exec */ final public function executeOne() { @@ -198,7 +200,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { /** * Execute the query, loading all visible results. * - * @return list Result objects. + * @return R[] Result objects. * @task exec */ final public function execute() { @@ -473,7 +475,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * automatically populated as a side effect of objects surviving policy * filtering. * - * @param map $objects Objects to add to + * @param array $objects Objects to add to * the query workspace. * @return $this * @task workspace @@ -552,7 +554,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * * PHIDs which are "in flight" are actively being queried for. * - * @return map PHIDs currently in flight. + * @return array PHIDs currently in flight. */ public function getPHIDsInFlight() { $results = $this->inFlightPHIDs; @@ -595,7 +597,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * from the database. They should attempt to return the number of results * hinted by @{method:getRawResultLimit}. * - * @return list List of filterable policy objects. + * @return R[] List of filterable policy objects. * @task policyimpl */ abstract protected function loadPage(); @@ -606,7 +608,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * return new results. Generally, you should adjust a cursor position based * on the provided result page. * - * @param list $page The current page of results. + * @param R[] $page The current page of results. * @return void * @task policyimpl */ @@ -627,8 +629,8 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * This method will only be called if data is available. Implementations * do not need to handle the case of no results specially. * - * @param list $page Results from `loadPage()`. - * @return list Objects for policy filtering. + * @param R[] $page Results from `loadPage()`. + * @return R[] Objects for policy filtering. * @task policyimpl */ protected function willFilterPage(array $page) { @@ -650,8 +652,8 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * This method will only be called if data is available. Implementations do * not need to handle the case of no results specially. * - * @param list $page Results from @{method:willFilterPage()}. - * @return list Objects after additional + * @param R[] $page Results from @{method:willFilterPage()}. + * @return R[] Objects after additional * non-policy processing. */ protected function didFilterPage(array $page) { @@ -665,7 +667,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * filtered for policy reasons. The query should remove them from any cached * or partial result sets. * - * @param list $results List of objects that should not be returned by + * @param R[] $results List of objects that should not be returned by * alternate result mechanisms. * @return void * @task policyimpl @@ -680,8 +682,8 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { * used by @{class:PhabricatorCursorPagedPolicyAwareQuery} to reverse results * that are queried during reverse paging. * - * @param list $results Query results. - * @return list Final results. + * @param R[] $results Query results. + * @return R[] Final results. * @task policyimpl */ protected function didLoadResults(array $results) { diff --git a/src/view/control/AphrontCursorPagerView.php b/src/view/control/AphrontCursorPagerView.php index cf4563fb9e..f82c0e702a 100644 --- a/src/view/control/AphrontCursorPagerView.php +++ b/src/view/control/AphrontCursorPagerView.php @@ -70,6 +70,11 @@ final class AphrontCursorPagerView extends AphrontView { return $this->prevPageID; } + /** + * @param T[] $results + * @return T[] + * @template T + */ public function sliceResults(array $results) { if (count($results) > $this->getPageSize()) { $page_size = (int)$this->getPageSize(); -- 2.51.2