LibraryImport causes CS8795

[FlsPrpht]

Version Used:
VS 17.10.5
.NET Runtime is 8.0.7
.NET SDK - 8.0.303.

Steps to Reproduce:
I cannot reproduce this with a newly created project and have been unable to deduce the difference between my app and the same app.

namespace CS8795
{
    using System;
    using System.Runtime.InteropServices;

    internal static partial class NativeMethods
    {
        [LibraryImport("user32.dll")]
        [return: MarshalAs(UnmanagedType.Bool)]
        internal static partial Boolean IsIconic(IntPtr hwnd);
    }
}

Diagnostic Id:

error CS8795: Partial method 'NativeMethods.IsIconic(nint)' must have an implementation part because it has accessibility modifiers.

Expected Behavior:
Per dotnet/vscode-csharp#5999 we should not be seeing this anymore

Actual Behavior:
I am seeing

error CS8795: Partial method 'NativeMethods.IsIconic(nint)' must have an implementation part because it has accessibility modifiers.

[huoyaoyuan]

This is because the library import generator doesn't run successfully. It doesn't "cause" the error, the absence of it causes.

The linked issue from vscode-csharp is unrelated if you are not using vscode.

[FlsPrpht]

I am not using VS Code but the full Visual Studio IDE so that linked issue is perhaps unrelated, but the symptom is the same. Perhaps the analyzer needs a better message to describe how to fix the problem no matter what causes it?

[huoyaoyuan]

Perhaps the analyzer needs a better message to describe how to fix the problem no matter what causes it?

There is a chance that the generator hasn't been invoked at all.

To better diagnose this, you may collect: whether this reproduces on command line build, the msbuild binlog, and whether Project->Dependencies->Analyzers in Visual Studio shows Microsoft.Interop.LibraryImportGenerator.

[FlsPrpht]

I can provide the list of analyzers here

What level of verboseness should I enable to see more on the command line than what I am seeing in VS? I can see the same error messages when I run dotnet build.

[huoyaoyuan]

I don't have more ideas about this. The statement is very simple and unlikely to cause the generator to crash or get rejected by it, assuming you are not seeing an AD0001/SYSLIB warning. The generator ships inbox with SDK and is unlikely to be misconfigured. The internal behavior of roslyn isn't visible from msbuild logs.

Usually expanding the Microsoft.Interop.LibraryInteropGenerator you will see generated source and be able to view it. Since it's failing for you, you should be seeing nothing.

[FlsPrpht]

Oh interesting, I never would have thought to expand that and look for warnings/generated code there! Thank you! I am indeed seeing SYSLIB errors but I am also seeing generated code as well.

Here is the generated code. You can see right off the bat, the first warning about using LibraryImportAttribubte instead of DllImportAttribute is accurate (the generated code is using DllImport)

internal static unsafe partial class NativeMethods
{
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.LibraryImportGenerator", "8.0.10.31311")]
[global::System.Runtime.CompilerServices.SkipLocalsInitAttribute]
internal static partial bool IsIconic(nint hwnd)
{
bool __retVal;
int __retVal_native;
{
__retVal_native = __PInvoke(hwnd);
}
// Unmarshal - Convert native data to managed data.
__retVal = __retVal_native != 0;
return __retVal;
// Local P/Invoke
[global::System.Runtime.InteropServices.DllImportAttribute("user32.dll", EntryPoint = "IsIconic", ExactSpelling = true)]
static extern unsafe int __PInvoke(nint __hwnd_native);
}
}

[huoyaoyuan]

I am indeed seeing SYSLIB errors but I am also seeing generated code as well.

Those are the warning IDs that the generator/analyzer is capable to produce (it registers to roslyn). If you they produce any warnings, you will see them together with other code warning/errors.

So the generator is able to generate corresponding code, but for some reason the definition and generated parts aren't recognized and matched together.

[jasonmalinowski]

@FlsPrpht If you're seeing the same errors in dotnet build, then run dotnet build /bl and share the log. That either means the generator isn't running, or your code isn't correct in the first place.

[FlsPrpht]

@jasonmalinowski Hi Jason, thank you for the reply. Can I send the log to your email account directly? I was unable to reproduce this in a newly created trivial project and don't want to share details about my project to the larger internet world.

When I run exactly dotnet build /bl I see the exact same error message (and some C++ errors unrelated to this issue).

C:\git<redacted>\NativeMethods.cs(24,41): error CS8795: Partial method 'NativeMethods.IsIconic(n
int)' must have an implementation part because it has accessibility modifiers. [C:\git<redacted>.csproj]

I can send the more detailed log with detailed logs to your email address if that would be helpful.

[jasonmalinowski]

@FlsPrpht Absolutely! My Microsoft email is in my profile.

And yes I should have explained more: when you run dotnet build /bl, that creates an msbuild.binlog file in the same folder which is a log of the build that will contain a lot more information for us to investigate with.

[jasonmalinowski]

Moving this over to dotnet/wpf. @FlsPrpht privately shared a binlog which indicated a bit more what is going on here: when the WPF project is being built, it's producing a wpftmp.csproj which runs an inner build of the project. For some reason, this project doesn't have any source generators included, so the interop generator isn't even added, therefore causing the build to fail.

I don't understand why this happening though. #6680 was implemented to ensure source generators do make it to the inner build, and I can see the analyzers correctly being passed to the build task that produces the .csproj. But the wpftmp.csproj project in the binlog doesn't show the analyzers actually made it there. So I'm guessing there's some bug where with the customer's project or targets something is going wrong with the editing and it fails in this way. I'm unable to reproduce it with a new project (@FlsPrpht said the same) so there's more to the story.

[ThomasGoulet73]

@FlsPrpht Could you try with a source generator other than LibraryImport ? You could try with GeneratedRegex, something like this:

internal static partial class Test1
{
    [GeneratedRegex(".*")]
    internal static partial Regex Test2();
}

Could you also try with a source generator coming from a Nuget. You could try with CommunityToolkit.Mvvm, you just need to install the nuget and add something like this to your project:

internal partial class ViewModel : ObservableObject
{
    [ObservableProperty]
    public int _test1;

    public ViewModel()
    {
        var m1 = Test1;
    }
}

I just want to make sure that the problem is not specific to the LibraryImport source generator.

[jasonmalinowski]

@ThomasGoulet73 The binlogs showed all analyzers being removed, not just that one. I'd expect any other generator to have the same problem.

[FlsPrpht]

@jasonmalinowski @ThomasGoulet73 Thank you both for your help on this. I have performed the experiments suggested by ThomasGoulet73.

The first, use another code generator such as [GeneratedRegex(".*")], resulted in the same error as predicted by jasonmalinowski.

error CS8795: Partial method 'Test1.Test2()' must have an implementation part because it has accessibility modifiers.

The second however, use a generator from nuget such as CommunityToolkit.Mvvm appeared to succeed.

[jasonmalinowski]

I'd imagine the CommunityToolkit.Mvvm one works only if you're not calling any of the generated methods in your code -- add some calls there and you'll get errors about the missing methods.

[ThomasGoulet73]

The binlogs showed all analyzers being removed, not just that one. I'd expect any other generator to have the same problem.

@jasonmalinowski Were you able to see in the binlog where the analyzers were removed ? I don't have access to the binlog and I'm not sure @FlsPrpht would be comfortable to send me the binlog since I don't work for Microsoft. My test with GeneratedRegex was mainly to make sure that it wasn't related to LibraryImport and my test with CommunityToolkit.Mvvm was because it is included differently when coming from a Nuget vs coming from the SDK.

I'd imagine the CommunityToolkit.Mvvm one works only if you're not calling any of the generated methods in your code -- add some calls there and you'll get errors about the missing methods.

Yes the line var m1 = Test1; in the constructor in my example is important. It makes sure to use a property that is generated. @FlsPrpht: I just want to double check that you included it in your test.

[jasonmalinowski]

The problem is in the task/target GenerateTemporaryTargetAssembly. The list of analyzers is being passed to the task correctly:

