Report a dictionary value by its key

CHANGELOG.md

@@ -16,7 +16,7 @@ Breaking changes are marked **BREAKING**.
 
 - **BREAKING** The member names of an object in an item that is a collection. The validator now enumerates the items first, so the path no longer goes through a property of the item. Change any code that matches these names:
   - Array, multi-dimensional array, `ArrayList`: `Value[0].SyncRoot[0].Name` is now `Value[0][0].Name`.
-  - Dictionary, `Hashtable`: `Value[0].Values[0].Name` is now `Value[0][0].Value.Name`, and `Value[0].Keys[0].Name` is now `Value[0][0].Key.Name`.
+  - Dictionary, `Hashtable`: `Value[0].Values[0].Name` is now `Value[0][a].Name` for the key `a` (see the dictionary entry below), and `Value[0].Keys[0].Name` is now `Value[0][0].Key.Name` for a key that is an object.
   - `LinkedList<T>`: `Value[0].First.List[1].Name` is now `Value[0][1].Name`.
   - `SortedSet<T>`: `Value[0].Min.Name` is now `Value[0][0].Name`.
   - A collection of your own with a property that returns its items, such as `View`: `Value[0].View[0].Name` is now `Value[0][0].Name`.
@@ -37,6 +37,7 @@ Breaking changes are marked **BREAKING**.
 - The NuGet package title now reads "Recursive DataAnnotations Validation". It was misspelled "Recurisive".
 - The NuGet package now has a description. It showed "Package Description" before.
 - **BREAKING** A struct that the validator reaches by two routes is reported once where the validator can tell. For a collection that is an item or the root object, the properties that framework types declare, such as `ArraySegment<T>.Array`, `LinkedList<T>.First` and `Dictionary<TKey, TValue>.Values`, are no longer walked, as for a collection that a property holds. A collection that a second route reaches is enumerated again, so an item added in the meantime is found, but its struct items are not reported again. Change any code that counts the results: `Value[0].Array[0].Text`, `Value[0].First.Value.Text` and `Value[0].Values[0].Text` are gone, and so is `Second[0].Text` for an array that `First` also holds. An object that only such a property reached is no longer validated, such as an item of the array outside an `ArraySegment<T>`, or the comparer of a dictionary. A jagged array of simple values, such as `int[][]`, is no longer enumerated, because it cannot hold an invalid object.
+- **BREAKING** A value of a dictionary is reported by its key, in the form that MVC model binding uses: `Map[0].Value.Name` is now `Map[Primary].Name`, and `[0].Value.Name` for a root dictionary is now `[Primary].Name`. This holds for any type that implements `IDictionary`, `IDictionary<TKey, TValue>` or `IReadOnlyDictionary<TKey, TValue>`, when the key is a string, a primitive, an enum or a struct that formats itself, such as a `decimal` or a `Guid`, and has nothing to validate. The key is formatted with the invariant culture and is not escaped. Any other key, such as an object, keeps the old form for that entry: `Map[0].Key.Name` and `Map[0].Value.Name`. A value is now one level below the dictionary, as an item of a list is, so it can come earlier in the results and counts one level less for the maximum depth. Change any code that matches these names.
 - Known limit: two properties that each return the same struct, such as a property and a computed copy of it, report it twice, with two member names. The validator cannot match two copies of a struct by reference. Objects of a class are reported once.
 - README: say what `Validator.TryValidateObject` leaves out, show the usage as C#, and tell the `Tuple` classes, which are walked, from value tuples, which are not.
 

README.md

@@ -59,13 +59,21 @@ An object that several paths reach is validated once, and its errors name the sh
 ### Collections
 
 - The items of a collection that a property holds are validated. The error names the property and the index of the item: `Items[1].Name`.
