Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serilog003 gives false positive on my custom abstraction #19

Closed
tiesmaster opened this issue Apr 14, 2017 · 7 comments · Fixed by #20
Closed

Serilog003 gives false positive on my custom abstraction #19

tiesmaster opened this issue Apr 14, 2017 · 7 comments · Fixed by #20

Comments

@tiesmaster
Copy link
Contributor

Hi, awesome analyzer, btw. I really like it. Though, I'm getting a false postive on the following line:

                int id = 1;
                _log.Error("The id is {Id}", id);

Where _log is an instance of our own abstraction ILogger (it might not be too valuable, but that's a different discussion), which looks like this:

    public interface ILogger
    {
        [MessageTemplateFormatMethod("messageTemplate")]
        void Debug(string messageTemplate);
        [MessageTemplateFormatMethod("messageTemplate")]
        void Debug(string messageTemplate, params object[] values);
        [MessageTemplateFormatMethod("messageTemplate")]
        void Error(string messageTemplate, params object[] values);
        [MessageTemplateFormatMethod("messageTemplate")]
        void Error(Exception exception, string messageTemplate, params object[] values);
        [MessageTemplateFormatMethod("messageTemplate")]
        void Information(string messageTemplate);
        [MessageTemplateFormatMethod("messageTemplate")]
        void Information(string messageTemplate, params object[] values);
        [MessageTemplateFormatMethod("messageTemplate")]
        void Warning(string messageTemplate, params object[] propertyValues);
        [MessageTemplateFormatMethod("messageTemplate")]
        void Warning(string messageTemplate);
    }

And with that, it will give me:

1>C:\src\duprix\DuPrix.Services\CompetitorPricing\RecommendationService.cs(124,39,124,43): error Serilog003: Error while binding properties: There is no argument that corresponds to the named property 'Id'

If I have time, I'll see if I can reproduce this in a unit test, and sent you a PR, if you're accepting.

@Suchiman
Copy link
Owner

My guess would be that its related to the way how i determine parameters:
https://github.com/Suchiman/SerilogAnalyzer/blob/master/SerilogAnalyzer/SerilogAnalyzer/DiagnosticAnalyzer.cs#L153
That makes me wonder right now if the code even works with params object[] (it checks Name.StartsWith("propertyValue") since in the generic overloads, they're numbered T1 propertyValue1, T2 propertyValue2)

@tiesmaster
Copy link
Contributor Author

Indeed, looks that way. If I have time this easter weekend, I'll take a stab at it. You're accepting PRs, right?

@Suchiman
Copy link
Owner

Of course 👍 much appreciated

tiesmaster added a commit to tiesmaster/SerilogAnalyzer that referenced this issue Apr 16, 2017
which just checks if the given parameter is "params", and in that case
adds the argument to be checked, as well.
tiesmaster added a commit to tiesmaster/SerilogAnalyzer that referenced this issue Apr 16, 2017
which just checks if the given parameter is "params", and in that case
adds the argument to be checked, as well.
@tiesmaster
Copy link
Contributor Author

@Suchiman A coworker of mine ran into this issue again the other day, so it would be nice if this fix would be released. Are you planning a release by any chance?

@Suchiman
Copy link
Owner

Suchiman commented May 3, 2017

Sorry, i've been slacking lately.
I will do a point release this evening to get the fix out.

@Suchiman
Copy link
Owner

Suchiman commented May 3, 2017

@tiesmaster I've just pushed https://github.com/Suchiman/SerilogAnalyzer/releases/tag/0.9.1 to nuget and visual studio gallery

@tiesmaster
Copy link
Contributor Author

Awesome! I'll update my repos straight away tomorrow. Thanks man!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants