Skip to content

Commit

Permalink
Merge pull request #3833 from bjornhellander/feature/sa1612-primary-c…
Browse files Browse the repository at this point in the history 
…onstructor-3786

Update SA1612 to also check primary constructor parameters
  • Loading branch information
@sharwell
sharwell authored Apr 29, 2024
2 parents 4d099a2 + 24dd901 commit d855e3a
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 21 deletions.
74 changes: 60 additions & 14 deletions StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1612UnitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand All @@ -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) {}" };
}
}
}

Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);

Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);

Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
17 changes: 10 additions & 7 deletions ...yzers/DocumentationRules/SA1612ElementParameterDocumentationMustMatchElementParameters.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
}
}

0 comments on commit d855e3a

Please sign in to comment.