diff --git a/src/xunit.analyzers.tests/Analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublicTests.cs b/src/xunit.analyzers.tests/Analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublicTests.cs new file mode 100644 index 00000000..4829d0b4 --- /dev/null +++ b/src/xunit.analyzers.tests/Analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublicTests.cs @@ -0,0 +1,166 @@ +using Microsoft.CodeAnalysis; +using Xunit; +using Verify = CSharpVerifier; + +public class ConstructorsOnFactAttributeSubclassShouldBePublicTests +{ + [Fact] + public async void DefaultConstructor_DoesNotTrigger() + { + var source = @" +using System; +using Xunit; + +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] +internal sealed class CustomFactAttribute : FactAttribute { } + +public class Tests { + [CustomFact] + public void TestCustomFact() { } + + [Fact] + public void TestFact() { } +}"; + + await Verify.VerifyAnalyzer(source); + } + + [Fact] + public async void ParameterlessPublicConstructor_DoesNotTrigger() + { + var source = @" +using System; +using Xunit; + +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] +internal sealed class CustomFactAttribute : FactAttribute { + public CustomFactAttribute() { + this.Skip = ""xxx""; + } +} + +public class Tests { + [CustomFact] + public void TestCustomFact() { } + + [Fact] + public void TestFact() { } +}"; + + await Verify.VerifyAnalyzer(source); + } + + [Fact] + public async void PublicConstructorWithParameters_DoesNotTrigger() + { + var source = @" +using System; +using Xunit; + +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] +internal sealed class CustomFactAttribute : FactAttribute { + public CustomFactAttribute(string skip) { + this.Skip = skip; + } +} + +public class Tests { + [CustomFact(""blah"")] + public void TestCustomFact() { } + + [Fact] + public void TestFact() { } +}"; + + await Verify.VerifyAnalyzer(source); + } + + [Fact] + public async void PublicConstructorWithOtherConstructors_DoesNotTrigger() + { + var source = @" +using System; +using Xunit; + +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] +internal sealed class CustomFactAttribute : FactAttribute { + public CustomFactAttribute() { + this.Skip = ""xxx""; + } + + internal CustomFactAttribute(string skip) { + this.Skip = skip; + } +} + +public class Tests { + [CustomFact] + public void TestCustomFact() { } + + [Fact] + public void TestFact() { } +}"; + + await Verify.VerifyAnalyzer(source); + } + + [Fact] + public async void InternalConstructor_Triggers() + { + var source = @" +using System; +using Xunit; + +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] +internal sealed class CustomFactAttribute : FactAttribute { + internal CustomFactAttribute(string skip, params int[] values) { } +} + +public class Tests { + [CustomFact(""Skip"", 42)] + public void TestCustomFact() { } + + [Fact] + public void TestFact() { } +}"; + var expected = + Verify + .Diagnostic() + .WithSeverity(DiagnosticSeverity.Error) + .WithSpan(11, 6, 11, 28) + .WithArguments("CustomFactAttribute.CustomFactAttribute(string, params int[])"); + + await Verify.VerifyAnalyzer(source, expected); + } + + [Fact] + public async void ProtectedInternalConstructor_Triggers() + { + var source = @" +using System; +using Xunit; + +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] +internal sealed class CustomFactAttribute : FactAttribute { + protected internal CustomFactAttribute() { + this.Skip = ""xxx""; + } +} + +public class Tests { + [CustomFact] + public void TestCustomFact() { } + + [Fact] + public void TestFact() { } +}"; + var expected = + Verify + .Diagnostic() + .WithSeverity(DiagnosticSeverity.Error) + .WithSpan(13, 6, 13, 16) + .WithArguments("CustomFactAttribute.CustomFactAttribute()"); + + await Verify.VerifyAnalyzer(source, expected); + } +} diff --git a/src/xunit.analyzers/Utility/Descriptors.cs b/src/xunit.analyzers/Utility/Descriptors.cs index 8e83a8cd..8142f9a5 100644 --- a/src/xunit.analyzers/Utility/Descriptors.cs +++ b/src/xunit.analyzers/Utility/Descriptors.cs @@ -421,7 +421,14 @@ static DiagnosticDescriptor Rule( "The member referenced by the MemberData attribute returns untyped data rows, such as object[]. Consider using TheoryData<> as the return type to provide better type safety." ); - // Placeholder for rule X1043 + public static DiagnosticDescriptor X1043_ConstructorOnFactAttributeSubclassShouldBePublic { get; } = + Rule( + "xUnit1043", + "Constructors on classes derived from FactAttribute must be public when used on test methods", + Usage, + Error, + "Constructor '{0}' must be public to be used on a test method." + ); // Placeholder for rule X1044 diff --git a/src/xunit.analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublic.cs b/src/xunit.analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublic.cs new file mode 100644 index 00000000..8a76aa82 --- /dev/null +++ b/src/xunit.analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublic.cs @@ -0,0 +1,60 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Xunit.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class ConstructorsOnFactAttributeSubclassShouldBePublic : XunitDiagnosticAnalyzer +{ + public ConstructorsOnFactAttributeSubclassShouldBePublic() : + base(Descriptors.X1043_ConstructorOnFactAttributeSubclassShouldBePublic) + { } + + public override void AnalyzeCompilation( + CompilationStartAnalysisContext context, + XunitContext xunitContext) + { + Guard.ArgumentNotNull(context); + Guard.ArgumentNotNull(xunitContext); + + if (xunitContext.Core.FactAttributeType is null) + return; + + context.RegisterSymbolAction(context => + { + if (context.Symbol is not IMethodSymbol method) + return; + + var attributes = method.GetAttributes(); + foreach (var attribute in attributes) + { + var attributeClass = attribute.AttributeClass; + if (attributeClass is null) + continue; + + if (!xunitContext.Core.FactAttributeType.IsAssignableFrom(attributeClass)) + continue; + + var constructor = attribute.AttributeConstructor; + if (constructor is null) + continue; + + if (constructor.DeclaredAccessibility == Accessibility.ProtectedOrInternal + || constructor.DeclaredAccessibility == Accessibility.Internal) + { + if (attribute.ApplicationSyntaxReference?.GetSyntax(context.CancellationToken) is not AttributeSyntax attributeSyntax) + return; + + context.ReportDiagnostic( + Diagnostic.Create( + Descriptors.X1043_ConstructorOnFactAttributeSubclassShouldBePublic, + attributeSyntax.GetLocation(), + constructor.ToDisplayString() + ) + ); + } + } + }, SymbolKind.Method); + } +}