Skip to content

Commit dea901e

Browse files
committed
An omitted argument reads a constant table under the key its own default
names
1 parent a9eb166 commit dea901e

7 files changed

Lines changed: 151 additions & 66 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
125125
- **An array literal keeps the values it was written with.** `[1, 1.5, '123']` says exactly what it holds, but the values were widened to `int|float|string` the moment they were stored, so reading an entry back could not be proven to be anything the individual values were. Handing `$values[$key]` to a parameter or return type of `numeric` was reported, even though every entry of the array is numeric on its own, and the same held for a literal key: `$values[2]` read as `string` rather than as the numeric `'123'` written at that position. The values a literal names now survive into its type, so a read off it, a `foreach` over it, and an inferred `@return` all see them, and a key that is only known at runtime resolves to the set of entries the array actually has. Widening now happens where the array is changed instead: a push or a keyed write says the array is being built up rather than written out, so the value arriving there stands in for however many more follow. An alternative already covered by a broader sibling is folded away, so a list mixing a plain `string` with two string literals stays `list<string>`, and a literal naming more distinct values than a set of alternatives is worth reasoning about falls back to the base types.
126126
- **A constant table constrains a plain signature too.** `@param key-of<ID_TABLE>` says a parameter takes one of the table's keys, and `@return value-of<ID_TABLE>` says the result is one of its values. PHPantom only looked behind the constant's name while working out a call's `@template` bindings, which a function that declares no `@template` never does, so both tags widened to whatever a key or a value could be in general: `acceptsKey('nope')` went unreported, and a return that can only ever be `int|string` was read as `mixed`. The constant is now read wherever a declared parameter or return type names one, so an untemplated function or method is held to the table's own keys and hands back the table's own values. The declaration reads the same way from inside the body: the parameter holds the keys the table has, so hover names them and passing one on is judged against them, and a `@return` naming the table is held to what the table holds, so returning a key the table does not have is reported where it is written. The `Class::TABLE` and `self::TABLE` spellings read the same way, and a constant that cannot be reached, or whose value is not an array literal, still widens rather than being guessed at.
127127
- **A lookup into a constant table reads as the entry its key names.** A constant holding an array literal is the ordinary way to write a table of settings, and a function that reads one out of it can say so: `@template T of key-of<ID_TABLE>` with `@return ID_TABLE[T]` names the value under whichever key the caller passed. PHPantom read neither tag, because the docblock only ever sees the constant's *name* and nothing looked behind it, so the declaration's own `int|string` stood for every call and `takesInt(lookUp('immutable'))` was reported for passing a `string`. The constant's initializer is now read where a type operator asks for it, so each call resolves to its own entry: hover names it, argument checks judge against it, and a key the table does not hold is still rejected. The `Class::TABLE` spelling reads the same way, and a constant whose value is not an array literal is left alone rather than guessed at.
128+
- **An omitted argument reads a constant table under the key its own default names.** `@template T of key-of<ID_TABLE>` with `@return ID_TABLE[T]` resolves to a single entry at every call site that writes the key out, but a parameter carrying its own default (`function lookUp(string $type = 'immutable')`) bound nothing when the caller left the argument off, so `lookUp()` fell back to the whole table's value union and `takesInt(lookUp())` was reported for passing a string the call can never return. A default value is as known at the declaration site as an argument is at the call site, so it now binds the template the same way: `lookUp()` resolves exactly as `lookUp('immutable')` does, for a method as much as for a function.
128129
- **A `for` loop's update clause carries its type into the next iteration.** `for ($node = $head; $node !== null; $node = $node->next)` is how a linked list is walked by hand, and the reassignment in the update clause counted for nothing. The clause was read far enough to hover and navigate the variables in it, but the type it produced was never fed back into the loop, so the body saw whatever the initialiser bound on the first trip through on every trip, and the variable kept that type after the loop as well, even where the update clause was the only thing that could have changed it. The clause now runs where PHP runs it, after the body and before the condition is checked again: the body sees the type the update produces alongside the one the initialiser bound, and a walk that ends because it ran out of nodes leaves the cursor holding `null` rather than the node it started from. The initialisers stay the one-time seed they are, so a variable the update clause retypes is no longer reset to its starting type.
129130
- **Arithmetic on a refined `int` no longer widens to `int|float`.** `int + int` is `int`, and that held for the bare spelling, but `strlen()`, `count()`, and most of the standard library's counting functions are declared with a refinement like `int<0,max>` rather than plain `int`, and accumulating one of those (`$length += strlen($text);`) read as an unrecognised operand and fell back to the conservative `int|float`, reported several lines away at the function's `return` rather than at the addition that caused it. Every `int` refinement (`positive-int`, `non-negative-int`, `int<min,max>`, and the rest) is now classified as `int` for arithmetic, and the same holds for `float`'s own refinements.
130131
- **A generic type argument is no longer coerced away.** A file that does not `declare(strict_types=1)` lets PHP convert a `'7'` handed to an `int` parameter, and that leniency was applied to type arguments as well, so a `Box<string>` where a `Box<int>` was required came back compatible and the class-hierarchy check that had the last word compared the two by name and called it a `Box`. Nothing is converted inside a value that is passed on whole, so the arguments are now compared without the juggling rules whatever the file declares, on arguments, returns, and property assignments alike. A class with type arguments that cannot be reconciled with the declared ones is also settled on those arguments rather than on its name, so a future reader of the type engine's own subtype check gets the same answer the diagnostics do. What we cannot prove still passes, unchanged: an argument our inference had to widen, one a `@template-contravariant` declaration means to be wider, a class named without its type arguments, and a name that could be the same class written unqualified.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ unlikely to move the needle for most users.
102102
| D5 | [External tool diagnostic suppression actions](todo/diagnostics.md#d5-external-tool-diagnostic-suppression-actions) | Low | Low |
103103
| D15 | [Unused parameter diagnostic](todo/diagnostics.md#d15-unused-parameter-diagnostic) | Low | Low |
104104
| | **[Bug Fixes](todo/bugs.md)** | | |
105-
| B97 | [`CONSTANT[T]` reads as the whole table when `T` comes from a parameter's default value](todo/bugs.md#b97-constantt-reads-as-the-whole-table-when-t-comes-from-a-parameters-default-value) | Low-Medium | Medium |
106105
| B96 | [A docblock `@param` type narrower than its native nullable type hint is not flagged](todo/bugs.md#b96-a-docblock-param-type-narrower-than-its-native-nullable-type-hint-is-not-flagged) | Low | Medium |
107106
| | **[Code Actions](todo/actions.md)** | | |
108107
| A40 | [Generate method from call](todo/actions.md#a40-generate-method-from-call) | Medium-High | Medium |

docs/todo/bugs.md

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -40,35 +40,3 @@ or an array type. Only Qodana flags either case in
4040
documented parameter/return type against its native type hint for
4141
compatibility, most likely reusing the existing type-compatibility check
4242
rather than a new one.
43-
44-
### B97. `CONSTANT[T]` reads as the whole table when `T` comes from a parameter's default value
45-
46-
**Impact: Low-Medium · Effort: Medium**
47-
48-
```php
49-
const ID_TABLE = ['immutable' => 1, 'mutable' => 'two'];
50-
51-
/**
52-
* @template T of key-of<ID_TABLE>
53-
* @param T $type
54-
* @return ID_TABLE[T]
55-
*/
56-
function lookUp(string $type = 'immutable'): int|string {
57-
return ID_TABLE[$type];
58-
}
59-
60-
takesInt(lookUp('immutable')); // passes: correctly reads as int
61-
takesInt(lookUp()); // reported: got 1|'two' — should also read as int
62-
```
63-
64-
Per-key resolution of `CONSTANT[T]` (a template bound to `key-of<CONSTANT>`)
65-
now works correctly when the call site passes the key as an explicit literal
66-
argument, but falls back to the whole table's value union specifically when
67-
the caller omits the argument and the template binds from the parameter's
68-
*default* value instead. The default value (`'immutable'`) is known at
69-
the declaration site the same way an explicit argument is known at the call
70-
site, so `lookUp()` should resolve identically to `lookUp('immutable')`.
71-
72-
**Fix:** wherever the explicit-argument case resolves `T` to the literal
73-
passed at the call site, apply the same resolution when the argument is
74-
omitted and a literal default value is available.

examples/demo.php

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -965,6 +965,11 @@ public function demo(): void
965965
$ink = scaffoldingToolDefault('ink'); // TOOL_DEFAULTS['ink'] → 'black'
966966
var_dump(strtoupper($ink));
967967

968+
// An omitted argument binds the template from the parameter's own
969+
// default, which is as known here as a key written at the call site.
970+
$fallback = scaffoldingDefaultToolSetting(); // TOOL_DEFAULTS['ink'] → 'black'
971+
var_dump(strtoupper($fallback));
972+
968973
// The class-constant spelling reads the same way.
969974
$retries = ScaffoldingLimits::lookUp('retries');
970975
var_dump($retries + 1); // LIMITS['retries'] → 3
@@ -3150,7 +3155,7 @@ public function returnType(): GtdResult { return new GtdResult(); }
31503155
* @return GtdResult Ctrl+Click GtdResult
31513156
* @throws GtdNotFoundException Ctrl+Click GtdNotFoundException
31523157
*/
3153-
public function docblockTypes($items) { return $items; }
3158+
public function docblockTypes($items) { return new GtdResult(); }
31543159

31553160
/**
31563161
* Callable types in docblocks. Ctrl+Click on any class name inside the
@@ -6691,6 +6696,16 @@ function scaffoldingToolDefault(string $setting): int|string
66916696
return TOOL_DEFAULTS[$setting];
66926697
}
66936698

6699+
/**
6700+
* @template T of key-of<TOOL_DEFAULTS>
6701+
* @param T $setting
6702+
* @return TOOL_DEFAULTS[T]
6703+
*/
6704+
function scaffoldingDefaultToolSetting(string $setting = 'ink'): int|string
6705+
{
6706+
return TOOL_DEFAULTS[$setting];
6707+
}
6708+
66946709
/** @param key-of<TOOL_DEFAULTS> $setting */
66956710
function scaffoldingToolSettingName(string $setting): string
66966711
{
@@ -7653,6 +7668,7 @@ function runDemoAssertions(): void
76537668
// ── Constant table read through a type operator ─────────────────────
76547669
assert(scaffoldingToolDefault('width') === 2, "TOOL_DEFAULTS['width'] really is the int 2");
76557670
assert(scaffoldingToolDefault('ink') === 'black', "TOOL_DEFAULTS['ink'] really is the string 'black'");
7671+
assert(scaffoldingDefaultToolSetting() === 'black', "an omitted argument really reads the entry its default names");
76567672
assert(ScaffoldingLimits::lookUp('retries') === 3, "LIMITS['retries'] really is the int 3");
76577673
assert(ScaffoldingLimits::lookUp('label') === 'off', "LIMITS['label'] really is the string 'off'");
76587674
assert(scaffoldingToolSettingName('ink') === 'ink', "'ink' really is one of TOOL_DEFAULTS' keys");

src/type_engine/call_resolution/template_subs.rs

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -82,37 +82,50 @@ impl Backend {
8282
.and_then(|p| p.type_hint.as_ref());
8383
let binding_mode = classify_template_binding(tpl_name, param_hint);
8484

85+
let tpl_bound = method.template_param_bounds.get(&atom(tpl_name));
86+
8587
let arg_text = match bound.get(param_idx).and_then(|o| o.as_deref()) {
8688
Some(text) => text,
8789
None => {
8890
let default_value = method
8991
.parameters
9092
.get(param_idx)
9193
.and_then(|p| p.default_value.as_deref());
92-
match &binding_mode {
93-
TemplateBindingMode::ClassStringInner => match default_value {
94-
Some(d) if !subs.contains_key(tpl_name.as_str()) => d,
95-
None => continue,
96-
_ => continue,
97-
},
98-
TemplateBindingMode::Direct => match default_value {
99-
Some(d)
100-
if !subs.contains_key(tpl_name.as_str())
101-
&& (d == "null" || d.ends_with("::class")) =>
102-
{
103-
d
104-
}
94+
// A template bounded by a type operator resolves
95+
// against the one literal it binds to, and an omitted
96+
// argument has such a literal whenever the parameter
97+
// declares a scalar default — known at the declaration
98+
// site exactly as an explicit argument is known at the
99+
// call site.
100+
match default_value {
101+
Some(d)
102+
if !subs.contains_key(tpl_name.as_str())
103+
&& type_operator_bound_literal(tpl_bound, d).is_some() =>
104+
{
105+
d
106+
}
107+
_ => match &binding_mode {
108+
TemplateBindingMode::ClassStringInner => match default_value {
109+
Some(d) if !subs.contains_key(tpl_name.as_str()) => d,
110+
None => continue,
111+
_ => continue,
112+
},
113+
TemplateBindingMode::Direct => match default_value {
114+
Some(d)
115+
if !subs.contains_key(tpl_name.as_str())
116+
&& (d == "null" || d.ends_with("::class")) =>
117+
{
118+
d
119+
}
120+
_ => continue,
121+
},
105122
_ => continue,
106123
},
107-
_ => continue,
108124
}
109125
}
110126
};
111127

112-
if let Some(literal) = type_operator_bound_literal(
113-
method.template_param_bounds.get(&atom(tpl_name)),
114-
arg_text,
115-
) {
128+
if let Some(literal) = type_operator_bound_literal(tpl_bound, arg_text) {
116129
crate::type_engine::variable::rhs_resolution::insert_or_union(
117130
&mut subs,
118131
tpl_name.to_string(),

src/type_engine/variable/rhs_resolution/calls.rs

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -86,27 +86,42 @@ pub(crate) fn build_function_template_subs(
8686
.parameters
8787
.get(param_idx)
8888
.and_then(|p| p.default_value.as_deref());
89+
let tpl_bound = func_info
90+
.template_param_bounds
91+
.get(&crate::atom::atom(tpl_name));
8992
let arg_text: &str = match provided_arg {
9093
Some(text) => text,
91-
None => match &binding_mode {
92-
TemplateBindingMode::ClassStringInner => match default_value {
93-
Some(d) => d,
94-
None => continue,
95-
},
96-
TemplateBindingMode::Direct => match default_value {
97-
Some(d) if d.ends_with("::class") => d,
94+
// A template bounded by a type operator resolves against the
95+
// one literal it binds to, and an omitted argument has such a
96+
// literal whenever the parameter declares a scalar default —
97+
// known at the declaration site exactly as an explicit
98+
// argument is known at the call site.
99+
None => match default_value {
100+
Some(d)
101+
if crate::type_engine::call_resolution::type_operator_bound_literal(
102+
tpl_bound, d,
103+
)
104+
.is_some() =>
105+
{
106+
d
107+
}
108+
_ => match &binding_mode {
109+
TemplateBindingMode::ClassStringInner => match default_value {
110+
Some(d) => d,
111+
None => continue,
112+
},
113+
TemplateBindingMode::Direct => match default_value {
114+
Some(d) if d.ends_with("::class") => d,
115+
_ => continue,
116+
},
98117
_ => continue,
99118
},
100-
_ => continue,
101119
},
102120
};
103121

104-
if let Some(literal) = crate::type_engine::call_resolution::type_operator_bound_literal(
105-
func_info
106-
.template_param_bounds
107-
.get(&crate::atom::atom(tpl_name)),
108-
arg_text,
109-
) {
122+
if let Some(literal) =
123+
crate::type_engine::call_resolution::type_operator_bound_literal(tpl_bound, arg_text)
124+
{
110125
insert_or_union(&mut subs, tpl_name.to_string(), literal);
111126
continue;
112127
}

0 commit comments

Comments
 (0)