PersistentVariablesSource can process nullable operator (#259)

1. PersistentVariablesSource can process nullable operator ?. 2. Added method Source.TrySetResultForNullableOperator(ISelectorInfo selectorInfo) Call this method from all ISources except DefaultSource
axuno · Mar 9, 2022 · b5f2d42 · b5f2d42
1 parent 865f49a
commit b5f2d42
Show file tree

Hide file tree

Showing 10 changed files with 111 additions and 65 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -166,7 +166,7 @@ var pvs = new PersistentVariablesSource
     { "global", varGroup }
 };
 // Best to put it to the top of source extensions
-smart.AddExtensions(0, pvs);
+smart.InsertExtension(0, pvs);
 
 // Note: We don't need args to the formatter for PersistentVariablesSource variables
 _ = smart.Format(CultureInfo.InvariantCulture,

diff --git a/src/SmartFormat.Extensions.Newtonsoft.Json/NewtonsoftJsonSource.cs b/src/SmartFormat.Extensions.Newtonsoft.Json/NewtonsoftJsonSource.cs
@@ -26,12 +26,8 @@ public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
                 JValue jsonValue => jsonValue.Value,
                 _ => selectorInfo.CurrentValue
             };
-
-            if (current is null && HasNullableOperator(selectorInfo))
-            {
-                selectorInfo.Result = null;
-                return true;
-            }
+
+            if (TrySetResultForNullableOperator(selectorInfo)) return true;
 
             if (current is null) return false;
 

diff --git a/src/SmartFormat.Extensions.System.Text.Json/SystemTextJsonSource.cs b/src/SmartFormat.Extensions.System.Text.Json/SystemTextJsonSource.cs
@@ -26,11 +26,7 @@ public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
                 _ => selectorInfo.CurrentValue
             };
 
-            if (current is null && HasNullableOperator(selectorInfo))
-            {
-                selectorInfo.Result = null;
-                return true;
-            }
+            if (TrySetResultForNullableOperator(selectorInfo)) return true;
 
             if (current is not JsonElement element) return false;
 
@@ -63,4 +59,4 @@ public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
             return true;
         }
     }
-}
+}
diff --git a/src/SmartFormat.Tests/Extensions/PersistentVariableSourceTests.cs b/src/SmartFormat.Tests/Extensions/PersistentVariableSourceTests.cs
@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
-using System.Text;
 using System.Threading.Tasks;
 using NUnit.Framework;
 using SmartFormat.Core.Formatting;
@@ -228,6 +227,31 @@ public void PersistentVariablesSource_NameGroupPair()
             Assert.That(pvs.Group.ContainsKey("varName"), Is.True);
         }
 
+        [TestCase("not-null", "not-null")]
+        [TestCase(null, "")]
+        public void Should_Handle_null_for_Nullable(object? valueToUse, string expected)
+        {
+            if (valueToUse is "not-null")
+                valueToUse = new KeyValuePair<string, object?>("Anything", "not-null");
+
+            // The group with just one nullable variable
+            var varGroup = new VariablesGroup { { "NullableVar", new ObjectVariable(valueToUse) } };
+
+            var smart = new SmartFormatter();
+            smart.FormatterExtensions.Add(new DefaultFormatter());
+            var pvs = new PersistentVariablesSource
+            {
+                // top container's name
+                { "Global", varGroup }
+            };
+
+            smart.AddExtensions(new KeyValuePairSource());
+            smart.InsertExtension(0, pvs);
+
+            var result = smart.Format("{Global.NullableVar?.Anything}");
+            Assert.That(result, Is.EqualTo(expected));
+        }
+
         [Test]
         public void Format_Args_Should_Override_Persistent_Vars()
         {

diff --git a/src/SmartFormat/Core/Sources/Source.cs b/src/SmartFormat/Core/Sources/Source.cs
@@ -49,11 +49,36 @@ public virtual void Initialize(SmartFormatter smartFormatter)
         /// <remarks>
         /// The nullable operator '?' can be followed by a dot (like '?.') or a square brace (like '.[')
         /// </remarks>
-        protected virtual bool HasNullableOperator(ISelectorInfo selectorInfo)
+        private bool HasNullableOperator(ISelectorInfo selectorInfo)
         {
             return _smartSettings != null && selectorInfo.Placeholder != null &&
                    selectorInfo.Placeholder.Selectors.Any(s =>
                        s.OperatorLength > 1 && s.BaseString[s.OperatorStartIndex] == _smartSettings.Parser.NullableOperator);
         }
+
+        /// <summary>
+        /// If any of the <see cref="Placeholder"/>'s <see cref="Placeholder.Selectors"/> has
+        /// nullable <c>?</c> as their first operator, and <see cref="ISelectorInfo.CurrentValue"/>
+        /// is <see langword="null"/>, <see cref="ISelectorInfo.Result"/> will be set to <see langword="null"/>.
+        /// </summary>
+        /// <param name="selectorInfo"></param>
+        /// <returns>
+        /// <see langword="true"/>, if any of the <see cref="Placeholder"/>'s
+        /// <see cref="Placeholder.Selectors"/> has  nullable <c>?</c> as their first
+        /// operator, and <see cref="ISelectorInfo.CurrentValue"/> is <see langword="null"/>.
+        /// </returns>
+        /// <remarks>
+        /// The nullable operator '?' can be followed by a dot (like '?.') or a square brace (like '.[')
+        /// </remarks>
+        protected virtual bool TrySetResultForNullableOperator(ISelectorInfo selectorInfo)
+        {
+            if (HasNullableOperator(selectorInfo) && selectorInfo.CurrentValue is null)
+            {
+                selectorInfo.Result = null;
+                return true;
+            }
+
+            return false;
+        }
     }
 }