-- An item that is itself a collection is validated too, at each level: `Matrix[0][2].Name`. This holds for lists, arrays, sets, dictionaries and your own collection types. A dictionary is enumerated as `KeyValuePair` items, so a value that is an object is reported as `Map[0].Value.Name`. A value that is a struct is walked when it has something to validate, as it is when a property holds it (see "Structs" below).
+- An item that is itself a collection is validated too, at each level: `Matrix[0][2].Name`. This holds for lists, arrays, sets, dictionaries and your own collection types. A value of a dictionary is reported by its key (see "Dictionaries" below). A value that is a struct is walked when it has something to validate, as it is when a property holds it (see "Structs" below).
 - An item that is a collection is validated as an object first, so its own attributes and `IValidatableObject.Validate` run, then its items. The properties that a framework type declares on it, such as `ArraySegment<T>.Array`, `LinkedList<T>.First` or `Dictionary<TKey, TValue>.Values`, are not walked, because they repeat the items. A collection that a property holds is treated the same way. The properties that your own collection type adds are walked.
 - A collection of simple values, such as `List<int>` or `string[]`, is not enumerated, because it cannot hold an invalid object.
 - A collection that you pass to the validator as the root object is validated as an object, then its items are validated. The error starts with the index of the item: `[1].Name`.
 - A collection that is a struct, such as `ImmutableArray<T>`, is validated when a property holds it and when it is an item of another collection: `Lines[0].Sku`.
 - The validator runs each collection it enumerates, so a lazy sequence, such as a LINQ query or an iterator, runs during validation. A sequence that never ends makes validation hang. The validator does not catch exceptions, so an exception that a collection throws when it is enumerated reaches your code. A struct collection that is its default value, such as an `ImmutableArray<T>` or an `ArraySegment<T>` that nobody set, is skipped when it is an item or when the property is declared as that struct, because it holds nothing and enumerating it throws. The validator compares the memory of the struct with its default. It does not call the `Equals` of your struct. A property declared as an interface or as `object` is always enumerated.
 
+### Dictionaries
+
+- A value of a dictionary is reported by its key: `Endpoints[Primary].Url`, `Ports[8080].Name`, `ByDay[Monday].Name`. This is the form that MVC model binding uses for a dictionary. A dictionary is a type that implements `IDictionary`, `IDictionary<TKey, TValue>` or `IReadOnlyDictionary<TKey, TValue>`. A list of `KeyValuePair` items is not one, so it keeps `Pairs[0].Value.Name`.
+- A key names its value when it is a string, a number or another primitive, an enum, or a struct that formats itself, such as a `decimal`, a `Guid` or a `DateTime`, and it has nothing to validate. The key is formatted with the invariant culture, so a `decimal` key of 1.5 is `Map[1.5]` on every server.
+- The key is not quoted or escaped. A key that contains `]` or `.`, such as `b.c[0]`, gives a path that you cannot tell apart from a deeper one: `Map[b.c[0]].Name`.
+- Any other key, such as an object of your own, keeps the position of the entry, so the key is validated too: `Map[0].Key.Name` and `Map[0].Value.Name`. The position is the order in which the dictionary enumerates its entries. Each entry chooses its own form.
+- A value is one level below the dictionary, as an item of a list is, for the maximum depth and the order of results.
+
 ### Structs
 
 - A struct that a property holds is walked when it has something to validate: a validation attribute on the struct or on one of its properties, `IValidatableObject`, or a property that holds an object, such as the `Value` of a `KeyValuePair<string, Child>`. The error names the path, as for a class: `Price.Amount`, `Pair.Value.Name`.

src/RecursiveDataAnnotationsValidation/Extensions/TypeExtensions.cs

@@ -5,6 +5,7 @@
 using System.ComponentModel;
 using System.ComponentModel.DataAnnotations;
 using System.Diagnostics;
+using System.Globalization;
 using System.IO;
 using System.Linq;
 using System.Reflection;
@@ -38,6 +39,12 @@ internal static class TypeExtensions
         private static readonly ConcurrentDictionary<Type, bool> UnsafeToWalkCache =
             new ConcurrentDictionary<Type, bool>();
 
+        private static readonly ConcurrentDictionary<Type, bool> DictionaryTypeCache =
+            new ConcurrentDictionary<Type, bool>();
+
+        private static readonly ConcurrentDictionary<Type, (PropertyInfo Key, PropertyInfo Value)> KeyValuePairProperties =
+            new ConcurrentDictionary<Type, (PropertyInfo Key, PropertyInfo Value)>();
+
         // Framework types whose properties throw, wait, or never end, when the validator walks them.
         // They carry no validation attributes. Only the properties these types and their framework
         // subclasses declare are skipped, so a user subclass still has its own properties walked.
