Windows icon indicating copy to clipboard operation
Windows copied to clipboard
verified publisher icon CommunityToolkit

Clean up suppression of CS0109 and CS0114

Open Balkoth opened this issue 2 years ago • 5 comments

Describe the bug

There are four locations in the codebase where CS0109 is suppressed. I can not find a reason why the suppressions are not just simply cleaned up.

image

Steps to reproduce

Build the codebase.

Expected behavior

No suppression of CS0109.

Screenshots

No response

Code Platform

  • [ ] UWP
  • [X] WinAppSDK / WinUI 3
  • [ ] Web Assembly (WASM)
  • [ ] Android
  • [ ] iOS
  • [ ] MacOS
  • [ ] Linux / GTK

Windows Build Number

  • [ ] Windows 10 1809 (Build 17763)
  • [ ] Windows 10 1903 (Build 18362)
  • [ ] Windows 10 1909 (Build 18363)
  • [ ] Windows 10 2004 (Build 19041)
  • [ ] Windows 10 20H2 (Build 19042)
  • [ ] Windows 10 21H1 (Build 19043)
  • [ ] Windows 10 21H2 (Build 19044)
  • [X] Windows 10 22H2 (Build 19045)
  • [ ] Windows 11 21H2 (Build 22000)
  • [ ] Other (specify)

Other Windows Build number

No response

App minimum and target SDK version

  • [ ] Windows 10, version 1809 (Build 17763)
  • [ ] Windows 10, version 1903 (Build 18362)
  • [ ] Windows 10, version 1909 (Build 18363)
  • [ ] Windows 10, version 2004 (Build 19041)
  • [X] Windows 10, version 2104 (Build 20348)
  • [ ] Windows 11, version 22H2 (Build 22000)
  • [ ] Other (specify)

Other SDK version

No response

Visual Studio Version

2022

Visual Studio Build Number

17.7.4

Device form factor

Desktop

Additional context

No response

Help us help you

Yes, I'd like to be assigned to work on this item.

Balkoth avatar Oct 05 '23 14:10 Balkoth

@Balkoth pretty sure these are here for Uno, they're only needed when compiling on certain Uno platforms but not for Windows.

Don't remember the old issue/discussion we had on this somewhere here or in our tooling repo. Probably similar for #250 that you filed too?

FYI @Arlodotexe who may remember more.

michael-hawker avatar Oct 11 '23 14:10 michael-hawker

I guessed that too after i filed the issues and created the pull requests. Imho the methods definitions should be ifdefd for the correct platforms, then there would be no need to suppress the warnings.

If someone guides me on how to build for all the supported platforms (on my windows machine) i will gladly help out.

Balkoth avatar Oct 11 '23 14:10 Balkoth

Noting CS0114 is used in some places with the same message "Member does not hide an inherited member. The new keyword is not required":

image

Relevant docs:

  • https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0109
  • https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0114

Git history suggests these were added to resolve a CI issue during the port from the old repo (see https://github.com/CommunityToolkit/Windows/pull/83 @ https://github.com/CommunityToolkit/Windows/pull/83/commits/8960c7a8a5e991af58b821178858c6674810983f). The logs for this CI run have since expired, but we can check if/why it may be needed by just removing it and seeing if any issues show in the CI.

Arlodotexe avatar Jan 23 '24 21:01 Arlodotexe

Here's the full error message, pulled from the latest CI run:

2024-01-23T22:39:43.4070369Z        "C:\a\Windows\Windows\CommunityToolkit.AllComponents.sln" (default target) (1:2) ->
2024-01-23T22:39:43.4071154Z        "C:\a\Windows\Windows\components\LayoutTransformControl\src\CommunityToolkit.WinUI.Controls.LayoutTransformControl.csproj" (default target) (29:39) ->
2024-01-23T22:39:43.4071954Z        "C:\a\Windows\Windows\components\LayoutTransformControl\src\CommunityToolkit.WinUI.Controls.LayoutTransformControl.csproj" (Build target) (29:55) ->
2024-01-23T22:39:43.4072106Z        (CoreCompile target) -> 
2024-01-23T22:39:43.4074755Z          C:\a\Windows\Windows\components\LayoutTransformControl\src\LayoutTransformControl.Properties.cs(72,22): error CS0114: 'LayoutTransformControl.Transform' hides inherited member 'UIView.Transform'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword. [C:\a\Windows\Windows\components\LayoutTransformControl\src\CommunityToolkit.WinUI.Controls.LayoutTransformControl.csproj::TargetFramework=net7.0-ios]
2024-01-23T22:39:43.4075139Z 
2024-01-23T22:39:43.4075143Z 
2024-01-23T22:39:43.4075545Z        "C:\a\Windows\Windows\CommunityToolkit.AllComponents.sln" (default target) (1:2) ->
2024-01-23T22:39:43.4076322Z        "C:\a\Windows\Windows\components\LayoutTransformControl\src\CommunityToolkit.WinUI.Controls.LayoutTransformControl.csproj" (default target) (29:39) ->
2024-01-23T22:39:43.4077075Z        "C:\a\Windows\Windows\components\LayoutTransformControl\src\CommunityToolkit.WinUI.Controls.LayoutTransformControl.csproj" (Build target) (29:53) ->
2024-01-23T22:39:43.4079760Z          C:\a\Windows\Windows\components\LayoutTransformControl\src\LayoutTransformControl.Properties.cs(72,22): error CS0114: 'LayoutTransformControl.Transform' hides inherited member 'UIView.Transform'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword. [C:\a\Windows\Windows\components\LayoutTransformControl\src\CommunityToolkit.WinUI.Controls.LayoutTransformControl.csproj::TargetFramework=net7.0-maccatalyst]
2024-01-23T22:39:43.4079767Z

It looks like it's only an issue under ios and mac targets. I'd like to wait for #281 to close first, in case the situation has changed in the latest version of Uno.

Arlodotexe avatar Jan 23 '24 23:01 Arlodotexe

Looks like there's no change under the latest version of Uno 5. We'll keep things as-is for now.

I've filed an issue with Uno to hopefully get it a fix that doesn't include warning suppression.

Arlodotexe avatar Jan 29 '24 20:01 Arlodotexe