Skip to content

Enable PHPStan checked-exception analysis - #292

Open
oschwald wants to merge 3 commits into
mainfrom
greg/phpstan-checked-exceptions
Open

oschwald wants to merge 3 commits into
mainfrom
greg/phpstan-checked-exceptions

Conversation

@oschwald

@oschwald oschwald commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

This enables PHPStan checked-exception analysis for the library.

  • Enable exceptions.check.missingCheckedExceptionInThrows. PHPStan now reports a checked exception that a method throws but does not declare in its @throws tag.
  • Configure Error and LogicException as unchecked. They signal programmer errors, such as mixing the $values array with named arguments. Setting them explicitly makes the result the same on all PHPStan 2.2 releases. 2.2.5 treats Error as checked by default, and 2.2.16 does not.
  • Add @throws InvalidInputException to the request methods and their validation helpers. They throw it when input validation is enabled.
  • Tests do not declare @throws, because PHPUnit handles any exception a test throws. A path-scoped ignoreErrors entry ignores missingType.checkedException under tests/.
  • Add tests for the InvalidArgumentException that the with*() methods and report() throw if $values is non-empty and named arguments are provided.
  • Set minimum versions for PHPStan (^2.2), PHP_CodeSniffer (^4.0), and php-cs-fixer (^3.95). composer.lock is not committed, so these constraints decide what CI installs.
  • tooWideThrowType is not enabled.

When web-service-common-php releases its new @throws tags (maxmind/web-service-common-php#142), PHPStan will report the RuntimeException from the Client constructor, get(), and post(). We will then need to declare it here.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Clarified that, when input validation is enabled, MinFraud request-building methods can throw InvalidInputException for invalid values, types, or keys.
    • Documented that InvalidArgumentException is thrown when a non-empty values array is combined with named arguments.
    • Updated the 3.8.0 changelog to note the validation exceptions for with() and with*() methods.
  • Tests
    • Added coverage for calls that combine an array of values with named arguments.

Copilot AI balanced review requested due to automatic review settings October 2, 2026 00:17
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 1c4f27d0-05dd-49ba-acbc-454d34510a3a

📥 Commits

Reviewing files that changed from the base of the PR and between 5253e38 and 3a8c923.

📒 Files selected for processing (7)
  • CHANGELOG.md
  • composer.json
  • phpstan.neon
  • src/MinFraud.php
  • src/MinFraud/ServiceClient.php
  • tests/MaxMind/Test/MinFraud/ReportTransaction/ReportTransactionTest.php
  • tests/MaxMind/Test/MinFraudTest.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The pull request documents input-validation and mixed-argument exceptions in PHPDoc, adds tests for mixed array and named-argument calls, and updates PHPStan settings and development-tool constraints. It does not change executable behavior or public signatures.

Changes

Exception documentation and analysis

Layer / File(s) Summary
Document exception conditions
src/MinFraud.php, src/MinFraud/ServiceClient.php, CHANGELOG.md
PHPDoc describes validation exceptions for request methods and helpers. The 3.8.0 changelog records validation exceptions for with() and with*() methods.
Test mixed-argument calls
tests/MaxMind/Test/MinFraudTest.php, tests/MaxMind/Test/MinFraud/ReportTransaction/ReportTransactionTest.php
Tests assert that listed methods and report() throw InvalidArgumentException when an array and named arguments are combined.
Check exception declarations
phpstan.neon, composer.json
PHPStan enables missing checked-exception checks and configures an exception for test files. Development-tool dependency constraints are updated.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 3a8c9

The changes document existing exception behavior, add tests, and update static-analysis configuration. No issue requiring a fix before merge is established.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 5 files. (3 skipped: 3 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: enabling PHPStan checked-exception analysis and the related exception documentation.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit reads the throws with care,
Then checks each method’s documented share.
The tests confirm the arguments clash,
While PHPStan checks the declarations stash.
The changelog notes what callers may meet,
And hops away on quiet feet.

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The exception annotations match the implementations, and the targeted PHPStan configuration passes lint validation.

Review effort: Balanced
Findings: None

What changed in this PR

Enables PHPStan checked-exception analysis and documents exceptions exposed by request-building APIs.

Changes:

  • Enables checked-exception validation with targeted exclusions.
  • Adds missing @throws declarations.
  • Records the documentation change in the changelog.
File Description
CHANGELOG.md Documents updated exception annotations.
phpstan.neon Enables checked-exception analysis and scoped ignores.
src/​MinFraud.php Documents request-building exceptions.
src/​MinFraud/​ReportTransaction.php Documents report() exceptions.
src/​MinFraud/​ServiceClient.php Documents validation-helper exceptions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

oschwald and others added 3 commits October 2, 2026 14:44
Turn on exceptions.check.missingCheckedExceptionInThrows so PHPStan
reports a checked exception that a method throws but does not declare
in its @throws tag. Tests are excluded, because PHPUnit handles any
exception a test throws.

Configure Error and LogicException as unchecked. They signal programmer
errors, such as mixing the $values array with named arguments. Setting
them explicitly also makes the result independent of the PHPStan
version, because older 2.2 releases treat Error as checked by default.

Declare the InvalidInputException that the request methods and their
private validation helpers throw when input validation is enabled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The with*() methods and ReportTransaction::report() throw
InvalidArgumentException if $values is non-empty and named arguments
are provided. No test covered this.

Also add withDevice to the withMethods data provider, so the unknown-key
test covers it too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
composer.lock is not committed, so the require-dev constraints alone
decide which tool versions CI and developers install. PHPStan and
PHP_CodeSniffer used "*", which allows any version, including a future
major release that changes behavior. php-cs-fixer allowed any 3.x
release.

Require the major and minor versions that CI installs now: PHPStan 2.2,
PHP_CodeSniffer 4.0, and php-cs-fixer 3.95.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oschwald
oschwald force-pushed the greg/phpstan-checked-exceptions branch from 5253e38 to 3a8c923 Compare October 2, 2026 14:44
Copilot AI balanced review requested due to automatic review settings October 2, 2026 14:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants