Skip to content

PHPLIB-599: Typing improvements #959

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Sep 13, 2022
Merged

PHPLIB-599: Typing improvements #959

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b0f110f
PHPLIB-600: Add type definitions for parameters
alcaeus Aug 24, 2022
d273370
PHPLIB-602: Add return type definitions where applicable
alcaeus Aug 24, 2022
68d68e3
Enable object type for parameters
alcaeus Aug 24, 2022
e6e08b3
Add comment about using declare
alcaeus Aug 25, 2022
012650f
Remove useless param and return annotations
alcaeus Aug 25, 2022
1dec7b2
Remove redundant type casts
alcaeus Aug 25, 2022
a32be43
Add return types to internal classes
alcaeus Aug 26, 2022
9a9742e
Use object consistently in GridFS classes
alcaeus Aug 26, 2022
c7272cc
Remove useless type annotations
alcaeus Aug 26, 2022
eb6fb73
Remove unnecessary cast
alcaeus Aug 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions phpcs.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,13 @@
<!-- **************************************************************************** -->
<rule ref="SlevomatCodingStandard.TypeHints.ParameterTypeHint">
<properties>
<!-- Requires PHP 7.4 -->
<property name="enableObjectTypeHint" value="false" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment was wrong - the object type was introduced in PHP 7.2.

<!-- Requires PHP 8.0 -->
<property name="enableMixedTypeHint" value="false" />
<!-- Requires PHP 8.0 -->
<property name="enableUnionTypeHint" value="false" />
</properties>

<exclude name="SlevomatCodingStandard.TypeHints.ParameterTypeHint.MissingTraversableTypeHintSpecification" />
<exclude name="SlevomatCodingStandard.TypeHints.ParameterTypeHint.UselessAnnotation" />
</rule>

<rule ref="SlevomatCodingStandard.TypeHints.PropertyTypeHint">
Expand All @@ -128,13 +125,10 @@
</properties>

<exclude name="SlevomatCodingStandard.TypeHints.PropertyTypeHint.MissingTraversableTypeHintSpecification" />
<exclude name="SlevomatCodingStandard.TypeHints.PropertyTypeHint.UselessAnnotation" />
</rule>

<rule ref="SlevomatCodingStandard.TypeHints.ReturnTypeHint">
<properties>
<!-- Requires PHP 7.2 -->
<property name="enableObjectTypeHint" value="false" />
<!-- Requires PHP 8.0 -->
<property name="enableStaticTypeHint" value="false" />
<!-- Requires PHP 8.0 -->
Expand All @@ -144,7 +138,6 @@
</properties>

<exclude name="SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingTraversableTypeHintSpecification" />
<exclude name="SlevomatCodingStandard.TypeHints.ReturnTypeHint.UselessAnnotation" />
</rule>


Expand All @@ -162,12 +155,9 @@
</rule>


<!-- *********************************************************************************** -->
<!-- Require native type hints for all parameters, properties, and return types in tests -->
<!-- *********************************************************************************** -->
<rule ref="SlevomatCodingStandard.TypeHints.ParameterTypeHint.MissingNativeTypeHint">
<exclude-pattern>src</exclude-pattern>
</rule>
<!-- *********************************************************** -->
<!-- Require native type hints for all code without a BC promise -->
<!-- *********************************************************** -->
<rule ref="SlevomatCodingStandard.TypeHints.PropertyTypeHint.MissingNativeTypeHint">
<exclude-pattern>src</exclude-pattern>
</rule>
Expand Down
3 changes: 1 addition & 2 deletions src/BulkWriteResult.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ class BulkWriteResult
private $isAcknowledged;

