Skip to content

finish migrating to tagspec v0.6.0 #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

finish migrating to tagspec v0.6.0 #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d5cd9d9
finish migrating to tagspec v0.6.0
joshuadavidthomas Nov 5, 2025
e4ee797
finish migrating to tagspec v0.6.0
joshuadavidthomas Nov 5, 2025
e265ed1
lippy
joshuadavidthomas Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/djls-ide/src/completions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ fn generate_argument_completions(
}
}
}
TagArg::Var { name, .. } => {
TagArg::Variable { name, .. } => {
// For variables, we could offer variable completions from context
// For now, just provide a hint
if partial.is_empty() {
Expand Down
49 changes: 10 additions & 39 deletions crates/djls-ide/src/snippets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub fn generate_snippet_from_args(args: &[TagArg]) -> String {
// At this point, we know it's required (optional literals were skipped above)
lit.to_string()
}
TagArg::Var { name, .. } | TagArg::Expr { name, .. } => {
TagArg::Variable { name, .. } | TagArg::Any { name, .. } => {
// Variables and expressions become placeholders
let result = format!("${{{}:{}}}", placeholder_index, name.as_ref());
placeholder_index += 1;
Expand Down Expand Up @@ -125,22 +125,10 @@ mod tests {
#[test]
fn test_snippet_for_for_tag() {
let args = vec![
TagArg::Var {
name: "item".into(),
required: true,
},
TagArg::Literal {
lit: "in".into(),
required: true,
},
TagArg::Var {
name: "items".into(),
required: true,
},
TagArg::Literal {
lit: "reversed".into(),
required: false,
},
TagArg::var("item", true),
TagArg::syntax("in", true),
TagArg::var("items", true),
TagArg::modifier("reversed", false),
];

let snippet = generate_snippet_from_args(&args);
Expand All @@ -149,10 +137,7 @@ mod tests {

#[test]
fn test_snippet_for_if_tag() {
let args = vec![TagArg::Expr {
name: "condition".into(),
required: true,
}];
let args = vec![TagArg::expr("condition", true)];

let snippet = generate_snippet_from_args(&args);
assert_eq!(snippet, "${1:condition}");
Expand Down Expand Up @@ -198,18 +183,10 @@ mod tests {
end_tag: Some(EndTag {
name: "endblock".into(),
required: true,
args: vec![TagArg::Var {
name: "name".into(),
required: false,
}]
.into(),
args: vec![TagArg::var("name", false)].into(),
}),
intermediate_tags: Cow::Borrowed(&[]),
args: vec![TagArg::Var {
name: "name".into(),
required: true,
}]
.into(),
args: vec![TagArg::var("name", true)].into(),
};

let snippet = generate_snippet_for_tag_with_end("block", &spec);
Expand Down Expand Up @@ -254,14 +231,8 @@ mod tests {
name: "args".into(),
required: false,
},
TagArg::Literal {
lit: "as".into(),
required: false,
},
TagArg::Var {
name: "varname".into(),
required: false,
},
TagArg::syntax("as", false),
TagArg::var("varname", false),
];

let snippet = generate_snippet_from_args(&args);
Expand Down
170 changes: 73 additions & 97 deletions crates/djls-semantic/src/arguments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ fn validate_argument_order(
}

match arg {
TagArg::Literal { lit, required } => {
TagArg::Literal { lit, required, .. } => {
// kind field is ignored for validation - it's only for semantic hints
let matches_literal = bits[bit_index] == lit.as_ref();
if *required {
if matches_literal {
Expand Down Expand Up @@ -188,63 +189,74 @@ fn validate_argument_order(
// Optional choice didn't match - don't consume, continue
}

TagArg::Var { .. } | TagArg::String { .. } => {
// Simple arguments consume exactly one token
TagArg::String { .. } => {
// String arguments consume exactly one token
bit_index += 1;
}

TagArg::Expr { .. } => {
// Expression arguments consume tokens greedily until:
// - We hit the next literal keyword (if any)
// - We run out of tokens
// Must consume at least one token

let start_index = bit_index;
let next_literal = args[arg_index + 1..].find_next_literal();
TagArg::Variable { count, .. } | TagArg::Any { count, .. } => {
match count {
crate::templatetags::TokenCount::Exact(n) => {
// Consume exactly N tokens
bit_index += n;
}
crate::templatetags::TokenCount::Greedy => {
// Greedy: consume tokens until next literal or end
let start_index = bit_index;
let next_literal = args[arg_index + 1..].find_next_literal();

while bit_index < bits.len() {
if let Some(ref lit) = next_literal {
if bits[bit_index] == *lit {
break; // Stop before the literal
}
}
bit_index += 1;
}

// Consume tokens greedily until we hit a known literal
while bit_index < bits.len() {
if let Some(ref lit) = next_literal {
if bits[bit_index] == *lit {
break; // Stop before the literal
// Ensure we consumed at least one token
if bit_index == start_index {
bit_index += 1;
}
}
bit_index += 1;
}

// Ensure we consumed at least one token for the expression
if bit_index == start_index {
bit_index += 1;
}
}
Comment on lines +197 to 223
Copy link

@coderabbitai coderabbitai bot Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Prevent silently accepting too few tokens for Exact arguments.

TokenCount::Exact(n) blindly bumps bit_index even when there are < n tokens left. In that case we step past bits.len() and the later “missing argument” scan never fires, so a required argument that only received a partial token sequence is treated as valid (e.g. {% with value as %} slips through). Please gate consumption on the available token count and surface a MissingArgument for required args; optional args should simply leave the remaining tokens untouched.

-            TagArg::Variable { count, .. } | TagArg::Any { count, .. } => {
-                match count {
-                    crate::templatetags::TokenCount::Exact(n) => {
-                        // Consume exactly N tokens
-                        bit_index += n;
-                    }
+            TagArg::Variable {
+                name,
+                required,
+                count,
+            }
+            | TagArg::Any {
+                name,
+                required,
+                count,
+            } => {
+                match count {
+                    crate::templatetags::TokenCount::Exact(n) => {
+                        let available = bits.len().saturating_sub(bit_index);
+                        if available < n {
+                            if *required {
+                                ValidationErrorAccumulator(ValidationError::MissingArgument {
+                                    tag: tag_name.to_string(),
+                                    argument: name.to_string(),
+                                    span,
+                                })
+                                .accumulate(db);
+                                return;
+                            }
+                            continue;
+                        }
+
+                        // Consume exactly N tokens
+                        bit_index += n;
+                    }
🤖 Prompt for AI Agents 
In crates/djls-semantic/src/arguments.rs around lines 197–223, the
TokenCount::Exact(n) branch unconditionally advances bit_index by n and can run
past bits.len(), letting required args silently accept too few tokens; change
this to first compute available = bits.len() - bit_index, and if available < n
then: for required TagArg variants return/surface a MissingArgument error (do
not advance bit_index), and for optional variants leave bit_index unchanged (do
not consume any tokens). Otherwise (available >= n) advance bit_index by n but
never beyond bits.len(). Ensure no panics from out-of-bounds indexing and that
the greedy branch behavior is unchanged.


TagArg::Assignment { .. } => {
// Assignment arguments can appear as:
// 1. Single token: var=value
// 2. Multi-token: expr as varname
// Consume until we find = or "as", or hit next literal

let next_literal = args[arg_index + 1..].find_next_literal();

while bit_index < bits.len() {
let token = &bits[bit_index];
bit_index += 1;

// If token contains =, we've found the assignment
if token.contains('=') {
break;
}

// If we hit "as", consume the variable name after it
if token == "as" && bit_index < bits.len() {
bit_index += 1; // Consume the variable name
break;
TagArg::Assignment { count, .. } => {
match count {
crate::templatetags::TokenCount::Exact(n) => {
// Consume exactly N tokens
bit_index += n;
}

// Stop if we hit the next literal argument
if let Some(ref lit) = next_literal {
if token == lit {
break;
crate::templatetags::TokenCount::Greedy => {
// Assignment arguments can appear as:
// 1. Single token: var=value
// 2. Multi-token: expr as varname
// Consume until we find = or "as", or hit next literal

let next_literal = args[arg_index + 1..].find_next_literal();

while bit_index < bits.len() {
let token = &bits[bit_index];
bit_index += 1;

// If token contains =, we've found the assignment
if token.contains('=') {
break;
}

// If we hit "as", consume the variable name after it
if token == "as" && bit_index < bits.len() {
bit_index += 1; // Consume the variable name
break;
}

// Stop if we hit the next literal argument
if let Some(ref lit) = next_literal {
if token == lit {
break;
}
}
}
}
}
Expand Down Expand Up @@ -403,16 +415,13 @@ mod tests {
fn test_if_tag_with_comparison_operator() {
// Issue #1: {% if message.input_tokens > 0 %}
// Parser tokenizes as: ["message.input_tokens", ">", "0"]
// Spec expects: [Expr{name="condition"}]
// Spec expects: [Any{name="condition", count=Greedy}]
let bits = vec![
"message.input_tokens".to_string(),
">".to_string(),
"0".to_string(),
];
let args = vec![TagArg::Expr {
name: "condition".into(),
required: true,
}];
let args = vec![TagArg::expr("condition", true)];

let errors = check_validation_errors("if", &bits, &args);
assert!(
Expand Down Expand Up @@ -448,22 +457,10 @@ mod tests {
"reversed".to_string(),
];
let args = vec![
TagArg::Var {
name: "item".into(),
required: true,
},
TagArg::Literal {
lit: "in".into(),
required: true,
},
TagArg::Var {
name: "items".into(),
required: true,
},
TagArg::Literal {
lit: "reversed".into(),
required: false,
},
TagArg::var("item", true),
TagArg::syntax("in", true),
TagArg::var("items", true),
TagArg::modifier("reversed", false),
];

let errors = check_validation_errors("for", &bits, &args);
Expand All @@ -481,10 +478,7 @@ mod tests {
"and".to_string(),
"user.is_staff".to_string(),
];
let args = vec![TagArg::Expr {
name: "condition".into(),
required: true,
}];
let args = vec![TagArg::expr("condition", true)];

let errors = check_validation_errors("if", &bits, &args);
assert!(
Expand Down Expand Up @@ -521,10 +515,7 @@ mod tests {
fn test_with_assignment() {
// {% with total=items|length %}
let bits = vec!["total=items|length".to_string()];
let args = vec![TagArg::Assignment {
name: "bindings".into(),
required: true,
}];
let args = vec![TagArg::assignment("bindings", true)];

let errors = check_validation_errors("with", &bits, &args);
assert!(
Expand Down Expand Up @@ -556,14 +547,8 @@ mod tests {
"reversed".to_string(),
];
let args = vec![
TagArg::Expr {
name: "condition".into(),
required: true,
},
TagArg::Literal {
lit: "reversed".into(),
required: false,
},
TagArg::expr("condition", true),
TagArg::modifier("reversed", false),
];

let errors = check_validation_errors("if", &bits, &args);
Expand Down Expand Up @@ -675,18 +660,9 @@ mod tests {
"library".to_string(),
];
let args = vec![
TagArg::VarArgs {
name: "tags".into(),
required: false,
},
TagArg::Literal {
lit: "from".into(),
required: false,
},
TagArg::Var {
name: "library".into(),
required: false,
},
TagArg::varargs("tags", false),
TagArg::syntax("from", false),
TagArg::var("library", false),
];

let errors = check_validation_errors("load", &bits, &args);
Expand Down
2 changes: 2 additions & 0 deletions crates/djls-semantic/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ pub use resolution::TemplateReference;
pub use semantic::build_semantic_forest;
pub use templatetags::django_builtin_specs;
pub use templatetags::EndTag;
pub use templatetags::LiteralKind;
pub use templatetags::TagArg;
pub use templatetags::TagSpec;
pub use templatetags::TagSpecs;
pub use templatetags::TokenCount;

/// Validate a Django template node list and return validation errors.
///
Expand Down
2 changes: 2 additions & 0 deletions crates/djls-semantic/src/templatetags.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ mod specs;

pub use builtins::django_builtin_specs;
pub use specs::EndTag;
pub use specs::LiteralKind;
pub use specs::TagArg;
pub(crate) use specs::TagArgSliceExt;
pub use specs::TagSpec;
pub use specs::TagSpecs;
pub use specs::TokenCount;
Loading