(the redaction is just for analyzers that aren't inbox ones)

But for whatever reason, none of the analyzer items are created in the resulting project. The binlog contains the generated project and there's everything else you expect to see (Compile, ReferencePath)...but just no analyzers.

[ThomasGoulet73]

You mean if you double click on the *_wpftmp.csproj in the binlog you don't see the corresponding <Analyzer> items ? That does seem weird.

[ThomasGoulet73]

Could I bother you to also check the value of the property IncludePackageReferencesDuringMarkupCompilation in the binlog ?

[jasonmalinowski]

You mean if you double click on the *_wpftmp.csproj in the binlog you don't see the corresponding items ? That does seem weird.

Correct. The code in the PR looks pretty straightforward, so not sure how it could happen.

IncludePackageReferencesDuringMarkupCompilation

This property is set to false. Strangely, wpftmp.csproj still has PackageReferences in it.

[ThomasGoulet73]

This property is set to false.

That's the problem: it needs to be true for source generators to work. It is enabled by default since .Net 6.0 but it was probably set to false in the project that has the issue which explains why we can't reproduce it in a new project.

If I create a new WPF project, set IncludePackageReferencesDuringMarkupCompilation to false and add the repro code I can reproduce the "issue".

[jasonmalinowski]

Ah for some reason I tried that when you mentioned it an hour ago and it worked for me, forcing a clean build fails now. So yes, @FlsPrpht seems like the answer is to delete that line in your .props file.

@ThomasGoulet73 So why does that option still exist at this point? I'm also a bit more confused by the history here -- I see #3846 was originally merged that introduced the split of the two different methods, but your change #6534 would have made it even work. I guess it seems like the first PR never would have worked, but also seems like it was focused on passing along PackageReferences when the real thing would have been the Analyzer elements in the first place.

[ThomasGoulet73]

AFAIK the switch was added to allow projects to opt-in on this new mode of XAML compilation that allowed source generator. Then for .Net 6.0 it was changed to be opt-out and was enabled by default since then. IIRC my PR only fixed source generators coming from targeting packs and I believe other source generators (PackageReference and ProjectReference) were working before my PR. The XAML compilation is all around very confusing and looks like it's held together by duct tape. I've been planning on cleaning this up for awhile but just haven't gotten around to it.

So why does that option still exist at this point?

I agree that we should probably remove it or at least rename it to maybe something like _IncludePackageReferencesDuringMarkupCompilation to allow people that disabled it to transition to the new mode that was the default since two major .Net releases. There were bug fixes in the last 2 major versions that probably fixed some of the reasons people may have opted-out.

@dotnet/wpf-developers: Should we remove or rename the switch ?

[FlsPrpht]

Bingo guys that was it! Thank you both @jasonmalinowski and @ThomasGoulet73!

Does IncludePackageReferencesDuringMarkupCompilation = false do anything useful at this point? I'm not sure why we had it set to false TBH. I searched our enterprise github instance and found a half dozen projects with it set to false and I'd like to advise the owners of those repos accurately.

Also for completeness I did have the var m1 = Test1; in my CommunityToolkit.Mvvm test.

[jasonmalinowski]

@FlsPrpht I imagine at this point you'd only need it as a workaround for something else. Any chance source control history for you might give a hint when it was added? Maybe it was some point-in-time workaround for a bug we since fixed.

[jasonmalinowski]

IIRC my PR only fixed source generators coming from targeting packs and I believe other source generators (PackageReference and ProjectReference) were working before my PR.

I would imagine your PR would have potentially made everything work; any analyzers coming from Package/Project references would have been converted to Analzyer items and passed along by your change as well.

[ThomasGoulet73]

@FlsPrpht: At this point I would consider IncludePackageReferencesDuringMarkupCompilation = false to be obsolete but in the past when we worked on IncludePackageReferencesDuringMarkupCompilation = true and there were bugs, we often suggested setting IncludePackageReferencesDuringMarkupCompilation to false to workaround the bug (Which is probably the reason you turned it off in your projects). Unfortunately I can't help you pinpoint why it was set it to false, if you want to find the reason you might want to check this list of closed issues that references IncludePackageReferencesDuringMarkupCompilation and maybe check the date it was added in your projects and see if an issue is around that time.

What I'll say is that from experience 99% of the problems that were caused by having IncludePackageReferencesDuringMarkupCompilation set to true were happening at build time and not run time. So what I would suggest is to just try enabling it in your projects and see if it builds.

[FlsPrpht]

@ThomasGoulet73 that makes sense. I looked in our history and we enabled it when migrating from VS 2019 to VS 2022. Based on the comment of the checkin, these were indeed compile time fixes. Things build just fine now with it set to true.

With can close this issue, unless either of you want to take further action regarding the IncludePackageReferencesDuringMarkupCompilation property.

[jasonmalinowski]

Things build just fine now with it set to true.

@FlsPrpht: as a small suggestion if you didn't do it this way, since the default is now 'true', feel free to just delete the line rather than change it. That way if we add any other logic at some point where it needs to be on or off at some point, you won't break again.

[FlsPrpht]

Thanks @jasonmalinowski deleting it was my plan.

[jasonmalinowski]

@dotnet/dotnet-wpf-triage Anything we still want to track for follow up from this issue or should we close it?