dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
22.21k stars 1.75k forks source link

Follow up TODOs list for CA1416 analysis PR #6348

Open buyaa-n opened 2 years ago

buyaa-n commented 2 years ago

List of TODOs left from Fix violations of Platform Compatibility Analyzer CA1416 effort

Most of the warnings found with Platform Compatibility Analyzer are handled In Handle CA1416 violations found in MAUI repo, though there are plenty of warnings I was unsure how to handle or has blocked with other issues, those are commented with TODO.

In the PR I have propagated some of the warnings found in the API to the type, only when it is obvious that the type is useless/will not work without that API/functionality. There were more places that could be done, but I was not sure if that is the main functionality for the type or not, therefore suppressed them with a TODO comment.

TODOs needs the team action

In general all comments with TODO are needed to be reviewed by the the code owners and handled as needed. Here is the list of most questionable TODOs needs immediate action :

  1. ApplicationHandler.cs row 28: MapOpenWindow and MapCloseWindow in ApplicationHandler.iOS.cs are supported only on iOS 13.0+, which area called in ApplicationHandler.cs on row 28, currently suppressed we might need to propagate the warning to the caller. They are need to be propagated to the type if those APIs are main functionality for the type and without them the type is unusable

  2. MauiUIApplicationDelegate.Menu.cs - lots of APIs annotated with SupportedOSPlatform("ios13.0")], may need type level annotation. They are need to be propagated to the type if those APIs are main functionality for the type and without them the type is unusable

  3. MediaPicker.ios row 59: within a conditional block if (pickExisting && !OperatingSystem.IsIOSVersionAtLeast(11, 0)) (means ios 11.0 or below) Permissions.Photos used which is only for iOS 14.0+

  4. src/Compatibility/Core/src/iOS/Forms.cs has IsiOSVersionOrNewer properties which also checks tvOS version not only iOS but used in callsites accessesing APIs not supported on TvOS. Normally I would add [SupportedOSPlatformGuard("ios11.0")] and [SupportedOSPlatformGuard("tvos11.0")] for those, but commented the [SupportedOSPlatformGuard("tvos11.0")] part with ] TODO: the block guarded by this property calling API unsupported on TvOS or version not supported

  5. Plenty of warnings caused by unsupported APIs, MAUI team informed that those are mostly deprecated but still works, there is many more found in other projects which needs review from the team. These are mostly suppressed with TODO:

    • #pragma warning disable CA1416 // TODO: 'UIApplication.KeyWindow' is unsupported on: 'ios' 13.0 and later. Or asserted: System.Diagnostics.Debug.Assert(!OperatingSystem.IsIOSVersionAtLeast(14));
    • If the APIs are obsoleted and still works we can keep the suppression and remove the TODO, if the API is really unsupported then the call should be guarded or the API annotated to inform the callers
  6. src/Essentials/src/Browser/Browser.ios.cs row 32: creating instance of SFSafariViewController using SFSafariViewController(NSUrl, bool) that is unsupported from 'ios' 11.0, there is an overload SFSafariViewController(NSUrl, SFSafariViewControllerConfiguration) supported from ios 11.0+, need to call the constructors conditionally

List of actions needed after the blocking issues are fixed:

  1. After Platform annotation issues in Microsoft.iOS assembly fixed suppressions with #pragma warning disable CA1416 // https://github.com/xamarin/xamarin-macios/issues/14619 need to be reviewed and handled as needed
  2. When remaining 2 analyzer bugs are fixed corresponding suppressions can be removed
  3. When Paint.Color is attributed with [SupportedOSPlatform("android29.0")] causing warnings in Maui is fixed remove suppressions with #pragma warning disable CA1416 // https://github.com/xamarin/xamarin-android/issues/6962

CC @PureWeen @matthewrdev @Redth

Public API Changes

If warnings propagated to the caller eventually could change public API annotation

Intended Use-Case

Validate platform compatibility

MichaelRumpler commented 2 years ago

In the iOS EntryCellRenderer the GetCell method is marked as

    [UnsupportedOSPlatform("ios14.0")]
    [UnsupportedOSPlatform("tvos14.0")]
    public override UITableViewCell GetCell(Cell item, UITableViewCell reusableCell, UITableView tv)

But that would mean that the EntryCell could not be used on iOS > 14.0 at all. Is this an error? The other cells are not marked as unsupported.

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.