Skip to content

docs(mock-validity): a missing use in a callback signature passes the suite - #125

Merged
CybotTM merged 2 commits into
mainfrom
retro/missing-import-passes-the-suite
Aug 13, 2026
Merged

docs(mock-validity): a missing use in a callback signature passes the suite#125
CybotTM merged 2 commits into
mainfrom
retro/missing-import-passes-the-suite

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 13, 2026

Copy link
Copy Markdown
Member

mock-validity.md already covers a callback whose parameter list is stale. This adds the sibling failure, where the list is right and a parameter type is not imported.

PHP resolves a parameter's class type only when a value is checked against it, and null never is. So a ?SomeType $x = null in a willReturnCallback signature whose use is missing resolves to the current namespace, is never loaded, and the test is green — until production passes a real object, at which point the failure names a class nobody recognises.

PHPStan catches it (Parameter $run of anonymous function has invalid type …\Tests\Unit\…\AgentRunReference) and the suite structurally cannot. That makes it the concrete reason for the "run the analyser after writing tests" ordering already in quality-tools.md — a green runTests.sh -s unit is not evidence a new test file is type-correct.

Two habits included: copy a signature's use statements along with the signature, and treat every ?Type $x = null in a test double as unverified until something passes a non-null value.

Found 2026-08-13 in netresearch/t3x-nr-llm: two new tests passed with OK (2 tests, 3 assertions) while the import was missing.

… suite

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Copilot AI lite review requested due to automatic review settings August 13, 2026 13:06
github-actions[bot]
github-actions Bot previously approved these changes Aug 13, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation skill labels Aug 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

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.

The paragraph cited an ordering rule in quality-tools.md that does not exist
in that page. What IS verifiable there is that the recommended PHPStan config
lists Tests in paths; the ordering now stands on its own instead of on a
citation.

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
@sonarqubecloud

Copy link
Copy Markdown

@CybotTM

CybotTM commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

Self-review (Copilot quota-blocked all day; this is the review of record).

One finding, fixed here — and it is the failure this skill's own guidance warns about. The paragraph cited "the ordering rule in quality-tools.md: run the static analyser after writing tests". That rule is not in quality-tools.md. It is in my personal notes, and I cross-referenced it into a shared skill without opening the page.

What is verifiable there, and now what the text says: the recommended PHPStan configuration lists Tests in paths (with only Tests/Acceptance/_output/* excluded), so the analyser does see test files. The ordering argument now stands on its own instead of on a citation.

Refutations attempted on the technical claim:

  • PHP would still fail to load the class even for null. It does not. A nullable parameter short-circuits on null before any class resolution, which is exactly why the suite stays green.
  • declare(strict_types=1) changes this. It governs scalar coercion, not whether a class type is resolved.
  • PHPUnit's own mock generator would catch it. The unresolvable name is in the callback's signature, not in the mocked interface — nothing generates or reflects over it.
  • A variadic callback (recommended two sections above) sidesteps the whole thing. True, and worth knowing — but the section above recommends variadics for forward compatibility across versions, not as a way to avoid naming types. Naming them and importing them is still better; the addition says how to keep the two in step.

@CybotTM
CybotTM merged commit 1ae807a into main Aug 13, 2026
23 checks passed
@CybotTM
CybotTM deleted the retro/missing-import-passes-the-suite branch August 13, 2026 13:28
@CybotTM CybotTM mentioned this pull request Aug 13, 2026
CybotTM added a commit that referenced this pull request Aug 13, 2026
Minor release. Delta since v5.19.1: mock-validity — a missing use import
in a callback signature passes the suite (#125).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants