From 7196a47eb85f38420cd1dc41e1c78d7b7ed9cdf4 Mon Sep 17 00:00:00 2001
From: Esko Luontola <esko@luontola.fi>
Date: Thu, 14 Dec 2023 16:11:00 +0200
Subject: [PATCH] Fix formatting of nil attributes at runtime

Fix forms like (html [:br (identity nil)]) to produce "<br />" or "<br>"
rather than "<br></br>".

The use case where this issue can happen is if the user has a function
which returns either a map of attributes or nil.
---
 src/hiccup/compiler.clj             | 38 +++++++++++++++++------------
 test/hiccup/compiler_test.clj       | 15 ++++++------
 test/hiccup2/optimizations_test.clj |  2 +-
 3 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/src/hiccup/compiler.clj b/src/hiccup/compiler.clj
index 0533f74..e1db82f 100644
--- a/src/hiccup/compiler.clj
+++ b/src/hiccup/compiler.clj
@@ -331,26 +331,34 @@
   (compile-element (apply vector tag {} content)))
 
 (defmethod compile-element ::literal-tag
-  [[tag attrs & content]]
+  [[tag attrs-or-content & content]]
   (let [[tag tag-attrs _] (normalize-element-form [tag])
-        attrs-sym         (gensym "attrs")]
-    `(let [~attrs-sym ~attrs]
+        attrs-or-content-sym (gensym "attrs_or_content__")
+        attrs?-sym           (gensym "attrs?__")
+        content?-sym         (gensym "content?__")]
+    `(let [~attrs-or-content-sym ~attrs-or-content
+           ~attrs?-sym (map? ~attrs-or-content-sym)
+           ~content?-sym (and (not ~attrs?-sym)
+                              (some? ~attrs-or-content-sym))]
        (build-string
-        (if (map? ~attrs-sym)
-          ~(if (container-tag? tag content)
-             `(build-string ~(str "<" tag)
-                            (render-attr-map (merge ~tag-attrs ~attrs-sym))
-                            ">")
-             `(build-string ~(str "<" tag)
-                            (render-attr-map (merge ~tag-attrs ~attrs-sym))
-                            ~(end-tag)))
-          (build-string ~(str "<" tag (render-attr-map tag-attrs) ">")
-                        ~@(compile-seq [attrs-sym])))
+        ;; start tag
+        "<" ~tag
+        (if ~attrs?-sym
+          (render-attr-map (merge ~tag-attrs ~attrs-or-content-sym))
+          ~(render-attr-map tag-attrs))
+        ~(if (container-tag? tag content)
+           ">"
+           `(if ~content?-sym ">" ~(end-tag)))
+
+        ;; contents
+        (when ~content?-sym
+          (render-html ~attrs-or-content-sym))
         ~@(compile-seq content)
-        ;; ending tag, when the above code did not emit an ending tag
+
+        ;; end tag
         ~(if (container-tag? tag content)
            (str "</" tag ">")
-           `(when-not (map? ~attrs-sym)
+           `(when ~content?-sym
               ~(str "</" tag ">")))))))
 
 (defmethod compile-element ::default
diff --git a/test/hiccup/compiler_test.clj b/test/hiccup/compiler_test.clj
index 91b3830..2c9fca9 100644
--- a/test/hiccup/compiler_test.clj
+++ b/test/hiccup/compiler_test.clj
@@ -53,18 +53,17 @@
       (is (= (str (html {:mode :sgml} [:br (identity "x")])) "<br>x</br>"))))
 
   (testing "runtime nil,"
+    ;; use case: a function which returns a map of attributes or nil
     (testing "normal tag"
       (is (= (str (html {:mode :xhtml} [:p (identity nil)])) "<p></p>"))
       (is (= (str (html {:mode :html} [:p (identity nil)])) "<p></p>"))
-      (is (= (str (html {:mode :xml} [:p (identity nil)])) "<p></p>"))
-      (is (= (str (html {:mode :sgml} [:p (identity nil)])) "<p></p>")))
+      (is (= (str (html {:mode :xml} [:p (identity nil)])) "<p />"))
+      (is (= (str (html {:mode :sgml} [:p (identity nil)])) "<p>")))
     (testing "void tag"
-      ;; TODO: this might not be desired behavior (use case: the user has
-      ;;       a function which returns a map of attributes or nil)
-      (is (= (str (html {:mode :xhtml} [:br (identity nil)])) "<br></br>"))
-      (is (= (str (html {:mode :html} [:br (identity nil)])) "<br></br>"))
-      (is (= (str (html {:mode :xml} [:br (identity nil)])) "<br></br>"))
-      (is (= (str (html {:mode :sgml} [:br (identity nil)])) "<br></br>")))))
+      (is (= (str (html {:mode :xhtml} [:br (identity nil)])) "<br />"))
+      (is (= (str (html {:mode :html} [:br (identity nil)])) "<br>"))
+      (is (= (str (html {:mode :xml} [:br (identity nil)])) "<br />"))
+      (is (= (str (html {:mode :sgml} [:br (identity nil)])) "<br>")))))
 
 (deftest test-compile-element-default
   (testing "runtime tag"
diff --git a/test/hiccup2/optimizations_test.clj b/test/hiccup2/optimizations_test.clj
index 44fef99..b55c519 100644
--- a/test/hiccup2/optimizations_test.clj
+++ b/test/hiccup2/optimizations_test.clj
@@ -75,4 +75,4 @@
                          (partition 2 1)
                          (map (fn [[a b]] (- b a))))]
       (is (< (apply max diffs)
-             (* 1.1 (apply min diffs)))))))
+             (* 1.2 (apply min diffs)))))))