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

Update AnalyzerReleases shipped file for 7.0 release #6246

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

mavasani
Copy link
Contributor

@mavasani mavasani commented Nov 3, 2022

Fixes #6245

I verified that locally built NuGet package with this fix resolves this issue. The NuGet package contains analysislevel_7_XXX.editorconfig set of editorconfig files, and setting AnalysisLevel to latest-XXX or latest or 7.0 passes in the appropriate editorconfig file to compiler command line

Filed #6247 to enhance the tooling to avoid regressions such as #6245 in future.

@mavasani mavasani requested a review from a team as a code owner November 3, 2022 09:07
@mavasani
Copy link
Contributor Author

mavasani commented Nov 3, 2022

@Evangelink @jmarolf for review

@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Merging #6246 (de1013f) into main (55d7cdc) will increase coverage by 0.00%.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6246   +/-   ##
=======================================
  Coverage   96.04%   96.04%           
=======================================
  Files        1360     1360           
  Lines      312054   312054           
  Branches    10047    10047           
=======================================
+ Hits       299720   299724    +4     
+ Misses       9925     9924    -1     
+ Partials     2409     2406    -3     

@mavasani mavasani merged commit 7bd7d0a into dotnet:main Nov 3, 2022
@mavasani mavasani deleted the UpdateShippedReleases branch November 3, 2022 10:26
@github-actions github-actions bot added this to the vNext milestone Nov 3, 2022
@mavasani
Copy link
Contributor Author

mavasani commented Nov 3, 2022

@jmarolf can we do an insertion of the latest NetAnalyzers package into dotnet/sdk after this change?

@jmarolf
Copy link
Contributor

jmarolf commented Nov 3, 2022

let me verify that is happening

@znakeeye
Copy link

znakeeye commented Nov 8, 2022

Just installed Visual Studio 17.4.0 and re-compiled my project. Still no CAxxxx warnings produced when targeting net7.0. Is this expected?

@mavasani
Copy link
Contributor Author

mavasani commented Nov 9, 2022

@znakeeye The fix still needs to flow from this repo to dotnet/sdk repo and make it into the next .NET 7 servicing release. Additionally, VS also needs to move to the newer .NET 7 release with the fix for it to automatically light up. This whole code flow and release process can take some time, so meanwhile I would suggest you to directly install the latest prerelease NuGet package from this feed instead: https://dev.azure.com/dnceng/public/_artifacts/feed/dotnet7/NuGet/Microsoft.CodeAnalysis.NetAnalyzers

Note that you should avoid setting EnableNETAnalyzers MSBuild property to true in your project file/props/targets, as that would in turn override the installed NuGet package analyzers with the old ones shipping inside the .NET SDK (which don't yet have this fix).

@znakeeye
Copy link

@znakeeye The fix still needs to flow from this repo to dotnet/sdk repo and make it into the next .NET 7 servicing release. Additionally, VS also needs to move to the newer .NET 7 release with the fix for it to automatically light up. This whole code flow and release process can take some time, so meanwhile I would suggest you to directly install the latest prerelease NuGet package from this feed instead: https://dev.azure.com/dnceng/public/_artifacts/feed/dotnet7/NuGet/Microsoft.CodeAnalysis.NetAnalyzers

Note that you should avoid setting EnableNETAnalyzers MSBuild property to true in your project file/props/targets, as that would in turn override the installed NuGet package analyzers with the old ones shipping inside the .NET SDK (which don't yet have this fix).

Ok, so in order to get it to work with .NET 7 you should:

  1. Install VS 17.4
  2. Comment out EnableNETAnalyzers
  3. Install latest prelease of Microsoft.CodeAnalysis.NetAnalyzers

Correct? Or do we need 17.5p1? (I saw it was referenced here, somewhere...)

@jmarolf jmarolf modified the milestones: vNext, .NET 7.x Nov 15, 2022
@jmarolf
Copy link
Contributor

jmarolf commented Nov 15, 2022

@mavasani I can confirm this made it into the .NET SDK here: dotnet/sdk#28864. It should be part of the first servicing release.

@mavasani
Copy link
Contributor Author

Thank you @jmarolf

@znakeeye
Copy link

I suppose this was fixed in SDK 7.0.101 then?

@mavasani
Copy link
Contributor Author

I suppose this was fixed in SDK 7.0.101 then?

Yes, correct.

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.

TargetFramework net7.0 disables all CAxxxx rules
4 participants