Skip to content

Analyzers for v2#45

Merged
ltrzesniewski merged 15 commits intov2from
v2-analyzers
Jan 22, 2022
Merged

Analyzers for v2#45
ltrzesniewski merged 15 commits intov2from
v2-analyzers

Conversation

@ltrzesniewski
Copy link
Member

@ltrzesniewski ltrzesniewski commented Jan 16, 2022

I wanted to play a bit with Roslyn, and ZeroLog was a good candidate for some analyzers 🙂

  • Implicitly discarding a log message (if you forget to call .Log() for instance) will cause an error:

    image

    This works just like if every method that returns a LogMessage (even user-defined ones) were marked with [MustUseReturnValue]. You need to discard the message explicitly if you intend to do so, for instance in helper methods:

    private static void AppendSomething(LogMessage message)
    {
        if (DateTime.Today.DayOfWeek == DayOfWeek.Monday)
            _ = message.Append(" :'( ");
    }
  • Passing a string interpolation to ZeroLog in language versions below C# 10 will cause an error a warning:

    image

    In older language versions, this would call string.Format and allocate. This screenshot is from VS, I don't know why Rider doesn't display the squiggles, but you'll still get a build error warning.

    Note that in multi-targeted projects, you'll get an older language version by default for targets older than .NET 6.

    I suppose you really don't want to allocate if you use ZeroLog, but maybe this should be a warning instead? 🤔

  • You'll get a warning if you reference ZeroLog in a project that uses a language version below C# 10. Maybe that's a bit too harsh, I don't know... That's probably redundant with the previous check.

  • The build will fail if you use ZeroLog with an SDK older than .NET 6 (or a Visual Studio older than 2022), since the analyzers wouldn't run.

This will merge into #44, but I opened a separate PR for feedback. Also, feel free to suggest a better wording for the error messages. 🙂

@ltrzesniewski
Copy link
Member Author

Here's why the squiggles don't appear in Rider: https://youtrack.jetbrains.com/issue/RIDER-73394

@ltrzesniewski
Copy link
Member Author

ltrzesniewski commented Jan 20, 2022

I added a code fix that converts automatically from this:

private static void Test()
{
    _log.Info()
        .Append("Foo ")
        .Append(Guid.NewGuid(), "B")
        .Append(" Bar")
        .Log();
}

to this:

private static void Test()
{
    _log.Info($"Foo {Guid.NewGuid():B} Bar");
}

🙂

It needs a bit more work though.

@ltrzesniewski ltrzesniewski marked this pull request as draft January 21, 2022 08:32
@ltrzesniewski ltrzesniewski marked this pull request as ready for review January 22, 2022 12:06
@ltrzesniewski ltrzesniewski merged commit 2c8ee7e into v2 Jan 22, 2022
@ltrzesniewski ltrzesniewski deleted the v2-analyzers branch January 22, 2022 15:09
@ltrzesniewski ltrzesniewski added this to the v2.0 milestone May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant