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

Code Quality: SYSLIB1054 - Use CsWin32 instead of DllImportAttribute to generate p/invoke marshalling code at compile time. #15000

Open
gumbarros opened this issue Mar 19, 2024 · 8 comments
Labels
codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability)

Comments

@gumbarros
Copy link
Contributor

gumbarros commented Mar 19, 2024

Description

Related to https://learn.microsoft.com/en-us/dotnet/fundamentals/syslib-diagnostics/syslib1050-1069.

Concerned code

Found issues (67 issues)
	Found usages (67 issues)
		<src>\<core> (4 issues)
			<src>\<core>\<Files.Core> (4 issues)
				<src>\<core>\<Files.Core>\Helpers (4 issues)
					<src>\<core>\<Files.Core>\Helpers\NativeFindStorageItemHelper.cs (4 issues)
						Mark the method 'FindFirstFileExFromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'FindNextFile' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'FindClose' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'FileTimeToSystemTime' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
		<src>\<platforms> (63 issues)
			<src>\<platforms>\<Files.App> (63 issues)
				<src>\<platforms>\<Files.App>\Utils\Shell (3 issues)
					<src>\<platforms>\<Files.App>\Utils\Shell\PreviewHandler.cs (1 issue)
						Mark the method 'CoCreateInstance' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
					<src>\<platforms>\<Files.App>\Utils\Shell\ItemStreamHelper.cs (2 issues)
						Mark the method 'SHCreateStreamOnFileEx' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'SHCreateItemFromParsingName' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
				<src>\<platforms>\<Files.App>\Services (1 issue)
					<src>\<platforms>\<Files.App>\Services\SideloadUpdateService.cs (1 issue)
						Mark the method 'RegisterApplicationRestart' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
				<src>\<platforms>\<Files.App>\Helpers (1 issue)
					<src>\<platforms>\<Files.App>\Helpers\NaturalStringComparer.cs (1 issue)
						Mark the method 'CompareStringEx' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
				<src>\<platforms>\<Files.App>\Helpers\Win32 (1 issue)
					<src>\<platforms>\<Files.App>\Helpers\Win32\Win32Helper.Storage.cs (1 issue)
						Mark the method 'SHQueryRecycleBin' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
				<src>\<platforms>\<Files.App>\Helpers\Interop (56 issues)
					<src>\<platforms>\<Files.App>\Helpers\Interop\NativeWinApiHelper.cs (8 issues)
						Mark the method 'GetKeyState' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetDpiForMonitor' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'OpenProcessToken' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetCurrentProcess' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetTokenInformation' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CloseHandle' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetLengthSid' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'IsWow64Process2' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
					<src>\<platforms>\<Files.App>\Helpers\Interop\NativeIoDeviceControlHelper.cs (4 issues)
						Mark the method 'CreateFileFromAppW' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'DeviceIoControl' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'DeviceIoControl' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CloseHandle' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
					<src>\<platforms>\<Files.App>\Helpers\Interop\NativeFileOperationsHelper.cs (21 issues)
						Mark the method 'CloseHandle' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CreateFileFromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'DeviceIoControl' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CreateFile2FromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CreateDirectoryFromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'MoveFileFromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CopyFileFromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'DeleteFileFromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'RemoveDirectoryFromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetFileAttributesExFromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'SetFileAttributesFromApp' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'SetFilePointer' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'ReadFile' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'WriteFile' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'WriteFileEx' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetFileTime' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'SetFileTime' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetFileInformationByHandleEx' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetFileInformationByHandleEx' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'FindFirstStreamW' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'FindNextStreamW' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
					<src>\<platforms>\<Files.App>\Helpers\Interop\NativeDirectoryChangesHelper.cs (9 issues)
						Mark the method 'CloseHandle' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetOverlappedResult' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CancelIo' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CancelIoEx' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'WaitForMultipleObjectsEx' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CreateEvent' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'ResetEvent' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'WaitForSingleObjectEx' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'ReadDirectoryChangesW' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
					<src>\<platforms>\<Files.App>\Helpers\Interop\InteropHelpers.cs (10 issues)
						Mark the method 'SetPropW' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetCursorPos' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CreateEvent' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'SetEvent' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetDpiForWindow' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'CoWaitForMultipleObjects' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'SetWindowLongPtr32' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'SetWindowLongPtr64' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'GetKeyState' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'SHBrowseForFolder' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
					<src>\<platforms>\<Files.App>\Helpers\Interop\FileUtils.cs (4 issues)
						Mark the method 'RmRegisterResources' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'RmStartSession' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'RmEndSession' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
						Mark the method 'RmGetList' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)
				<src>\<platforms>\<Files.App>\Helpers\Environment (1 issue)
					<src>\<platforms>\<Files.App>\Helpers\Environment\ElevationHelpers.cs (1 issue)
						Mark the method '_IsElevationRequired' with 'LibraryImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time (0 issues)

