Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow inheritdoc for class constructors with base types #3719

Merged
merged 21 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

#nullable disable
#nullable enable
MartyIX marked this conversation as resolved.
Show resolved Hide resolved

namespace StyleCop.Analyzers.Test.DocumentationRules
{
Expand All @@ -18,6 +18,40 @@ namespace StyleCop.Analyzers.Test.DocumentationRules
/// </summary>
public class SA1648UnitTests
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
{
[Fact]
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
public async Task TestConstructorInheritsFromParentAsync()
{
var testCode = @"class Base
{
/// <summary>Base constructor.</summary>
public Base() { }
}

class Test : Base
{
/// <inheritdoc />
public Test() { }
}";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);

testCode = @"class Base
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>Base constructor.</summary>
public Base(string s, int a) { }
}

class Test : Base
{
/// <inheritdoc />
public Test(string s, int b)
: base(s, b) { }
}
";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
public async Task TestClassOverridesClassAsync()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,56 @@ private static void HandleMemberDeclaration(SyntaxNodeAnalysisContext context)
}
}

if (memberSyntax is ConstructorDeclarationSyntax constructorDeclarationSyntax)
{
ISymbol symbol = context.SemanticModel.GetDeclaredSymbol(constructorDeclarationSyntax, context.CancellationToken);
MartyIX marked this conversation as resolved.
Show resolved Hide resolved

if (symbol is IMethodSymbol constructorMethodSymbol && constructorMethodSymbol.ContainingType is INamedTypeSymbol enclosingNamedTypeSymbol)
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
{
INamedTypeSymbol baseType = enclosingNamedTypeSymbol.BaseType;

if (baseType.SpecialType != SpecialType.System_Object)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want it to trigger for this case? Maybe not the best documentation, but the use of inheritdoc seems to be correct:

image

What do you think, @sharwell?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added support for this case to follow the VS approach.

{
bool foundMatchingConstructorsToInheritFrom = false;

foreach (IMethodSymbol baseConstructorMethod in baseType.Constructors)
{
// Constructors must have the same number of parameters.
if (constructorMethodSymbol.Parameters.Count() != baseConstructorMethod.Parameters.Count())
{
continue;
}

// Our constructor and the base constructor must have the same signature. But variable names can be different.
bool success = true;

for (int i = 0; i < constructorMethodSymbol.Parameters.Length; i++)
{
IParameterSymbol constructorParameter = constructorMethodSymbol.Parameters[i];
IParameterSymbol baseParameter = baseConstructorMethod.Parameters[i];

if (!constructorParameter.Type.Equals(baseParameter.Type))
{
success = false;
break;
}
}

if (success)
{
foundMatchingConstructorsToInheritFrom = true;
break;
}
}

if (foundMatchingConstructorsToInheritFrom)
{
return;
}
}
}
}

if (documentation.Content.GetFirstXmlElement(XmlCommentHelper.IncludeXmlTag) is XmlEmptyElementSyntax includeElement)
{
if (declaredSymbol == null)
Expand Down