Skip to content

fix: stop relying on the deprecated ReflectionType::__toString() - #21

Merged
teofanis merged 2 commits into
teofanis:mainfrom
alright:fix/reflection-type-to-string
Aug 26, 2026
Merged

fix: stop relying on the deprecated ReflectionType::__toString()#21
teofanis merged 2 commits into
teofanis:mainfrom
alright:fix/reflection-type-to-string

Conversation

@alright

@alright alright commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What / Why

PHPStan currently fails on main. The last green run was June 26; since composer.lock is gitignored, CI resolves fresh PHPStan/larastan on every run, and newer releases flag two call sites:

src/Conditions/HasMethod.php:57    Casting class ReflectionType to string is deprecated.
src/Conditions/HasProperty.php:52  Casting class ReflectionType to string is deprecated.
src/Commands/ShowCommand.php:21    Call to function is_string() with non-falsy-string will always evaluate to true.

They're right on both counts. ReflectionType::__toString() has been deprecated since PHP 8.0 — it emits no runtime notice on 8.4 or 8.5 (I checked), but it's a documented deprecation sitting on the package's only type-matching path.

This PR makes composer analyse green again. It is split out from #20 (Laravel 13 / PHP 8.5) so neither diff carries the other's changes — #20 goes green once this lands.

How

Support\TypeName reproduces PHP's own rendering from the concrete ReflectionType subclasses rather than calling the deprecated method:

shape rendered as
named string, void
nullable named ?int — but never for null or mixed, which PHP leaves bare
union `string
nullable union `string
intersection A&B
DNF `(A&B)
absent ''

HasMethod and HasProperty now call it. Since TypeName::from(null) returns '' — exactly what both sites produced for an absent type — the null guards fall away:

-$type = $m->getReturnType();
-$actual = $type ? ltrim((string) $type, '\\') : '';
+$actual = ltrim(TypeName::from($m->getReturnType()), '\\');

ShowCommand: $type && is_string($type)is_string($type) && $type. The truthiness check was narrowing $type to a non-falsy string before is_string() ran. Semantics are identical, "0" included — it's falsy and still takes the else branch.

Screenshots / Output (if applicable)

Behaviour is unchanged. TypeName::from() was diffed against (string) $type across 18 type declarations covering every shape in the table, on both PHP 8.4 and 8.5:

compared 18 type declarations, 0 mismatches
$ vendor/bin/phpstan analyse --memory-limit=2G
 [OK] No errors

$ vendor/bin/pest
 Tests:  53 passed (192 assertions)      # 47 existing + 6 new

$ vendor/bin/pest --type-coverage --min=100
 Total: 100.0 %

Type of change

  • Bug fix
  • New feature
  • Refactor / internal cleanup
  • Docs only
  • Tests only
  • Chore (CI, build, tooling)

Checklist

  • Tests added/updated and pass locally (composer test) — tests/Unit/TypeNameTest.php, 6 cases
  • Code formatted (./vendor/bin/pint) — every touched file passes pint --test
  • No writes to vendor/composer/autoload_classmap.php (tests use hookpress.composer.classmap_path)
  • README / docs updated (if user-facing) — n/a, no user-facing change
  • Backwards compatible (or breaking changes clearly documented)

Notes for reviewers

  • The new fixtures are deliberately not in the fake classmap, so discovery can't pick them up and no existing expectation shifts.
  • DNF types are PHP 8.2+, so DnfShowcase lives in its own fixture file and its test is ->skip(PHP_VERSION_ID < 80200). Below 8.2 the class is never referenced, so the file is never autoloaded or parsed — the suite still runs on the 8.1 floor CONTRIBUTING sets.
  • I committed with --no-verify, for two reasons worth knowing about:
    • composer run rector can't run on main at all — strictBooleans: was removed from Rector's withPreparedSets() upstream, so rector.php throws Unknown named parameter $strictBooleans. Fixed in feat: add support for Laravel 13 and PHP 8.5 #20; happy to split that out if you'd rather have it here.
    • A current Pint reformats 18 unrelated files (fully_qualified_strict_types now applies more broadly). That's independent of this change and belongs in its own chore(style) commit.

`HasMethod` and `HasProperty` cast a `ReflectionType` to string to compare
it against the configured type. `ReflectionType::__toString()` has been
deprecated since PHP 8.0, and current PHPStan flags both call sites.

Add `Support\TypeName`, which reproduces PHP's own rendering from the
concrete reflection subclasses instead:

- named types, with the `?` prefix that PHP omits for `null` and `mixed`
- union types, including the `null` member of a nullable union
- intersection types
- DNF types, where the nested intersection is parenthesised: `(A&B)|C`

Behaviour is unchanged. `TypeName::from()` was diffed against `(string)
$type` across 18 type declarations covering every shape above, on PHP 8.4
and 8.5, with no mismatches.

`TypeName::from(null)` returns `''`, which is what both call sites did
for an absent type, so the null guards are no longer needed.

The new fixtures are deliberately absent from the fake classmap, so
discovery does not pick them up and existing expectations are untouched.
DNF types are PHP 8.2+, so that fixture lives in its own file and its
test is skipped below 8.2 — the file is then never autoloaded or parsed.
`$type && is_string($type)` narrows `$type` to a non-falsy string on the
truthiness check, so PHPStan reports the `is_string()` that follows as
always true. Swapping the two operands puts the type check first, where
it is still meaningful.

Semantics are identical, truthiness included: the argument `"0"` is
falsy and still takes the else branch.
@alright
alright force-pushed the fix/reflection-type-to-string branch 2 times, most recently from deb830b to d8ea640 Compare August 25, 2026 13:36

@teofanis teofanis left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you @alright !

@teofanis
teofanis merged commit f5db086 into teofanis:main Aug 26, 2026
1 check passed
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