@@ -169,6 +176,71 @@ public static List<PropertyInfo> GetWalkedProperties(this Type type)
                 .ToList();
         }
 
+        /// <summary>
+        /// True for a dictionary: a type that implements IDictionary, IDictionary&lt;TKey, TValue&gt; or
+        /// IReadOnlyDictionary&lt;TKey, TValue&gt;. The validator reports a value of a dictionary by its key.
+        /// A list of KeyValuePair items is not a dictionary.
+        /// </summary>
+        public static bool IsDictionaryType(this Type type)
+        {
+            return DictionaryTypeCache.GetOrAdd(type, t =>
+                typeof(IDictionary).IsAssignableFrom(t)
+                || t.GetInterfaces().Any(i => i.IsGenericType
+                    && (i.GetGenericTypeDefinition() == typeof(IDictionary<,>)
+                        || i.GetGenericTypeDefinition() == typeof(IReadOnlyDictionary<,>))));
+        }
+
+        /// <summary>
+        /// Splits an entry that a dictionary yields, a KeyValuePair&lt;TKey, TValue&gt; or a
+        /// DictionaryEntry, into its key and value. False for any other item.
+        /// </summary>
+        public static bool TryGetDictionaryEntry(this object item, out object key, out object value)
+        {
+            if (item is DictionaryEntry entry)
+            {
+                key = entry.Key;
+                value = entry.Value;
+                return true;
+            }
+
+            var type = item.GetType();
+            if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(KeyValuePair<,>))
+            {
+                var properties = KeyValuePairProperties.GetOrAdd(type, t => (t.GetProperty("Key"), t.GetProperty("Value")));
+                key = properties.Key.GetValue(item, null);
+                value = properties.Value.GetValue(item, null);
+                return true;
+            }
+
+            key = null;
+            value = null;
+            return false;
+        }
+
+        /// <summary>
+        /// The text that names a dictionary value in a path, such as "Primary" in Map[Primary].Name,
+        /// or null for a key that cannot name it. A key can when it has nothing to validate (see
+        /// <see cref="IsLeafType"/>) and is a string, a primitive, an enum, or a struct that formats
+        /// itself, such as a decimal, a Guid or a DateTime. The text uses the invariant culture, so a
+        /// path does not change with the culture of the thread. It is not escaped, as in the names
+        /// that MVC model binding uses for a dictionary, so a key that contains "]" or "." can make a
+        /// path ambiguous.
+        /// </summary>
+        public static string GetKeyText(this object key)
+        {
+            var type = key.GetType();
+            var isSimple = type == typeof(string)
+                || type.IsPrimitive
+                || type.IsEnum
+                || (type.IsValueType && key is IFormattable);
+
+            if (!isSimple || !type.IsLeafType()) return null;
+
+            return key is IFormattable formattable
+                ? formattable.ToString(null, CultureInfo.InvariantCulture)
+                : key.ToString();
+        }
+
         /// <summary>
         /// True when the type's Equals(object) is not object.Equals, so two different objects
         /// of the type can be equal. Records, structs (through ValueType.Equals) and classes

src/RecursiveDataAnnotationsValidation/RecursiveDataAnnotationValidator.cs

@@ -197,14 +197,11 @@ public bool Run(object root)
                 return valid;
             }
 
-            //Queues an object that was found at the end of a path. The path is the parent path plus a
-            //property name, or plus an index if the name is null. A path is a chain of steps, and it is
-            //turned into a string only for an object that has a result.
+            //Queues an object that was found at the end of a path. A path is a chain of steps, and it
+            //is turned into a string only for an object that has a result.
             private void Enqueue(
                 object value,
-                PathStep parent,
-                string propertyName,
-                int index,
+                PathStep path,
                 int depth,
                 EqualsAncestor ancestors
                 )
