Skip to content

Commit 9590a26

Browse files
committed
Be more agressive about when we do compleation
1 parent e3eac17 commit 9590a26

11 files changed

Lines changed: 131 additions & 219 deletions

src/completion/handler.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -531,14 +531,8 @@ impl Backend {
531531
}
532532
}
533533

534-
// Fallback: return the default PHPantomLSP completion item
535-
Ok(Some(CompletionResponse::Array(vec![CompletionItem {
536-
label: "PHPantomLSP".to_string(),
537-
kind: Some(CompletionItemKind::TEXT),
538-
detail: Some("PHPantomLSP completion".to_string()),
539-
insert_text: Some("PHPantomLSP".to_string()),
540-
..CompletionItem::default()
541-
}])))
534+
// Nothing matched — return no completions.
535+
Ok(None)
542536
}
543537

544538
/// Insert `);` at the given cursor position in `content`.

src/server.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ impl LanguageServer for Backend {
4040
"'".to_string(),
4141
"\"".to_string(),
4242
"[".to_string(),
43+
" ".to_string(),
4344
]),
4445
all_commit_characters: None,
4546
work_done_progress_options: WorkDoneProgressOptions {

tests/completion_basic.rs

Lines changed: 16 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ async fn test_completion_interface_constant_via_double_colon() {
142142
// ─── Basic Completion Tests ─────────────────────────────────────────────────
143143

144144
#[tokio::test]
145-
async fn test_completion_returns_phpantomlsp() {
145+
async fn test_completion_returns_none_when_nothing_matches() {
146146
let backend = create_test_backend();
147147

148148
let uri = Url::parse("file:///test.php").unwrap();
@@ -172,15 +172,10 @@ async fn test_completion_returns_phpantomlsp() {
172172
};
173173

174174
let result = backend.completion(completion_params).await.unwrap();
175-
assert!(result.is_some(), "Completion should return results");
176-
177-
match result.unwrap() {
178-
CompletionResponse::Array(items) => {
179-
assert!(!items.is_empty(), "Should have at least one item");
180-
assert_eq!(items[0].label, "PHPantomLSP");
181-
}
182-
_ => panic!("Expected CompletionResponse::Array"),
183-
}
175+
assert!(
176+
result.is_none(),
177+
"Completion should return None when nothing matches"
178+
);
184179
}
185180

186181
#[tokio::test]
@@ -306,15 +301,10 @@ async fn test_completion_outside_class_returns_fallback() {
306301
};
307302

308303
let result = backend.completion(completion_params).await.unwrap();
309-
assert!(result.is_some());
310-
311-
match result.unwrap() {
312-
CompletionResponse::Array(items) => {
313-
assert_eq!(items.len(), 1, "Should fall back to default item");
314-
assert_eq!(items[0].label, "PHPantomLSP");
315-
}
316-
_ => panic!("Expected CompletionResponse::Array"),
317-
}
304+
assert!(
305+
result.is_none(),
306+
"Cursor outside class with no operator should return None"
307+
);
318308
}
319309

320310
#[tokio::test]
@@ -421,16 +411,11 @@ async fn test_completion_empty_class_falls_back() {
421411
};
422412

423413
let result = backend.completion(completion_params).await.unwrap();
424-
assert!(result.is_some());
425-
426-
// Empty class has no methods or properties, so should fall back to PHPantomLSP
427-
match result.unwrap() {
428-
CompletionResponse::Array(items) => {
429-
assert_eq!(items.len(), 1);
430-
assert_eq!(items[0].label, "PHPantomLSP");
431-
}
432-
_ => panic!("Expected CompletionResponse::Array"),
433-
}
414+
// Empty class has no methods or properties, so should return None
415+
assert!(
416+
result.is_none(),
417+
"Empty class with no members should return None"
418+
);
434419
}
435420

436421
#[tokio::test]
@@ -475,14 +460,6 @@ async fn test_completion_no_access_operator_shows_fallback() {
475460
};
476461

477462
let result = backend.completion(completion_params).await.unwrap();
478-
assert!(result.is_some(), "Completion should return fallback");
479-
480-
// Without `->` or `::`, no class members should be suggested — only fallback
481-
match result.unwrap() {
482-
CompletionResponse::Array(items) => {
483-
assert_eq!(items.len(), 1, "Should only have fallback item");
484-
assert_eq!(items[0].label, "PHPantomLSP");
485-
}
486-
_ => panic!("Expected CompletionResponse::Array"),
487-
}
463+
// Without `->` or `::`, no class members should be suggested
464+
assert!(result.is_none(), "No access operator should return None");
488465
}

tests/completion_cross_file.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -411,15 +411,10 @@ async fn test_cross_file_no_psr4_mapping_falls_back() {
411411
};
412412

413413
let result = backend.completion(completion_params).await.unwrap();
414-
assert!(result.is_some());
415-
416-
match result.unwrap() {
417-
CompletionResponse::Array(items) => {
418-
assert_eq!(items.len(), 1, "Should fall back to default item");
419-
assert_eq!(items[0].label, "PHPantomLSP");
420-
}
421-
_ => panic!("Expected CompletionResponse::Array"),
422-
}
414+
assert!(
415+
result.is_none(),
416+
"Should return None when no PSR-4 mapping resolves the class"
417+
);
423418
}
424419

425420
#[tokio::test]

tests/completion_object_shapes.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -778,9 +778,12 @@ async fn test_bare_object_no_completions() {
778778
};
779779

