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

APIs that are marked as UnsupportedOSPlatform("browser") under System.ComponentModel.TypeConverter seems like all were false positives. #45162

Closed
buyaa-n opened this issue Nov 24, 2020 · 5 comments
Labels
Milestone

Comments

@buyaa-n
Copy link
Contributor

buyaa-n commented Nov 24, 2020

I looked through the APIs that are marked as UnsupportedOSPlatform("browser") under System.ComponentModel.TypeConverter and to me it seems like all were false positives. I do see a common case where all are using Activator.CreateInstance or Type.GetConstructor which might have been the common pattern where the analyzer is flagging APIs are unsupported. These are the APIs that are marked as not supported:

public static object CreateWithContext(Type type, LicenseContext creationContext)

public static object CreateWithContext(Type type, LicenseContext creationContext, object[] args)


public virtual object CreateInstance(IServiceProvider provider, Type objectType, Type[] argTypes, object[] args)

public static object CreateInstance(IServiceProvider provider, Type objectType, Type[] argTypes, object[] args)

public override object ConvertTo(ITypeDescriptorContext context, CultureInfo culture, object value, Type destinationType)

@steveisok @eerhardt do you agree that all these should be supported? That is the way it looks to me.

Originally posted by @safern in #43363 (comment)

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.ComponentModel untriaged New issue has not been triaged by the area owner labels Nov 24, 2020
@ghost
Copy link

ghost commented Nov 24, 2020

Tagging subscribers to this area: @safern
See info in area-owners.md if you want to be subscribed.

Issue Details

I looked through the APIs that are marked as UnsupportedOSPlatform("browser") under System.ComponentModel.TypeConverter and to me it seems like all were false positives. I do see a common case where all are using Activator.CreateInstance or Type.GetConstructor which might have been the common pattern where the analyzer is flagging APIs are unsupported. These are the APIs that are marked as not supported:

public static object CreateWithContext(Type type, LicenseContext creationContext)

public static object CreateWithContext(Type type, LicenseContext creationContext, object[] args)


public virtual object CreateInstance(IServiceProvider provider, Type objectType, Type[] argTypes, object[] args)

public static object CreateInstance(IServiceProvider provider, Type objectType, Type[] argTypes, object[] args)

public override object ConvertTo(ITypeDescriptorContext context, CultureInfo culture, object value, Type destinationType)

@steveisok @eerhardt do you agree that all these should be supported? That is the way it looks to me.

Originally posted by @safern in #43363 (comment)

Author: buyaa-n
Assignees: -
Labels:

area-System.ComponentModel, untriaged

Milestone: -

@safern safern added arch-wasm WebAssembly architecture and removed untriaged New issue has not been triaged by the area owner labels Nov 24, 2020
@safern safern added this to the 6.0.0 milestone Nov 24, 2020
@buyaa-n
Copy link
Contributor Author

buyaa-n commented Nov 24, 2020

It looks like those should all be supported. I'd like to understand why the tool gave a false positive because:

  • Maybe we are missing something by just doing code inspection. And these APIs should really be flagged.
  • Maybe there is a bug in the tool, and we marked a bunch of other APIs as unsupported, when really they should be.

Originally posted by @eerhardt in #43363 (comment)

@steveisok
Copy link
Member

We used the tool to look for PNSE and NotSupportedException. Unfortunately, in the TypeConverter case, NSE is an expected throw and it was missed in review.

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Nov 25, 2020

I have removed the attributes from TypeDescriptor.CreateInstance, TypeDescriptionProvider.CreateInstance as it was referenced in my PR and causing warnings, remaining APIs ca be handled with this PR

@maryamariyan maryamariyan modified the milestones: 6.0.0, 7.0.0 Jul 23, 2021
@maryamariyan maryamariyan modified the milestones: 7.0.0, 6.0.0 Jul 23, 2021
@lewing
Copy link
Member

lewing commented Jul 31, 2021

Is there anything that needs to be done here? If so please reopen.

@lewing lewing closed this as completed Jul 31, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants