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

CS8795 Showing When it Shouldn't #58913

Closed
stap123 opened this issue Jan 18, 2022 · 22 comments
Closed

CS8795 Showing When it Shouldn't #58913

stap123 opened this issue Jan 18, 2022 · 22 comments
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@stap123
Copy link

stap123 commented Jan 18, 2022

Not sure if this is the right repo if not let me know and I can move.

When using the new LoggerMessage attribute we get errors all over the place even though this is working correctly. In case it's relevant we use partial classes to keep the individual messages close to the code that actually uses them rather than having a single huge Log class in one place.

There is also a similar/related report on Dev Communitey here: https://developercommunity.visualstudio.com/t/issue-when-using-loggermessage-and-static-extensio/1638197

Version:

VS2022 Current (17.0.5)

Steps to Reproduce:

Put this in a new console app and install the NuGet package Microsoft.Extensions.Logging you should see the error CS8795 Partial method ‘Log.Example(ILogger)’ must have an implementation part because it has accessibility modifiers.

using Microsoft.Extensions.Logging;

internal static partial class Log
{
    [LoggerMessage(
        EventId = 0,
        Level = LogLevel.Warning,
        Message = "A message",
        SkipEnabledCheck = true)]
    public static partial void Example(this ILogger logger);
}

Expected Behavior:

There should be no error shown as this is valid code because the attribute generates the code.

Actual Behavior:

The error is shown.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 18, 2022
@Youssef1313
Copy link
Member

Youssef1313 commented Jan 18, 2022

@stap123 Does the build output produce the same error? or it's only Error List?

If build output is correct, then I think this is an IDE issue that could being fixed in #58363

cc @sharwell Can you confirm if your PR will fix this?

@stap123
Copy link
Author

stap123 commented Jan 18, 2022

@Youssef1313 With the build output verbosity set to Minimal in VS2022 it does not show an error in the output window when the project is built.

It appears in the "Error List" tab as you suspected.

@Youssef1313
Copy link
Member

@stap123 In that case I think #58363 is likely to fix this.

@stap123
Copy link
Author

stap123 commented Jan 18, 2022

@Youssef1313 Do you think that will make it into 17.1?

@Youssef1313
Copy link
Member

The milestone set for the other-related issues are 17.2. But I have no idea if the PR can be merged faster and get into 17.1, or if it will get backported to 17.1.

@sharwell
Copy link
Member

sharwell commented Jan 18, 2022

I'm guessing this is the same issue as:
https://developercommunity.visualstudio.com/t/Code-generated-by-Source-Generator-is-ma/1636837

We don't yet understand why this is happening, but we are able to reproduce it and it's a high priority work item.

@stap123
Copy link
Author

stap123 commented Jan 18, 2022

Thanks guys, if I can help with anything or provide more information let me know.

@jasonmalinowski
Copy link
Member

This is likely fixed by #58990. @stap123 if you restart VS, does the problem go away, and future additions/removals of [LoggerMessage] methods work fine?

@jasonmalinowski
Copy link
Member

And to confirm, in your repro steps there's one missing step of "add the NuGet package Microsoft.Extensions.Logging"? This may seem like an obvious question but for the bug it actually matters. 😄

@stap123
Copy link
Author

stap123 commented Jan 24, 2022

And to confirm, in your repro steps there's one missing step of "add the NuGet package Microsoft.Extensions.Logging"? This may seem like an obvious question but for the bug it actually matters. 😄

You're correct, my bad. I've updated that

@stap123
Copy link
Author

stap123 commented Jan 24, 2022

This is likely fixed by #58990. @stap123 if you restart VS, does the problem go away, and future additions/removals of [LoggerMessage] methods work fine?

Yeah that does seem to solve the problem. I'll keep that in mind and try and see when the error shows up with a bit more context around this as I work today 😄

@jasonmalinowski
Copy link
Member

@stap123 OK, thanks for the extra info. I'm going to close then as fixed by #58990 then. The reason I asked about the NuGet package addition specifically was because the bug was manifesting if source generators changed. When you restart VS everything gets loaded the first time and so we'll have everything correct then.

The fix is on its way and targeting Preview 5, so hopefully you don't see this after that. If you still are, please reactivate!

@jasonmalinowski
Copy link
Member

@stap123 Have you had any chance to try Preview 5? If not, that's totally fine, but if you've upgraded and things are working better that'd be great to know.

@stap123
Copy link
Author

stap123 commented Feb 4, 2022

@jasonmalinowski It does seem to work in preview 5 following my repro steps 🍾

@jasonmalinowski
Copy link
Member

@stap123 Awesome, thanks for following up!

@stap123
Copy link
Author

stap123 commented Feb 18, 2022

@jasonmalinowski This is still happening in our main app after updating to VS17.1. I can't get it to repro in the sample using my original steps however. When I add a new partial class with new methods I get a CS8795 for each method and a IDE0060 for each parameter.

Can you think of anything that would be different between a new and existing app that would make this happen?

If I close and re-open VS no errors are shown

Once this error shows up in one file any other files that include source generated logger messages start to error once they are opened in an editor.

@sharwell
Copy link
Member

@stap123 This is probably caused by an exception in the generator (or the generator driver). Currently it's not possible to see those exceptions (#54098), which makes it difficult to fix. The generator also doesn't recover from those errors until you reload the project or restart the IDE (#58625).

@stap123
Copy link
Author

stap123 commented Feb 18, 2022

@sharwell Thanks for the info I will follow those issues and test when fixes are made 😃

@onionhammer
Copy link

I believe this is unfixed with the retiring of omnisharp

@sharwell
Copy link
Member

@onionhammer this bug is quite specific to Visual Studio (not VS Code). Can you file a new issue for the behavior you are seeing?

@onionhammer
Copy link

@sharwell looks like it's #65850

@thsackos
Copy link

thsackos commented Nov 8, 2024

Although not a great solution, I was able to work around the issue by removing the access modifier from the partial function and defining wrapper functions for code to call into:

internal static partial class Log
{
    public void Example(this ILogger logger) => logger.ExampleInternal();

    [LoggerMessage(
        EventId = 0,
        Level = LogLevel.Warning,
        Message = "A message",
        SkipEnabledCheck = true)]
    static partial void ExampleInternal(this ILogger logger);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

7 participants