Skip to content

Commit a354629

Browse files
committed
Fix diagnostics, import actions, and references for global classes
1 parent 410f8c4 commit a354629

5 files changed

Lines changed: 406 additions & 6 deletions

File tree

docs/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3838

3939
### Fixed
4040

41+
- **Missing diagnostics and import actions in files without a namespace.** When a namespaced class (e.g. `Carbon\Carbon`) had already been parsed, using its short name (`Carbon`) in a file without a `namespace` declaration incorrectly resolved against the namespaced class. This suppressed both the "class not found" diagnostic and the "Import" code action. Bare-name lookups now only match classes that are themselves in the global namespace.
42+
- **Find-references false positives for global classes.** Searching for references to a global-scope class (e.g. `Helper` with no namespace) could include references to unrelated namespaced classes with the same short name (e.g. `App\Helper`). Short-name fallback matching now only applies when the resolved name is unqualified.
4143
- **Fluent chains only flag the first broken link.** In a chain where the first method does not exist, only that method is flagged instead of every subsequent call receiving its own warning.
4244
- **Null narrowing from `!== null` checks.** Null-initialized variables guarded by `$var !== null`, `!is_null()`, or bare truthy checks now have `null` narrowed away inside the then-body and in subsequent `&&` operands. Works in chained conditions, ternary expressions, and return statements.
4345
- **Variables assigned inside `if`/`while` conditions now resolve in the body.** `if ($admin = AdminUser::first())` and `while ($row = nextRow())` register the assignment so the variable has a type inside the branch or loop body.

