Skip to content

Commit aaa9092

Browse files
Add constructor reference support for attributes (#155)
1 parent 404b7df commit aaa9092

8 files changed

Lines changed: 615 additions & 5 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4646
- **Conditional `is null` return types** resolve consistently regardless of how the call site is parsed, and an explicitly passed `null` now selects the null branch.
4747
- **Go-to-definition, rename, and highlight accuracy.** References in `@see` tags to qualified names like `App\Foo::bar()` now land on the correct location, and renaming a property selects the whole `$name` instead of `$nam`.
4848
- **Renaming variables captured by nested closures and arrow functions.** Renaming or finding references to a variable used inside deeply nested arrow functions (`fn () => fn () => $var`) or closures with `use ($var)` now updates every occurrence, whether the rename is triggered on the declaration or from deep inside the nesting. Contributed by @calebdw in https://github.com/AJenbo/phpantom_lsp/pull/145.
49+
- **Find references on a constructor lists every call site.** Finding references to a `__construct` declaration now reports the `new ClassName(...)` instantiations, `#[ClassName(...)]` attribute usages, and explicit delegation calls written as `parent::__construct()`, `self::__construct()`, or `Class::__construct()`, including for subclasses that inherit the constructor (and excluding subclasses that override it). Attribute classes that are never written as `new` are now found. Contributed by @RemcoSmitsDev in https://github.com/AJenbo/phpantom_lsp/pull/155.
4950
- **Positions on lines with multibyte characters.** Signature help, go-to-definition on virtual properties, named-argument completion, unused-import removal, and the `@phpstan-ignore` quickfix placed cursors and edits at the wrong column on lines containing multibyte characters; they now use the correct UTF-16 columns. Type strings containing `*` wildcards or variance annotations are also no longer mangled.
5051
- **Unused-import hint location.** When two imports share a name prefix (`use App\Foo;` and `use App\FooBar;`), the "unused import" dimming now lands on the correct statement.
5152
- **Document outline ranges.** Methods, properties, constants, and functions in the outline and breadcrumbs now report a range covering the whole declaration, with the name nested inside, as editors expect for folding and breadcrumb extent.

docs/todo/bugs.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,4 @@ the fix is not yet observable for this pattern.)
3939
**Fix:** Teach the class indexer / current-class and scope detection to
4040
descend into control-flow blocks when locating the class-like declaration
4141
that encloses the cursor.
42+

src/diagnostics/invalid_class_kind.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,10 @@ impl Backend {
8282
_ => continue,
8383
};
8484

85-
// Only check references with a known context.
86-
if ref_ctx == ClassRefContext::Other {
85+
// Only check references with a known context. Attribute
86+
// usages (`#[Foo]`) are valid on any instantiable class, so
87+
// they are skipped just like `Other`.
88+
if ref_ctx == ClassRefContext::Other || ref_ctx == ClassRefContext::Attribute {
8789
continue;
8890
}
8991

@@ -346,7 +348,7 @@ fn check_kind_in_context(
346348
None
347349
}
348350
}
349-
ClassRefContext::Other | ClassRefContext::UseImport => None,
351+
ClassRefContext::Other | ClassRefContext::UseImport | ClassRefContext::Attribute => None,
350352
}
351353
}
352354

src/references/mod.rs

Lines changed: 244 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use std::sync::Arc;
3030
use tower_lsp::lsp_types::{Location, Position, Range, Url};
3131