@@ -227,7 +224,7 @@ EqualsAncestor ancestors
 
                 queue.Enqueue(new WorkItem(
                     value,
-                    new PathStep(parent, propertyName, index),
+                    path,
                     depth,
                     ancestors,
                     false,
@@ -246,20 +243,47 @@ EqualsAncestor ancestors
                 )
             {
                 var enumeratedBefore = !enumeratedCollections.Add(items);
+                var isDictionary = items.GetType().IsDictionaryType();
 
                 var arrayIndex = -1;
                 foreach (var item in items)
                 {
                     arrayIndex++;
+                    if (item == null) continue;
 
-                    //NOTE: Possibly should have a separate case for Dictionary which reports on the key
+                    //A value of a dictionary is reported by its key, as in Map[Primary].Name, when the
+                    //key can name it (see GetKeyText). Otherwise the whole entry is an item, as in any
+                    //collection, so a key that is an object is validated too: Map[0].Key.Name.
+                    var queued = item;
+                    PathStep path = null;
+                    if (isDictionary && item.TryGetDictionaryEntry(out var key, out var value) && key != null)
+                    {
+                        var keyText = key.GetKeyText();
+                        if (keyText != null)
+                        {
+                            if (value == null) continue;
+                            queued = value;
+                            path = PathStep.ForKey(collectionPath, keyText);
+                        }
+                    }
 
-                    if (item == null) continue;
-                    if (enumeratedBefore && item.GetType().IsValueType) continue;
-                    Enqueue(item, collectionPath, null, arrayIndex, itemDepth, ancestors);
+                    if (enumeratedBefore && HasNoIdentity(queued)) continue;
+                    Enqueue(queued, path ?? new PathStep(collectionPath, null, arrayIndex), itemDepth, ancestors);
                 }
             }
 
+            //True for a struct, which queuedObjects cannot match by reference (see enumeratedCollections).
+            //A dictionary entry is a struct too, but a key or value of a class in it has an identity.
+            private static bool HasNoIdentity(object item)
+            {
+                if (!item.GetType().IsValueType) return false;
+
+                return !item.TryGetDictionaryEntry(out var key, out var value)
+                    || !(IsOfAClass(key) || IsOfAClass(value));
+            }
+
+            private static bool IsOfAClass(object value) => value != null && !value.GetType().IsValueType;
+
             //Walks an object: validates it, then queues what it holds. Returns false if the object has a result.
             private bool Walk(WorkItem item)
             {
@@ -353,7 +377,7 @@ private bool Walk(WorkItem item)
                             break;
 
                         default:
-                            Enqueue(value, item.Path, property.Name, -1, item.Depth + 1, ancestors);
+                            Enqueue(value, new PathStep(item.Path, property.Name, -1), item.Depth + 1, ancestors);
                             break;
                     }
                 }
@@ -481,14 +505,26 @@ private sealed class PathStep
             private readonly string propertyName;
             private readonly int index;
 
+            //the text of a dictionary key, which takes the place of the index
+            private readonly string key;
+
             public PathStep(PathStep parent, string propertyName, int index)
             {
                 this.parent = parent;
                 this.propertyName = propertyName;
                 this.index = index;
             }
 
-            //"Orders[0].Lines[1].Product": a name after the first one is joined with a dot, and an index with none
+            private PathStep(PathStep parent, string key)
+            {
+                this.parent = parent;
+                this.key = key;
+            }
+
+            public static PathStep ForKey(PathStep parent, string key) => new PathStep(parent, key);
+
+            //"Orders[0].Lines[1].Product", "Endpoints[Primary].Url": a name after the first one is
+            //joined with a dot, and an index or a key with none
             public override string ToString()
             {
                 var steps = new List<PathStep>();
@@ -503,7 +539,17 @@ public override string ToString()
                     var step = steps[i];
                     if (step.propertyName == null)
                     {
-                        path.Append('[').Append(step.index).Append(']');
+                        path.Append('[');
+                        if (step.key != null)
+                        {
+                            path.Append(step.key);
+                        }
+                        else
+                        {
+                            path.Append(step.index);
+                        }
+
+                        path.Append(']');
                         continue;
                     }