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

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.

src/SMAPI/Framework/Serialisation/ColorConverter.cs

@@ -1,46 +1,25 @@
 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>
+        /// <summary>Read a JSON object.</summary>
-        public override bool CanWrite => false;
+        /// <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)
-        /*********
-        ** 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(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));
@@ -48,15 +27,14 @@ namespace StardewModdingAPI.Framework.Serialisation
             return new Color(r, g, b, a);
         }
 
-                case JTokenType.String:
+        /// <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)
         {
-                        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}'.");
+                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]);
@@ -64,19 +42,5 @@ namespace StardewModdingAPI.Framework.Serialisation
             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.");
-            }
-        }
-
-        /// <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.");
-        }
     }
 }

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.";
+                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);
             }

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>
+        /// <summary>Read a JSON object.</summary>
-        public override bool CanWrite => false;
+        /// <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)
-        /*********
-        ** 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(Point);
-        }
-
-        /// <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)
-        {
-            // 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:
+        /// <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)
         {
-                        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 {token.Path}, invalid value '{str}'.");
+                throw new SParseException($"Can't parse {typeof(Point).Name} from invalid value '{str}' (path: {path}).");
 
             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.");
-        }
     }
 }

src/SMAPI/Framework/Serialisation/RectangleConverter.cs

@@ -1,47 +1,26 @@
 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>
+        /// <summary>Read a JSON object.</summary>
-        public override bool CanWrite => false;
+        /// <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)
-        /*********
-        ** 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(Rectangle);
-        }
-
-        /// <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: { "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));
@@ -49,15 +28,17 @@ namespace StardewModdingAPI.Framework.Serialisation
             return new Rectangle(x, y, width, height);
         }
 
-                case JTokenType.String:
+        /// <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)
         {
-                        string str = token.Value<string>();
             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.");
+                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);
@@ -66,19 +47,5 @@ namespace StardewModdingAPI.Framework.Serialisation
 
             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.");
-        }
     }
 }

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}).");
+        }
+    }
+}

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" />