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 CA1849 for DbContext methods #6719

Merged
merged 7 commits into from
Jul 4, 2023
Merged

Conversation

CollinAlpert
Copy link
Contributor

@CollinAlpert CollinAlpert commented Jun 29, 2023

This PR adds exclusions for the DbContext.Add() and DbContext.AddRange() methods and fixes a false negative for IDbContextFactory<DbContext>.CreateDbContext() in the UseAsyncMethodInAsyncContext analyzer.

Fixes #6684.

@CollinAlpert CollinAlpert requested a review from a team as a code owner June 29, 2023 09:59
@codecov
Copy link

codecov bot commented Jun 29, 2023

Codecov Report

Merging #6719 (fdd3145) into main (2150eeb) will increase coverage by 0.00%.
The diff coverage is 97.61%.

❗ Current head fdd3145 differs from pull request most recent head 4a46fe7. Consider uploading reports for the commit 4a46fe7 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6719   +/-   ##
=======================================
  Coverage   96.36%   96.36%           
=======================================
  Files        1395     1395           
  Lines      325148   325211   +63     
  Branches    10678    10679    +1     
=======================================
+ Hits       313323   313398   +75     
+ Misses       9254     9245    -9     
+ Partials     2571     2568    -3     

@CollinAlpert
Copy link
Contributor Author

The build is complaining that there is a missing entry in RulesMissingDocumentation.md for rule CA2200, however documentation does exist for that rule. Is this a bug in the build?

@buyaa-n
Copy link
Member

buyaa-n commented Jun 30, 2023

The build is complaining that there is a missing entry in RulesMissingDocumentation.md for rule CA2200, however documentation does exist for that rule. Is this a bug in the build?

Strange, usually it complains when there is an entry that needs to be removed because the corresponding doc is added.

Anyway, better to run the msbuild /t:pack to see if there were any doc update happened

@CollinAlpert
Copy link
Contributor Author

I ran that command already and no changes were made, which makes sense. I can try again tomorrow.

@buyaa-n buyaa-n enabled auto-merge (squash) July 4, 2023 06:14
Copy link
Member

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

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

Thank you @CollinAlpert!

@buyaa-n buyaa-n merged commit bdcbcac into dotnet:main Jul 4, 2023
10 of 11 checks passed
@CollinAlpert CollinAlpert deleted the issue_6684 branch July 4, 2023 16:28
@molinch
Copy link

molinch commented Aug 11, 2023

Do we know if this is already shipped with a .NET8 preview? Or it will land in next one?

@cremor
Copy link

cremor commented Aug 11, 2023

Will this only ship with a .NET 8 SDK, or will it also be included in a .NET 7 SDK update?

@buyaa-n
Copy link
Member

buyaa-n commented Aug 11, 2023

It should have included in .NET 8 preview 7, it is not ported to .NET 7

@Spacefish
Copy link
Contributor

Spacefish commented Aug 16, 2023

I get a lot of CA1849 warnings with .NET 8 Preview 7 for my DbSet.Add(..) calls..
As we are not calling the "Add" directly on the DbContext but instead on the Sets on the Contexts.. Maybe we should include that as well.

I created an PR here: #6858

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.

CA1849 "Call async methods when in an async method" false positive on DbContext.AddRange()
5 participants