encapsulate common JSON converter code, improve parse errors (#423)

7 files changed, 168 insertions, 190 deletions

diff --git a/docs/release-notes.md b/docs/release-notes.md
index 27110bff..31ceb7de 100644
--- a/docs/release-notes.md
+++ b/docs/release-notes.md
@@ -12,6 +12,7 @@
 
 * For modders:
   * Added `SButton` `IsActionButton()` and `IsUseToolButton()` extensions.
+  * Improved JSON parse errors to provide more useful info for troubleshooting.
   * Fixed events being raised while the game is loading a save file.
   * Fixed input events not recognising controller input as an action or use-tool button.
   * Fixed input events setting the same `IsActionButton` and `IsUseToolButton` values for all buttons pressed in an update tick.
diff --git a/src/SMAPI/Framework/Serialisation/ColorConverter.cs b/src/SMAPI/Framework/Serialisation/ColorConverter.cs
index d29f73b8..d2315a00 100644
--- a/src/SMAPI/Framework/Serialisation/ColorConverter.cs
+++ b/src/SMAPI/Framework/Serialisation/ColorConverter.cs
@@ -1,82 +1,46 @@
 using System;
 using Microsoft.Xna.Framework;
-using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 using StardewModdingAPI.Framework.Exceptions;
 
 namespace StardewModdingAPI.Framework.Serialisation
 {
     /// <summary>Handles deserialisation of <see cref="Color"/> for crossplatform compatibility.</summary>
-    internal class ColorConverter : JsonConverter
+    /// <remarks>
+    /// - Linux/Mac format: { "B": 76, "G": 51, "R": 25, "A": 102 }
+    /// - Windows format:   "26, 51, 76, 102"
+    /// </remarks>
+    internal class ColorConverter : SimpleReadOnlyConverter<Color>
     {
         /*********
-        ** Accessors
+        ** Protected methods
         *********/
-        /// <summary>Whether this converter can write JSON.</summary>
-        public override bool CanWrite => false;
-
-
-        /*********
-        ** Public methods
-        *********/
-        /// <summary>Get whether this instance can convert the specified object type.</summary>
-        /// <param name="objectType">The object type.</param>
-        public override bool CanConvert(Type objectType)
+        /// <summary>Read a JSON object.</summary>
+        /// <param name="obj">The JSON object to read.</param>
+        /// <param name="path">The path to the current JSON node.</param>
+        protected override Color ReadObject(JObject obj, string path)
         {
-            return objectType == typeof(Color);
-        }
-
-        /// <summary>Reads the JSON representation of the object.</summary>
-        /// <param name="reader">The JSON reader.</param>
-        /// <param name="objectType">The object type.</param>
-        /// <param name="existingValue">The object being read.</param>
-        /// <param name="serializer">The calling serializer.</param>
-        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
-        {
-            //    Linux/Mac: { "B": 76, "G": 51, "R": 25, "A": 102 }
-            //    Windows:   "26, 51, 76, 102"
-            JToken token = JToken.Load(reader);
-            switch (token.Type)
-            {
-                case JTokenType.Object:
-                    {
-                        JObject obj = (JObject)token;
-                        int r = obj.Value<int>(nameof(Color.R));
-                        int g = obj.Value<int>(nameof(Color.G));
-                        int b = obj.Value<int>(nameof(Color.B));
-                        int a = obj.Value<int>(nameof(Color.A));
-                        return new Color(r, g, b, a);
-                    }
-
-                case JTokenType.String:
-                    {
-                        string str = token.Value<string>();
-                        if (string.IsNullOrWhiteSpace(str))
-                            return null;
-
-                        string[] parts = str.Split(',');
-                        if (parts.Length != 4)
-                            throw new SParseException($"Can't parse {typeof(Color).Name} from {token.Path}, invalid value '{str}'.");
-
-                        int r = Convert.ToInt32(parts[0]);
-                        int g = Convert.ToInt32(parts[1]);
-                        int b = Convert.ToInt32(parts[2]);
-                        int a = Convert.ToInt32(parts[3]);
-                        return new Color(r, g, b, a);
-                    }
-
-                default:
-                    throw new SParseException($"Can't parse {typeof(Point).Name} from {token.Path}, must be an object or string.");
-            }
+            int r = obj.Value<int>(nameof(Color.R));
+            int g = obj.Value<int>(nameof(Color.G));
+            int b = obj.Value<int>(nameof(Color.B));
+            int a = obj.Value<int>(nameof(Color.A));
+            return new Color(r, g, b, a);
         }
 
-        /// <summary>Writes the JSON representation of the object.</summary>
-        /// <param name="writer">The JSON writer.</param>
-        /// <param name="value">The value.</param>
-        /// <param name="serializer">The calling serializer.</param>
-        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
+        /// <summary>Read a JSON string.</summary>
+        /// <param name="str">The JSON string value.</param>
+        /// <param name="path">The path to the current JSON node.</param>
+        protected override Color ReadString(string str, string path)
         {
-            throw new InvalidOperationException("This converter does not write JSON.");
+            string[] parts = str.Split(',');
+            if (parts.Length != 4)
+                throw new SParseException($"Can't parse {typeof(Color).Name} from invalid value '{str}' (path: {path}).");
+
+            int r = Convert.ToInt32(parts[0]);
+            int g = Convert.ToInt32(parts[1]);
+            int b = Convert.ToInt32(parts[2]);
+            int a = Convert.ToInt32(parts[3]);
+            return new Color(r, g, b, a);
         }
     }
 }
diff --git a/src/SMAPI/Framework/Serialisation/JsonHelper.cs b/src/SMAPI/Framework/Serialisation/JsonHelper.cs
index f66f9dfb..90a6d258 100644
--- a/src/SMAPI/Framework/Serialisation/JsonHelper.cs
+++ b/src/SMAPI/Framework/Serialisation/JsonHelper.cs
@@ -63,11 +63,16 @@ namespace StardewModdingAPI.Framework.Serialisation
             {
                 return this.Deserialise<TModel>(json);
             }
-            catch (JsonReaderException ex)
+            catch (Exception ex)
             {
-                string error = $"The file at {fullPath} doesn't seem to be valid JSON.";
-                if (json.Contains("“") || json.Contains("”"))
-                    error += " Found curly quotes in the text; note that only straight quotes are allowed in JSON.";
+                string error = $"Can't parse JSON file at {fullPath}.";
+
+                if (ex is JsonReaderException)
+                {
+                    error += " This doesn't seem to be valid JSON.";
+                    if (json.Contains("“") || json.Contains("”"))
+                        error += " Found curly quotes in the text; note that only straight quotes are allowed in JSON.";
+                }
                 error += $"\nTechnical details: {ex.Message}";
                 throw new JsonReaderException(error);
             }
diff --git a/src/SMAPI/Framework/Serialisation/PointConverter.cs b/src/SMAPI/Framework/Serialisation/PointConverter.cs
index d35660be..bdcefaa5 100644
--- a/src/SMAPI/Framework/Serialisation/PointConverter.cs
+++ b/src/SMAPI/Framework/Serialisation/PointConverter.cs
@@ -1,79 +1,42 @@
 using System;
 using Microsoft.Xna.Framework;
-using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 using StardewModdingAPI.Framework.Exceptions;
 
 namespace StardewModdingAPI.Framework.Serialisation
 {
     /// <summary>Handles deserialisation of <see cref="PointConverter"/> for crossplatform compatibility.</summary>
-    internal class PointConverter : JsonConverter
+    /// <remarks>
+    /// - Linux/Mac format: { "X": 1, "Y": 2 }
+    /// - Windows format:   "1, 2"
+    /// </remarks>
+    internal class PointConverter : SimpleReadOnlyConverter<Point>
     {
         /*********
-        ** Accessors
+        ** Protected methods
         *********/
-        /// <summary>Whether this converter can write JSON.</summary>
-        public override bool CanWrite => false;
-
-
-        /*********
-        ** Public methods
-        *********/
-        /// <summary>Get whether this instance can convert the specified object type.</summary>
-        /// <param name="objectType">The object type.</param>
-        public override bool CanConvert(Type objectType)
+        /// <summary>Read a JSON object.</summary>
+        /// <param name="obj">The JSON object to read.</param>
+        /// <param name="path">The path to the current JSON node.</param>
+        protected override Point ReadObject(JObject obj, string path)
         {
-            return objectType == typeof(Point);
+            int x = obj.Value<int>(nameof(Point.X));
+            int y = obj.Value<int>(nameof(Point.Y));
+            return new Point(x, y);
         }
 
-        /// <summary>Reads the JSON representation of the object.</summary>
-        /// <param name="reader">The JSON reader.</param>
-        /// <param name="objectType">The object type.</param>
-        /// <param name="existingValue">The object being read.</param>
-        /// <param name="serializer">The calling serializer.</param>
-        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
+        /// <summary>Read a JSON string.</summary>
+        /// <param name="str">The JSON string value.</param>
+        /// <param name="path">The path to the current JSON node.</param>
+        protected override Point ReadString(string str, string path)
         {
-            // point
-            //    Linux/Mac: { "X": 1, "Y": 2 }
-            //    Windows:   "1, 2"
-            JToken token = JToken.Load(reader);
-            switch (token.Type)
-            {
-                case JTokenType.Object:
-                    {
-                        JObject obj = (JObject)token;
-                        int x = obj.Value<int>(nameof(Point.X));
-                        int y = obj.Value<int>(nameof(Point.Y));
-                        return new Point(x, y);
-                    }
-
-                case JTokenType.String:
-                    {
-                        string str = token.Value<string>();
-                        if (string.IsNullOrWhiteSpace(str))
-                            return null;
+            string[] parts = str.Split(',');
+            if (parts.Length != 2)
+                throw new SParseException($"Can't parse {typeof(Point).Name} from invalid value '{str}' (path: {path}).");
 
-                        string[] parts = str.Split(',');
-                        if (parts.Length != 2)
-                            throw new SParseException($"Can't parse {typeof(Point).Name} from {token.Path}, invalid value '{str}'.");
-
-                        int x = Convert.ToInt32(parts[0]);
-                        int y = Convert.ToInt32(parts[1]);
-                        return new Point(x, y);
-                    }
-
-                default:
-                    throw new SParseException($"Can't parse {typeof(Point).Name} from {token.Path}, must be an object or string.");
-            }
-        }
-
-        /// <summary>Writes the JSON representation of the object.</summary>
-        /// <param name="writer">The JSON writer.</param>
-        /// <param name="value">The value.</param>
-        /// <param name="serializer">The calling serializer.</param>
-        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
-        {
-            throw new InvalidOperationException("This converter does not write JSON.");
+            int x = Convert.ToInt32(parts[0]);
+            int y = Convert.ToInt32(parts[1]);
+            return new Point(x, y);
         }
     }
 }
diff --git a/src/SMAPI/Framework/Serialisation/RectangleConverter.cs b/src/SMAPI/Framework/Serialisation/RectangleConverter.cs
index 74df54e2..bbf60435 100644
--- a/src/SMAPI/Framework/Serialisation/RectangleConverter.cs
+++ b/src/SMAPI/Framework/Serialisation/RectangleConverter.cs
@@ -1,84 +1,51 @@
 using System;
 using System.Text.RegularExpressions;
 using Microsoft.Xna.Framework;
-using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
 using StardewModdingAPI.Framework.Exceptions;
 
 namespace StardewModdingAPI.Framework.Serialisation
 {
     /// <summary>Handles deserialisation of <see cref="Rectangle"/> for crossplatform compatibility.</summary>
-    internal class RectangleConverter : JsonConverter
+    /// <remarks>
+    /// - Linux/Mac format: { "X": 1, "Y": 2, "Width": 3, "Height": 4 }
+    /// - Windows format:   "{X:1 Y:2 Width:3 Height:4}"
+    /// </remarks>
+    internal class RectangleConverter : SimpleReadOnlyConverter<Rectangle>
     {
         /*********
-        ** Accessors
+        ** Protected methods
         *********/
-        /// <summary>Whether this converter can write JSON.</summary>
-        public override bool CanWrite => false;
-
-
-        /*********
-        ** Public methods
-        *********/
-        /// <summary>Get whether this instance can convert the specified object type.</summary>
-        /// <param name="objectType">The object type.</param>
-        public override bool CanConvert(Type objectType)
+        /// <summary>Read a JSON object.</summary>
+        /// <param name="obj">The JSON object to read.</param>
+        /// <param name="path">The path to the current JSON node.</param>
+        protected override Rectangle ReadObject(JObject obj, string path)
         {
-            return objectType == typeof(Rectangle);
+            int x = obj.Value<int>(nameof(Rectangle.X));
+            int y = obj.Value<int>(nameof(Rectangle.Y));
+            int width = obj.Value<int>(nameof(Rectangle.Width));
+            int height = obj.Value<int>(nameof(Rectangle.Height));
+            return new Rectangle(x, y, width, height);
         }
 
-        /// <summary>Reads the JSON representation of the object.</summary>
-        /// <param name="reader">The JSON reader.</param>
-        /// <param name="objectType">The object type.</param>
-        /// <param name="existingValue">The object being read.</param>
-        /// <param name="serializer">The calling serializer.</param>
-        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
+        /// <summary>Read a JSON string.</summary>
+        /// <param name="str">The JSON string value.</param>
+        /// <param name="path">The path to the current JSON node.</param>
+        protected override Rectangle ReadString(string str, string path)
         {
-            //    Linux/Mac: { "X": 1, "Y": 2, "Width": 3, "Height": 4 }
-            //    Windows:   "{X:1 Y:2 Width:3 Height:4}"
-            JToken token = JToken.Load(reader);
-            switch (token.Type)
-            {
-                case JTokenType.Object:
-                    {
-                        JObject obj = (JObject)token;
-                        int x = obj.Value<int>(nameof(Rectangle.X));
-                        int y = obj.Value<int>(nameof(Rectangle.Y));
-                        int width = obj.Value<int>(nameof(Rectangle.Width));
-                        int height = obj.Value<int>(nameof(Rectangle.Height));
-                        return new Rectangle(x, y, width, height);
-                    }
-
-                case JTokenType.String:
-                    {
-                        string str = token.Value<string>();
-                        if (string.IsNullOrWhiteSpace(str))
-                            return Rectangle.Empty;
+            if (string.IsNullOrWhiteSpace(str))
+                return Rectangle.Empty;
 
-                        var match = Regex.Match(str, @"^\{X:(?<x>\d+) Y:(?<y>\d+) Width:(?<width>\d+) Height:(?<height>\d+)\}$");
-                        if (!match.Success)
-                            throw new SParseException($"Can't parse {typeof(Rectangle).Name} from {reader.Path}, invalid string format.");
+            var match = Regex.Match(str, @"^\{X:(?<x>\d+) Y:(?<y>\d+) Width:(?<width>\d+) Height:(?<height>\d+)\}$");
+            if (!match.Success)
+                throw new SParseException($"Can't parse {typeof(Rectangle).Name} from invalid value '{str}' (path: {path}).");
 
-                        int x = Convert.ToInt32(match.Groups["x"].Value);
-                        int y = Convert.ToInt32(match.Groups["y"].Value);
-                        int width = Convert.ToInt32(match.Groups["width"].Value);
-                        int height = Convert.ToInt32(match.Groups["height"].Value);
+            int x = Convert.ToInt32(match.Groups["x"].Value);
+            int y = Convert.ToInt32(match.Groups["y"].Value);
+            int width = Convert.ToInt32(match.Groups["width"].Value);
+            int height = Convert.ToInt32(match.Groups["height"].Value);
 
-                        return new Rectangle(x, y, width, height);
-                    }
-
-                default:
-                    throw new SParseException($"Can't parse {typeof(Rectangle).Name} from {reader.Path}, must be an object or string.");
-            }
-        }
-
-        /// <summary>Writes the JSON representation of the object.</summary>
-        /// <param name="writer">The JSON writer.</param>
-        /// <param name="value">The value.</param>
-        /// <param name="serializer">The calling serializer.</param>
-        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
-        {
-            throw new InvalidOperationException("This converter does not write JSON.");
+            return new Rectangle(x, y, width, height);
         }
     }
 }
diff --git a/src/SMAPI/Framework/Serialisation/SimpleReadOnlyConverter.cs b/src/SMAPI/Framework/Serialisation/SimpleReadOnlyConverter.cs
new file mode 100644
index 00000000..9308456b
--- /dev/null
+++ b/src/SMAPI/Framework/Serialisation/SimpleReadOnlyConverter.cs
@@ -0,0 +1,77 @@
+using System;
+using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
+using StardewModdingAPI.Framework.Exceptions;
+
+namespace StardewModdingAPI.Framework.Serialisation
+{
+    /// <summary>The base implementation for simplified converters which deserialise <typeparamref name="T"/> without overriding serialisation.</summary>
+    /// <typeparam name="T">The type to deserialise.</typeparam>
+    internal abstract class SimpleReadOnlyConverter<T> : JsonConverter
+    {
+        /*********
+        ** Accessors
+        *********/
+        /// <summary>Whether this converter can write JSON.</summary>
+        public override bool CanWrite => false;
+
+
+        /*********
+        ** Public methods
+        *********/
+        /// <summary>Get whether this instance can convert the specified object type.</summary>
+        /// <param name="objectType">The object type.</param>
+        public override bool CanConvert(Type objectType)
+        {
+            return objectType == typeof(T);
+        }
+
+        /// <summary>Writes the JSON representation of the object.</summary>
+        /// <param name="writer">The JSON writer.</param>
+        /// <param name="value">The value.</param>
+        /// <param name="serializer">The calling serializer.</param>
+        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
+        {
+            throw new InvalidOperationException("This converter does not write JSON.");
+        }
+
+        /// <summary>Reads the JSON representation of the object.</summary>
+        /// <param name="reader">The JSON reader.</param>
+        /// <param name="objectType">The object type.</param>
+        /// <param name="existingValue">The object being read.</param>
+        /// <param name="serializer">The calling serializer.</param>
+        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
+        {
+            string path = reader.Path;
+            switch (reader.TokenType)
+            {
+                case JsonToken.StartObject:
+                    return this.ReadObject(JObject.Load(reader), path);
+                case JsonToken.String:
+                    return this.ReadString(JToken.Load(reader).Value<string>(), path);
+                default:
+                    throw new SParseException($"Can't parse {typeof(T).Name} from {reader.TokenType} (path: {reader.Path}).");
+            }
+        }
+
+
+        /*********
+        ** Protected methods
+        *********/
+        /// <summary>Read a JSON object.</summary>
+        /// <param name="obj">The JSON object to read.</param>
+        /// <param name="path">The path to the current JSON node.</param>
+        protected virtual T ReadObject(JObject obj, string path)
+        {
+            throw new SParseException($"Can't parse {typeof(T).Name} from object (path: {path}).");
+        }
+
+        /// <summary>Read a JSON string.</summary>
+        /// <param name="str">The JSON string value.</param>
+        /// <param name="path">The path to the current JSON node.</param>
+        protected virtual T ReadString(string str, string path)
+        {
+            throw new SParseException($"Can't parse {typeof(T).Name} from string (path: {path}).");
+        }
+    }
+}
diff --git a/src/SMAPI/StardewModdingAPI.csproj b/src/SMAPI/StardewModdingAPI.csproj
index e02e1ab4..dd4f6134 100644
--- a/src/SMAPI/StardewModdingAPI.csproj
+++ b/src/SMAPI/StardewModdingAPI.csproj
@@ -110,6 +110,7 @@
     <Compile Include="Framework\Exceptions\SAssemblyLoadFailedException.cs" />
     <Compile Include="Framework\ModLoading\AssemblyLoadStatus.cs" />
     <Compile Include="Framework\Reflection\InterfaceProxyBuilder.cs" />
+    <Compile Include="Framework\Serialisation\SimpleReadOnlyConverter.cs" />
     <Compile Include="Framework\Serialisation\RectangleConverter.cs" />
     <Compile Include="Framework\Serialisation\ColorConverter.cs" />
     <Compile Include="Framework\Serialisation\PointConverter.cs" />