New Rule : dictionaries must be concrete type Dictionary<TKey,TValue> and not an interface

[tsimbalar]

From Serilog's documentation :

Serilog also recognises Dictionary<TKey,TValue>, as long as the key type is one of the scalar types listed above.

... IDictionary<TKey,TValue> - objects implementing dictionary interfaces are not serialised as dictionaries. Firstly because it is less efficient in .NET to check for generic interface compatibility, and second because a single object may implement more than one generic dictionary interface, creating an ambiguity.

What this means is that to be properly destructured, dictionary parameters must be of the concrete Dictionary<TKey, TValue> type, so it would be useful to show a warning and a code fix for cases where a IDictionary<TKey, TValue> or a IReadOnlyDictionary<TKey, TValue> is provided.

The fix would be, either to downcast to Dictionary<TKey, TValue> if that makes sense, or to suggest adding the @ destructuring operator (I'm not sure that second fix is a good idea, though).

Just to be sure I didn't say anything silly or wrong : @nblumhardt am I correct with my assumptions ? :)

[Suchiman]

Sounds reasonable but thinking about it...

I'd assume most cases of IDictionary<TKey, TValue> are actually Dictionary<TKey, TValue>. For cases where its not, i cannot judge if the custom type implements a reasonable ToString() that is more right than using @. Also if i understand the documentation correctly, IDictionary<TKey, TValue> is never serialized like Dictionary<TKey, TValue>, even with @.

Adding an explicit upcast cast does nothing, since checking for Dictionary<TKey, TValue> is a runtime check anyways, but clutter the code and introduce a possible runtime exception (which goes against Serilogs design to never throw exceptions).

Awaiting final judgement from @nblumhardt

[tsimbalar]

Oh, maybe I didn't understand the documentation properly ...

I had understood that this is fine

Dictionary<string, string> d = new Dictionary<..,..>() ;
Logger.Debug("{Data}", d)

... but this is not fine :

IDictionary<string, string> d = new Dictionary<..,..>() ;
Logger.Debug("{Data}", d)

... but if it's a runtime check, indeed, that is irrelevant :p

Also, one option for the codefix would be to suggest passing a new Dictionary<X,Y>(myThingThatImplementsIDictionaryOfXY) instead of the direct object.

[nblumhardt]

Yep, this is a runtime check, so probably not something SerilogAnalyzer can help with 👍

[tsimbalar]

Ok, closing the issue, thanks ! 👍