From 8a3b6c735d7640117e5c80f5bb7e84e6c2997c51 Mon Sep 17 00:00:00 2001
From: Kaz Kylheku <kaz@kylheku.com>
Date: Mon, 29 May 2023 20:11:52 -0700
Subject: Add warning when local macros used in attr expression.

The with-html-output ("WHO") macro defines some local macrolets.
Those are in scope of the entire thing, but they can only be
meaningfully used in the imperative Lisp forms that are placed
into the HTML tag body. If they are used in an attribute
expression, it's probably a programming error; an output
side effect doesn't belong there.  We implement a diagnostic
strategy which produces a warning in this situation.

* who.tl (attr-warning-macrolet): New function, which
wraps macrolet code around the expression passed in.
These macrolets intercept the local macros and generate
a warning. Then they decline to expand, deferring to the
real macros.
(convert-attributes): Wrap the warning macrolets around
the run-time evaluation of the val expression. We don't
do anything for the compile-time evaluation of a constant
attribute expression, because the macro calls we want
to intercept do not appear in a constant expression.

* test/simple.tl: New test 41 validating that warnings
are generated for all the local macros when they are called
from an attr expression.
---
 test/simple.tl | 13 +++++++++++++
 who.tl         | 14 +++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/test/simple.tl b/test/simple.tl
index 3601814..b9d1a65 100644
--- a/test/simple.tl
+++ b/test/simple.tl
@@ -449,3 +449,16 @@
         (with-html-output-to-string (out)
           (:foo (fmt "<a href=\"foo.html\">click</a>"))))
       "<foo><a href=\"foo.html\">click</a></foo>")
+
+;;; 41
+;;; Test that warning is produced when any of the WHO local macros are used in
+;;; an Lisp expression that calculates an attribute
+(each ((sym '(htm noesc-fmt fmt esc str)))
+  (test (catch
+          (eval '(progn
+                   (with-html-output-to-string (out)
+                     (:foo :attr (,sym "abc")))
+                   nil))
+          (warning (x)
+            t))
+        t))
diff --git a/who.tl b/who.tl
index 4d0c63d..90932f5 100644
--- a/who.tl
+++ b/who.tl
@@ -85,6 +85,18 @@
        (set body (rest sexp)))))
     (convert-tag-to-string-list tag attr-list body body-fn)))
 
+(defun attr-warning-macrolet (form)
+  (with-gensyms (warn)
+    ^(macrolet ((,warn (f . rest)
+                  ^(compile-warning ,f
+                                    "not recommended in attribute expr")))
+       (macrolet ((htm (:form f . rest) (,warn f) f)
+                  (noesc-fmt (:form f . rest) (,warn f) f)
+                  (fmt (:form f . rest) (,warn f) f)
+                  (esc (:form f . rest) (,warn f) f)
+                  (str (:form f . rest) (,warn f) f))
+       ,form))))
+
 ;; Helper function for convert-tag-to-string-list which converts the
 ;; alist attr-list of attributes into a list of strings and/or Lisp
 ;; forms.
@@ -110,7 +122,7 @@
                   ` @attr=@aqc@(tostringp eval)@aqc`)
                 (t ` @attr=@aqc@(html-encode (tostringp eval))@aqc`)))
             ;; For non-constant, do the same things as above but at runtime
-            ^(let ((,=var= ,val))
+            ^(let ((,=var= ,(attr-warning-macrolet val)))
                (cond
                  ((null ,=var=))
                  ((eq ,=var= t)
-- 
cgit v1.2.3