Skip to content

Commit dc4f1b1

Browse files
committed
Fix cross resolve for facades, consistency for model-properties, a
couple of compleation issue
1 parent 5b2c536 commit dc4f1b1

8 files changed

Lines changed: 259 additions & 38 deletions

File tree

src/class_lookup.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -459,9 +459,11 @@ pub(crate) fn is_subtype_of_typed(
459459
if let Some(model_name) = args[0].base_name()
460460
&& let Some(cls) = class_loader(model_name)
461461
{
462-
return crate::virtual_members::laravel::where_property::collect_column_names(&cls)
463-
.iter()
464-
.any(|col| col == prop_name);
462+
let resolved = crate::inheritance::resolve_class_with_inheritance(&cls, class_loader);
463+
return resolved.properties.iter().any(|p| &*p.name == prop_name)
464+
|| crate::virtual_members::laravel::where_property::collect_column_names(&cls)
465+
.iter()
466+
.any(|col| col == prop_name);
465467
}
466468
return true;
467469
}

src/completion/context/override_completion.rs

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -722,24 +722,50 @@ fn preceded_by_use_keyword_bytes(bytes: &[u8], keyword_start: usize) -> bool {
722722
check_keyword_ending_at_bytes(bytes, before, b"use")
723723
}
724724

725-
fn has_const_keyword_before_name(bytes: &[u8], pos: usize) -> bool {
725+
pub(super) fn has_const_keyword_before_name(bytes: &[u8], pos: usize) -> bool {
726726
let mut line_start = pos;
727727
while line_start > 0 && bytes[line_start - 1] != b'\n' {
728728
line_start -= 1;
729729
}
730-
bytes[line_start..pos]
730+
// Find the standalone `const` keyword governing this line (skipping
731+
// `use const` imports).
732+
let Some(const_end) = bytes[line_start..pos]
731733
.windows(b"const".len())
732734
.enumerate()
733-
.any(|(idx, window)| {
735+
.find_map(|(idx, window)| {
734736
if window != b"const" {
735-
return false;
737+
return None;
736738
}
737739
let start = line_start + idx;
738740
let end = start + b"const".len();
739-
(start == 0 || !is_ident_byte(bytes[start - 1]))
740-
&& (end >= bytes.len() || !is_ident_byte(bytes[end]))
741-
&& !preceded_by_use_keyword_bytes(bytes, start)
741+
let is_word = (start == 0 || !is_ident_byte(bytes[start - 1]))
742+
&& (end >= bytes.len() || !is_ident_byte(bytes[end]));
743+
(is_word && !preceded_by_use_keyword_bytes(bytes, start)).then_some(end)
742744
})
745+
else {
746+
return false;
747+
};
748+
// A declarator *name* follows `const` or a top-level `,`; the cursor is
749+
// in the initializer once a top-level `=` is seen (until the next
750+
// comma). `const MAX = PHP_INT_M|` is a value position, not a name.
751+
!in_const_initializer(bytes, const_end, pos)
752+
}
753+
754+
/// Whether the byte range `from..pos` (starting just after a `const`
755+
/// keyword) ends inside a declarator initializer rather than a name slot.
756+
fn in_const_initializer(bytes: &[u8], from: usize, pos: usize) -> bool {
757+
let mut depth = 0i32;
758+
let mut in_value = false;
759+
for &b in &bytes[from..pos] {
760+
match b {
761+
b'(' | b'[' | b'{' => depth += 1,
762+
b')' | b']' | b'}' => depth = depth.saturating_sub(1),
763+
b',' if depth == 0 => in_value = false,
764+
b'=' if depth == 0 => in_value = true,
765+
_ => {}
766+
}
767+
}
768+
in_value
743769
}
744770

745771
/// Property name after `$` on a property declaration line (not a parameter).
@@ -773,11 +799,14 @@ fn is_function_or_const_name_position_at_offset(bytes: &[u8], cursor: usize) ->
773799
return false;
774800
}
775801

776-
if check_keyword_ending_at_bytes(bytes, i, b"fn")
777-
|| check_keyword_ending_at_bytes(bytes, i, b"case")
778-
{
802+
if check_keyword_ending_at_bytes(bytes, i, b"fn") {
779803
return true;
780804
}
805+
// `case` names an enum case (a member) but also labels a `switch`
806+
// branch, where a class/const/enum name completion is wanted.
807+
if check_keyword_ending_at_bytes(bytes, i, b"case") {
808+
return case_is_enum_declaration(bytes, i);
809+
}
781810
if check_keyword_ending_at_bytes(bytes, i, b"function") {
782811
return !preceded_by_use_keyword_bytes(bytes, i - "function".len());
783812
}
@@ -787,6 +816,40 @@ fn is_function_or_const_name_position_at_offset(bytes: &[u8], cursor: usize) ->
787816
has_const_keyword_before_name(bytes, i)
788817
}
789818

819+
/// Whether the `case` keyword ending at `keyword_end` declares an enum case
820+
/// (directly inside an `enum` body) rather than a `switch` branch label.
821+
/// Scans back to the nearest enclosing unmatched `{` and checks whether it
822+
/// opens an enum.
823+
pub(super) fn case_is_enum_declaration(bytes: &[u8], keyword_end: usize) -> bool {
824+
let mut depth = 0i32;
825+
let mut k = keyword_end - "case".len();
826+
while k > 0 {
827+
k -= 1;
828+
match bytes[k] {
829+
b'}' => depth += 1,
830+
b'{' => {
831+
if depth == 0 {
832+
return brace_opens_enum(bytes, k);
833+
}
834+
depth -= 1;
835+
}
836+
_ => {}
837+
}
838+
}
839+
false
840+
}
841+
842+
/// Whether the block opened by the `{` at `brace_pos` is an `enum` body,
843+
/// determined from the block header (the text back to the previous
844+
/// statement boundary).
845+
fn brace_opens_enum(bytes: &[u8], brace_pos: usize) -> bool {
846+
let mut start = brace_pos;
847+
while start > 0 && !matches!(bytes[start - 1], b';' | b'{' | b'}') {
848+
start -= 1;
849+
}
850+
contains_ascii_word(&bytes[start..brace_pos], b"enum")
851+
}
852+
790853
fn is_property_declaration_name_position_at_offset(bytes: &[u8], cursor: usize) -> bool {
791854
// Skip partial name.
792855
let mut i = cursor;

src/completion/context/type_hint_completion.rs

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -243,11 +243,14 @@ pub(crate) fn is_function_or_const_name_position(content: &str, position: Positi
243243
return false;
244244
}
245245

246-
if check_keyword_ending_at_bytes(bytes, i, b"fn")
247-
|| check_keyword_ending_at_bytes(bytes, i, b"case")
248-
{
246+
if check_keyword_ending_at_bytes(bytes, i, b"fn") {
249247
return true;
250248
}
249+
// `case` names an enum case (a member) but also labels a `switch`
250+
// branch, where a class/const/enum name completion is wanted.
251+
if check_keyword_ending_at_bytes(bytes, i, b"case") {
252+
return super::override_completion::case_is_enum_declaration(bytes, i);
253+
}
251254
// `function` / `const` declare member names, but not after `use`
252255
// (`use function foo`, `use const BAR` still need symbol completion).
253256
if check_keyword_ending_at_bytes(bytes, i, b"function") {
@@ -342,23 +345,7 @@ fn preceded_by_use_keyword_bytes(bytes: &[u8], keyword_start: usize) -> bool {
342345
}
343346

344347
fn has_const_keyword_before_name(bytes: &[u8], pos: usize) -> bool {
345-
let mut line_start = pos;
346-
while line_start > 0 && bytes[line_start - 1] != b'\n' {
347-
line_start -= 1;
348-
}
349-
bytes[line_start..pos]
350-
.windows(b"const".len())
351-
.enumerate()
352-
.any(|(idx, window)| {
353-
if window != b"const" {
354-
return false;
355-
}
356-
let start = line_start + idx;
357-
let end = start + b"const".len();
358-
(start == 0 || !is_ident_byte(bytes[start - 1]))
359-
&& (end >= bytes.len() || !is_ident_byte(bytes[end]))
360-
&& !preceded_by_use_keyword_bytes(bytes, start)
361-
})
348+
super::override_completion::has_const_keyword_before_name(bytes, pos)
362349
}
363350

364351
// ─── Private helpers ────────────────────────────────────────────────────────

src/completion/context/type_hint_completion_tests.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,51 @@ fn case_name_position_in_enum() {
341341
));
342342
}
343343

344+
#[test]
345+
fn switch_case_label_is_not_name_position() {
346+
use super::is_function_or_const_name_position;
347+
// A `case` inside a `switch` wants class/const/enum completion, unlike
348+
// an enum-case declaration.
349+
let src = "<?php\nfunction f($x) {\n switch ($x) {\n case Sta\n }\n}";
350+
assert!(!is_function_or_const_name_position(
351+
src,
352+
Position {
353+
line: 3,
354+
character: 16
355+
}
356+
));
357+
}
358+
359+
#[test]
360+
fn const_initializer_is_not_name_position() {
361+
use super::is_function_or_const_name_position;
362+
// The right-hand side of a constant initializer is a value expression,
363+
// so class/const completion (e.g. `PHP_INT_MAX`) must not be suppressed.
364+
let src = "<?php\nclass Foo {\n const MAX = PHP_INT_M\n}";
365+
assert!(!is_function_or_const_name_position(
366+
src,
367+
Position {
368+
line: 2,
369+
character: 25
370+
}
371+
));
372+
}
373+
374+
#[test]
375+
fn second_const_declarator_is_name_position() {
376+
use super::is_function_or_const_name_position;
377+
// A subsequent declarator name in a `const A = 1, B` list is still a
378+
// name position even though a `const` value precedes it on the line.
379+
let src = "<?php\nclass Foo {\n const A = 1, B\n}";
380+
assert!(is_function_or_const_name_position(
381+
src,
382+
Position {
383+
line: 2,
384+
character: 18
385+
}
386+
));
387+
}
388+
344389
#[test]
345390
fn property_name_after_dollar_is_not_function_name_position() {
346391
use super::is_function_or_const_name_position;

src/type_engine/variable/rhs_resolution/calls.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1998,10 +1998,18 @@ fn facade_accessor_concrete_owner(
19981998
let merged =
19991999
crate::virtual_members::resolve_class_fully_maybe_cached(facade, class_loader, cache);
20002000

2001-
if let Some(concrete) = crate::virtual_members::laravel::parse_facade_accessor(content)
2002-
.and_then(facade_accessor_to_class_name)
2003-
.and_then(|name| class_loader(&name))
2004-
.filter(|class| class_has_method(class, method_name, class_loader, cache))
2001+
// When the facade is declared in the current file, read its
2002+
// `getFacadeAccessor()` return directly from source: the body walker
2003+
// that `try_infer_body_return_type` relies on is not always active in
2004+
// the hover/completion paths, and it honours the declared `: string`
2005+
// return type rather than the `::class` value we need. Scope the parse
2006+
// to the facade's own class so a file declaring several facades does
2007+
// not cross-resolve.
2008+
if let Some(concrete) =
2009+
crate::virtual_members::laravel::parse_facade_accessor_for_class(content, &facade.name)
2010+
.and_then(facade_accessor_to_class_name)
2011+
.and_then(|name| class_loader(&name))
2012+
.filter(|class| class_has_method(class, method_name, class_loader, cache))
20052013
{
20062014
return Some(Arc::unwrap_or_clone(concrete));
20072015
}

src/virtual_members/laravel/aliases.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,48 @@ pub(crate) fn parse_facade_accessor(content: &str) -> Option<FacadeAccessor> {
340340
})
341341
}
342342

343+
/// Like [`parse_facade_accessor`] but scoped to the class named
344+
/// `class_name` within `content`.
345+
///
346+
/// A single file may declare several facades (or a facade alongside
347+
/// unrelated classes), so resolving the accessor for a specific facade
348+
/// must not pick up the first `getFacadeAccessor()` in the file. Returns
349+
/// `None` when no class of that name is present (e.g. the facade lives in
350+
/// another file).
351+
pub(crate) fn parse_facade_accessor_for_class(
352+
content: &str,
353+
class_name: &str,
354+
) -> Option<FacadeAccessor> {
355+
with_parsed(content, |program, resolved| {
356+
let return_value =
357+
find_facade_accessor_return_in_class(Node::Program(program), class_name)?;
358+
if let Some((text, _, _)) = super::helpers::extract_string_literal(return_value, content) {
359+
return Some(FacadeAccessor::Alias(text.to_string()));
360+
}
361+
class_const_fqn(return_value, resolved).map(FacadeAccessor::Class)
362+
})
363+
}
364+
365+
/// Find the `getFacadeAccessor()` return value inside the class named
366+
/// `class_name` reachable from `node`.
367+
fn find_facade_accessor_return_in_class<'ast, 'arena>(
368+
node: Node<'ast, 'arena>,
369+
class_name: &str,
370+
) -> Option<&'ast Expression<'arena>> {
371+
if let Node::Class(class) = node
372+
&& bytes_to_str(class.name.value).eq_ignore_ascii_case(class_name)
373+
{
374+
return find_facade_accessor_return(node);
375+
}
376+
let mut found = None;
377+
node.visit_children(|child| {
378+
if found.is_none() {
379+
found = find_facade_accessor_return_in_class(child, class_name);
380+
}
381+
});
382+
found
383+
}
384+
343385
/// Find the first return-statement value inside a `getFacadeAccessor()` method
344386
/// reachable from `node`.
345387
fn find_facade_accessor_return<'ast, 'arena>(

src/virtual_members/laravel/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ mod trans_keys;
101101
mod view_names;
102102
pub(crate) mod where_property;
103103

104-
pub(crate) use aliases::{FacadeAccessor, LaravelAliases, parse_facade_accessor};
104+
pub(crate) use aliases::{FacadeAccessor, LaravelAliases, parse_facade_accessor_for_class};
105105
pub(crate) use auth::{GUARD_FQN, REQUEST_FQN, patch_auth_user_class, resolve_auth_user_type};
106106
pub(crate) use config_keys::find_config_references;
107107
pub(crate) use config_keys::{

tests/integration/hover.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7140,6 +7140,80 @@ namespace {
71407140
);
71417141
}
71427142

7143+
#[test]
7144+
fn hover_facade_concrete_target_scoped_to_own_class() {
7145+
// Two facades declared in the same file: resolving a call on `Second`
7146+
// must read `Second::getFacadeAccessor()`, not the first accessor found
7147+
// in the file (which belongs to `First`).
7148+
let backend = create_test_backend();
7149+
let uri = "file:///test.php";
7150+
let content = r#"<?php
7151+
7152+
namespace Illuminate\Support\Facades {
7153+
abstract class Facade
7154+
{
7155+
public static function __callStatic(string $method, array $args): mixed
7156+
{
7157+
return null;
7158+
}
7159+
}
7160+
}
7161+
7162+
namespace {
7163+
use Illuminate\Support\Facades\Facade;
7164+
7165+
final class WrongType {}
7166+
final class RightType {}
7167+
7168+
final class FirstTarget
7169+
{
7170+
public function handle(): WrongType
7171+
{
7172+
return new WrongType();
7173+
}
7174+
}
7175+
7176+
final class SecondTarget
7177+
{
7178+
public function handle(): RightType
7179+
{
7180+
return new RightType();
7181+
}
7182+
}
7183+
7184+
final class First extends Facade
7185+
{
7186+
protected static function getFacadeAccessor(): string
7187+
{
7188+
return FirstTarget::class;
7189+
}
7190+
}
7191+
7192+
final class Second extends Facade
7193+
{
7194+
protected static function getFacadeAccessor(): string
7195+
{
7196+
return SecondTarget::class;
7197+
}
7198+
}
7199+
7200+
$result = Second::handle();
7201+
$result;
7202+
}
7203+
"#;
7204+
7205+
let hover = hover_at(&backend, uri, content, 51, 6).expect("hover on $result should resolve");
7206+
let text = hover_text(&hover);
7207+
assert!(
7208+
text.contains("RightType"),
7209+
"facade call should resolve through its own accessor, got: {text}"
7210+
);
7211+
assert!(
7212+
!text.contains("WrongType"),
7213+
"facade call must not cross-resolve to another facade's accessor, got: {text}"
7214+
);
7215+
}
7216+
71437217
#[test]
71447218
fn hover_foreach_over_variable_assigned_via_elvis_with_static_call() {
71457219
let backend = create_test_backend();

0 commit comments

Comments
 (0)