3232
use crate::Backend;
33-
use crate::symbol_map::{SelfStaticParentKind, SymbolKind, SymbolMap, VarDefKind};
33+
use crate::symbol_map::{ClassRefContext, SelfStaticParentKind, SymbolKind, SymbolMap, VarDefKind};
3434
use crate::types::ClassInfo;
3535
use crate::util::{
3636
build_fqn, collect_php_files_gitignore, find_class_at_offset, offset_to_position,
@@ -185,6 +185,27 @@ impl Backend {
185185
member_name,
186186
);
187187

188+
// Constructors are not invoked through member accesses
189+
// (`$obj->__construct()`); they are invoked through
190+
// `new ClassName(...)`. An explicit `parent::__construct()`
191+
// call still lands here, so route to the constructor finder
192+
// seeded with the subject's resolved class(es).
193+
if is_constructor_name(member_name) {
194+
let seeds = self
195+
.get_file_content(uri)
196+
.map(|content| {
197+
self.resolve_subject_to_fqns(
198+
subject_text,
199+
*is_static,
200+
&self.file_context(uri),
201+
span_start,
202+
&content,
203+
)
204+
})
205+
.unwrap_or_default();
206+
return self.find_constructor_references(&seeds, include_declaration);
207+
}
208+
188209
self.find_member_references(
189210
member_name,
190211
*is_static,
@@ -201,6 +222,19 @@ impl Backend {
201222
self.find_constant_references(name, include_declaration)
202223
}
203224
SymbolKind::MemberDeclaration { name, is_static } => {
225+
// A constructor declaration's "references" are the
226+
// `new ClassName(...)` instantiation sites (and `#[...]`
227+
// attribute usages), not `->__construct()` member accesses
228+
// (which don't exist in normal PHP code).
229+
if is_constructor_name(name) {
230+
let ctx = self.file_context(uri);
231+
let seeds: Vec<String> =
232+
crate::util::find_class_at_offset(&ctx.classes, span_start)
233+
.map(|cc| vec![build_fqn(&cc.name, ctx.namespace.as_deref())])
234+
.unwrap_or_default();
235+
return self.find_constructor_references(&seeds, include_declaration);
236+
}
237+
204238
// Resolve the enclosing class to scope the search.
205239
let hierarchy =
206240
self.resolve_member_declaration_hierarchy(uri, span_start, name, *is_static);
@@ -803,6 +837,208 @@ impl Backend {
803837
locations
804838
}
805839

840+
/// Find all references to a constructor (`__construct`).
841+
///
842+
/// Unlike ordinary methods, constructors are not invoked through
843+
/// member-access syntax (`$obj->__construct()`); the call sites are
844+
/// `new ClassName(...)` instantiation expressions plus explicit
845+
/// `parent::__construct()` / `self::__construct()` style calls.
846+
///
847+
/// `owner_fqns` are the class(es) that declare the constructor under
848+
/// the cursor. A `new SubClass()` expression only invokes this
849+
/// constructor when `SubClass` inherits it (i.e. does not declare its
850+
/// own), so the search scope is expanded to inheriting descendants and
851+
/// pruned at overriding ones (see
852+
/// [`Self::collect_constructor_hierarchy`]).
853+
fn find_constructor_references(
854+
&self,
855+
owner_fqns: &[String],
856+
include_declaration: bool,
857+
) -> Vec<Location> {
858+
if owner_fqns.is_empty() {
859+
return Vec::new();
860+
}
861+
862+
// Expand the owners to the set of classes whose instantiation
863+
// invokes this same constructor (inheriting descendants), pruning
864+
// at descendants that override it.
865+
let scoped = self.collect_constructor_hierarchy(owner_fqns);
866+
if scoped.is_empty() {
867+
return Vec::new();
868+
}
869+
870+
let mut locations = Vec::new();
871+
let snapshot = self.user_file_symbol_maps();
872+
873+
for (file_uri, symbol_map) in &snapshot {
874+
let resolved_names = self.resolved_names.read().get(file_uri).cloned();
875+
let file_namespace = self.first_file_namespace(file_uri);
876+
let file_use_map = std::cell::OnceCell::new();
877+
let file_ctx = std::cell::OnceCell::new();
878+
879+
let Some(parsed_uri) = Url::parse(file_uri).ok() else {
880+
continue;
881+
};
882+
883+
let mut file_content: Option<Arc<String>> = None;
884+
885+
for span in &symbol_map.spans {
886+
let matched = match &span.kind {
887+
// `new ClassName(...)` carries `ClassRefContext::New`;
888+
// `#[ClassName(...)]` attribute usages carry
889+
// `ClassRefContext::Attribute`. Both invoke the
890+
// constructor.
891+
SymbolKind::ClassReference {
892+
name,
893+
is_fqn,
894+
context: ClassRefContext::New | ClassRefContext::Attribute,
895+
} => {
896+
let resolved = if *is_fqn {
897+
name
898+
} else if let Some(fqn) =
899+
resolved_names.as_ref().and_then(|rn| rn.get(span.start))
900+
{
901+
fqn
902+
} else {
903+
let use_map = file_use_map.get_or_init(|| {
904+
self.file_imports
905+
.read()
906+
.get(file_uri)
907+
.cloned()
908+
.unwrap_or_default()
909+
});
910+
&Self::resolve_to_fqn(name, use_map, &file_namespace)
911+
};
912+
scoped.contains(&normalize_fqn(strip_fqn_prefix(resolved)))
913+
}
914+
// Explicit constructor delegation written as
915+
// `parent::__construct()`, `self::__construct()`, or
916+
// `Foo::__construct()` lands here. Resolve the subject
917+
// class and keep the call when it falls within the
918+
// constructor's owning hierarchy.
919+
SymbolKind::MemberAccess {
920+
subject_text,
921+
member_name,
922+
is_static,
923+
..
924+
} if is_constructor_name(member_name) => {
925+
if file_content.is_none() {
926+
file_content = self.get_file_content_arc(file_uri);
927+
}
928+
match &file_content {
929+
Some(content) => {
930+
let ctx = file_ctx.get_or_init(|| self.file_context(file_uri));
931+
self.resolve_subject_to_fqns(
932+
subject_text,
933+
*is_static,
934+
ctx,
935+
span.start,
936+
content,
937+
)
938+
.iter()
939+
.any(|fqn| scoped.contains(&normalize_fqn(strip_fqn_prefix(fqn))))
940+
}
941+
None => false,
942+
}
943+
}
944+
_ => false,
945+
};
946+
947+
if matched {
948+
if file_content.is_none() {
949+
file_content = self.get_file_content_arc(file_uri);
950+
}
951+
if let Some(content) = &file_content {
952+
let start = offset_to_position(content, span.start as usize);
953+
let end = offset_to_position(content, span.end as usize);
954+
push_unique_location(&mut locations, &parsed_uri, start, end);
955+
}
956+
}
957+
}
958+
959+
// Optionally include the constructor declaration site(s).
960+
if include_declaration && let Some(classes) = self.get_classes_for_uri(file_uri) {
961+
for class in &classes {
962+
let class_fqn = normalize_fqn(&class.fqn()).to_string();
963+
if !scoped.contains(&class_fqn) {
964+
continue;
965+
}
966+
967+
for method in class.methods.iter() {
968+
if is_constructor_name(&method.name) && method.name_offset != 0 {
969+
if file_content.is_none() {
970+
file_content = self.get_file_content_arc(file_uri);
971+
}
972+
let Some(content) = &file_content else {
973+
break;
974+
};
975+
let offset = method.name_offset as usize;
976+
let start = offset_to_position(content, offset);
977+
let end = offset_to_position(content, offset + method.name.len());
978+
push_unique_location(&mut locations, &parsed_uri, start, end);
979+
}
980+
}
981+
}
982+
}
983+
}
984+
985+
locations.sort_by(|a, b| {
986+
a.uri
987+
.as_str()
988+
.cmp(b.uri.as_str())
989+
.then(a.range.start.line.cmp(&b.range.start.line))
990+
.then(a.range.start.character.cmp(&b.range.start.character))
991+
});
992+
locations.dedup();
993+
locations
994+
}
995+
996+
/// Expand the constructor owner class(es) into the full set of classes
997+
/// whose instantiation (`new X(...)`) invokes the same constructor.
998+
///
999+
/// Starting from `owner_fqns` (the class(es) that declare the
1000+
/// constructor under the cursor), walk down the inheritance tree and
1001+
/// include every descendant that does *not* declare its own
1002+
/// constructor (those inherit the owner's), pruning the walk at any
1003+
/// descendant that overrides it.
1004+
fn collect_constructor_hierarchy(&self, owner_fqns: &[String]) -> HashSet<String> {
1005+
let class_loader = |name: &str| -> Option<Arc<ClassInfo>> { self.find_or_load_class(name) };
1006+
let declares_ctor = |fqn: &str| -> bool {
1007+
class_loader(fqn)
1008+
.map(|c| c.methods.iter().any(|m| is_constructor_name(&m.name)))
1009+
.unwrap_or(false)
1010+
};
1011+
1012+
let owners: Vec<String> = owner_fqns.iter().map(|f| normalize_fqn(f)).collect();
1013+
let mut result: HashSet<String> = owners.iter().cloned().collect();
1014+
1015+
// Walk down from each owner, including inheriting descendants and
1016+
// pruning at overrides.
1017+
let gti = self.gti_index.read();
1018+
let mut queue: std::collections::VecDeque<String> = owners.iter().cloned().collect();
1019+
let mut seen: HashSet<String> = owners.iter().cloned().collect();
1020+
while let Some(fqn) = queue.pop_front() {
1021+
if let Some(descendants) = gti.get(&fqn) {
1022+
for desc in descendants {
1023+
let normalized = normalize_fqn(desc).to_string();
1024+
if !seen.insert(normalized.clone()) {
1025+
continue;
1026+
}
1027+
// A descendant that declares its own constructor uses a
1028+
// different constructor — exclude it and stop walking
1029+
// past it.
1030+
if declares_ctor(&normalized) {
1031+
continue;
1032+
}
1033+
result.insert(normalized.clone());
1034+
queue.push_back(normalized);
1035+
}
1036+
}
1037+
}
1038+
1039+
result
1040+
}
1041+
8061042
/// Find all references to a member (method, property, or constant)
8071043
/// across all files.
8081044
///
@@ -1791,6 +2027,13 @@ fn normalize_fqn(fqn: &str) -> String {
17912027
strip_fqn_prefix(fqn).to_string()
17922028
}
17932029

2030+
/// Whether a member name is the PHP constructor (`__construct`).
2031+
///
2032+
/// PHP method names are case-insensitive, so `__CONSTRUCT` matches too.
2033+
fn is_constructor_name(name: &str) -> bool {
2034+
name.eq_ignore_ascii_case("__construct")
2035+
}
2036+
17942037
/// Check whether a resolved class name matches the target FQN.
17952038
///
17962039
/// Two names match if their fully-qualified forms are equal, or if both

0 commit comments

Comments
 (0)