ladderlife / autochrome

Structural diffs for clojure source code
Eclipse Public License 1.0
186 stars 11 forks source link

Autochrome fails on namespaced maps #8

Closed jumarko closed 6 years ago

jumarko commented 6 years ago

I have code which uses namespaced maps and autochrome fails to produce such diffs. I guess this is because autochrome tries to parse this as a reader literal.

One part of the code looks like this:

(let [slack-config #:notification-provider{:type "slack"
                                                                     :active? (= "on" slack-active) }]

    slack-config)

Exception:

23 changed files
Exception in thread "main" clojure.lang.ExceptionInfo: bad dispatch form {:bad-token {:type :keyword, :text ":notification-provider"}}
        at clojure.core$ex_info.invokeStatic(core.clj:4617)
        at clojure.core$ex_info.invoke(core.clj:4617)
        at autochrome.parse$_parse_one.invokeStatic(parse.clj:233)
        at autochrome.parse$_parse_one.invoke(parse.clj:196)
        at autochrome.parse$parse_list.invokeStatic(parse.clj:278)
        at autochrome.parse$parse_list.invoke(parse.clj:264)
        at autochrome.parse$parse_list.invokeStatic(parse.clj:270)
        at autochrome.parse$parse_list.invoke(parse.clj:264)
        at autochrome.parse$parse_list.invokeStatic(parse.clj:270)
        at autochrome.parse$parse_list.invoke(parse.clj:264)
        at autochrome.parse$_parse_one.invokeStatic(parse.clj:200)
        at autochrome.parse$_parse_one.invoke(parse.clj:196)
        at autochrome.parse$parse_many.invokeStatic(parse.clj:295)
        at autochrome.parse$parse_many.invoke(parse.clj:288)
        at autochrome.parse$parse.invokeStatic(parse.clj:313)
        at autochrome.parse$parse.invoke(parse.clj:310)
        at autochrome.page$clojure_diff.invokeStatic(page.clj:117)
        at autochrome.page$clojure_diff.invoke(page.clj:113)
        at autochrome.page$diff_page$fn__5631.invoke(page.clj:155)
        at clojure.lang.AFn.applyToHelper(AFn.java:154)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.core$apply.invokeStatic(core.clj:646)
        at clojure.core$apply.invoke(core.clj:641)
        at com.climate.claypoole$pmap_core$start_task__5141$fn__5142.invoke(claypoole.clj:323)
        at clojure.lang.AFn.applyToHelper(AFn.java:152)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.lang.AFunction$1.doInvoke(AFunction.java:29)
        at clojure.lang.RestFn.invoke(RestFn.java:397)
        at com.climate.claypoole.impl$binding_conveyor_fn$fn__5019.invoke(impl.clj:46)
        at clojure.lang.AFn.applyToHelper(AFn.java:152)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.lang.AFunction$1.doInvoke(AFunction.java:29)
        at clojure.lang.RestFn.invoke(RestFn.java:397)
        at clojure.lang.AFn.call(AFn.java:18)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:748)
        at java.util.concurrent.FutureTask.report(FutureTask.java:122)
        at java.util.concurrent.FutureTask.get(FutureTask.java:192)
        at com.climate.claypoole.impl$deref_future.invokeStatic(impl.clj:52)
        at com.climate.claypoole.impl$deref_future.invoke(impl.clj:49)
        at com.climate.claypoole$future_call$reify__5132.deref(claypoole.clj:264)
        at clojure.core$deref.invokeStatic(core.clj:2228)
        at clojure.core$deref.invoke(core.clj:2214)
        at com.climate.claypoole.impl$deref_fixing_exceptions.invokeStatic(impl.clj:65)
        at com.climate.claypoole.impl$deref_fixing_exceptions.invoke(impl.clj:58)
        at clojure.core$map$fn__4785.invoke(core.clj:2646)
        at clojure.lang.LazySeq.sval(LazySeq.java:40)
        at clojure.lang.LazySeq.seq(LazySeq.java:49)
        at clojure.lang.RT.seq(RT.java:521)
        at clojure.core$seq__4357.invokeStatic(core.clj:137)
        at clojure.core$concat$fn__4446.invoke(core.clj:706)
        at clojure.lang.LazySeq.sval(LazySeq.java:40)
        at clojure.lang.LazySeq.seq(LazySeq.java:49)
        at clojure.lang.Cons.next(Cons.java:39)
        at clojure.lang.RT.boundedLength(RT.java:1749)
        at clojure.lang.RestFn.applyTo(RestFn.java:130)
        at clojure.core$apply.invokeStatic(core.clj:646)
        at clojure.core$apply.invoke(core.clj:641)
        at autochrome.page$diff_page.invokeStatic(page.clj:160)
        at autochrome.page$diff_page.invoke(page.clj:144)
        at autochrome.page$pull_request_diff.invokeStatic(page.clj:171)
        at autochrome.page$pull_request_diff.invoke(page.clj:169)
        at autochrome.core$do_main$fn__5901.invoke(core.clj:26)
        at autochrome.core$do_main.invokeStatic(core.clj:22)
        at autochrome.core$do_main.doInvoke(core.clj:18)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invokeStatic(core.clj:646)
        at clojure.core$apply.invoke(core.clj:641)
        at autochrome.core$_main.invokeStatic(core.clj:44)
        at autochrome.core$_main.doInvoke(core.clj:42)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at autochrome.core.main(Unknown Source)
