Skip to content

Commit d816150

Browse files
committed
fix(diagnostics): narrow literal types through compound expressions
The literal narrowing pass in the diagnostic path only handled direct Expression::Literal nodes, so compound expressions like ternaries ($flag ? 'asc' : 'desc'), null-coalesce, match, and parenthesized wrappers resolved to the widened base type (e.g. string) instead of the precise literal union ('asc'|'desc'). Extract the inline narrowing logic into a recursive narrow_literal_type() helper that walks the complete set of PHP expression forms whose result is one of N sub-expression values: ternary, null-coalesce, match, and parenthesized wrappers. This is a closed set — no other PHP expressions select among sub-values. When any leaf is a variable or non-literal expression, the function returns None and the caller keeps the widened type from the general resolver, preserving the existing conservative behaviour. Closes #180
1 parent 579bce9 commit d816150

2 files changed

Lines changed: 88 additions & 34 deletions

File tree

src/diagnostics/type_errors/mod.rs

Lines changed: 70 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,75 @@ fn extract_array_string_literals(expr: &Expression<'_>) -> Vec<(String, usize, u
127127
literals
128128
}
129129

130+
/// Narrow an expression to its precise literal type for diagnostics.
131+
///
132+
/// The general expression resolver widens literals to base types
133+
/// (`string`, `int`, `float`) for variable and array-shape tracking.
134+
/// This function recovers precise literal types in the diagnostic
135+
/// path by walking the expression tree.
136+
///
137+
/// It handles every PHP expression form that selects among
138+
/// sub-expressions at runtime — ternary, null-coalesce, match,
139+
/// and parenthesised wrappers — plus scalar literals and negated
140+
/// numeric literals. This is a closed set: these are the only
141+
/// expression forms whose result is "one of N sub-expression values."
142+
///
143+
/// Returns `Some(literal_type)` when every leaf is a narrowable
144+
/// scalar literal. Returns `None` when any leaf is a variable or
145+
/// other non-literal, letting the caller keep the widened type.
146+
fn narrow_literal_type(expr: &Expression<'_>) -> Option<PhpType> {
147+
match expr {
148+
Expression::Literal(Literal::String(s)) => {
149+
Some(PhpType::literal_string_raw(bytes_to_str(s.raw).to_string()))
150+
}
151+
Expression::Literal(Literal::Integer(i)) => {
152+
i.value.map(|value| PhpType::literal_int(value.to_string()))
153+
}
154+
Expression::Literal(Literal::Float(f)) => {
155+
Some(PhpType::literal_float(bytes_to_str(f.raw).to_string()))
156+
}
157+
Expression::UnaryPrefix(unary)
158+
if matches!(
159+
unary.operator,
160+
mago_syntax::cst::unary::UnaryPrefixOperator::Negation(_)
161+
) =>
162+
{
163+
match unary.operand {
164+
Expression::Literal(Literal::Integer(i)) => i
165+
.value
166+
.map(|value| PhpType::literal_int(format!("-{value}"))),
167+
Expression::Literal(Literal::Float(f)) => {
168+
Some(PhpType::literal_float(format!("-{}", bytes_to_str(f.raw))))
169+
}
170+
_ => None,
171+
}
172+
}
173+
Expression::Conditional(cond) => {
174+
let then_expr = cond.then.unwrap_or(cond.condition);
175+
let then_ty = narrow_literal_type(then_expr)?;
176+
let else_ty = narrow_literal_type(cond.r#else)?;
177+
Some(PhpType::union(vec![then_ty, else_ty]))
178+
}
179+
Expression::Binary(binary) if binary.operator.is_null_coalesce() => {
180+
let lhs = narrow_literal_type(binary.lhs)?;
181+
let rhs = narrow_literal_type(binary.rhs)?;
182+
Some(PhpType::union(vec![lhs, rhs]))
183+
}
184+
Expression::Match(match_expr) => {
185+
let mut types = Vec::new();
186+
for arm in match_expr.arms.iter() {
187+
types.push(narrow_literal_type(arm.expression())?);
188+
}
189+
if types.is_empty() {
190+
return None;
191+
}
192+
Some(PhpType::union(types))
193+
}
194+
Expression::Parenthesized(p) => narrow_literal_type(p.expression),
195+
_ => None,
196+
}
197+
}
198+
130199
/// Extract the model name from a `model-property<Model>` type that
131200
/// appears as a generic argument of an array/list type.
132201
fn extract_model_property_from_array_type(ty: &PhpType) -> Option<String> {
@@ -362,40 +431,7 @@ impl Backend {
362431
// literal unions (`1|2|3`). The raw text would not
363432
// parse back into a number and would be flagged as
364433
// an incompatible argument.
365-
let ty = match arg_expr {
366-
Expression::Literal(Literal::String(s)) => {
367-
PhpType::literal_string_raw(bytes_to_str(s.raw).to_string())
368-
}
369-
Expression::Literal(Literal::Integer(i)) => match i.value {
370-
Some(value) => PhpType::literal_int(value.to_string()),
371-
None => ty,
372-
},
373-
Expression::Literal(Literal::Float(f)) => {
374-
PhpType::literal_float(bytes_to_str(f.raw).to_string())
375-
}
376-
// Negative numeric literals (`-1`, `-1.5`) parse
377-
// as a unary negation wrapping the literal, not
378-
// as a single `Literal` node, so they need their
379-
// own case to be narrowed the same way.
380-
Expression::UnaryPrefix(unary)
381-
if matches!(
382-
unary.operator,
383-
mago_syntax::cst::unary::UnaryPrefixOperator::Negation(_)
384-
) =>
385-
{
386-
match unary.operand {
387-
Expression::Literal(Literal::Integer(i)) => match i.value {
388-
Some(value) => PhpType::literal_int(format!("-{value}")),
389-
None => ty,
390-
},
391-
Expression::Literal(Literal::Float(f)) => {
392-
PhpType::literal_float(format!("-{}", bytes_to_str(f.raw)))
393-
}
394-
_ => ty,
395-
}
396-
}
397-
_ => ty,
398-
};
434+
let ty = narrow_literal_type(arg_expr).unwrap_or(ty);
399435
// Resolve any short class names in the arg type
400436
// to FQN via the class loader. Variable
401437
// resolution may return raw docblock names

tests/integration/diagnostics_type_errors.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4636,6 +4636,24 @@ function test(): void {
46364636
);
46374637
}
46384638

4639+
#[test]
4640+
fn no_false_positive_for_ternary_with_string_literal_branches() {
4641+
let php = r#"<?php
4642+
/** @param 'asc'|'desc' $direction */
4643+
function orderBy(string $column, string $direction): void {}
4644+
4645+
function test(bool $flag): void {
4646+
orderBy('id', $flag ? 'asc' : 'desc');
4647+
}
4648+
"#;
4649+
let diags = collect(php);
4650+
let msgs = type_error_messages(&diags);
4651+
assert!(
4652+
msgs.is_empty(),
4653+
"Ternary with literal branches should match 'asc'|'desc' (issue #180), got: {msgs:?}"
4654+
);
4655+
}
4656+
46394657
#[test]
46404658
fn no_false_positive_for_int_literal_matching_literal_type() {
46414659
let php = r#"<?php

0 commit comments

Comments
 (0)