enforce key uniqueness on with expression, fix scanner.c

CLAUDE.md

@@ -57,8 +57,9 @@ that place it:
   positional filter expressions, then `name=value` keywords, then `as name`. `args`: `true` (`*args`), a
   count (at most N), or `false`. `kwargs`: `true` (`**kwargs`), `false`, or `{name: rule}` for a known
   signature. Put the Python signature in a comment above the call site.
-- `arrangements(items)` — every ordered subset of `items`, used by `simpleTag` so that known keyword
-  arguments may appear in any order but never twice. Grows as `Σ C(n,k)·k!` (65 for four names), which is
+- `arrangements(items)` — every ordered subset of `items`, so that a tag's options may be written in any
+  order but never twice. Used by `translate` and `include`; keyword arguments are guarded by the scanner
+  instead. Grows as `Σ C(n,k)·k!` (65 for four names), which is
   why it is only used for signatures that name their arguments.
 
 Consequences worth knowing before editing a rule:
@@ -138,9 +139,10 @@ Twelve external tokens, for the constraints a context-free grammar cannot expres
   never a second candidate name to try. Serialized as `<kind><name> ` per entry (`tag_kind_chars`).
 - `verbatim_content`, `comment_content` — raw text up to the matching close tag.
 - `_tag_open`, `_bt_option`, `_kwarg_name` — what a tag has already been given, so that a repeated
-  argument is refused: `_bt_option` for `{% blocktranslate %}`'s five options, `_kwarg_name` for the
-  `name=value` arguments of a tag registered with `simple_tag`, whose names are open-ended and so are kept
-  as names rather than as a mask. `_kwarg_name` returns nothing unless the name is followed by `=`, which
+  argument is refused: `_bt_option` for `{% blocktranslate %}`'s five options, `_kwarg_name` for `name=value`
+  arguments — those of a tag registered with `simple_tag`, and the `with` bindings of `{% with %}`,
+  `{% include %}` and `{% blocktranslate %}` — whose names are open-ended and so are kept as names rather
+  than as a mask. `_kwarg_name` returns nothing unless the name is followed by `=`, which
   is also what tells an argument apart from the `as name` clause or the end of the tag. `_tag_open` clears
   both, and every rule that uses either has one. Both are zero width and hidden, so no tree changes. The state is one byte of
   flags, not a stack, because the tag cannot nest: its body admits no tags. It lives outside the
@@ -159,10 +161,12 @@ and the reason is worth keeping straight: its five options are a fixed set, so u
 `arrangements` expresses exactly that — and the scanner is standing in for a grammar that would be correct
 but takes minutes to build. Where the set is open, as with `{% querystring %}`'s `**kwargs`, no grammar
 can express it at all, and the scanner remembers the names the tag has been given instead of a mask of
-known ones. Which tags get that guard is itself a differential question: only `parse_bits` refuses a
-repeat, so `{% with a=1 a=2 %}`, `{% include "t" with a=1 a=2 %}` and `{% blocktranslate with a=1 a=2 %}`
-are all accepted by Django — `token_kwargs` builds a dict and the later value simply wins — and must not
-be guarded.
+known ones. Only `parse_bits` actually refuses a repeat: the `with` bindings of
+`{% with %}`, `{% include %}` and `{% blocktranslate %}` all go through `token_kwargs`, which builds a dict
+and lets the later value win. They are guarded here anyway, as a deliberate divergence — writing a name
+twice has no use and is a mistake worth reporting. `{% blocktranslate %}`'s `count` is not part of that,
+because Django keeps it in a dict of its own, so `{% blocktranslate with a=1 count a=2 %}` is accepted here
+as it is there.
 
 Zero-width guards carry one hazard: `recover_with_missing` can supply one without the scanner running. The
 `if (valid_symbols[MatcherError]) return false;` near the top of `scan` is what stops a guard being scanned
@@ -216,6 +220,9 @@ documentation or memory. Both checks are worth repeating whenever tags, filters
 
 - Django has no number token: `[\w.]+` is one lexeme and `int()`/`float()` decides whether it is a literal,
   so `1a`, `1e`, `0x1f`, `1.2.3`, `1__0` and `1.` are variable _lookups_ there and errors here.
+- A `with` binding written twice (`{% with a=1 a=2 %}`, and the same in `{% include %}` and
+  `{% blocktranslate %}`) is an error here and not in Django, where `token_kwargs` builds a dict and the
+  later value wins. It has no use, so it is treated as the mistake it almost certainly is.
 - `{% csp_nonce_attr "a" media="b" %}` is accepted, matching Django, which only rejects it at render time.
 - A misspelled tag or filter cannot be caught: `{{ x|lenght }}` is indistinguishable from a filter some
   library registered. Cross-referencing names against `{% load %}` needs the project's Python, so it

