Skip to content

Commit 80f7b78

Browse files
committed
Fix diagnostices on array acccess of unknown type
1 parent 6267022 commit 80f7b78

6 files changed

Lines changed: 240 additions & 102 deletions

File tree

src/completion/resolver.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -818,12 +818,11 @@ pub(crate) fn resolve_target_classes_expr(
818818
) {
819819
return resolved.into_iter().map(Arc::new).collect();
820820
}
821-
// Fall through to variable resolution if the base is a bare variable
822-
if let SubjectExpr::Variable(_) = **base {
823-
resolve_variable_fallback(&base_var, access_kind, ctx)
824-
} else {
825-
vec![]
826-
}
821+
// Segment walk failed — the base type does not have
822+
// array-shape, generic, or iterable annotations that
823+
// cover bracket access. Return empty: `$var['key']` is
824+
// never the same type as `$var`.
825+
vec![]
827826
}
828827

829828
// ── Bare variable ───────────────────────────────────────

src/completion/source/helpers.rs

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -428,12 +428,45 @@ fn walk_array_segments_and_resolve(
428428
let mut current = PhpType::parse(&current_str);
429429

430430
for seg in segments {
431-
current = match seg {
431+
// Try pure-type extraction first (array shapes, generics).
432+
let extracted = match seg {
432433
BracketSegment::StringKey(key) => current
433434
.shape_value_type(key)
434435
.or_else(|| current.extract_value_type(true))
435-
.cloned()?,
436-
BracketSegment::ElementAccess => current.extract_value_type(true)?.clone(),
436+
.cloned(),
437+
BracketSegment::ElementAccess => current.extract_value_type(true).cloned(),
438+
};
439+
440+
current = if let Some(t) = extracted {
441+
t
442+
} else {
443+
// Fallback: when the current type is a plain class name (e.g.
444+
// `Application`, `OpeningHours`), resolve the class and check
445+
// its iterable generics (`@extends`, `@implements`) for the
446+
// element type. This handles bracket access on classes that
447+
// implement `ArrayAccess` with generic type parameters.
448+
let type_str = current.to_string();
449+
let class_element = crate::completion::type_resolution::type_hint_to_classes(
450+
&type_str,
451+
current_class_name,
452+
all_classes,
453+
class_loader,
454+
)
455+
.into_iter()
456+
.find_map(|cls| {
457+
let merged =
458+
crate::virtual_members::resolve_class_fully(&cls, class_loader);
459+
crate::completion::variable::foreach_resolution::extract_iterable_element_type_from_class(
460+
&merged,
461+
class_loader,
462+
)
463+
});
464+
465+
if let Some(element) = class_element {
466+
PhpType::parse(&element)
467+
} else {
468+
return None;
469+
}
437470
};
438471

439472
// After each segment, the resulting type might itself be an

src/completion/variable/raw_type_inference.rs

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -228,9 +228,14 @@ pub(in crate::completion) fn resolve_array_func_raw_type(
228228
/// (e.g. `"User"`) for the output.
229229
///
230230
/// Used by `resolve_rhs_expression` so that `$item = array_pop($users)`
231-
/// resolves `$item` to `User`. This handles both element-extracting
232-
/// functions (array_pop, current, etc.) and `array_map` (via callback
233-
/// return type).
231+
/// resolves `$item` to `User`. This only covers true element-extracting
232+
/// functions (array_pop, current, etc.) that return a single element.
233+
///
234+
/// Array-producing functions like `array_map` and `iterator_to_array`
235+
/// are handled exclusively by [`resolve_array_func_raw_type`] which
236+
/// preserves the container type (e.g. `list<User>`). Returning the
237+
/// element type here would lose the array wrapper and break downstream
238+
/// consumers that need to walk bracket segments (e.g. `$result[0]->`).
234239
pub(in crate::completion) fn resolve_array_func_element_type(
235240
func_name: &str,
236241
args: &ArgumentList<'_>,
@@ -248,20 +253,6 @@ pub(in crate::completion) fn resolve_array_func_element_type(
248253
.map(|t| t.to_string());
249254
}
250255

251-
// array_map: callback return type is the element type.
252-
if func_name.eq_ignore_ascii_case("array_map") {
253-
return extract_array_map_element_type(args, ctx);
254-
}
255-
256-
// iterator_to_array: the element type is the iterator's value type.
257-
if func_name.eq_ignore_ascii_case("iterator_to_array") {
258-
let iter_expr = super::resolution::first_arg_expr(args)?;
259-
let raw = super::resolution::resolve_arg_raw_type(iter_expr, ctx)?;
260-
return crate::php_type::PhpType::parse(&raw)
261-
.extract_value_type(true)
262-
.map(|t| t.to_string());
263-
}
264-
265256
None
266257
}
267258

src/diagnostics/unknown_members.rs

Lines changed: 128 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -170,12 +170,10 @@ enum SubjectOutcome {
170170
Scalar(String),
171171
/// Subject resolved to a class name that couldn't be loaded.
172172
UnresolvableClass(String),
173-
/// Subject is a chain or call expression whose type couldn't be
174-
/// resolved.
175-
UnresolvableChain,
176-
/// Subject is a bare variable with no type information at all.
177-
/// No diagnostic should be emitted (the opt-in
178-
/// `unresolved-member-access` diagnostic covers this case).
173+
/// Subject type could not be resolved — no class information
174+
/// available. The opt-in `unresolved-member-access` diagnostic
175+
/// covers this case regardless of whether the subject is a bare
176+
/// variable, a chain, an array access, or a function call.
179177
Untyped,
180178
}
181179

@@ -263,15 +261,6 @@ fn resolve_subject_outcome(
263261
return SubjectOutcome::UnresolvableClass(unresolved);
264262
}
265263

266-
// Check if the subject is a chain or call expression.
267-
let is_chain = matches!(
268-
expr,
269-
SubjectExpr::PropertyChain { .. } | SubjectExpr::CallExpr { .. }
270-
);
271-
if is_chain {
272-
return SubjectOutcome::UnresolvableChain;
273-
}
274-
275264
SubjectOutcome::Untyped
276265
}
277266

@@ -548,65 +537,39 @@ impl Backend {
548537
));
549538
}
550539

551-
SubjectOutcome::UnresolvableChain => {
552-
let range = match offset_range_to_lsp_range(
553-
content,
554-
span.start as usize,
555-
span.end as usize,
556-
) {
557-
Some(r) => r,
558-
None => continue,
559-
};
560-
let kind_label = if is_method_call { "method" } else { "property" };
561-
let message = format!(
562-
"Cannot verify {} '{}' — subject type could not be resolved",
563-
kind_label, member_name,
564-
);
565-
out.push(make_diagnostic(
566-
range,
567-
DiagnosticSeverity::WARNING,
568-
UNKNOWN_MEMBER_CODE,
569-
message,
570-
));
571-
broken_chain_prefixes.push(broken_chain_prefix(
572-
subject_text,
573-
member_name,
574-
is_static,
575-
is_method_call,
576-
));
577-
}
578-
579540
SubjectOutcome::Untyped => {
580541
// When the opt-in `unresolved-member-access` diagnostic
581-
// is enabled, emit it here instead of in a separate
582-
// collector pass. This avoids a second full walk of
583-
// the same symbol spans with duplicate type resolution.
542+
// is enabled, report every member access where the
543+
// subject type could not be resolved — regardless of
544+
// whether the subject is a bare variable, a chain, an
545+
// array access, or a function call result.
584546
if self.config().diagnostics.unresolved_member_access_enabled() {
585-
// Skip call-expression subjects — the failure is
586-
// usually because the symbol map's subject_text
587-
// doesn't preserve full argument text, not because
588-
// the user is missing a type annotation.
589-
if !subject_text.is_empty() && !subject_text.contains('(') {
590-
let range = match offset_range_to_lsp_range(
591-
content,
592-
span.start as usize,
593-
span.end as usize,
594-
) {
595-
Some(r) => r,
596-
None => continue,
597-
};
598-
let subject_display = subject_text.trim();
599-
let message = format!(
600-
"Cannot resolve type of '{}'. Add a type annotation or PHPDoc tag to enable full IDE support.",
601-
subject_display,
602-
);
603-
out.push(make_diagnostic(
604-
range,
605-
DiagnosticSeverity::HINT,
606-
UNRESOLVED_MEMBER_ACCESS_CODE,
607-
message,
608-
));
609-
}
547+
let range = match offset_range_to_lsp_range(
548+
content,
549+
span.start as usize,
550+
span.end as usize,
551+
) {
552+
Some(r) => r,
553+
None => continue,
554+
};
555+
let subject_display = subject_text.trim();
556+
let kind_label = if is_method_call { "method" } else { "property" };
557+
let message = format!(
558+
"Cannot verify {} '{}' — type of '{}' could not be resolved",
559+
kind_label, member_name, subject_display,
560+
);
561+
out.push(make_diagnostic(
562+
range,
563+
DiagnosticSeverity::HINT,
564+
UNRESOLVED_MEMBER_ACCESS_CODE,
565+
message,
566+
));
567+
broken_chain_prefixes.push(broken_chain_prefix(
568+
subject_text,
569+
member_name,
570+
is_static,
571+
is_method_call,
572+
));
610573
}
611574
}
612575

@@ -4969,6 +4932,100 @@ function test(): void {
49694932
);
49704933
}
49714934

