From 81de76985e4a0b68a9adfe49241e8f18d1904879 Mon Sep 17 00:00:00 2001 From: etherfield <148793545+etherfield@users.noreply.github.com> Date: Sun, 26 Nov 2023 21:16:28 -0800 Subject: [PATCH] Detect when CollectionDefinition class is not in same assembly as test class (xunit/xunit#2311) (#169) --- ...nDefinitionMustBeInTheSameAssemblyTests.cs | 57 ++++++++++++++++++ src/xunit.analyzers/Utility/Descriptors.cs | 9 ++- .../Utility/EmptyCoreContext.cs | 2 + src/xunit.analyzers/Utility/ICoreContext.cs | 2 + .../Utility/SymbolAssemblyVisitor.cs | 60 +++++++++++++++++++ .../Utility/TypeSymbolFactory.cs | 3 + src/xunit.analyzers/Utility/V2CoreContext.cs | 5 ++ src/xunit.analyzers/Utility/V3CoreContext.cs | 5 ++ ...ectionDefinitionMustBeInTheSameAssembly.cs | 58 ++++++++++++++++++ 9 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 src/xunit.analyzers.tests/Analyzers/X1000/CollectionDefinitionMustBeInTheSameAssemblyTests.cs create mode 100644 src/xunit.analyzers/Utility/SymbolAssemblyVisitor.cs create mode 100644 src/xunit.analyzers/X1000/CollectionDefinitionMustBeInTheSameAssembly.cs diff --git a/src/xunit.analyzers.tests/Analyzers/X1000/CollectionDefinitionMustBeInTheSameAssemblyTests.cs b/src/xunit.analyzers.tests/Analyzers/X1000/CollectionDefinitionMustBeInTheSameAssemblyTests.cs new file mode 100644 index 00000000..5c97dad3 --- /dev/null +++ b/src/xunit.analyzers.tests/Analyzers/X1000/CollectionDefinitionMustBeInTheSameAssemblyTests.cs @@ -0,0 +1,57 @@ +using Microsoft.CodeAnalysis; +using Xunit; +using Verify = CSharpVerifier; + +public class CollectionDefinitionMustBeInTheSameAssemblyTests +{ + public static TheoryData NoDiagnosticsCases = new() + { + { + "[Collection(\"Test collection definition\")]", + "[CollectionDefinition(\"Test collection definition\")]" + }, + { + string.Empty, + "[CollectionDefinition(\"Test collection definition\")]" + }, + { + string.Empty, + string.Empty + } + }; + + static readonly string Template = @" +using Xunit; + +{0} +public class TestClass {{ }} + +namespace TestNamespace {{ + {1} + public class TestDefinition {{ }} +}}"; + + [Theory] + [MemberData(nameof(NoDiagnosticsCases))] + public async void CollectionDefinitionIsPresentInTheAssembly_NoDiagnostics(string classAttribute, string definitionAttribute) + { + var source = string.Format(Template, classAttribute, definitionAttribute); + + await Verify.VerifyAnalyzer(source); + } + + [Fact] + public async void CollectionDefinitionIsMissingInTheAssembly_ReturnsError() + { + var source = string.Format(Template, "[Collection(\"Test collection definition\")]", string.Empty); + + var expected = + Verify + .Diagnostic() + .WithSpan(5, 14, 5, 23) + .WithSeverity(DiagnosticSeverity.Error) + .WithArguments("Test collection definition", "TestProject"); + + await Verify.VerifyAnalyzer(source, expected); + } +} diff --git a/src/xunit.analyzers/Utility/Descriptors.cs b/src/xunit.analyzers/Utility/Descriptors.cs index da7aee2e..de793ba1 100644 --- a/src/xunit.analyzers/Utility/Descriptors.cs +++ b/src/xunit.analyzers/Utility/Descriptors.cs @@ -404,7 +404,14 @@ static DiagnosticDescriptor Rule( "The type argument {0} from {1}.{2} is nullable, while the type of the corresponding test method parameter {3} is not. Make the TheoryData type non-nullable, or make the test method parameter nullable." ); - // Placeholder for rule X1041 + public static DiagnosticDescriptor X1041_CollectionDefinitionMustBeInTheSameAssembly { get; } = + Rule( + "xUnit1041", + "Collection definitions must be in the same assembly as the test that uses them", + Extensibility, + Error, + "A class for '{0}' collection definition must be declared in the '{1}' assembly" + ); // Placeholder for rule X1042 diff --git a/src/xunit.analyzers/Utility/EmptyCoreContext.cs b/src/xunit.analyzers/Utility/EmptyCoreContext.cs index 7c0d265e..b81f5cd4 100644 --- a/src/xunit.analyzers/Utility/EmptyCoreContext.cs +++ b/src/xunit.analyzers/Utility/EmptyCoreContext.cs @@ -9,6 +9,8 @@ public class EmptyCoreContext : ICoreContext public INamedTypeSymbol? ClassDataAttributeType => null; + public INamedTypeSymbol? CollectionAttributeType => null; + public INamedTypeSymbol? CollectionDefinitionAttributeType => null; public INamedTypeSymbol? DataAttributeType => null; diff --git a/src/xunit.analyzers/Utility/ICoreContext.cs b/src/xunit.analyzers/Utility/ICoreContext.cs index 6a4a6bb1..f7c01d5b 100644 --- a/src/xunit.analyzers/Utility/ICoreContext.cs +++ b/src/xunit.analyzers/Utility/ICoreContext.cs @@ -6,6 +6,8 @@ public interface ICoreContext { INamedTypeSymbol? ClassDataAttributeType { get; } + INamedTypeSymbol? CollectionAttributeType { get; } + INamedTypeSymbol? CollectionDefinitionAttributeType { get; } INamedTypeSymbol? DataAttributeType { get; } diff --git a/src/xunit.analyzers/Utility/SymbolAssemblyVisitor.cs b/src/xunit.analyzers/Utility/SymbolAssemblyVisitor.cs new file mode 100644 index 00000000..a94d17ad --- /dev/null +++ b/src/xunit.analyzers/Utility/SymbolAssemblyVisitor.cs @@ -0,0 +1,60 @@ +using System; +using System.Linq; +using Microsoft.CodeAnalysis; + +namespace Xunit.Analyzers; + +/// +/// Visits every type in every namespace within an assembly. Expressions are provided which indicate +/// whether the item being searched for has been found. Searches stop once at least one of the +/// short circuit expressions returned true. +/// +public class SymbolAssemblyVisitor : SymbolVisitor +{ + readonly Func[] shortCircuitExpressions; + + public SymbolAssemblyVisitor(params Func[] shortCircuitExpressions) + { + this.shortCircuitExpressions = shortCircuitExpressions; + } + + public bool ShortCircuitTriggered { get; private set; } + + public override void VisitAssembly(IAssemblySymbol symbol) + { + symbol.GlobalNamespace.Accept(this); + } + + public override void VisitNamespace(INamespaceSymbol symbol) + { + var namespaceOrTypes = symbol.GetMembers(); + foreach (var member in namespaceOrTypes) + { + if (ShortCircuitTriggered) + return; + + member.Accept(this); + } + } + + public override void VisitNamedType(INamedTypeSymbol symbol) + { + if (shortCircuitExpressions.Any(e => e(symbol))) + { + ShortCircuitTriggered = true; + return; + } + + var nestedTypes = symbol.GetTypeMembers(); + if (nestedTypes.IsDefaultOrEmpty) + return; + + foreach (var nestedType in nestedTypes) + { + if (ShortCircuitTriggered) + return; + + nestedType.Accept(this); + } + } +} diff --git a/src/xunit.analyzers/Utility/TypeSymbolFactory.cs b/src/xunit.analyzers/Utility/TypeSymbolFactory.cs index 2ae87b5a..19fb8e24 100644 --- a/src/xunit.analyzers/Utility/TypeSymbolFactory.cs +++ b/src/xunit.analyzers/Utility/TypeSymbolFactory.cs @@ -11,6 +11,9 @@ public class TypeSymbolFactory public static INamedTypeSymbol? ClassDataAttribute(Compilation compilation) => compilation.GetTypeByMetadataName("Xunit.ClassDataAttribute"); + public static INamedTypeSymbol? CollectionAttribute(Compilation compilation) => + compilation.GetTypeByMetadataName("Xunit.CollectionAttribute"); + public static INamedTypeSymbol? CollectionDefinitionAttribute(Compilation compilation) => compilation.GetTypeByMetadataName("Xunit.CollectionDefinitionAttribute"); diff --git a/src/xunit.analyzers/Utility/V2CoreContext.cs b/src/xunit.analyzers/Utility/V2CoreContext.cs index c7c9b600..b2217d47 100644 --- a/src/xunit.analyzers/Utility/V2CoreContext.cs +++ b/src/xunit.analyzers/Utility/V2CoreContext.cs @@ -10,6 +10,7 @@ public class V2CoreContext : ICoreContext internal static readonly Version Version_2_4_0 = new("2.4.0"); readonly Lazy lazyClassDataAttributeType; + readonly Lazy lazyCollectionAttributeType; readonly Lazy lazyCollectionDefinitionAttributeType; readonly Lazy lazyDataAttributeType; readonly Lazy lazyFactAttributeType; @@ -26,6 +27,7 @@ public class V2CoreContext : ICoreContext Version = version; lazyClassDataAttributeType = new(() => TypeSymbolFactory.ClassDataAttribute(compilation)); + lazyCollectionAttributeType = new(() => TypeSymbolFactory.CollectionAttribute(compilation)); lazyCollectionDefinitionAttributeType = new(() => TypeSymbolFactory.CollectionDefinitionAttribute(compilation)); lazyDataAttributeType = new(() => TypeSymbolFactory.DataAttribute(compilation)); lazyFactAttributeType = new(() => TypeSymbolFactory.FactAttribute(compilation)); @@ -39,6 +41,9 @@ public class V2CoreContext : ICoreContext public INamedTypeSymbol? ClassDataAttributeType => lazyClassDataAttributeType.Value; + public INamedTypeSymbol? CollectionAttributeType => + lazyCollectionAttributeType.Value; + public INamedTypeSymbol? CollectionDefinitionAttributeType => lazyCollectionDefinitionAttributeType.Value; diff --git a/src/xunit.analyzers/Utility/V3CoreContext.cs b/src/xunit.analyzers/Utility/V3CoreContext.cs index 4012bdf9..1b0e084c 100644 --- a/src/xunit.analyzers/Utility/V3CoreContext.cs +++ b/src/xunit.analyzers/Utility/V3CoreContext.cs @@ -7,6 +7,7 @@ namespace Xunit.Analyzers; public class V3CoreContext : ICoreContext { readonly Lazy lazyClassDataAttributeType; + readonly Lazy lazyCollectionAttributeType; readonly Lazy lazyCollectionDefinitionAttributeType; readonly Lazy lazyDataAttributeType; readonly Lazy lazyFactAttributeType; @@ -23,6 +24,7 @@ public class V3CoreContext : ICoreContext Version = version; lazyClassDataAttributeType = new(() => TypeSymbolFactory.ClassDataAttribute(compilation)); + lazyCollectionAttributeType = new(() => TypeSymbolFactory.CollectionAttribute(compilation)); lazyCollectionDefinitionAttributeType = new(() => TypeSymbolFactory.CollectionDefinitionAttribute(compilation)); lazyDataAttributeType = new(() => TypeSymbolFactory.DataAttribute(compilation)); lazyFactAttributeType = new(() => TypeSymbolFactory.FactAttribute(compilation)); @@ -36,6 +38,9 @@ public class V3CoreContext : ICoreContext public INamedTypeSymbol? ClassDataAttributeType => lazyClassDataAttributeType.Value; + public INamedTypeSymbol? CollectionAttributeType => + lazyCollectionAttributeType.Value; + public INamedTypeSymbol? CollectionDefinitionAttributeType => lazyCollectionDefinitionAttributeType.Value; diff --git a/src/xunit.analyzers/X1000/CollectionDefinitionMustBeInTheSameAssembly.cs b/src/xunit.analyzers/X1000/CollectionDefinitionMustBeInTheSameAssembly.cs new file mode 100644 index 00000000..76fcf8a5 --- /dev/null +++ b/src/xunit.analyzers/X1000/CollectionDefinitionMustBeInTheSameAssembly.cs @@ -0,0 +1,58 @@ +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Xunit.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class CollectionDefinitionMustBeInTheSameAssembly : XunitDiagnosticAnalyzer +{ + public CollectionDefinitionMustBeInTheSameAssembly() : + base(Descriptors.X1041_CollectionDefinitionMustBeInTheSameAssembly) + { } + + public override void AnalyzeCompilation( + CompilationStartAnalysisContext context, + XunitContext xunitContext) + { + context.RegisterSymbolAction(context => + { + if (context.Symbol is not INamedTypeSymbol namedType) + return; + + var collectionAttributeType = xunitContext.Core.CollectionAttributeType; + var collectionAttribute = namedType + .GetAttributes() + .FirstOrDefault(a => a.AttributeClass.IsAssignableFrom(collectionAttributeType)); + + var collectionDefinitionName = collectionAttribute?.ConstructorArguments[0].Value?.ToString(); + if (collectionDefinitionName == null) + return; + + var collectionDefinitionAttributeType = xunitContext.Core.CollectionDefinitionAttributeType; + var visitor = new SymbolAssemblyVisitor(symbol => + symbol + .GetAttributes() + .Any(a => + a.AttributeClass.IsAssignableFrom(collectionDefinitionAttributeType) && + !a.ConstructorArguments.IsDefaultOrEmpty && + a.ConstructorArguments[0].Value?.ToString() == collectionDefinitionName + ) + ); + + var currentAssembly = context.Compilation.Assembly; + visitor.Visit(currentAssembly); + if (visitor.ShortCircuitTriggered) + return; + + context.ReportDiagnostic( + Diagnostic.Create( + Descriptors.X1041_CollectionDefinitionMustBeInTheSameAssembly, + namedType.Locations.First(), + collectionDefinitionName, + currentAssembly.Name + ) + ); + }, SymbolKind.NamedType); + } +}