Snap to Roslyn 4.4 #4194
Snap to Roslyn 4.4 #4194
Conversation
|
This change essentially means that we will force folks to use .NET 8 toolchain even if they are targeting |
|
No, this forces the .NET 7 toolchain, not .NET 8. The option code generator, which is now in dotnet/runtime also requires the .NET 7 toolchain, so we're being consistent. Note that code generators were introduced in .NET 5, so anybody using the code generators would at least be on .NET 5 SDK |
geeknoid
force-pushed
the
geeknoid/roslyn
branch
3 times, most recently
from
efda71d to
3476c5d
Compare
geeknoid
force-pushed
the
geeknoid/roslyn
branch
from
3476c5d to
2ab1496
Compare
geeknoid
force-pushed
the
geeknoid/roslyn
branch
from
2ab1496 to
e6a2fd5
Compare
geeknoid
force-pushed
the
geeknoid/roslyn
branch
from
e6a2fd5 to
a83d33b
Compare
|
As noted offline this also forces newer Visual Studio as well. @marcpopMSFT exactly which VS version would be the minimum after this change? |
geeknoid
force-pushed
the
geeknoid/roslyn
branch
from
a83d33b to
2f4e8f2
Compare
| <ProjectReference Condition="'$(SkipExtraAnalyzers)' != 'true'" Include="$(MSBuildThisFileDirectory)\..\..\src\Analyzers\Microsoft.Analyzers.Extra\Microsoft.Analyzers.Extra.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" Private="false" IncludeAssets="runtime; build; native; contentfiles; analyzers; buildtransitive" /> | ||
| <ProjectReference Condition="'$(SkipLocalAnalyzers)' != 'true'" Include="$(MSBuildThisFileDirectory)\..\..\src\Analyzers\Microsoft.Analyzers.Local\Microsoft.Analyzers.Local.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" Private="false" IncludeAssets="runtime; build; native; contentfiles; analyzers; buildtransitive" /> |
There was a problem hiding this comment.
❤️ the change of the names
We should rename the other analyzers and generators too. /ducks
| <PackageVersion Include="Microsoft.CodeAnalysis.Common" Version="4.4.0" /> | ||
| <PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.4.0" /> | ||
| <PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="4.4.0" /> | ||
| <PackageVersion Include="Microsoft.CodeAnalysis" Version="4.4.0" /> |
There was a problem hiding this comment.
If all of these packages should be on the same version then it's worth considering introducing a variable (put it in /eng/Versions.props) and use the variable here.
Lines 73 to 81 in 131ada7
|
|
||
| <PropertyGroup> | ||
| <SkipLocalAnalyzers>true</SkipLocalAnalyzers> | ||
| <AnalyzerLanguage>cs</AnalyzerLanguage> | ||
| <AnalyzerRoslynVersion>4.0</AnalyzerRoslynVersion> | ||
| <DefineConstants>$(DefineConstants);ROSLYN_4_0_OR_GREATER</DefineConstants> | ||
| <AnalyzerLanguage>cs</AnalyzerLanguage> |
There was a problem hiding this comment.
There's an established naming convention in the SDK to suffix assemblies with the language. E.g.,
- Microsoft.Analyzers.Local.dll implies a language agnostic analyzers,
- Microsoft.Analyzers.Local.CSharp.dll implies C#-specific analyzers.
It'd worth considering adapting to the same convention.
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <AnalyzerLanguage>cs</AnalyzerLanguage> |
There was a problem hiding this comment.
Are we applies this property explicitly to all the analyzer projects? If so, consider declaring it centrally in one of the Directory.Build.props.
The 4.4 Roslyn matches with the 17.4 VS which ships with the 7.0.1xx SDK if that's what you're asking. |
|
Thanks @marcpopMSFT that was what I was asking. Essentially they need to have VS 2022 because if they're <17.4 they can presumably update. @ScottThurlow do we have any data on the versions of VS that internal folks use when consuming these bits? Put another way, are more than a trivial proportion using VS2019? btw I'm assuming this PR only concerns analyzers and generators that folks consuming our components need, I don't think we care about anything that's only relevant to building them themselves. |
|
eh, we can move this conversation to the original chat. I should have asked there. |
ghost
locked as resolved and limited conversation to collaborators
This means folks trying to use the generators and analyzers will need to be using at minimum the .NET 7 SDK.
Resolves #4127
Closes #4183 as superseded.
Microsoft Reviewers: Open in CodeFlow