borkdude / grasp

Grep Clojure code using clojure.spec regexes
Eclipse Public License 1.0
242 stars 7 forks source link

Allow to record more information for matches #26

Closed mk closed 2 years ago

mk commented 2 years ago

For a grasp analysis I did on one of our projects, I was interested in finding all usages of :pre and :post maps in defns. For this, I thought the fully-qualified var name would make a better identifier than the plain name and the file. But since grasp/resolve-symbol depends on the *ctx* being bound, I believe we need to add a way to run a user-provided function during analysis to enable this.

Another (smaller) benefit of this could be that we can avoid needing to run conform twice like in the arg-vec example.

borkdude commented 2 years ago

As we now are going to expose the function, I find it a bit dangerous to settle on a fixed argument function since we lose the ability to pass more information to it later. Perhaps we should make it a map argument, but not sure how this would affect performance. Alternatively we could force people to provide a multi-arity function, but this would be a more awkward API.

borkdude commented 2 years ago

I'll experiment a bit with it. If the map argument's performance is roughly the same as the fixed args, I'd say we go with that.

borkdude commented 2 years ago

Performance test using the reify test, done 10 times:

Before:

$ bb test:clj

Running tests in #{"test"}

Testing grasp.api-test
"Elapsed time: 2189.353677 msecs"

Ran 12 tests containing 31 assertions.
0 failures, 0 errors.

After:

$ bb test:clj

Running tests in #{"test"}

Testing grasp.api-test
"Elapsed time: 1920.635267 msecs"

Ran 12 tests containing 31 assertions.
0 failures, 0 errors.

Conclusion: it seems that there is no significant effect on performance.

I'll merge your commit together with my map API change.