diff --git a/Cargo.lock b/Cargo.lock index 48cf438..3665fe4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -509,6 +509,7 @@ dependencies = [ "anyhow", "camino", "dashmap", + "djls-templates", "salsa", "url", ] diff --git a/crates/djls-server/src/workspace/store.rs b/crates/djls-server/src/workspace/store.rs index 1a9efbb..6ecc537 100644 --- a/crates/djls-server/src/workspace/store.rs +++ b/crates/djls-server/src/workspace/store.rs @@ -5,24 +5,28 @@ use anyhow::anyhow; use anyhow::Result; use camino::Utf8PathBuf; use djls_project::TemplateTags; -use djls_workspace::{FileId, FileKind, TextSource, Vfs}; +use djls_workspace::{FileId, FileKind, FileStore, TextSource, Vfs}; use tower_lsp_server::lsp_types::CompletionItem; use tower_lsp_server::lsp_types::CompletionItemKind; use tower_lsp_server::lsp_types::CompletionResponse; +use tower_lsp_server::lsp_types::Diagnostic; +use tower_lsp_server::lsp_types::DiagnosticSeverity; use tower_lsp_server::lsp_types::DidChangeTextDocumentParams; use tower_lsp_server::lsp_types::DidCloseTextDocumentParams; use tower_lsp_server::lsp_types::DidOpenTextDocumentParams; -use tower_lsp_server::lsp_types::TextDocumentContentChangeEvent; use tower_lsp_server::lsp_types::Documentation; use tower_lsp_server::lsp_types::InsertTextFormat; use tower_lsp_server::lsp_types::MarkupContent; use tower_lsp_server::lsp_types::MarkupKind; use tower_lsp_server::lsp_types::Position; +use tower_lsp_server::lsp_types::Range; +use tower_lsp_server::lsp_types::TextDocumentContentChangeEvent; use super::document::{ClosingBrace, LanguageId, LineIndex, TextDocument}; pub struct Store { vfs: Arc, + file_store: FileStore, file_ids: HashMap, line_indices: HashMap, versions: HashMap, @@ -33,6 +37,7 @@ impl Default for Store { fn default() -> Self { Self { vfs: Arc::new(Vfs::default()), + file_store: FileStore::new(), file_ids: HashMap::new(), line_indices: HashMap::new(), versions: HashMap::new(), @@ -64,6 +69,10 @@ impl Store { // Set overlay content in VFS self.vfs.set_overlay(file_id, Arc::from(content.as_str()))?; + // Sync VFS snapshot to FileStore for Salsa tracking + let snapshot = self.vfs.snapshot(); + self.file_store.apply_vfs_snapshot(&snapshot); + // Create TextDocument metadata let document = TextDocument::new(uri_str.clone(), version, language_id.clone(), file_id); self.documents.insert(uri_str.clone(), document); @@ -111,6 +120,10 @@ impl Store { self.vfs .set_overlay(file_id, Arc::from(new_content.as_str()))?; + // Sync VFS snapshot to FileStore for Salsa tracking + let snapshot = self.vfs.snapshot(); + self.file_store.apply_vfs_snapshot(&snapshot); + // Update cached line index and version self.line_indices .insert(file_id, LineIndex::new(&new_content)); @@ -174,9 +187,18 @@ impl Store { return None; } + // Try to get cached AST from FileStore for better context analysis + // This demonstrates using the cached AST, though we still fall back to string parsing + let file_id = document.file_id(); + if let Some(_ast) = self.file_store.get_template_ast(file_id) { + // TODO: In a future enhancement, we could use the AST to provide + // more intelligent completions based on the current node context + // For now, we continue with the existing string-based approach + } + // Get template tag context from document let vfs_snapshot = self.vfs.snapshot(); - let line_index = self.get_line_index(document.file_id())?; + let line_index = self.get_line_index(file_id)?; let context = document.get_template_tag_context(&vfs_snapshot, line_index, position)?; let mut completions: Vec = tags @@ -214,6 +236,57 @@ impl Store { Some(CompletionResponse::Array(completions)) } } + + /// Get template parsing diagnostics for a file. + /// + /// This method uses the cached template errors from Salsa to generate LSP diagnostics. + /// The errors are only re-computed when the file content changes, providing efficient + /// incremental error reporting. + pub fn get_template_diagnostics(&self, uri: &str) -> Vec { + let Some(document) = self.get_document(uri) else { + return vec![]; + }; + + // Only process template files + if document.language_id != LanguageId::HtmlDjango { + return vec![]; + } + + let file_id = document.file_id(); + let Some(_line_index) = self.get_line_index(file_id) else { + return vec![]; + }; + + // Get cached template errors from FileStore + let errors = self.file_store.get_template_errors(file_id); + + // Convert template errors to LSP diagnostics + errors + .iter() + .map(|error| { + // For now, we'll place all errors at the start of the file + // In a future enhancement, we could use error spans for precise locations + let range = Range { + start: Position { + line: 0, + character: 0, + }, + end: Position { + line: 0, + character: 0, + }, + }; + + Diagnostic { + range, + severity: Some(DiagnosticSeverity::ERROR), + source: Some("djls-templates".to_string()), + message: error.clone(), + ..Default::default() + } + }) + .collect() + } } /// Apply text changes to content, handling multiple changes correctly diff --git a/crates/djls-templates/src/ast.rs b/crates/djls-templates/src/ast.rs index f355e70..1b62d4d 100644 --- a/crates/djls-templates/src/ast.rs +++ b/crates/djls-templates/src/ast.rs @@ -5,7 +5,7 @@ use crate::tokens::Token; use crate::tokens::TokenStream; use crate::tokens::TokenType; -#[derive(Clone, Debug, Default, Serialize)] +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize)] pub struct Ast { nodelist: Vec, line_offsets: LineOffsets, @@ -36,7 +36,7 @@ impl Ast { } } -#[derive(Clone, Debug, Serialize)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] pub struct LineOffsets(pub Vec); impl LineOffsets { @@ -75,7 +75,7 @@ impl Default for LineOffsets { } } -#[derive(Clone, Debug, Serialize)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] pub enum Node { Tag { name: String, diff --git a/crates/djls-templates/src/lib.rs b/crates/djls-templates/src/lib.rs index 4835056..7eab1f6 100644 --- a/crates/djls-templates/src/lib.rs +++ b/crates/djls-templates/src/lib.rs @@ -5,7 +5,7 @@ mod parser; mod tagspecs; mod tokens; -use ast::Ast; +pub use ast::Ast; pub use error::QuickFix; pub use error::TemplateError; use lexer::Lexer; diff --git a/crates/djls-workspace/Cargo.toml b/crates/djls-workspace/Cargo.toml index 9b5ee6c..3d079cb 100644 --- a/crates/djls-workspace/Cargo.toml +++ b/crates/djls-workspace/Cargo.toml @@ -4,6 +4,8 @@ version = "0.0.0" edition = "2021" [dependencies] +djls-templates = { workspace = true } + anyhow = { workspace = true } camino = { workspace = true } dashmap = { workspace = true } diff --git a/crates/djls-workspace/src/bridge.rs b/crates/djls-workspace/src/bridge.rs index 97f894e..45f92fc 100644 --- a/crates/djls-workspace/src/bridge.rs +++ b/crates/djls-workspace/src/bridge.rs @@ -9,7 +9,7 @@ use std::{collections::HashMap, sync::Arc}; use salsa::Setter; use super::{ - db::{Database, FileKindMini, SourceFile, TemplateLoaderOrder}, + db::{parse_template, template_errors, Database, FileKindMini, SourceFile, TemplateAst, TemplateLoaderOrder}, vfs::{FileKind, VfsSnapshot}, FileId, }; @@ -99,6 +99,28 @@ impl FileStore { pub fn file_kind(&self, id: FileId) -> Option { self.files.get(&id).map(|sf| sf.kind(&self.db)) } + + /// Get the parsed template AST for a file by its [`FileId`]. + /// + /// This method leverages Salsa's incremental computation to cache parsed ASTs. + /// The AST is only re-parsed when the file's content changes in the VFS. + /// Returns `None` if the file is not tracked or is not a template file. + pub fn get_template_ast(&self, id: FileId) -> Option> { + let source_file = self.files.get(&id)?; + parse_template(&self.db, *source_file) + } + + /// Get template parsing errors for a file by its [`FileId`]. + /// + /// This method provides quick access to template errors without needing the full AST. + /// Useful for diagnostics and error reporting. Returns an empty slice for + /// non-template files or files not tracked in the store. + pub fn get_template_errors(&self, id: FileId) -> Arc<[String]> { + self.files + .get(&id) + .map(|sf| template_errors(&self.db, *sf)) + .unwrap_or_else(|| Arc::from(vec![])) + } } impl Default for FileStore { @@ -106,3 +128,108 @@ impl Default for FileStore { Self::new() } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::vfs::{TextSource, Vfs}; + use camino::Utf8PathBuf; + + #[test] + fn test_filestore_template_ast_caching() { + let mut store = FileStore::new(); + let vfs = Vfs::default(); + + // Create a template file in VFS + let url = url::Url::parse("file:///test.html").unwrap(); + let path = Utf8PathBuf::from("/test.html"); + let content: Arc = Arc::from("{% if user %}Hello {{ user.name }}{% endif %}"); + let file_id = vfs.intern_file( + url.clone(), + path.clone(), + FileKind::Template, + TextSource::Overlay(content.clone()), + ); + vfs.set_overlay(file_id, content.clone()).unwrap(); + + // Apply VFS snapshot to FileStore + let snapshot = vfs.snapshot(); + store.apply_vfs_snapshot(&snapshot); + + // Get template AST - should parse and cache + let ast1 = store.get_template_ast(file_id); + assert!(ast1.is_some()); + + // Get again - should return cached + let ast2 = store.get_template_ast(file_id); + assert!(ast2.is_some()); + assert!(Arc::ptr_eq(&ast1.unwrap(), &ast2.unwrap())); + } + + #[test] + fn test_filestore_template_errors() { + let mut store = FileStore::new(); + let vfs = Vfs::default(); + + // Create a template with an unclosed tag + let url = url::Url::parse("file:///error.html").unwrap(); + let path = Utf8PathBuf::from("/error.html"); + let content: Arc = Arc::from("{% if user %}Hello {{ user.name }"); // Missing closing + let file_id = vfs.intern_file( + url.clone(), + path.clone(), + FileKind::Template, + TextSource::Overlay(content.clone()), + ); + vfs.set_overlay(file_id, content).unwrap(); + + // Apply VFS snapshot + let snapshot = vfs.snapshot(); + store.apply_vfs_snapshot(&snapshot); + + // Get errors - should contain parsing errors + let errors = store.get_template_errors(file_id); + // The template has unclosed tags, so there should be errors + // We don't assert on specific error count as the parser may evolve + + // Verify errors are cached + let errors2 = store.get_template_errors(file_id); + assert!(Arc::ptr_eq(&errors, &errors2)); + } + + #[test] + fn test_filestore_invalidation_on_content_change() { + let mut store = FileStore::new(); + let vfs = Vfs::default(); + + // Create initial template + let url = url::Url::parse("file:///change.html").unwrap(); + let path = Utf8PathBuf::from("/change.html"); + let content1: Arc = Arc::from("{% if user %}Hello{% endif %}"); + let file_id = vfs.intern_file( + url.clone(), + path.clone(), + FileKind::Template, + TextSource::Overlay(content1.clone()), + ); + vfs.set_overlay(file_id, content1).unwrap(); + + // Apply snapshot and get AST + let snapshot1 = vfs.snapshot(); + store.apply_vfs_snapshot(&snapshot1); + let ast1 = store.get_template_ast(file_id); + + // Change content + let content2: Arc = Arc::from("{% for item in items %}{{ item }}{% endfor %}"); + vfs.set_overlay(file_id, content2).unwrap(); + + // Apply new snapshot + let snapshot2 = vfs.snapshot(); + store.apply_vfs_snapshot(&snapshot2); + + // Get AST again - should be different due to content change + let ast2 = store.get_template_ast(file_id); + assert!(ast1.is_some() && ast2.is_some()); + assert!(!Arc::ptr_eq(&ast1.unwrap(), &ast2.unwrap())); + } +} diff --git a/crates/djls-workspace/src/db.rs b/crates/djls-workspace/src/db.rs index 91aa1cc..bbdf0de 100644 --- a/crates/djls-workspace/src/db.rs +++ b/crates/djls-workspace/src/db.rs @@ -89,3 +89,145 @@ pub struct TemplateLoaderOrder { #[returns(ref)] pub roots: Arc<[String]>, } + +/// Container for a parsed Django template AST. +/// +/// [`TemplateAst`] wraps the parsed AST from djls-templates along with any parsing errors. +/// This struct is designed to be cached by Salsa and shared across multiple consumers +/// without re-parsing. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct TemplateAst { + /// The parsed AST from djls-templates + pub ast: djls_templates::Ast, + /// Any errors encountered during parsing (stored as strings for simplicity) + pub errors: Vec, +} + +/// Parse a Django template file into an AST. +/// +/// This Salsa tracked function parses template files on-demand and caches the results. +/// The parse is only re-executed when the file's text content changes, enabling +/// efficient incremental template analysis. +/// +/// Returns `None` for non-template files. +#[salsa::tracked] +pub fn parse_template(db: &dyn salsa::Database, file: SourceFile) -> Option> { + // Only parse template files + if file.kind(db) != FileKindMini::Template { + return None; + } + + let text = file.text(db); + + // Call the pure parsing function from djls-templates + match djls_templates::parse_template(&text) { + Ok((ast, errors)) => { + // Convert errors to strings + let error_strings = errors.into_iter().map(|e| e.to_string()).collect(); + Some(Arc::new(TemplateAst { ast, errors: error_strings })) + }, + Err(err) => { + // Even on fatal errors, return an empty AST with the error + Some(Arc::new(TemplateAst { + ast: djls_templates::Ast::default(), + errors: vec![err.to_string()], + })) + } + } +} + +/// Get template parsing errors for a file. +/// +/// This Salsa tracked function extracts just the errors from the parsed template, +/// useful for diagnostics without needing the full AST. +/// +/// Returns an empty vector for non-template files. +#[salsa::tracked] +pub fn template_errors(db: &dyn salsa::Database, file: SourceFile) -> Arc<[String]> { + parse_template(db, file) + .map(|ast| Arc::from(ast.errors.clone())) + .unwrap_or_else(|| Arc::from(vec![])) +} + +#[cfg(test)] +mod tests { + use super::*; + use salsa::Setter; + + #[test] + fn test_template_parsing_caches_result() { + let db = Database::default(); + + // Create a template file + let template_content: Arc = Arc::from("{% if user %}Hello {{ user.name }}{% endif %}"); + let file = SourceFile::new(&db, FileKindMini::Template, template_content.clone()); + + // First parse - should execute the parsing + let ast1 = parse_template(&db, file); + assert!(ast1.is_some()); + + // Second parse - should return cached result (same Arc) + let ast2 = parse_template(&db, file); + assert!(ast2.is_some()); + + // Verify they're the same Arc (cached) + assert!(Arc::ptr_eq(&ast1.unwrap(), &ast2.unwrap())); + } + + #[test] + fn test_template_parsing_invalidates_on_change() { + let mut db = Database::default(); + + // Create a template file + let template_content1: Arc = Arc::from("{% if user %}Hello{% endif %}"); + let file = SourceFile::new(&db, FileKindMini::Template, template_content1); + + // First parse + let ast1 = parse_template(&db, file); + assert!(ast1.is_some()); + + // Change the content + let template_content2: Arc = Arc::from("{% for item in items %}{{ item }}{% endfor %}"); + file.set_text(&mut db).to(template_content2); + + // Parse again - should re-execute due to changed content + let ast2 = parse_template(&db, file); + assert!(ast2.is_some()); + + // Verify they're different Arcs (re-parsed) + assert!(!Arc::ptr_eq(&ast1.unwrap(), &ast2.unwrap())); + } + + #[test] + fn test_non_template_files_return_none() { + let db = Database::default(); + + // Create a Python file + let python_content: Arc = Arc::from("def hello():\n print('Hello')"); + let file = SourceFile::new(&db, FileKindMini::Python, python_content); + + // Should return None for non-template files + let ast = parse_template(&db, file); + assert!(ast.is_none()); + + // Errors should be empty for non-template files + let errors = template_errors(&db, file); + assert!(errors.is_empty()); + } + + #[test] + fn test_template_errors_tracked_separately() { + let db = Database::default(); + + // Create a template with an error (unclosed tag) + let template_content: Arc = Arc::from("{% if user %}Hello {{ user.name }"); + let file = SourceFile::new(&db, FileKindMini::Template, template_content); + + // Get errors + let errors1 = template_errors(&db, file); + let errors2 = template_errors(&db, file); + + // Should be cached (same Arc) + assert!(Arc::ptr_eq(&errors1, &errors2)); + } +} diff --git a/crates/djls-workspace/src/lib.rs b/crates/djls-workspace/src/lib.rs index aea4a53..7eb862a 100644 --- a/crates/djls-workspace/src/lib.rs +++ b/crates/djls-workspace/src/lib.rs @@ -4,7 +4,7 @@ mod vfs; // Re-export public API pub use bridge::FileStore; -pub use db::{Database, FileKindMini, SourceFile, TemplateLoaderOrder}; +pub use db::{parse_template, template_errors, Database, FileKindMini, SourceFile, TemplateAst, TemplateLoaderOrder}; pub use vfs::{FileKind, FileMeta, FileRecord, Revision, TextSource, Vfs, VfsSnapshot}; /// Stable, compact identifier for files across the subsystem.