New Rule : detect usages of Log.ForContext<ClassX> in class ClassY

[tsimbalar]

This has happened to me a few times already to find this kind of code, as a result of excessive copy/pasting :

public class SuperDuperClass{
    private static readonly ILogger Logger = Log.ForContext<AnotherUnrelatedClass>();
    // snip
}

i.e. cases where the SourceContext will end up not matching the actual containing class. This is quite annoying when you are, later on, trying to figure out the origin of a log event.

It would be nice if we could detect those mismatches and suggest a fix.

[Suchiman]

Indeed 👍 i've tripped on this on several occasions as well. Where should this apply? only on directly initialized private variables? What happens if multiple ILogger variables are declared and initialized?

[tsimbalar]

I would say limiting it to private static directly initialized variables covers the most common pattern, but when there are several I guess one can no longer guess the intent.

I'm not too sure about how it's done in the .NET Core / ASP.NET Core world with DI etc ... Do people still declare a static property or do they rely solely on constructor injection ? and in that case, at what point is the SourceContext specified ?

[Suchiman]

In ASP.NET Core, statics are most evil, so everything is constructor injected. So you demand an ILogger<class> and when it logs, the serilog adapter sets the SourceContext based on that class.

[tsimbalar]

@Suchiman oh ok. I wonder if it makes sense to have a similar check for those cases then ....

public class SuperDuperClass{
    public SuperDuperClass(ILogger<AnotherUnrelatedClass> logger)
    {
        //snip
    }
}

[nblumhardt]

Handy analysis! 👍

I don't always use static, but in Serilog code often have per-instance loggers:

    readonly ILogger _log = Log.ForContext<X>();

so I think checking either static or instance fields would be useful :-)

The ASP.NET case might be a good one for https://github.com/aspnet/Logging/tree/dev/src/Microsoft.Extensions.Logging.Analyzers ( @pakrym was looking for analysis suggestions a few weeks ago).

[tsimbalar]

Thanks !