From 03f18d22117a2b658b3faf816902add5023e54e2 Mon Sep 17 00:00:00 2001 From: Josh Thomas Date: Mon, 8 Sep 2025 21:56:40 -0500 Subject: [PATCH] simplify Span struct and remove salsa tracking (#205) --- crates/djls-templates/src/ast.rs | 137 ++++++++---------------- crates/djls-templates/src/lib.rs | 4 +- crates/djls-templates/src/parser.rs | 16 +-- crates/djls-templates/src/validation.rs | 45 +++----- 4 files changed, 68 insertions(+), 134 deletions(-) diff --git a/crates/djls-templates/src/ast.rs b/crates/djls-templates/src/ast.rs index 4435858..dce0592 100644 --- a/crates/djls-templates/src/ast.rs +++ b/crates/djls-templates/src/ast.rs @@ -57,8 +57,8 @@ impl Default for LineOffsets { #[derive(Clone, Debug, PartialEq, Eq, salsa::Update)] pub enum Node<'db> { Tag(TagNode<'db>), - Comment(CommentNode<'db>), - Text(TextNode<'db>), + Comment(CommentNode), + Text(TextNode), Variable(VariableNode<'db>), } @@ -66,7 +66,7 @@ pub enum Node<'db> { pub struct TagNode<'db> { pub name: TagName<'db>, pub bits: Vec, - pub span: Span<'db>, + pub span: Span, } #[salsa::interned(debug)] @@ -75,22 +75,22 @@ pub struct TagName<'db> { } #[derive(Debug, Clone, PartialEq, Eq, salsa::Update)] -pub struct CommentNode<'db> { +pub struct CommentNode { pub content: String, - pub span: Span<'db>, + pub span: Span, } #[derive(Debug, Clone, PartialEq, Eq, salsa::Update)] -pub struct TextNode<'db> { +pub struct TextNode { pub content: String, - pub span: Span<'db>, + pub span: Span, } #[derive(Debug, Clone, PartialEq, Eq, salsa::Update)] pub struct VariableNode<'db> { pub var: VariableName<'db>, pub filters: Vec>, - pub span: Span<'db>, + pub span: Span, } #[salsa::interned(debug)] @@ -103,29 +103,29 @@ pub struct FilterName<'db> { pub text: String, } -#[salsa::tracked(debug)] -pub struct Span<'db> { - #[tracked] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize)] +pub struct Span { pub start: u32, - #[tracked] pub length: u32, } -impl<'db> Span<'db> { - pub fn from_token(db: &'db dyn crate::db::Db, token: &Token) -> Self { - let start = token.start().unwrap_or(0); - let length = u32::try_from(token.lexeme().len()).unwrap_or(0); - Span::new(db, start, length) +impl Span { + #[must_use] + pub fn new(start: u32, length: u32) -> Self { + Self { start, length } } #[must_use] - pub fn to_lsp_range( - &self, - db: &'db dyn crate::db::Db, - line_offsets: &LineOffsets, - ) -> tower_lsp_server::lsp_types::Range { - let start_pos = self.start(db) as usize; - let end_pos = (self.start(db) + self.length(db)) as usize; + pub fn from_token(token: &Token) -> Self { + let start = token.start().unwrap_or(0); + let length = u32::try_from(token.lexeme().len()).unwrap_or(0); + Self { start, length } + } + + #[must_use] + pub fn to_lsp_range(&self, line_offsets: &LineOffsets) -> tower_lsp_server::lsp_types::Range { + let start_pos = self.start as usize; + let end_pos = (self.start + self.length) as usize; let (start_line, start_char) = line_offsets.position_to_line_col(start_pos); let (end_line, end_char) = line_offsets.position_to_line_col(end_pos); @@ -151,58 +151,35 @@ pub enum AstError { InvalidTagStructure { tag: String, reason: String, - span_start: u32, - span_length: u32, + span: Span, }, #[error("Unbalanced structure: '{opening_tag}' missing closing '{expected_closing}'")] UnbalancedStructure { opening_tag: String, expected_closing: String, - opening_span_start: u32, - opening_span_length: u32, - closing_span_start: Option, - closing_span_length: Option, + opening_span: Span, + closing_span: Option, }, #[error("Invalid {node_type} node: {reason}")] InvalidNode { node_type: String, reason: String, - span_start: u32, - span_length: u32, + span: Span, }, #[error("Unclosed tag: {tag}")] - UnclosedTag { - tag: String, - span_start: u32, - span_length: u32, - }, + UnclosedTag { tag: String, span: Span }, #[error("Orphaned tag '{tag}' - {context}")] OrphanedTag { tag: String, context: String, - span_start: u32, - span_length: u32, + span: Span, }, #[error("endblock '{name}' does not match any open block")] - UnmatchedBlockName { - name: String, - span_start: u32, - span_length: u32, - }, + UnmatchedBlockName { name: String, span: Span }, #[error("Tag '{tag}' requires at least {min} argument{}", if *.min == 1 { "" } else { "s" })] - MissingRequiredArguments { - tag: String, - min: usize, - span_start: u32, - span_length: u32, - }, + MissingRequiredArguments { tag: String, min: usize, span: Span }, #[error("Tag '{tag}' accepts at most {max} argument{}", if *.max == 1 { "" } else { "s" })] - TooManyArguments { - tag: String, - max: usize, - span_start: u32, - span_length: u32, - }, + TooManyArguments { tag: String, max: usize, span: Span }, } impl AstError { @@ -210,46 +187,16 @@ impl AstError { #[must_use] pub fn span(&self) -> Option<(u32, u32)> { match self { - AstError::UnbalancedStructure { - opening_span_start, - opening_span_length, - .. - } => Some((*opening_span_start, *opening_span_length)), - AstError::InvalidTagStructure { - span_start, - span_length, - .. + AstError::UnbalancedStructure { opening_span, .. } => { + Some((opening_span.start, opening_span.length)) } - | AstError::InvalidNode { - span_start, - span_length, - .. - } - | AstError::UnclosedTag { - span_start, - span_length, - .. - } - | AstError::OrphanedTag { - span_start, - span_length, - .. - } - | AstError::UnmatchedBlockName { - span_start, - span_length, - .. - } - | AstError::MissingRequiredArguments { - span_start, - span_length, - .. - } - | AstError::TooManyArguments { - span_start, - span_length, - .. - } => Some((*span_start, *span_length)), + AstError::InvalidTagStructure { span, .. } + | AstError::InvalidNode { span, .. } + | AstError::UnclosedTag { span, .. } + | AstError::OrphanedTag { span, .. } + | AstError::UnmatchedBlockName { span, .. } + | AstError::MissingRequiredArguments { span, .. } + | AstError::TooManyArguments { span, .. } => Some((span.start, span.length)), AstError::EmptyAst => None, } } diff --git a/crates/djls-templates/src/lib.rs b/crates/djls-templates/src/lib.rs index ab4d055..c43ab6b 100644 --- a/crates/djls-templates/src/lib.rs +++ b/crates/djls-templates/src/lib.rs @@ -163,8 +163,8 @@ fn accumulate_error(db: &dyn Db, error: &TemplateError, line_offsets: &LineOffse let range = error .span() .map(|(start, length)| { - let span = crate::ast::Span::new(db, start, length); - span.to_lsp_range(db, line_offsets) + let span = crate::ast::Span::new(start, length); + span.to_lsp_range(line_offsets) }) .unwrap_or_default(); diff --git a/crates/djls-templates/src/parser.rs b/crates/djls-templates/src/parser.rs index 792e92f..336e1f2 100644 --- a/crates/djls-templates/src/parser.rs +++ b/crates/djls-templates/src/parser.rs @@ -101,7 +101,7 @@ impl<'db> Parser<'db> { Ok(Node::Comment(CommentNode { content: token.content(), - span: Span::from_token(self.db, &token), + span: Span::from_token(&token), })) } @@ -116,7 +116,7 @@ impl<'db> Parser<'db> { let name_str = args.first().ok_or(ParserError::EmptyTag)?.clone(); let name = TagName::new(self.db, name_str); // Intern the tag name let bits = args.into_iter().skip(1).collect(); - let span = Span::from_token(self.db, &token); + let span = Span::from_token(&token); Ok(Node::Tag(TagNode { name, bits, span })) } @@ -137,7 +137,7 @@ impl<'db> Parser<'db> { .skip(1) .map(|s| FilterName::new(self.db, s.trim().to_string())) // Intern filter names .collect(); - let span = Span::from_token(self.db, &token); + let span = Span::from_token(&token); Ok(Node::Variable(VariableNode { var, filters, span })) } @@ -175,7 +175,7 @@ impl<'db> Parser<'db> { let offset = u32::try_from(text.find(content.as_str()).unwrap_or(0)) .expect("Offset should fit in u32"); let length = u32::try_from(content.len()).expect("Content length should fit in u32"); - let span = Span::new(self.db, start + offset, length); + let span = Span::new(start + offset, length); Ok(Node::Text(TextNode { content, span })) } @@ -404,20 +404,20 @@ mod tests { Node::Tag(TagNode { name, bits, span }) => TestNode::Tag { name: name.text(db).to_string(), bits: bits.clone(), - span: (span.start(db), span.length(db)), + span: (span.start, span.length), }, Node::Comment(CommentNode { content, span }) => TestNode::Comment { content: content.clone(), - span: (span.start(db), span.length(db)), + span: (span.start, span.length), }, Node::Text(TextNode { content, span }) => TestNode::Text { content: content.clone(), - span: (span.start(db), span.length(db)), + span: (span.start, span.length), }, Node::Variable(VariableNode { var, filters, span }) => TestNode::Variable { var: var.text(db).to_string(), filters: filters.iter().map(|f| f.text(db).to_string()).collect(), - span: (span.start(db), span.length(db)), + span: (span.start, span.length), }, } } diff --git a/crates/djls-templates/src/validation.rs b/crates/djls-templates/src/validation.rs index d5c3ee8..1b3bdf3 100644 --- a/crates/djls-templates/src/validation.rs +++ b/crates/djls-templates/src/validation.rs @@ -92,8 +92,7 @@ impl<'db> TagValidator<'db> { while let Some(tag) = self.stack.pop() { self.errors.push(AstError::UnclosedTag { tag: tag.name.text(self.db), - span_start: tag.span.start(self.db), - span_length: tag.span.length(self.db), + span: tag.span, }); } @@ -104,7 +103,7 @@ impl<'db> TagValidator<'db> { &mut self, name: &str, bits: &[String], - span: Span<'db>, + span: Span, arg_spec: Option<&ArgSpec>, ) { let Some(arg_spec) = arg_spec else { @@ -116,8 +115,7 @@ impl<'db> TagValidator<'db> { self.errors.push(AstError::MissingRequiredArguments { tag: name.to_string(), min, - span_start: span.start(self.db), - span_length: span.length(self.db), + span, }); } } @@ -127,14 +125,13 @@ impl<'db> TagValidator<'db> { self.errors.push(AstError::TooManyArguments { tag: name.to_string(), max, - span_start: span.start(self.db), - span_length: span.length(self.db), + span, }); } } } - fn handle_intermediate(&mut self, name: &str, span: Span<'db>) { + fn handle_intermediate(&mut self, name: &str, span: Span) { // Check if this intermediate tag has the required parent let parent_tags = self.db.tag_specs().get_parent_tags_for_intermediate(name); if parent_tags.is_empty() { @@ -159,13 +156,12 @@ impl<'db> TagValidator<'db> { self.errors.push(AstError::OrphanedTag { tag: name.to_string(), context, - span_start: span.start(self.db), - span_length: span.length(self.db), + span, }); } } - fn handle_closer(&mut self, name: TagName<'db>, bits: &[String], span: Span<'db>) { + fn handle_closer(&mut self, name: TagName<'db>, bits: &[String], span: Span) { let name_str = name.text(self.db); if self.stack.is_empty() { @@ -173,10 +169,8 @@ impl<'db> TagValidator<'db> { self.errors.push(AstError::UnbalancedStructure { opening_tag: name_str.to_string(), expected_closing: String::new(), - opening_span_start: span.start(self.db), - opening_span_length: span.length(self.db), - closing_span_start: None, - closing_span_length: None, + opening_span: span, + closing_span: None, }); return; } @@ -188,10 +182,8 @@ impl<'db> TagValidator<'db> { self.errors.push(AstError::UnbalancedStructure { opening_tag: name_str.to_string(), expected_closing: String::new(), - opening_span_start: span.start(self.db), - opening_span_length: span.length(self.db), - closing_span_start: None, - closing_span_length: None, + opening_span: span, + closing_span: None, }); return; }; @@ -234,8 +226,7 @@ impl<'db> TagValidator<'db> { // Report the mismatch self.errors.push(AstError::UnmatchedBlockName { name: bits[0].clone(), - span_start: span.start(self.db), - span_length: span.length(self.db), + span, }); // Find the nearest block to close (and report it as unclosed) @@ -249,8 +240,7 @@ impl<'db> TagValidator<'db> { // Report that we're closing the wrong block self.errors.push(AstError::UnclosedTag { tag: nearest_block.name.text(self.db), - span_start: nearest_block.span.start(self.db), - span_length: nearest_block.span.length(self.db), + span: nearest_block.span, }); // Pop everything after as unclosed @@ -264,10 +254,8 @@ impl<'db> TagValidator<'db> { self.errors.push(AstError::UnbalancedStructure { opening_tag: opener_name, expected_closing: name_str.to_string(), - opening_span_start: span.start(self.db), - opening_span_length: span.length(self.db), - closing_span_start: None, - closing_span_length: None, + opening_span: span, + closing_span: None, }); } } @@ -277,8 +265,7 @@ impl<'db> TagValidator<'db> { if let Some(unclosed) = self.stack.pop() { self.errors.push(AstError::UnclosedTag { tag: unclosed.name.text(self.db), - span_start: unclosed.span.start(self.db), - span_length: unclosed.span.length(self.db), + span: unclosed.span, }); } }