Skip to content

Commit a13c8a2

Browse files
westonruterwestonruter
committed
Filesystem API: Improve type safety across the transport classes.
Change the optional constructor argument of `WP_Filesystem_FTPext`, `WP_Filesystem_ftpsockets`, and `WP_Filesystem_SSH2` from an empty string default to an empty `array`, matching how the argument is actually consumed, and improve the associated DocBlocks. These classes were also brought to adherence with PHPStan rule level 10: * Add `FileListing` and `Options` array shapes, and initialize each transport's `$options` to a complete default array before any early return. * Correct several inaccurate `@return` descriptions, including the `group()` methods that had been describing the owner. * Allow `WP_Filesystem_SSH2::connect()` to be retried after a failed connection attempt. * Stop `WP_Filesystem_FTPext::parselisting()` from leaking its intermediate date-parsing keys into the returned listing. * Add `ext-ftp` and `ext-ssh2` to the suggested extensions in `composer.json`. Developed in WordPress#11593. Follow-up to r62635, r62636. Props soean, westonruter, mukesh27. See #65584, #64898. Fixes #65409. git-svn-id: https://develop.svn.wordpress.org/trunk@62637 602fd350-edb4-49c9-b593-d223f7449a82
1 parent b956786 commit a13c8a2

6 files changed

Lines changed: 363 additions & 126 deletions

