Update SA1612 to also check primary constructor parameters

bjornhellander · bjornhellander · commit 24dd9011d6ec · 2024-04-28T09:52:06.000+02:00
#3786
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs
@@ -11,6 +11,7 @@ namespace StyleCop.Analyzers.Test.DocumentationRules
     using System.Threading.Tasks;
     using Microsoft.CodeAnalysis.Testing;
     using StyleCop.Analyzers.DocumentationRules;
+    using StyleCop.Analyzers.Lightup;
     using StyleCop.Analyzers.Test.Verifiers;
     using Xunit;
     using static StyleCop.Analyzers.Test.Verifiers.CustomDiagnosticVerifier<StyleCop.Analyzers.DocumentationRules.SA1612ElementParameterDocumentationMustMatchElementParameters>;
@@ -24,9 +25,26 @@ public static IEnumerable<object[]> Declarations
         {
             get
             {
-                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 { } }" };
+                yield return new[] { "    public ClassName {|#0:Method|}(string foo, string bar, string @new) { return null; }" };
+                yield return new[] { "    public delegate ClassName {|#0:Method|}(string foo, string bar, string @new);" };
+                yield return new[] { "    public ClassName {|#0:this|}[string foo, string bar, string @new] { get { return null; } set { } }" };
+
+                if (LightupHelpers.SupportsCSharp9)
+                {
+                    yield return new[] { "    public record {|#0:TestType|}(string foo, string bar, string @new) {}" };
+                }
+
+                if (LightupHelpers.SupportsCSharp10)
+                {
+                    yield return new[] { "    public record struct {|#0:TestType|}(string foo, string bar, string @new) {}" };
+                    yield return new[] { "    public record class {|#0:TestType|}(string foo, string bar, string @new) {}" };
+                }
+
+                if (LightupHelpers.SupportsCSharp12)
+                {
+                    yield return new[] { "    public struct {|#0:TestType|}(string foo, string bar, string @new) {}" };
+                    yield return new[] { "    public class {|#0:TestType|}(string foo, string bar, string @new) {}" };
+                }
             }
         }
 
@@ -165,11 +183,12 @@ public class ClassName
             var diagnostic = Diagnostic()
                 .WithMessageFormat("The parameter documentation for '{0}' should be at position {1}");
 
-            var expected = new[]
+            var normallyExpected = new[]
             {
                 diagnostic.WithLocation(10, 21).WithArguments("new", 3),
                 diagnostic.WithLocation(11, 21).WithArguments("foo", 1),
             };
+            var expected = GetExpectedDiagnostics(normallyExpected, declaration);
 
             await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false);
         }
@@ -193,12 +212,13 @@ public class ClassName
 $$
 }";
 
-            var expected = new[]
+            var normallyExpected = new[]
             {
                 Diagnostic().WithLocation(10, 21).WithArguments("boo"),
                 Diagnostic().WithLocation(11, 21).WithArguments("far"),
                 Diagnostic().WithLocation(12, 21).WithArguments("foe"),
             };
+            var expected = GetExpectedDiagnostics(normallyExpected, declaration);
 
             await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false);
         }
@@ -248,12 +268,13 @@ public class ClassName
             var diagnostic = Diagnostic()
                 .WithMessageFormat("The parameter documentation for '{0}' should be at position {1}");
 
-            var expected = new[]
+            var normallyExpected = new[]
             {
                 diagnostic.WithLocation(10, 22).WithArguments("bar", 2),
                 diagnostic.WithLocation(11, 22).WithArguments("new", 3),
                 diagnostic.WithLocation(12, 22).WithArguments("foo", 1),
             };
+            var expected = GetExpectedDiagnostics(normallyExpected, declaration);
 
             await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false);
 
@@ -267,10 +288,11 @@ public class ClassName
 }
 ";
 
-            expected = new[]
+            normallyExpected = new[]
             {
                 diagnostic.WithLocation(12, 22).WithArguments("foo", 1),
             };
