microsoft / CsWin32

A source generator to add a user-defined set of Win32 P/Invoke methods and supporting types to a C# project.
MIT License
2.09k stars 89 forks source link

Class modifiers would conflict when different two packages have different modifier #1254

Closed 0x5bfa closed 1 month ago

0x5bfa commented 2 months ago

Actual behavior

When I install WinUIEx (configures internal modifier) into my main application project and create a class project and I want to refer CsWin32 both from the app project (Files.App) and from the class project (Files.App.Server), I'd create a new class project that refers to CsWin32 (configures public modifier) (Files.App.CsWin32) and then reference it from both projects. Then because WinUIEx's generated code has internal modifier, modifier error persists while defined functions in Files.App.CsWin32 are generated as public ones.

     'WinUIEx' ← 'CsWin32' internal
         |
         ↓
    'Files.App'
         ↑
         |
'Files.App.CsWin32 ← 'CsWin32' public
         |
         ↓
 'Files.App.Server'

Shortly, WinUIEx's internally generated code is respected by the C# complier, when the same definitions exist in both WinUIEx and Files.App.CsWin32 on the emitter table (NativeMethods.txt).

I literally don't have any idea how to fix on CsWin32 side, do you have?

Expected behavior

Respect publicly generate codes.

Repro steps

  1. NativeMethods.txt content in both WinUIEx and Files.App.CsWin32:
    WNDPROC

2.a. NativeMethods.json content in WinUIEx:

{
  modifier: 'internal'
}

2.b. NativeMethods.json content in Files.App.CsWin32

{
  modifier: 'public'
}
  1. Any of your own code that should be shared?

None

Context

0x5bfa commented 2 months ago

I had a PR on my work but because of this the work was abandoned.

https://github.com/files-community/Files/pull/15793

gh pr checkout 15793 will check out to my PR in Files repo.

AArnott commented 1 month ago

modifier: 'public' isn't valid JSON. I suspect you mean "public": true or "public": false.

Are you using InternalsVisibleTo anywhere? If not, the internal members of a referenced assembly shouldn't break you at all. What they can do is change a c# error from "this member doesn't exist" to "this member is inaccessible", which can make it look like there's a problem when really all you need to do is have CsWin32 declare the API locally.

It's hard for me to say anything more because you haven't actually quoted an actual compilation error. I recognize you gave me a repro that involves cloning a repo but I don't usually have time to do that, especially for large repos. If you can come up with a minimal repro that would be good.

0x5bfa commented 1 month ago

Thanks for the reply.

Are you using InternalsVisibleTo anywhere?

No, neither do.

you haven't actually quoted an actual compilation error.

The error was CS0122 "'member' is inaccessible due to its protection level". WinUIEx defined HWND and Files.App.CsWin32 as well, then HWND had the error.

If you can come up with a minimal repro that would be good.

Unfortunately I workaround'd this by removing WinUIEx dependency. Therefore, closing as I no longer can reproduce. When I get some time, I'll let you know here.