Update xUnit1020 to also report on existing non-public property getters

xunit · Sep 18, 2021 · c5586d4 · c5586d4
1 parent 1ce5470
commit c5586d4
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 9 deletions.
diff --git a/src/xunit.analyzers.tests/Analyzers/MemberDataShouldReferenceValidMemberTests.cs b/src/xunit.analyzers.tests/Analyzers/MemberDataShouldReferenceValidMemberTests.cs
@@ -352,6 +352,28 @@ public void TestMethod() { }
 		await Verify.VerifyAnalyzerAsync(source, expected);
 	}
 
+	[Theory]
+	[InlineData("internal")]
+	[InlineData("protected")]
+	[InlineData("private")]
+	public async void FindsError_ForMemberPropertyWithNonPublicGetter(string visibility)
+	{
+		var source = $@"
+public class TestClass {{
+    public static System.Collections.Generic.IEnumerable<object[]> Data {{ {visibility} get {{ return null; }} set {{ }} }}
+
+    [Xunit.MemberData(nameof(Data))]
+    public void TestMethod() {{ }}
+}}";
+		var expected =
+			Verify
+				.Diagnostic("xUnit1020")
+				.WithSpan(5, 6, 5, 36)
+				.WithSeverity(DiagnosticSeverity.Error);
+
+		await Verify.VerifyAnalyzerAsync(source, expected);
+	}
+
 	[Theory]
 	[InlineData("'a', 123")]
 	[InlineData("new object[] { 'a', 123 }")]

diff --git a/src/xunit.analyzers/MemberDataShouldReferenceValidMember.cs b/src/xunit.analyzers/MemberDataShouldReferenceValidMember.cs
@@ -153,14 +153,16 @@ compilation is CSharpCompilation cSharpCompilation
 								)
 							);
 						}
-						if (memberSymbol.Kind == SymbolKind.Property && ((IPropertySymbol)memberSymbol).GetMethod == null)
+						if (memberSymbol.Kind == SymbolKind.Property && memberSymbol.DeclaredAccessibility == Accessibility.Public)
 						{
-							context.ReportDiagnostic(
-								Diagnostic.Create(
-									Descriptors.X1020_MemberDataPropertyMustHaveGetter,
-									attribute.GetLocation()
-								)
-							);
+							var getMethod = ((IPropertySymbol)memberSymbol).GetMethod;
+							if (getMethod == null || getMethod.DeclaredAccessibility != Accessibility.Public)
+								context.ReportDiagnostic(
+									Diagnostic.Create(
+										Descriptors.X1020_MemberDataPropertyMustHaveGetter,
+										attribute.GetLocation()
+									)
+								);
 						}
 						var extraArguments = attribute.ArgumentList.Arguments.Skip(1).TakeWhile(a => a.NameEquals == null).ToList();
 						if (memberSymbol.Kind == SymbolKind.Property || memberSymbol.Kind == SymbolKind.Field)

diff --git a/src/xunit.analyzers/Utilities/Descriptors.cs b/src/xunit.analyzers/Utilities/Descriptors.cs
@@ -225,10 +225,10 @@ static DiagnosticDescriptor Rule(
 		public static DiagnosticDescriptor X1020_MemberDataPropertyMustHaveGetter { get; } =
 			Rule(
 				"xUnit1020",
-				"MemberData must reference a property with a getter",
+				"MemberData must reference a property with a public getter",
 				Usage,
 				Error,
-				"MemberData must reference a property with a getter"
+				"MemberData must reference a property with a public getter. Add a public getter to the data member, or change the visibility of the existing getter to public."
 			);
 
 		public static DiagnosticDescriptor X1021_MemberDataNonMethodShouldNotHaveParameters { get; } =