diff --git a/laravel-lsp/src/tests/env_completion_context_gate.rs b/laravel-lsp/src/tests/env_completion_context_gate.rs new file mode 100644 index 00000000..a7014cec --- /dev/null +++ b/laravel-lsp/src/tests/env_completion_context_gate.rs @@ -0,0 +1,308 @@ +//! Which files the completion handler treats as env files (issue #345). +//! +//! `completion` picks the context helper for the cursor from the file's +//! classification: an env file gets `${...}` interpolation, a PHPUnit XML file +//! gets its `` and `` attributes, and everything +//! else — PHP and Blade included — gets `env('...')` call completion. That +//! choice used to gate on `path.contains(".env")`, a substring test over the +//! whole path, so every file under a directory such as `.envs/` classified as +//! an env file. A `.php` file there was handed the interpolation helper, so +//! its `env('…')` calls never reached the call-context helper that completes +//! them. +//! +//! `env_key_locator::path_is_env_file` now owns the decision, and its own unit +//! tests pin the predicate. They say nothing about whether this dispatch +//! consults it: a predicate can be perfect while the handler that classifies +//! with it reads something else entirely. `env_source_registration_gate` +//! makes the same argument for the Salsa-registration call site; this module +//! is its counterpart for the completion call site. +//! +//! So everything here drives the **real** `textDocument/completion` request +//! and asserts on the items it returns. The two directions are both pinned, +//! because a one-directional test cannot tell a narrowed gate from a closed +//! one: +//! +//! - a rejected file must not get interpolation completion, and +//! - a real `.env` variant must still get it. + +use crate::LaravelLanguageServer; +use std::fs; +use std::path::{Path, PathBuf}; +use tempfile::TempDir; +use tower_lsp::lsp_types::{ + CompletionParams, CompletionResponse, DidChangeTextDocumentParams, Position, + TextDocumentContentChangeEvent, TextDocumentIdentifier, TextDocumentPositionParams, Url, + VersionedTextDocumentIdentifier, +}; +use tower_lsp::{LanguageServer, LspService}; + +/// The variable the project's own `.env` declares. Deliberately unlike any +/// real environment variable. +/// +/// The handler offers only what the project declares. It *used to* merge the +/// server's own `std::env::vars()` into that list and no longer does — the +/// leak was issue #342, `main.rs` states its removal outright where the items +/// are built, and `env_completion_system_leak` pins it. So this sentinel +/// guards against a regression rather than describing current behaviour: were +/// the merge ever to return, a name colliding with the machine's environment +/// would make a rejection test's empty result mean "no match" instead of +/// "no context", and it would pass for the wrong reason. +const SENTINEL: &str = "ZL345_SENTINEL"; + +/// The prefix typed at the cursor. Must remain a prefix of [`SENTINEL`], so +/// every fixture below builds its template from this constant rather than +/// spelling the prefix out: a rejection test asserts that *nothing* is +/// offered, and a prefix that had drifted out of sync with the sentinel would +/// satisfy that assertion by matching nothing at all. The positive tests share +/// the constant and so keep it honest — they fail if it stops matching. +/// +/// That downstream protection is real, but it lands in the wrong place: drift +/// reddens the two positive controls, whose names point at env-variant +/// classification and PHP call completion and say nothing about constant +/// drift. The assertion below fails at the constants instead. +const PREFIX: &str = "ZL345_"; + +const _: () = assert!( + is_proper_prefix(PREFIX.as_bytes(), SENTINEL.as_bytes()), + "PREFIX must remain a proper prefix of SENTINEL: every fixture here types \ + PREFIX at the cursor and expects SENTINEL back, so drift would leave the \ + rejection tests asserting emptiness against a prefix that matches nothing" +); + +/// Whether `prefix` is a proper prefix of `full`. A `const fn` because the +/// invariant it serves is a property of the constants themselves, so it is +/// checked when the crate compiles rather than when some test happens to run. +const fn is_proper_prefix(prefix: &[u8], full: &[u8]) -> bool { + if prefix.len() >= full.len() { + return false; + } + let mut i = 0; + while i < prefix.len() { + if prefix[i] != full[i] { + return false; + } + i += 1; + } + true +} + +/// A backend with `root_path` primed and the Salsa debounce removed, so a +/// `did_change` can be awaited deterministically instead of slept on. +/// Mirrors `env_source_registration_gate::backend_for`. +async fn backend_for(root: &Path) -> LaravelLanguageServer { + let (service, _socket) = LspService::new(LaravelLanguageServer::new); + let backend = service.inner().clone(); + *backend.root_path.write().await = Some(root.to_path_buf()); + *backend.auto_complete_debounce_ms.write().await = 0; + backend +} + +/// Register the project's `.env` as a real Salsa env source by driving the +/// `did_change` handler, so the completion under test has something to offer. +/// Without it every *rejection* assertion below would read "no items" and pass +/// for the wrong reason. The two positive controls are what stop that going +/// unnoticed: they require the sentinel back, so an unseeded harness fails +/// them loudly instead of quietly satisfying their negative siblings. +async fn seed_env_source(backend: &LaravelLanguageServer, root: &Path) { + let text = format!("{SENTINEL}=from_dot_env\n"); + let path = root.join(".env"); + fs::write(&path, &text).unwrap(); + let uri = Url::from_file_path(&path).unwrap(); + backend + .did_change(DidChangeTextDocumentParams { + text_document: VersionedTextDocumentIdentifier { + uri: uri.clone(), + version: 1, + }, + content_changes: vec![TextDocumentContentChangeEvent { + range: None, + range_length: None, + text: text.clone(), + }], + }) + .await; + if let Some(handle) = backend.pending_salsa_updates.write().await.remove(&uri) { + let _ = handle.await; + } +} + +/// Build `(content, cursor position)` from a template carrying a single `◊` +/// marker at the desired cursor, matching the convention in +/// `query_chain_completion_handler`. The marker is stripped from the returned +/// content; the position is its 0-based line + code-point column. +fn cursor_at(template: &str) -> (String, Position) { + const MARK: char = '◊'; + let byte_idx = template + .find(MARK) + .expect("template must contain the ◊ cursor marker"); + let before = &template[..byte_idx]; + let line = before.matches('\n').count() as u32; + let character = before.rsplit('\n').next().unwrap_or(before).chars().count() as u32; + (template.replace(MARK, ""), Position { line, character }) +} + +/// Write `rel` under `root`, open it in the server's document map, and drive +/// the real `completion` request at the template's `◊` marker. Returns the +/// offered labels — empty when the handler declines, which is what "not a +/// completion context here" looks like from the client's side. +async fn completion_labels( + backend: &LaravelLanguageServer, + root: &Path, + rel: &str, + template: &str, +) -> Vec { + let (content, position) = cursor_at(template); + let path: PathBuf = root.join(rel); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(&path, &content).unwrap(); + let uri = Url::from_file_path(&path).unwrap(); + backend + .documents + .write() + .await + .insert(uri.clone(), (content, 1)); + + let response = backend + .completion(CompletionParams { + text_document_position: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri }, + position, + }, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + context: None, + }) + .await + .expect("completion must not error"); + + match response { + None => Vec::new(), + Some(CompletionResponse::Array(items)) => items.into_iter().map(|i| i.label).collect(), + Some(CompletionResponse::List(list)) => list.items.into_iter().map(|i| i.label).collect(), + } +} + +/// A `.php` file under a `.env`-named directory, with the cursor inside a +/// `${...}` interpolation. Interpolation is env-file syntax, so the handler +/// must not offer it here — under the old `path.contains(".env")` gate this +/// file classified as an env file and the interpolation completed. +#[tokio::test] +async fn php_under_an_env_named_directory_is_not_offered_interpolation() { + let dir = TempDir::new().unwrap(); + let backend = backend_for(dir.path()).await; + seed_env_source(&backend, dir.path()).await; + + let labels = completion_labels( + &backend, + dir.path(), + ".envs/deploy.php", + &format!("` arm would be credited by +/// `.env.local` alone and leave `.env.example` and `.env.testing` unproven. +#[tokio::test] +async fn real_env_variants_are_still_offered_interpolation() { + let dir = TempDir::new().unwrap(); + let backend = backend_for(dir.path()).await; + seed_env_source(&backend, dir.path()).await; + + let mut missing = Vec::new(); + for variant in [".env", ".env.local", ".env.example", ".env.testing"] { + let labels = completion_labels( + &backend, + dir.path(), + variant, + &format!("{SENTINEL}=from_dot_env\nOTHER=${{{PREFIX}◊}}\n"), + ) + .await; + if !labels.iter().any(|l| l == SENTINEL) { + missing.push(variant); + } + } + + assert!( + missing.is_empty(), + "every Laravel env variant must still complete `${{...}}` \ + interpolation through this dispatch; these did not: {missing:?}" + ); +} diff --git a/laravel-lsp/src/tests/mod.rs b/laravel-lsp/src/tests/mod.rs index 9fd33eed..81f81b08 100644 --- a/laravel-lsp/src/tests/mod.rs +++ b/laravel-lsp/src/tests/mod.rs @@ -20,6 +20,7 @@ mod diagnostic_severity; mod directive_navigation_containment; mod directive_view_fallback; mod dynamic_where_sparseness; +mod env_completion_context_gate; mod env_completion_system_leak; mod env_source_registration_gate; mod expected_path_diagnostic_containment;