fazzone commented 6 years ago

Hi, thank you for the report! As you may have guessed we are not using clojure 1.9 yet at my company so this was an oversight on my part. I believe I have fixed it with this commit on master. A bit of a hack since it is treating namespaced maps as a kind of data reader, but for the purpose of diffing this should be fine. If this fixes it for you then feel free to close this issue, otherwise let me know how it breaks!

jumarko commented 6 years ago

Hi, @fazzone, thanks a lot for the prompt response - incredible :). I've just tried it and it seems to work - at least, it breaks with another error. I'm not exactly sure what's the problem in this case and cannot locate the problematic parts in my code, so sorry for the incomplete report

Exception in thread "main" clojure.lang.ExceptionInfo: unhashable {:form #object[clojure.core$val 0x507b79f7 "clojure.core$val@507b79f7"]}
        at clojure.core$ex_info.invokeStatic(core.clj:4617)
        at clojure.core$ex_info.invoke(core.clj:4617)
        at autochrome.tree$put_hashes.invokeStatic(tree.clj:53)
        at autochrome.tree$put_hashes.invoke(tree.clj:42)
        at autochrome.tree$put_hashes.invokeStatic(tree.clj:59)
        at autochrome.tree$put_hashes.invoke(tree.clj:42)
        at autochrome.tree$put_hashes.invokeStatic(tree.clj:59)
        at autochrome.tree$put_hashes.invoke(tree.clj:42)
        at autochrome.diff$diff_prep.invokeStatic(diff.clj:70)
        at autochrome.diff$diff_prep.invoke(diff.clj:65)
        at autochrome.align$get_diffs.invokeStatic(align.clj:7)
        at autochrome.align$get_diffs.invoke(align.clj:5)
        at autochrome.page$two_file_diff.invokeStatic(page.clj:83)
        at autochrome.page$two_file_diff.invoke(page.clj:81)
        at autochrome.page$clojure_diff.invokeStatic(page.clj:125)
        at autochrome.page$clojure_diff.invoke(page.clj:113)
        at autochrome.page$diff_page$fn__5631.invoke(page.clj:155)
        at clojure.lang.AFn.applyToHelper(AFn.java:154)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.core$apply.invokeStatic(core.clj:646)
        at clojure.core$apply.invoke(core.clj:641)
        at com.climate.claypoole$pmap_core$start_task__5141$fn__5142.invoke(claypoole.clj:323)
        at clojure.lang.AFn.applyToHelper(AFn.java:152)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.lang.AFunction$1.doInvoke(AFunction.java:29)
        at clojure.lang.RestFn.invoke(RestFn.java:397)
        at com.climate.claypoole.impl$binding_conveyor_fn$fn__5019.invoke(impl.clj:46)
        at clojure.lang.AFn.applyToHelper(AFn.java:152)
        at clojure.lang.AFn.applyTo(AFn.java:144)
        at clojure.lang.AFunction$1.doInvoke(AFunction.java:29)
        at clojure.lang.RestFn.invoke(RestFn.java:397)
        at clojure.lang.AFn.call(AFn.java:18)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:748)
        at java.util.concurrent.FutureTask.report(FutureTask.java:122)
        at java.util.concurrent.FutureTask.get(FutureTask.java:192)
        at com.climate.claypoole.impl$deref_future.invokeStatic(impl.clj:52)
        at com.climate.claypoole.impl$deref_future.invoke(impl.clj:49)
        at com.climate.claypoole$future_call$reify__5132.deref(claypoole.clj:264)
        at clojure.core$deref.invokeStatic(core.clj:2228)
        at clojure.core$deref.invoke(core.clj:2214)
        at com.climate.claypoole.impl$deref_fixing_exceptions.invokeStatic(impl.clj:65)
        at com.climate.claypoole.impl$deref_fixing_exceptions.invoke(impl.clj:58)
        at clojure.core$map$fn__4785.invoke(core.clj:2646)
        at clojure.lang.LazySeq.sval(LazySeq.java:40)
        at clojure.lang.LazySeq.seq(LazySeq.java:49)
        at clojure.lang.RT.seq(RT.java:521)
        at clojure.core$seq__4357.invokeStatic(core.clj:137)
        at clojure.core$concat$fn__4446.invoke(core.clj:706)
        at clojure.lang.LazySeq.sval(LazySeq.java:40)
        at clojure.lang.LazySeq.seq(LazySeq.java:49)
        at clojure.lang.Cons.next(Cons.java:39)
        at clojure.lang.RT.boundedLength(RT.java:1749)
        at clojure.lang.RestFn.applyTo(RestFn.java:130)
        at clojure.core$apply.invokeStatic(core.clj:646)
        at clojure.core$apply.invoke(core.clj:641)
        at autochrome.page$diff_page.invokeStatic(page.clj:160)
        at autochrome.page$diff_page.invoke(page.clj:144)
        at autochrome.page$pull_request_diff.invokeStatic(page.clj:171)
        at autochrome.page$pull_request_diff.invoke(page.clj:169)