src/code_actions/import_class.rs

Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,4 +727,292 @@ mod tests {
727727
"should not offer conflicting import"
728728
);
729729
}
730+
731+
// ── No-namespace file tests ─────────────────────────────────────────
732+
733+
#[test]
734+
fn import_action_offered_in_no_namespace_file_for_new_expression() {
735+
let backend = crate::Backend::new_test();
736+
let uri = "file:///test.php";
737+
// File has NO namespace declaration.
738+
let content = "<?php\n\nnew Request();\n";
739+
740+
backend.update_ast(uri, content);
741+
742+
{
743+
let mut cmap = backend.classmap.write();
744+
cmap.insert(
745+
"Illuminate\\Http\\Request".to_string(),
746+
"/vendor/laravel/framework/src/Illuminate/Http/Request.php".into(),
747+
);
748+
}
749+
750+
// Range covering "Request" on line 2.
751+
let params = CodeActionParams {
752+
text_document: TextDocumentIdentifier {
753+
uri: uri.parse().unwrap(),
754+
},
755+
range: Range {
756+
start: Position::new(2, 4),
757+
end: Position::new(2, 11),
758+
},
759+
context: CodeActionContext {
760+
diagnostics: vec![],
761+
only: None,
762+
trigger_kind: None,
763+
},
764+
work_done_progress_params: Default::default(),
765+
partial_result_params: Default::default(),
766+
};
767+
768+
let actions = backend.handle_code_action(uri, content, &params);
769+
assert!(
770+
actions.iter().any(|a| {
771+
if let CodeActionOrCommand::CodeAction(ca) = a {
772+
ca.title.contains("Illuminate\\Http\\Request")
773+
} else {
774+
false
775+
}
776+
}),
777+
"expected an import action for Illuminate\\Http\\Request in no-namespace file, got: {:?}",
778+
actions
779+
.iter()
780+
.map(|a| match a {
781+
CodeActionOrCommand::CodeAction(ca) => ca.title.clone(),
782+
CodeActionOrCommand::Command(c) => c.title.clone(),
783+
})
784+
.collect::<Vec<_>>()
785+
);
786+
}
787+
788+
#[test]
789+
fn import_action_offered_in_no_namespace_file_for_static_call() {
790+
let backend = crate::Backend::new_test();
791+
let uri = "file:///test.php";
792+
// File has NO namespace — reproduces issue #59.
793+
let content = "<?php\n\nfunction () {\n return Carbon::now();\n};\n";
794+
795+
backend.update_ast(uri, content);
796+
797+
{
798+
let mut cmap = backend.classmap.write();
799+
cmap.insert(
800+
"Carbon\\Carbon".to_string(),
801+
"/vendor/nesbot/carbon/src/Carbon/Carbon.php".into(),
802+
);
803+
}
804+
805+
// Range covering "Carbon" on line 3 (the class name in Carbon::now()).
806+
let params = CodeActionParams {
807+
text_document: TextDocumentIdentifier {
808+
uri: uri.parse().unwrap(),
809+
},
810+
range: Range {
811+
start: Position::new(3, 11),
812+
end: Position::new(3, 17),
813+
},
814+
context: CodeActionContext {
815+
diagnostics: vec![],
816+
only: None,
817+
trigger_kind: None,
818+
},
819+
work_done_progress_params: Default::default(),
820+
partial_result_params: Default::default(),
821+
};
822+
823+
let actions = backend.handle_code_action(uri, content, &params);
824+
assert!(
825+
actions.iter().any(|a| {
826+
if let CodeActionOrCommand::CodeAction(ca) = a {
827+
ca.title.contains("Carbon\\Carbon")
828+
} else {
829+
false
830+
}
831+
}),
832+
"expected an import action for Carbon\\Carbon in no-namespace file, got: {:?}",
833+
actions
834+
.iter()
835+
.map(|a| match a {
836+
CodeActionOrCommand::CodeAction(ca) => ca.title.clone(),
837+
CodeActionOrCommand::Command(c) => c.title.clone(),
838+
})
839+
.collect::<Vec<_>>()
840+
);
841+
}
842+
843+
#[test]
844+
fn import_action_inserts_use_after_php_open_in_no_namespace_file() {
845+
let backend = crate::Backend::new_test();
846+
let uri = "file:///test.php";
847+
let content = "<?php\n\nnew Request();\n";
848+
849+
backend.update_ast(uri, content);
850+
851+
{
852+
let mut cmap = backend.classmap.write();
853+
cmap.insert(
854+
"Illuminate\\Http\\Request".to_string(),
855+
"/vendor/laravel/framework/src/Illuminate/Http/Request.php".into(),
856+
);
857+
}
858+
859+
let params = CodeActionParams {
860+
text_document: TextDocumentIdentifier {
861+
uri: uri.parse().unwrap(),
862+
},
863+
range: Range {
864+
start: Position::new(2, 4),
865+
end: Position::new(2, 11),
866+
},
867+
context: CodeActionContext {
868+
diagnostics: vec![],
869+
only: None,
870+
trigger_kind: None,
871+
},
872+
work_done_progress_params: Default::default(),
873+
partial_result_params: Default::default(),
874+
};
875+
876+
let actions = backend.handle_code_action(uri, content, &params);
877+
let action = actions
878+
.iter()
879+
.find_map(|a| match a {
880+
CodeActionOrCommand::CodeAction(ca)
881+
if ca.title.contains("Illuminate\\Http\\Request") =>
882+
{
883+
Some(ca)
884+
}
885+
_ => None,
886+
})
887+
.expect("expected import action");
888+
889+
let edit = action.edit.as_ref().expect("expected workspace edit");
890+
let changes = edit.changes.as_ref().expect("expected changes");
891+
let file_edits = changes
892+
.get(&uri.parse::<Url>().unwrap())
893+
.expect("expected edits for the file");
894+
assert_eq!(file_edits.len(), 1);
895+
assert_eq!(file_edits[0].new_text, "use Illuminate\\Http\\Request;\n");
896+
// Should insert after `<?php` (line 1), not line 0.
897+
assert_eq!(file_edits[0].range.start.line, 1);
898+
}
899+
900+
#[test]
901+
fn no_import_action_for_known_global_class_in_no_namespace_file() {
902+
let backend = crate::Backend::new_test();
903+
let uri_dep = "file:///dep.php";
904+
let content_dep = "<?php\nclass Helper {}\n";
905+
backend.update_ast(uri_dep, content_dep);
906+
907+
{
908+
let mut idx = backend.class_index.write();
909+
idx.insert("Helper".to_string(), uri_dep.to_string());
910+
}
911+
912+
let uri = "file:///test.php";
913+
let content = "<?php\n\nnew Helper();\n";
914+
backend.update_ast(uri, content);
915+
916+
let params = CodeActionParams {
917+
text_document: TextDocumentIdentifier {
918+
uri: uri.parse().unwrap(),
919+
},
920+
range: Range {
921+
start: Position::new(2, 4),
922+
end: Position::new(2, 10),
923+
},
924+
context: CodeActionContext {
925+
diagnostics: vec![],
926+
only: None,
927+
trigger_kind: None,
928+
},
929+
work_done_progress_params: Default::default(),
930+
partial_result_params: Default::default(),
931+
};
932+
933+
let actions = backend.handle_code_action(uri, content, &params);
934+
let import_actions: Vec<_> = actions
935+
.iter()
936+
.filter(|a| match a {
937+
CodeActionOrCommand::CodeAction(ca) => ca.title.starts_with("Import"),
938+
_ => false,
939+
})
940+
.collect();
941+
assert!(
942+
import_actions.is_empty(),
943+
"should not offer import for a known global class in no-namespace file, got: {:?}",
944+
import_actions
945+
.iter()
946+
.map(|a| match a {
947+
CodeActionOrCommand::CodeAction(ca) => ca.title.clone(),
948+
_ => String::new(),
949+
})
950+
.collect::<Vec<_>>()
951+
);
952+
}
953+
954+
#[test]
955+
fn import_action_offered_when_namespaced_class_in_ast_map() {
956+
// Reproduces issue #59: when a namespaced class like `Carbon\Carbon`
957+
// is already parsed and in the ast_map, `find_or_load_class("Carbon")`
958+
// must NOT match it — the bare name `"Carbon"` is a global-scope
959+
// lookup and should not resolve to `Carbon\Carbon`.
960+
//
961+
// Without the fix, `find_class_in_ast_map("Carbon")` ignores the
962+
// namespace filter when `expected_ns` is `None`, so ANY class with
963+
// short name `Carbon` matches. The import action then skips it
964+
// thinking "this class resolves in global scope".
965+
let backend = crate::Backend::new_test();
966+
967+
// Parse the dependency file so Carbon\Carbon is in the ast_map.
968+
let uri_dep = "file:///vendor/carbon.php";
969+
let content_dep = "<?php\nnamespace Carbon;\n\nclass Carbon {}\n";
970+
backend.update_ast(uri_dep, content_dep);
971+
{
972+
let mut idx = backend.class_index.write();
973+
idx.insert("Carbon\\Carbon".to_string(), uri_dep.to_string());
974+
}
975+
976+
// The file under edit has NO namespace.
977+
let uri = "file:///test.php";
978+
let content = "<?php\n\nfunction () {\n return Carbon::now();\n};\n";
979+
backend.update_ast(uri, content);
980+
981+
// Range covering "Carbon" on line 3.
982+
let params = CodeActionParams {
983+
text_document: TextDocumentIdentifier {
984+
uri: uri.parse().unwrap(),
985+
},
986+
range: Range {
987+
start: Position::new(3, 11),
988+
end: Position::new(3, 17),
989+
},
990+
context: CodeActionContext {
991+
diagnostics: vec![],
992+
only: None,
993+
trigger_kind: None,
994+
},
995+
work_done_progress_params: Default::default(),
996+
partial_result_params: Default::default(),
997+
};
998+
999+
let actions = backend.handle_code_action(uri, content, &params);
1000+
assert!(
1001+
actions.iter().any(|a| {
1002+
if let CodeActionOrCommand::CodeAction(ca) = a {
1003+
ca.title.contains("Carbon\\Carbon")
1004+
} else {
1005+
false
1006+
}
1007+
}),
1008+
"expected an import action for Carbon\\Carbon when the namespaced class is in ast_map, got: {:?}",
1009+
actions
1010+
.iter()
1011+
.map(|a| match a {
1012+
CodeActionOrCommand::CodeAction(ca) => ca.title.clone(),
1013+
CodeActionOrCommand::Command(c) => c.title.clone(),
1014+
})
1015+
.collect::<Vec<_>>()
1016+
);
1017+
}
7301018
}

