Improve round-trip

.gitignore

@@ -12,3 +12,6 @@ pom.xml.asc
 .lein-plugins
 .lein-repl-history
 .nrepl-port
+.calva
+.clj-kondo
+.lsp

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## 0.4.0
+
+- Update dependencies and build targets
+- Add generative tests
+- Add deps.edn support (via git coordinate)
+- Fix issue where falsey map keys were not handled correctly by diff or patch.
+- Fix issue where collections could not be cleared.
+- Fix issue #3 where patch throws instead of replacing a vector with an number.
+- Fix issue #2 where records were treated as maps and threw
+
 ## 0.3.3
 
 - Update dependencies and build targets

README.md

@@ -12,6 +12,12 @@ Add the following the to your `project.clj`:
 
 [![Clojars Project](http://clojars.org/differ/latest-version.svg)](http://clojars.org/differ)
 
+Or git coordinates in deps.edn:
+
+```clojure
+  :deps {io.github.robinheghan/differ {:git/tag "THE_TAG" :git/sha "THE_SHA"}}
+```
+
 ## Usage
 
 First of all, you need to require the proper namespace:

deps.edn

@@ -0,0 +1,3 @@
+{:aliases
+ {:test {:extra-paths ["test"]
+         :extra-deps {org.clojure/test.check {:mvn/version "1.1.1"}}}}}

project.clj

@@ -1,4 +1,4 @@
-(defproject differ "0.3.3"
+(defproject differ "0.4.0"
   :description "A library for diffing, and patching, Clojure(script) data structures"
   :url "https://github.com/Skinney/differ"
   :license {:name "MIT"
@@ -10,13 +10,15 @@
 
   :profiles {:1.8 {:dependencies [[org.clojure/clojure "1.8.0"]]}
              :1.9 {:dependencies [[org.clojure/clojure "1.9.0"]]}
-             :1.10 {:dependencies [[org.clojure/clojure "1.10.1"]]}
-             :cljs {:dependencies [[org.clojure/clojurescript "1.10.520"]]
-                    :plugins [[lein-cljsbuild "1.1.2"]]
+             :1.10 {:dependencies [[org.clojure/clojure "1.10.3"]]}
+             :1.11 {:dependencies [[org.clojure/clojure "1.11.1"]]}
+             :cljs {:dependencies [[org.clojure/clojurescript "1.11.60"]]
+                    :plugins [[lein-cljsbuild "1.1.8"]]
                     :cljsbuild {:test-commands {"phantom" ["phantomjs" :runner "target/testable.js"]}
                                 :builds [{:source-paths ["src" "test"]
                                           :compiler {:output-to "target/testable.js"
                                                      :optimizations :none}}]}
-                    :prep-tasks [["cljsbuild" "once"]]}}
+                    :prep-tasks [["cljsbuild" "once"]]}
+             :test {:dependencies [[org.clojure/test.check "1.1.1"]]}}
 
-  :aliases {"all-tests" ["with-profile" "cljs:1.8:1.9:1.10" "test"]})
+  :aliases {"all-tests" ["with-profile" "cljs:1.8:1.9:1.10:1.11" "test"]})

src/differ/diff.cljc

@@ -11,20 +11,24 @@
 
 
 (defn- map-alterations [state new-state]
-  (loop [[k & ks] (keys new-state)
+  (loop [[e & es] (seq new-state)
          diff (transient {})]
-    (if-not k
+    (if-not e
       (persistent! diff)
-      (let [old-val (get state k ::none)
+      (let [k (key e)
+            old-val (get state k ::none)
             new-val (alterations old-val (get new-state k))]
-        (cond (and (coll? old-val) (coll? new-val) (empty? new-val))
-              (recur ks diff)
+        (cond (and (coll? old-val) (coll? new-val) (empty? new-val)
+                   (not (record? old-val)) (not (record? new-val))
+                   (= (sequential? old-val) (sequential? new-val))
+                   (= (set? old-val) (set? new-val)))
+              (recur es diff)
 
               (= old-val new-val)
-              (recur ks diff)
+              (recur es diff)
 
               :else
-              (recur ks (assoc! diff k new-val)))))))
+              (recur es (assoc! diff k new-val)))))))
 
 (defn- vec-alterations [state new-state]
   (loop [idx 0
@@ -48,7 +52,10 @@
   The datastructure returned will be of the same type as the first argument
   passed. Works recursively on nested datastructures."
   [state new-state]
-  (cond (and (map? state) (map? new-state))
+  (cond (or (record? state) (record? new-state))
+        new-state
+
+        (and (map? state) (map? new-state))
         (map-alterations state new-state)
 
         (and (sequential? state) (sequential? new-state))
@@ -65,18 +72,19 @@
 
 (defn- map-removals [state new-state]
   (let [new-keys (set (keys new-state))]
-    (loop [[k & ks] (keys state)
+    (loop [[e & es] (seq state)
            diff (transient {})]
-      (if-not k
+      (if-not (some? e)
         (persistent! diff)
-        (if-not (contains? new-keys k)
-          (recur ks (assoc! diff k 0))
+        (let [k (key e)]
+         (if-not (contains? new-keys k)
+          (recur es (assoc! diff k 0))
           (let [old-val (get state k)
                 new-val (get new-state k)
                 rms (removals old-val new-val)]
             (if (and (coll? rms) (seq rms))
-              (recur ks (assoc! diff k rms))
-              (recur ks diff))))))))
+              (recur es (assoc! diff k rms))
+              (recur es diff)))))))))
 
 (defn- vec-removals [state new-state]
   (let [diff (- (count state) (count new-state))
@@ -93,7 +101,7 @@
             base))
         (let [new-rem (removals old-val new-val)]
           (if (or (and (coll? new-rem) (empty? new-rem))
-                  (= old-val new-rem))
+                  (and (= old-val new-rem) (not (or (sequential? old-val) (map? old-val) (set? old-val)))))
             (recur (inc idx) old-rest new-rest rem)
             (recur (inc idx) old-rest new-rest (conj! (conj! rem idx) new-rem))))))))
 
@@ -119,5 +127,11 @@
         (and (set? state) (set? new-state))
         (set/difference state new-state)
 
+        (record? state)
+        state
+
+        (coll? state)
+        (empty state)
+
         :else
-        (empty state)))
+        state))