+            expected = GetExpectedDiagnostics(normallyExpected, declaration);
 
             await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), testSettings, expected, CancellationToken.None).ConfigureAwait(false);
         }
@@ -298,7 +320,8 @@ public class ClassName
             var diagnostic = Diagnostic()
                 .WithMessageFormat("The parameter documentation for '{0}' should be at position {1}");
 
-            var expected = diagnostic.WithLocation(13, 22).WithArguments("bar", 2);
+            var normallyExpected = diagnostic.WithLocation(13, 22).WithArguments("bar", 2);
+            var expected = GetExpectedDiagnostics(normallyExpected, declaration);
 
             await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false);
         }
@@ -409,12 +432,13 @@ public class ClassName
 $$
 }";
 
-            var expected = new[]
+            var normallyExpected = new[]
             {
                 Diagnostic().WithLocation(0).WithArguments("boo"),
                 Diagnostic().WithLocation(0).WithArguments("far"),
                 Diagnostic().WithLocation(0).WithArguments("foe"),
             };
+            var expected = GetExpectedDiagnostics(normallyExpected, declaration);
 
             await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false);
         }
@@ -451,12 +475,13 @@ public class ClassName
             var diagnostic = Diagnostic()
                 .WithMessageFormat("The parameter documentation for '{0}' should be at position {1}");
 
-            var expected = new[]
+            var normallyExpected = new[]
             {
                 diagnostic.WithLocation(0).WithArguments("new", 3),
                 diagnostic.WithLocation(0).WithArguments("foo", 1),
                 diagnostic.WithLocation(0).WithArguments("bar", 2),
             };
+            var expected = GetExpectedDiagnostics(normallyExpected, declaration);
 
             await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false);
 
@@ -473,11 +498,12 @@ public class ClassName
 }
 ";
 
-            expected = new[]
+            normallyExpected = new[]
             {
                 diagnostic.WithLocation(0).WithArguments("foo", 1),
                 diagnostic.WithLocation(0).WithArguments("bar", 2),
             };
+            expected = GetExpectedDiagnostics(normallyExpected, declaration);
 
             await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), testSettings, expected, CancellationToken.None).ConfigureAwait(false);
         }
@@ -499,7 +525,8 @@ public class ClassName
             var diagnostic = Diagnostic()
                 .WithMessageFormat("The parameter documentation for '{0}' should be at position {1}");
 
-            var expected = diagnostic.WithLocation(0).WithArguments("bar", 2);
+            var normallyExpected = diagnostic.WithLocation(0).WithArguments("bar", 2);
+            var expected = GetExpectedDiagnostics(normallyExpected, declaration);
 
             await VerifyCSharpDiagnosticAsync(testCode.Replace("$$", declaration), expected, CancellationToken.None).ConfigureAwait(false);
         }
@@ -535,8 +562,27 @@ public class ClassName
             await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
-        private static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult expected, CancellationToken cancellationToken)
-            => VerifyCSharpDiagnosticAsync(source, testSettings: null, new[] { expected }, ignoreCompilerDiagnostics: false, cancellationToken);
+        private static DiagnosticResult[] GetExpectedDiagnostics(DiagnosticResult normallyExpected, string declaration)
+        {
+            return GetExpectedDiagnostics(new[] { normallyExpected }, declaration);
+        }
+
+        // Syntax node actions for type declarations with a primary constructor were called twice
+        // before support for c# 11 was added.
+        private static DiagnosticResult[] GetExpectedDiagnostics(DiagnosticResult[] normallyExpected, string declaration)
+        {
+            var isPrimaryConstructor = declaration.Contains("record") || declaration.Contains("class") || declaration.Contains("struct");
+
+            if (isPrimaryConstructor && !LightupHelpers.SupportsCSharp11)
+            {
+                // Diagnostic issued twice because of https://github.com/dotnet/roslyn/issues/53136 and https://github.com/dotnet/roslyn/issues/70488
+                return normallyExpected.Concat(normallyExpected).ToArray();
+            }
+            else
+            {
+                return normallyExpected;
+            }
+        }
 
         private static Task VerifyCSharpDiagnosticAsync(string source, DiagnosticResult[] expected, CancellationToken cancellationToken)
             => VerifyCSharpDiagnosticAsync(source, testSettings: null, expected, ignoreCompilerDiagnostics: false, cancellationToken);
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1612ElementParameterDocumentationMustMatchElementParameters.cs b/StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1612ElementParameterDocumentationMustMatchElementParameters.cs
@@ -13,6 +13,7 @@ namespace StyleCop.Analyzers.DocumentationRules
     using Microsoft.CodeAnalysis.CSharp.Syntax;
     using Microsoft.CodeAnalysis.Diagnostics;
     using StyleCop.Analyzers.Helpers;
