From a0d193046acfab620323fcadb03da9fdaac8c7de Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Wed, 19 Jul 2017 18:29:03 -0600 Subject: [PATCH 1/8] Move to 0.1.2-SNAPSHOT --- project.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project.clj b/project.clj index 832027b2..af4ac87d 100644 --- a/project.clj +++ b/project.clj @@ -1,4 +1,4 @@ -(defproject expound "0.1.1" +(defproject expound "0.1.2-SNAPSHOT" :description "Human-optimized error messages for clojure.spec" :url "https://github.com/bhb/expound" :license {:name "Eclipse Public License" From 64a949f31cd408c7bd6e46ceb1f587b47f0a6396 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Wed, 19 Jul 2017 18:29:31 -0600 Subject: [PATCH 2/8] Refactors to add support for s/assert --- README.md | 29 ++++++++++++ docs/development.md | 2 +- project.clj | 2 +- resources/public/index.html | 8 ++++ src/expound/alpha.cljc | 87 ++++++++++++++++++++++-------------- test/expound/alpha_test.cljc | 48 +++++++++++++++++++- 6 files changed, 138 insertions(+), 38 deletions(-) create mode 100644 resources/public/index.html diff --git a/README.md b/README.md index a433fe8c..74b13bea 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,8 @@ Expound is in alpha while `clojure.spec` is in alpha. [![Clojars Project](https://img.shields.io/clojars/v/expound.svg)](https://clojars.org/expound) +### `expound` + Replace calls to `clojure.spec.alpha/explain` with `expound.alpha/expound` and to `clojure.spec.alpha/explain-str` with `expound.alpha/expound-str`. ```clojure @@ -71,6 +73,33 @@ Replace calls to `clojure.spec.alpha/explain` with `expound.alpha/expound` and t ;; Detected 1 error ``` +### `*explain-out*` + +To use other Spec functions, you can set `clojure.spec.alpha/*explain-out*` (or `cljs.spec.alpha/*explain-out*`. + +(Setting `*explain-out*` does not work correctly in Clojurescript versions prior to `1.9.562` due to differences in `explain-data`) + +```clojure +;; Temporarily set the dynamic var +(require '[clojure.spec.alpha :as s]) +;; for clojurescript: +;; (require '[cljs.spec.alpha :as s]) +(require '[expound.alpha :as expound]) + +(s/def :example.place/city string?) + +;; Use s/assert +(s/check-asserts true) ; enable asserts + +;; Set printer in the scope of 'binding' +(binding [s/*explain-out* printer] + (s/assert :example.place/city 1)) + +;; Or set it globally +(set! s/*explain-out* printer) +(s/assert :example.place/city 1) +``` + ## Prior Art * Error messages in [Elm](http://elm-lang.org/), in particular the [error messages catalog](https://github.com/elm-lang/error-message-catalog) diff --git a/docs/development.md b/docs/development.md index e82f9767..9e1b0fb7 100644 --- a/docs/development.md +++ b/docs/development.md @@ -4,7 +4,7 @@ ``` lein with-profile +test-web,+cljs-repl repl -```` +``` ``` M-x cider-connect diff --git a/project.clj b/project.clj index af4ac87d..122abf46 100644 --- a/project.clj +++ b/project.clj @@ -4,7 +4,7 @@ :license {:name "Eclipse Public License" :url "http://www.eclipse.org/legal/epl-v10.html"} :dependencies [[org.clojure/clojure "1.9.0-alpha17" :scope "provided"] - [org.clojure/clojurescript "1.9.542" :scope "provided"] + [org.clojure/clojurescript "1.9.562" :scope "provided"] [org.clojure/spec.alpha "0.1.123" :scope "provided"]] :plugins [[com.jakemccrary/lein-test-refresh "0.20.0"] diff --git a/resources/public/index.html b/resources/public/index.html new file mode 100644 index 00000000..4b0499ce --- /dev/null +++ b/resources/public/index.html @@ -0,0 +1,8 @@ + + + + + +

REPL should be connected...

+ + diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 84012bdf..956b6a85 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -228,7 +228,7 @@ :cljs (implements? cljs.core.INamed x))) (defn pr-pred [pred] - (if (named? pred) + (if (or (symbol? pred) (named? pred)) (name pred) (pr-str pred))) @@ -255,8 +255,11 @@ should have additional elements. The next element is named `%s` and satisfies (defn missing-key [form] #?(:cljs (let [[contains _arg key-keyword] form] - (s/assert #{'contains?} contains) - key-keyword) + (if (contains? #{'cljs.core/contains? 'contains?} contains) + key-keyword + (let [[fn _ [contains _arg key-keyword] & rst] form] + (s/assert #{'cljs.core/contains? 'contains?} contains) + key-keyword))) ;; FIXME - this duplicates the structure of how ;; spec builds the 'contains?' function. Extract this into spec ;; and use conform instead of this ad-hoc validation. @@ -276,12 +279,15 @@ should have additional elements. The next element is named `%s` and satisfies (def section-label (partial label section-size)) (defn relevant-specs [problems] - (format - "%s + (let [sp-str (specs-str problems)] + (if (string/blank? sp-str) + "" + (format + "%s %s" - (section-label "Relevant specs") - (specs-str problems))) + (section-label "Relevant specs") + sp-str)))) (defn multi-spec-parts [spec] (let [[_multi-spec mm retag] (s/form spec)] @@ -298,7 +304,11 @@ should have additional elements. The next element is named `%s` and satisfies (let [pred (:pred problem)] (and (list? pred) (map? (:val problem)) - (= 'contains? (first pred)))) + (or (= 'contains? (first pred)) + (let [[fn _ [contains _] & rst] pred] + (and + (= 'cljs.core/fn fn) + (= 'cljs.core/contains? contains)))))) :clj (let [pred (:pred problem)] (and (seq? pred) @@ -508,34 +518,43 @@ should satisfy ;;;;;; public ;;;;;; -(defn expound-str - "Given a spec and a value that fails to conform, returns a human-readable explanation as a string." - [spec form] - (let [problems (::s/problems (s/explain-data spec form)) - _ (doseq [problem problems] - (s/assert (s/nilable #{"Insufficient input" "Extra input" "no method"}) (:reason problem))) - leaf-problems (leaf-problems (map (partial adjust-in form) (::s/problems (s/explain-data spec form)))) - - _ (assert (every? :in1 leaf-problems) leaf-problems) - ;; We attempt to sort the problems by path, but it's not feasible to sort in - ;; all cases, since paths could contain arbitrary user-defined data structures. - ;; If there is an error, we just give up on sorting. - grouped-problems (safe-sort-by first compare-paths - (path+problem-type->problems leaf-problems))] - (if (empty? problems) - "Success!\n" - (let [problems-str (string/join "\n\n" (for [[[in1 type] problems] grouped-problems] - (problem-group-str type form in1 problems)))] - (no-trailing-whitespace - (format - "%s +(defn printer [explain-data] + ;; TODO - use namespace destructuring + (print + (if-not explain-data + "Success!\n" + (let [problems (::s/problems explain-data) + form (::s/value explain-data) + spec (::s/spec explain-data) + _ (doseq [problem problems] + (s/assert (s/nilable #{"Insufficient input" "Extra input" "no method"}) (:reason problem))) + leaf-problems (leaf-problems (map (partial adjust-in form) problems)) + _ (assert (every? :in1 leaf-problems) leaf-problems) + ;; We attempt to sort the problems by path, but it's not feasible to sort in + ;; all cases, since paths could contain arbitrary user-defined data structures. + ;; If there is an error, we just give up on sorting. + grouped-problems (safe-sort-by first compare-paths + (path+problem-type->problems leaf-problems))] + (let [problems-str (string/join "\n\n" (for [[[in1 type] problems] grouped-problems] + (problem-group-str type form in1 problems)))] + (no-trailing-whitespace + (format + ;; TODO - add a newline here to make specs look nice + "%s %s Detected %s %s" - problems-str - (section-label) - (count grouped-problems) - (if (= 1 (count grouped-problems)) "error" "errors"))))))) + problems-str + (section-label) + (count grouped-problems) + (if (= 1 (count grouped-problems)) "error" "errors")))))))) + +(defn expound-str + "Given a spec and a value that fails to conform, returns a human-readable explanation as a string." + [spec form] + (binding [s/*explain-out* printer] + (s/explain-str spec form))) (defn expound [spec form] - (println (expound-str spec form))) + (binding [s/*explain-out* printer] + (s/explain spec form))) diff --git a/test/expound/alpha_test.cljc b/test/expound/alpha_test.cljc index fc59a9ac..1cefbb31 100644 --- a/test/expound/alpha_test.cljc +++ b/test/expound/alpha_test.cljc @@ -247,7 +247,7 @@ should satisfy ------------------------- Detected 1 error" - #?(:cljs "(pos? (count %))" + #?(:cljs "(cljs.core/fn [%] (cljs.core/pos? (cljs.core/count %)))" :clj "(clojure.core/fn [%] (clojure.core/pos? (clojure.core/count %)))") ) (expound/expound-str :and-spec/name "")))) @@ -292,7 +292,7 @@ should satisfy ------------------------- Detected 2 errors" - #?(:cljs "(pos? (count %))" + #?(:cljs "(cljs.core/fn [%] (cljs.core/pos? (cljs.core/count %)))" :clj "(clojure.core/fn [%] (clojure.core/pos? (clojure.core/count %)))") ) (expound/expound-str :and-spec/names ["bob" "sally" "" 1]))))) @@ -571,6 +571,7 @@ Detected 1 error" (s/def :cat-wrapped-in-or-spec/kv-or-string (s/or :map (s/keys :req [:cat-wrapped-in-or-spec/type]) :kv :cat-wrapped-in-or-spec/kv)) + (deftest cat-wrapped-in-or-spec ;; FIXME - make multiple types of specs on the same value display as single error (is (= (pf "-- Spec failed -------------------- @@ -780,3 +781,46 @@ Detected 1 error") k gen/simple-type-printable] (is (= -1 (expound/compare-paths [(expound/->KeyPathSegment k)] [k]))) (is (= 1 (expound/compare-paths [k] [(expound/->KeyPathSegment k)]))))) + +(s/def :test-assert/name string?) +(deftest test-assert + (testing "assertion passes" + (is (= "hello" + (s/assert :test-assert/name "hello")))) + (testing "assertion fails" + #?(:cljs + (try + (binding [s/*explain-out* expound/printer] + (s/assert :test-assert/name :hello)) + (catch :default e + (is (= "Spec assertion failed\n-- Spec failed -------------------- + + :hello + +should satisfy + + string? + + + +------------------------- +Detected 1 error" + (.-message e))))) + :clj + (try + (binding [s/*explain-out* expound/printer] + (s/assert :test-assert/name :hello)) + (catch Exception e + (is (= "Spec assertion failed\n-- Spec failed -------------------- + + :hello + +should satisfy + + string? + + + +------------------------- +Detected 1 error" + (:cause (Throwable->map e))))))))) From 2759f52af87ee9caab4829c2b81a8112b50175a9 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Wed, 19 Jul 2017 18:48:15 -0600 Subject: [PATCH 3/8] Refactor to preserve 'expound' and 'expound-str' on 1.9.542 --- project.clj | 3 +++ src/expound/alpha.cljc | 24 ++++++++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/project.clj b/project.clj index 122abf46..e83a46ce 100644 --- a/project.clj +++ b/project.clj @@ -4,6 +4,9 @@ :license {:name "Eclipse Public License" :url "http://www.eclipse.org/legal/epl-v10.html"} :dependencies [[org.clojure/clojure "1.9.0-alpha17" :scope "provided"] + ;; expound launched with this version + ;; and only supported expound and expound-str + ;;[org.clojure/clojurescript "1.9.542" :scope "provided"] [org.clojure/clojurescript "1.9.562" :scope "provided"] [org.clojure/spec.alpha "0.1.123" :scope "provided"]] diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 956b6a85..23842cfe 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -519,13 +519,11 @@ should satisfy ;;;;;; public ;;;;;; (defn printer [explain-data] - ;; TODO - use namespace destructuring (print (if-not explain-data "Success!\n" - (let [problems (::s/problems explain-data) - form (::s/value explain-data) - spec (::s/spec explain-data) + (let [{:keys [::s/problems ::s/value]} explain-data + form value _ (doseq [problem problems] (s/assert (s/nilable #{"Insufficient input" "Extra input" "no method"}) (:reason problem))) leaf-problems (leaf-problems (map (partial adjust-in form) problems)) @@ -549,12 +547,18 @@ Detected %s %s" (count grouped-problems) (if (= 1 (count grouped-problems)) "error" "errors")))))))) +(defn expound [spec form] + ;; expound was initially released with support + ;; for CLJS 1.9.542 which did not include + ;; the value in the explain data, so we patch it + ;; in to avoid breaking back compat (at least for now) + (let [explain-data (s/explain-data spec form)] + (printer (if explain-data + (assoc explain-data + ::s/value form) + nil)))) + (defn expound-str "Given a spec and a value that fails to conform, returns a human-readable explanation as a string." [spec form] - (binding [s/*explain-out* printer] - (s/explain-str spec form))) - -(defn expound [spec form] - (binding [s/*explain-out* printer] - (s/explain spec form))) + (with-out-str (expound spec form))) From 1ba966d65a035597b843dfd986484d0012fea9cf Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Wed, 19 Jul 2017 19:16:33 -0600 Subject: [PATCH 4/8] Adds test for explain-str --- test/expound/alpha_test.cljc | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/expound/alpha_test.cljc b/test/expound/alpha_test.cljc index 1cefbb31..bc111a4a 100644 --- a/test/expound/alpha_test.cljc +++ b/test/expound/alpha_test.cljc @@ -824,3 +824,23 @@ should satisfy ------------------------- Detected 1 error" (:cause (Throwable->map e))))))))) + +(s/def :test-explain-str/name string?) +(deftest test-explain-str + (is (= (pf "-- Spec failed -------------------- + + :hello + +should satisfy + + string? + +-- Relevant specs ------- + +:test-assert/name: + pf.core/string? + +------------------------- +Detected 1 error") + (binding [s/*explain-out* expound/printer] + (s/explain-str :test-assert/name :hello))))) From ee7bac959ec46c2ac86540591ed4a9d29b55758f Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Thu, 20 Jul 2017 20:49:54 -0600 Subject: [PATCH 5/8] Adds basic support for instrumentation --- src/expound/alpha.cljc | 47 ++++++--- test/expound/alpha_test.cljc | 199 +++++++++++++++++++++++------------ 2 files changed, 164 insertions(+), 82 deletions(-) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 23842cfe..a41c7a9a 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -20,11 +20,11 @@ (def section-size 25) (def indent-level 2) -(def headers {:problem/missing-key "Spec failed" - :problem/not-in-set "Spec failed" - :problem/missing-spec "Missing spec" - :problem/regex-failure "Syntax error" - :problem/unknown "Spec failed"}) +(def headers {:problem/missing-key "Spec failed" + :problem/not-in-set "Spec failed" + :problem/missing-spec "Missing spec" + :problem/regex-failure "Syntax error" + :problem/unknown "Spec failed"}) #?(:cljs (defn format [fmt & args] @@ -85,6 +85,7 @@ (cond (list? form) (outer path (apply list (map-indexed (fn [idx x] (inner (conj path idx) x)) form))) (vector? form) (outer path (mapv-indexed (fn [idx x] (inner (conj path idx) x)) form)) + (seq? form) (outer path (doall (map-indexed (fn [idx x] (inner (conj path idx) x)) form))) (record? form) (outer path form) (map? form) (outer path (reduce (fn [m [idx [k v]]] (conj m @@ -171,7 +172,7 @@ (int? k) (recur (nth form k) rst)))) -;; TODO - perhaps a more useful API would be an API on 'problems'? +;; FIXME - perhaps a more useful API would be an API on 'problems'? ;; - group problems ;; - print out data structure given problem ;; - categorize problem @@ -518,12 +519,25 @@ should satisfy ;;;;;; public ;;;;;; +(defn instrumentation-info [failure caller] + ;; As of version 1.9.562, Clojurescript does + ;; not include failure or caller info, so + ;; if these are null, print a placeholder + (if (= :instrument failure) + (format "%s:%s +\n" + (:file caller "") + (:line caller "")) + "")) + (defn printer [explain-data] (print (if-not explain-data "Success!\n" - (let [{:keys [::s/problems ::s/value]} explain-data - form value + (let [{:keys [::s/problems ::s/value ::s/args ::s/failure :clojure.spec.test.alpha/caller]} explain-data + form (if (= :instrument failure) + args + value) _ (doseq [problem problems] (s/assert (s/nilable #{"Insufficient input" "Extra input" "no method"}) (:reason problem))) leaf-problems (leaf-problems (map (partial adjust-in form) problems)) @@ -534,18 +548,21 @@ should satisfy grouped-problems (safe-sort-by first compare-paths (path+problem-type->problems leaf-problems))] (let [problems-str (string/join "\n\n" (for [[[in1 type] problems] grouped-problems] + #_"foobar" (problem-group-str type form in1 problems)))] (no-trailing-whitespace - (format - ;; TODO - add a newline here to make specs look nice - "%s + (str + (instrumentation-info failure caller) + (format + ;; TODO - add a newline here to make specs look nice + "%s %s Detected %s %s" - problems-str - (section-label) - (count grouped-problems) - (if (= 1 (count grouped-problems)) "error" "errors")))))))) + problems-str + (section-label) + (count grouped-problems) + (if (= 1 (count grouped-problems)) "error" "errors"))))))))) (defn expound [spec form] ;; expound was initially released with support diff --git a/test/expound/alpha_test.cljc b/test/expound/alpha_test.cljc index bc111a4a..4153626b 100644 --- a/test/expound/alpha_test.cljc +++ b/test/expound/alpha_test.cljc @@ -3,6 +3,7 @@ [com.gfredericks.test.chuck.clojure-test :refer [checking]] [clojure.test.check.generators :as gen] [clojure.spec.alpha :as s] + [clojure.spec.test.alpha :as st] [expound.alpha :as expound] [expound.test-utils :as test-utils] [clojure.string :as string])) @@ -15,10 +16,11 @@ "Fixes platform-specific namespaces and also formats using printf syntax" [s & args] (apply expound/format - #?(:cljs (string/replace s "pf." "cljs.") - :clj (string/replace s "pf." "clojure.")) - args)) + #?(:cljs (string/replace s "pf." "cljs.") + :clj (string/replace s "pf." "clojure.")) + args)) +(defn get-args [& args] args) (deftest highlighted-form (testing "atomic value" (is (= "\"Fred\"\n^^^^^^" @@ -46,7 +48,12 @@ :c "cccccccd" :d "dddddddd" :e "eeeeeeee"}} - [:letters]))))) + [:letters])))) + (testing "args to function" + (is (= "(1 ... ...)\n ^" + (expound/highlighted-form + (get-args 1 2 3) + [0]))))) (s/def :simple-type-based-spec/str string?) @@ -57,7 +64,7 @@ (testing "invalid value" (is (= - (pf "-- Spec failed -------------------- + (pf "-- Spec failed -------------------- 1 @@ -72,7 +79,7 @@ should satisfy ------------------------- Detected 1 error") - (expound/expound-str :simple-type-based-spec/str 1))))) + (expound/expound-str :simple-type-based-spec/str 1))))) (s/def :set-based-spec/tag #{:foo :bar}) (s/def :set-based-spec/nilable-tag (s/nilable :set-based-spec/tag)) @@ -130,7 +137,7 @@ Detected 2 errors") (deftest nested-type-based-spec (is (= - (pf "-- Spec failed -------------------- + (pf "-- Spec failed -------------------- [... ... 33] ^^ @@ -148,14 +155,14 @@ should satisfy ------------------------- Detected 1 error") - (expound/expound-str :nested-type-based-spec/strs ["one" "two" 33])))) + (expound/expound-str :nested-type-based-spec/strs ["one" "two" 33])))) (s/def :nested-type-based-spec-special-summary-string/int int?) (s/def :nested-type-based-spec-special-summary-string/ints (s/coll-of :nested-type-based-spec-special-summary-string/int)) (deftest nested-type-based-spec-special-summary-string (is (= - (pf "-- Spec failed -------------------- + (pf "-- Spec failed -------------------- [... ... \"...\"] ^^^^^ @@ -174,7 +181,7 @@ should satisfy ------------------------- Detected 1 error") - (expound/expound-str :nested-type-based-spec-special-summary-string/ints [1 2 "..."])))) + (expound/expound-str :nested-type-based-spec-special-summary-string/ints [1 2 "..."])))) (s/def :or-spec/str-or-int (s/or :int int? :str string?)) (s/def :or-spec/vals (s/coll-of :or-spec/str-or-int)) @@ -254,7 +261,7 @@ Detected 1 error" (testing "shows both failures in order" (is (= - (pf "-- Spec failed -------------------- + (pf "-- Spec failed -------------------- [... ... \"\" ...] ^^ @@ -295,14 +302,14 @@ Detected 2 errors" #?(:cljs "(cljs.core/fn [%] (cljs.core/pos? (cljs.core/count %)))" :clj "(clojure.core/fn [%] (clojure.core/pos? (clojure.core/count %)))") ) - (expound/expound-str :and-spec/names ["bob" "sally" "" 1]))))) + (expound/expound-str :and-spec/names ["bob" "sally" "" 1]))))) (s/def :coll-of-spec/big-int-coll (s/coll-of int? :min-count 10)) (deftest coll-of-spec (testing "min count" (is (= - (pf "-- Spec failed -------------------- + (pf "-- Spec failed -------------------- [] @@ -320,7 +327,7 @@ Detected 1 error" #?(:cljs "9007199254740991" :clj "Integer/MAX_VALUE") ) - (expound/expound-str :coll-of-spec/big-int-coll []))))) + (expound/expound-str :coll-of-spec/big-int-coll []))))) (s/def :cat-spec/kw (s/cat :k keyword? :v any?)) (deftest cat-spec @@ -433,7 +440,7 @@ Detected 1 error" (deftest multi-spec (testing "missing dispatch key" (is (= - (pf "-- Missing spec ------------------- + (pf "-- Missing spec ------------------- Cannot find spec for @@ -453,10 +460,10 @@ Cannot find spec for ------------------------- Detected 1 error") - (expound/expound-str :multi-spec/el {})))) + (expound/expound-str :multi-spec/el {})))) (testing "invalid dispatch value" (is (= - (pf "-- Missing spec ------------------- + (pf "-- Missing spec ------------------- Cannot find spec for @@ -476,11 +483,11 @@ Cannot find spec for ------------------------- Detected 1 error") - (expound/expound-str :multi-spec/el {:multi-spec/el-type :image})))) + (expound/expound-str :multi-spec/el {:multi-spec/el-type :image})))) (testing "valid dispatch value, but other error" (is (= - (pf "-- Spec failed -------------------- + (pf "-- Spec failed -------------------- {:multi-spec/el-type :text} @@ -495,7 +502,7 @@ should contain keys: `:multi-spec/value` ------------------------- Detected 1 error") - (expound/expound-str :multi-spec/el {:multi-spec/el-type :text}))))) + (expound/expound-str :multi-spec/el {:multi-spec/el-type :text}))))) (s/def :recursive-spec/tag #{:text :group}) (s/def :recursive-spec/on-tap (s/coll-of map? :kind vector?)) @@ -539,7 +546,7 @@ Detected 1 error" [:recursive-spec/tag] :opt-un [:recursive-spec/props :recursive-spec/children])" - :clj ":recursive-spec/on-tap: + :clj ":recursive-spec/on-tap: (clojure.spec.alpha/coll-of clojure.core/map? :kind @@ -715,58 +722,58 @@ Detected 1 error") (deftest generated-simple-spec (checking - "simple spec" - 30 - [simple-spec simple-spec-gen + "simple spec" + 30 + [simple-spec simple-spec-gen :let [sp-form (s/form simple-spec)] form gen/any-printable] - (expound/expound-str simple-spec form))) + (expound/expound-str simple-spec form))) #_(deftest generated-coll-of-specs (checking - "'coll-of' spec" - 30 - [simple-spec simple-spec-gen - every-args (s/gen :specs/every-args) - :let [spec (apply-coll-of simple-spec every-args)] - :let [sp-form (s/form spec)] - form gen/any-printable] - (expound/expound-str spec form))) + "'coll-of' spec" + 30 + [simple-spec simple-spec-gen + every-args (s/gen :specs/every-args) + :let [spec (apply-coll-of simple-spec every-args)] + :let [sp-form (s/form spec)] + form gen/any-printable] + (expound/expound-str spec form))) #_(deftest generated-and-specs (checking - "'and' spec" - 30 - [simple-spec1 simple-spec-gen - simple-spec2 simple-spec-gen - :let [spec (s/and simple-spec1 simple-spec2)] - :let [sp-form (s/form spec)] - form gen/any-printable] - (expound/expound-str spec form))) + "'and' spec" + 30 + [simple-spec1 simple-spec-gen + simple-spec2 simple-spec-gen + :let [spec (s/and simple-spec1 simple-spec2)] + :let [sp-form (s/form spec)] + form gen/any-printable] + (expound/expound-str spec form))) #_(deftest generated-or-specs (checking - "'or' spec" - 30 - [simple-spec1 simple-spec-gen - simple-spec2 simple-spec-gen - :let [spec (s/or :or1 simple-spec1 :or2 simple-spec2)] - :let [sp-form (s/form spec)] - form gen/any-printable] - (expound/expound-str spec form))) + "'or' spec" + 30 + [simple-spec1 simple-spec-gen + simple-spec2 simple-spec-gen + :let [spec (s/or :or1 simple-spec1 :or2 simple-spec2)] + :let [sp-form (s/form spec)] + form gen/any-printable] + (expound/expound-str spec form))) ;; TODO - get these two tests running! #_(deftest generated-map-of-specs (checking - "'map-of' spec" - 25 - [simple-spec1 simple-spec-gen - simple-spec2 simple-spec-gen - every-args (s/gen :specs/every-args) - :let [spec (apply-map-of simple-spec1 simple-spec2 every-args) - sp-form (s/form spec)] - form any-printable-wo-nan] - (expound/expound-str spec form))) + "'map-of' spec" + 25 + [simple-spec1 simple-spec-gen + simple-spec2 simple-spec-gen + every-args (s/gen :specs/every-args) + :let [spec (apply-map-of simple-spec1 simple-spec2 every-args) + sp-form (s/form spec)] + form any-printable-wo-nan] + (expound/expound-str spec form))) ;; TODO - keys ;; TODO - cat + alt, + ? * @@ -774,19 +781,19 @@ Detected 1 error") ;; TODO - test coll-of that is a set . can i should a bad element of a set? #_(deftest compare-paths-test - (checking - "path to a key comes before a path to a value" - 10 - [m (gen/map gen/simple-type-printable gen/simple-type-printable) - k gen/simple-type-printable] - (is (= -1 (expound/compare-paths [(expound/->KeyPathSegment k)] [k]))) - (is (= 1 (expound/compare-paths [k] [(expound/->KeyPathSegment k)]))))) + (checking + "path to a key comes before a path to a value" + 10 + [m (gen/map gen/simple-type-printable gen/simple-type-printable) + k gen/simple-type-printable] + (is (= -1 (expound/compare-paths [(expound/->KeyPathSegment k)] [k]))) + (is (= 1 (expound/compare-paths [k] [(expound/->KeyPathSegment k)]))))) (s/def :test-assert/name string?) (deftest test-assert (testing "assertion passes" - (is (= "hello" - (s/assert :test-assert/name "hello")))) + (is (= "hello" + (s/assert :test-assert/name "hello")))) (testing "assertion fails" #?(:cljs (try @@ -823,6 +830,7 @@ should satisfy ------------------------- Detected 1 error" + ;; FIXME - move assertion out of catch, similar to instrument tests (:cause (Throwable->map e))))))))) (s/def :test-explain-str/name string?) @@ -844,3 +852,60 @@ should satisfy Detected 1 error") (binding [s/*explain-out* expound/printer] (s/explain-str :test-assert/name :hello))))) + + +(s/def :test-instrument/name string?) +(s/fdef test-instrument-adder + :args (s/cat :x int? :y int?)) +(defn test-instrument-adder [x y] + (+ x y)) + +(defn no-linum [s] + (string/replace s #".cljc:\d+" ".cljc:LINUM")) + +(deftest test-instrument + (st/instrument `test-instrument-adder) + #?(:cljs (is (= + "Call to #'expound.alpha-test/test-instrument-adder did not conform to spec: +: + +-- Spec failed -------------------- + + (\"\" ...) + ^^ + +should satisfy + + int? + + + +------------------------- +Detected 1 error" + (.-message (try + (binding [s/*explain-out* expound/printer] + (test-instrument-adder "" :x)) + (catch :default e e))))) + :clj + (is (= "Call to #'expound.alpha-test/test-instrument-adder did not conform to spec: +alpha_test.cljc:LINUM + +-- Spec failed -------------------- + + (\"\" ...) + ^^ + +should satisfy + + int? + + + +------------------------- +Detected 1 error" + (no-linum + (:cause + (Throwable->map (try + (binding [s/*explain-out* expound/printer] + (test-instrument-adder "" :x)) + (catch Exception e e))))))))) From be1b0addc2ab8add603d7c9261c41423bc455c4d Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Thu, 20 Jul 2017 21:00:33 -0600 Subject: [PATCH 6/8] Updates README --- README.md | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 74b13bea..df226dbc 100644 --- a/README.md +++ b/README.md @@ -75,9 +75,9 @@ Replace calls to `clojure.spec.alpha/explain` with `expound.alpha/expound` and t ### `*explain-out*` -To use other Spec functions, you can set `clojure.spec.alpha/*explain-out*` (or `cljs.spec.alpha/*explain-out*`. +To use other Spec functions, you can set `clojure.spec.alpha/*explain-out*` (or `cljs.spec.alpha/*explain-out*` for ClojureScript). -(Setting `*explain-out*` does not work correctly in Clojurescript versions prior to `1.9.562` due to differences in `explain-data`) +(Setting `*explain-out*` does not work correctly in ClojureScript versions prior to `1.9.562` due to differences in `explain-data`) ```clojure ;; Temporarily set the dynamic var @@ -87,8 +87,9 @@ To use other Spec functions, you can set `clojure.spec.alpha/*explain-out*` (or (require '[expound.alpha :as expound]) (s/def :example.place/city string?) +(s/def :example.place/state string?) -;; Use s/assert +;; Use `assert` (s/check-asserts true) ; enable asserts ;; Set printer in the scope of 'binding' @@ -96,8 +97,22 @@ To use other Spec functions, you can set `clojure.spec.alpha/*explain-out*` (or (s/assert :example.place/city 1)) ;; Or set it globally -(set! s/*explain-out* printer) +(set! s/*explain-out* expound/printer) (s/assert :example.place/city 1) + +;; Use `instrument` +(require '[clojure.spec.test.alpha :as st]) + +(s/fdef pr-loc :args (s/cat :city :example.place/city + :state :example.place/state)) +(defn pr-loc [city state] + (str city ", " state)) + +(st/instrument `pr-loc) +(pr-loc "denver" :CO) + +;; You can use `explain` without converting to expound +(s/explain :example.place/city 123) ``` ## Prior Art From 908715b33a28ecdac6607a9a6055338e2fe287e7 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sat, 22 Jul 2017 14:23:27 -0600 Subject: [PATCH 7/8] Adds some specs to dogfood instrumentation. Added orchestra --- dev/user.clj | 11 +++++++ project.clj | 3 +- src/expound/alpha.cljc | 60 ++++++++++++++++++++++++++---------- test/expound/alpha_test.cljc | 4 ++- test/expound/test_utils.cljc | 13 ++++++++ 5 files changed, 73 insertions(+), 18 deletions(-) create mode 100644 dev/user.clj diff --git a/dev/user.clj b/dev/user.clj new file mode 100644 index 00000000..68c28cc4 --- /dev/null +++ b/dev/user.clj @@ -0,0 +1,11 @@ +(ns user + (:require [clojure.spec.alpha :as s] + [orchestra.spec.test :as st] + [expound.alpha :as expound])) + +(defn setup [] + (set! s/*explain-out* expound/printer) + (st/instrument)) + +(comment + (setup)) diff --git a/project.clj b/project.clj index e83a46ce..682a6a7b 100644 --- a/project.clj +++ b/project.clj @@ -74,7 +74,8 @@ } :profiles {:dev {:dependencies [[binaryage/devtools "0.9.2"] [figwheel-sidecar "0.5.10"] - [com.cemerick/piggieback "0.2.1"]] + [com.cemerick/piggieback "0.2.1"] + [orchestra "2017.07.04-1"]] ;; need to add dev source path here to get user.clj loaded :source-paths ["src" "dev"] ;; for CIDER diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index a41c7a9a..ae492a3a 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -13,6 +13,19 @@ ;;;;;; specs ;;;;;; (s/def ::singleton (s/coll-of any? :count 1)) +(s/def ::path vector?) + +;;;;;; types ;;;;;; + +(defrecord KeyPathSegment [key]) + +(defrecord KeyValuePathSegment [idx]) + +(defn kps? [x] + (instance? KeyPathSegment x)) + +(defn kvps? [x] + (instance? KeyValuePathSegment x)) ;;;;;; private ;;;;;; @@ -31,11 +44,17 @@ (apply goog.string/format fmt args)) :clj (def format clojure.core/format)) +(s/fdef pprint-str + :args (s/cat :x any?) + :ret string?) (defn pprint-str "Returns the pretty-printed string" [x] (pprint/write x :stream nil)) +(s/fdef no-trailing-whitespace + :args (s/cat :s string?) + :ret string?) (defn no-trailing-whitespace "Given an potentially multi-line string, returns that string with all trailing whitespace removed." @@ -45,6 +64,12 @@ (map string/trimr) (string/join "\n"))) +(s/fdef indent + :args (s/cat + :first-line-indent-level (s/? nat-int?) + :indent-level (s/? nat-int?) + :s string?) + :ret string?) (defn indent "Given an potentially multi-line string, returns that string indented by 'indent-level' spaces. Optionally, can indent first line and other lines @@ -59,6 +84,11 @@ (into [(str (apply str (repeat first-line-indent " ")) line)] (map #(str (apply str (repeat rest-lines-indent " ")) %) lines)))))) +(s/fdef prefix-path? + :args (s/cat + :partial-path ::path + :partial-path ::path) + :ret boolean?) (defn prefix-path? "True if partial-path is a prefix of full-path." [partial-path full-path] @@ -66,16 +96,6 @@ (= partial-path (subvec full-path 0 (count partial-path))))) -(defrecord KeyPathSegment [key]) - -(defrecord KeyValuePathSegment [idx]) - -(defn kps? [x] - (instance? KeyPathSegment x)) - -(defn kvps? [x] - (instance? KeyValuePathSegment x)) - (def mapv-indexed (comp vec map-indexed)) (defn walk-with-path @@ -101,17 +121,23 @@ ([f path form] (walk-with-path (partial postwalk-with-path f) f path form))) +(s/fdef kps-path? + :args (s/cat :x any?) + :ret boolean?) (defn kps-path? "True if path points to a key" [x] - (and (vector? x) - (kps? (last x)))) + (boolean (and (vector? x) + (kps? (last x))))) +(s/fdef kvps-path? + :args (s/cat :x any?) + :ret boolean?) (defn kvps-path? "True if path points to a key/value pair" [x] - (and (vector? x) - (some kvps? x))) + (boolean (and (vector? x) + (some kvps? x)))) (defn summary-form "Given a form and a path to highlight, returns a data structure that marks @@ -534,9 +560,11 @@ should satisfy (print (if-not explain-data "Success!\n" - (let [{:keys [::s/problems ::s/value ::s/args ::s/failure :clojure.spec.test.alpha/caller]} explain-data + (let [{:keys [::s/problems ::s/value ::s/args ::s/ret ::s/failure :clojure.spec.test.alpha/caller]} explain-data form (if (= :instrument failure) - args + (if (contains? explain-data ::s/ret) + ret + args) value) _ (doseq [problem problems] (s/assert (s/nilable #{"Insufficient input" "Extra input" "no method"}) (:reason problem))) diff --git a/test/expound/alpha_test.cljc b/test/expound/alpha_test.cljc index 4153626b..6412d6c0 100644 --- a/test/expound/alpha_test.cljc +++ b/test/expound/alpha_test.cljc @@ -8,7 +8,9 @@ [expound.test-utils :as test-utils] [clojure.string :as string])) -(use-fixtures :once test-utils/check-spec-assertions) +(use-fixtures :once + test-utils/check-spec-assertions + test-utils/instrument-all) (def any-printable-wo-nan (gen/such-that (complement test-utils/contains-nan?) gen/any-printable)) diff --git a/test/expound/test_utils.cljc b/test/expound/test_utils.cljc index 32bbfd8e..4b78bc25 100644 --- a/test/expound/test_utils.cljc +++ b/test/expound/test_utils.cljc @@ -1,5 +1,12 @@ (ns expound.test-utils (:require [clojure.spec.alpha :as s] + #?(:cljs + [clojure.spec.test.alpha :as st] + ;; orchestra is supposed to work with cljs but + ;; it isn't working for me right now + #_[orchestra-cljs.spec.test :as st] + :clj [orchestra.spec.test :as st]) + [expound.alpha :as expound] [clojure.test :as ct] [com.gfredericks.test.chuck.clojure-test :as chuck])) @@ -17,6 +24,12 @@ (test-fn) (s/check-asserts false)) +(defn instrument-all [test-fn] + (set! s/*explain-out* expound/printer) + (st/instrument) + (test-fn) + (st/unstrument)) + (defn nan? [x] #?(:clj false :cljs (and (number? x) (js/isNaN x)))) From 54e7a306738fd4b583faa6b26be03211b226c0f6 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sat, 22 Jul 2017 15:04:49 -0600 Subject: [PATCH 8/8] Fixes README and updates changelog --- CHANGES.md | 9 ++++++++- README.md | 9 +++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d8d223db..2ff2c77a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,12 @@ # Changes +## master + +## 0.1.2 + +- Adds support for instrumentation https://github.com/bhb/expound/issues/4 +- Adds support for Spec asserts https://github.com/bhb/expound/issues/5 + ## 0.1.1 -- Fixes bug https://github.com/bhb/expound/issues/31 +- Fixes bug https://github.com/bhb/expound/issues/3 diff --git a/README.md b/README.md index df226dbc..b972d800 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,8 @@ Expound formats `clojure.spec` errors in a way that is optimized for humans to r Expound is in alpha while `clojure.spec` is in alpha. +Expound is tested with Clojure 1.9.0-alpha17 and Clojurescript 1.9.562. Clojurescript 1.9.542 only supports using `expound` and `expound-str` functions directly. + ## Usage [![Clojars Project](https://img.shields.io/clojars/v/expound.svg)](https://clojars.org/expound) @@ -75,12 +77,11 @@ Replace calls to `clojure.spec.alpha/explain` with `expound.alpha/expound` and t ### `*explain-out*` -To use other Spec functions, you can set `clojure.spec.alpha/*explain-out*` (or `cljs.spec.alpha/*explain-out*` for ClojureScript). +To use other Spec functions, set `clojure.spec.alpha/*explain-out*` (or `cljs.spec.alpha/*explain-out*` for ClojureScript) to `expound/printer`. (Setting `*explain-out*` does not work correctly in ClojureScript versions prior to `1.9.562` due to differences in `explain-data`) ```clojure -;; Temporarily set the dynamic var (require '[clojure.spec.alpha :as s]) ;; for clojurescript: ;; (require '[cljs.spec.alpha :as s]) @@ -92,8 +93,8 @@ To use other Spec functions, you can set `clojure.spec.alpha/*explain-out*` (or ;; Use `assert` (s/check-asserts true) ; enable asserts -;; Set printer in the scope of 'binding' -(binding [s/*explain-out* printer] +;; Set var in the scope of 'binding' +(binding [s/*explain-out* expound/printer] (s/assert :example.place/city 1)) ;; Or set it globally