Skip to content

Commit 1eed3f8

Browse files
committed
Improve heuristic for object compleation
1 parent 9590a26 commit 1eed3f8

3 files changed

Lines changed: 597 additions & 18 deletions

File tree

src/completion/class_completion.rs

Lines changed: 116 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,44 @@ use crate::types::*;
1717

1818
use super::builder::{build_callable_snippet, build_use_edit, find_use_insert_position};
1919

20+
/// Heuristic check for class names that are unlikely to be instantiable.
21+
///
22+
/// Returns `true` when the short name matches common naming conventions
23+
/// for abstract classes and interfaces:
24+
///
25+
/// - **Abstract:** case-insensitive `"abstract"` as prefix or suffix
26+
/// (e.g. `AbstractController`, `HandlerAbstract`)
27+
/// - **Interface:** case-insensitive `"interface"` as suffix
28+
/// (e.g. `LoggerInterface`)
29+
/// - **I-prefix:** `I` followed by an uppercase letter
30+
/// (e.g. `ILogger`, `IRepository` — C#-style interface naming)
31+
/// - **Base-prefix:** `Base` followed by an uppercase letter
32+
/// (e.g. `BaseController`, `BaseModel`)
33+
fn likely_non_instantiable(short_name: &str) -> bool {
34+
let lower = short_name.to_ascii_lowercase();
35+
if lower.ends_with("abstract") || lower.starts_with("abstract") || lower.ends_with("interface")
36+
{
37+
return true;
38+
}
39+
// I[A-Z] prefix — C#-style interface naming (ILogger, IRepository).
40+
// The length check + ascii uppercase guard avoids matching `Image`, `Item`, etc.
41+
if short_name.starts_with('I') && short_name.len() >= 2 {
42+
let second = short_name.as_bytes()[1];
43+
if second.is_ascii_uppercase() {
44+
return true;
45+
}
46+
}
47+
// Base[A-Z] prefix (BaseController, BaseModel).
48+
// Requires uppercase after "Base" to avoid matching e.g. `Baseline`.
49+
if short_name.starts_with("Base") && short_name.len() >= 5 {
50+
let fifth = short_name.as_bytes()[4];
51+
if fifth.is_ascii_uppercase() {
52+
return true;
53+
}
54+
}
55+
false
56+
}
57+
2058
impl Backend {
2159
/// Extract the partial identifier (class name fragment) that the user
2260
/// is currently typing at the given cursor position.
@@ -229,6 +267,11 @@ impl Backend {
229267
if !seen_fqns.insert(fqn.clone()) {
230268
continue;
231269
}
270+
// After `new`, skip interfaces, traits, enums and abstract
271+
// classes that we already know about.
272+
if is_new && !self.is_instantiable_or_unloaded(fqn) {
273+
continue;
274+
}
232275
let (insert_text, insert_text_format) = if is_new {
233276
Self::build_new_insert(short_name, None)
234277
} else {
@@ -270,6 +313,10 @@ impl Backend {
270313
if !cls.name.to_lowercase().contains(&prefix_lower) {
271314
continue;
272315
}
316+
// After `new`, only concrete classes are valid.
317+
if is_new && (cls.kind != ClassLikeKind::Class || cls.is_abstract) {
318+
continue;
319+
}
273320
let fqn = format!("{}\\{}", ns, cls.name);
274321
if !seen_fqns.insert(fqn.clone()) {
275322
continue;
@@ -313,21 +360,32 @@ impl Backend {
313360
if !seen_fqns.insert(fqn.clone()) {
314361
continue;
315362
}
363+
// After `new`, skip non-concrete classes that are loaded.
364+
if is_new && !self.is_instantiable_or_unloaded(fqn) {
365+
continue;
366+
}
316367
let (insert_text, insert_text_format) = if is_new {
317368
// class_index is a FQN → URI map; the class may or
318369
// may not be fully loaded — just insert empty parens.
319370
(format!("{short_name}()$0"), Some(InsertTextFormat::SNIPPET))
320371
} else {
321372
(short_name.to_string(), None)
322373
};
374+
// In `new` context, demote names that look like abstract
375+
// classes or interfaces so concrete-looking names appear first.
376+
let sort_prefix = if is_new && likely_non_instantiable(short_name) {
377+
"7"
378+
} else {
379+
"2"
380+
};
323381
items.push(CompletionItem {
324382
label: short_name.to_string(),
325383
kind: Some(CompletionItemKind::CLASS),
326384
detail: Some(fqn.clone()),
327385
insert_text: Some(insert_text),
328386
insert_text_format,
329387
filter_text: Some(short_name.to_string()),
330-
sort_text: Some(format!("2_{}", short_name.to_lowercase())),
388+
sort_text: Some(format!("{}_{}", sort_prefix, short_name.to_lowercase())),
331389
additional_text_edits: build_use_edit(fqn, use_insert_pos, file_namespace),
332390
..CompletionItem::default()
333391
});
@@ -349,14 +407,21 @@ impl Backend {
349407
} else {
350408
(short_name.to_string(), None)
351409
};
410+
// In `new` context, demote names that look like abstract
411+
// classes or interfaces so concrete-looking names appear first.
412+
let sort_prefix = if is_new && likely_non_instantiable(short_name) {
413+
"8"
414+
} else {
415+
"3"
416+
};
352417
items.push(CompletionItem {
353418
label: short_name.to_string(),
354419
kind: Some(CompletionItemKind::CLASS),
355420
detail: Some(fqn.clone()),
356421
insert_text: Some(insert_text),
357422
insert_text_format,
358423
filter_text: Some(short_name.to_string()),
359-
sort_text: Some(format!("3_{}", short_name.to_lowercase())),
424+
sort_text: Some(format!("{}_{}", sort_prefix, short_name.to_lowercase())),
360425
additional_text_edits: build_use_edit(fqn, use_insert_pos, file_namespace),
361426
..CompletionItem::default()
362427
});
@@ -379,14 +444,21 @@ impl Backend {
379444
} else {
380445
(short_name.to_string(), None)
381446
};
447+
// In `new` context, demote names that look like abstract
448+
// classes or interfaces so concrete-looking names appear first.
449+
let sort_prefix = if is_new && likely_non_instantiable(short_name) {
450+
"9"
451+
} else {
452+
"4"
453+
};
382454
items.push(CompletionItem {
383455
label: short_name.to_string(),
384456
kind: Some(CompletionItemKind::CLASS),
385457
detail: Some(name.to_string()),
386458
insert_text: Some(insert_text),
387459
insert_text_format,
388460
filter_text: Some(short_name.to_string()),
389-
sort_text: Some(format!("4_{}", short_name.to_lowercase())),
461+
sort_text: Some(format!("{}_{}", sort_prefix, short_name.to_lowercase())),
390462
additional_text_edits: build_use_edit(name, use_insert_pos, file_namespace),
391463
..CompletionItem::default()
392464
});
@@ -473,6 +545,47 @@ impl Backend {
473545
}
474546
}
475547

548+
/// Check whether the class identified by `class_name` is instantiable
549+
/// or simply not loaded yet.
550+
///
551+
/// Returns `true` when the class is **not** found in the `ast_map`
552+
/// (unloaded — we cannot tell, so we allow it) **or** when it is
553+
/// found and is a concrete, non-abstract `class`.
554+
///
555+
/// Returns `false` only when the class **is** loaded and is an
556+
/// interface, trait, enum, or abstract class. This never triggers
557+
/// disk I/O.
558+
fn is_instantiable_or_unloaded(&self, class_name: &str) -> bool {
559+
let normalized = class_name.strip_prefix('\\').unwrap_or(class_name);
560+
let last_segment = normalized.rsplit('\\').next().unwrap_or(normalized);
561+
let expected_ns: Option<&str> = if normalized.contains('\\') {
562+
Some(&normalized[..normalized.len() - last_segment.len() - 1])
563+
} else {
564+
None
565+
};
566+
567+
let Some(map) = self.ast_map.lock().ok() else {
568+
return true;
569+
};
570+
let nmap = self.namespace_map.lock().ok();
571+
for (uri, classes) in map.iter() {
572+
if let Some(c) = classes.iter().find(|c| c.name == last_segment) {
573+
if let Some(exp_ns) = expected_ns {
574+
let file_ns = nmap
575+
.as_ref()
576+
.and_then(|nm| nm.get(uri))
577+
.and_then(|opt| opt.as_deref());
578+
if file_ns != Some(exp_ns) {
579+
continue;
580+
}
581+
}
582+
return c.kind == ClassLikeKind::Class && !c.is_abstract;
583+
}
584+
}
585+
// Not found in ast_map — unloaded, so allow it.
586+
true
587+
}
588+
476589
/// Check whether the class identified by `class_name` is a concrete,
477590
/// non-abstract `class` (i.e. `ClassLikeKind::Class` and not
478591
/// `is_abstract`) in the `ast_map`.

src/completion/handler.rs

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -513,20 +513,34 @@ impl Backend {
513513
&content,
514514
is_new,
515515
);
516-
let (constant_items, const_incomplete) = self.build_constant_completions(&partial);
517-
let (function_items, func_incomplete) = self.build_function_completions(&partial);
518-
519-
if !class_items.is_empty()
520-
|| !constant_items.is_empty()
521-
|| !function_items.is_empty()
522-
{
523-
let mut items = class_items;
524-
items.extend(constant_items);
525-
items.extend(function_items);
526-
return Ok(Some(CompletionResponse::List(CompletionList {
527-
is_incomplete: class_incomplete || const_incomplete || func_incomplete,
528-
items,
529-
})));
516+
517+
// After `new`, only class names are valid — skip
518+
// constants and functions.
519+
if is_new {
520+
if !class_items.is_empty() {
521+
return Ok(Some(CompletionResponse::List(CompletionList {
522+
is_incomplete: class_incomplete,
523+
items: class_items,
524+
})));
525+
}
526+
} else {
527+
let (constant_items, const_incomplete) =
528+
self.build_constant_completions(&partial);
529+
let (function_items, func_incomplete) =
530+
self.build_function_completions(&partial);
531+
532+
if !class_items.is_empty()
533+
|| !constant_items.is_empty()
534+
|| !function_items.is_empty()
535+
{
536+
let mut items = class_items;
537+
items.extend(constant_items);
538+
items.extend(function_items);
539+
return Ok(Some(CompletionResponse::List(CompletionList {
540+
is_incomplete: class_incomplete || const_incomplete || func_incomplete,
541+
items,
542+
})));
543+
}
530544
}
531545
}
532546
}

0 commit comments

Comments
 (0)