Skip to content

Commit

Permalink
xunit/xunit#2943: Revert to only supporting Roslyn 3.11
Browse files Browse the repository at this point in the history
  • Loading branch information
@bradwilson
bradwilson committed Jun 4, 2024
1 parent 84d0e79 commit 24e3c1c
Show file tree
Hide file tree
Showing 22 changed files with 8 additions and 312 deletions.
30 changes: 1 addition & 29 deletions src/Directory.Build.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,43 +19,15 @@
<LangVersion>12.0</LangVersion>
<MSBuildCopyContentTransitively>false</MSBuildCopyContentTransitively>
<Nullable>enable</Nullable>
<RoslynVersion>4.8.0</RoslynVersion>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>

<PropertyGroup Condition="$(MSBuildProjectName.StartsWith('xunit.analyzers.roslyn'))">
<AssemblyName>xunit.analyzers</AssemblyName>
</PropertyGroup>

<PropertyGroup Condition="$(MSBuildProjectName.StartsWith('xunit.analyzers.fixes.roslyn'))">
<AssemblyName>xunit.analyzers.fixes</AssemblyName>
</PropertyGroup>

<PropertyGroup Condition="$(MSBuildProjectName.Contains('roslyn311'))">
<DefineConstants>$(DefineConstants);ROSLYN_3_11;ROSLYN_3_11_OR_GREATER</DefineConstants>
<RoslynVersion>3.11.0</RoslynVersion>
</PropertyGroup>

<PropertyGroup Condition="$(MSBuildProjectName.Contains('roslyn44'))">
<DefineConstants>$(DefineConstants);ROSLYN_4_4;ROSLYN_3_11_OR_GREATER;ROSLYN_4_4_OR_GREATER</DefineConstants>
<RoslynVersion>4.4.0</RoslynVersion>
</PropertyGroup>

<PropertyGroup Condition="$(MSBuildProjectName.Contains('roslyn46'))">
<DefineConstants>$(DefineConstants);ROSLYN_4_6;ROSLYN_3_11_OR_GREATER;ROSLYN_4_4_OR_GREATER;ROSLYN_4_6_OR_GREATER</DefineConstants>
<RoslynVersion>4.6.0</RoslynVersion>
</PropertyGroup>

<PropertyGroup Condition=" '$(RoslynVersion)' == '4.8.0' ">
<DefineConstants>$(DefineConstants);ROSLYN_4_8;ROSLYN_3_11_OR_GREATER;ROSLYN_4_4_OR_GREATER;ROSLYN_4_6_OR_GREATER;ROSLYN_4_8_OR_GREATER</DefineConstants>
</PropertyGroup>

<ItemGroup>
<Compile Include="$(MSBuildThisFileDirectory)common\*.cs" LinkBase="Utility\Common" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis" Version="[$(RoslynVersion)]" />
<PackageReference Include="Microsoft.CodeAnalysis" Version="[3.11]" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="all" />
<PackageReference Include="Nerdbank.GitVersioning" Version="3.6.133">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand Down
16 changes: 0 additions & 16 deletions src/xunit.analyzers.fixes.roslyn311/xunit.analyzers.fixes.roslyn311.csproj
View file

This file was deleted.

16 changes: 0 additions & 16 deletions src/xunit.analyzers.fixes.roslyn44/xunit.analyzers.fixes.roslyn44.csproj
View file

This file was deleted.

16 changes: 0 additions & 16 deletions src/xunit.analyzers.fixes.roslyn46/xunit.analyzers.fixes.roslyn46.csproj
View file

This file was deleted.

12 changes: 0 additions & 12 deletions src/xunit.analyzers.roslyn311/xunit.analyzers.roslyn311.csproj
View file

This file was deleted.

12 changes: 0 additions & 12 deletions src/xunit.analyzers.roslyn44/xunit.analyzers.roslyn44.csproj
View file

This file was deleted.

12 changes: 0 additions & 12 deletions src/xunit.analyzers.roslyn46/xunit.analyzers.roslyn46.csproj
View file

This file was deleted.

17 changes: 0 additions & 17 deletions src/xunit.analyzers.tests.roslyn311/xunit.analyzers.tests.roslyn311.csproj
View file

This file was deleted.

17 changes: 0 additions & 17 deletions src/xunit.analyzers.tests.roslyn44/xunit.analyzers.tests.roslyn44.csproj
View file

This file was deleted.

17 changes: 0 additions & 17 deletions src/xunit.analyzers.tests.roslyn46/xunit.analyzers.tests.roslyn46.csproj
View file

This file was deleted.

12 changes: 1 addition & 11 deletions src/xunit.analyzers.tests/Suppressors/ConsiderCallingConfigureAwaitSuppressorTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
#if NETCOREAPP

using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Testing;
using Xunit;
using Xunit.Analyzers;
using Verify = CSharpVerifier<Xunit.Suppressors.ConsiderCallingConfigureAwaitSuppressor>;

#if ROSLYN_3_11
using Microsoft.CodeAnalysis;
#else
using System;
#endif