4935+
/// Bracket access on a class implementing `ArrayAccess` without
4936+
/// concrete generic annotations should NOT resolve to the container
4937+
/// class itself. `$app['config']` is not `Application`.
4938+
/// The diagnostic should say the subject type could not be resolved,
4939+
/// not that the member is missing on `Application`.
4940+
#[test]
4941+
fn flags_member_on_array_access_class_without_generics() {
4942+
let php = r#"<?php
4943+
class Application implements \ArrayAccess {
4944+
public function offsetExists(mixed $offset): bool { return true; }
4945+
public function offsetGet(mixed $offset): mixed { return null; }
4946+
public function offsetSet(mixed $offset, mixed $value): void {}
4947+
public function offsetUnset(mixed $offset): void {}
4948+
4949+
public function useStoragePath(string $path): void {}
4950+
}
4951+
4952+
function test(Application $app): void {
4953+
$app['config']->set('logging.default', 'stderr');
4954+
}
4955+
"#;
4956+
let backend = Backend::new_test();
4957+
// Enable unresolved-member-access so the Untyped outcome emits.
4958+
backend.config.lock().diagnostics.unresolved_member_access = Some(true);
4959+
let diags = collect(&backend, "file:///test.php", php);
4960+
// `$app['config']` returns `mixed` (no concrete generics), so
4961+
// we cannot know the type — the diagnostic should say the
4962+
// subject could not be resolved, NOT that 'set' is missing on
4963+
// `Application`.
4964+
assert!(
4965+
!diags.iter().any(|d| d.message.contains("Application")),
4966+
"should not report 'set' as missing on Application — bracket access returns mixed, got: {diags:?}",
4967+
);
4968+
assert!(
4969+
diags
4970+
.iter()
4971+
.any(|d| d.message.contains("could not be resolved")),
4972+
"expected 'could not be resolved' diagnostic for unresolvable bracket access, got: {diags:?}",
4973+
);
4974+
}
4975+
4976+
/// Same as above but with inheritance: `Application2 extends
4977+
/// Container2 implements ArrayAccess`. The `ArrayAccess` interface
4978+
/// is on the parent class, not the child.
4979+
#[test]
4980+
fn flags_member_on_array_access_subclass_without_generics() {
4981+
let php = r#"<?php
4982+
namespace Tests;
4983+
4984+
use ArrayAccess;
4985+
4986+
class Container2 implements ArrayAccess
4987+
{
4988+
public function offsetExists($offset): bool
4989+
{
4990+
return false;
4991+
}
4992+
4993+
public function offsetGet($offset): mixed
4994+
{
4995+
return '';
4996+
}
4997+
4998+
public function offsetSet($offset, $value): void
4999+
{
5000+
}
5001+
5002+
public function offsetUnset($offset): void
5003+
{
5004+
}
5005+
}
5006+
5007+
class Application2 extends Container2
5008+
{
5009+
}
5010+
5011+
class TestCase
5012+
{
5013+
public function defineEnvironment(): void
5014+
{
5015+
$test4 = new Application2();
5016+
$test4['config']->set('logging.channels.stack.channels', ['stderr']);
5017+
}
5018+
}
5019+
"#;
5020+
let backend = Backend::new_test();
5021+
backend.config.lock().diagnostics.unresolved_member_access = Some(true);
5022+
let diags = collect(&backend, "file:///test.php", php);
5023+
assert!(
5024+
!diags.iter().any(|d| d.message.contains("Application2")),
5025+
"should not report 'set' as missing on Application2 — bracket access returns mixed, got: {diags:?}",
5026+
);
5027+
}
5028+
49725029
/// B18 variant: assignment in while condition `while ($line = fgets($fp))`.
49735030
#[test]
49745031
fn assignment_in_while_condition_resolves_in_body() {

src/lib.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,14 @@ impl Backend {
897897
self.config.lock().clone()
898898
}
899899

900+
/// Replace the current configuration.
901+
///
902+
/// Used by integration tests to enable opt-in diagnostics like
903+
/// `unresolved-member-access` without needing a `.phpantom.toml` file.
904+
pub fn set_config(&self, config: config::Config) {
905+
*self.config.lock() = config;
906+
}
907+
900908
/// Set the PHP version (used by integration tests and during
901909
/// server initialization after reading `composer.json`).
902910
///

0 commit comments

Comments
 (0)