+    using StyleCop.Analyzers.Lightup;
     using StyleCop.Analyzers.Settings.ObjectModel;
 
     /// <summary>
@@ -45,7 +46,7 @@ internal class SA1612ElementParameterDocumentationMustMatchElementParameters : E
             new DiagnosticDescriptor(DiagnosticId, Title, MissingParamForDocumentationMessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
 
         private static readonly DiagnosticDescriptor OrderDescriptor =
-                   new DiagnosticDescriptor(DiagnosticId, Title, ParamWrongOrderMessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
+            new DiagnosticDescriptor(DiagnosticId, Title, ParamWrongOrderMessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
 
         public SA1612ElementParameterDocumentationMustMatchElementParameters()
             : base(matchElementName: XmlCommentHelper.ParamXmlTag, inheritDocSuppressesWarnings: true)
@@ -70,8 +71,8 @@ protected override void HandleXmlElement(SyntaxNodeAnalysisContext context, Styl
 
             var parameterList = GetParameters(node)?.ToImmutableArray();
 
-            bool hasNoParameters = !parameterList?.Any() ?? false;
-            if (hasNoParameters)
+            bool hasParameters = parameterList?.Any() ?? false;
+            if (!hasParameters)
             {
                 return;
             }
@@ -141,8 +142,8 @@ protected override void HandleCompleteDocumentation(SyntaxNodeAnalysisContext co
             var identifierLocation = identifier.Value.GetLocation();
             var parameterList = GetParameters(node)?.ToImmutableArray();
 
-            bool hasNoParameters = !parameterList?.Any() ?? false;
-            if (hasNoParameters)
+            bool hasParameters = parameterList?.Any() ?? false;
+            if (!hasParameters)
             {
                 return;
             }
@@ -203,14 +204,16 @@ private static IEnumerable<ParameterSyntax> GetParameters(SyntaxNode node)
         {
             return (node as BaseMethodDeclarationSyntax)?.ParameterList?.Parameters
                 ?? (node as IndexerDeclarationSyntax)?.ParameterList?.Parameters
-                ?? (node as DelegateDeclarationSyntax)?.ParameterList?.Parameters;
+                ?? (node as DelegateDeclarationSyntax)?.ParameterList?.Parameters
+                ?? (node as TypeDeclarationSyntax)?.ParameterList()?.Parameters;
         }
 
         private static SyntaxToken? GetIdentifier(SyntaxNode node)
         {
             return (node as MethodDeclarationSyntax)?.Identifier
                 ?? (node as IndexerDeclarationSyntax)?.ThisKeyword
-                ?? (node as DelegateDeclarationSyntax)?.Identifier;
+                ?? (node as DelegateDeclarationSyntax)?.Identifier
+                ?? (node as TypeDeclarationSyntax)?.Identifier;
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,7 @@ namespace StyleCop.Analyzers.DocumentationRules`
`13`	`13`	`using Microsoft.CodeAnalysis.CSharp.Syntax;`
`14`	`14`	`using Microsoft.CodeAnalysis.Diagnostics;`
`15`	`15`	`using StyleCop.Analyzers.Helpers;`
	`16`	`+ using StyleCop.Analyzers.Lightup;`
`16`	`17`	`using StyleCop.Analyzers.Settings.ObjectModel;`
`17`	`18`
`18`	`19`	`/// <summary>`
`@@ -45,7 +46,7 @@ internal class SA1612ElementParameterDocumentationMustMatchElementParameters : E`
`45`	`46`	`new DiagnosticDescriptor(DiagnosticId, Title, MissingParamForDocumentationMessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);`
`46`	`47`
`47`	`48`	`private static readonly DiagnosticDescriptor OrderDescriptor =`
`48`		`- new DiagnosticDescriptor(DiagnosticId, Title, ParamWrongOrderMessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);`
	`49`	`+ new DiagnosticDescriptor(DiagnosticId, Title, ParamWrongOrderMessageFormat, AnalyzerCategory.DocumentationRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);`
`49`	`50`
`50`	`51`	`public SA1612ElementParameterDocumentationMustMatchElementParameters()`
`51`	`52`	`: base(matchElementName: XmlCommentHelper.ParamXmlTag, inheritDocSuppressesWarnings: true)`
`@@ -70,8 +71,8 @@ protected override void HandleXmlElement(SyntaxNodeAnalysisContext context, Styl`
`70`	`71`
`71`	`72`	`var parameterList = GetParameters(node)?.ToImmutableArray();`
`72`	`73`
`73`		`- bool hasNoParameters = !parameterList?.Any() ?? false;`
`74`		`- if (hasNoParameters)`
	`74`	`+ bool hasParameters = parameterList?.Any() ?? false;`
	`75`	`+ if (!hasParameters)`
`75`	`76`	`{`
`76`	`77`	`return;`
`77`	`78`	`}`
`@@ -141,8 +142,8 @@ protected override void HandleCompleteDocumentation(SyntaxNodeAnalysisContext co`
`141`	`142`	`var identifierLocation = identifier.Value.GetLocation();`
`142`	`143`	`var parameterList = GetParameters(node)?.ToImmutableArray();`
`143`	`144`
`144`		`- bool hasNoParameters = !parameterList?.Any() ?? false;`
`145`		`- if (hasNoParameters)`
	`145`	`+ bool hasParameters = parameterList?.Any() ?? false;`
	`146`	`+ if (!hasParameters)`
`146`	`147`	`{`
`147`	`148`	`return;`
`148`	`149`	`}`
`@@ -203,14 +204,16 @@ private static IEnumerable<ParameterSyntax> GetParameters(SyntaxNode node)`
`203`	`204`	`{`
`204`	`205`	`return (node as BaseMethodDeclarationSyntax)?.ParameterList?.Parameters`
`205`	`206`	`?? (node as IndexerDeclarationSyntax)?.ParameterList?.Parameters`
`206`		`- ?? (node as DelegateDeclarationSyntax)?.ParameterList?.Parameters;`
	`207`	`+ ?? (node as DelegateDeclarationSyntax)?.ParameterList?.Parameters`
	`208`	`+ ?? (node as TypeDeclarationSyntax)?.ParameterList()?.Parameters;`
`207`	`209`	`}`
`208`	`210`
`209`	`211`	`private static SyntaxToken? GetIdentifier(SyntaxNode node)`
`210`	`212`	`{`
`211`	`213`	`return (node as MethodDeclarationSyntax)?.Identifier`
`212`	`214`	`?? (node as IndexerDeclarationSyntax)?.ThisKeyword`
`213`		`- ?? (node as DelegateDeclarationSyntax)?.Identifier;`
	`215`	`+ ?? (node as DelegateDeclarationSyntax)?.Identifier`
	`216`	`+ ?? (node as TypeDeclarationSyntax)?.Identifier;`
`214`	`217`	`}`
`215`	`218`	`}`
`216`	`219`	`}`