public sealed class ConsiderCallingConfigureAwaitSuppressorTests
{
[Fact]
Expand Down Expand Up @@ -48,16 +43,11 @@ public async Task TestMethod() {{
}}
}}";

#if ROSLYN_3_11
// Roslyn 3.11 still surfaces the diagnostic that has been suppressed
var expected =
new DiagnosticResult("CA2007", DiagnosticSeverity.Warning)
.WithSpan(8, 15, 8, 28)
.WithIsSuppressed(true);
#else
// Later versions of Roslyn just removes it from the results
var expected = Array.Empty<DiagnosticResult>();
#endif

This comment has been minimized.

Sign in to view

Copy link
@sharwell

sharwell Jun 4, 2024

Contributor

There was a bug in the test library for a long time which could impact this behavior. I believe the "3.11" behavior you have here is the correct behavior (using WithIsSuppressed(true)), and new versions of the test library should make this work across additional versions.

await Verify.VerifySuppressor(code, CodeAnalysisNetAnalyzers.CA2007(), expected);
}
Expand Down
9 changes: 0 additions & 9 deletions src/xunit.analyzers.tests/Suppressors/MakeTypesInternalSuppressorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@
using Xunit.Analyzers;
using Verify = CSharpVerifier<Xunit.Suppressors.MakeTypesInternalSuppressor>;

#if !ROSLYN_3_11
using System;
#endif

public sealed class MakeTypesInternalSuppressorTests
{
[Fact]
Expand Down Expand Up @@ -48,16 +44,11 @@ public class TestClass {{
public void TestMethod() {{ }}
}}";

#if ROSLYN_3_11
// Roslyn 3.11 still surfaces the diagnostic that has been suppressed
var expected =
new DiagnosticResult("CA1515", DiagnosticSeverity.Warning)
.WithSpan(6, 14, 6, 23)
.WithIsSuppressed(true);
#else
// Later versions of Roslyn just removes it from the results
var expected = Array.Empty<DiagnosticResult>();
#endif

await Verify.VerifySuppressor(code, CodeAnalysisNetAnalyzers.CA1515(), expected);
}
Expand Down
10 changes: 0 additions & 10 deletions src/xunit.analyzers/Utility/CodeAnalysisExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,6 @@ namespace Xunit.Analyzers;