grammar.js

@@ -545,13 +545,20 @@ const django = grammar({
     _translate_body: ($) => repeat1(choice($.content, $.template_variable)),
     /**
      * The "with" option of blocktranslate and of include takes at least one of
-     * these. Django lets a name repeat here — token_kwargs builds a dict and
-     * the later value wins — so unlike a simple_tag's arguments they are not
-     * guarded.
+     * these. Django lets a name repeat — token_kwargs builds a dict and the
+     * later value simply wins — but writing one twice has no use and is taken
+     * here to be a mistake, so the scanner guards these too.
      */
     _tag_kwargs: ($) =>
       repeat1(
-        part(seq(field("variable", $.identifier), "=", $.filtered_value)),
+        part(
+          seq(
+            $._kwarg_name,
+            field("variable", $.identifier),
+            "=",
+            $.filtered_value,
+          ),
+        ),
       ),
     cache_group: ($) =>
       seq(
@@ -689,6 +696,7 @@ const django = grammar({
     include: ($) =>
       block(
         "include",
+        $._tag_open,
         part($.filtered_value),
         optional(
           arrangements([seq(part("with"), $._tag_kwargs), part("only")]),
@@ -825,12 +833,7 @@ const django = grammar({
       ),
     with_group: ($) =>
       seq(
-        block(
-          "with",
-          repeat1(
-            part(seq(field("variable", $.identifier), "=", $.filtered_value)),
-          ),
-        ),
+        block("with", $._tag_open, $._tag_kwargs),
         optional($.template),
         block("endwith"),
       ),

src/grammar.json

@@ -2137,6 +2137,10 @@
           {
             "type": "SEQ",
             "members": [
+              {
+                "type": "SYMBOL",
+                "name": "_kwarg_name"
+              },
               {
                 "type": "FIELD",
                 "name": "variable",
@@ -4690,6 +4694,10 @@
             "value": "include"
           }
         },
+        {
+          "type": "SYMBOL",
+          "name": "_tag_open"
+        },
         {
           "type": "SEQ",
           "members": [
@@ -7849,37 +7857,12 @@
               }
             },
             {
-              "type": "REPEAT1",
-              "content": {
-                "type": "SEQ",
-                "members": [
-                  {
-                    "type": "PATTERN",
-                    "value": "[ \\t\\r\\n]+"
-                  },
-                  {
-                    "type": "SEQ",
-                    "members": [
-                      {
-                        "type": "FIELD",
-                        "name": "variable",
-                        "content": {
               "type": "SYMBOL",
-                          "name": "identifier"
-                        }
-                      },
-                      {
-                        "type": "STRING",
-                        "value": "="
+              "name": "_tag_open"
             },
             {
               "type": "SYMBOL",
-                        "name": "filtered_value"
-                      }
-                    ]
-                  }
-                ]
-              }
+              "name": "_tag_kwargs"
             },
             {
               "type": "CHOICE",
@@ -7961,12 +7944,25 @@
   },
   "extras": [],
   "conflicts": [
-    ["template"],
-    ["predicate"],
-    ["library", "load"],
-    ["_translate_option"],
-    ["_filtered_value_spaced"],
-    ["_filter_expression_spaced"]
+    [
+      "template"
+    ],
+    [
+      "predicate"
+    ],
+    [
+      "library",
+      "load"
+    ],
+    [
+      "_translate_option"
+    ],
+    [
+      "_filtered_value_spaced"
+    ],
+    [
+      "_filter_expression_spaced"
+    ]
   ],
   "precedences": [],
   "externals": [
@@ -8020,7 +8016,10 @@
     }
   ],
   "inline": [],
-  "supertypes": ["template_tag", "template_block_groups"],
+  "supertypes": [
+    "template_tag",
+    "template_block_groups"
+  ],
   "reserved": {
     "global": [
       {

src/scanner.c

@@ -65,13 +65,14 @@ typedef Array(OpenTag) Stack;
 
 struct Scanner {
   bool has_error;
-  /* one bit per option of the blocktranslate tag being read, by the order of
-   * translate_options. Kept out of the union below, whose members share their
-   * storage with each other. */
-  /* the "name=value" argument names the tag being read has been given, each
-   * followed by NAME_SEP. parse_bits refuses a repeated keyword argument, and
-   * the set of names is open, so this remembers the names rather than a mask
-   * of known ones. Kept out of the union, as translate_seen is. */
+  /* Everything below is readable only while has_error is false: the error
+   * string shares its storage with all of it.
+   *
+   * translate_seen holds one bit per option of the blocktranslate tag being
+   * read, by the order of translate_options. seen_kwargs holds the
+   * "name=value" argument names the tag has been given, each followed by
+   * NAME_SEP: parse_bits refuses a repeated keyword argument and the set of
+   * names is open, so the names are kept rather than a mask of known ones. */
   union {
     struct {
       uint8_t translate_seen;
@@ -92,8 +93,11 @@ static enum TokenType pop_token_for_kind(TagKind kind) {
 
 static void reset_scanner(struct Scanner *const scanner) {
   if (scanner->has_error) {
-    // sets everything to 0 values: 0, '\0', false, null
-    memset(&scanner, 0, sizeof(scanner));
+    /* The error shares its storage with the state below, so none of that is
+     * readable here and none of it holds a pointer to free: scanner_error
+     * resets before it sets has_error, which is what frees them. Zeroing the
+     * whole struct clears the error and the state it overlays at once. */
+    memset(scanner, 0, sizeof(*scanner));
   } else {
     scanner->translate_seen = 0;
     array_delete(&scanner->seen_kwargs);
@@ -167,9 +171,11 @@ write_serialization_error:;
   char *iter = buffer;
 
   iter += write_code(scanner->has_error + '0', iter);
+  /* Both of these overlay the error string, so they are only read when there is
+   * none; an error state serializes as the empty option mask and no names. */
   // five options fit in five bits, which stays inside printable ASCII
-  iter += write_code(scanner->translate_seen + '0', iter);
-  for (unsigned i = 0; i < scanner->seen_kwargs.size; ++i) {
+  iter += write_code((scanner->has_error ? 0 : scanner->translate_seen) + '0', iter);
+  for (unsigned i = 0; !scanner->has_error && i < scanner->seen_kwargs.size; ++i) {
     int32_t code = *array_get(&scanner->seen_kwargs, i);
     unsigned write_amt = write_code(code, iter);
     if (write_amt == 0) {

test/corpus/blocktranslate.txt

@@ -438,3 +438,12 @@ Blocktrans Repeating An Option Of The Tag Before It
   (blocktranslate_group
     (content))
   (content))
+
+==================
+Blocktrans With A Binding Twice
+:error
+==================
+
+{% blocktrans with a=1 a=2 %}x{% endblocktrans %}
+
+---

test/corpus/csp_nonce_attr.txt

@@ -79,3 +79,12 @@ Csp Nonce Attr Rejects Repeated Keyword
 
 (ERROR)
 
+
+==================
+Csp Nonce Attr With Media Twice
+:error
+==================
+
+{% csp_nonce_attr media="a" media="b" %}
+
+---

test/corpus/include.txt

@@ -162,31 +162,14 @@ Include With Only Before With
   (content))
 
 ==================
-Include With A Repeated Binding
+Include With A Binding Twice
+:error
 ==================
 
 {% include "t.html" with a=1 a=2 %}
 
 ---
 
-(template
-  (content)
-  (include
-    (filtered_value
-      (value
-        (literal
-          (string))))
-    variable: (identifier)
-    (filtered_value
-      (value
-        (literal
-          (number))))
-    variable: (identifier)
-    (filtered_value
-      (value
-        (literal
-          (number)))))
-  (content))
 
 ==================
 Include Without A Name

test/corpus/with.txt

@@ -59,3 +59,51 @@ With Multiple Variables
               (identifier)))))
       (content)))
   (content))
+
+==================
+With A Binding Twice
+:error
+==================
+
+{% with a=1 a=2 %}x{% endwith %}
+
+---
+
+
+==================
+With A Binding Repeated Later
+:error
+==================
+
+{% with a=1 b=2 a=3 %}x{% endwith %}
+
+---
+
+
+==================
+With Repeating A Binding Of The Tag Before It
+==================
+
+{% with a=1 %}x{% endwith %}{% with a=2 %}y{% endwith %}
+
+---
+
+(template
+  (content)
+  (with_group
+    variable: (identifier)
+    (filtered_value
+      (value
+        (literal
+          (number))))
+    (template
+      (content)))
+  (with_group
+    variable: (identifier)
+    (filtered_value
+      (value
+        (literal
+          (number))))
+    (template
+      (content)))
+  (content))