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

Follow up TODOs list for CA1416 analysis PR #6348

Open
buyaa-n opened this issue Apr 21, 2022 · 2 comments
Open

Follow up TODOs list for CA1416 analysis PR #6348

buyaa-n opened this issue Apr 21, 2022 · 2 comments
Labels
area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions Task neither bug nor feature but something that needs to be done in support of either
Milestone

Comments

@buyaa-n
Copy link
Member

buyaa-n commented Apr 21, 2022

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

@Redth Redth added this to the 6.0.300 milestone Apr 21, 2022
@buyaa-n buyaa-n changed the title Follow up TOOD list for CA1416 analysis result Follow up TODOs list for CA1416 analysis PR Apr 22, 2022
@Eilon Eilon added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Apr 28, 2022
@MichaelRumpler
Copy link
Contributor

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.

@mattleibow mattleibow modified the milestones: 6.0.300, 6.0-servicing Aug 29, 2022
@Redth Redth modified the milestones: 6.0-servicing, Backlog Aug 30, 2022
@ghost
Copy link

ghost commented Aug 30, 2022

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.

@samhouts samhouts added the Task neither bug nor feature but something that needs to be done in support of either label Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions Task neither bug nor feature but something that needs to be done in support of either
Projects
None yet
Development

No branches or pull requests

6 participants