diff --git a/src/xunit.analyzers.tests/Analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublicTests.cs b/src/xunit.analyzers.tests/Analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublicTests.cs index 1717b1ed..4829d0b4 100644 --- a/src/xunit.analyzers.tests/Analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublicTests.cs +++ b/src/xunit.analyzers.tests/Analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublicTests.cs @@ -1,217 +1,165 @@ using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Testing; using Xunit; using Verify = CSharpVerifier; public class ConstructorsOnFactAttributeSubclassShouldBePublicTests { [Fact] - public async void DoesNotFindError_ForPublicConstructor_InFactAttributeSubclass() + public async void DefaultConstructor_DoesNotTrigger() { var source = @" using System; using Xunit; [AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] -internal sealed class CustomFactAttribute : FactAttribute -{ - public CustomFactAttribute() - { - this.Skip = ""xxx""; - } -} +internal sealed class CustomFactAttribute : FactAttribute { } -public class Tests -{ +public class Tests { [CustomFact] - public void TestCustomFact() - { - } + public void TestCustomFact() { } [Fact] - public void TestFact() - { - } + public void TestFact() { } }"; await Verify.VerifyAnalyzer(source); } [Fact] - public async void DoesNotFindError_ForPublicConstructorWithArguments_InFactAttributeSubclass() + public async void ParameterlessPublicConstructor_DoesNotTrigger() { var source = @" using System; using Xunit; [AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] -internal sealed class CustomFactAttribute : FactAttribute -{ - public CustomFactAttribute(string blah) - { - this.Skip = blah; +internal sealed class CustomFactAttribute : FactAttribute { + public CustomFactAttribute() { + this.Skip = ""xxx""; } } -public class Tests -{ - [CustomFact(""blah"")] - public void TestCustomFact() - { - } +public class Tests { + [CustomFact] + public void TestCustomFact() { } [Fact] - public void TestFact() - { - } + public void TestFact() { } }"; await Verify.VerifyAnalyzer(source); } [Fact] - public async void DoesNotFindError_ForPublicConstructorWithOtherConstructors_InFactAttributeSubclass() + public async void PublicConstructorWithParameters_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 blah) - { - this.Skip = blah; +internal sealed class CustomFactAttribute : FactAttribute { + public CustomFactAttribute(string skip) { + this.Skip = skip; } } -public class Tests -{ - [CustomFact] - public void TestCustomFact() - { - } +public class Tests { + [CustomFact(""blah"")] + public void TestCustomFact() { } [Fact] - public void TestFact() - { - } + public void TestFact() { } }"; await Verify.VerifyAnalyzer(source); } [Fact] - public async void DoesNotFindError_ForDefaultConstructor_InFactAttributeSubclass() + public async void PublicConstructorWithOtherConstructors_DoesNotTrigger() { var source = @" using System; using Xunit; [AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] -internal sealed class CustomFactAttribute : FactAttribute -{ +internal sealed class CustomFactAttribute : FactAttribute { + public CustomFactAttribute() { + this.Skip = ""xxx""; + } + + internal CustomFactAttribute(string skip) { + this.Skip = skip; + } } -public class Tests -{ +public class Tests { [CustomFact] - public void TestCustomFact() - { - } + public void TestCustomFact() { } [Fact] - public void TestFact() - { - } + public void TestFact() { } }"; await Verify.VerifyAnalyzer(source); } [Fact] - public async void FindsError_ForInternalConstructor_InFactAttributeSubclass() + public async void InternalConstructor_Triggers() { var source = @" using System; using Xunit; [AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] -internal sealed class CustomFactAttribute : FactAttribute -{ - internal CustomFactAttribute() - { - this.Skip = ""xxx""; - } +internal sealed class CustomFactAttribute : FactAttribute { + internal CustomFactAttribute(string skip, params int[] values) { } } -public class Tests -{ - [CustomFact] - public void TestCustomFact() - { - } +public class Tests { + [CustomFact(""Skip"", 42)] + public void TestCustomFact() { } [Fact] - public void TestFact() - { - } + public void TestFact() { } }"; - - DiagnosticResult[] expected = - { + var expected = Verify - .Diagnostic("xUnit1043") + .Diagnostic() .WithSeverity(DiagnosticSeverity.Error) - .WithSpan(16, 6, 16, 16) - .WithArguments("CustomFactAttribute") - }; + .WithSpan(11, 6, 11, 28) + .WithArguments("CustomFactAttribute.CustomFactAttribute(string, params int[])"); await Verify.VerifyAnalyzer(source, expected); } [Fact] - public async void FindsError_ForProtectedInternalConstructor_InFactAttributeSubclass() + public async void ProtectedInternalConstructor_Triggers() { var source = @" using System; using Xunit; [AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] -internal sealed class CustomFactAttribute : FactAttribute -{ - protected internal CustomFactAttribute() - { +internal sealed class CustomFactAttribute : FactAttribute { + protected internal CustomFactAttribute() { this.Skip = ""xxx""; } } -public class Tests -{ +public class Tests { [CustomFact] - public void TestCustomFact() - { - } + public void TestCustomFact() { } [Fact] - public void TestFact() - { - } + public void TestFact() { } }"; - - DiagnosticResult[] expected = - { + var expected = Verify - .Diagnostic("xUnit1043") + .Diagnostic() .WithSeverity(DiagnosticSeverity.Error) - .WithSpan(16, 6, 16, 16) - .WithArguments("CustomFactAttribute") - }; + .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 a252960f..8142f9a5 100644 --- a/src/xunit.analyzers/Utility/Descriptors.cs +++ b/src/xunit.analyzers/Utility/Descriptors.cs @@ -424,10 +424,10 @@ static DiagnosticDescriptor Rule( public static DiagnosticDescriptor X1043_ConstructorOnFactAttributeSubclassShouldBePublic { get; } = Rule( "xUnit1043", - "Constructors on classes derived from FactAttribute must be public and not internal when used on test methods", + "Constructors on classes derived from FactAttribute must be public when used on test methods", Usage, Error, - "Constructors on class '{0}', derived from FactAttribute, must be marked as public and not internal to be found by test runners." + "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 index d7d8460f..8a76aa82 100644 --- a/src/xunit.analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublic.cs +++ b/src/xunit.analyzers/X1000/ConstructorsOnFactAttributeSubclassShouldBePublic.cs @@ -1,4 +1,3 @@ -using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; @@ -51,7 +50,7 @@ public override void AnalyzeCompilation( Diagnostic.Create( Descriptors.X1043_ConstructorOnFactAttributeSubclassShouldBePublic, attributeSyntax.GetLocation(), - attributeClass.Name + constructor.ToDisplayString() ) ); }