data_readers merging can produce duplicate keys, making the merged file unreadable

[imrekoszo]

Affects 1.3.0 and 1.4.0

Looks like this change can create unreadable code when multiple data readers maps have the same key. In fact, the examples in the test appear to be unreadable (note the duplicate common keys):

{project-a a
 common A
 project-b #?{:clj clj-b :cljs cljs-b}
 #?@{:clj [clj-only clj]}
 #?@{:cljs [cljs-only cljs]}
 common B}

{project-a a
 common A
 project-c c
 common C}

Stumbled upon this after bumping from 1.0.4 to 1.4.0:

clojure.lang.LispReader$ReaderException: java.lang.IllegalArgumentException: Duplicate key: time/period
 	at clojure.lang.LispReader.read(LispReader.java:316)
 	at clojure.lang.LispReader.read(LispReader.java:218)
 	at clojure.lang.LispReader.read(LispReader.java:207)
 	at clojure.core$read.invokeStatic(core.clj:3794)
 	at clojure.core$load_data_reader_file.invokeStatic(core.clj:8047)
...

Originally posted by @imrekoszo in #51

[tonsky]

PR? It should be relatively straightforward

uberdeps/src/uberdeps/api.clj

Lines 107 to 115 in 3cc9de1

	(def data-readers-merger
	{:collect
	(fn [acc content]
	(conj (or acc [])
	(second
	(re-matches #"(?ms)\s*\{(.*)\}\s*" content))))
	:combine
	(fn [acc]
	(str "{" (str/join "\n " acc) "}"))})

[imrekoszo]

Had no time since noticing it. At first I thought it would be straightforward but if you want to be 100% sure you'd have to unroll (potentially stupid) reader conditionals wouldn't you?

[tonsky]

Sure, but even without them should be good enough

[tonsky]

What I’m trying to say: we can solve simple problem first and already help some cases. When a person comes with crazy reader conditional AND a conflict we can work on that too. But even without it it will be a meaningful improvement

[sogaiu]

I stumbled upon this file doing some testing of a tree-sitter grammar and the sense I got was that one issue is that the reader conditionals are not quite right:

#?{:clj clj-b :cljs cljs-b}

May be what one wants is to use round parens and not curly braces, e.g. like this?

#?(:clj clj-b :cljs cljs-b)

Sorry if that's off 😅

[tonsky]

@sogaiu 100%, that is a mistake