Skip to content

Commit 95f48d8

Browse files
author
Anton Panov
committed
fix(completion,rename): preserve PHPDoc virtual property details and reject rename for virtual members
Implement property-shaped completion details for PHPDoc virtual properties and enforce stricter validation for rename operations on virtual members. Changes: - Extract symbol_completion_detail to handle property vs callable formatting - Add property_symbol_completion_detail to format PHPDoc properties as `@property-read type` instead of callable signature - Filter member completions by PHPDoc property access (read/write) - Refactor rename validation to use should_reject_phpdoc_virtual_member_rename helper that checks both resolved symbol kind and owner match - Add packaged extension stubs path discovery for `/extension/bin/<platform>/php-lsp` layout used by VS Code bundled binaries - Add test coverage for packaged extension stubs path resolution Fixes regression where indexed PHPDoc virtual properties were displayed with callable details and could be renamed despite being virtual members.
1 parent 7a9681d commit 95f48d8

4 files changed

Lines changed: 139 additions & 27 deletions

File tree

TASKS.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5528,3 +5528,18 @@ change.
55285528
- Files:
55295529
/home/apanov/ForTesting/monica/app/Domains/Contact/DavClient/Services/Utils/AddressBookGetter.php
55305530
/home/apanov/ForTesting/monica/app/Domains/Contact/DavClient/Services/Utils/Dav/DavClient.php
5531+
5532+
- [x] **H-DIAGNOSTICS-PACKAGED-STUBS-PATH-2026-06-09** Resolve bundled stubs from packaged VS Code binary layout *(done 2026-06-09)*
5533+
- False positives include `Unknown function: App\\Actions\\is_null` when running the packaged extension binary against Monica; the same source checkout binary reports no diagnostics.
5534+
- Root cause: CLI/server candidate stubs path discovery for `/extension/bin/linux-x64/php-lsp` misses `/extension/stubs` unless VS Code passes `bundledStubsPath`.
5535+
- Implemented: packaged binary stubs discovery now includes the VS Code extension layout `/extension/bin/<platform>/php-lsp` -> `/extension/stubs`, and the installed extension binary was rebuilt/replaced.
5536+
- Validation: `cargo fmt --all --check`; `git diff --check`; `cargo clippy --all-targets -- -D warnings`; `cargo test --all`; installed extension binary analyze for `AttemptToAuthenticateSocialite.php` reports `No diagnostics found`; Verifier subagent `GO`.
5537+
- Files:
5538+
/home/apanov/ForTesting/monica/app/Actions/AttemptToAuthenticateSocialite.php
5539+
5540+
- [x] **H-COMPLETION-PHPDOC-PROPERTY-DETAIL-REGRESSION-2026-06-09** Keep PHPDoc virtual property completion details property-shaped *(done 2026-06-09)*
5541+
- Regression: indexed PHPDoc virtual properties can be emitted through the generic symbol completion path and displayed as callable details like `(): int` instead of `@property-read int`.
5542+
- Implemented: generic symbol completion now formats properties as property types, preserves PHPDoc virtual property access tags, applies PHPDoc read/write access filtering to indexed virtual properties, and rejects rename/prepareRename for resolved PHPDoc virtual members.
5543+
- Validation: `cargo test -p php-lsp-server --test e2e_completion test_phpdoc_fixture_hover_completion_definition_and_diagnostics`; `cargo test -p php-lsp-completion test_member_completion_includes_phpdoc_virtual_members`; `cargo test --all`; Verifier subagent `GO`.
5544+
- Files:
5545+
/home/apanov/Projects/php-lsp/server/crates/php-lsp-server/tests/e2e_completion.rs

server/crates/php-lsp-completion/src/provider.rs

