Skip to content

chore: better PHPStan support #244

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
taka-oyama merged 4 commits into from
Dec 25, 2024
Merged

chore: better PHPStan support #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd90331
chore: better PHPStan support
zeriyoshi Dec 24, 2024
3d1c46a
[ci skip] update CHANGELOG.md
zeriyoshi Dec 24, 2024
c48659d
[ci skip] update CHANGELOG.md
zeriyoshi Dec 25, 2024
cdea69a
fix: update phpstan.neon
zeriyoshi Dec 25, 2024
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# v8.3.0 (2024-11-08)
# v8.3.0 (2024-12-25)

- add support for full text search (#235)
- add support for IDENTITY columns (#243)
Expand All @@ -10,6 +10,7 @@
- add support for `Query\Builder::whereNotInUnnest(...)` (#225)
- `Query\Builder::whereIn` will now wrap values in `UNNEST` if the number of values exceeds the limit (950). (#226)
- Commit options can now be set through config or through `Connection::setCommitOptions(...)` (#229)
- improve PHPStan support (#244)

# v8.2.0 (2024-08-05)

Expand Down
4 changes: 2 additions & 2 deletions phpstan.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ parameters:
path: src/Connection.php
- message: '#^Method Colopl\\Spanner\\Connection::selectWithOptions\(\) should return array<int, array> but returns mixed\.$#'
path: src/Connection.php
- message: "#^Method Colopl\\\\Spanner\\\\Connection\\:\\:cursor\\(\\) should return Generator\\<int, stdClass, mixed, mixed\\> but returns Generator\\<int, array, mixed, mixed\\>\\.$#"
path: src/Connection.php
- message: "#^Return type \\(Generator\\<int, array, mixed, mixed\\>\\) of method Colopl\\\\Spanner\\\\Connection\\:\\:cursor\\(\\) should be compatible with return type \\(Generator\\<int, stdClass, mixed, mixed\\>\\) of method Illuminate\\\\Database\\\\Connection\\:\\:cursor\\(\\)$#"
count: 1
path: src/Connection.php
Comment on lines +16 to +18
Copy link

@coderabbitai coderabbitai bot Dec 25, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Return type mismatch between parent and child class requires attention

The verification confirms there is indeed a type compatibility issue:

  • The parent class Illuminate\Database\Connection::cursor() expects Generator<int, stdClass, mixed, mixed>
  • The current implementation in src/Connection.php returns Generator<int, array<array-key, mixed>> as seen in the executeQuery method's return type documentation

This is a legitimate type mismatch that needs to be addressed in one of two ways:

  • Modify the implementation to convert the array entries to stdClass objects before yielding
  • Document the intentional deviation and suppress this specific PHPStan error if returning arrays is the desired behavior
🔗 Analysis chain

Confirm compatibility of the updated return type requirement.

With this new error message asserting that cursor() must return stdClass entries rather than array entries, ensure that the actual return type in src/Connection.php matches the expected Generator<int, stdClass, mixed, mixed> signature. If the intention is to return arrays, this rule might require further refinement or an added suppression/exclusion in phpstan.neon to accurately reflect the design.

Below is an optional script to locate the cursor() method in src/Connection.php and confirm its documented return type:

Let me gather more information about the return type implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the documented return type for Connection::cursor in src/Connection.php.

# Search for the cursor method signature and nearby doc comments to confirm consistency
rg -A 5 "function cursor" src/Connection.php

Length of output: 580

Script:

#!/bin/bash
# Check the executeQuery method to see the actual return type implementation
rg -B 2 -A 10 "executeQuery" src/Connection.php

Length of output: 1401

- message: '#^Generator expects value type array, mixed given.$#'
path: src/Connection.php
count: 1
Expand Down
3 changes: 2 additions & 1 deletion src/Connection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ public function select($query, $bindings = [], $useReadPdo = true): array
}

/**
* @inheritDoc
* {@inheritDoc}
* @return Generator<int, array<array-key, mixed>>
*/
public function cursor($query, $bindings = [], $useReadPdo = true): Generator
{
Expand Down
4 changes: 4 additions & 0 deletions src/Query/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@
use Illuminate\Contracts\Support\Arrayable;
use Illuminate\Database\Query\Builder as BaseBuilder;
use Illuminate\Support\Arr;
use Illuminate\Support\Collection;
use LogicException;

/**
* @method Collection<int, array<array-key, mixed>> get($columns = ['*'])
*/
class Builder extends BaseBuilder
{
use Concerns\SetsRequestTimeouts;
Expand Down
Loading