Example:

[DllImport("api-ms-win-core-timezone-l1-1-0.dll", SetLastError = true)]
public static extern bool FileTimeToSystemTime(
	ref FILETIME lpFileTime,
	out SYSTEMTIME lpSymTime);stemTime);

to

[LibraryImport("api-ms-win-core-timezone-l1-1-0.dll", SetLastError = true)]
public static extern bool FileTimeToSystemTime(
	ref FILETIME lpFileTime,
	out SYSTEMTIME lpSystemTime);

Gains

  • Better performance using source generators instead of reflection

Requirements

  • Running R# to generate the report of variables and fields that can be converted
  • Applying the change file by file

Comments

No response

@gumbarros gumbarros added the codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) label Mar 19, 2024
@gumbarros
Copy link
Contributor Author

https://learn.microsoft.com/en-us/dotnet/standard/native-interop/pinvoke-source-generation

Another reference explaining the benefits. I think this is an important issue.

@0x5bfa
Copy link
Member

0x5bfa commented Mar 26, 2024

I like this idea. Feel free to work on it with @yaira2’s approval.

@yaira2
Copy link
Member

yaira2 commented Mar 26, 2024

Sounds good, it might be worth splitting this into multiple PRs to make it easier to test and review.

@0x5bfa
Copy link
Member

0x5bfa commented Mar 27, 2024

Thanks for the initial try.
Do you know CsWin32?
If you know, you can replace them with API generated by CsWin32.
Using DllImport or LibraryImport is deprecation in Files app because we plan to use CsWin32 instead of those code and Vanara.

@gumbarros
Copy link
Contributor Author

Just checked about CsWin32, looks promising because of source generator usage like LibraryImportAttribute. Will make a try tonight at NativeFindStorageItemHelper.cs

@gumbarros gumbarros changed the title Code Quality: SYSLIB1054 - Use LibraryImportAttribute instead of DllImportAttribute to generate p/invoke marshalling code at compile time. Code Quality: SYSLIB1054 - Use CsWin32 instead of DllImportAttribute to generate p/invoke marshalling code at compile time. Mar 28, 2024
@gumbarros
Copy link
Contributor Author

Don't know if this is expected behavior when migrating:
microsoft/CsWin32#1167

@0x5bfa
Copy link
Member

0x5bfa commented Mar 29, 2024

I'm not sure, but this function in C language uses void*.
I assume that whether adding in or out and even ref is defined in win32metadata or CsWin32 repo.

#include <fileapifromapp.h>

WINSTORAGEAPI HANDLE FindFirstFileExFromAppW(
  LPCWSTR            lpFileName,
  FINDEX_INFO_LEVELS fInfoLevelId,
  LPVOID             lpFindFileData, // HERE
  FINDEX_SEARCH_OPS  fSearchOp,
  LPVOID             lpSearchFilter,
  DWORD              dwAdditionalFlags
) noexcept;

You can use Marshal.PtrToStructure:

using System.Runtime.InteropServices;

Marshal.PtrToStructure((nint)lpFindFileData, typeof(WIN32_FIND_DATA));

And you don't need to migrate all of inside of NativeFindStorageItemHelper because it would make us unreviewable so why not migrate step by step.
Also, if you're struggling with some issues you can skip, or you can pick functions up from Win32PInvoke that I made in my PR #15075 after merged because I gathered methods into single file and I think it's a good play ground as well for you.

@0x5bfa
Copy link
Member

0x5bfa commented Apr 1, 2024

Now you can see Files.App.Helpers.Win32 Win32PInvoke class. We can replace them with CsWin32 piece by piece (starting from about 10 functions).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability)
Projects
Status: 🏗 In progress
Development

No branches or pull requests

3 participants