diff --git a/src/SmartFormat/Extensions/DictionarySource.cs b/src/SmartFormat/Extensions/DictionarySource.cs
@@ -19,11 +19,7 @@ public class DictionarySource : Source
         public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
         {
             var current = selectorInfo.CurrentValue;
-            if (current is null && HasNullableOperator(selectorInfo))
-            {
-                selectorInfo.Result = null;
-                return true;
-            }
+            if (TrySetResultForNullableOperator(selectorInfo)) return true;
 
             if (current is null) return false;
 
@@ -54,4 +50,4 @@ public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
             return true;
         }
     }
-}
+}
diff --git a/src/SmartFormat/Extensions/KeyValuePairSource.cs b/src/SmartFormat/Extensions/KeyValuePairSource.cs
@@ -22,11 +22,7 @@ public class KeyValuePairSource : Source
         /// <inheritdoc />
         public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
         {
-            if (selectorInfo.CurrentValue is null && HasNullableOperator(selectorInfo))
-            {
-                selectorInfo.Result = null;
-                return true;
-            }
+            if (TrySetResultForNullableOperator(selectorInfo)) return true;
 
             switch (selectorInfo.CurrentValue)
             {

diff --git a/src/SmartFormat/Extensions/PersistentVariablesSource.cs b/src/SmartFormat/Extensions/PersistentVariablesSource.cs
@@ -24,7 +24,7 @@ namespace SmartFormat.Extensions
     /// <para>The smart string should take the placeholder format like {groupName.variableName}.</para>
     /// <para>Note: <see cref="IVariablesGroup"/>s from args to SmartFormatter.Format(...) take precedence over <see cref="PersistentVariablesSource"/>.</para>
     /// </summary>
-    public class PersistentVariablesSource : ISource, IDictionary<string, VariablesGroup>
+    public class PersistentVariablesSource : Source, IDictionary<string, VariablesGroup>
     {
         /// <summary>
         /// Contains <see cref="VariablesGroup"/>s and their name.
@@ -217,13 +217,19 @@ public IEnumerator GetEnumerator()
         }
 
         /// <inheritdoc/>
-        public bool TryEvaluateSelector(ISelectorInfo selectorInfo)
+        public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
         {
-            // First, we test the current value
-            // If selectorInfo.SelectorOperator== string.Empty, the CurrentValue comes from an arg to the SmartFormatter.Format(...)
-            // IVariablesGroups from args have priority over PersistentVariablesSource
-            if (selectorInfo.CurrentValue is IVariablesGroup grp && TryEvaluateGroup(selectorInfo, grp)) 
-                return true;
+            switch (selectorInfo.CurrentValue)
+            {
+                case null when TrySetResultForNullableOperator(selectorInfo):
+                    return true;
+
+                // Next, we test the current value
+                // If selectorInfo.SelectorOperator == string.Empty, the CurrentValue comes from an arg to the SmartFormatter.Format(...)
+                // IVariablesGroups from args have priority over PersistentVariablesSource
+                case IVariablesGroup grp when TryEvaluateGroup(selectorInfo, grp):
+                    return true;
+            }
 
             if (TryGetValue(selectorInfo.SelectorText, out var group))
             {

diff --git a/src/SmartFormat/Extensions/ReflectionSource.cs b/src/SmartFormat/Extensions/ReflectionSource.cs
@@ -45,20 +45,15 @@ public class ReflectionSource : Source
         /// <inheritdoc />
         public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
         {
-            const BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public;
+
             var current = selectorInfo.CurrentValue;
 
-            if (current is null && HasNullableOperator(selectorInfo))
-            {
-                selectorInfo.Result = null;
-                return true;
-            }
+            if (TrySetResultForNullableOperator(selectorInfo)) return true;
 
             // strings are processed by StringSource
             if (current is null or string) return false; 
 
             var selector = selectorInfo.SelectorText;
-
             var sourceType = current.GetType();
 
             // Check the type cache, if enabled
@@ -79,6 +74,18 @@ public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
                 return false;
             }
 
+            if (EvaluateMembers(selectorInfo, selector, current, sourceType)) return true;
+
+            // We also cache failures so we don't need to call GetMembers again
+            if (IsTypeCacheEnabled) TypeCache[(sourceType, selector)] = (null, null);
+
+            return false;
+        }
+
+        private bool EvaluateMembers(ISelectorInfo selectorInfo, string selector, object current, Type sourceType)
+        {
+            const BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public;
+
             // Important:
             // GetMembers (opposite to GetMember!) returns all members, 
             // both those defined by the type represented by the current T:System.Type object 
@@ -96,21 +103,8 @@ public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
                         return true;
                     case MemberTypes.Property:
                     case MemberTypes.Method:
-                        MethodInfo? method;
-                        if (member.MemberType == MemberTypes.Property)
-                        {
-                            //  Selector is a Property which is not WriteOnly
-                            if (member is PropertyInfo { CanRead: true } prop)
-                                method = prop.GetGetMethod();
-                            else
-                                continue;
-                        }
-                        else
-                        {
-                            //  Selector is a method
-                            method = member as MethodInfo;
-                        }
-
+                        if (!TryGetMethodInfo(member, out var method)) continue;
+
                         //  Check that this method is valid -- it needs to return a value and has to be parameter-less:
                         //  We are only looking for a parameter-less Function/Property:
                         if (method?.GetParameters().Length > 0) continue;
@@ -126,10 +120,27 @@ public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
                         return true;
                 }
 
-            // We also cache failures so we don't need to call GetMembers again
-            if (IsTypeCacheEnabled) TypeCache[(sourceType, selector)] = (null, null);
-
             return false;
         }
+
+        private static bool TryGetMethodInfo(MemberInfo member, out MethodInfo? method)
+        {
+            if (member.MemberType == MemberTypes.Property)
+            {
+                //  Selector is a Property which is not WriteOnly
+                if (member is PropertyInfo { CanRead: true } prop)
+                {
+                    method = prop.GetGetMethod();
+                    return true;
+                }
+
+                method = null;
+                return false;
+            }
+
+            //  Selector is a method
+            method = member as MethodInfo;
+            return true;
+        }
     }
-}
+}
diff --git a/src/SmartFormat/Extensions/StringSource.cs b/src/SmartFormat/Extensions/StringSource.cs
@@ -71,11 +71,7 @@ private void AddMethods()
         /// <inheritdoc />
         public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
         {
-            if (selectorInfo.CurrentValue is null && HasNullableOperator(selectorInfo))
-            {
-                selectorInfo.Result = null;
-                return true;
-            }
+            if (TrySetResultForNullableOperator(selectorInfo)) return true;
 
             if (selectorInfo.CurrentValue is not string currentValue) return false;
             var selector = selectorInfo.SelectorText;
@@ -215,4 +211,4 @@ private static CultureInfo GetCulture(FormatDetails formatDetails)
             return CultureInfo.CurrentUICulture;
         }
     }
-}
+}