Skip to content

Commit bf399a8

Browse files
committed
A Stringable object passed to a string parameter is checked against
the file's `strict_types` setting
1 parent c919f8d commit bf399a8

5 files changed

Lines changed: 31 additions & 45 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
134134
- **`assertInstanceOf()` on a mock leaves an intersection, not a choice.** A mock really is both the interface it was built as and the class it stands in for, so `assertInstanceOf(MethodNode::class, $mock)` on a `MockObject` leaves a `MethodNode&MockObject`. It was recorded as `MockObject|MethodNode` instead, which satisfies neither half, so returning the value from a method declared to return the intersection was reported as a type error. A subject that is *already* an intersection now narrows within it as well, so `(FunctionNode|MethodNode)&MockObject` proven to be a `MethodNode` becomes `MethodNode&MockObject` rather than staying as it was.
135135
- **An argument is not checked against a `@template` only it could have bound.** Comparing an argument to a parameter type that was substituted from that same argument is circular, and PHPantom already stood the check down where the template had a single binding site. Laravel's `travelTo` names `TDate` in both `$date` and its optional `$callback`, and a call passing only the date still binds `TDate` from that one argument — but the second site's existence was enough to re-enable the check, so `$this->travelTo(Carbon::create(2024))` was reported as expecting a `Carbon` and getting a `?Carbon`, contradicting the callee's own `@template TDate of …|null` bound. What counts is now the binding sites the caller actually filled.
136136
- **A `@phpstan-assert-if-true` promise about the receiver's own members is kept.** PHPStan's `Scope::isInTrait()` is annotated `@phpstan-assert-if-true !null $this->getTraitReflection()`, and a tag whose subject is a member of the receiver rather than a parameter was ignored outright, so the paired getter still read as nullable inside the guard. Those now narrow the member as read through the variable the call was written on. A `!null` promise about a plain parameter was dropped for a related reason (the tag names no class, so the class-based narrowing had nothing to rule out) and is now applied as the matching `is_*()` guard would apply it. PHPStan leaves the identical `isInClass()` bare, so that pairing is supplied for it: extensions are written against it regardless.
137+
- **A `Stringable` object passed to a `string` parameter is checked against the file's `strict_types` setting.** PHP only converts a `Stringable` object to a string automatically outside `declare(strict_types=1)`; under strict types the same call throws a `TypeError`. Every neighbouring type-juggling rule (int/float to string, numeric-string to int/float) already read the file's `strict_types` flag, but the `Stringable` rule was accepting the object either way, so a class relying on `__toString()` under strict types went unreported.
137138
- **A fully-qualified call to an array builtin gets the same answer as an unqualified one.** Writing `\array_sum($counts)` instead of `array_sum($counts)` disabled every one of the rules that read a builtin's return type off the array it was handed, because the rules were looked up under the bare name while the call arrived carrying its leading separator. `\array_sum()` on a list of integers went back to `int|float`, `\array_pop()` on a list of objects lost the object, and the same for the whole family. The separator is now stripped before the lookup, so the fully-qualified spelling that is house style in a good deal of library code behaves like the unqualified one.
138139
- **`array_chunk()` reports the chunks it makes, not the values it groups.** It was grouped with the builtins that rearrange an array's entries, all of which hand back the element type they were given, and it is the one that adds a level of nesting instead. `foreach (array_chunk($ids, 500) as $chunk)` gave `$chunk` a single ID rather than the batch of them, so passing it anywhere expecting an array was reported as a type error. Each chunk is now an array of the input's elements, renumbered from zero unless `$preserve_keys` asks for the original keys back.
139140
- **`max()` and `min()` answer with the values they compare.** Both were `mixed` for every call, which accepts anything: `takesInt(max("a", "b"))` and `takesInt(max($strings))` both passed unchecked. A single iterable argument now reports one of its elements, and comparing several values reports one of those.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ contributor even though it's short.
4242
| B81 | [Foreach element extraction widens `false` to `bool`](todo/bugs.md#b81-foreach-element-extraction-widens-false-to-bool) | Low-Medium | Medium |
4343
| B76 | [Blade variables typed from a component class are immune to condition narrowing](todo/bugs.md#b76-blade-variables-typed-from-a-component-class-are-immune-to-condition-narrowing) | Medium | Medium-High |
4444
| B77 | [A foreach over a proven non-empty array still merges the zero-iteration path](todo/bugs.md#b77-a-foreach-over-a-proven-non-empty-array-still-merges-the-zero-iteration-path) | Medium | Medium-High |
45-
| B85 | [A `Stringable` object is accepted for a `string` parameter under `strict_types=1`](todo/bugs.md#b85-a-stringable-object-is-accepted-for-a-string-parameter-under-strict_types1) | Low-Medium | Medium |
4645
| B75 | [A dim-write to the foreach value variable leaks through the loop back-edge](todo/bugs.md#b75-a-dim-write-to-the-foreach-value-variable-leaks-through-the-loop-back-edge) | Medium | High |
4746
| | **Release 0.10.0** | | |
4847

docs/todo/bugs.md

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -30,46 +30,7 @@ No outstanding items.
3030

3131
## Type comparison
3232

33-
### B85. A `Stringable` object is accepted for a `string` parameter under `strict_types=1`
34-
35-
**Impact: Low-Medium · Complexity: Medium**
36-
37-
```php
38-
<?php
39-
declare(strict_types=1);
40-
41-
class Name
42-
{
43-
public function __toString(): string { return 'x'; }
44-
}
45-
46-
function takesString(string $value): void {}
47-
48-
function run(Name $name): void {
49-
takesString($name); // not reported; PHP throws a TypeError here
50-
}
51-
```
52-
53-
PHP converts a `Stringable` object to a string only in weak mode. The
54-
calling file's `declare(strict_types=1)` turns the same call into a
55-
`TypeError`, and `is_type_compatible` accepts it either way: the
56-
Stringable rule is the one type-juggling rule in
57-
`src/diagnostics/type_errors/compatibility.rs` that does not consult
58-
the `strict_types` flag its neighbours (int/float → string,
59-
numeric-string → int/float, `numeric` → int/float) all check.
60-
61-
This is also why the rule cannot move down into
62-
`class_lookup::is_subtype_of_typed` with the other sound facts: it is
63-
not a subtype relationship, it is a coercion that depends on a
64-
per-file setting the core engine has no parameter for. Found while
65-
auditing those escape hatches; no sample-project site currently hits
66-
it.
67-
68-
**Fix:** gate the Stringable acceptance on `!strict_types`, the way
69-
the neighbouring juggling rules already are. Verify against the
70-
sample projects first: a file that declares `strict_types=1` and
71-
leans on `__toString()` would start reporting, and those reports are
72-
correct.
33+
No outstanding items.
7334

7435
## Standard-library return types
7536

src/diagnostics/type_errors/compatibility.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -520,10 +520,13 @@ pub(crate) fn is_type_compatible(
520520

521521
// ── Stringable objects accepted as string ────────────────────
522522
// PHP calls __toString() on Stringable objects when a string is
523-
// expected. We only accept objects whose class implements
524-
// \Stringable or declares __toString(). For bare `object` types
525-
// (no class name) we stay permissive since we can't check.
526-
if param_type.is_string_type() && arg_type.is_object_like() {
523+
// expected, but only outside strict_types: under
524+
// declare(strict_types=1) passing a Stringable object where a
525+
// `string` parameter is expected is a TypeError. We only accept
526+
// objects whose class implements \Stringable or declares
527+
// __toString(). For bare `object` types (no class name) we stay
528+
// permissive since we can't check.
529+
if !strict_types && param_type.is_string_type() && arg_type.is_object_like() {
527530
if let Some(class_name) = arg_type.base_name() {
528531
if let Some(cls) = class_loader(class_name) {
529532
let merged = crate::virtual_members::resolve_class_fully_maybe_cached(

tests/integration/diagnostics_type_errors.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2117,6 +2117,28 @@ function test(): void {
21172117
);
21182118
}
21192119

2120+
#[test]
2121+
fn type_error_for_stringable_to_string_under_strict_types() {
2122+
let php = r#"<?php
2123+
declare(strict_types=1);
2124+
2125+
class Name {
2126+
public function __toString(): string { return 'x'; }
2127+
}
2128+
2129+
function takes_string(string $value): void {}
2130+
2131+
function test(Name $name): void {
2132+
takes_string($name);
2133+
}
2134+
"#;
2135+
let diags = collect(php);
2136+
assert!(
2137+
has_type_error(&diags),
2138+
"Expected type error for Stringable object passed to string under strict_types=1, got: {diags:?}"
2139+
);
2140+
}
2141+
21202142
// ═══════════════════════════════════════════════════════════════════════════
21212143
// New rules: PHP type juggling
21222144
// ═══════════════════════════════════════════════════════════════════════════

0 commit comments

Comments
 (0)