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

Allow [CallerArgumnetExpressionAttribute] after CancellationToken #6558

Merged
merged 3 commits into from
Apr 8, 2023

Conversation

Tornhoof
Copy link
Contributor

Fixes #6557

@Tornhoof Tornhoof requested a review from a team as a code owner March 30, 2023 08:55
@@ -440,6 +440,46 @@ public class C
}");
}

#if NETCOREAPP3_1_OR_GREATER
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests are modelled after the original tests for the other Caller...Attributes
I had to limit the tests to .NET Core 3.1 and higher as .NET 4.7.2 does not contain the attribute.

I find it mildly irritating that the tests are run against netcoreapp3.1 instead of a more recent framework, as 3.1 is out of support.

Copy link
Member

Choose a reason for hiding this comment

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

@Tornhoof #6437 is moving tests to net8.0

@codecov
Copy link

codecov bot commented Mar 30, 2023

Codecov Report

Merging #6558 (75e67ec) into main (b84dbd4) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6558   +/-   ##
=======================================
  Coverage   96.43%   96.43%           
=======================================
  Files        1372     1372           
  Lines      320278   320319   +41     
  Branches    10295    10293    -2     
=======================================
+ Hits       308848   308896   +48     
+ Misses       8975     8968    -7     
  Partials     2455     2455           

@stephentoub
Copy link
Member

Since the analyzer is already excluding the other CallerXx attributes, makes sense to do it for this one as well.

Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

Can you use ReferenceAssemblies.Net.Net50 instead of the #if? Other than that, LGTM.

Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

LGTM

@mavasani mavasani merged commit 21e1f19 into dotnet:main Apr 8, 2023
@github-actions github-actions bot added this to the vNext milestone Apr 8, 2023
@Tornhoof Tornhoof deleted the UpdateCA1068 branch April 8, 2023 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Relax CA1068 if CallerArgumentExpression comes after CancellationToken
4 participants