src/diagnostics/unknown_classes.rs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,4 +1015,94 @@ mod tests {
10151015
diags.iter().map(|d| &d.message).collect::<Vec<_>>()
10161016
);
10171017
}
1018+
1019+
// ── No-namespace file tests ─────────────────────────────────────────
1020+
1021+
#[test]
1022+
fn diagnostic_when_namespaced_class_in_ast_map() {
1023+
// Reproduces issue #59: when `Carbon\Carbon` is already parsed
1024+
// and in the ast_map, `find_or_load_class("Carbon")` must NOT
1025+
// match it — the bare name is a global-scope lookup. Without
1026+
// the fix the no-namespace fallback at step 3 resolves the bare
1027+
// name to the namespaced class, suppressing the diagnostic.
1028+
let backend = Backend::new_test();
1029+
1030+
// Parse the dependency so Carbon\Carbon is in the ast_map.
1031+
let uri_dep = "file:///vendor/carbon.php";
1032+
let content_dep = "<?php\nnamespace Carbon;\n\nclass Carbon {}\n";
1033+
backend.update_ast(uri_dep, content_dep);
1034+
{
1035+
let mut idx = backend.class_index.write();
1036+
idx.insert("Carbon\\Carbon".to_string(), uri_dep.to_string());
1037+
}
1038+
1039+
let uri = "file:///test.php";
1040+
let content = "<?php\n\nfunction () {\n return Carbon::now();\n};\n";
1041+
1042+
let diags = collect(&backend, uri, content);
1043+
assert!(
1044+
diags.iter().any(|d| d.message.contains("Carbon")),
1045+
"expected unknown-class diagnostic for Carbon even when Carbon\\Carbon is in ast_map, got: {:?}",
1046+
diags.iter().map(|d| &d.message).collect::<Vec<_>>()
1047+
);
1048+
}
1049+
1050+
#[test]
1051+
fn diagnostic_for_unknown_class_in_no_namespace_file() {
1052+
// In a file without a namespace, an unresolved class name should
1053+
// still produce a diagnostic.
1054+
let backend = Backend::new_test();
1055+
let uri = "file:///test.php";
1056+
let content = "<?php\n\nnew Request();\n";
1057+
1058+
let diags = collect(&backend, uri, content);
1059+
assert!(
1060+
diags.iter().any(|d| d.message.contains("Request")),
1061+
"expected unknown-class diagnostic for Request in no-namespace file, got: {:?}",
1062+
diags.iter().map(|d| &d.message).collect::<Vec<_>>()
1063+
);
1064+
}
1065+
1066+
#[test]
1067+
fn diagnostic_for_unknown_static_class_in_no_namespace_file() {
1068+
// Reproduces issue #59: `Carbon::now()` in a file without a
1069+
// namespace should emit a diagnostic for unresolved `Carbon`.
1070+
let backend = Backend::new_test();
1071+
let uri = "file:///test.php";
1072+
let content = "<?php\n\nfunction () {\n return Carbon::now();\n};\n";
1073+
1074+
let diags = collect(&backend, uri, content);
1075+
assert!(
1076+
diags.iter().any(|d| d.message.contains("Carbon")),
1077+
"expected unknown-class diagnostic for Carbon in no-namespace file, got: {:?}",
1078+
diags.iter().map(|d| &d.message).collect::<Vec<_>>()
1079+
);
1080+
}
1081+
1082+
#[test]
1083+
fn no_diagnostic_for_imported_class_in_no_namespace_file() {
1084+
// A `use` statement in a no-namespace file should suppress the
1085+
// diagnostic, just like in a namespaced file.
1086+
let backend = Backend::new_test();
1087+
1088+
// Register the class so it can be found.
1089+
let uri_dep = "file:///carbon.php";
1090+
let content_dep = "<?php\nnamespace Carbon;\n\nclass Carbon {}\n";
1091+
backend.update_ast(uri_dep, content_dep);
1092+
{
1093+
let mut idx = backend.class_index.write();
1094+
idx.insert("Carbon\\Carbon".to_string(), uri_dep.to_string());
1095+
}
1096+
1097+
let uri = "file:///test.php";
1098+
let content =
1099+
"<?php\n\nuse Carbon\\Carbon;\n\nfunction () {\n return Carbon::now();\n};\n";
1100+
1101+
let diags = collect(&backend, uri, content);
1102+
assert!(
1103+
!diags.iter().any(|d| d.message.contains("Carbon")),
1104+
"should not flag imported Carbon class, got: {:?}",
1105+
diags.iter().map(|d| &d.message).collect::<Vec<_>>()
1106+
);
1107+
}
10181108
}

0 commit comments

Comments
 (0)