From aed6bc415cc4d79a0f9eff75fcf7abef090dad5d Mon Sep 17 00:00:00 2001 From: Justin Ruiter Date: Wed, 3 Apr 2024 17:02:17 +0200 Subject: [PATCH] Implemented ADmad's suggestions about dependencies, phpunit, psalm and phpstan isues --- .github/workflows/ci.yml | 13 +++---- composer.json | 2 +- phpunit.xml.dist | 2 +- src/Datasource/Connection.php | 3 +- src/Datasource/Query.php | 48 +++++++++++++----------- src/Datasource/ResultSet.php | 2 +- src/Datasource/Schema.php | 12 ++++-- src/Model/Endpoint.php | 26 +++++++------ src/Model/EndpointLocator.php | 2 +- src/Webservice/Driver/AbstractDriver.php | 8 ++-- src/Webservice/Webservice.php | 6 +-- 11 files changed, 66 insertions(+), 58 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6883142..85e56fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,11 +14,11 @@ jobs: strategy: fail-fast: false matrix: - php-version: ['8.2', '8.3'] + php-version: ['8.1', '8.2', '8.3'] db-type: ['mysql', 'pgsql'] prefer-lowest: [''] include: - - php-version: '8.3' + - php-version: '8.1' db-type: 'sqlite' prefer-lowest: 'prefer-lowest' @@ -61,14 +61,14 @@ jobs: if [[ ${{ matrix.db-type }} == 'mysql' ]]; then export DB_URL='mysql://root:root@127.0.0.1/cakephp'; fi if [[ ${{ matrix.db-type }} == 'pgsql' ]]; then export DB_URL='postgres://postgres:postgres@127.0.0.1/postgres'; fi - if [[ ${{ matrix.php-version }} == '8.3' && ${{ matrix.db-type }} == 'mysql' ]]; then + if [[ ${{ matrix.php-version }} == '8.1' && ${{ matrix.db-type }} == 'mysql' ]]; then vendor/bin/phpunit --coverage-clover=coverage.xml else vendor/bin/phpunit fi - name: Code Coverage Report - if: success() && matrix.php-version == '8.3' && matrix.db-type == 'mysql' + if: success() && matrix.php-version == '8.1' && matrix.db-type == 'mysql' uses: codecov/codecov-action@v1 cs-stan: @@ -81,7 +81,7 @@ jobs: - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: '8.3' + php-version: '8.1' extensions: mbstring, intl coverage: none tools: psalm:5.23.1, phpstan:1.10.65 @@ -90,8 +90,7 @@ jobs: run: composer require cakephp/cakephp-codesniffer:^4.2 - name: Run phpcs - # Exclude Type hint sniffing, as it interferes with psalm - run: vendor/bin/phpcs --standard=CakePHP --exclude=CakePHP.Classes.ReturnTypeHint src/ tests/ + run: vendor/bin/phpcs --standard=CakePHP src/ tests/ - name: Run psalm if: success() || failure() diff --git a/composer.json b/composer.json index 5937c3e..5361dbb 100644 --- a/composer.json +++ b/composer.json @@ -44,7 +44,7 @@ "require-dev": { "cakephp/cakephp": "^5.0", "cakephp/cakephp-codesniffer": "^5.0", - "phpunit/phpunit": "^10.1", + "phpunit/phpunit": "^10.5.5", "ext-mbstring": "*", "vimeo/psalm": "5.23.1", "phpstan/phpstan": "1.10.65" diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 36952e4..4891fcf 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,7 +1,7 @@ - + ./tests/ diff --git a/src/Datasource/Connection.php b/src/Datasource/Connection.php index 13b89df..39b492a 100644 --- a/src/Datasource/Connection.php +++ b/src/Datasource/Connection.php @@ -58,7 +58,8 @@ public function __construct(array $config) /** * @param \Psr\SimpleCache\CacheInterface $cacher The cacher instance to use for query caching. - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function setCacher(CacheInterface $cacher): ConnectionInterface { diff --git a/src/Datasource/Query.php b/src/Datasource/Query.php index 98cd211..f688e8f 100644 --- a/src/Datasource/Query.php +++ b/src/Datasource/Query.php @@ -231,7 +231,8 @@ public function all(): ResultSetInterface /** * @param \Closure|array|string $fields The field configuration for the order by clause * @param bool $overwrite Whether to overwrite the existing conditions - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function orderBy(Closure|array|string $fields, bool $overwrite = false): Query { @@ -374,7 +375,8 @@ public function toArray(): array * Set the default repository object that will be used by this query. * * @param \Cake\Datasource\RepositoryInterface $repository The default repository object to use. - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function setRepository(RepositoryInterface $repository): Query { @@ -399,7 +401,7 @@ public function getRepository(): RepositoryInterface /** * Mark the query as create * - * @return $this + * @return self */ public function create(): Query { @@ -411,7 +413,7 @@ public function create(): Query /** * Mark the query as read * - * @return $this + * @return self */ public function read(): Query { @@ -423,7 +425,7 @@ public function read(): Query /** * Mark the query as update * - * @return $this + * @return self */ public function update(): Query { @@ -435,7 +437,7 @@ public function update(): Query /** * Mark the query as delete * - * @return $this + * @return self */ public function delete(): Query { @@ -468,8 +470,7 @@ public function clause(string $name): mixed * Set the endpoint to be used * * @param \Muffin\Webservice\Model\Endpoint $endpoint The endpoint to use - * @return $this - * @psalm-suppress LessSpecificReturnStatement + * @return self */ public function setEndpoint(Endpoint $endpoint): Query { @@ -482,7 +483,6 @@ public function setEndpoint(Endpoint $endpoint): Query * Set the endpoint to be used * * @return \Muffin\Webservice\Model\Endpoint - * @psalm-suppress MoreSpecificReturnType */ public function getEndpoint(): Endpoint { @@ -493,7 +493,7 @@ public function getEndpoint(): Endpoint * Set the webservice to be used * * @param \Muffin\Webservice\Webservice\WebserviceInterface $webservice The webservice to use - * @return $this + * @return self */ public function setWebservice(WebserviceInterface $webservice): Query { @@ -549,8 +549,8 @@ public function aliasField(string $field, ?string $alias = null): array * @param \Closure|array|string|null $conditions The list of conditions. * @param array $types Not used, required to comply with QueryInterface. * @param bool $overwrite Whether to replace previous queries. - * @return $this - * @psalm-suppress ImplementedReturnTypeMismatch Not the nicest solution, but wishing to keep the functionality backwards compatible + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function where( Closure|array|string|null $conditions = null, @@ -570,7 +570,7 @@ public function where( * * @param array|string $conditions The conditions to add with AND. * @param array $types associative array of type names used to bind values to query - * @return $this + * @return self * @see \Cake\Database\Query::where() * @see \Cake\Database\Type * @psalm-suppress PossiblyInvalidArgument @@ -586,7 +586,7 @@ public function andWhere(string|array $conditions, array $types = []): Query * Charge this query's action * * @param int $action Action to use - * @return $this + * @return self */ public function action(int $action): Query { @@ -607,7 +607,8 @@ public function action(int $action): Query * @param int $num The page number you want. * @param int|null $limit The number of rows you want in the page. If null * the current limit clause will be used. - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function page(int $num, ?int $limit = null): Query { @@ -637,7 +638,8 @@ public function page(int $num, ?int $limit = null): Query * ``` * * @param ?int $limit number of records to be returned - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function limit(?int $limit): Query { @@ -650,7 +652,7 @@ public function limit(?int $limit): Query * Set fields to save in resources * * @param \Closure|array|string $fields The field to set - * @return $this + * @return self */ public function set(Closure|array|string $fields): Query { @@ -688,7 +690,8 @@ public function offset(?int $offset): Query|QueryInterface * * @param \Cake\Database\ExpressionInterface|\Closure|array|string $fields fields to be added to the list * @param bool $overwrite whether to reset order with field list or not - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function order(array|ExpressionInterface|Closure|string $fields, bool $overwrite = false): Query { @@ -880,7 +883,8 @@ public function jsonSerialize(): ResultSetInterface * * @param \Cake\Database\ExpressionInterface|\Closure|array|string|float|int $fields The list of fields to select from _source. * @param bool $overwrite Whether or not to replace previous selections. - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function select(ExpressionInterface|Closure|array|string|int|float $fields, bool $overwrite = false): Query { @@ -957,7 +961,7 @@ protected function decorateResults(iterable $result): ResultSetInterface * @param \Closure|null $mapper The mapper function * @param \Closure|null $reducer The reducing function * @param bool $overwrite Set to true to overwrite existing map + reduce functions. - * @return $this + * @return self * @see \Cake\Collection\Iterator\MapReduce for details on how to use emit data to the map reducer. */ public function mapReduce(?Closure $mapper = null, ?Closure $reducer = null, bool $overwrite = false): Query @@ -1013,7 +1017,7 @@ public function isEagerLoaded(): bool * passed, the current configured query `_eagerLoaded` value is returned. * * @param bool $value Whether to eager load. - * @return $this + * @return self */ public function eagerLoaded(bool $value): Query { @@ -1062,7 +1066,7 @@ public function eagerLoaded(bool $value): Query * * @param \Closure|null $formatter The formatting function * @param int|bool $mode Whether to overwrite, append or prepend the formatter. - * @return $this + * @return self * @throws \InvalidArgumentException */ public function formatResults(?Closure $formatter = null, int|bool $mode = self::APPEND): Query diff --git a/src/Datasource/ResultSet.php b/src/Datasource/ResultSet.php index 3a0b733..9e224e4 100644 --- a/src/Datasource/ResultSet.php +++ b/src/Datasource/ResultSet.php @@ -120,7 +120,7 @@ public function valid(): bool * Part of Iterator interface. * * @return int - * @psalm-suppress ImplementedReturnTypeMismatch This seems to be implemented with the key as an integer everywhere **/ + */ public function key(): int { return $this->_index; diff --git a/src/Datasource/Schema.php b/src/Datasource/Schema.php index a21b351..b303e56 100644 --- a/src/Datasource/Schema.php +++ b/src/Datasource/Schema.php @@ -157,7 +157,8 @@ public function name(): string * * @param string $name The name of the column * @param array|string $attrs The attributes for the column. - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function addColumn(string $name, array|string $attrs): Schema { @@ -219,7 +220,8 @@ public function hasColumn(string $name): bool * If the column is not defined in the table, no error will be raised. * * @param string $name The name of the column - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function removeColumn(string $name): Schema { @@ -233,7 +235,8 @@ public function removeColumn(string $name): Schema * * @param string $name Column name * @param string $type Type to set for the column - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function setColumnType(string $name, string $type): Schema { @@ -358,7 +361,8 @@ public function getPrimaryKey(): array * Set the schema options for an endpoint * * @param array $options Array of options to set - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function setOptions(array $options): Schema { diff --git a/src/Model/Endpoint.php b/src/Model/Endpoint.php index 170ff22..1ebe23f 100644 --- a/src/Model/Endpoint.php +++ b/src/Model/Endpoint.php @@ -241,7 +241,7 @@ public function initialize(array $config): void * Set the name of this endpoint * * @param string $name The name for this endpoint instance - * @return $this + * @return self */ public function setName(string $name): Endpoint { @@ -291,7 +291,8 @@ public function aliasField(string $field): string * Sets the table registry key used to create this table instance. * * @param string $registryAlias The key used to access this object. - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function setRegistryAlias(string $registryAlias): Endpoint { @@ -318,7 +319,7 @@ public function getRegistryAlias(): string * Sets the connection driver. * * @param \Muffin\Webservice\Datasource\Connection $connection Connection instance - * @return $this + * @return self */ public function setConnection(Connection $connection): Endpoint { @@ -347,7 +348,8 @@ public function getConnection(): ?Connection * out of it and used as the schema for this endpoint. * * @param \Muffin\Webservice\Datasource\Schema|array $schema Either an array of fields and config, or a schema object - * @return $this + * @return self + * @throws \Exception */ public function setSchema(Schema|array $schema): Endpoint { @@ -364,6 +366,7 @@ public function setSchema(Schema|array $schema): Endpoint * Returns the schema endpoint object describing this endpoint's properties. * * @return \Muffin\Webservice\Datasource\Schema|null + * @throws \Exception */ public function getSchema(): ?Schema { @@ -394,7 +397,7 @@ public function hasField(string $field): bool * Returns the current endpoint * * @param list|string|null $key sets a new name to be used as primary key - * @return $this + * @return self */ public function setPrimaryKey(string|array|null $key): Endpoint { @@ -427,7 +430,7 @@ public function getPrimaryKey(): array|string|null * Sets the endpoint display field * * @param array|string $field The new field to use as the display field - * @return $this + * @return self */ public function setDisplayField(string|array $field): Endpoint { @@ -464,7 +467,7 @@ public function getDisplayField(): string|array|null * Set the resource class name used to hydrate resources for this endpoint * * @param string $name Name of the class to use - * @return $this + * @return self * @throws \Muffin\Webservice\Model\Exception\MissingResourceClassException If the resource class specified does not exist */ public function setResourceClass(string $name): Endpoint @@ -515,7 +518,7 @@ public function getResourceClass(): string * Set a new inflection method * * @param string $method The name of the inflection method - * @return $this + * @return self */ public function setInflectionMethod(string $method): Endpoint { @@ -539,7 +542,7 @@ public function getInflectionMethod(): string * * @param string $alias Alias for the webservice * @param \Muffin\Webservice\Webservice\WebserviceInterface $webservice The webservice instance - * @return $this + * @return self * @throws \Muffin\Webservice\Webservice\Exception\UnexpectedDriverException When no driver exists for the endpoint */ public function setWebservice(string $alias, WebserviceInterface $webservice): Endpoint @@ -899,8 +902,6 @@ public function updateAll(Closure|array|string $fields, Closure|array|string|nul * @return int Count of affected rows. * @throws \Exception When the delete action could not be executed * @see \Muffin\Webservice\Endpoint::delete() - * @psalm-suppress InvalidReturnStatement - * @psalm-suppress InvalidReturnType */ public function deleteAll(mixed $conditions): int { @@ -1339,7 +1340,8 @@ public function __debugInfo(): array * Set the endpoint alias * * @param string $alias Alias for this endpoint - * @return $this + * @return self + * @psalm-suppress LessSpecificImplementedReturnType */ public function setAlias(string $alias): Endpoint { diff --git a/src/Model/EndpointLocator.php b/src/Model/EndpointLocator.php index dd7a8a9..2d6d470 100644 --- a/src/Model/EndpointLocator.php +++ b/src/Model/EndpointLocator.php @@ -23,8 +23,8 @@ class EndpointLocator extends AbstractLocator * @param string $alias The alias to set. * @param \Muffin\Webservice\Model\Endpoint $repository The repository to set. * @return \Muffin\Webservice\Model\Endpoint - * @psalm-suppress MoreSpecificImplementedParamType * @psalm-suppress MoreSpecificReturnType + * @psalm-suppress MoreSpecificImplementedParamType Not a nice solution, but in this plugin, we only support Endpoints */ public function set(string $alias, RepositoryInterface $repository): Endpoint { diff --git a/src/Webservice/Driver/AbstractDriver.php b/src/Webservice/Driver/AbstractDriver.php index d1e5b37..a955407 100644 --- a/src/Webservice/Driver/AbstractDriver.php +++ b/src/Webservice/Driver/AbstractDriver.php @@ -72,7 +72,7 @@ abstract public function initialize(): void; * Set the client instance this driver will use to make requests * * @param object $client Client instance - * @return $this + * @return self */ public function setClient(object $client): AbstractDriver { @@ -96,7 +96,7 @@ public function getClient(): ?object * * @param string $name The registry alias for the webservice instance * @param \Muffin\Webservice\Webservice\WebserviceInterface $webservice Instance of the webservice - * @return $this + * @return self */ public function setWebservice(string $name, WebserviceInterface $webservice): AbstractDriver { @@ -164,7 +164,7 @@ public function configName(): string /** * Enable query logging for the driver * - * @return $this + * @return self */ public function enableQueryLogging(): AbstractDriver { @@ -176,7 +176,7 @@ public function enableQueryLogging(): AbstractDriver /** * Disable query logging for the driver * - * @return $this + * @return self */ public function disableQueryLogging(): AbstractDriver { diff --git a/src/Webservice/Webservice.php b/src/Webservice/Webservice.php index b72bf89..08d2637 100644 --- a/src/Webservice/Webservice.php +++ b/src/Webservice/Webservice.php @@ -76,7 +76,7 @@ public function initialize(): void * Set the webservice driver and return the instance for chaining * * @param \Muffin\Webservice\Webservice\Driver\AbstractDriver $driver Instance of the driver - * @return $this + * @return self */ public function setDriver(AbstractDriver $driver): Webservice { @@ -103,7 +103,7 @@ public function getDriver(): AbstractDriver * Set the endpoint path this webservice uses * * @param string $endpoint Endpoint path - * @return $this + * @return self */ public function setEndpoint(string $endpoint): Webservice { @@ -209,8 +209,6 @@ public function describe(string $endpoint): Schema * @param \Muffin\Webservice\Datasource\Query $query The query to execute * @param array $options The options to use * @return \Muffin\Webservice\Model\Resource|\Muffin\Webservice\Datasource\ResultSet|int|bool - * @psalm-suppress NullableReturnStatement - * @psalm-suppress InvalidNullableReturnType */ protected function _executeQuery(Query $query, array $options = []): bool|int|Resource|ResultSet {