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

Don't emit CA1861 for static ReadOnlyCollections #6644

Merged
merged 2 commits into from
May 25, 2023

Conversation

CollinAlpert
Copy link
Contributor

This PR excludes static readonly ReadOnlyCollections from triggering CA1861. Fixes #6629.

@CollinAlpert CollinAlpert requested a review from a team as a code owner May 19, 2023 19:52
@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Merging #6644 (befdc3a) into main (c2fd19b) will decrease coverage by 0.01%.
The diff coverage is 82.22%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6644      +/-   ##
==========================================
- Coverage   96.40%   96.39%   -0.01%     
==========================================
  Files        1377     1379       +2     
  Lines      322008   322186     +178     
  Branches    10434    10459      +25     
==========================================
+ Hits       310438   310585     +147     
- Misses       9081     9109      +28     
- Partials     2489     2492       +3     

@buyaa-n
Copy link
Contributor

buyaa-n commented May 23, 2023

Thank you @CollinAlpert for the fix, with the feedback on the issue:

If you're assigning to a static, you're already publishing it to be shared / reused, regardless of whether that static is readonly. And lazy initialization is a key place where you assign to statics and we don't want false positives

Now we want to exclude any static field initializations, please update the PR accordingly

@buyaa-n
Copy link
Contributor

buyaa-n commented May 25, 2023

Now we want to exclude any static field initializations, please update the PR accordingly

OK, updated myself as I need this fix

@buyaa-n buyaa-n merged commit 3515b06 into dotnet:main May 25, 2023
@github-actions github-actions bot added this to the vNext milestone May 25, 2023
@CollinAlpert CollinAlpert deleted the issue_6629 branch May 26, 2023 06:08
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.

CA1861 exclude static ReadOnlyCollection<T> field/properties
2 participants