Relax regexp for clojure-find-def to recognize more complex metadata on vars

[frwdrik]

This PR tries to solve an issue that was originally reported on the CIDER project, but was later found to be related to clojure-mode: clojure-emacs/cider#3758.

In summary, the problem encountered is that running cider-test-run-test on a form like this

(deftest ^{:a {}} complex-metadata
  (println "complex-metadata")
  (is true))

does not correctly recognize this as a test (in fact it doesn't recognize this as a def-form at all). I traced the underlying issue to the regexp clojure-def-type-and-name-regex used by clojure-find-def, which I assume make a best-effort attempt at recognizing metadata on vars, but which is not able to match on the form above since it doesn't match metadata maps with more than one right-brace }.

The approach in this PR is to modify the regexp to allow for more than one right-brace at the end of the metadata, if it is a map. A more robust approach would be to use an actual Clojure parser, but that might be a bit excessive.

I didn't find any tests related to clojure-find-def, but please let me know if I should add one.

---

Before submitting a PR mark the checkboxes for the items you've done (if you
think a checkbox does not apply, then leave it unchecked):

• The commits are consistent with our contribution guidelines.
• You've added tests (if possible) to cover your change(s). Bugfix, indentation, and font-lock tests are extremely important!
• You've run M-x checkdoc and fixed any warnings in the code you've written.
• You've updated the changelog (if adding/changing user-visible functionality).
• You've updated the readme (if adding/changing user-visible functionality).

Thanks!

[vemv]

LGTM!

Because the addition is a single + to a regex, there's practically no risk of breaking anything with this.

You might enjoy adding some test cases - since this is a pure function it should easy to add a variety:

• without / with metadata
• def/defn/defwhatever
• ^:private

That's optional though.

[bbatsov]

The proposed changes look good to me. Just add a bugfix entry to the changelog and we're good to go.

[frwdrik]

Thanks for the quick feedback! I added a few tests based on the issue reported by @mbezjak.

While doing so, I came across this failing test case:

(with-clojure-buffer-point
    "(defn- bar |[x y z] z)"
    (expect (clojure-find-def) :to-equal '("defn-" "bar")))

In other words, clojure-find-def doesn't recognize defn- (it actually returns ("defn" "-")). I don't know whether this can be classified as a bug, or just ouf of scope for clojure-find-def.

[bbatsov]

In other words, clojure-find-def doesn't recognize defn- (it actually returns ("defn" "-")). I don't know whether this can be classified as a bug, or just ouf of scope for clojure-find-def.

Yeah, that's definitely a bug.

[frwdrik]

In other words, clojure-find-def doesn't recognize defn- (it actually returns ("defn" "-")). I don't know whether this can be classified as a bug, or just ouf of scope for clojure-find-def.

Yeah, that's definitely a bug.

Seems like the issue is that in this part of the clojure-def-type-and-name-regex regex:

          ;; Declaration
          "\\(def\\(?:\\sw\\|\\s_\\)*\\)\\>"

the sw and s_ only match word and symbol constituents, respectively, as described in https://www.gnu.org/software/emacs/manual/html_node/elisp/Syntax-Class-Table.html, and in particular don't match hyphens -. Adding - as another alternate in the regex seems to fix it. I can make a separate PR for that if desired.

[frwdrik]

Thanks again for your quick feedback! I'd be happy to make a PR for the defn- issue above (or add it to this one if you prefer). Apart from that, need I be taking any other steps to move this PR forward?

[bbatsov]

We're all good here. Thanks!

I'd be happy to make a PR for the defn- issue above

A separate PR would be great!