780780
let result = backend.completion(completion_params).await.unwrap();
781-
// Bare `object` has no properties — result may be None or contain only
782-
// the fallback PHPantomLSP item.
783-
if let Some(CompletionResponse::Array(items)) = result {
781+
// Bare `object` has no properties — result should be None.
782+
if let Some(resp) = result {
783+
let items = match resp {
784+
CompletionResponse::Array(items) => items,
785+
CompletionResponse::List(list) => list.items,
786+
};
784787
let class_members: Vec<&str> = items
785788
.iter()
786789
.filter(|i| {

tests/completion_parent.rs

Lines changed: 14 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -503,16 +503,11 @@ async fn test_completion_parent_double_colon_no_parent_falls_back() {
503503
};
504504

505505
let result = backend.completion(completion_params).await.unwrap();
506-
assert!(result.is_some());
507-
508-
// Should fall back to the default PHPantomLSP item since there's no parent
509-
match result.unwrap() {
510-
CompletionResponse::Array(items) => {
511-
assert_eq!(items.len(), 1, "Should fall back to default completion");
512-
assert_eq!(items[0].label, "PHPantomLSP");
513-
}
514-
_ => panic!("Expected CompletionResponse::Array"),
515-
}
506+
// Should return None since there's no parent class
507+
assert!(
508+
result.is_none(),
509+
"parent:: with no parent class should return None"
510+
);
516511
}
517512

518513
#[tokio::test]
@@ -627,21 +622,11 @@ async fn test_completion_static_double_colon_suppressed_on_final_class() {
627622
};
628623

629624
let result = backend.completion(completion_params).await.unwrap();
630-
assert!(result.is_some());
631-
632-
// Should fall back to the default PHPantomLSP item since the class is final
633-
match result.unwrap() {
634-
CompletionResponse::Array(items) => {
635-
assert_eq!(
636-
items.len(),
637-
1,
638-
"static:: in a final class should fall back to default, got: {:?}",
639-
items.iter().map(|i| &i.label).collect::<Vec<_>>()
640-
);
641-
assert_eq!(items[0].label, "PHPantomLSP");
642-
}
643-
_ => panic!("Expected CompletionResponse::Array"),
644-
}
625+
// Should return None since the class is final — static:: is suppressed
626+
assert!(
627+
result.is_none(),
628+
"static:: in a final class should return None"
629+
);
645630
}
646631

647632
/// `static::` inside a non-final class should work as normal.
@@ -744,20 +729,10 @@ async fn test_completion_static_double_colon_suppressed_on_enum() {
744729
};
745730

746731
let result = backend.completion(completion_params).await.unwrap();
747-
assert!(result.is_some());
748-
749-
match result.unwrap() {
750-
CompletionResponse::Array(items) => {
751-
assert_eq!(
752-
items.len(),
753-
1,
754-
"static:: in an enum should fall back to default (enums are final), got: {:?}",
755-
items.iter().map(|i| &i.label).collect::<Vec<_>>()
756-
);
757-
assert_eq!(items[0].label, "PHPantomLSP");
758-
}
759-
_ => panic!("Expected CompletionResponse::Array"),
760-
}
732+
assert!(
733+
result.is_none(),
734+
"static:: in an enum should return None (enums are final)"
735+
);
761736
}
762737

763738
/// `self::` should still work on final classes (only `static::` is suppressed).

tests/completion_properties.rs

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,22 +1238,11 @@ async fn test_completion_promoted_property_param_scalar_not_overridden() {
12381238

12391239
let result = backend.completion(completion_params).await.unwrap();
12401240
// Scalar `int` should not be overridden — no class members to complete.
1241-
match result {
1242-
None => {} // acceptable
1243-
Some(CompletionResponse::Array(items)) => {
1244-
let labels: Vec<&str> = items.iter().map(|i| i.label.as_str()).collect();
1245-
let meaningful: Vec<&&str> = labels
1246-
.iter()
1247-
.filter(|l| !l.contains("PHPantomLSP"))
1248-
.collect();
1249-
assert!(
1250-
meaningful.is_empty(),
1251-
"Scalar promoted property should not resolve to class. Got: {:?}",
1252-
labels
1253-
);
1254-
}
1255-
Some(CompletionResponse::List(_)) => panic!("Expected Array response"),
1256-
}
1241+
assert!(
1242+
result.is_none(),
1243+
"Scalar promoted property should not resolve to class, got: {:?}",
1244+
result
1245+
);
12571246
}
12581247

12591248
/// Property chain on non-$this variable with promoted property @param override.

tests/completion_property_chains.rs

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -950,25 +950,12 @@ async fn test_var_property_chain_nonexistent_property() {
950950
};
951951

952952
let result = backend.completion(completion_params).await.unwrap();
953-
// Should either return None, an empty list, or only the default
954-
// "PHPantomLSP" placeholder — no crash, no wrong results.
955-
match result {
956-
None => {} // acceptable
957-
Some(CompletionResponse::Array(items)) => {
958-
let labels: Vec<&str> = items.iter().map(|i| i.label.as_str()).collect();
959-
// The only acceptable items are none or the default placeholder.
960-
let meaningful: Vec<&&str> = labels
961-
.iter()
962-
.filter(|l| !l.contains("PHPantomLSP"))
963-
.collect();
964-
assert!(
965-
meaningful.is_empty(),
966-
"Should return no meaningful results for nonexistent property chain. Got: {:?}",
967-
labels
968-
);
969-
}
970-
Some(CompletionResponse::List(_)) => panic!("Expected Array response"),
971-
}
953+
// Should return None — no crash, no wrong results.
954+
assert!(
955+
result.is_none(),
956+
"Should return None for nonexistent property chain, got: {:?}",
957+
result
958+
);
972959
}
973960

974961
// ─── Foreach variable property chain ────────────────────────────────────────

0 commit comments

Comments
 (0)