Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

It seems Copilot could not make a fix before timing out.

@danmoseley I had copilot crank locally for quite a while and it couldn't figure it out but it did add some regression tests that should cover your scenario and pass without any product changes. Can you get me a complog of your build that didn't report the diagnostic?

Is it possible that you have RunAnalyzers set to false or that VS set it to false for one run?

@danmoseley I had copilot crank locally for quite a while and it couldn't figure it out but it did add some regression tests that should cover your scenario and pass without any product changes. Can you get me a complog of your build that didn't report the diagnostic?

Is it possible that you have RunAnalyzers set to false or that VS set it to false for one run?

@jkoritzinsky I updated the repro (top post #126687) to include a global.json -- can you still not repro with that?

@jkoritzinsky also added the install command for preview 3. Using preview 2, it doesn't repro. I verified this by hand again, and it does repro for me with global.json pointing to 11.0.100-preview.3.26170.106

C:\temp\repro>type global.json | find /i "ver"
    "version": "11.0.100-preview.3.26170.106",

C:\temp\repro>type program.cs
using System;
using System.Runtime.InteropServices;
using System.Text;

Console.WriteLine("If you see this, SYSLIB1051 was NOT reported.");

static partial class NativeMethods
{
    [LibraryImport("kernel32.dll", StringMarshalling = StringMarshalling.Utf16)]
    internal static partial int GetVolumeNameForVolumeMountPointW(
        string volumeMountPoint,
        [Out] StringBuilder volumeName,
        int bufferLength);
}
C:\temp\repro>dotnet build
Restore complete (0.3s)
    info NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  doit net11.0 succeeded (0.1s) → bin\Debug\net11.0\doit.dll

Build succeeded in 0.9s

and

C:\temp\repro>set emitcompilergeneratedfiles=true

C:\temp\repro>dotnet build --no-incremental
Restore complete (0.3s)
    info NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  doit net11.0 succeeded (1.5s) → bin\Debug\net11.0\doit.dll

Build succeeded in 2.3s

C:\temp\repro>type obj\Debug\net11.0\generated\Microsoft.Interop.LibraryImportGenerator\Microsoft.Interop.LibraryImportGenerator\LibraryImports.g.cs
// <auto-generated/>
static unsafe partial class NativeMethods
{
    [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Interop.LibraryImportGenerator", "11.0.14.17106")]
    [global::System.Runtime.CompilerServices.SkipLocalsInitAttribute]
    internal static partial int GetVolumeNameForVolumeMountPointW(string volumeMountPoint, global::System.Text.StringBuilder volumeName, int bufferLength)
    {
        int __retVal;
        // Pin - Pin data in preparation for calling the P/Invoke.
        fixed (void* __volumeMountPoint_native = &global::System.Runtime.InteropServices.Marshalling.Utf16StringMarshaller.GetPinnableReference(volumeMountPoint))
        {
            __retVal = __PInvoke((ushort*)__volumeMountPoint_native, volumeName, bufferLength);
        }

        return __retVal;
        // Local P/Invoke
        [global::System.Runtime.InteropServices.DllImportAttribute("kernel32.dll", EntryPoint = "GetVolumeNameForVolumeMountPointW", ExactSpelling = true)]
        static extern unsafe int __PInvoke(ushort* __volumeMountPoint_native, [System.Runtime.InteropServices.OutAttribute] global::System.Text.StringBuilder volumeName, int __bufferLength_native);
    }
}

The idea is that you get an IDE error, then you either fix the error or copy the generated code into your source and remove the LibraryImportAttribute. Then the error goes away as you aren't using the generator any more.

If I understand right then there are two bugs -- the error no longer firing, and the generated code not working (as I discovered in my original PR)

the issue with the generated code pasted above (I think) is that it doesn't have CharSet = CharSet.Unicode so I guess it defaulted to CharSet.None for volumeName which treats it as Ansi on Windows. (Aside, maybe generated code should always be explicit about CharSet anyway).

@danmoseley I found the bug. Apparently the analyzer wasn't firing because the source-generated code was marking the user part of the LibraryImport as generated and skipping it. I've updated the analyzer to catch this case.

I've also added logic to forward CharSet.Unicode when needed in the forwarder.

@jeffhandley do you own the code review agent nowadays? this is not the first time I've seen it time out:

Error: The action 'Execute GitHub Copilot CLI' has timed out after 20 minutes.

That blocks the PR being merged BTW

edit: I merged some changes to fix/improve this

🤖 Copilot Code Review — PR #126691

Holistic Assessment

Motivation: The PR addresses a real bug where LibraryImportGenerator failed to forward CharSet=Unicode to the inner [DllImport] for non-forwarder stubs when StringMarshalling.Utf16 was specified. This caused runtime-marshalled types (e.g. StringBuilder) to default to ANSI encoding—a genuine correctness issue. The PR is justified.

Approach: The core generator fix is correct and mirrors the existing forwarder path. The analyzer change (GeneratedCodeAnalysisFlags.Analyze) is well-motivated—[GeneratedCode] on the generated stub causes Roslyn to treat the user's partial declaration as generated code, silently suppressing diagnostics. However, there are cross-cutting concerns with incomplete fixes in sibling generators and a test assertion that appears incorrect.

Summary: ⚠️ Needs Human Review. The core generator fix (CharSet forwarding) is correct. However: (1) a test assertion likely does not match the generated code's actual text form, (2) the identical bug exists unfixed in DownlevelLibraryImportGenerator, and (3) the analyzer flag change is inconsistent with all sibling analyzers. A maintainer should verify the test actually passes and decide on the scope of cross-cutting fixes.

Detailed Findings

⚠️ Test Assertion Likely Incorrect — Compiles.cs line 741

The test InnerDllImportCharSetTest.VerifyFinalCompilation asserts:

Assert.Equal($"{nameof(CharSet)}.{nameof(CharSet.Unicode)}", charSetArgument.Expression.ToString());
// i.e. Assert.Equal("CharSet.Unicode", ...)

However, the generator emits the expression via AliasQualifiedName("global", IdentifierName(typeof(CharSet).FullName)) (line 406), which produces the text global::System.Runtime.InteropServices.CharSet.Unicode. The generated source is emitted as text via NormalizeWhitespace().ToFullString() (IncrementalGeneratorInitializationContextExtensions.cs line 54), then re-parsed by the compiler into the compilation's syntax trees. When the test reads compilation.SyntaxTrees.Last() and calls .Expression.ToString(), the result should be "global::System.Runtime.InteropServices.CharSet.Unicode", not "CharSet.Unicode".

This assertion was added in the last commit ("Potential fix for pull request finding", authored by Copilot Autofix). The simpler presence/absence check from the prior version of the test was correct; this stronger assertion may cause the test to fail when actually run.

Suggestion: Either revert to the simpler presence check, or update the expected value:

Assert.Equal("global::System.Runtime.InteropServices.CharSet.Unicode", charSetArgument.Expression.ToString());

⚠️ Same Bug Exists in DownlevelLibraryImportGenerator — Not Fixed

DownlevelLibraryImportGenerator.CreateTargetDllImportAsLocalStatement (lines 317–358) has the identical bug: it creates the inner DllImport with only ModuleName, EntryPoint, and ExactSpelling—no CharSet forwarding. Its forwarder path (CreateForwarderDllImport, lines 378–387) already has the fix, confirming the non-forwarder path was just missed.

This is the same pattern as the bug being fixed here. Consider applying the same fix to the downlevel generator in this PR or a follow-up.

⚠️ Analyzer Configuration Inconsistency — Only This Analyzer Changed

The change from GeneratedCodeAnalysisFlags.None to GeneratedCodeAnalysisFlags.Analyze is well-motivated (the [GeneratedCode] attribute on generated stubs prevents user-code diagnostics). However, all sibling diagnostics analyzers still use None:

• DownlevelLibraryImportDiagnosticsAnalyzer — None
• ComInterfaceGeneratorDiagnosticsAnalyzer — None
• VtableIndexStubDiagnosticsAnalyzer — None
• ComClassGeneratorDiagnosticsAnalyzer — None

Since all these generators apply [GeneratedCode] to generated stubs via SignatureContext.cs, the DownlevelLibraryImportDiagnosticsAnalyzer likely has the same diagnostic suppression problem. The ComInterface analyzers may also be affected depending on how they target user-written code. Worth investigating whether the same fix is needed for the downlevel analyzer at minimum.

💡 Code Duplication — Consider Extracting Helper

The new CharSet expression construction (lines 401–407) manually inlines the same pattern as CreateEnumExpressionSyntax<T> (lines 484–490), which is a local function inside CreateForwarderDllImport and therefore inaccessible. Consider extracting it to a file-level private static method so both call sites can reuse it.

✅ Core Fix Correctness — CharSet Forwarding Logic

The fix correctly mirrors the existing forwarder behavior:

• Checks IsUserDefined.HasFlag(InteropAttributeMember.StringMarshalling) to ensure the user explicitly set the value
• Only forwards for StringMarshalling.Utf16 (not Utf8 or Custom), since CharSet only maps to Utf16
• Uses the same AliasQualifiedName + MemberAccessExpression pattern as the forwarder path

This ensures types forwarded to the CLR runtime marshaller (like StringBuilder) use the correct encoding. (Flagged as correct by both review models.)

✅ Diagnostic Tests — Well-Structured

The new tests in Diagnostics.cs are well-structured:

• StringBuilderNotSupported_ReportsDiagnostic uses [Theory] with three inline data cases (Utf16, Utf8, empty)
• StringBuilderNotSupported_WithStringParam_ReportsDiagnostic tests a more realistic scenario matching the linked issue
• Both correctly verify ParameterTypeNotSupported (SYSLIB1051) diagnostic location and arguments

✅ Analyzer Change Rationale — Correct for This Analyzer

The commit message (69a3b5c) explains: "Since we put the GeneratedCodeAttribute on the stub implementation, the user part of the partial is also marked as generated code." Using Analyze (without ReportDiagnostics) is the right choice—it allows the analyzer to inspect user-written partial declarations while diagnostics are reported at user code locations, not on generated stubs.

Generated by Code Review for issue #126691 · ◷

The trigger attribute isn't applied to a member that we put the GeneratedCodeAttribute on with the COM source generator, so this isn't a problem there.

@copilot please fix for the DownlevelLibraryImportGenerator and its corresponding analyzer as well

@copilot please fix for the DownlevelLibraryImportGenerator and its corresponding analyzer as well

Applied the same two fixes to the DownlevelLibraryImportGenerator in commit d52c93e:

1. DownlevelLibraryImportDiagnosticsAnalyzer: changed GeneratedCodeAnalysisFlags.None → Analyze (same root-cause fix as the main analyzer)
2. DownlevelLibraryImportGenerator.CreateTargetDllImportAsLocalStatement: now forwards CharSet = Unicode when StringMarshalling.Utf16 is set, consistent with what CreateForwarderDllImport already did

Also fixed a test assertion (Assert.Equal → Assert.EndsWith) in the InnerDllImportCharSetTest to handle the global::System.Runtime.InteropServices.CharSet.Unicode qualified name emitted by the generator.