Skip to content

Commit

Permalink
Add xUnit2027
Browse files Browse the repository at this point in the history
  • Loading branch information
@bradwilson
bradwilson committed Dec 25, 2023
1 parent 3903c5e commit ead9a3b
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 84 deletions.
190 changes: 135 additions & 55 deletions src/xunit.analyzers.tests/Analyzers/X2000/SetEqualityAnalyzerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,62 +4,38 @@

public class SetEqualityAnalyzerTests
{
const string customSet = @"
const string customSetAndComparer = @"
using System.Collections;
using System.Collections.Generic;
public class MySet : ISet<int> {
public int Count => throw new System.NotImplementedException();
public bool IsReadOnly => throw new System.NotImplementedException();
public bool Add(int item) => throw new System.NotImplementedException();
public void Clear() => throw new System.NotImplementedException();
public bool Contains(int item) => throw new System.NotImplementedException();
public void CopyTo(int[] array, int arrayIndex) => throw new System.NotImplementedException();
public void ExceptWith(IEnumerable<int> other) => throw new System.NotImplementedException();
public IEnumerator<int> GetEnumerator() => throw new System.NotImplementedException();
public void IntersectWith(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool IsProperSubsetOf(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool IsProperSupersetOf(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool IsSubsetOf(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool IsSupersetOf(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool Overlaps(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool Remove(int item) => throw new System.NotImplementedException();
public bool SetEquals(IEnumerable<int> other) => throw new System.NotImplementedException();
public void SymmetricExceptWith(IEnumerable<int> other) => throw new System.NotImplementedException();
public void UnionWith(IEnumerable<int> other) => throw new System.NotImplementedException();
void ICollection<int>.Add(int item) => throw new System.NotImplementedException();
IEnumerator IEnumerable.GetEnumerator() => throw new System.NotImplementedException();
}";

[Theory]
[InlineData("Equal", "List", "List")]
[InlineData("Equal", "HashSet", "List")]
[InlineData("Equal", "List", "HashSet")]
[InlineData("NotEqual", "List", "List")]
[InlineData("NotEqual", "HashSet", "List")]
[InlineData("NotEqual", "List", "HashSet")]
public async void ForSetWithNonSet_DoesNotTrigger(
string method,
string collection1Type,
string collection2Type)
{
var code = @$"
using Xunit;
using System.Collections.Generic;
public class TestClass {{
[Fact]
public void TestMethod() {{
var collection1 = new {collection1Type}<int>();
var collection2 = new {collection2Type}<int>();
Assert.{method}(collection1, collection2, (int e1, int e2) => true);
}}
}}";
public int Count => throw new System.NotImplementedException();
public bool IsReadOnly => throw new System.NotImplementedException();
public bool Add(int item) => throw new System.NotImplementedException();
public void Clear() => throw new System.NotImplementedException();
public bool Contains(int item) => throw new System.NotImplementedException();
public void CopyTo(int[] array, int arrayIndex) => throw new System.NotImplementedException();
public void ExceptWith(IEnumerable<int> other) => throw new System.NotImplementedException();
public IEnumerator<int> GetEnumerator() => throw new System.NotImplementedException();
public void IntersectWith(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool IsProperSubsetOf(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool IsProperSupersetOf(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool IsSubsetOf(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool IsSupersetOf(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool Overlaps(IEnumerable<int> other) => throw new System.NotImplementedException();
public bool Remove(int item) => throw new System.NotImplementedException();
public bool SetEquals(IEnumerable<int> other) => throw new System.NotImplementedException();
public void SymmetricExceptWith(IEnumerable<int> other) => throw new System.NotImplementedException();
public void UnionWith(IEnumerable<int> other) => throw new System.NotImplementedException();
void ICollection<int>.Add(int item) => throw new System.NotImplementedException();
IEnumerator IEnumerable.GetEnumerator() => throw new System.NotImplementedException();
}
await Verify.VerifyAnalyzer(code);
}
public class MyComparer : IEqualityComparer<int> {
public bool Equals(int x, int y) => throw new System.NotImplementedException();
public int GetHashCode(int obj) => throw new System.NotImplementedException();
}";

public class X2026_SetsMustBeComparedWithEqualityComparer
{
Expand Down Expand Up @@ -92,7 +68,7 @@ public void TestMethod() {{
}}
}}";

await Verify.VerifyAnalyzer(LanguageVersion.CSharp7, new[] { code, customSet });
await Verify.VerifyAnalyzer(LanguageVersion.CSharp7, new[] { code, customSetAndComparer });
}

[Theory]
Expand Down Expand Up @@ -130,7 +106,7 @@ public void TestMethod() {{
}}
}}";

await Verify.VerifyAnalyzer(LanguageVersion.CSharp7, new[] { code, customSet });
await Verify.VerifyAnalyzer(LanguageVersion.CSharp7, new[] { code, customSetAndComparer });
}

[Theory]
Expand Down Expand Up @@ -161,7 +137,7 @@ public void TestMethod() {{
.WithSpan(12, 9, 12, 68 + method.Length)
.WithArguments(method);

await Verify.VerifyAnalyzer(LanguageVersion.CSharp7, new[] { code, customSet }, expected);
await Verify.VerifyAnalyzer(LanguageVersion.CSharp7, new[] { code, customSetAndComparer }, expected);
}

#if ROSLYN_4_2_OR_GREATER // No C# 10 in Roslyn 3.11, so no local functions
Expand Down Expand Up @@ -217,10 +193,114 @@ bool LocalFunc(int obj1, int obj2)
.WithSpan(26, 9, 26, 44 + method.Length + comparerFuncSyntax.Length)
.WithArguments(method);

await Verify.VerifyAnalyzer(LanguageVersion.CSharp10, new[] { code, customSet }, expected);
await Verify.VerifyAnalyzer(LanguageVersion.CSharp10, new[] { code, customSetAndComparer }, expected);
}

#endif

}

public class X2027_SetsShouldNotBeComparedToLinearContainers
{
public static MatrixTheoryData<string, string> MethodAndLinearContainers =>
new(
new[] { "Equal", "NotEqual" },
new[] { "new List<int>()", "new SortedSet<int>()", "new HashSet<int>().OrderBy(x => x)", "new MySet().OrderBy(x => x)" }
);

[Theory]
[MemberData(nameof(MethodAndLinearContainers))]
public async void LinearContainers_DoesNotTrigger(
string method,
string collection)
{
var code = @$"
using Xunit;
using System.Collections.Generic;
using System.Linq;
public class TestClass {{
[Fact]
public void TestMethod() {{
var collection1 = new List<int>();
var collection2 = {collection};
Assert.{method}(collection1, collection2);
Assert.{method}(collection1, collection2, (int e1, int e2) => true);
Assert.{method}(collection1, collection2, new MyComparer());
}}
}}";

await Verify.VerifyAnalyzer(LanguageVersion.CSharp7, new[] { code, customSetAndComparer });
}

public static MatrixTheoryData<string, (string type, string initializer)> MethodAndTypeAndInitializer =>
new(
new[] { "Equal", "NotEqual" },
new[] {
("HashSet<int>", "new HashSet<int>()"),
("ImmutableHashSet<int>", "new HashSet<int>().ToImmutableHashSet()"),
("MySet", "new MySet()")
}
);

[Theory]
[MemberData(nameof(MethodAndTypeAndInitializer), DisableDiscoveryEnumeration = true)]
public async void SetWithLinearContainer_Triggers(
string method,
(string type, string initializer) collection)
{
var code = @$"
using Xunit;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
public class TestClass {{
[Fact]
public void TestMethod() {{
var collection1 = new List<int>();
var collection2 = {collection.initializer};
Assert.{method}(collection1, collection2);
Assert.{method}(collection1, collection2, (int e1, int e2) => true);
Assert.{method}(collection1, collection2, new MyComparer());
Assert.{method}(collection2, collection1);
Assert.{method}(collection2, collection1, (int e1, int e2) => true);
Assert.{method}(collection2, collection1, new MyComparer());
}}
}}";
var expected = new[]
{
Verify
.Diagnostic("xUnit2027")
.WithSpan(13, 9, 13, 42 + method.Length)
.WithArguments("List<int>", collection.type),
Verify
.Diagnostic("xUnit2027")
.WithSpan(14, 9, 14, 68 + method.Length)
.WithArguments("List<int>", collection.type),
Verify
.Diagnostic("xUnit2027")
.WithSpan(15, 9, 15, 60 + method.Length)
.WithArguments("List<int>", collection.type),

Verify
.Diagnostic("xUnit2027")
.WithSpan(17, 9, 17, 42 + method.Length)
.WithArguments(collection.type, "List<int>"),
Verify
.Diagnostic("xUnit2027")
.WithSpan(18, 9, 18, 68 + method.Length)
.WithArguments(collection.type, "List<int>"),
Verify
.Diagnostic("xUnit2027")
.WithSpan(19, 9, 19, 60 + method.Length)
.WithArguments(collection.type, "List<int>"),
};

await Verify.VerifyAnalyzer(LanguageVersion.CSharp7, new[] { code, customSetAndComparer }, expected);
}
}
}
9 changes: 8 additions & 1 deletion src/xunit.analyzers/Utility/Descriptors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,14 @@ static DiagnosticDescriptor Rule(
"Comparison of two sets may produce inconsistent results if GetHashCode() is not overriden. Consider using Assert.{0}(IEnumerable<T>, IEnumerable<T>, IEqualityComparer<T>) instead."
);

// Placeholder for rule X2027
public static DiagnosticDescriptor X2027_SetsShouldNotBeComparedToLinearContainers { get; } =
Rule(
"xUnit2027",
"Comparison of sets to linear containers have undefined results",
Assertions,
Warning,
"Comparing an instance of {0} with an instance of {1} has undefined results, because the order of items in the set is not predictable. Create a stable order for the set (i.e., by using OrderBy from Linq)."
);

// Placeholder for rule X2028

Expand Down
3 changes: 3 additions & 0 deletions src/xunit.analyzers/Utility/TypeSymbolFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ public static IArrayTypeSymbol ObjectArray(Compilation compilation) =>
public static INamedTypeSymbol? OptionalAttribute(Compilation compilation) =>
Guard.ArgumentNotNull(compilation).GetTypeByMetadataName("System.Runtime.InteropServices.OptionalAttribute");

public static INamedTypeSymbol? SortedSetOfT(Compilation compilation) =>
Guard.ArgumentNotNull(compilation).GetTypeByMetadataName("System.Collections.Generic.SortedSet`1");

public static INamedTypeSymbol? Task(Compilation compilation) =>
Guard.ArgumentNotNull(compilation).GetTypeByMetadataName("System.Threading.Tasks.Task");

Expand Down
91 changes: 63 additions & 28 deletions src/xunit.analyzers/X2000/SetEqualityAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ public class SetEqualityAnalyzer : AssertUsageAnalyzerBase
};

public SetEqualityAnalyzer()
: base(Descriptors.X2026_SetsMustBeComparedWithEqualityComparer, targetMethods)
: base(
new[] {
Descriptors.X2026_SetsMustBeComparedWithEqualityComparer,
Descriptors.X2027_SetsShouldNotBeComparedToLinearContainers,
},
targetMethods
)
{ }

protected override void AnalyzeInvocation(
Expand All @@ -33,10 +39,6 @@ protected override void AnalyzeInvocation(
Guard.ArgumentNotNull(invocationOperation);
Guard.ArgumentNotNull(method);

var arguments = invocationOperation.Arguments;
if (arguments.Length != 3)
return;

var semanticModel = context.Operation.SemanticModel;
if (semanticModel == null)
return;
Expand All @@ -45,15 +47,17 @@ protected override void AnalyzeInvocation(
var readOnlySetType = TypeSymbolFactory.IReadOnlySetOfT(context.Compilation)?.ConstructUnboundGenericType();
var setInterfaces = new HashSet<INamedTypeSymbol>(new[] { setType, readOnlySetType }.WhereNotNull(), SymbolEqualityComparer.Default);

var arguments = invocationOperation.Arguments;
if (arguments.Length < 2)
return;

if (semanticModel.GetTypeInfo(arguments[0].Value.Syntax).Type is not INamedTypeSymbol collection0Type)
return;
var interface0Type =
collection0Type
.AllInterfaces
.Where(i => i.IsGenericType)
.FirstOrDefault(i => setInterfaces.Contains(i.ConstructUnboundGenericType()));
if (interface0Type is null)
return;

if (semanticModel.GetTypeInfo(arguments[1].Value.Syntax).Type is not INamedTypeSymbol collection1Type)
return;
Expand All @@ -62,32 +66,63 @@ protected override void AnalyzeInvocation(
.AllInterfaces
.Where(i => i.IsGenericType)
.FirstOrDefault(i => setInterfaces.Contains(i.ConstructUnboundGenericType()));
if (interface1Type is null)
return;

if (arguments[2].Value is not IDelegateCreationOperation && arguments[2].Value is not ILocalReferenceOperation)
// No sets
if (interface0Type is null && interface1Type is null)
return;

if (arguments[2].Value.Type is not INamedTypeSymbol funcTypeSymbol || funcTypeSymbol.DelegateInvokeMethod == null)
return;
// Both sets, make sure they don't use the comparer function override
if (interface0Type is not null && interface1Type is not null)
{
if (arguments.Length != 3)
return;

var funcDelegate = funcTypeSymbol.DelegateInvokeMethod;
var isFuncOverload =
funcDelegate.ReturnType.SpecialType == SpecialType.System_Boolean &&
funcDelegate.Parameters.Length == 2 &&
funcDelegate.Parameters[0].Type.Equals(interface0Type.TypeArguments[0], SymbolEqualityComparer.Default) &&
funcDelegate.Parameters[1].Type.Equals(interface1Type.TypeArguments[0], SymbolEqualityComparer.Default);
if (arguments[2].Value is not IDelegateCreationOperation && arguments[2].Value is not ILocalReferenceOperation)
return;

// Wrong method overload
if (!isFuncOverload)
return;
if (arguments[2].Value.Type is not INamedTypeSymbol funcTypeSymbol || funcTypeSymbol.DelegateInvokeMethod == null)
return;

context.ReportDiagnostic(
Diagnostic.Create(
Descriptors.X2026_SetsMustBeComparedWithEqualityComparer,
invocationOperation.Syntax.GetLocation(),
method.Name
)
);
var funcDelegate = funcTypeSymbol.DelegateInvokeMethod;
var isFuncOverload =
funcDelegate.ReturnType.SpecialType == SpecialType.System_Boolean &&
funcDelegate.Parameters.Length == 2 &&
funcDelegate.Parameters[0].Type.Equals(interface0Type.TypeArguments[0], SymbolEqualityComparer.Default) &&
funcDelegate.Parameters[1].Type.Equals(interface1Type.TypeArguments[0], SymbolEqualityComparer.Default);

// Wrong method overload
if (!isFuncOverload)
return;

context.ReportDiagnostic(
Diagnostic.Create(
Descriptors.X2026_SetsMustBeComparedWithEqualityComparer,
invocationOperation.Syntax.GetLocation(),
method.Name
)
);
}
// One set, one linear container
else
{
// Make a special allowance for SortedSet<>, since we know it's sorted
var sortedSet = TypeSymbolFactory.SortedSetOfT(context.Compilation);
if (sortedSet is not null)
{
if (interface0Type is not null && sortedSet.Construct(interface0Type.TypeArguments[0]).IsAssignableFrom(collection0Type))
return;
if (interface1Type is not null && sortedSet.Construct(interface1Type.TypeArguments[0]).IsAssignableFrom(collection1Type))
return;
}

context.ReportDiagnostic(
Diagnostic.Create(
Descriptors.X2027_SetsShouldNotBeComparedToLinearContainers,
invocationOperation.Syntax.GetLocation(),
collection0Type.ToMinimalDisplayString(semanticModel, 0),
collection1Type.ToMinimalDisplayString(semanticModel, 0)
)
);
}
}
}

0 comments on commit ead9a3b

Please sign in to comment.