Repository: chrovis/cljam
Pull request #138 (2018-07-05 07:14:30)

Character escaping support for string fields in VCF meta info
This PR provides character escaping support for VCF string fields (that was reported in #135).

With this change, you'll be able to use the characters `\` and `"` in VCF string fields by escaping them with `\`, such like `\\` and `\"`.

```clj
(require '[cljam.io.vcf.reader :as reader])
(#'reader/parse-structured-line "ID=ID,Description=\"\\\"This\\\" is a description\",Note=\"You can use \\\" in string fields by escaping it with \\\\\"")
;=> {:id "ID", :description "\"This\" is a description", :note "You can use \" in string fields by escaping it with \\"}
```

It also improves error checking for string field format:

```clj
(#'reader/parse-structured-line "x=\"unbalanced double quote")
;; RuntimeException Unexpected end of string field

(#'reader/parse-structured-line "x=\"unknown \\x escape sequence\"")
;; RuntimeException Either '\' or '"' was expected immediately after '\', but got 'x'
```

My only concern at the moment is that the VCF reader sometimes ends up making a weird result with no error if there are some redundant whitespaces around a comma (`,`):

```clj
(#'reader/parse-structured-line "x=\"foo\"  ,  y=\"bar\"")
;=> {:x "foo", :,-y "bar"}
```

I'm not sure if this form of meta info line is valid against the spec and wondering whether or not we should take a special care of those particular cases.


Patch:
diff --git a/src/cljam/io/vcf/reader.clj b/src/cljam/io/vcf/reader.clj
index a9f3134..62c58cf 100644
--- a/src/cljam/io/vcf/reader.clj
+++ b/src/cljam/io/vcf/reader.clj
@@ -64,10 +64,26 @@
         => {:id \"NS\" :number \"1\" :type \"Integer\"
             :description \"Number of Samples With Data\"}"
   [s]
-  (->> (re-seq #"([\w:/\.\?\-]*)=(?:(?:\"([^\"]*?)\")|([\w:/\.\?\-]*))" s)
-       (map (fn [[_ k v1 v2]]
-              {(->kebab-case-keyword k) (dot->nil (or v2 v1))}))
-       (apply merge)))
+  (letfn [(parse-string-field [s]
+            (loop [s s, acc []]
+              (if-let [[_ pre c post] (re-matches #"([^\\\"]*?)([\\\"])(.*)" s)]
+                (if (= c "\\")
+                  ;; expecting one of the special characters occurs immediately after \\
+                  (if (#{\\ \"} (first post))
+                    (recur (subs post 1) (conj acc pre (first post)))
+                    (let [msg (str "Either '\\' or '\"' was expected immediately after '\\', "
+                                   "but got '" (first post) "'")]
+                      (throw (RuntimeException. msg))))
+                  ;; c == \", that is, we are now at the end of the string field
+                  [nil (cstr/join (conj acc pre)) (cstr/replace post #"^," "")])
+                (throw (RuntimeException. "Unexpected end of string field")))))]
+    (loop [s s, m {}]
+      (if-let [[_ k more] (some->> s (re-matches #"([^=]+?)=(.*)"))]
+        (let [[_ v more] (if (= (first more) \")
+                           (parse-string-field (subs more 1))
+                           (re-matches #"([^,]*?)(?:,(.*))?" more))]
+          (recur more (assoc m (->kebab-case-keyword k) (dot->nil v))))
+        m))))
 
 (defn- parse-meta-info-contig
   [m]
diff --git a/src/cljam/io/vcf/writer.clj b/src/cljam/io/vcf/writer.clj
index ab81245..b021d26 100644
--- a/src/cljam/io/vcf/writer.clj
+++ b/src/cljam/io/vcf/writer.clj
@@ -49,10 +49,15 @@
     (cstr/upper-case (name k))
     (->camelCaseString k)))
 
+(defn- escape-special-chars [s]
+  (-> s (cstr/replace "\\" "\\\\") (cstr/replace "\"" "\\\"")))
+
 (defn- add-extra-fields
   [fields m required-keys]
   (reduce-kv (fn [fields k v]
-               (conj fields (str (->PascalCaseString k) "=\"" v "\"")))
+               (conj fields
+                     (str (->PascalCaseString k) "=\""
+                          (escape-special-chars v) "\"")))
              fields
              (apply dissoc m required-keys)))
 
@@ -68,7 +73,7 @@
         (:assembly m) (conj (str "assembly=" (:assembly m)))
         (:md-5 m) (conj (str "md5=" (:md-5 m)))
         (:url m) (conj (str "URL=" (:url m)))
-        (:species m) (conj (str "species=\"" (:species m) "\""))
+        (:species m) (conj (str "species=\"" (escape-special-chars (:species m)) "\""))
         (:taxonomy m) (conj (str "taxonomy=" (:taxonomy m)))
         (:idx m) (conj (str "IDX=" (:idx m))))
       (pack-meta-info m [:id :length :assembly :md-5 :url :species :taxonomy :idx])))
@@ -78,17 +83,17 @@
   (-> [(str "ID=" (:id m))
        (str "Number=" (nil->dot (:number m)))
        (str "Type=" (nil->dot (:type m)))
-       (str "Description=\"" (:description m) "\"")]
+       (str "Description=\"" (escape-special-chars (:description m)) "\"")]
       (cond->
-        (:source m) (conj (str "Source=\"" (:source m) "\""))
-        (:version m) (conj (str "Version=\"" (:version m) "\""))
+        (:source m) (conj (str "Source=\"" (escape-special-chars (:source m)) "\""))
+        (:version m) (conj (str "Version=\"" (escape-special-chars (:version m)) "\""))
         (:idx m) (conj (str "IDX=" (:idx m))))
       (pack-meta-info m [:id :number :type :description :source :version :idx])))
 
 (defn- stringify-meta-info-filter
   [m]
   (-> [(str "ID=" (:id m))
-       (str "Description=\"" (:description m) "\"")]
+       (str "Description=\"" (escape-special-chars (:description m)) "\"")]
       (cond->
         (:idx m) (conj (str "IDX=" (:idx m))))
       (pack-meta-info m [:id :description :idx])))
@@ -98,7 +103,7 @@
   (-> [(str "ID=" (:id m))
        (str "Number=" (nil->dot (:number m)))
        (str "Type=" (nil->dot (:type m)))
-       (str "Description=\"" (:description m) "\"")]
+       (str "Description=\"" (escape-special-chars (:description m)) "\"")]
       (cond->
         (:idx m) (conj (str "IDX=" (:idx m))))
       (pack-meta-info m [:id :number :type :description :idx])))
@@ -106,7 +111,7 @@
 (defn- stringify-meta-info-alt
   [m]
   (-> [(str "ID=" (:id m))
-       (str "Description=\"" (:description m) "\"")]
+       (str "Description=\"" (escape-special-chars (:description m)) "\"")]
       (pack-meta-info m [:id :description])))
 
 (defn- stringify-meta-info-sample
@@ -115,7 +120,7 @@
       (cond->
         (:genomes m) (conj (str "Genomes=" (:genomes m)))
         (:mixture m) (conj (str "Mixture=" (:mixture m))))
-      (conj (str "Description=\"" (:description m) "\""))
+      (conj (str "Description=\"" (escape-special-chars (:description m)) "\""))
       (pack-meta-info m [:id :genomes :mixture :description])))
 
 (defn- stringify-meta-info-pedigree


Test patch:
diff --git a/test/cljam/io/vcf/reader_test.clj b/test/cljam/io/vcf/reader_test.clj
new file mode 100644
index 0000000..25a766f
--- /dev/null
+++ b/test/cljam/io/vcf/reader_test.clj
@@ -0,0 +1,14 @@
+(ns cljam.io.vcf.reader-test
+  (:require [clojure.test :refer [deftest is]]
+            [cljam.io.vcf.reader :as vcf-reader]))
+
+(deftest parse-structured-line-test
+  (is (= {:id "ID", :description "\"This\" is a description",
+          :note "You can use \" in string fields by escaping it with \\"}
+       (#'vcf-reader/parse-structured-line "ID=ID,Description=\"\\\"This\\\" is a description\",Note=\"You can use \\\" in string fields by escaping it with \\\\\"")))
+  (is (thrown-with-msg? RuntimeException
+                        #"Unexpected end of string field"
+                        (#'vcf-reader/parse-structured-line "x=\"unbalanced double quote")))
+  (is (thrown-with-msg? RuntimeException
+                        #"Either '\\' or '\"' was expected immediately after '\\', but got 'x'"
+                        (#'vcf-reader/parse-structured-line "x=\"unknown \\x escape sequence\""))))
diff --git a/test/cljam/test_common.clj b/test/cljam/test_common.clj
index 5324a28..cc64c12 100644
--- a/test/cljam/test_common.clj
+++ b/test/cljam/test_common.clj
@@ -649,7 +649,7 @@
             {:id "Sample2", :assay "Exome", :ethnicity "CEU", :disease "Cancer",
              :tissue "Breast", :description "European patient exome from breast cancer"}
             {:id "Blood", :genomes "Germline", :mixture "1.", :description "Patient germline genome"}
-            {:id "TissueSample", :genomes "Germline", :mixture ".3", :description "Patient germline genome;Patient tumor genome"}]
+            {:id "TissueSample", :genomes "Germline;Tumor", :mixture ".3;.7", :description "Patient germline genome;Patient tumor genome"}]
    :pedigree [{:id "TumourSample", :original "GermlineID"}
               {:id "SomaticNonTumour", :original "GermlineID"}
               {:id "ChildID", :father "FatherID", :mother "MotherID"}