From cfecff03c32e0c42671caa9d48102d46d03866f1 Mon Sep 17 00:00:00 2001
From: Jules Villard <jul@fb.com>
Date: Mon, 31 Jul 2017 05:53:48 -0700
Subject: [PATCH] [reportdiff] allow "current" and "previous" to be specified
 in any order and among other fields in file_renamings.json

Summary: This seems more in line with the expectations of the JSON format.

Reviewed By: mbouaziz

Differential Revision: D5500939

fbshipit-source-id: 76dcc47
---
 infer/src/backend/DifferentialFilters.ml   | 31 +++++++++++++++++-----
 infer/src/unit/DifferentialFiltersTests.ml |  3 ++-
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/infer/src/backend/DifferentialFilters.ml b/infer/src/backend/DifferentialFilters.ml
index a078c418a..674354d55 100644
--- a/infer/src/backend/DifferentialFilters.ml
+++ b/infer/src/backend/DifferentialFilters.ml
@@ -25,13 +25,30 @@ module FileRenamings = struct
   let from_json input : t =
     let j = Yojson.Basic.from_string input in
     let renaming_of_assoc assoc : renaming =
-      match assoc with
-      | `Assoc [("current", `String current); ("previous", `String previous)]
-       -> {current; previous}
-      | _
-       -> failwithf "Expected JSON object of the following form: '%s', but instead got: '%s'"
-            "{\"current\": \"aaa.java\", \"previous\": \"BBB.java\"}"
-            (Yojson.Basic.to_string assoc)
+      try
+        match assoc with
+        | `Assoc l
+         -> (
+            let current_opt = List.Assoc.find ~equal:String.equal l "current" in
+            let previous_opt = List.Assoc.find ~equal:String.equal l "previous" in
+            match (current_opt, previous_opt) with
+            | Some `String current, Some `String previous
+             -> {current; previous}
+            | None, _
+             -> raise (Yojson.Json_error "\"current\" field missing")
+            | Some _, None
+             -> raise (Yojson.Json_error "\"previous\" field missing")
+            | Some _, Some `String _
+             -> raise (Yojson.Json_error "\"current\" field is not a string")
+            | Some _, Some _
+             -> raise (Yojson.Json_error "\"previous\" field is not a string") )
+        | _
+         -> raise (Yojson.Json_error "not a record")
+      with Yojson.Json_error err ->
+        failwithf
+          "Error parsing file renamings: %s@\nExpected JSON object of the following form: '%s', but instead got: '%s'"
+          err "{\"current\": \"aaa.java\", \"previous\": \"BBB.java\"}"
+          (Yojson.Basic.to_string assoc)
     in
     match j with
     | `List json_renamings
diff --git a/infer/src/unit/DifferentialFiltersTests.ml b/infer/src/unit/DifferentialFiltersTests.ml
index 267a3c566..7e5efbb17 100644
--- a/infer/src/unit/DifferentialFiltersTests.ml
+++ b/infer/src/unit/DifferentialFiltersTests.ml
@@ -49,7 +49,8 @@ let test_file_renamings_from_json =
     , "[{\"current\": 1, \"previous\": \"BBB.java\"}]"
     , Raise
         (Failure
-           ( "Expected JSON object of the following form: "
+           ( "Error parsing file renamings: \"current\" field is not a string"
+           ^ "\nExpected JSON object of the following form: "
            ^ "'{\"current\": \"aaa.java\", \"previous\": \"BBB.java\"}', "
            ^ "but instead got: '{\"current\":1,\"previous\":\"BBB.java\"}'" )) )
   ; ( "test_file_renamings_from_json_with_malformed_input"