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

Flag when platform specific type used as generic type parameter #4403

Merged
merged 12 commits into from
Nov 9, 2020

Conversation

buyaa-n
Copy link
Contributor

@buyaa-n buyaa-n commented Nov 2, 2020

Warn when platform-specific type used as generic type parameter, useful when a type created using reflections

Fixes #4362

Cherry-picked from #4390 to target release\5.0.2xx branch

@codecov
Copy link

codecov bot commented Nov 2, 2020

Codecov Report

Merging #4403 (1c106e3) into master (c2ebde6) will increase coverage by 0.00%.
The diff coverage is 98.67%.

@@           Coverage Diff            @@
##           master    #4403    +/-   ##
========================================
  Coverage   95.80%   95.81%            
========================================
  Files        1174     1174            
  Lines      267562   267805   +243     
  Branches    16097    16105     +8     
========================================
+ Hits       256351   256601   +250     
+ Misses       9145     9137     -8     
- Partials     2066     2067     +1     

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Nov 3, 2020

@jeffhandley confirmed that we want it for 5.0.2xx and we have a separate issue for tracking inheritance related scenarios, I think this PR good to go now @mavasani please take another look

@buyaa-n buyaa-n requested a review from mavasani November 5, 2020 20:09
Copy link
Contributor

@mavasani mavasani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looking good - want to make sure that we handle field/property/event access off a generic type with disallowed type argument. Looks good to sign off once that is addressed. Thanks!

@buyaa-n buyaa-n requested a review from mavasani November 6, 2020 01:59
var containingSymbol = context.ContainingSymbol;
if (containingSymbol is IMethodSymbol method && method.IsAccessorMethod())
{
containingSymbol = method.AssociatedSymbol;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we are not adding parent attributes to the accessor methods so that it will not report a duplicate warning when the containing symbol had any attribute (for example property has no accessor level attribute but the entire property has attribute). But when the property getter or setter any statements inside it became the containing symbol for those statements. When accessors used as call site, as their attributes are not including the parent attributes call site attributes are incomplete and causing warnings. So here when accessor is the containing symbol change it to the AssociatedSymbol which is the real Property or Event which has complete attributes for call site

@buyaa-n buyaa-n changed the base branch from release/5.0.2xx to master November 9, 2020 05:05
@buyaa-n
Copy link
Contributor Author

buyaa-n commented Nov 9, 2020

Retargeted to master, as there is no any other change merging

@buyaa-n buyaa-n merged commit 00a8869 into dotnet:master Nov 9, 2020
@buyaa-n buyaa-n deleted the flag_generic_type_param branch November 19, 2020 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Platform compatibility analyzer not warning for platform specific type parameter
3 participants