‎composer.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
},
1818
"suggest": {
1919
"ext-dom": "*",
20-
"ext-mysqli": "*"
20+
"ext-ftp": "*",
21+
"ext-mysqli": "*",
22+
"ext-ssh2": "*"
2123
},
2224
"require-dev": {
2325
"composer/ca-bundle": "1.5.12",

‎src/wp-admin/includes/class-wp-filesystem-base.php‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,23 @@
1010
* Base WordPress Filesystem class which Filesystem implementations extend.
1111
*
1212
* @since 2.5.0
13+
*
14+
* @phpstan-type FileListing array{
15+
* name: string,
16+
* perms?: string,
17+
* permsn?: string,
18+
* number?: int|string|false,
19+
* owner?: string|int<1, max>|false,
20+
* group?: string|int<1, max>|false,
21+
* size: int|string|false,
22+
* lastmodunix?: int|string|false,
23+
* lastmod?: string|false,
24+
* time: int|string|false,
25+
* type: 'd'|'f'|'l',
26+
* islink?: bool,
27+
* isdir?: bool,
28+
* files?: mixed[]|false, // The mixed[] is actually FileListing[] but PHPStan does not support recursive or self-referencing array shapes.
29+
* }
1330
*/
1431
#[AllowDynamicProperties]
1532
class WP_Filesystem_Base {
@@ -26,7 +43,7 @@ class WP_Filesystem_Base {
2643
* Cached list of local filepaths to mapped remote filepaths.
2744
*
2845
* @since 2.7.0
29-
* @var array
46+
* @var array<string, string>
3047
*/
3148
public $cache = array();
3249

@@ -44,6 +61,7 @@ class WP_Filesystem_Base {
4461
public $errors = null;
4562

4663
/**
64+
* @var array<string, mixed>
4765
*/
4866
public $options = array();
4967

@@ -52,7 +70,7 @@ class WP_Filesystem_Base {
5270
*
5371
* @since 2.7.0
5472
*
55-
* @return string The location of the remote path.
73+
* @return string|false The location of the remote path, or false on failure.
5674
*/
5775
public function abspath() {
5876
$folder = $this->find_folder( ABSPATH );
@@ -73,7 +91,7 @@ public function abspath() {
7391
*
7492
* @since 2.7.0
7593
*
76-
* @return string The location of the remote path.
94+
* @return string|false The location of the remote path, or false on failure.
7795
*/
7896
public function wp_content_dir() {
7997
return $this->find_folder( WP_CONTENT_DIR );
@@ -84,7 +102,7 @@ public function wp_content_dir() {
84102
*
85103
* @since 2.7.0
86104
*
87-
* @return string The location of the remote path.
105+
* @return string|false The location of the remote path, or false on failure.
88106
*/
89107
public function wp_plugins_dir() {
90108
return $this->find_folder( WP_PLUGIN_DIR );
@@ -97,10 +115,10 @@ public function wp_plugins_dir() {
97115
*
98116
* @param string|false $theme Optional. The theme stylesheet or template for the directory.
99117
* Default false.
100-
* @return string The location of the remote path.
118+
* @return string|false The location of the remote path, or false on failure.
101119
*/
102120
public function wp_themes_dir( $theme = false ) {
103-
$theme_root = get_theme_root( $theme );
121+
$theme_root = get_theme_root( is_string( $theme ) ? $theme : '' );
104122

105123
// Account for relative theme roots.
106124
if ( '/themes' === $theme_root || ! is_dir( $theme_root ) ) {
@@ -115,7 +133,7 @@ public function wp_themes_dir( $theme = false ) {
115133
*
116134
* @since 3.2.0
117135
*
118-
* @return string The location of the remote path.
136+
* @return string|false The location of the remote path, or false on failure.
119137
*/
120138
public function wp_lang_dir() {
121139
return $this->find_folder( WP_LANG_DIR );
@@ -134,7 +152,7 @@ public function wp_lang_dir() {
134152
*
135153
* @param string $base Optional. The folder to start searching from. Default '.'.
136154
* @param bool $verbose Optional. True to display debug information. Default false.
137-
* @return string The location of the remote path.
155+
* @return string|false The location of the remote path, or false on failure.
138156
*/
139157
public function find_base_dir( $base = '.', $verbose = false ) {
140158
_deprecated_function( __FUNCTION__, '2.7.0', 'WP_Filesystem_Base::abspath() or WP_Filesystem_Base::wp_*_dir()' );
@@ -155,7 +173,7 @@ public function find_base_dir( $base = '.', $verbose = false ) {
155173
*
156174
* @param string $base Optional. The folder to start searching from. Default '.'.
157175
* @param bool $verbose Optional. True to display debug information. Default false.
158-
* @return string The location of the remote path.
176+
* @return string|false The location of the remote path, or false on failure.
159177
*/
160178
public function get_base_dir( $base = '.', $verbose = false ) {
161179
_deprecated_function( __FUNCTION__, '2.7.0', 'WP_Filesystem_Base::abspath() or WP_Filesystem_Base::wp_*_dir()' );
@@ -194,7 +212,9 @@ public function find_folder( $folder ) {
194212
}
195213

196214
if ( $folder === $dir ) {
197-
return trailingslashit( constant( $constant ) );
215+
/** @var string $constant_value */
216+
$constant_value = constant( $constant );
217+
return trailingslashit( $constant_value );
198218
}
199219
}
200220

@@ -205,7 +225,9 @@ public function find_folder( $folder ) {
205225
}
206226

207227
if ( 0 === stripos( $folder, $dir ) ) { // $folder starts with $dir.
208-
$potential_folder = preg_replace( '#^' . preg_quote( $dir, '#' ) . '/#i', trailingslashit( constant( $constant ) ), $folder );
228+
/** @var string $constant_value */
229+
$constant_value = constant( $constant );
230+
$potential_folder = (string) preg_replace( '#^' . preg_quote( $dir, '#' ) . '/#i', trailingslashit( $constant_value ), $folder );
209231
$potential_folder = trailingslashit( $potential_folder );
210232

211233
if ( $this->is_dir( $potential_folder ) ) {
@@ -221,7 +243,7 @@ public function find_folder( $folder ) {
221243
return trailingslashit( $folder );
222244
}
223245

224-
$folder = preg_replace( '|^([a-z]{1}):|i', '', $folder ); // Strip out Windows drive letter if it's there.
246+
$folder = (string) preg_replace( '|^([a-z]{1}):|i', '', $folder ); // Strip out Windows drive letter if it's there.
225247
$folder = str_replace( '\\', '/', $folder ); // Windows path sanitization.
226248

227249
if ( isset( $this->cache[ $folder ] ) ) {
@@ -258,7 +280,8 @@ public function find_folder( $folder ) {
258280
*/
259281
public function search_for_folder( $folder, $base = '.', $loop = false ) {
260282
if ( empty( $base ) || '.' === $base ) {
261-
$base = trailingslashit( $this->cwd() );
283+
$cwd = $this->cwd();
284+
$base = is_string( $cwd ) ? trailingslashit( $cwd ) : '/';
262285
}
263286

264287
$folder = untrailingslashit( $folder );
@@ -420,7 +443,7 @@ public function getchmod( $file ) {
420443
public function getnumchmodfromh( $mode ) {
421444
$realmode = '';
422445
$legal = array( '', 'w', 'r', 'x', '-' );
423-
$attarray = preg_split( '//', $mode );
446+
$attarray = (array) preg_split( '//', $mode );
424447

425448
for ( $i = 0, $c = count( $attarray ); $i < $c; $i++ ) {
426449
$key = array_search( $attarray[ $i ], $legal, true );
@@ -440,9 +463,9 @@ public function getnumchmodfromh( $mode ) {
440463
$mode = strtr( $mode, $trans );
441464

442465
$newmode = $mode[0];
443-
$newmode .= $mode[1] + $mode[2] + $mode[3];
444-
$newmode .= $mode[4] + $mode[5] + $mode[6];
445-
$newmode .= $mode[7] + $mode[8] + $mode[9];
466+
$newmode .= (int) $mode[1] + (int) $mode[2] + (int) $mode[3];
467+
$newmode .= (int) $mode[4] + (int) $mode[5] + (int) $mode[6];
468+
$newmode .= (int) $mode[7] + (int) $mode[8] + (int) $mode[9];
446469

447470
return $newmode;
448471
}
@@ -508,7 +531,7 @@ public function get_contents( $file ) {
508531
* @abstract
509532
*
510533
* @param string $file Path to the file.
511-
* @return array|false File contents in an array on success, false on failure.
534+
* @return string[]|false File contents in an array on success, false on failure.
512535
*/
513536
public function get_contents_array( $file ) {
514537
return false;
@@ -851,12 +874,13 @@ public function rmdir( $path, $recursive = false ) {
851874
* False if not available.
852875
* @type string|false $lastmod Last modified month (3 letters) and day (without leading 0), or
853876
* false if not available.
854-
* @type string|false $time Last modified time, or false if not available.
877+
* @type int|string|false $time Last modified time. A Unix timestamp on FTP transports, or false if not available.
855878
* @type string $type Type of resource. 'f' for file, 'd' for directory, 'l' for link.
856879
* @type array|false $files If a directory and `$recursive` is true, contains another array of
857880
* files. False if unable to list directory contents.
858881
* }
859882
* }
883+
* @phpstan-return array<string, FileListing>|false
860884
*/
861885
public function dirlist( $path, $include_hidden = true, $recursive = false ) {
862886
return false;

‎src/wp-admin/includes/class-wp-filesystem-direct.php‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* @since 2.5.0
1313
*
1414
* @see WP_Filesystem_Base
15+
* @phpstan-import-type FileListing from WP_Filesystem_Base
1516
*/
1617
class WP_Filesystem_Direct extends WP_Filesystem_Base {
1718

@@ -23,6 +24,8 @@ class WP_Filesystem_Direct extends WP_Filesystem_Base {
2324
* @param mixed $arg Not used.
2425
*/
2526
public function __construct( $arg ) {
27+
// The $arg parameter is required for signature parity with the other transports, but is unused here.
28+
unset( $arg );
2629
$this->method = 'direct';
2730
$this->errors = new WP_Error();
2831
}
@@ -45,7 +48,7 @@ public function get_contents( $file ) {
4548
* @since 2.5.0
4649
*
4750
* @param string $file Path to the file.
48-
* @return array|false File contents in an array on success, false on failure.
51+
* @return string[]|false File contents in an array on success, false on failure.
4952
*/
5053
public function get_contents_array( $file ) {
5154
return @file( $file );
@@ -138,6 +141,9 @@ public function chgrp( $file, $group, $recursive = false ) {
138141
// Is a directory, and we want recursive.
139142
$file = trailingslashit( $file );
140143
$filelist = $this->dirlist( $file );
144+
if ( false === $filelist ) {
145+
return false;
146+
}
141147

142148
foreach ( $filelist as $file_listing ) {
143149
$this->chgrp( $file . $file_listing['name'], $group, $recursive );
@@ -226,6 +232,9 @@ public function chown( $file, $owner, $recursive = false ) {
226232

227233
// Is a directory, and we want recursive.
228234
$filelist = $this->dirlist( $file );
235+
if ( false === $filelist ) {
236+
return false;
237+
}
229238

230239
foreach ( $filelist as $file_listing ) {
231240
$this->chown( $file . '/' . $file_listing['name'], $owner, $recursive );
@@ -240,7 +249,7 @@ public function chown( $file, $owner, $recursive = false ) {
240249
* @since 2.5.0
241250
*
242251
* @param string $file Path to the file.
243-
* @return string|false Username of the owner on success, false on failure.
252+
* @return string|int<1, max>|false Username of the owner on success, or UID of file owner if not available; false on failure.
244253
*/
245254
public function owner( $file ) {
246255
$owneruid = @fileowner( $file );
@@ -285,7 +294,7 @@ public function getchmod( $file ) {
285294
* @since 2.5.0
286295
*
287296
* @param string $file Path to the file.
288-
* @return string|false The group on success, false on failure.
297+
* @return string|int<1, max>|false Group name on success, or GID of the file's group if not available; false on failure.
289298
*/
290299
public function group( $file ) {
291300
$gid = @filegroup( $file );
@@ -639,6 +648,7 @@ public function rmdir( $path, $recursive = false ) {
639648
* files. False if unable to list directory contents.
640649
* }
641650
* }
651+
* @phpstan-return array<string, FileListing>|false
642652
*/
643653
public function dirlist( $path, $include_hidden = true, $recursive = false ) {
644654
if ( $this->is_file( $path ) ) {
@@ -684,8 +694,8 @@ public function dirlist( $path, $include_hidden = true, $recursive = false ) {
684694
$struc['group'] = $this->group( $path . $entry );
685695
$struc['size'] = $this->size( $path . $entry );
686696
$struc['lastmodunix'] = $this->mtime( $path . $entry );
687-
$struc['lastmod'] = gmdate( 'M j', $struc['lastmodunix'] );
688-
$struc['time'] = gmdate( 'h:i:s', $struc['lastmodunix'] );
697+
$struc['lastmod'] = is_int( $struc['lastmodunix'] ) ? gmdate( 'M j', $struc['lastmodunix'] ) : false;
698+
$struc['time'] = is_int( $struc['lastmodunix'] ) ? gmdate( 'h:i:s', $struc['lastmodunix'] ) : false;
689699
$struc['type'] = $this->is_dir( $path . $entry ) ? 'd' : 'f';
690700

691701
if ( 'd' === $struc['type'] ) {

0 commit comments

Comments
 (0)