Change analyzer versions such that the repo can be built with .NET 7 RC2 SDK #3077
Conversation
…RC2 SDK. Took the versions from dotnet/runtime.
|
I was debugging some of the failures too for my runtime PR, after updating to MicrosoftCodeAnalysisVersion 4.5.* the roslyn analyzers will start generating duplicate warnings on attribute scenarios seems like the compilation now takes into account more stuff and creates callbacks for object creations that it didn't before. I though it was just going to be a change like removing the CheckAttributeInstantiation method but seems like it's more conversome than that |
|
Never mind, removing CheckAttributeInstantiation is the solution. I saw some tests failing after removing the method but it was because before CheckAttributeInstantiation was producing double warnings. With the current analysis, only one warning is produced (which is the right outcome). |
|
@tlakollo would you mind porting the fixes from your branch to this PR? |
Sure, working on it |
Remove global attributes since they are no longer necessary Workaround the fact that compiler injects System.Runtime.CompilerServices.RefSafetyRulesAttribute into every assembly to describe the language version
| @@ -205,21 +188,6 @@ public override void Initialize (AnalysisContext context) | |||
| foreach (var extraSymbolAction in ExtraSymbolActions) | |||
| context.RegisterSymbolAction (extraSymbolAction.Action, extraSymbolAction.SymbolKind); | |||
|
|
|||
| void CheckAttributeInstantiation ( | |||
There was a problem hiding this comment.
Why is it OK to remove this?
Where is the place we perform this check now? (and why did it change with a new analyzer package version)
There was a problem hiding this comment.
There is new callback calls in attributes, for at least one scenario (I didn't test all scenarios) instead of going through a type argument it went through an object creation, basically when it sees the use of the attribute it created a callback for object creation and then we have a CheckMemberCall that verifies that it has requires producing the warning. My guess is that it was a limitation of the analyzer that we had to work around, just like the global attribute and at somepoint it got fixed
There was a problem hiding this comment.
OK - I guess if all the tests are passing, it's at least not completely broken
|
One observation on a machine with RC2 installed:
Once the repo is built with So unfortunately this means that if you need to use VS - don't use I'm guessing that this is due to some kind of breaking change between RC1 and RC2. The problems should go away once we switch the SDK to RC2 in the global.json (but we have to wait for dotnet/runtime to do that first). |
|
Does anybody know where we get the |
|
It's included in the SDK, and I don't think we try to override the dotnet-format version anywhere. |
|
Unfortunately the RC2 dotnet format fails with: Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.5.0.0,So it seems that the only way to run linter is to clean the enlistment and use the |
|
I don't get it - RC1 ships format with Microsoft.CodeAnalysis 4.3.0.0 - and it works |
|
Notice that runtime has this same behavior, you cannot run dotnet format at all. I've been changing the versions.props file to be able to run the formater for the linker to runtime move PR. In the runtime repo it fails with "Could not load file or assembly 'Microsoft.CodeAnalysis.Workspaces, Version=4.4.0.0" which Im guessing is part of the Microsoft.CodeAnalysis |
…RC2 SDK (dotnet#3077) Change analyzer versions such that the repo can be built with .NET 7 RC2 SDK. - Took the versions from dotnet/runtime. - Remove CheckAttributeInstantiation method since is no longer necessary - Remove global attributes since they are no longer necessary - Workaround the fact that compiler injects System.Runtime.CompilerServices.RefSafetyRulesAttribute into every assembly to describe the language version Co-authored-by: tlakollo <tlakaelel_axayakatl@outlook.com>
…RC2 SDK (dotnet/linker#3077) Change analyzer versions such that the repo can be built with .NET 7 RC2 SDK. - Took the versions from dotnet/runtime. - Remove CheckAttributeInstantiation method since is no longer necessary - Remove global attributes since they are no longer necessary - Workaround the fact that compiler injects System.Runtime.CompilerServices.RefSafetyRulesAttribute into every assembly to describe the language version Co-authored-by: tlakollo <tlakaelel_axayakatl@outlook.com> Commit migrated from dotnet/linker@c0bd208
…RC2 SDK (dotnet/linker#3077) Change analyzer versions such that the repo can be built with .NET 7 RC2 SDK. - Took the versions from dotnet/runtime. - Remove CheckAttributeInstantiation method since is no longer necessary - Remove global attributes since they are no longer necessary - Workaround the fact that compiler injects System.Runtime.CompilerServices.RefSafetyRulesAttribute into every assembly to describe the language version Co-authored-by: tlakollo <tlakaelel_axayakatl@outlook.com> Commit migrated from dotnet/linker@c0bd208
Took the versions from dotnet/runtime.