Skip to content

Fix parsing ns :require vectors with #_ comments#416

Merged
weavejester merged 1 commit into
weavejester:masterfrom
camsaul:fix-415
May 24, 2026
Merged

Fix parsing ns :require vectors with #_ comments#416
weavejester merged 1 commit into
weavejester:masterfrom
camsaul:fix-415

Conversation

@camsaul
Copy link
Copy Markdown
Contributor

@camsaul camsaul commented May 22, 2026

Fixes #415

Metadata ^... or uneval #_ comment forms in various places in a form within ns :require weren't handled correctly (before the RHS symbol of :as, before the :refers vector, before symbols within :refers, etc.), so I fixed everything and made the tests must more comprehensive; I also realized that technically :refer (x) (list instead of vector) is legal so I updated this to handle that situation as well.

@camsaul camsaul force-pushed the fix-415 branch 2 times, most recently from 5dfa2fe to 779aea3 Compare May 22, 2026 19:56
(str "[" ignore-str "thing.core :as t]")
(str ignore-str " [" ignore-str "thing.core :as t]")]
:let [ns-str (str "(ns example (:require " ns-vec-str "))")]]
require-str ["<>[<>thing.core :as <>t :refer <>[<>my-defn]]"
Copy link
Copy Markdown
Contributor Author

@camsaul camsaul May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There would have been over 5000 variations getting tested here if we did just put the metadata or uneval #_ comment form in each individual location it could go separately so instead I'm just putting it in every possible slot at once. <> is a placeholder for places it can go here and then we str/replace below

"(t/defn foo [x]"
"(+ x 1))"
""
"(my-defn foo [x]"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to also test :refer handling at the same time


#?(:clj
(defn- refer-zloc->refer-mapping [refer-zloc]
(let [refer-vec (some-> refer-zloc z/right z/sexpr)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refer-vec was incorrect since x in :refer x can be either a list or a vector

@camsaul camsaul force-pushed the fix-415 branch 4 times, most recently from 9a33f06 to 5f88abc Compare May 22, 2026 22:04
"<>[<>thing <>[<>core :as <>t :refer <>[<>my-defn]]]"]
:let [ns-str (str "(ns my-namespace (:require " require-str "))")]
ns-str [ns-str
(-> ns-str
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try with both vectors for everything and lists for everything since either is allowed I guess, but change everything at once so we don't end up with too much of a combinatorial explosion

Comment on lines +334 to +335
ns-str [(str/replace ns-str #"<>" ignore-str)
(str/replace ns-str #"<>" (str ignore-str ignore-str))]]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found another bug where multiple metadata forms e.g. ^:private ^:dynamic or whatever were not skipped over correctly in skip-meta, so I fixed that was well; updated this test to make sure we're checking strings where we use multiple uneval #_... comment forms or ^... metadata forms in a row

@weavejester
Copy link
Copy Markdown
Owner

Thanks for the patch. I reviewed the code and everything looks fine, so I'll go ahead and merge.

@weavejester weavejester merged commit 22a974b into weavejester:master May 24, 2026
1 check passed
@camsaul camsaul deleted the fix-415 branch May 26, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cljfmt errors when namespace contains #_ comment forms inside :require vectors

2 participants