/**
* @param WriteResult $writeResult
* @param mixed[] $insertedIds
* @param mixed[] $insertedIds
*/
public function __construct(WriteResult $writeResult, array $insertedIds)
{
Expand Down
15 changes: 4 additions & 11 deletions src/ChangeStream.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ class ChangeStream implements Iterator

/**
* @internal
* @param ChangeStreamIterator $iterator
* @param callable $resumeCallable
*/
public function __construct(ChangeStreamIterator $iterator, callable $resumeCallable)
{
Expand Down Expand Up @@ -194,10 +192,8 @@ public function valid()
* Determines if an exception is a resumable error.
*
* @see https://github.com/mongodb/specifications/blob/master/source/change-streams/change-streams.rst#resumable-error
* @param RuntimeException $exception
* @return boolean
*/
private function isResumableError(RuntimeException $exception)
private function isResumableError(RuntimeException $exception): bool
{
if ($exception instanceof ConnectionException) {
return true;
Expand All @@ -224,7 +220,7 @@ private function isResumableError(RuntimeException $exception)
* @param boolean $incrementKey Increment $key if there is a current result
* @throws ResumeTokenException
*/
private function onIteration($incrementKey)
private function onIteration(bool $incrementKey): void
{
/* If the cursorId is 0, the server has invalidated the cursor and we
* will never perform another getMore nor need to resume since any
Expand All @@ -251,10 +247,8 @@ private function onIteration($incrementKey)

/**
* Recreates the ChangeStreamIterator after a resumable server error.
*
* @return void
*/
private function resume()
private function resume(): void
{
$this->iterator = call_user_func($this->resumeCallable, $this->getResumeToken(), $this->hasAdvanced);
$this->iterator->rewind();
Expand All @@ -265,10 +259,9 @@ private function resume()
/**
* Either resumes after a resumable error or re-throws the exception.
*
* @param RuntimeException $exception
* @throws RuntimeException
*/
private function resumeOrThrow(RuntimeException $exception)
private function resumeOrThrow(RuntimeException $exception): void
{
if ($this->isResumableError($exception)) {
$this->resume();
Expand Down
12 changes: 6 additions & 6 deletions src/Client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class Client
* @throws DriverInvalidArgumentException for parameter/option parsing errors in the driver
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
*/
public function __construct($uri = 'mongodb://127.0.0.1/', array $uriOptions = [], array $driverOptions = [])
public function __construct(string $uri = 'mongodb://127.0.0.1/', array $uriOptions = [], array $driverOptions = [])
{
$driverOptions += ['typeMap' => self::$defaultTypeMap];

Expand All @@ -116,7 +116,7 @@ public function __construct($uri = 'mongodb://127.0.0.1/', array $uriOptions = [

$driverOptions['driver'] = $this->mergeDriverInfo($driverOptions['driver'] ?? []);

$this->uri = (string) $uri;
$this->uri = $uri;
$this->typeMap = $driverOptions['typeMap'] ?? null;

unset($driverOptions['typeMap']);
Expand Down Expand Up @@ -155,7 +155,7 @@ public function __debugInfo()
* @param string $databaseName Name of the database to select
* @return Database
*/
public function __get($databaseName)
public function __get(string $databaseName)
{
return $this->selectDatabase($databaseName);
}
Expand Down Expand Up @@ -201,7 +201,7 @@ public function createClientEncryption(array $options)
* @throws InvalidArgumentException for parameter/option parsing errors
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
*/
public function dropDatabase($databaseName, array $options = [])
public function dropDatabase(string $databaseName, array $options = [])
{
if (! isset($options['typeMap'])) {
$options['typeMap'] = $this->typeMap;
Expand Down Expand Up @@ -314,7 +314,7 @@ public function listDatabases(array $options = [])
* @return Collection
* @throws InvalidArgumentException for parameter/option parsing errors
*/
public function selectCollection($databaseName, $collectionName, array $options = [])
public function selectCollection(string $databaseName, string $collectionName, array $options = [])
{
$options += ['typeMap' => $this->typeMap];

Expand All @@ -330,7 +330,7 @@ public function selectCollection($databaseName, $collectionName, array $options
* @return Database
* @throws InvalidArgumentException for parameter/option parsing errors
*/
public function selectDatabase($databaseName, array $options = [])
public function selectDatabase(string $databaseName, array $options = [])
{
$options += ['typeMap' => $this->typeMap];

Expand Down
12 changes: 6 additions & 6 deletions src/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,13 @@ class Collection
* @param array $options Collection options
* @throws InvalidArgumentException for parameter/option parsing errors
*/
public function __construct(Manager $manager, $databaseName, $collectionName, array $options = [])
public function __construct(Manager $manager, string $databaseName, string $collectionName, array $options = [])
{
if (strlen((string) $databaseName) < 1) {
if (strlen($databaseName) < 1) {
throw new InvalidArgumentException('$databaseName is invalid: ' . $databaseName);
}

if (strlen((string) $collectionName) < 1) {
if (strlen($collectionName) < 1) {
throw new InvalidArgumentException('$collectionName is invalid: ' . $collectionName);
}

Expand All @@ -153,8 +153,8 @@ public function __construct(Manager $manager, $databaseName, $collectionName, ar
}

$this->manager = $manager;
$this->databaseName = (string) $databaseName;
$this->collectionName = (string) $collectionName;
$this->databaseName = $databaseName;
$this->collectionName = $collectionName;
$this->readConcern = $options['readConcern'] ?? $this->manager->getReadConcern();
$this->readPreference = $options['readPreference'] ?? $this->manager->getReadPreference();
$this->typeMap = $options['typeMap'] ?? self::$defaultTypeMap;
Expand Down Expand Up @@ -452,7 +452,7 @@ public function deleteOne($filter, array $options = [])
* @throws InvalidArgumentException for parameter/option parsing errors
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
*/
public function distinct($fieldName, $filter = [], array $options = [])
public function distinct(string $fieldName, $filter = [], array $options = [])
{
if (! isset($options['readPreference']) && ! is_in_transaction($options)) {
$options['readPreference'] = $this->readPreference;
Expand Down
11 changes: 4 additions & 7 deletions src/Command/ListCollections.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class ListCollections implements Executable
* @param array $options Command options
* @throws InvalidArgumentException for parameter/option parsing errors
*/
public function __construct($databaseName, array $options = [])
public function __construct(string $databaseName, array $options = [])
{
if (isset($options['authorizedCollections']) && ! is_bool($options['authorizedCollections'])) {
throw InvalidArgumentException::invalidType('"authorizedCollections" option', $options['authorizedCollections'], 'boolean');
Expand All @@ -95,15 +95,14 @@ public function __construct($databaseName, array $options = [])
throw InvalidArgumentException::invalidType('"session" option', $options['session'], Session::class);
}

$this->databaseName = (string) $databaseName;
$this->databaseName = $databaseName;
$this->options = $options;
}

/**
* Execute the operation.
*
* @see Executable::execute()
* @param Server $server
* @return CachingIterator
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
*/
Expand All @@ -117,10 +116,8 @@ public function execute(Server $server)

/**
* Create the listCollections command.
*
* @return Command
*/
private function createCommand()
private function createCommand(): Command
{
$cmd = ['listCollections' => 1];

Expand All @@ -146,7 +143,7 @@ private function createCommand()
* @see https://php.net/manual/en/mongodb-driver-server.executecommand.php
* @return array
*/
private function createOptions()
private function createOptions(): array
{
$options = [];

Expand Down
7 changes: 2 additions & 5 deletions src/Command/ListDatabases.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ public function __construct(array $options = [])
* Execute the operation.
*
* @see Executable::execute()
* @param Server $server
* @return array An array of database info structures
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to use an array return type hint on execute() here? Correct me if I'm wrong, but I think that'd be permitted by covariance.

If this is out of scope for this PR, feel free to ignore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Covariance would indeed allow us to add an array type here. However, backward compatibility promise won't allow us to do so, as anyone extending ListDatabases would be encountering fatal errors. Granted, it doesn't make much sense extending these classes, but they are all marked as part of the API and non-final, so we can't add return types to any public or protected methods.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good point about extension. I didn't consider that.

Shame on us for not making these things final by default. Somewhere, @Ocramius is silently shaking his head in disappointment.

* @throws UnexpectedValueException if the command response was malformed
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
Expand All @@ -119,10 +118,8 @@ public function execute(Server $server)

/**
* Create the listDatabases command.
*
* @return Command
*/
private function createCommand()
private function createCommand(): Command
{
$cmd = ['listDatabases' => 1];

Expand All @@ -148,7 +145,7 @@ private function createCommand()
* @see https://php.net/manual/en/mongodb-driver-server.executecommand.php
* @return array
*/
private function createOptions()
private function createOptions(): array
{
$options = [];

Expand Down
19 changes: 9 additions & 10 deletions src/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ class Database
* @param array $options Database options
* @throws InvalidArgumentException for parameter/option parsing errors
*/
public function __construct(Manager $manager, $databaseName, array $options = [])
public function __construct(Manager $manager, string $databaseName, array $options = [])
{
if (strlen((string) $databaseName) < 1) {
if (strlen($databaseName) < 1) {
throw new InvalidArgumentException('$databaseName is invalid: ' . $databaseName);
}

Expand All @@ -127,7 +127,7 @@ public function __construct(Manager $manager, $databaseName, array $options = []
}

$this->manager = $manager;
$this->databaseName = (string) $databaseName;
$this->databaseName = $databaseName;
$this->readConcern = $options['readConcern'] ?? $this->manager->getReadConcern();
$this->readPreference = $options['readPreference'] ?? $this->manager->getReadPreference();
$this->typeMap = $options['typeMap'] ?? self::$defaultTypeMap;
Expand Down Expand Up @@ -164,7 +164,7 @@ public function __debugInfo()
* @param string $collectionName Name of the collection to select
* @return Collection
*/
public function __get($collectionName)
public function __get(string $collectionName)
{
return $this->selectCollection($collectionName);
}
Expand Down Expand Up @@ -257,14 +257,13 @@ public function command($command, array $options = [])
* Create a new collection explicitly.
*
* @see CreateCollection::__construct() for supported options
* @param string $collectionName
* @param array $options
* @param array $options
* @return array|object Command result document
* @throws UnsupportedException if options are not supported by the selected server
* @throws InvalidArgumentException for parameter/option parsing errors
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
*/
public function createCollection($collectionName, array $options = [])
public function createCollection(string $collectionName, array $options = [])
{
if (! isset($options['typeMap'])) {
$options['typeMap'] = $this->typeMap;
Expand Down Expand Up @@ -340,7 +339,7 @@ public function drop(array $options = [])
* @throws InvalidArgumentException for parameter/option parsing errors
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
*/
public function dropCollection($collectionName, array $options = [])
public function dropCollection(string $collectionName, array $options = [])
{
if (! isset($options['typeMap'])) {
$options['typeMap'] = $this->typeMap;
Expand Down Expand Up @@ -477,7 +476,7 @@ public function listCollections(array $options = [])
* @throws InvalidArgumentException for parameter/option parsing errors
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
*/
public function modifyCollection($collectionName, array $collectionOptions, array $options = [])
public function modifyCollection(string $collectionName, array $collectionOptions, array $options = [])
{
if (! isset($options['typeMap'])) {
$options['typeMap'] = $this->typeMap;
Expand Down Expand Up @@ -537,7 +536,7 @@ public function renameCollection(string $fromCollectionName, string $toCollectio
* @return Collection
* @throws InvalidArgumentException for parameter/option parsing errors
*/
public function selectCollection($collectionName, array $options = [])
public function selectCollection(string $collectionName, array $options = [])
{
$options += [
'readConcern' => $this->readConcern,
Expand Down
4 changes: 2 additions & 2 deletions src/Exception/BadMethodCallException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class BadMethodCallException extends BaseBadMethodCallException implements Excep
* @param string $class Class name
* @return self
*/
public static function classIsImmutable($class)
public static function classIsImmutable(string $class)
{
return new static(sprintf('%s is immutable', $class));
}
Expand All @@ -40,7 +40,7 @@ public static function classIsImmutable($class)
* @param string $method Method name
* @return self
*/
public static function unacknowledgedWriteResultAccess($method)
public static function unacknowledgedWriteResultAccess(string $method)
{
return new static(sprintf('%s should not be called for an unacknowledged write result', $method));
}
Expand Down
2 changes: 1 addition & 1 deletion src/Exception/InvalidArgumentException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class InvalidArgumentException extends DriverInvalidArgumentException implements
* @param string|string[] $expectedType Expected type
* @return self
*/
public static function invalidType($name, $value, $expectedType)
public static function invalidType(string $name, $value, $expectedType)
{
if (is_array($expectedType)) {
switch (count($expectedType)) {
Expand Down
Loading