alexanderkiel / phrase

Clojure(Script) library for phrasing spec problems.
Eclipse Public License 1.0
289 stars 8 forks source link

Simple or not doable??? #31

Closed jsa-aerial closed 5 years ago

jsa-aerial commented 5 years ago

I cannot find a way to make a simple 'or' spec work:

(s/def ::id-stg
  (s/and
   string?
   #(re-matches #"^[0-9]+$" %)))

(defphraser string?
  {:via [::id-stg]} [_ _]
  "Id must be string")

(defphraser #(re-find re %)
  [_ _ re]
  (str "Id must "
       (case  (str/replace #"/" "" (str re))
         "^[0-9]+$" "contain only digits")))

(phrase-first {} ::id-stg 1) ; works - returns "Id must be string"
(phrase-first {} ::id-stg "a") ; returns nil

That's just one set of trial and error attempts.

This lib looks like it could be on track to doing 'The Right Thing'. But it is extremely opaque. When / how to use specifiers, context, problem etc to get a 'proper' dispatch is completely guess ware. If you could at least give an example with an 'or spec' and a s/keys spec (for maps) maybe one could stumble towards success.

jsa-aerial commented 5 years ago

I'm beginning to see how this can be made to work, but it is amazingly confusing...

alexanderkiel commented 5 years ago

I'm sorry that you have problems using phrase. In More Complex Example in the README, you can fins something similar to your example. In your example, the problem is, that you use re-matches in your spec but re-find in the phrase definition. As for s/or, I don't have an example in the README. But you use s/and and only that would make sense in your case. So please give me an example with s/or and I'll have a look into it.

jsa-aerial commented 5 years ago

As, I mentioned, I'm beginning to see how this is supposed to work. For example, it was totally unclear that predicates need to match symbolically, not functionally. Also it can be very tricky - something like

(s/def ::id-stg #(re-matches #"^[0-9]+$" %))

Won't work, even if your defphraser uses re-matches because spec wraps the anonymous function inside another anonymous function. Which will never symbolically match. Actually, as I've come to see, using anonymous functions in this stuff is highly dubious for a number of reasons.

But the real problem is none of this is really described or defined. The simplest definition and/or description of the parts (context, specifiers, context-binding-form, problem-binding-form, capture-binding-forms) what they are and how they affect dispatching (in phrase) would be enormously helpful. I still have no idea what 'context' is - it never seems to be used in phrase or `phrase, despite the doc string inphrase-firstsaying it uses the context andphrasedoc string saying it uses it to dispatch. Thedispatchdispatch function forphrase*` multi method does not look to use it at all. It's passed around, but never used.

So, I have managed to figure out s/or as follows:

(defn digits? [s] (re-matches #"^[0-9]+$" s))
(s/def ::id-int int?)
(s/def ::id-stg (s/and string? digits?))
(s/def ::id (s/or :idint ::id-int :idstg ::id-stg))

;;; OK, digits? keeps spec from wrapping the re-matches. Then we can:

(defphraser int?
  {:via [::id-int]} [_ _]
  "an integer")
(defphraser string?
  {:via [::id-stg]} [_ _]
  "a string")
(defphraser digits?
  {:via [::id-stg]} [_ _]
  (str "a string containing only digits"))

;;; Now things will match up symbolically, so we can now add the following:
(defn validate-msg
  [spec x & {:keys [sep prefix suffix]
             :or {sep " or " prefix "Must be " suffix ""}}]
  (let [msgbody (->> (s/explain-data spec  x) ::s/problems
                     (mapv #(phrase {} %))
                     (cljstr/join sep))]
    (str prefix msgbody suffix)))

(validate-msg ::id 'foo) ; => "Must be an integer or a string"
(validate-msg ::id "12xx") ; => "Must be an integer or a string containing only digits"

I'm starting to look at maps with s/keys now with the hope something similar can be achieved.

jsa-aerial commented 5 years ago

I'm getting the 'hang' of how this works and the idiomatic usage for it. And once you have that, it works very nicely!

Here's some more stuff I now have:

(defn make-validator [spec & validate-msg-kvs]
  (fn [x]
    (if (not (s/valid? spec x))
      (apply validate-msg spec x validate-msg-kvs)
      "")))

(s/def :example.place/city string?)
(s/def :example.place/state string?)
(s/def :example.place/country string?)
(s/def :example/place
  (s/keys :req-un
          [:example.place/city
           :example.place/state
           :example.place/country]))

(defphraser string?
  {:via [:example.place/city]} [_ _]
  "`city` must be a string")

(defphraser string?
  {:via [:example.place/state]} [_ _]
  "`state` must be a string")

(defphraser string?
  {:via [:example.place/country]} [_ _]
  "`country` must be a string")

(defphraser #(contains? % key)
  [_ _ key]
  (format "Must contain `%s`" (name key)))

(def validate-place (make-validator :example/place :prefix "" :sep " and "))
(map #(vector % (validate-place %))
     [{:city "a city", :state "a state" :country "a country"}
      {:state "a state" :country "a country"}
      {:state :a-state :country "a country"}
      {:city "a city", :state :a-state :country "a country"}])
=> 
([{:city "a city", :state "a state", :country "a country"} ""]
 [{:state "a state", :country "a country"} "Must contain `city`"]
 [{:state :a-state, :country "a country"}
  "Must contain `city` and `state` must be a string"]
 [{:city "a city", :state :a-state, :country "a country"}
  "`state` must be a string"])

;;; Nested maps as well
(s/def ::first-name (s/and string? #(not (empty? %))))
(s/def ::person (s/keys :req-un [::first-name]))
(s/def ::foo int?)
(s/def ::bar int?)
(s/def ::location (s/keys :opt-un [::person]
                          :req-un [::foo ::bar]))

(p/defphraser int?
  {:via [::foo]} [_ _]
  "`foo` must be an integer")
(p/defphraser int?
  {:via [::bar]} [_ _]
  "`bar` must be an integer")
(p/defphraser #(not (empty? %))
  [_ problem]
  (let [via (:via problem)
        path (map name via)]
    (format "`%s` %s" (cljstr/join "." path) " must not be empty")))

(def validate-location (make-validator ::location :prefix "" :sep " and "))
(map #(vector % (validate-location %))
     [{:person {:first-name ""}}
      {:person {:first-name "Kiki"}}
      {:person {:first-name "Kiki"} :foo "abc" :bar 12}
      {:person {:first-name "Kiki"} :foo 37 :bar 12}])
=>
([{:person {:first-name ""}}
  "Must contain `foo` and Must contain `bar` and `location.person.first-name`  must not be empty"]
 [{:person {:first-name "Kiki"}}
  "Must contain `foo` and Must contain `bar`"]
 [{:person {:first-name "Kiki"}, :foo "abc", :bar 12}
  "`foo` must be an integer"]
 [{:person {:first-name "Kiki"}, :foo 37, :bar 12} ""])
jsa-aerial commented 5 years ago

OK, once you have a good idea of how the multimethods are built to dispatch on normalized symbolic representations of the predicates involved and optionally the fully qualified path, you can really make some nice messages.

This thing is very clever and, once you understand it, works very well.

Maybe I will write up an overview description of how the machinery is intended to work and how to best use it. That would help a lot.

I'm closing this!!