xunit/xunit#2120: Create analyzer for non-public constructors of type…

…s derived from FactAttribute (#175)

src/xunit.analyzers.tests/Analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublicTests.cs

@@ -0,0 +1,166 @@
+using Microsoft.CodeAnalysis;
+using Xunit;
+using Verify = CSharpVerifier<Xunit.Analyzers.ConstructorsOnFactAttributeSubclassShouldBePublic>;
+
+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);
+	}
+}

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
 

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);
+	}
+}