-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Docs: Clarify return value semantics of wpdb query methods
#11855
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
Open
apermo
wants to merge
22
commits into
WordPress:trunk
Choose a base branch
from
apermo:trac-65261-wpdb-get-results-phpdoc
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+83
−14
Open
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
ae78820
Docs: Clarify return value semantics of `wpdb::get_results()`.
apermo cdd144a
Docs: Add `@phpstan-return` and tighten types for `wpdb::get_results()`.
apermo 31989de
Docs: Add `@phpstan-return` for `wpdb::get_row()`.
apermo 72a1952
Docs: Add `@phpstan-return` for `wpdb::get_col()`.
apermo f89caa9
Docs: Clarify null return semantics of `wpdb::get_var()`.
apermo 522405a
Docs: Narrow `@phpstan-param` for `$output` on `wpdb::get_row()` and …
apermo ba0a402
Docs: Refine return shapes in `@phpstan-return` for `wpdb::get_result…
apermo fad4875
Docs: Hoist `wpdb::get_var()` empty-string explanation out of `@return`.
apermo 9655035
Docs: Drop blank line between `@return` and `@phpstan-` tags.
apermo a21d9f4
Merge branch 'trunk' into trac-65261-wpdb-get-results-phpdoc
westonruter 45ca744
Improve see reference for last_error
westonruter f3cae5c
Suggest ext-mysqli in composer.json
westonruter 326418d
Docs: Assert row array key types in `wpdb::get_row()`, `get_col()`, a…
westonruter 4d8e28b
Eliminate needless setting of key on array to preserve list semantics
westonruter 8883af6
Docs: Assert the column value type in `wpdb::get_var()`.
westonruter 70cbaf7
Use more specific non-empty-string as in return type for get_col() an…
westonruter d0ce171
Docs: Assert the result key type in `wpdb::get_results()` for OBJECT_K.
westonruter f102823
Docs: Allow null in `wpdb::get_results()` OBJECT return type.
westonruter 3639132
Docs: Allow integer keys in `wpdb::get_row()` and `get_results()` ARR…
westonruter 015feb1
Fix `wpdb::get_results()` OBJECT_K null key deprecation for SQL NULL …
westonruter e811575
Merge inline comment with phpdoc
westonruter 1e55aa9
Merge inline comment with phpdoc in get_var
westonruter File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3019,12 +3019,16 @@ protected function process_field_lengths( $data, $table ) { | |||||
| * the value in the column and row specified is returned. If $query is null, | ||||||
| * the value in the specified column and row from the previous SQL result is returned. | ||||||
| * | ||||||
| * Returns null both on failure and when the matched cell value is an empty | ||||||
| * string. To distinguish the two cases, check {@see self::$last_error}. | ||||||
| * | ||||||
| * @since 0.71 | ||||||
| * | ||||||
| * @param string|null $query Optional. SQL query. Defaults to null, use the result from the previous query. | ||||||
| * @param int $x Optional. Column of value to return. Indexed from 0. Default 0. | ||||||
| * @param int $y Optional. Row of value to return. Indexed from 0. Default 0. | ||||||
| * @return string|null Database query result (as string), or null on failure. | ||||||
| * @return string|null Database query result (as string), or null on failure or when the value is an empty string. | ||||||
| * @phpstan-return non-empty-string|null | ||||||
| */ | ||||||
| public function get_var( $query = null, $x = 0, $y = 0 ) { | ||||||
| $this->func_call = "\$db->get_var(\"$query\", $x, $y)"; | ||||||
|
|
@@ -3039,6 +3043,14 @@ public function get_var( $query = null, $x = 0, $y = 0 ) { | |||||
|
|
||||||
| // Extract var out of cached results based on x,y vals. | ||||||
| if ( ! empty( $this->last_result[ $y ] ) ) { | ||||||
| /** | ||||||
| * Column values. | ||||||
| * | ||||||
| * These are returned from the database as strings, or null for SQL NULL, but get_object_vars() types the | ||||||
| * property values as mixed. | ||||||
| * | ||||||
| * @var list<string|null> $values | ||||||
| */ | ||||||
| $values = array_values( get_object_vars( $this->last_result[ $y ] ) ); | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -3059,6 +3071,24 @@ public function get_var( $query = null, $x = 0, $y = 0 ) { | |||||
| * respectively. Default OBJECT. | ||||||
| * @param int $y Optional. Row to return. Indexed from 0. Default 0. | ||||||
| * @return array|object|null Database query result in format specified by $output or null on failure. | ||||||
| * @phpstan-param 'OBJECT'|'ARRAY_A'|'ARRAY_N' $output | ||||||
| * @phpstan-return ( | ||||||
| * $query is non-falsy-string | ||||||
| * ? ( | ||||||
| * $output is 'OBJECT' | ||||||
| * ? stdClass|null | ||||||
| * : ( | ||||||
| * $output is 'ARRAY_A' | ||||||
| * ? array<array-key, mixed>|null | ||||||
| * : ( | ||||||
| * $output is 'ARRAY_N' | ||||||
| * ? list<mixed>|null | ||||||
| * : null | ||||||
| * ) | ||||||
| * ) | ||||||
| * ) | ||||||
| * : null | ||||||
| * ) | ||||||
| */ | ||||||
| public function get_row( $query = null, $output = OBJECT, $y = 0 ) { | ||||||
| $this->func_call = "\$db->get_row(\"$query\",$output,$y)"; | ||||||
|
|
@@ -3104,6 +3134,7 @@ public function get_row( $query = null, $output = OBJECT, $y = 0 ) { | |||||
| * @param string|null $query Optional. SQL query. Defaults to previous query. | ||||||
| * @param int $x Optional. Column to return. Indexed from 0. Default 0. | ||||||
| * @return array Database query result. Array indexed from 0 by SQL result row number. | ||||||
| * @phpstan-return list<non-empty-string|null> | ||||||
| */ | ||||||
| public function get_col( $query = null, $x = 0 ) { | ||||||
| if ( $query ) { | ||||||
|
|
@@ -3118,7 +3149,7 @@ public function get_col( $query = null, $x = 0 ) { | |||||
| // Extract the column values. | ||||||
| if ( $this->last_result ) { | ||||||
| for ( $i = 0, $j = count( $this->last_result ); $i < $j; $i++ ) { | ||||||
| $new_array[ $i ] = $this->get_var( null, $x, $i ); | ||||||
| $new_array[] = $this->get_var( null, $x, $i ); | ||||||
| } | ||||||
| } | ||||||
| return $new_array; | ||||||
|
|
@@ -3129,18 +3160,47 @@ public function get_col( $query = null, $x = 0 ) { | |||||
| * | ||||||
| * Executes a SQL query and returns the entire SQL result. | ||||||
| * | ||||||
| * Returns an empty array when no rows match or when the database | ||||||
| * reports an error for the query. Returns null when $query is empty, | ||||||
| * when $output is not one of the recognized constants, or when the | ||||||
| * query cannot run because the connection is not ready. | ||||||
| * | ||||||
| * @since 0.71 | ||||||
| * | ||||||
| * @param string $query SQL query. | ||||||
| * @param string $output Optional. Any of ARRAY_A | ARRAY_N | OBJECT | OBJECT_K constants. | ||||||
| * With one of the first three, return an array of rows indexed | ||||||
| * from 0 by SQL result row number. Each row is an associative array | ||||||
| * (column => value, ...), a numerically indexed array (0 => value, ...), | ||||||
| * or an object ( ->column = value ), respectively. With OBJECT_K, | ||||||
| * return an associative array of row objects keyed by the value | ||||||
| * of each row's first column's value. Duplicate keys are discarded. | ||||||
| * Default OBJECT. | ||||||
| * @return array|object|null Database query results. | ||||||
| * @param string|null $query SQL query. | ||||||
| * @param string $output Optional. Any of ARRAY_A | ARRAY_N | OBJECT | OBJECT_K constants. | ||||||
| * With one of the first three, return an array of rows indexed | ||||||
| * from 0 by SQL result row number. Each row is an associative array | ||||||
| * (column => value, ...), a numerically indexed array (0 => value, ...), | ||||||
| * or an object ( ->column = value ), respectively. With OBJECT_K, | ||||||
| * return an associative array of row objects keyed by the value | ||||||
| * of each row's first column's value. Duplicate keys are discarded. | ||||||
| * Default OBJECT. | ||||||
| * @return array|null Database query results. Empty array when no rows match | ||||||
| * or on database error. Null when $query is empty, when | ||||||
| * $output is invalid, or when the connection is not ready. | ||||||
| * @phpstan-param 'OBJECT'|'OBJECT_K'|'ARRAY_A'|'ARRAY_N' $output | ||||||
| * @phpstan-return ( | ||||||
| * $query is non-falsy-string | ||||||
| * ? ( | ||||||
| * $output is 'OBJECT' | ||||||
| * ? list<stdClass>|null | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally this case would be rather as follows to match the other
Suggested change
However, that would introduce a behavior change which should be discussed. Whenever |
||||||
| * : ( | ||||||
| * $output is 'OBJECT_K' | ||||||
| * ? array<array-key, stdClass> | ||||||
| * : ( | ||||||
| * $output is 'ARRAY_A' | ||||||
| * ? list<array<array-key, mixed>> | ||||||
| * : ( | ||||||
| * $output is 'ARRAY_N' | ||||||
| * ? list<list<mixed>> | ||||||
| * : null | ||||||
| * ) | ||||||
| * ) | ||||||
| * ) | ||||||
| * ) | ||||||
| * : null | ||||||
| * ) | ||||||
| */ | ||||||
| public function get_results( $query = null, $output = OBJECT ) { | ||||||
| $this->func_call = "\$db->get_results(\"$query\", $output)"; | ||||||
|
|
@@ -3167,7 +3227,15 @@ public function get_results( $query = null, $output = OBJECT ) { | |||||
| if ( $this->last_result ) { | ||||||
| foreach ( $this->last_result as $row ) { | ||||||
| $var_by_ref = get_object_vars( $row ); | ||||||
| $key = array_shift( $var_by_ref ); | ||||||
| /** | ||||||
| * The first column's value is used as the key. | ||||||
| * | ||||||
| * A SQL NULL value surfaces as null here, so coerce it to an empty string to avoid the deprecated | ||||||
| * use of null as an array offset (PHP 8.5+). | ||||||
| * | ||||||
| * @var array-key $key | ||||||
| */ | ||||||
| $key = array_shift( $var_by_ref ) ?? ''; | ||||||
| if ( ! isset( $new_array[ $key ] ) ) { | ||||||
| $new_array[ $key ] = $row; | ||||||
| } | ||||||
|
|
||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This change ensures that PHPStan sees
$new_arrayas alist. The result is an equivalent 0-indexed array.