Skip to content

Commit 37bcf9a

Browse files
committed
feat: warn on type-incompatible match arm conditions
When a match expression's subject has a known scalar type, literal arm conditions of a different scalar type are flagged as unreachable since match uses strict comparison (===). For example, an int literal in a match against a string subject will never match. Uses the Walker trait to collect match expressions from the AST, then resolves the subject type via the forward walker. Handles union and nullable subject types conservatively: a literal is only flagged when it is incompatible with every branch of the union. Closes #200
1 parent 8265515 commit 37bcf9a

5 files changed

Lines changed: 460 additions & 0 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3434
- **Larastan `model-property<Model>` type validation and completion.** The `model-property<Model>` pseudo-type is now resolved against the model's known properties during argument type checking. String literals that do not match a declared or virtual property are flagged as type mismatches, while non-literal strings are accepted conservatively. Typing inside a string argument whose parameter is typed as `model-property<Model>` now offers completion of the model's property names. The type parser now also handles hyphenated generic pseudo-types that the PHPDoc type grammar does not recognise natively. Contributed by @calebdw.
3535
- **Workspace-wide diagnostics.** PHPantom now surfaces problems across the whole project, not just the files you have open. After startup and the full background index finish, diagnostics run in the background over every file and stream into the editor's problems panel as they're found, so issues in files you haven't opened yet are already visible when you navigate to them. Configured tools (PHPStan, PHPCS, Mago) also run once over the whole project afterwards, when the project has its own configuration file for that tool. Both passes are deliberately deferred until after startup so they never slow down the time it takes for the editor to become usable. Disable with `[diagnostics] workspace = false` (native pass) or `workspace-external = false` (external tools) in `.phpantom.toml`.
3636
- **Enum declaration diagnostics.** PHPantom now flags invalid enum declarations: backed enum cases missing a value, unit enum cases that have a value, backing types other than `int` or `string`, and duplicate backed values across cases. Contributed by @calebdw.
37+
- **Match arm type checking.** `match` expressions where a literal arm condition can never match the subject's type under strict comparison (`===`) now produce a warning. For example, an `int` literal in a match against a `string` subject is flagged as unreachable. Contributed by @calebdw.
3738
- **Incompatible `static` return type override diagnostic.** Overriding a method that returns `static` with a return type of `self` is now flagged as an error, matching PHP's fatal error at runtime. Contributed by @calebdw.
3839
- **Unimplemented trait abstract method diagnostics.** Concrete classes that use a trait with abstract methods without implementing them are now flagged as errors, matching PHP's fatal error at runtime. The "Implement missing methods" code action also offers to stub them. Contributed by @calebdw.
3940

Lines changed: 290 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,290 @@
1+
use std::collections::HashMap;
2+
3+
use mago_span::HasSpan;
4+
use mago_syntax::cst::control_flow::r#match::{Match, MatchArm};
5+
use mago_syntax::cst::expression::Expression;
6+
use mago_syntax::cst::literal::Literal;
7+
use mago_syntax::walker::Walker;
8+
9+
use tower_lsp::lsp_types::*;
10+
11+
use crate::Backend;
12+
use crate::parser::{with_parse_cache, with_parsed_program};
13+
use crate::php_type::PhpType;
14+
use crate::type_engine::resolver::{Loaders, VarResolutionCtx};
15+
use crate::type_engine::variable::foreach_resolution::resolve_expression_type;
16+
use crate::types::ClassInfo;
17+
18+
use super::helpers::{find_innermost_enclosing_class, make_diagnostic};
19+
20+
struct LiteralCondition {
21+
scalar_type: &'static str,
22+
start: usize,
23+
end: usize,
24+
}
25+
26+
struct MatchExprData {
27+
subject_offset: u32,
28+
conditions: Vec<LiteralCondition>,
29+
}
30+
31+
struct MatchArmIssue {
32+
start: usize,
33+
end: usize,
34+
literal_type: &'static str,
35+
subject_type: String,
36+
}
37+
38+
impl Backend {
39+
pub fn collect_match_type_diagnostics(
40+
&self,
41+
uri: &str,
42+
content: &str,
43+
out: &mut Vec<Diagnostic>,
44+
) {
45+
let file_ctx = self.file_context(uri);
46+
let _parse_guard = with_parse_cache(content);
47+
let class_loader = self.class_loader(&file_ctx);
48+
let function_loader_cl = self.function_loader(&file_ctx);
49+
let constant_loader_cl = self.constant_loader();
50+
let default_class = ClassInfo::default();
51+
52+
let matches: Vec<MatchExprData> =
53+
with_parsed_program(content, "match_type_diagnostics", |program, _| {
54+
let mut data = Vec::new();
55+
let walker = MatchCollector;
56+
for stmt in program.statements.iter() {
57+
walker.walk_statement(stmt, &mut data);
58+
}
59+
data
60+
});
61+
62+
if matches.is_empty() {
63+
return;
64+
}
65+
66+
let mut issues: Vec<MatchArmIssue> = Vec::new();
67+
68+
with_parsed_program(content, "match_type_resolve", |program, _| {
69+
for match_data in &matches {
70+
let enclosing =
71+
find_innermost_enclosing_class(&file_ctx.classes, match_data.subject_offset);
72+
let current_class = enclosing.unwrap_or(&default_class);
73+
74+
let config_resolver = |key: &str| self.resolve_config_type(key);
75+
let loaders = Loaders {
76+
function_loader: Some(&function_loader_cl),
77+
constant_loader: Some(&constant_loader_cl),
78+
config_resolver: Some(&config_resolver),
79+
};
80+
81+
let var_ctx = VarResolutionCtx {
82+
var_name: "",
83+
top_level_scope: None,
84+
current_class,
85+
all_classes: &file_ctx.classes,
86+
content,
87+
cursor_offset: match_data.subject_offset,
88+
class_loader: &class_loader,
89+
loaders,
90+
resolved_class_cache: Some(&self.resolved_class_cache),
91+
enclosing_return_type: None,
92+
branch_aware: true,
93+
match_arm_narrowing: HashMap::new(),
94+
scope_var_resolver: None,
95+
};
96+
97+
let subject_expr =
98+
find_expression_at_offset(program.statements.iter(), match_data.subject_offset);
99+
let subject_expr = match subject_expr {
100+
Some(e) => e,
101+
None => continue,
102+
};
103+
104+
let subject_type = match resolve_expression_type(subject_expr, &var_ctx) {
105+
Some(ty) => ty,
106+
None => continue,
107+
};
108+
109+
let subject_scalars = subject_scalar_types(&subject_type);
110+
if subject_scalars.is_empty() {
111+
continue;
112+
}
113+
114+
let subject_display = subject_type.to_string();
115+
116+
for cond in &match_data.conditions {
117+
if !types_compatible_strict(cond.scalar_type, &subject_scalars) {
118+
issues.push(MatchArmIssue {
119+
start: cond.start,
120+
end: cond.end,
121+
literal_type: cond.scalar_type,
122+
subject_type: subject_display.clone(),
123+
});
124+
}
125+
}
126+
}
127+
});
128+
129+
for issue in &issues {
130+
let range = match self.offset_range_to_lsp_range(uri, content, issue.start, issue.end) {
131+
Some(r) => r,
132+
None => continue,
133+
};
134+
out.push(make_diagnostic(
135+
range,
136+
DiagnosticSeverity::WARNING,
137+
"unreachable_match_arm",
138+
format!(
139+
"Match arm of type '{}' will never match subject of type '{}' (match uses ===)",
140+
issue.literal_type, issue.subject_type
141+
),
142+
));
143+
}
144+
}
145+
}
146+
147+
struct MatchCollector;
148+
149+
impl<'a, 'b> Walker<'a, 'b, Vec<MatchExprData>> for MatchCollector {
150+
fn walk_in_match(&self, match_expr: &'a Match<'b>, data: &mut Vec<MatchExprData>) {
151+
if match_expr.expression.is_true() {
152+
return;
153+
}
154+
155+
let subject_offset = match_expr.expression.span().start.offset;
156+
let mut conditions = Vec::new();
157+
158+
for arm in match_expr.arms.iter() {
159+
let arm_conditions = match arm {
160+
MatchArm::Expression(expr_arm) => &expr_arm.conditions,
161+
MatchArm::Default(_) => continue,
162+
};
163+
for condition in arm_conditions.iter() {
164+
if let Some(lc) = literal_scalar_type(condition) {
165+
conditions.push(lc);
166+
}
167+
}
168+
}
169+
170+
if !conditions.is_empty() {
171+
data.push(MatchExprData {
172+
subject_offset,
173+
conditions,
174+
});
175+
}
176+
}
177+
}
178+
179+
fn find_expression_at_offset<'a, 'b>(
180+
stmts: impl Iterator<Item = &'a mago_syntax::cst::statement::Statement<'b>>,
181+
offset: u32,
182+
) -> Option<&'a Expression<'b>>
183+
where
184+
'b: 'a,
185+
{
186+
struct MatchFinder {
187+
target_offset: u32,
188+
}
189+
190+
impl<'a, 'b> Walker<'a, 'b, Option<(*const Expression<'b>, std::marker::PhantomData<&'a ()>)>>
191+
for MatchFinder
192+
{
193+
fn walk_in_match(
194+
&self,
195+
match_expr: &'a Match<'b>,
196+
result: &mut Option<(*const Expression<'b>, std::marker::PhantomData<&'a ()>)>,
197+
) {
198+
if match_expr.expression.span().start.offset == self.target_offset {
199+
*result = Some((
200+
match_expr.expression as *const Expression<'b>,
201+
std::marker::PhantomData,
202+
));
203+
}
204+
}
205+
}
206+
207+
let finder = MatchFinder {
208+
target_offset: offset,
209+
};
210+
let mut result = None;
211+
for stmt in stmts {
212+
finder.walk_statement(stmt, &mut result);
213+
if result.is_some() {
214+
break;
215+
}
216+
}
217+
result.map(|(ptr, _)| unsafe { &*ptr })
218+
}
219+
220+
fn scalar_type_label(ty: &PhpType) -> Option<&'static str> {
221+
match ty {
222+
PhpType::Named(n) => {
223+
let s: &str = n;
224+
match s {
225+
"int" | "integer" => Some("int"),
226+
"string" => Some("string"),
227+
"float" | "double" => Some("float"),
228+
"bool" | "boolean" => Some("bool"),
229+
_ => None,
230+
}
231+
}
232+
_ => None,
233+
}
234+
}
235+
236+
fn subject_scalar_types(ty: &PhpType) -> Vec<&'static str> {
237+
match ty {
238+
PhpType::Union(members) => members.iter().filter_map(scalar_type_label).collect(),
239+
PhpType::Nullable(inner) => {
240+
let mut types = subject_scalar_types(inner);
241+
if !types.contains(&"null") {
242+
types.push("null");
243+
}
244+
types
245+
}
246+
other => scalar_type_label(other).into_iter().collect(),
247+
}
248+
}
249+
250+
fn literal_scalar_type(expr: &Expression<'_>) -> Option<LiteralCondition> {
251+
match expr {
252+
Expression::Literal(lit) => {
253+
let (ty, start, end) = match lit {
254+
Literal::Integer(i) => ("int", i.span.start.offset, i.span.end.offset),
255+
Literal::String(s) => ("string", s.span.start.offset, s.span.end.offset),
256+
Literal::Float(f) => ("float", f.span.start.offset, f.span.end.offset),
257+
Literal::True(k) | Literal::False(k) => {
258+
("bool", k.span.start.offset, k.span.end.offset)
259+
}
260+
Literal::Null(k) => ("null", k.span.start.offset, k.span.end.offset),
261+
};
262+
Some(LiteralCondition {
263+
scalar_type: ty,
264+
start: start as usize,
265+
end: end as usize,
266+
})
267+
}
268+
Expression::UnaryPrefix(prefix) => match prefix.operand {
269+
Expression::Literal(Literal::Integer(i)) => Some(LiteralCondition {
270+
scalar_type: "int",
271+
start: prefix.operator.span().start.offset as usize,
272+
end: i.span.end.offset as usize,
273+
}),
274+
Expression::Literal(Literal::Float(f)) => Some(LiteralCondition {
275+
scalar_type: "float",
276+
start: prefix.operator.span().start.offset as usize,
277+
end: f.span.end.offset as usize,
278+
}),
279+
_ => None,
280+
},
281+
_ => None,
282+
}
283+
}
284+
285+
fn types_compatible_strict(literal_type: &str, subject_types: &[&str]) -> bool {
286+
if subject_types.is_empty() {
287+
return true;
288+
}
289+
subject_types.contains(&literal_type)
290+
}

src/diagnostics/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ pub(crate) mod ignore_rules;
185185
mod implementation_errors;
186186
mod incompatible_override;
187187
mod invalid_class_kind;
188+
mod match_type_errors;
188189
pub(crate) mod namespace_mismatch;
189190
mod property_type_errors;
190191
mod pull;
@@ -324,6 +325,7 @@ impl Backend {
324325
self.collect_deprecated_diagnostics_with_context(ctx, uri_str, content, out);
325326
}
326327
self.collect_undefined_variable_diagnostics(uri_str, content, out);
328+
self.collect_match_type_diagnostics(uri_str, content, out);
327329
if let Some(ctx) = &file_ctx {
328330
self.collect_invalid_class_kind_diagnostics_with_context(ctx, uri_str, content, out);
329331
self.collect_enum_error_diagnostics_with_context(ctx, uri_str, content, out);

0 commit comments

Comments
 (0)