fazzone commented 6 years ago

Oops, that was a silly mistake by me. Should be fixed...

jumarko commented 6 years ago

Moving forward :). This time there's a problem with namespaced maps destructuring like this:

{:notification/keys [template] :as notification}
Caused by: java.lang.Exception: walking (fn format-message [{:notification/keys [template] :as notification}] (format-template template notification))
        at autochrome.scope$walk_with_scope.invokeStatic(scope.clj:316)
        at autochrome.scope$walk_with_scope.invoke(scope.clj:289)
        at autochrome.scope$walk_body.invokeStatic(scope.clj:67)
        at autochrome.scope$walk_body.invoke(scope.clj:59)
        at autochrome.scope$walk_binding_form.invokeStatic(scope.clj:287)
        at autochrome.scope$walk_binding_form.invoke(scope.clj:263)
        at autochrome.scope$walk_with_scope.invokeStatic(scope.clj:315)
        ... 82 more
Caused by: clojure.lang.ExceptionInfo: unsupported map destructuring (:notification/keys) {:binding-form {:type :coll, :delim \{, :wscontents [{:type :keyword, :text ":notification/keys"} {:type :ws, :text " "} {:type :coll, :delim \[, :wscontents [{:type :symbol, :text "template"}], :contents ({:type :symbol, :text "template"})} {:type :ws, :text " "} {:type :keyword, :text ":as"} {:type :ws, :text " "} {:type :symbol, :text "notification"}], :contents ({:type :keyword, :text ":notification/keys"} {:type :coll, :delim \[, :wscontents [{:type :symbol, :text "template"}], :contents ({:type :symbol, :text "template"})} {:type :keyword, :text ":as"} {:type :symbol, :text "notification"})}, :rendered "{:notification/keys [template] :as notification}"}
fazzone commented 6 years ago

We are observing why "uses its own clojure parser" was listed as a misfeature in the readme. Should be fixed by another hack

jumarko commented 6 years ago

Wonderful - it now works like a charm. Thanks a ton for such a great tool and world-class support.

fazzone commented 6 years ago

👍