Skip to content

Fix static analysis violations with cursor pagination#134

Merged
swember merged 1 commit intochartmogul:mainfrom
tolry:patch-1
Mar 12, 2026
Merged

Fix static analysis violations with cursor pagination#134
swember merged 1 commit intochartmogul:mainfrom
tolry:patch-1

Conversation

@tolry
Copy link
Copy Markdown
Contributor

@tolry tolry commented Dec 19, 2024

When using cursor pagination, access to has_more and cursor are labelled as violations by phpstan or psalm, since those properties are protected on AbstractModel although they are readable publicly via __get()

sample code

$cursor = null;
do {
    $filter = ["customer_uuid" => $uuid];
    if ($cursor) {
        $filter["cursor"] = $cursor;
    }

    $collection = \ChartMogul\CustomerInvoices::all($filter);

    foreach ($collection->invoices as $invoice) {
        // do something
    }

    $cursor = $collection->cursor;
} while ($collection->has_more);

When using cursor pagination, access to `has_more` and `cursor` are labelled as violations by phpstan or psalm, since those properties are protected on `AbstractModel` although they are readable publicly via `__get()`

sample code 

```php
$cursor = null;
do {
    $filter = ["customer_uuid" => $uuid];
    if ($cursor) {
        $filter["cursor"] = $cursor;
    }

    $collection = \ChartMogul\CustomerInvoices::all($filter);

    foreach ($collection->invoices as $invoice) {
        // do something
    }

    $cursor = $collection->cursor;
} while ($collection->has_more);
```
Copy link
Copy Markdown
Contributor

@swember swember left a comment

Choose a reason for hiding this comment

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

Hello @tolry, thanks a lot for the contribution. Could you please provide which settings for phpstan you use. I tried to create a new project and use your code in it and I haven't got any complaints from phpstan.

@tolry
Copy link
Copy Markdown
Contributor Author

tolry commented Feb 17, 2026

Hi @swember, thanks for being patient. We baselined these issue and haven't thought about them for a while. I double-checked right now, and we still need to put it into our baseline.

The issue does occur for us running PHPStan at level 8. So everything from level 8 or higher should trigger the issue.

If I remove this from the baseline file, the following violation is printed:

$ vendor/bin/phpstan

Note: Using configuration file [...]/phpstan.neon.

 ------ ----------------------------------------------------------------------------------------
  Line   [...]/Command/SynchronizeChartMogulCommand.php
 ------ ----------------------------------------------------------------------------------------
  251    Access to protected property ChartMogul\CustomerInvoices::$has_more.
         🪪  property.protected
 ------ ----------------------------------------------------------------------------------------

@swember
Copy link
Copy Markdown
Contributor

swember commented Mar 12, 2026

PHPstan prints a bunch of other warnings on level 8 and further. This PR is safe to merge, so I'll do it. Thanks for you contribution.

@swember swember merged commit 6245e7e into chartmogul:main Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants