Skip to content

Commit 49a86a0

Browse files
committed
Eliminate false positives for properties narrowed via instanceof checks
1 parent ed7b308 commit 49a86a0

6 files changed

Lines changed: 72 additions & 8 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2929
- **Magic `__get` property access.** Accessing undefined properties on objects with a `__get` method now resolves to the method's declared return type, even when `__get` has no template parameters (e.g. `SimpleXMLElement::$child` resolves to `SimpleXMLElement`).
3030
- **Magic `__call` method return type.** Calling undefined methods on objects with a `__call` method now resolves to `__call`'s declared return type for hover and type inference.
3131
- **SoapClient arbitrary methods.** Calling any method on `SoapClient` (or subclasses) no longer produces false-positive "unknown member" diagnostics. SoapClient is a SOAP proxy where any method name is valid at runtime.
32+
- **Property narrowing in type error diagnostics.** Properties narrowed via `instanceof` checks (e.g. `if ($this->service instanceof MockInterface)`) now use the narrowed type when checking argument compatibility, eliminating false positives in test code that uses Mockery or similar patterns.
3233
- **`self::class` and `static::class` in template arguments.** Passing `self::class` or `static::class` to a method with `@template T` + `@param class-string<T>` now correctly resolves T to the enclosing class instead of failing to resolve or resolving to the called class.
3334
- **Bare array return values no longer flagged against typed array parameters.** When a method returns `array` (without generic parameters), passing the result to a parameter expecting `array<T>` no longer triggers a false type mismatch error.
3435
- **Foreach over `::class` literal arrays resolves static access.** Variables assigned from `foreach ([A::class, B::class] as $className)` are now recognized as class-strings, so `$className::CONST` and `$className::method()` no longer produce unresolved-member-access diagnostics.

src/completion/resolver.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1359,7 +1359,7 @@ pub(in crate::completion) fn resolve_static_owner_class(
13591359
/// [`super::types::narrowing`] already support property paths via
13601360
/// [`super::types::narrowing::expr_to_subject_key`], so no changes
13611361
/// to those functions are required.
1362-
fn apply_property_narrowing(
1362+
pub(crate) fn apply_property_narrowing(
13631363
property_path: &str,
13641364
current_class: &ClassInfo,
13651365
rctx: &ResolutionCtx<'_>,

src/completion/variable/class_string_resolution.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,7 @@ fn walk_class_string_assignments<'b>(
174174
{
175175
// Extract class names from the iterated expression
176176
// (e.g. `[Page::class, CustomPage::class]`).
177-
let class_names =
178-
extract_class_string_names_from_array(foreach.expression);
177+
let class_names = extract_class_string_names_from_array(foreach.expression);
179178
if !class_names.is_empty() {
180179
results.clear();
181180
for cn in class_names {
@@ -187,9 +186,7 @@ fn walk_class_string_assignments<'b>(
187186
cn
188187
};
189188
let lookup = short_name(&resolved_name);
190-
if let Some(cls) =
191-
ctx.all_classes.iter().find(|c| c.name == lookup)
192-
{
189+
if let Some(cls) = ctx.all_classes.iter().find(|c| c.name == lookup) {
193190
ClassInfo::push_unique(results, ClassInfo::clone(cls));
194191
} else if let Some(cls) = (ctx.class_loader)(&resolved_name) {
195192
ClassInfo::push_unique(results, Arc::unwrap_or_clone(cls));

src/completion/variable/rhs_resolution.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,40 @@ fn resolve_rhs_expression_inner<'b>(
314314
return from_scope;
315315
}
316316
}
317-
resolve_rhs_property_access(access, ctx)
317+
let result = resolve_rhs_property_access(access, ctx);
318+
// When no scope_var_resolver is available (e.g. type error
319+
// diagnostic), apply property narrowing from enclosing
320+
// if-conditions (instanceof checks). This ensures that
321+
// `$this->prop` inside `if ($this->prop instanceof X)`
322+
// resolves to X instead of the declared property type.
323+
if ctx.scope_var_resolver.is_none()
324+
&& !result.is_empty()
325+
&& let Some(key) = crate::completion::types::narrowing::expr_to_subject_key(expr)
326+
&& key.contains("->")
327+
{
328+
let rctx = ctx.as_resolution_ctx();
329+
let mut classes: Vec<Arc<ClassInfo>> =
330+
result.iter().filter_map(|r| r.class_info.clone()).collect();
331+
if !classes.is_empty() {
332+
crate::completion::resolver::apply_property_narrowing(
333+
&key,
334+
ctx.current_class,
335+
&rctx,
336+
&mut classes,
337+
);
338+
// If narrowing changed the classes, return the narrowed result.
339+
let original_names: Vec<&str> = result
340+
.iter()
341+
.filter_map(|r| r.class_info.as_ref().map(|c| c.name.as_str()))
342+
.collect();
343+
let narrowed_names: Vec<&str> =
344+
classes.iter().map(|c| c.name.as_str()).collect();
345+
if original_names != narrowed_names {
346+
return ResolvedType::from_classes(classes);
347+
}
348+
}
349+
}
350+
result
318351
}
319352
Expression::Parenthesized(p) => resolve_rhs_expression(p.expression, ctx),
320353
Expression::Match(match_expr) => {

tests/integration/diag_timing.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
//! invalidation, classes from unedited files stay cached and the second
1616
//! pass is significantly faster.
1717
18-
use crate::common::{create_psr4_workspace, create_test_backend, create_test_backend_with_full_stubs};
18+
use crate::common::{
19+
create_psr4_workspace, create_test_backend, create_test_backend_with_full_stubs,
20+
};
1921
use std::time::Instant;
2022
use tower_lsp::lsp_types::NumberOrString;
2123

tests/integration/diagnostics_type_errors.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3831,3 +3831,34 @@ class Controller {
38313831
"Should not flag bare array from method return as incompatible with typed array param, got: {msgs:?}"
38323832
);
38333833
}
3834+
3835+
#[test]
3836+
fn no_false_positive_for_property_narrowed_via_instanceof() {
3837+
let php = r#"<?php
3838+
interface MockInterface {
3839+
public function shouldReceive(string $name): self;
3840+
}
3841+
3842+
class EpaymentService {
3843+
public function annul(): bool { return true; }
3844+
}
3845+
3846+
class TestCase {
3847+
private EpaymentService $service;
3848+
3849+
protected function mockMethod(MockInterface $mock, string $method): void {}
3850+
3851+
public function test(): void {
3852+
if ($this->service instanceof MockInterface) {
3853+
$this->mockMethod($this->service, 'annul');
3854+
}
3855+
}
3856+
}
3857+
"#;
3858+
let diags = collect(php);
3859+
let msgs = type_error_messages(&diags);
3860+
assert!(
3861+
msgs.is_empty(),
3862+
"Property narrowed via instanceof should be accepted as MockInterface, got: {msgs:?}"
3863+
);
3864+
}

0 commit comments

Comments
 (0)