From 4f5ba0b7ef2d6ca756ca1c7ff92a95c67db287e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Hellander?= Date: Fri, 16 Feb 2024 10:41:04 +0100 Subject: [PATCH 1/2] Improve tests for SA1612 --- .../DocumentationRules/SA1612UnitTests.cs | 205 ++++++++++++++---- 1 file changed, 165 insertions(+), 40 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs index 57c7f1fe4..2458f6c71 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs @@ -6,6 +6,7 @@ namespace StyleCop.Analyzers.Test.DocumentationRules { using System.Collections.Generic; + using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Testing; @@ -19,16 +20,53 @@ namespace StyleCop.Analyzers.Test.DocumentationRules /// public class SA1612UnitTests { + public static IEnumerable DeclarationsWithMemberColumn + { + get + { + yield return new object[] { " public ClassName Method(string foo, string bar, string @new) { return null; }", 22 }; + yield return new object[] { " public delegate ClassName Method(string foo, string bar, string @new);", 31 }; + yield return new object[] { " public ClassName this[string foo, string bar, string @new] { get { return null; } set { } }", 22 }; + } + } + public static IEnumerable Declarations { get { - yield return new[] { " public ClassName Method(string foo, string bar, string @new) { return null; }" }; - yield return new[] { " public delegate ClassName Method(string foo, string bar, string @new);" }; - yield return new[] { " public ClassName this[string foo, string bar, string @new] { get { return null; } set { } }" }; + return DeclarationsWithMemberColumn.Select(x => new[] { x[0] }); } } + [Fact] + public async Task VerifyClassIsNotReportedAsync() + { + var testCode = @" + /// + /// Foo + /// +public class ClassName +{ +}"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task VerifyMethodWithNoParametersIsNotReportedAsync() + { + var testCode = @" +public class ClassName +{ + /// + /// Foo + /// + public void Method() { } +}"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + [Fact] public async Task TestMemberWithoutDocumentationAsync() { @@ -198,7 +236,7 @@ public class ClassName [Theory] [MemberData(nameof(Declarations))] - public async Task TestMembersWithAllDocumentationWrongOrderAsync(string p) + public async Task TestMembersWithAllDocumentationWrongOrderAsync(string declaration) { var testCode = @" /// @@ -212,7 +250,7 @@ public class ClassName /// Param 2 /// Param 3 /// Param 1 - $$ +$$ }"; var diagnostic = Diagnostic() @@ -225,12 +263,29 @@ public class ClassName diagnostic.WithLocation(12, 22).WithArguments("foo", 1), }; - await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", p), expected, CancellationToken.None).ConfigureAwait(false); + await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false); + + var testSettings = @" +{ + ""settings"": { + ""documentationRules"": { + ""documentExposedElements"": false + } + } +} +"; + + expected = new[] + { + diagnostic.WithLocation(12, 22).WithArguments("foo", 1), + }; + + await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), testSettings, expected, CancellationToken.None).ConfigureAwait(false); } [Theory] [MemberData(nameof(Declarations))] - public async Task TestMembersWithTooManyDocumentationAsync(string p) + public async Task TestMembersWithTooManyDocumentationAsync(string declaration) { var testCode = @" /// @@ -245,7 +300,7 @@ public class ClassName /// Param 2 /// Param 3 /// Param 4 - $$ +$$ }"; var diagnostic = Diagnostic() @@ -253,7 +308,24 @@ public class ClassName var expected = diagnostic.WithLocation(13, 22).WithArguments("bar", 2); - await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", p), expected, CancellationToken.None).ConfigureAwait(false); + await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestDelegateWithoutIdentifierWithTooManyDocumentationIsNotReportedAsync() + { + var testCode = @" +public class ClassName +{ + /// + /// Foo + /// + /// Param 1 + /// Param 2 + public delegate void (int foo); +}"; + + await VerifyCSharpDiagnosticAsync(testCode, testSettings: null, DiagnosticResult.EmptyDiagnosticResults, ignoreCompilerDiagnostics: true, CancellationToken.None).ConfigureAwait(false); } [Theory] @@ -267,11 +339,23 @@ public async Task VerifyInheritedDocumentationReportsNoDiagnosticsAsync(string d public class ClassName { /// - $$ +$$ }"; await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); } + [Fact] + public async Task VerifyIncludedClassIsNotReportedAsync() + { + var testCode = @" +/// +public class ClassName +{ +}"; + + await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + [Fact] public async Task VerifyIncludedMemberWithoutParamsIsNotReportedAsync() { @@ -303,8 +387,9 @@ public class ClassName await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); } - [Fact] - public async Task VerifyIncludedMemberWithValidParamsIsNotReportedAsync() + [Theory] + [MemberData(nameof(Declarations))] + public async Task VerifyIncludedMemberWithValidParamsIsNotReportedAsync(string declaration) { var testCode = @" /// @@ -313,13 +398,14 @@ public async Task VerifyIncludedMemberWithValidParamsIsNotReportedAsync() public class ClassName { /// - public ClassName Method(string foo, string bar, string @new) { return null; } +$$ }"; - await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); } - [Fact] - public async Task VerifyIncludedMemberWithInvalidParamsIsReportedAsync() + [Theory] + [MemberData(nameof(DeclarationsWithMemberColumn))] + public async Task VerifyIncludedMemberWithInvalidParamsIsReportedAsync(string declaration, int memberColumn) { var testCode = @" /// @@ -328,17 +414,17 @@ public async Task VerifyIncludedMemberWithInvalidParamsIsReportedAsync() public class ClassName { /// - public ClassName Method(string foo, string bar, string @new) { return null; } +$$ }"; var expected = new[] { - Diagnostic().WithLocation(8, 22).WithArguments("boo"), - Diagnostic().WithLocation(8, 22).WithArguments("far"), - Diagnostic().WithLocation(8, 22).WithArguments("foe"), + Diagnostic().WithLocation(8, memberColumn).WithArguments("boo"), + Diagnostic().WithLocation(8, memberColumn).WithArguments("far"), + Diagnostic().WithLocation(8, memberColumn).WithArguments("foe"), }; - await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); + await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false); } [Fact] @@ -351,13 +437,14 @@ public async Task VerifyIncludedMemberWithInvalidParamsThatShouldBeHandledBySA16 public class ClassName { /// - public ClassName Method(string foo, string bar, string @new) { return null; } + public ClassName Method(string foo, string bar, string @new) { return null; } }"; await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); } - [Fact] - public async Task VerifyIncludedMemberWithAllDocumentationWrongOrderIsReportedAsync() + [Theory] + [MemberData(nameof(DeclarationsWithMemberColumn))] + public async Task VerifyIncludedMemberWithAllDocumentationWrongOrderIsReportedAsync(string declaration, int memberColumn) { var testCode = @" /// @@ -365,8 +452,8 @@ public async Task VerifyIncludedMemberWithAllDocumentationWrongOrderIsReportedAs /// public class ClassName { - /// - public ClassName Method(string bar, string @new, string foo) { return null; } + /// +$$ }"; var diagnostic = Diagnostic() @@ -374,12 +461,12 @@ public class ClassName var expected = new[] { - diagnostic.WithLocation(8, 22).WithArguments("foo", 3), - diagnostic.WithLocation(8, 22).WithArguments("bar", 1), - diagnostic.WithLocation(8, 22).WithArguments("new", 2), + diagnostic.WithLocation(8, memberColumn).WithArguments("new", 3), + diagnostic.WithLocation(8, memberColumn).WithArguments("foo", 1), + diagnostic.WithLocation(8, memberColumn).WithArguments("bar", 2), }; - await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); + await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false); // This is even reported if the documentation is not required, except that no warning is reported for the // first param element (which is actually the last parameter) since it would otherwise be allowed to skip @@ -396,15 +483,16 @@ public class ClassName expected = new[] { - diagnostic.WithLocation(8, 22).WithArguments("bar", 1), - diagnostic.WithLocation(8, 22).WithArguments("new", 2), + diagnostic.WithLocation(8, memberColumn).WithArguments("foo", 1), + diagnostic.WithLocation(8, memberColumn).WithArguments("bar", 2), }; - await VerifyCSharpDiagnosticAsync(testCode, testSettings, expected, CancellationToken.None).ConfigureAwait(false); + await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), testSettings, expected, CancellationToken.None).ConfigureAwait(false); } - [Fact] - public async Task VerifyIncludedMemberWithTooManyDocumentationIsReportedAsync() + [Theory] + [MemberData(nameof(DeclarationsWithMemberColumn))] + public async Task VerifyIncludedMemberWithTooManyDocumentationIsReportedAsync(string declaration, int memberColumn) { var testCode = @" /// @@ -413,15 +501,31 @@ public async Task VerifyIncludedMemberWithTooManyDocumentationIsReportedAsync() public class ClassName { /// - public ClassName Method(string foo, string bar, string @new) { return null; } +$$ }"; var diagnostic = Diagnostic() .WithMessageFormat("The parameter documentation for '{0}' should be at position {1}"); - var expected = diagnostic.WithLocation(8, 22).WithArguments("bar", 2); + var expected = diagnostic.WithLocation(8, memberColumn).WithArguments("bar", 2); + + await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task VerifyIncludedDelegateWithoutIdentifierWithTooManyDocumentationIsNotReportedAsync() + { + var testCode = @" +/// +/// Foo +/// +public class ClassName +{ + /// + public delegate void (int foo); +}"; - await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); + await VerifyCSharpDiagnosticAsync(testCode, testSettings: null, DiagnosticResult.EmptyDiagnosticResults, ignoreCompilerDiagnostics: true, CancellationToken.None).ConfigureAwait(false); } [Fact] @@ -440,12 +544,15 @@ public class ClassName } private static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult expected, CancellationToken cancellationToken) - => VerifyCSharpDiagnosticAsync(source, testSettings: null, new[] { expected }, cancellationToken); + => VerifyCSharpDiagnosticAsync(source, testSettings: null, new[] { expected }, false, cancellationToken); private static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult[] expected, CancellationToken cancellationToken) - => VerifyCSharpDiagnosticAsync(source, testSettings: null, expected, cancellationToken); + => VerifyCSharpDiagnosticAsync(source, testSettings: null, expected, false, cancellationToken); private static Task VerifyCSharpDiagnosticAsync(string source, string testSettings, DiagnosticResult[] expected, CancellationToken cancellationToken) + => VerifyCSharpDiagnosticAsync(source, testSettings, expected, false, cancellationToken); + + private static Task VerifyCSharpDiagnosticAsync(string source, string testSettings, DiagnosticResult[] expected, bool ignoreCompilerDiagnostics, CancellationToken cancellationToken) { string contentWithoutParamDocumentation = @" @@ -467,6 +574,18 @@ private static Task VerifyCSharpDiagnosticAsync(string source, string testSettin Param 3 +"; + string contentWithWrongOrderParamDocumentation = @" + + + + Foo + + Param 3 + Param 1 + Param 2 + + "; string contentWithInvalidParamDocumentation = @" @@ -523,6 +642,7 @@ private static Task VerifyCSharpDiagnosticAsync(string source, string testSettin { { "MissingParamDocumentation.xml", contentWithoutParamDocumentation }, { "WithParamDocumentation.xml", contentWithParamDocumentation }, + { "WithWrongOrderParamDocumentation.xml", contentWithWrongOrderParamDocumentation }, { "WithInvalidParamDocumentation.xml", contentWithInvalidParamDocumentation }, { "WithSA1613ParamDocumentation.xml", contentWithSA1613ParamDocumentation }, { "WithTooManyParamDocumentation.xml", contentWithTooManyParamDocumentation }, @@ -530,6 +650,11 @@ private static Task VerifyCSharpDiagnosticAsync(string source, string testSettin }, }; + if (ignoreCompilerDiagnostics) + { + test.CompilerDiagnostics = CompilerDiagnostics.None; + } + test.ExpectedDiagnostics.AddRange(expected); return test.RunAsync(cancellationToken); } From ed0ef91f8f74c1f31441b81947de395ab7eb83a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Hellander?= Date: Wed, 13 Mar 2024 19:38:58 +0100 Subject: [PATCH 2/2] Fix review comments --- .../DocumentationRules/SA1612UnitTests.cs | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs index 2458f6c71..e48f37635 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs @@ -20,21 +20,13 @@ namespace StyleCop.Analyzers.Test.DocumentationRules /// public class SA1612UnitTests { - public static IEnumerable DeclarationsWithMemberColumn - { - get - { - yield return new object[] { " public ClassName Method(string foo, string bar, string @new) { return null; }", 22 }; - yield return new object[] { " public delegate ClassName Method(string foo, string bar, string @new);", 31 }; - yield return new object[] { " public ClassName this[string foo, string bar, string @new] { get { return null; } set { } }", 22 }; - } - } - public static IEnumerable Declarations { get { - return DeclarationsWithMemberColumn.Select(x => new[] { x[0] }); + yield return new object[] { " public ClassName {|#0:Method|}(string foo, string bar, string @new) { return null; }" }; + yield return new object[] { " public delegate ClassName {|#0:Method|}(string foo, string bar, string @new);" }; + yield return new object[] { " public ClassName {|#0:this|}[string foo, string bar, string @new] { get { return null; } set { } }" }; } } @@ -404,8 +396,8 @@ public class ClassName } [Theory] - [MemberData(nameof(DeclarationsWithMemberColumn))] - public async Task VerifyIncludedMemberWithInvalidParamsIsReportedAsync(string declaration, int memberColumn) + [MemberData(nameof(Declarations))] + public async Task VerifyIncludedMemberWithInvalidParamsIsReportedAsync(string declaration) { var testCode = @" /// @@ -419,9 +411,9 @@ public class ClassName var expected = new[] { - Diagnostic().WithLocation(8, memberColumn).WithArguments("boo"), - Diagnostic().WithLocation(8, memberColumn).WithArguments("far"), - Diagnostic().WithLocation(8, memberColumn).WithArguments("foe"), + Diagnostic().WithLocation(0).WithArguments("boo"), + Diagnostic().WithLocation(0).WithArguments("far"), + Diagnostic().WithLocation(0).WithArguments("foe"), }; await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false); @@ -443,8 +435,8 @@ public class ClassName } [Theory] - [MemberData(nameof(DeclarationsWithMemberColumn))] - public async Task VerifyIncludedMemberWithAllDocumentationWrongOrderIsReportedAsync(string declaration, int memberColumn) + [MemberData(nameof(Declarations))] + public async Task VerifyIncludedMemberWithAllDocumentationWrongOrderIsReportedAsync(string declaration) { var testCode = @" /// @@ -461,9 +453,9 @@ public class ClassName var expected = new[] { - diagnostic.WithLocation(8, memberColumn).WithArguments("new", 3), - diagnostic.WithLocation(8, memberColumn).WithArguments("foo", 1), - diagnostic.WithLocation(8, memberColumn).WithArguments("bar", 2), + diagnostic.WithLocation(0).WithArguments("new", 3), + diagnostic.WithLocation(0).WithArguments("foo", 1), + diagnostic.WithLocation(0).WithArguments("bar", 2), }; await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false); @@ -483,16 +475,16 @@ public class ClassName expected = new[] { - diagnostic.WithLocation(8, memberColumn).WithArguments("foo", 1), - diagnostic.WithLocation(8, memberColumn).WithArguments("bar", 2), + diagnostic.WithLocation(0).WithArguments("foo", 1), + diagnostic.WithLocation(0).WithArguments("bar", 2), }; await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), testSettings, expected, CancellationToken.None).ConfigureAwait(false); } [Theory] - [MemberData(nameof(DeclarationsWithMemberColumn))] - public async Task VerifyIncludedMemberWithTooManyDocumentationIsReportedAsync(string declaration, int memberColumn) + [MemberData(nameof(Declarations))] + public async Task VerifyIncludedMemberWithTooManyDocumentationIsReportedAsync(string declaration) { var testCode = @" /// @@ -507,7 +499,7 @@ public class ClassName var diagnostic = Diagnostic() .WithMessageFormat("The parameter documentation for '{0}' should be at position {1}"); - var expected = diagnostic.WithLocation(8, memberColumn).WithArguments("bar", 2); + var expected = diagnostic.WithLocation(0).WithArguments("bar", 2); await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false); }