Clojure macro introduces name -- Joker thinks it is not declared

[seancorfield]

I have a macro that introduces a local symbol and it's used like this:

  (api/with-validated-params [::api-spec/reset-new-stats nil]
    (act/visit (:user-id-int params) (:pagename params))
    (resp/response {})))

The macro takes (:params req), applies the conforming clojure.spec stuff, and introduces a local binding of params. Unfortunately, Joker flags the use of params on the next line as invalid.

I verified that :known-macros is working correctly here: if I add foo immediately after with-validated-params in that code, Joker doesn't flag that -- so it's assuming that a macro is invoked with a symbol and that is defining something new.

I guess it doesn't allow for a deliberately unhygienic macro that introduces a new local binding?

[candid82]

:know-macros only disables symbol resolution on top level. Once it's inside a nested call (which is the case in your example: (:user-id-int params) and (:pagename params) are nested calls.), it re-enables symbol resolution. That's why it flags params as unresolvable.
Joker is trying to balance between false positives and false negatives, and I think it's doing reasonable thing here. Completely disabling symbol resolution inside macros would make Joker much less useful. For example, it would not catch the problem in the below snippet:

(api/with-validated-params [::api-spec/reset-new-stats nil]
    (act/visit (:user-id-int params) (typo (:pagename params)))
    (resp/response {})))

I am open to suggestions on how to improve the algorithm though.

As a side note, I almost feel like Joker should flag things like this and discourage macros that implicitly introduce new bindings. They may be very confusing, especially for someone new to the codebase, and can generally be the source of subtle bugs. For example, what if the above snippet is wrapped in something like (let [params ...] (api/with-validated-params ...))? A reader not intimately familiar with with-validated-params may assume that params inside with-validated-params comes from the surrounding let.

[seancorfield]

Yup, that's all totally reasonable. I realized that if the macro accepted the symbol name to introduce as the first element of the vector, the warning goes away:

  (api/with-validated-params [params ::api-spec/reset-new-stats nil]
    (act/visit (:user-id-int params) (:pagename params))
    (resp/response {})))

It looks like the macro logic assumes that symbols in the first argument are introduced as global symbols? I put a test symbol in that vector and Joker no longer flagged undefined instances of that same symbol lower in the file.

That's not a criticism (nor really a bug) -- I'm just trying to establish the rules that Joker uses.

I'd actually be happy to update our macro so the params symbol was part of that vector spec so developers could use a different name, which would have the side effect of satisfying Joker :)

I'll be opening a new issue soon with some Expectations code that Joker flags, that I'm not sure what to do about (summary: Expectations has a defexpect macro that wraps forms like (expect 13 (something)) and expect and several other nested symbols have meaning inside defexpect -- I would assume that's a fairly common scenario with DSLs).

[candid82]

Your second example works because Joker first encounters params at the top level (that is, not inside a nested call) and assumes it's a binding introduced by the macro, so subsequent occurrences of params are not flagged even inside nested calls. So Joker works well with macros that introduce new bindings explicitly and have the following form:

(macro-name [bindings]
  body)

[seancorfield]

I would expect such macros to only introduce local names tho', not top-level names.

(macro-name foo ...) would introduce a top-level name.

(macro-name [foo ...] ...) normally only introduces names inside the body of the macro.

[candid82]

Good point. Not sure if distinguishing those two cases is worth additional complexity but I'll see what I can do.
I did implement support for macros that introduce new bindings implicitly. In the next release of Joker you'll be able to specify symbols that are introduced by a macro in .joker file:

{:known-macros [[riemann.streams/where [service event]]]}

So each element in :known-macros vector can now be either a symbol (as before) or a vector with two elements: macro's name and a list of symbols introduced by this macro.

[seancorfield]

Nice! So I'd be able to say {:known-macros [[api.support/with-validated-params [params]] ...]} -- that's great!

[seancorfield]

@candid82 Looks like some nice changes since v0.8.3 -- is v0.8.4 on the horizon yet, or do you have other features/fixes planned before the next release?

[candid82]

I'll see if I can squeeze in other fixes, but planning on releasing v0.8.4 this week.

RE: distinguishing top level vs. local bindings inside macros. I played with the idea a bit but ultimately decided to abandon it (at least for now). Simple heuristic like "top level symbols inside a macro call introduce top level var, everything else creates a local binding" break down in macros like defrecord or defprotocol that introduce top level bindings (methods) inside nested lists. Trying to come up with "smarter" heuristic feels like playing whack-a-mole.

[seancorfield]

I look forward to testing the new release. Thank you!