Lines changed: 70 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,11 @@ fn provide_member_completions(
228228
if !member_is_visible(&member, object_expr == "$this", current_class_fqn) {
229229
continue;
230230
}
231+
if let Some(property_access) = phpdoc_property_access_for_symbol(&member) {
232+
if !phpdoc_property_matches_access(property_access, access_mode) {
233+
continue;
234+
}
235+
}
231236

232237
items.push(symbol_to_completion_item(
233238
&member,
@@ -689,27 +694,7 @@ fn symbol_to_completion_item(
689694
) -> CompletionItem {
690695
let kind = symbol_kind_to_completion_kind(sym.kind);
691696

692-
let detail = sym.signature.as_ref().map(|sig| {
693-
let params_str: Vec<String> = sig
694-
.params
695-
.iter()
696-
.map(|p| {
697-
let mut s = String::new();
698-
if let Some(ref t) = p.type_info {
699-
s.push_str(&t.to_string());
700-
s.push(' ');
701-
}
702-
s.push('$');
703-
s.push_str(&p.name);
704-
s
705-
})
706-
.collect();
707-
let mut detail = format!("({})", params_str.join(", "));
708-
if let Some(ref ret) = sig.return_type {
709-
detail.push_str(&format!(": {}", ret));
710-
}
711-
detail
712-
});
697+
let detail = symbol_completion_detail(sym);
713698

714699
let label =
715700
if sym.kind == PhpSymbolKind::Property && is_static_access && !sym.name.starts_with('$') {
@@ -751,6 +736,70 @@ fn symbol_to_completion_item(
751736
item
752737
}
753738

739+
fn symbol_completion_detail(sym: &SymbolInfo) -> Option<String> {
740+
if sym.kind == PhpSymbolKind::Property {
741+
return property_symbol_completion_detail(sym);
742+
}
743+
744+
sym.signature.as_ref().map(|sig| {
745+
let params_str: Vec<String> = sig
746+
.params
747+
.iter()
748+
.map(|p| {
749+
let mut s = String::new();
750+
if let Some(ref t) = p.type_info {
751+
s.push_str(&t.to_string());
752+
s.push(' ');
753+
}
754+
s.push('$');
755+
s.push_str(&p.name);
756+
s
757+
})
758+
.collect();
759+
let mut detail = format!("({})", params_str.join(", "));
760+
if let Some(ref ret) = sig.return_type {
761+
detail.push_str(&format!(": {}", ret));
762+
}
763+
detail
764+
})
765+
}
766+
767+
fn property_symbol_completion_detail(sym: &SymbolInfo) -> Option<String> {
768+
let property_name = sym.name.trim_start_matches('$');
769+
if let Some(ref doc_comment) = sym.doc_comment {
770+
let phpdoc = parse_phpdoc(doc_comment);
771+
if let Some(property) = phpdoc
772+
.properties
773+
.iter()
774+
.find(|property| property.name == property_name)
775+
{
776+
let access = phpdoc_property_tag(property.access);
777+
return Some(match &property.type_info {
778+
Some(type_info) => format!("{} {}", access, type_info),
779+
None => access.to_string(),
780+
});
781+
}
782+
}
783+
784+
sym.signature
785+
.as_ref()
786+
.and_then(|sig| sig.return_type.as_ref())
787+
.map(|return_type| return_type.to_string())
788+
}
789+
790+
fn phpdoc_property_access_for_symbol(sym: &SymbolInfo) -> Option<PhpDocPropertyAccess> {
791+
if sym.kind != PhpSymbolKind::Property {
792+
return None;
793+
}
794+
let property_name = sym.name.trim_start_matches('$');
795+
let phpdoc = parse_phpdoc(sym.doc_comment.as_ref()?);
796+
phpdoc
797+
.properties
798+
.iter()
799+
.find(|property| property.name == property_name)
800+
.map(|property| property.access)
801+
}
802+
754803
fn completion_prefix_rank(label: &str, member_prefix: Option<&str>) -> &'static str {
755804
let Some(prefix) = member_prefix
756805
.map(str::trim)

server/crates/php-lsp-server/src/indexing/stubs.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ fn candidate_stubs_paths_for_exe(
5858
if let Some(dir) = exe.and_then(Path::parent) {
5959
push_candidate_path(&mut candidate_paths, dir.join("data/stubs"));
6060
push_candidate_path(&mut candidate_paths, dir.join("../stubs"));
61+
push_candidate_path(&mut candidate_paths, dir.join("../../stubs"));
6162
push_candidate_path(&mut candidate_paths, dir.join("../../data/stubs"));
6263
}
6364

@@ -318,6 +319,22 @@ mod tests {
318319
);
319320
}
320321

322+
#[test]
323+
fn test_candidate_stubs_paths_include_packaged_extension_stubs_from_platform_binary() {
324+
let root = Path::new("/tmp/project");
325+
let exe = Path::new(
326+
"/home/user/.vscode/extensions/hightemp.ht-php-lsp-0.6.0/bin/linux-x64/php-lsp",
327+
);
328+
let paths = candidate_stubs_paths_for_exe(root, None, Some(exe));
329+
330+
assert!(
331+
paths.iter().any(|path| {
332+
path == Path::new("/home/user/.vscode/extensions/hightemp.ht-php-lsp-0.6.0/stubs")
333+
}),
334+
"expected packaged extension stubs path in {paths:?}"
335+
);
336+
}
337+
321338
#[test]
322339
fn test_load_configured_stubs_exposes_standard_builtin_functions_from_source_checkout() {
323340
let stubs_path = source_stubs_path();

server/crates/php-lsp-server/src/lsp/rename.rs

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,10 @@ impl PhpLspBackend {
9292
}
9393

9494
let resolved_for_rename = self.resolve_fqn_with_fallback(&sym.fqn, sym.ref_kind);
95-
if resolved_for_rename.is_none()
96-
&& phpdoc_virtual_member_for_symbol(&self.index, &sym).is_some()
97-
{
95+
let phpdoc_virtual_member = phpdoc_virtual_member_for_symbol(&self.index, &sym);
96+
if phpdoc_virtual_member.as_ref().is_some_and(|member| {
97+
should_reject_phpdoc_virtual_member_rename(&resolved_for_rename, member)
98+
}) {
9899
return Err(tower_lsp::jsonrpc::Error::invalid_params(
99100
"Cannot rename PHPDoc virtual members",
100101
));
@@ -269,9 +270,10 @@ impl PhpLspBackend {
269270

270271
// Don't rename built-in or PHPDoc virtual symbols
271272
let resolved = self.resolve_fqn_with_fallback(&sym.fqn, sym.ref_kind);
272-
if resolved.is_none()
273-
&& phpdoc_virtual_member_for_symbol(&self.index, &sym).is_some()
274-
{
273+
let phpdoc_virtual_member = phpdoc_virtual_member_for_symbol(&self.index, &sym);
274+
if phpdoc_virtual_member.as_ref().is_some_and(|member| {
275+
should_reject_phpdoc_virtual_member_rename(&resolved, member)
276+
}) {
275277
return Ok(None);
276278
}
277279
if resolved.is_none()
@@ -560,6 +562,35 @@ fn is_member_ref_kind(kind: RefKind) -> bool {
560562
)
561563
}
562564

565+
fn should_reject_phpdoc_virtual_member_rename(
566+
resolved: &Option<Arc<php_lsp_types::SymbolInfo>>,
567+
member: &PhpDocVirtualMember,
568+
) -> bool {
569+
match resolved.as_deref() {
570+
Some(symbol) => resolved_symbol_is_phpdoc_virtual_member(symbol, member),
571+
None => true,
572+
}
573+
}
574+
575+
fn resolved_symbol_is_phpdoc_virtual_member(
576+
symbol: &php_lsp_types::SymbolInfo,
577+
member: &PhpDocVirtualMember,
578+
) -> bool {
579+
let expected_kind = match member.kind {
580+
PhpDocVirtualMemberKind::Property => php_lsp_types::PhpSymbolKind::Property,
581+
PhpDocVirtualMemberKind::Method => php_lsp_types::PhpSymbolKind::Method,
582+
};
583+
if symbol.kind != expected_kind
584+
|| symbol.parent_fqn.as_deref() != Some(member.owner.fqn.as_str())
585+
{
586+
return false;
587+
}
588+
589+
let symbol_name = symbol.name.trim_start_matches('$');
590+
symbol_name == member.name.trim_start_matches('$')
591+
&& symbol.doc_comment.as_deref() == member.owner.doc_comment.as_deref()
592+
}
593+
563594
#[cfg(test)]
564595
mod tests {
565596
use super::*;

0 commit comments

Comments
 (0)