src/differ/patch.cljc

@@ -10,13 +10,14 @@
 
 
 (defn- map-alterations [state diff]
-  (loop [[k & ks] (keys diff)
+  (loop [[e & es] (seq diff)
          result (transient state)]
-    (if-not k
+    (if-not e
       (with-meta (persistent! result) (meta state))
-      (let [old-val (get result k)
+      (let [k (key e)
+            old-val (get result k)
             diff-val (get diff k)]
-        (recur ks (assoc! result k (alterations old-val diff-val)))))))
+        (recur es (assoc! result k (alterations old-val diff-val)))))))
 
 (defn- vec-alterations [state diff]
   (loop [idx 0
@@ -40,7 +41,10 @@
 (defn alterations
   "Returns a new datastructure, containing the changes in the provided diff."
   [state diff]
-  (cond (and (map? state) (map? diff))
+  (cond (or (record? state) (record? diff))
+        diff
+
+        (and (map? state) (map? diff))
         (map-alterations state diff)
 
         (and (sequential? state) (sequential? diff))
@@ -60,20 +64,22 @@
 
 
 (defn- map-removals [state diff]
-  (loop [[k & ks] (keys diff)
+  (loop [[e & es] (seq diff)
          result (transient state)]
-    (if-not k
+    (if-not e
       (with-meta (persistent! result) (meta state))
-      (let [old-val (get result k)
+      (let [k (key e)
+            old-val (get result k)
             diff-val (get diff k)]
         (if (= 0 diff-val)
-          (recur ks (dissoc! result k))
-          (recur ks (assoc! result k (removals old-val diff-val))))))))
+          (recur es (dissoc! result k))
+          (recur es (assoc! result k (removals old-val diff-val))))))))
 
 (defn- vec-removals [state diff]
   (if-not (seq diff)
     state
-    (let [max-index (- (count state) (first diff))]
+    (let [removed-count (as-> (first diff) x (if (integer? x) x (count state)))
+          max-index (- (count state) removed-count)]
       (loop [index 0
              [old-val & old-rest :as old-coll] state
              [diff-index diff-val & diff-rest :as diff-coll] (rest diff)
@@ -91,7 +97,10 @@
   "Returns a new datastructure, not containing the elements in the
   provided diff."
   [state diff]
-  (cond (and (map? state) (map? diff))
+  (cond (record? state)
+        state
+
+        (and (map? state) (map? diff))
         (map-removals state diff)
 
         (and (sequential? state) (sequential? diff))

test/differ/diff_test.cljc

@@ -6,6 +6,7 @@
             #?(:clj [clojure.test :refer [is deftest testing]]
                :cljs [cljs.test :refer-macros [is deftest testing]])))
 
+(defrecord TestRecord [x])
 
 (let [state {:one 1
              :two {:three 2
@@ -40,7 +41,10 @@
 
   (deftest map-alterations
     (testing "alterations"
+      (is (= {false 0} (diff/alterations state (assoc state false 0))))
       (is (= {:one 2} (diff/alterations state (assoc state :one 2))))
+      (is (= {:vector {}} (diff/alterations state (assoc state :vector {}))))
+      (is (= {:set []} (diff/alterations state (assoc state :set []))))
       (is (= {:one 2, :seven 5} (diff/alterations state (assoc state :seven 5, :one 2)))))
 
     (testing "works with nesting"
@@ -80,7 +84,8 @@
       (is (= [1 [:+ 5]] (diff/alterations [1 []] [1 [5]])))
       (is (= [2 {:a 5}] (diff/alterations [1 2 {:a 4, :b 10}]
                                           [1 2 {:a 5, :b 10}])))
-      (is (= [] (diff/alterations [5 [1 2]] [5 [1 2]]))))
+      (is (= [] (diff/alterations [5 [1 2]] [5 [1 2]])))
+      (is (= [1 #{3}] (diff/alterations [5 #{1 2}] [5 #{3}]))))
 
     (testing "values can be added"
       (is (= [:+ 1] (diff/alterations [] [1])))
@@ -128,6 +133,7 @@
 
     (testing "return state when values are not collections"
       (is (= 1 (diff/removals 1 2)))
+      (is (= (->TestRecord 0) (diff/removals (->TestRecord 0) [:b])))
       (is (= true (diff/removals true false)))))
 
   (deftest map-removals
@@ -149,7 +155,8 @@
 
     (testing "works with nesting"
       (is (= [1 1 [1]] (diff/removals [1 [3 4 5] 6] [1 [3 5]])))
-      (is (= [0 1 {:a 0}] (diff/removals [1 {:a 2} 3] [1 {} 3])))))
+      (is (= [0 1 {:a 0}] (diff/removals [1 {:a 2} 3] [1 {} 3])))
+      (is (= [0 1 #{0}] (diff/removals [1 #{0} 3] [1 #{} 3])))))
 
   (deftest list-removals
     (testing "removals"

test/differ/gen_test.cljc

@@ -0,0 +1,109 @@
+(ns differ.gen-test
+  (:require [differ.core :as core]
+            [differ.patch :as patch]
+            clojure.pprint
+            [clojure.test :refer [deftest is testing]]
+            [clojure.test.check.clojure-test :refer [defspec]]
+            [clojure.test.check.generators :as gen]
+            [clojure.test.check.properties :as prop]))
+
+(def ^:private
+  key-gen
+  (gen/elements [0 1 :a :b "c" "d" true false nil]))
+
+(defrecord TestRecord [x])
+
+(def ^:private
+  simple-type-printable-equatable
+  "Like gen/simple-type-printable, but only generates objects that
+  can be equal to other objects (e.g., not a NaN) and protects against
+  `0` and `0.0` and `-0.0` being in the same map."
+  (gen/one-of
+   [gen/small-integer #?(:clj gen/size-bounded-bigint :cljs gen/large-integer)
+    gen/ratio gen/boolean gen/keyword gen/keyword-ns gen/symbol gen/symbol-ns gen/uuid
+    (gen/double* {:NaN? false, :infinite? false, :min 0.00000001, :max 9007199254740991})
+    (gen/double* {:NaN? false, :infinite? false, :max -0.00000001, :min -9007199254740991})
+    (gen/fmap ->TestRecord gen/small-integer)
+    gen/char-ascii
+    gen/string-ascii]))
+
+(def ^:private
+  diffable-value (gen/recursive-gen gen/container-type simple-type-printable-equatable))
+
+(def ^:private
+  coll-gen
+  (gen/one-of [(gen/tuple (gen/map key-gen diffable-value) (gen/map key-gen diffable-value))
+               (gen/tuple (gen/vector diffable-value)      (gen/vector diffable-value))
+               (gen/tuple (gen/set diffable-value)         (gen/set diffable-value))
+               (gen/tuple (gen/list diffable-value)        (gen/list diffable-value))]))
+
+;; Generative testing: round-trip through diff-patch
+(defspec diff-patch-list
+  100
+  (prop/for-all [[old new] coll-gen]
+                (let [diff (core/diff old new)]
+                  (= new (core/patch old diff)))))
+
+;; Examples that have failed before
+(deftest round-trip
+  (doseq [[old new]
+          [[{} {false 0}]
+           [{0 []} {0 #{}}]
+           [{:a []} {:a {}}]
+           ['(#{0}) '(#{})]
+           [{false [[]]} {false 0}]
+           [[{{} 0}] [{}]]
+           [[0 0 0 {0 0}] [0 0 0 {}]]
+           ['([[1N]]) '([[]])]
+           [(list (list (list 1N)))
+            (list (list (list ) #{} (list false 0.4788818359375)) [true] #{#{:+?/N92a}} \" "D" #{:Bb. -41627433298775N false} {})]
+           [(list 0 0 0 0 0 0 0 0 [0 0 0 0 0 0 0 0 0 0 0 0 0 [1 1 2]])
+            (list 0 0 0 0 0 0 0 0 [0 0 0 0 0 0 0 0 0 0 0 0 0 []])]
+           [(list [#differ.gen_test.TestRecord{:x 0}]) (list [0])]
+           [{0 #differ.gen_test.TestRecord{:x 0}} {0 {}}]]]
+    (testing (pr-str 'round-trip old '-> new)
+      (is (= new (core/patch old (core/diff old new)))
+          (pr-str 'round-trip 'failed 'given 'diff (core/diff old new))))))
+
+(defn- describe
+  [old new]
+  (let [diff (delay (core/diff old new))
+        intermediate (delay (patch/removals old @diff))
+        patched (delay (core/patch old @diff))
+        success? (delay (= new @patched))]
+    (println "----- original (old) -----")
+    (clojure.pprint/pprint old)
+    (println "----- diff [alterations removals]  -----")
+    (clojure.pprint/pprint @diff)
+    (println "----- intermedia (removals applied)  -----")
+    (clojure.pprint/pprint @intermediate)
+    (println "----- expected (new) -----")
+    (clojure.pprint/pprint new)
+    (println "----- patched -----")
+    (clojure.pprint/pprint @patched)
+    (println "-----" (if @success? "success" "FAILED") "-----")
+    (if @success? :success :failed)))
+
+(defn- bi-describe
+  [old new]
+  (println "===================== old -> new ========================")
+  (describe old new)
+  (println "===================== new -> old ========================")
+  (describe new old))
+
+(comment
+  (bi-describe {} {false 0})
+  (bi-describe {0 []} {0 #{}})
+  (bi-describe {:a []} {:a {}})
+  (bi-describe '(#{0}) '(#{}))
+  (bi-describe {false [[]]} {false 0})
+  (bi-describe [0 0 0 {0 0}] [0 0 0 {}])
+  (bi-describe {"c" {}} {"c" {{Double/NaN 0} 0}})
+  (bi-describe '([[1N]]) '([[]]))
+  (bi-describe (list (list (list 1N))) (list (list (list ) #{} (list false 0.4788818359375)) [true] #{#{:+?/N92a}} \" "D" #{:Bb. -41627433298775N false} {}))
+  (bi-describe (list 0 0 0 0 0 0 0 0 [0 0 0 0 0 0 0 0 0 0 0 0 0 [1 1 2]])
+               (list 0 0 0 0 0 0 0 0 [0 0 0 0 0 0 0 0 0 0 0 0 0 []]))
+  (bi-describe (list [#differ.gen_test.TestRecord{:x 0}]) (list [0]))
+  (bi-describe (list 0 [#differ.gen_test.TestRecord{:x 0}]) (list 0 [#differ.gen_test.TestRecord{:x -1}]))
+  (bi-describe {0 #differ.gen_test.TestRecord{:x 0}} {0 {}})
+  :-)

test/differ/patch_test.cljc

@@ -26,6 +26,8 @@
     (testing "maps"
       (is (= (assoc state :one 2)
              (patch/alterations state {:one 2})))
+      (is (= (assoc state false 2)
+             (patch/alterations state {false 2})))
       (is (= (-> state
                  (assoc :seven 7)
                  (assoc-in [:two :three] {:booya "boom"}))