Skip to content

Commit 14483a9

Browse files
KrinkleKrinkle
committed
Add return types and make Reader and Writer signatures more consistent
This specifically does not add native parameter type hints to public methods like get(), set() and exists() as we indeed currently permit non-strings to (accidentally or otherwise) make it to these methods in existing consumer code. Doing so would be a semver-major breaking change. Instead, only add return types, and improve code, docs, and tests for the existing parameter types. * Fix warnings in Writer/PHP::set() from internal write() and strlen() functions still receiving non-string. * Fix inconsistency around Reader::get(). The Reader\PHP class recognised "mixed key" and contained strval() casting. Apply the same to the Reader interface and the Reader\DBA implementation. Change-Id: I39bfcb4fcda830649ff9de367f05a41a4a54209e
1 parent 42122fb commit 14483a9

File tree

10 files changed

+57
-53
lines changed

10 files changed

+57
-53
lines changed

‎src/Reader.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ abstract class Reader {
3131
* @param string $fileName
3232
* @return Reader
3333
*/
34-
public static function open( $fileName ) {
34+
public static function open( $fileName ): Reader {
3535
return self::haveExtension() ?
3636
new Reader\DBA( $fileName ) :
3737
new Reader\PHP( $fileName );
@@ -43,7 +43,7 @@ public static function open( $fileName ) {
4343
* @return bool
4444
* @codeCoverageIgnore
4545
*/
46-
public static function haveExtension() {
46+
public static function haveExtension(): bool {
4747
if ( !function_exists( 'dba_handlers' ) ) {
4848
return false;
4949
}
@@ -58,21 +58,23 @@ public static function haveExtension() {
5858
/**
5959
* Close the file. Optional, you can just let the variable go out of scope.
6060
*/
61-
abstract public function close();
61+
abstract public function close(): void;
6262

6363
/**
6464
* Get a value with a given key. Only string values are supported.
6565
*
66-
* @param string $key
66+
* @param string|int $key
67+
* @return string|false
6768
*/
6869
abstract public function get( $key );
6970

7071
/**
7172
* Check whether key exists
7273
*
7374
* @param string $key
75+
* @return bool
7476
*/
75-
abstract public function exists( $key );
77+
abstract public function exists( $key ): bool;
7678

7779
/**
7880
* Fetch first key

‎src/Reader/DBA.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,19 @@ public function __construct( $fileName ) {
3737
}
3838
}
3939

40-
public function close() {
40+
public function close(): void {
4141
if ( $this->handle ) {
4242
dba_close( $this->handle );
4343
}
4444
$this->handle = null;
4545
}
4646

4747
public function get( $key ) {
48-
return dba_fetch( $key, $this->handle );
48+
return dba_fetch( (string)$key, $this->handle );
4949
}
5050

51-
public function exists( $key ) {
52-
return dba_exists( $key, $this->handle );
51+
public function exists( $key ): bool {
52+
return dba_exists( (string)$key, $this->handle );
5353
}
5454

5555
public function firstkey() {

‎src/Reader/Hash.php‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,27 +38,24 @@ class Hash extends Reader {
3838
/**
3939
* Create the object and open the file
4040
*
41-
* @param string[] $data An associative PHP array.
41+
* @param string[] $data An associative array
4242
*/
43-
public function __construct( $data ) {
44-
if ( !is_array( $data ) ) {
45-
throw new \InvalidArgumentException( __METHOD__ . ': "$data" must be an array.' );
46-
}
43+
public function __construct( array $data ) {
4744
$this->data = $data;
4845
}
4946

5047
/**
5148
* Close the file. Optional, you can just let the variable go out of scope.
5249
*/
53-
public function close() {
50+
public function close(): void {
5451
$this->data = [];
5552
$this->keys = null;
5653
}
5754

5855
/**
5956
* Get a value with a given key. Only string values are supported.
6057
*
61-
* @param string $key
58+
* @param string|int $key
6259
* @return string|false The value associated with $key, or false if $key is not known.
6360
*/
6461
public function get( $key ) {
@@ -68,10 +65,10 @@ public function get( $key ) {
6865
/**
6966
* Check whether key exists
7067
*
71-
* @param string $key
68+
* @param string|int $key
7269
* @return bool
7370
*/
74-
public function exists( $key ) {
71+
public function exists( $key ): bool {
7572
return isset( $this->data[ $key ] );
7673
}
7774

‎src/Reader/PHP.php‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class PHP extends Reader {
9393
* @throws Exception If CDB file cannot be opened or if it contains fewer
9494
* than 2048 bytes of data.
9595
*/
96-
public function __construct( $fileName ) {
96+
public function __construct( string $fileName ) {
9797
$this->fileName = $fileName;
9898
$this->handle = fopen( $fileName, 'rb' );
9999
if ( !$this->handle ) {
@@ -108,7 +108,7 @@ public function __construct( $fileName ) {
108108
/**
109109
* Close the handle on the CDB file.
110110
*/
111-
public function close() {
111+
public function close(): void {
112112
if ( $this->handle ) {
113113
fclose( $this->handle );
114114
}
@@ -118,12 +118,11 @@ public function close() {
118118
/**
119119
* Get the value of a key.
120120
*
121-
* @param mixed $key
122-
* @return bool|string The key's value or false if not found.
121+
* @param string|int $key
122+
* @return string|false The key's value or false if not found
123123
*/
124124
public function get( $key ) {
125-
// strval is required
126-
if ( $this->find( strval( $key ) ) ) {
125+
if ( $this->find( (string)$key ) ) {
127126
return $this->read( $this->dataPos, $this->dataLen );
128127
}
129128

@@ -133,9 +132,8 @@ public function get( $key ) {
133132
/**
134133
* Read data from the CDB file.
135134
*
136-
* @throws Exception When attempting to read past the end of the file.
137-
* @param int $start Start reading from this position.
138-
* @param int $len Number of bytes to read.
135+
* @param int $start Start reading from this position
136+
* @param int $len Number of bytes to read
139137
* @return string Read data.
140138
*/
141139
protected function read( $start, $len ) {
@@ -300,11 +298,11 @@ protected function find( $key ) {
300298
/**
301299
* Check if a key exists in the CDB file.
302300
*
303-
* @param string $key
301+
* @param string|int $key
304302
* @return bool Whether the key exists.
305303
*/
306-
public function exists( $key ) {
307-
return $this->find( strval( $key ) );
304+
public function exists( $key ): bool {
305+
return $this->find( (string)$key );
308306
}
309307

310308
/**

‎src/Util.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class Util {
3333
* @param int $b
3434
* @return int
3535
*/
36-
public static function unsignedMod( $a, $b ) {
36+
public static function unsignedMod( $a, $b ): int {
3737
if ( $a & 0x80000000 ) {
3838
$m = ( $a & 0x7fffffff ) % $b + 2 * ( 0x40000000 % $b );
3939

@@ -50,7 +50,7 @@ public static function unsignedMod( $a, $b ) {
5050
* @param int $b
5151
* @return int
5252
*/
53-
public static function unsignedShiftRight( $a, $b ) {
53+
public static function unsignedShiftRight( $a, $b ): int {
5454
if ( $b == 0 ) {
5555
return $a;
5656
}
@@ -67,7 +67,7 @@ public static function unsignedShiftRight( $a, $b ) {
6767
* @param string $s
6868
* @return int
6969
*/
70-
public static function hash( $s ) {
70+
public static function hash( $s ): int {
7171
$h = 5381;
7272
$len = strlen( $s );
7373
for ( $i = 0; $i < $len; $i++ ) {

‎src/Writer.php‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,25 +50,19 @@ public static function open( $fileName ) {
5050
new Writer\PHP( $fileName );
5151
}
5252

53-
/**
54-
* Create the object and open the file
55-
*
56-
* @param string $fileName
57-
*/
58-
abstract public function __construct( $fileName );
59-
6053
/**
6154
* Set a key to a given value. The value will be converted to string.
55+
*
6256
* @param string $key
6357
* @param string $value
6458
*/
65-
abstract public function set( $key, $value );
59+
abstract public function set( $key, $value ): void;
6660

6761
/**
6862
* Close the writer object. You should call this function before the object
6963
* goes out of scope, to write out the final hashtables.
7064
*/
71-
abstract public function close();
65+
abstract public function close(): void;
7266

7367
/**
7468
* If the object goes out of scope, close it

‎src/Writer/DBA.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ class DBA extends Writer {
3131
protected $handle;
3232

3333
/**
34+
* Create the object and open the file.
35+
*
3436
* @param string $fileName
3537
*/
36-
public function __construct( $fileName ) {
38+
public function __construct( string $fileName ) {
3739
$this->realFileName = $fileName;
3840
$this->tmpFileName = $fileName . '.tmp.' . mt_rand( 0, 0x7fffffff );
3941
$this->handle = dba_open( $this->tmpFileName, 'n', 'cdb_make' );
@@ -42,11 +44,11 @@ public function __construct( $fileName ) {
4244
}
4345
}
4446

45-
public function set( $key, $value ) {
46-
return dba_insert( $key, $value, $this->handle );
47+
public function set( $key, $value ): void {
48+
dba_insert( $key, $value, $this->handle );
4749
}
4850

49-
public function close() {
51+
public function close(): void {
5052
if ( $this->handle ) {
5153
dba_close( $this->handle );
5254
if ( $this->isWindows() ) {

‎src/Writer/PHP.php‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,11 @@ class PHP extends Writer {
4141
protected $pos;
4242

4343
/**
44+
* Create the object and open the file.
45+
*
4446
* @param string $fileName
4547
*/
46-
public function __construct( $fileName ) {
48+
public function __construct( string $fileName ) {
4749
$this->realFileName = $fileName;
4850
$this->tmpFileName = $fileName . '.tmp.' . mt_rand( 0, 0x7fffffff );
4951
$this->handle = fopen( $this->tmpFileName, 'wb' );
@@ -63,8 +65,9 @@ public function __construct( $fileName ) {
6365
* @param string $key
6466
* @param string $value
6567
*/
66-
public function set( $key, $value ) {
67-
if ( strval( $key ) === '' ) {
68+
public function set( $key, $value ): void {
69+
$key = (string)$key;
70+
if ( $key === '' ) {
6871
// DBA cross-check hack
6972
return;
7073
}
@@ -74,7 +77,7 @@ public function set( $key, $value ) {
7477
$this->addend( strlen( $key ), strlen( $value ), Util::hash( $key ) );
7578
}
7679

77-
public function close() {
80+
public function close(): void {
7881
if ( $this->handle ) {
7982
$this->finish();
8083
fclose( $this->handle );
@@ -92,7 +95,7 @@ public function close() {
9295
/**
9396
* @param string $buf
9497
*/
95-
protected function write( $buf ) {
98+
protected function write( $buf ): void {
9699
$len = fwrite( $this->handle, $buf );
97100
if ( $len !== strlen( $buf ) ) {
98101
$this->throwException( 'Error writing to CDB file "' . $this->tmpFileName . '".' );
@@ -143,7 +146,7 @@ protected function addbegin( $keylen, $datalen ) {
143146
$this->write( $buf );
144147
}
145148

146-
protected function finish() {
149+
protected function finish(): void {
147150
// Hack for DBA cross-check
148151
$this->hplist = array_reverse( $this->hplist );
149152

‎tests/CdbTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,14 @@ public function testReadWrite() {
9393
}
9494
}
9595

96+
// The above will randomly sometimes generate a number-like string,
97+
// which then becomes an integer when iterated below in foreach.
98+
// Add an explicit test case for this so that we reliably ensure that
99+
// passing integers is allowed, as these can commonly become strings.
100+
$w1->set( 42, 'xyz' );
101+
$w2->set( 42, 'xyz' );
102+
$data[42] = 'xyz';
103+
96104
$w1->close();
97105
$w2->close();
98106

‎tests/Reader/HashTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
class HashTest extends \PHPUnit\Framework\TestCase {
1010

1111
public function testConstructor_fail() {
12-
$this->expectException( \InvalidArgumentException::class );
12+
$this->expectException( \TypeError::class );
1313
new Hash( 'not an array' );
1414
}
1515

0 commit comments

Comments
 (0)