From 697d08f581b857a594b8041c95deec2c54b01ca6 Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Tue, 3 Jun 2025 20:08:07 +0200 Subject: [PATCH] PhpDoc: Replace non-standard dict type with array Summary: Make static code analysis more correct (which does not also mean less noisy) by replacing "dict" and "dictionary" types in PhpDoc with what they actually are: an array. The "dictionary" type is not mentioned in `arcanist/src/parser/PhutilTypeSpec.php` either, thus no side effects. See also related discussions in https://we.phorge.it/D26037 and https://we.phorge.it/D26039#27821 Test Plan: Grep and read the code, use static code analysis. Reviewers: O1 Blessed Committers, valerio.bozzolan, amybones Reviewed By: O1 Blessed Committers, valerio.bozzolan, amybones Subscribers: amybones, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D26059 --- src/aphront/AphrontRequest.php | 10 +++++----- .../AphrontApplicationConfiguration.php | 9 +++++---- src/aphront/site/AphrontRoutingMap.php | 9 ++++++++- src/aphront/site/AphrontRoutingResult.php | 12 +++++++++++ .../PhabricatorConduitAPIController.php | 2 +- .../parser/DifferentialHunkParser.php | 3 ++- .../feed/story/PhabricatorFeedStory.php | 4 ++-- .../files/query/PhabricatorFileQuery.php | 4 ++-- .../files/storage/PhabricatorFile.php | 2 +- .../HarbormasterBuildStepImplementation.php | 2 +- .../PhabricatorMetaMTAEmailBodyParser.php | 4 ++-- .../phid/type/PhabricatorPHIDType.php | 6 +++--- ...habricatorApplicationTransactionEditor.php | 10 +++++----- .../cache/PhutilKeyValueCache.php | 4 ++-- .../cache/PhutilMemcacheKeyValueCache.php | 2 +- .../daemon/PhutilDaemonHandle.php | 2 +- .../markup/markuprule/PhutilRemarkupRule.php | 8 ++++---- src/infrastructure/storage/lisk/LiskDAO.php | 20 +++++++++---------- 18 files changed, 67 insertions(+), 46 deletions(-) diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index 554cd449aa..9ac3d02033 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -714,7 +714,7 @@ final class AphrontRequest extends Phobject { * This is primarily useful if you want to ask the user for more input and * then resubmit their request. * - * @return dict Original request parameters. + * @return array Original request parameters. */ public function getPassthroughRequestParameters($include_quicksand = false) { return self::flattenData( @@ -724,7 +724,7 @@ final class AphrontRequest extends Phobject { /** * Get request data other than "magic" parameters. * - * @return dict Request data, with magic filtered out. + * @return array Request data, with magic filtered out. */ public function getPassthroughRequestData($include_quicksand = false) { $data = $this->getRequestData(); @@ -748,9 +748,9 @@ final class AphrontRequest extends Phobject { * into a list of key-value pairs suitable for submitting via HTTP request * (with arrays flattened). * - * @param dict $data Data to flatten. - * @return dict Flat data suitable for inclusion in an HTTP - * request. + * @param array $data Data to flatten. + * @return array Flat data suitable for inclusion in an HTTP + * request. */ public static function flattenData(array $data) { $result = array(); diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php index 12917c34f4..17f057258e 100644 --- a/src/aphront/configuration/AphrontApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontApplicationConfiguration.php @@ -387,8 +387,8 @@ final class AphrontApplicationConfiguration /** * Build a controller to respond to the request. * - * @return pair Controller and dictionary of request - * parameters. + * @return pair Controller and dictionary of request + * parameters. * @task routing */ private function buildController() { @@ -512,8 +512,9 @@ final class AphrontApplicationConfiguration * * @param list $maps List of routing maps. * @param string $path Path to route. - * @return pair|null Controller and dictionary of - * request parameters, or null if no paths to route were found. + * @return array>|null Controller + * subclass and dictionary of request parameters, or null if no paths to + * route were found. * @task routing */ private function routePath(array $maps, $path) { diff --git a/src/aphront/site/AphrontRoutingMap.php b/src/aphront/site/AphrontRoutingMap.php index 687aae93bb..52a921c57d 100644 --- a/src/aphront/site/AphrontRoutingMap.php +++ b/src/aphront/site/AphrontRoutingMap.php @@ -21,6 +21,9 @@ final class AphrontRoutingMap extends Phobject { return $this; } + /** + * @return AphrontSite + */ public function getSite() { return $this->site; } @@ -30,6 +33,9 @@ final class AphrontRoutingMap extends Phobject { return $this; } + /** + * @return PhabricatorApplication + */ public function getApplication() { return $this->application; } @@ -87,7 +93,8 @@ final class AphrontRoutingMap extends Phobject { * @param string $route Pattern from the map. * @param string $value Value from the map. * @param string $path Path to route. - * @return dict|null Match details, if path matches sub-map. + * @return array|string>|null Match details, if path + * matches sub-map. * @task routing */ private function tryRoute($route, $value, $path) { diff --git a/src/aphront/site/AphrontRoutingResult.php b/src/aphront/site/AphrontRoutingResult.php index 9865e819af..1e5b04ed35 100644 --- a/src/aphront/site/AphrontRoutingResult.php +++ b/src/aphront/site/AphrontRoutingResult.php @@ -21,6 +21,9 @@ final class AphrontRoutingResult extends Phobject { return $this; } + /** + * @return AphrontSite + */ public function getSite() { return $this->site; } @@ -30,6 +33,9 @@ final class AphrontRoutingResult extends Phobject { return $this; } + /** + * @return PhabricatorApplication + */ public function getApplication() { return $this->application; } @@ -39,6 +45,9 @@ final class AphrontRoutingResult extends Phobject { return $this; } + /** + * @return AphrontController + */ public function getController() { return $this->controller; } @@ -48,6 +57,9 @@ final class AphrontRoutingResult extends Phobject { return $this; } + /** + * @return array + */ public function getURIData() { return $this->uriData; } diff --git a/src/applications/conduit/controller/PhabricatorConduitAPIController.php b/src/applications/conduit/controller/PhabricatorConduitAPIController.php index c220d0ead4..e229d9ff4b 100644 --- a/src/applications/conduit/controller/PhabricatorConduitAPIController.php +++ b/src/applications/conduit/controller/PhabricatorConduitAPIController.php @@ -160,7 +160,7 @@ final class PhabricatorConduitAPIController * Authenticate the client making the request to a Phabricator user account. * * @param ConduitAPIRequest $api_request Request being executed. - * @param dict $metadata Request metadata. + * @param array $metadata Dictionary of request metadata. * @param wild $method * @return null|pair Null to indicate successful authentication, or * an error code and error message pair. diff --git a/src/applications/differential/parser/DifferentialHunkParser.php b/src/applications/differential/parser/DifferentialHunkParser.php index 2902022184..78c6d62094 100644 --- a/src/applications/differential/parser/DifferentialHunkParser.php +++ b/src/applications/differential/parser/DifferentialHunkParser.php @@ -14,7 +14,8 @@ final class DifferentialHunkParser extends Phobject { * datastructure is used to determine when to render "Context not available." * in diffs with multiple hunks. * - * @return dict Map of lines where hunks start, other than line 1. + * @return array Map of lines where hunks start, other than + * line 1. */ public function getHunkStartLines(array $hunks) { assert_instances_of($hunks, 'DifferentialHunk'); diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index 9ef05bddea..5d61b9ab05 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -32,8 +32,8 @@ abstract class PhabricatorFeedStory * construct appropriate @{class:PhabricatorFeedStory} wrappers for each * data row. * - * @param list $rows List of @{class:PhabricatorFeedStoryData} rows - * from the database. + * @param array> $rows List of + * @{class:PhabricatorFeedStoryData} rows from the database. * @param PhabricatorUser $viewer * @return list List of @{class:PhabricatorFeedStory} * objects. diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php index f0dee1175b..fffc375e4b 100644 --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -96,8 +96,8 @@ final class PhabricatorFileQuery * `PHID-FILE-aaaa` and all transformations of the file with PHID * `PHID-FILE-bbbb`. * - * @param list $specs List of transform specifications, described - * above. + * @param array> $specs List of transform + * specifications, described above. * @return $this */ public function withTransforms(array $specs) { diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 8d9616a21f..70f5cb688e 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -1215,7 +1215,7 @@ final class PhabricatorFile extends PhabricatorFileDAO * @param PhabricatorUser $user Viewing user. * @param list $builtins List of builtin file * specs. - * @return dict Dictionary of named builtins. + * @return array Dictionary of named builtins. */ public static function loadBuiltins(PhabricatorUser $user, array $builtins) { $builtins = mpull($builtins, null, 'getBuiltinFileKey'); diff --git a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php index 851a020687..cd3d4ee66f 100644 --- a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php @@ -191,7 +191,7 @@ abstract class HarbormasterBuildStepImplementation extends Phobject { * @{function:vcsprintf}. * @param string $pattern User-provided pattern string containing * `${variables}`. - * @param dict $variables List of available replacement variables. + * @param array $variables List of available replacement variables. * @return string String with variables replaced safely into it. */ protected function mergeVariables($function, $pattern, array $variables) { diff --git a/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php b/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php index 45b7735ce0..322e22ca3a 100644 --- a/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php +++ b/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php @@ -26,8 +26,8 @@ final class PhabricatorMetaMTAEmailBodyParser extends Phobject { * ), * ) * - * @param string $body Raw mail text body. - * @return dict Parsed body. + * @param string $body Raw mail text body. + * @return array Parsed body. */ public function parseBody($body) { $body = $this->stripTextBody($body); diff --git a/src/applications/phid/type/PhabricatorPHIDType.php b/src/applications/phid/type/PhabricatorPHIDType.php index 37216340e7..a10e6167eb 100644 --- a/src/applications/phid/type/PhabricatorPHIDType.php +++ b/src/applications/phid/type/PhabricatorPHIDType.php @@ -147,7 +147,7 @@ abstract class PhabricatorPHIDType extends Phobject { * To get PHID types a given user has access to, see * @{method:getAllInstalledTypes}. * - * @return dict Map of type constants to types. + * @return array Map of type constants to types. */ final public static function getAllTypes() { return self::newClassMapQuery() @@ -172,7 +172,7 @@ abstract class PhabricatorPHIDType extends Phobject { * Get all PHID types of applications installed for a given viewer. * * @param PhabricatorUser $viewer Viewing user. - * @return dict Map of constants to installed + * @return array Map of constants to installed * types. */ public static function getAllInstalledTypes(PhabricatorUser $viewer) { @@ -216,7 +216,7 @@ abstract class PhabricatorPHIDType extends Phobject { * Get all PHID types of an application. * * @param string $application Class name of an application - * @return dict Map of constants of application + * @return array Map of constants of application */ public static function getAllTypesForApplication( string $application) { diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 861815b9d8..3b29ad499f 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -4548,7 +4548,7 @@ abstract class PhabricatorApplicationTransactionEditor * * This data will be loaded with @{method:loadWorkerState} in the worker. * - * @return dict Serializable editor state. + * @return array Serializable editor state. * @task workers */ private function getWorkerState() { @@ -4573,7 +4573,7 @@ abstract class PhabricatorApplicationTransactionEditor /** * Hook; return custom properties which need to be passed to workers. * - * @return dict Custom properties. + * @return array Custom properties. * @task workers */ protected function getCustomWorkerState() { @@ -4588,7 +4588,7 @@ abstract class PhabricatorApplicationTransactionEditor * This primarily allows binary data to be passed to workers and survive * JSON encoding. * - * @return dict Property encodings. + * @return array Property encodings. * @task workers */ protected function getCustomWorkerStateEncoding() { @@ -4601,7 +4601,7 @@ abstract class PhabricatorApplicationTransactionEditor * * This method is used to load state when running worker operations. * - * @param dict $state Editor state, from + * @param array $state Editor state, from @{method:getWorkerState}. * @return $this * @task workers @@ -4628,7 +4628,7 @@ abstract class PhabricatorApplicationTransactionEditor * Hook; set custom properties on the editor from data emitted by * @{method:getCustomWorkerState}. * - * @param dict $state Custom state, + * @param array $state Custom state, * from @{method:getCustomWorkerState}. * @return $this * @task workers diff --git a/src/infrastructure/cache/PhutilKeyValueCache.php b/src/infrastructure/cache/PhutilKeyValueCache.php index 4e7ffb3d0b..80a76b22a3 100644 --- a/src/infrastructure/cache/PhutilKeyValueCache.php +++ b/src/infrastructure/cache/PhutilKeyValueCache.php @@ -77,7 +77,7 @@ abstract class PhutilKeyValueCache extends Phobject { * Get data from the cache. * * @param list $keys List of cache keys to retrieve. - * @return dict Dictionary of keys that were found in the + * @return array Dictionary of keys that were found in the * cache. Keys not present in the cache are * omitted, so you can detect a cache miss. * @task kvimpl @@ -92,7 +92,7 @@ abstract class PhutilKeyValueCache extends Phobject { * after a specified number of seconds. By default, there is no expiration * policy and data will persist in cache indefinitely. * - * @param dict $keys Map of cache keys to values. + * @param array $keys Map of cache keys to values. * @param int|null $ttl (optional) TTL for cache keys, in seconds. * @return $this * @task kvimpl diff --git a/src/infrastructure/cache/PhutilMemcacheKeyValueCache.php b/src/infrastructure/cache/PhutilMemcacheKeyValueCache.php index b8fdd4b3dc..74fab5e581 100644 --- a/src/infrastructure/cache/PhutilMemcacheKeyValueCache.php +++ b/src/infrastructure/cache/PhutilMemcacheKeyValueCache.php @@ -100,7 +100,7 @@ final class PhutilMemcacheKeyValueCache extends PhutilKeyValueCache { * ), * )); * - * @param list $servers List of server specifications. + * @param array $servers List of server specifications. * @return $this * @task memcache */ diff --git a/src/infrastructure/daemon/PhutilDaemonHandle.php b/src/infrastructure/daemon/PhutilDaemonHandle.php index 36b0ea9935..4f57bafc0d 100644 --- a/src/infrastructure/daemon/PhutilDaemonHandle.php +++ b/src/infrastructure/daemon/PhutilDaemonHandle.php @@ -329,7 +329,7 @@ final class PhutilDaemonHandle extends Phobject { * Dispatch an event to event listeners. * * @param string $type Event type. - * @param dict $params (optional) Event parameters. + * @param array $params (optional) Event parameters. * @return void */ private function dispatchEvent($type, array $params = array()) { diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php index 09f6ae9a5c..782033ffcd 100644 --- a/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php +++ b/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php @@ -60,10 +60,10 @@ abstract class PhutilRemarkupRule extends Phobject { * This method acts as @{function:phutil_tag}, but checks attributes before * using them. * - * @param string $name Tag name. - * @param dict $attrs Tag attributes. - * @param wild $content (optional) Tag content. - * @return PhutilSafeHTML Tag object. + * @param string $name Tag name. + * @param array $attrs Dictionary of tag attributes. + * @param wild $content (optional) Tag content. + * @return PhutilSafeHTML Tag object. */ protected function newTag($name, array $attrs, $content = null) { foreach ($attrs as $key => $attr) { diff --git a/src/infrastructure/storage/lisk/LiskDAO.php b/src/infrastructure/storage/lisk/LiskDAO.php index c696f1213d..f97e3b4e50 100644 --- a/src/infrastructure/storage/lisk/LiskDAO.php +++ b/src/infrastructure/storage/lisk/LiskDAO.php @@ -450,8 +450,8 @@ abstract class LiskDAO extends Phobject /** * Loads all of the objects, unconditionally. * - * @return dict Dictionary of all persisted objects of this type, keyed - * on object ID. + * @return array Dictionary of all persisted objects of this + * type, keyed on object ID. * * @task load */ @@ -470,7 +470,7 @@ abstract class LiskDAO extends Phobject * * @param string $pattern queryfx()-style SQL WHERE clause. * @param mixed $args,... Zero or more conversions. - * @return dict Dictionary of matching objects, keyed on ID. + * @return array Dictionary of matching objects, keyed on ID. * * @task load */ @@ -574,9 +574,9 @@ abstract class LiskDAO extends Phobject * convenient to pull data from elsewhere directly (e.g., a complicated * join via @{method:queryData}) and then load from an array representation. * - * @param dict $row Dictionary of properties, which should be equivalent - * to selecting a row from the table or calling - * @{method:getProperties}. + * @param array $row Dictionary of properties, which + * should be equivalent to selecting a row from the table or calling + * @{method:getProperties}. * @return $this * * @task load @@ -650,7 +650,7 @@ abstract class LiskDAO extends Phobject * This is a lot messier than @{method:loadAllWhere}, but more flexible. * * @param list $rows List of property dictionaries. - * @return dict List of constructed objects, keyed on ID. + * @return array List of constructed objects, keyed on ID. * * @task load */ @@ -739,8 +739,8 @@ abstract class LiskDAO extends Phobject * database. * Properties that should not be persisted must be declared as private. * - * @return dict Dictionary of normalized (lowercase) to canonical (original - * case) property names. + * @return array Dictionary of normalized (lowercase) to + * canonical (original case) property names. * * @task info */ @@ -874,7 +874,7 @@ abstract class LiskDAO extends Phobject * using legacy features with CONFIG_CONVERT_CAMELCASE, but in that case you * should just go ahead and die in a fire). * - * @return dict Dictionary of object properties. + * @return array Dictionary of object properties. * * @task info */