From 8dd2be8c67a0caed6493d418cb407ab480af4a3b Mon Sep 17 00:00:00 2001
From: Deukhoofd <Deukhoofd@gmail.com>
Date: Wed, 21 Nov 2018 14:04:43 +0100
Subject: [PATCH] Better error handling for setting userdata values

---
 Upsilon/BaseTypes/UserData/UserData.cs     |  4 +--
 Upsilon/BaseTypes/UserData/UserDataType.cs | 14 +++++----
 UpsilonTests/UserDataTests.cs              | 34 ++++++++++++++++++++++
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/Upsilon/BaseTypes/UserData/UserData.cs b/Upsilon/BaseTypes/UserData/UserData.cs
index 2230647..4565c14 100644
--- a/Upsilon/BaseTypes/UserData/UserData.cs
+++ b/Upsilon/BaseTypes/UserData/UserData.cs
@@ -37,10 +37,10 @@ namespace Upsilon.BaseTypes.UserData
 
         public void Set(Diagnostics diagnostics, TextSpan span, string s, LuaType value)
         {
-            var failed = _typeInfo.Set(Value, s, value);
+            var (failed, error) = _typeInfo.Set(Value, s, value);
             if (failed)
             {
-                diagnostics.LogError($"Cannot find member '{s}' on type '{Value.GetType()}'", span);
+                diagnostics.LogError(error, span);
             }
         }
 
diff --git a/Upsilon/BaseTypes/UserData/UserDataType.cs b/Upsilon/BaseTypes/UserData/UserDataType.cs
index d91ea5f..b51004c 100644
--- a/Upsilon/BaseTypes/UserData/UserDataType.cs
+++ b/Upsilon/BaseTypes/UserData/UserDataType.cs
@@ -53,23 +53,27 @@ namespace Upsilon.BaseTypes.UserData
             return (null, true);
         }
 
-        public bool Set(object value, string member, LuaType newValue)
+        public (bool failed, string error) Set(object value, string member, LuaType newValue)
         {
             member = member.ToLowerInvariant();
             if (value.GetType() != Type)
-                return true;
+                return (true, "Invalid Type");
             if (Variables.TryGetValue(member, out var info))
             {
                 info.SetValue(value, newValue.ToCSharpObject());
-                return false;
+                return (false, null);
             }
             if (Properties.TryGetValue(member, out var property))
             {
+                if (property.SetMethod == null || property.SetMethod.IsPrivate)
+                {
+                    return (true, $"Property '{member}' on type '{Type}' does not have a publicly available setter.");
+                }
                 property.SetValue(value, newValue.ToCSharpObject());
-                return false;
+                return (false, null);
             }
 
-            return true;
+            return (true, $"Cannot find member '{member}' on type '{Type}'");
         }
 
         public (LuaType Type, bool Failed) BinaryOperator(object value, LuaType par1, OperatorType op, LuaType par2)
diff --git a/UpsilonTests/UserDataTests.cs b/UpsilonTests/UserDataTests.cs
index a191a77..5a247e5 100644
--- a/UpsilonTests/UserDataTests.cs
+++ b/UpsilonTests/UserDataTests.cs
@@ -26,6 +26,8 @@ namespace UpsilonTests
             public string FieldString = "TestField";
             public string FieldStringSet;
             private string _privateTestField = "hidden";
+            public bool GetOnly { get; } = false;
+            public bool PrivateSet { get; private set; } = false;
 
             public bool TestMethodHasRun { get; private set; }
             public void TestMethod()
@@ -121,5 +123,37 @@ end
             Assert.Single(script.Diagnostics.Messages);
         }
 
+        [Fact]
+        public void CantSetToFieldsWithNoSetter()
+        {
+            var obj = new UserDataHelper();
+            const string input = @"
+function test(o)
+    o.GetOnly = true
+end
+";
+            var script = new Script(input);
+            Assert.Empty(script.Diagnostics.Messages);
+            script.EvaluateFunction("test", new[] {obj});
+            Assert.Single(script.Diagnostics.Messages);
+            Assert.False(obj.GetOnly);
+        }
+
+        [Fact]
+        public void CantSetToFieldsWithPrivateSetter()
+        {
+            var obj = new UserDataHelper();
+            const string input = @"
+function test(o)
+    o.PrivateSet = true
+end
+";
+            var script = new Script(input);
+            Assert.Empty(script.Diagnostics.Messages);
+            script.EvaluateFunction("test", new[] {obj});
+            Assert.Single(script.Diagnostics.Messages);
+            Assert.False(obj.PrivateSet);
+        }
+
     }
 }
\ No newline at end of file