Migrate to tools.reader to solve aliased keyword pains #198
Conversation
- Takes dependency on clojure.tools.reader 1.0.2, lein deps :tree remains clean for this project.clj - Introduces kibit.check.reader namespace for managing all our reading concerns. - Migrate reading fns from kibit.check to kibit.check.reader - Add alias analyzing & util fns to kibit.check.reader - Add tests for analyzing fns to kibit.check-reader - Add keywords.clj test resource which includes several iterations of alias backflips to make sure that some simple edge cases for keyword reading are handled.
|
👍 this looks excellent. Thanks for taking the time. I have no complaints after my read, but I'll let @danielcompton click the button. |
| (ns resources.keywords | ||
| (:require [clojure.java.io :as io])) | ||
|
|
||
| (defn aliased-keyword-access |
There was a problem hiding this comment.
It could be good to add some auto qualified namespaced keys in the test too: ::foo.
|
|
||
| (defn using-a-different-keyword-alias-in-different-ns | ||
| [z] | ||
| (::str/another-fake-key z)) |
There was a problem hiding this comment.
Perhaps in a different file, I'd like to add tests for what a Kibit suggestion would look like, particularly around which namespace/namespace alias is used in an expansion. They don't have to actually be correct expansions, but it would be good to document via test what our expectations are.
|
Hi Alex. Massive thanks to you for putting this together. I really like it, and it works really well. I added a few things that I noted in my review (that was mainly for my own notes), and also added support for |
Fixes #14
clean for this project.clj
concerns.
alias backflips to make sure that some simple edge cases for
keyword reading are handled.
Notes:
I tested this across our internal projects which we want to integrate kibit into our CI process on, where we use a lot of namespace aliased keywords (largely due to spec)
We had previously had a blind spot past any aliased keyword in any file, so using this branch surfaced many more kibit improvements in our code base. Additionally, kibit ran to completion successfully in both projects.
We still have a blind spot in that some of our ClojureScript code contains
#js {:foo :bar}tagged literals that are not properly handled in either clojure.core/read or clojure.tools.reader/read.Love to hear feedback if people can play with this branch on their own source trees and vet it for correctness.