Skip to content

Remove dead code in Profiler, add static analysis annotations#1

Closed
simon-mundy wants to merge 2 commits into
phpunit-test-coveragefrom
refactor/dead-code-cleanup
Closed

Remove dead code in Profiler, add static analysis annotations#1
simon-mundy wants to merge 2 commits into
phpunit-test-coveragefrom
refactor/dead-code-cleanup

Conversation

@simon-mundy

Copy link
Copy Markdown
Owner

Summary

Minimal src/-only changes that remove provably dead code and add static analysis annotations. No behavioural changes.

Changes

src/Adapter/Profiler/Profiler.php — Remove unreachable else-branch in profilerStart()

The method signature is string|StatementContainerInterface $target. The if handles StatementContainerInterface, the elseif handles string. PHP's union type enforcement means no other type can reach the method body — the else branch with its InvalidArgumentException throw is dead code. Removed the branch and cleaned up the now-unused InvalidArgumentException import and is_string() import.

src/Adapter/Driver/Pdo/AbstractPdo.php — Add @codeCoverageIgnore to checkEnvironment() throw

The throw fires when extension_loaded('PDO') returns false. But AbstractPdo itself requires the PDO extension to be loaded (it imports PDO classes). Any environment where this class is instantiable necessarily has PDO loaded, making the throw unreachable in practice.

src/ResultSet/AbstractResultSet.php — Add @phpstan-ignore for Traversable assignment

In initialize(), after the array and IteratorAggregate branches are handled, the remaining $dataSource is a Traversable that is necessarily an Iterator. PHPStan cannot narrow Traversable to Iterator here, so the ignore annotation is appropriate.

Test plan

  • ./vendor/bin/phpunit — all 1446 tests pass
  • ./vendor/bin/phpstan — 0 errors
  • ./vendor/bin/phpcs — no new violations
  • Verify no behavioural changes — only dead code removal and annotations

- Profiler::profilerStart(): remove unreachable else-branch. The union
  type `string|StatementContainerInterface` guarantees only those two
  types can be passed; the if/elseif handles both exhaustively, making
  the else+throw dead code. Clean up unused imports (InvalidArgumentException,
  is_string).

- AbstractPdo::checkEnvironment(): mark the throw inside
  `if (!extension_loaded('PDO'))` with @codeCoverageIgnore. PDO is a
  hard requirement — this branch cannot execute in any environment where
  the class is loadable.

- AbstractResultSet::initialize(): add @PHPStan-Ignore for Traversable
  assignment. After array and IteratorAggregate branches are handled, the
  remaining Traversable is necessarily an Iterator, but PHPStan cannot
  narrow it.
@simon-mundy
simon-mundy marked this pull request as draft March 25, 2026 09:49
Rename ~30 test methods across Adapter test suite to follow the
convention of naming tests after what they verify rather than which
method they call.

Pattern applied:
- Fluent/chaining tests: testFluent<Method>
- Getter tests: test<Method>Returns<What>
- Throw tests: test<Method>ThrowsOn<Condition>
- Delegation tests: test<Method>DelegatesToX

Simple getter/setter tests (testSetSql, testOffsetGet, etc.) left
as-is where the method name already describes the behaviour.
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.

1 participant