static class CodeAnalysisExtensions
{
#if ROSLYN_4_4_OR_GREATER
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static IOperation.OperationList Children(this IOperation operation) =>
operation.ChildOperations;

This comment has been minimized.

Sign in to view

Copy link
@sharwell

sharwell Jun 4, 2024

Contributor

📝 If this change causes significant performance overhead in some future measurable scenario, we should be able to address it without updating the Roslyn version by using light-up. It's a bit complex since we can't expose the OperationList type directly from light-up, but I have other tricks. 😂

This comment has been minimized.

Sign in to view

Copy link
@bradwilson

bradwilson Jun 4, 2024

Author Member

With VS 2019 into just extended support I thought I could drop Roslyn 3.11 but I did an informal poll and there seems to still be a small but significant portion of people tied to VS 2019 (one cited reason is Unreal Engine).

This comment has been minimized.

Sign in to view

Copy link
@sharwell

sharwell Jun 4, 2024

Contributor

I think Unity also builds with Roslyn 3

This comment has been minimized.

Sign in to view

Copy link
@bradwilson

bradwilson Jun 5, 2024

Author Member

I think I probably meant Unity 😂
#else
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static IEnumerable<IOperation> Children(this IOperation operation) =>
operation.Children;
#endif

public static INamedTypeSymbol? FindNamedType(
this IAssemblySymbol assembly,
Func<INamedTypeSymbol, bool> selector)
Expand Down
10 changes: 4 additions & 6 deletions src/xunit.analyzers/X1000/DoNotUseBlockingTaskOperations.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#if ROSLYN_3_11
#pragma warning disable RS1024 // Incorrectly triggered by Roslyn 3.11

This comment has been minimized.

Sign in to view

Copy link
@sharwell

sharwell Jun 4, 2024

Contributor

The Roslyn analyzers can be updated separately from the rest of the Roslyn references. A false positive of this warning need not impact this project, even if this project references Roslyn 3.11.

This comment has been minimized.

Sign in to view

Copy link
@bradwilson

bradwilson Jun 4, 2024

Author Member

What package do I need to reference to pull the newer analyzers?

This comment has been minimized.

Sign in to view

Copy link
@sharwell

sharwell Jun 4, 2024

Contributor

Microsoft.CodeAnalysis.Analyzers

This comment has been minimized.

Sign in to view

Copy link
@sharwell

sharwell Jun 4, 2024

Contributor

It's a transitive dependency of the Roslyn packages, but you can manually upgrade it using a direct reference

This comment has been minimized.

Sign in to view

Copy link
@bradwilson

bradwilson Jun 5, 2024

Author Member

Thanks, using 3.3.4 does resolve the issue for me. I can re-enable RS1024.

On a side note: Should I use a beta version? There have been quite a few in the 16 months since 3.3.4 was released.

This comment has been minimized.

Sign in to view

Copy link
@sharwell

sharwell Jun 5, 2024

Contributor

I typically do, since it's the fastest way to apply a bug fix if there's ever a problem. It's also the first place performance improvements are applied.
#endif

using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -224,7 +222,7 @@ static bool TaskIsKnownToBeCompleted(

bool validateSafeTasks(IOperation op)
{
foreach (var childOperation in op.Children())
foreach (var childOperation in op.Children)
{
// Stop looking once we've found the operation that is ours, since any
// code after that operation isn't something we should consider
Expand All @@ -243,7 +241,7 @@ bool validateSafeTasks(IOperation op)
if (unfoundSymbols.Count == 0)
return true;

if (childOperation.Children().Any(c => validateSafeTasks(c)))
if (childOperation.Children.Any(c => validateSafeTasks(c)))
return true;
}

Expand All @@ -270,9 +268,9 @@ static void ValidateTaskFromWhenAny(
{
if (!unfoundSymbols.Contains(operation.Symbol))
return;
if (operation.Children().FirstOrDefault() is not IVariableInitializerOperation variableInitializerOperation)
if (operation.Children.FirstOrDefault() is not IVariableInitializerOperation variableInitializerOperation)
return;
if (variableInitializerOperation.Value.Children().FirstOrDefault() is not IInvocationOperation variableInitializerInvocationOperation)
if (variableInitializerOperation.Value.Children.FirstOrDefault() is not IInvocationOperation variableInitializerInvocationOperation)
return;
if (!FindSymbol(variableInitializerInvocationOperation.TargetMethod, variableInitializerInvocationOperation, taskType, whenAny, xunitContext, out var _))
return;
Expand Down
2 changes: 0 additions & 2 deletions src/xunit.analyzers/X1000/EnsureFixturesHaveASource.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#if ROSLYN_3_11
#pragma warning disable RS1024 // Incorrectly triggered by Roslyn 3.11
#endif

using System.Collections.Generic;
using System.Collections.Immutable;
Expand Down
2 changes: 0 additions & 2 deletions src/xunit.analyzers/X1000/TestMethodCannotHaveOverloads.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#if ROSLYN_3_11
#pragma warning disable RS1024 // Incorrectly triggered by Roslyn 3.11
#endif

using System.Collections.Generic;
using System.Linq;
Expand Down
2 changes: 0 additions & 2 deletions src/xunit.analyzers/X1000/TestMethodMustNotHaveMultipleFactAttributes.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#if ROSLYN_3_11
#pragma warning disable RS1024 // Incorrectly triggered by Roslyn 3.11
#endif

using System.Collections.Generic;
using System.Collections.Immutable;
Expand Down
12 changes: 0 additions & 12 deletions src/xunit.analyzers/X1000/TheoryDataRowArgumentsShouldBeSerializable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,7 @@ argumentOperation.SemanticModel is null

for (var idx = 0; idx < objectCreation.Arguments.Length; ++idx)
{
#if ROSLYN_3_11
var elementValue = objectCreation.Arguments[idx].Children.FirstOrDefault();
#else
var elementValue = objectCreation.Arguments[idx].ChildOperations.FirstOrDefault();
#endif
while (elementValue is IConversionOperation conversion)
elementValue = conversion.Operand;

Expand All @@ -107,21 +103,13 @@ argumentOperation.SemanticModel is null
if (objectCreation.Arguments.FirstOrDefault() is not IArgumentOperation argumentOperation)
return null;

#if ROSLYN_3_11
var firstArgument = argumentOperation.Children.FirstOrDefault();
#else
var firstArgument = argumentOperation.ChildOperations.FirstOrDefault();
#endif
if (firstArgument is null)
return null;

// Common pattern: implicit array creation for the params array
if (firstArgument is IArrayCreationOperation arrayCreation &&
#if ROSLYN_3_11
arrayCreation.Children.Skip(1).FirstOrDefault() is IArrayInitializerOperation arrayInitializer)
#else
arrayCreation.ChildOperations.Skip(1).FirstOrDefault() is IArrayInitializerOperation arrayInitializer)
#endif
{
var result = new List<IOperation>();

Expand Down
2 changes: 0 additions & 2 deletions src/xunit.analyzers/X1000/TheoryMethodShouldUseAllParameters.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#if ROSLYN_3_11
#pragma warning disable RS1024 // Incorrectly triggered by Roslyn 3.11
#endif

using System.Collections.Generic;
using System.Linq;
Expand Down
2 changes: 0 additions & 2 deletions src/xunit.analyzers/X2000/SetEqualityAnalyzer.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#if ROSLYN_3_11
#pragma warning disable RS1024 // Incorrectly triggered by Roslyn 3.11
#endif

using System.Collections.Generic;
using System.Linq;
Expand Down
Loading

0 comments on commit 24e3c1c

Please sign in to comment.