Uploaded image for project: 'Clojure'
  1. CLJ-2013

[spec] Alternative s/cat options not error-reported

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects versions: Release 1.9
    • Fix versions: None
    • Labels:
    • Environment:

      alpha14

    • Approval:
      Triaged

      Description

      This problem was detected in context of this discussion https://groups.google.com/d/msg/clojure/mIlKaOiujlo/tF71zZ2BCwAJ

      A minimal version of how specs error reporting failed the users intuition there looks like this:

      He used an invalid ns form

      (ns foo (require [clojure.spec :as s])) ; should be :require
      

      The error reported by spec:

      In: [1] val: ((require [clojure.spec :as s])) fails at: [:args] predicate: (cat :docstring (? string?) :attr-map (? map?) :clauses :clojure.core.specs/ns-clauses),  Extra input
      :clojure.spec/args  (foo (require [clojure.spec :as s]))
        clojure.core/ex-info (core.clj:4725)
      

      While the error is technically true, it doesn't show the user /how/ each of the alternative options of the reported s/cat failed.

      To get a better understanding why the users data is not correct, he should know precisely what spec tried and how it failed.

      A good example of how this works is s/alt, where all failing alternatives are always reported to the user.

      The problem has been investigated, first experimentally, then in specs code. Finally, a patch that brings error reporting like s/alts comes attached.

      It has been observed that specs error reporting behavior for cat with optional branches is the following:

      1. If the cat failed after one or many optional branches, the entire cat is reported as failing
      2. If the cat failed after one or many optional branches /and/ a subsequent required branch, only the subsequent required branch is reported with no remarks to the alternative optional branches.

      Rule 1 explains the ns example.
      Rule 2 can fail the users intuition significantly worse:

      (s/explain (s/cat :maybe-num (s/? number?)
                        :keyword keyword?)
                 ["3"])
      

      gives

      In: [0] val: "3" fails at: [:keyword] predicate: keyword?
      

      The report clearly doesn't address the users intent of putting in a number. Instead he is made to believe that he should have entered a keyword.

      Solution:

      A simple patch has been programmed that changes op-explain to have the following behaviour:

      • All alternatives that have been tried in a s/cat are reported individually.

      It improves the reported errors significantly because it makes clearly transparent how the users data failed the validation.

      (ns foo (require [clojure.spec :as s])) ; should be :require
      

      now gives

      ExceptionInfo Call to clojure.core/ns did not conform to spec:
      In: [1] val: (require [clojure.spec :as s]) fails at: [:args :docstring] predicate: string?
      In: [1] val: (require [clojure.spec :as s]) fails at: [:args :attr-map] predicate: map?
      In: [1 0] val: require fails spec: :clojure.core.specs/ns-refer-clojure at: [:args :clauses :refer-clojure :clause] predicate: #{:refer-clojure}
      In: [1 0] val: require fails spec: :clojure.core.specs/ns-require at: [:args :clauses :require :clause] predicate: #{:require}
      In: [1 0] val: require fails spec: :clojure.core.specs/ns-import at: [:args :clauses :import :clause] predicate: #{:import}
      In: [1 0] val: require fails spec: :clojure.core.specs/ns-use at: [:args :clauses :use :clause] predicate: #{:use}
      In: [1 0] val: require fails spec: :clojure.core.specs/ns-refer at: [:args :clauses :refer :clause] predicate: #{:refer}
      In: [1 0] val: require fails spec: :clojure.core.specs/ns-load at: [:args :clauses :load :clause] predicate: #{:load}
      In: [1 0] val: require fails spec: :clojure.core.specs/ns-gen-class at: [:args :clauses :gen-class :clause] predicate: #{:gen-class}
      :clojure.spec/args  (foo (require [clojure.spec :as s]))
        clojure.core/ex-info (core.clj:4725)
      

      It would be even better if explain-data sorted ::s/problems by length of their :path which would push the first two unintended options to the end.

      (s/explain (s/cat :maybe-num (s/? number?)
                        :keyword keyword?)
                 ["3"])
      

      now gives

      In: [0] val: "3" fails at: [:maybe-num] predicate: number?
      In: [0] val: "3" fails at: [:keyword] predicate: keyword?
      

      While examples can be made up where this reporting produces more noise (like defn) I believe its the right tradeoff for aforementioned reasons and:

      • We programmers always ask our users for the most specific information when something went wrong - It is correct to apply the same to specs error reporting
      • Custom error reporters (s/explain-out) get more data to generate narrow reports matching the users intent even better

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              Leon Grapenthin
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated: