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 14 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,14 +1,13 @@
// 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

namespace StyleCop.Analyzers.Test.DocumentationRules
{
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using StyleCop.Analyzers.DocumentationRules;
using StyleCop.Analyzers.Test.Helpers;
using StyleCop.Analyzers.Test.Verifiers;
using Xunit;
using static StyleCop.Analyzers.Test.Verifiers.CustomDiagnosticVerifier<StyleCop.Analyzers.DocumentationRules.SA1648InheritDocMustBeUsedWithInheritingClass>;
Expand All @@ -18,6 +17,87 @@ namespace StyleCop.Analyzers.Test.DocumentationRules
/// </summary>
public class SA1648UnitTests
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
{
[Theory]
[MemberData(nameof(CommonMemberData.ReferenceTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
public async Task TestConstructorWithNoParametersInheritsFromParentAsync(string keyword)
{
var testCode = @"$KEYWORD$ Base
{
/// <summary>Base constructor.</summary>
public Base() { }
}

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

await VerifyCSharpDiagnosticAsync(testCode.Replace("$KEYWORD$", keyword), DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.ReferenceTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
public async Task TestConstructorWithParametersInheritsFromParentAsync(string keyword)
{
var testCode = @"$KEYWORD$ Base
{
/// <summary>Base constructor.</summary>
public Base(string s, int a) { }
}

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

await VerifyCSharpDiagnosticAsync(testCode.Replace("$KEYWORD$", keyword), DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.ReferenceTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
public async Task TestConstructorInheritsImplicitlyFromSystemObjectAsync(string keyword)
{
var testCode = @"$KEYWORD$ Test
{
/// <inheritdoc/>
public Test() { }
}";

await VerifyCSharpDiagnosticAsync(testCode.Replace("$KEYWORD$", keyword), DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(CommonMemberData.ReferenceTypeDeclarationKeywords), MemberType = typeof(CommonMemberData))]
public async Task TestConstructorInheritsExplicitlyFromSystemObjectAsync(string keyword)
{
var testCode = @"$KEYWORD$ Test : System.Object
{
/// <inheritdoc/>
public Test() { }
}";

await VerifyCSharpDiagnosticAsync(testCode.Replace("$KEYWORD$", keyword), DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
public async Task TestConstructorInheritsExplicitlyFromTypeInDifferentAssemblyAsync()
{
var testCode = @"class MyArgumentException : System.ArgumentException
{
/// <inheritdoc/>
public MyArgumentException() { }

/// <inheritdoc/>
public MyArgumentException(string message) : base(message) { }
}";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
}

[Fact]
public async Task TestClassOverridesClassAsync()
{
Expand Down Expand Up @@ -90,7 +170,6 @@ public async Task TestTypeWithEmptyBaseListAndCrefAttributeAsync(string declarat
}

[Theory]
[InlineData("Test() { }")]
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
[InlineData("void Foo() { }")]
[InlineData("string foo;")]
[InlineData("string Foo { get; set; }")]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// 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

namespace StyleCop.Analyzers.DocumentationRules
{
using System;
Expand Down Expand Up @@ -70,7 +68,7 @@ public override void Initialize(AnalysisContext context)

private static void HandleBaseTypeLikeDeclaration(SyntaxNodeAnalysisContext context)
{
BaseTypeDeclarationSyntax baseType = context.Node as BaseTypeDeclarationSyntax;
BaseTypeDeclarationSyntax? baseType = context.Node as BaseTypeDeclarationSyntax;

// baseType can be null here if we are looking at a delegate declaration
if (baseType != null && baseType.BaseList != null && baseType.BaseList.Types.Any())
Expand Down Expand Up @@ -149,10 +147,32 @@ private static void HandleMemberDeclaration(SyntaxNodeAnalysisContext context)
Location location;

ISymbol declaredSymbol = context.SemanticModel.GetDeclaredSymbol(memberSyntax, context.CancellationToken);

if (memberSyntax is ConstructorDeclarationSyntax constructorDeclarationSyntax && declaredSymbol is IMethodSymbol constructorMethodSymbol)
{
if (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)
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
{
// Exception: If the base type is System.Object, then we can use <inheritdoc/> if our constructor has zero parameters.
if (constructorMethodSymbol.Parameters.Length == 0)
{
return;
}
}
else if (HasMatchingSignature(baseType.Constructors, constructorMethodSymbol))
{
return;
}
}
}

if (declaredSymbol == null && memberSyntax.IsKind(SyntaxKind.EventFieldDeclaration))
{
var eventFieldDeclarationSyntax = (EventFieldDeclarationSyntax)memberSyntax;
VariableDeclaratorSyntax firstVariable = eventFieldDeclarationSyntax.Declaration?.Variables.FirstOrDefault();
VariableDeclaratorSyntax? firstVariable = eventFieldDeclarationSyntax.Declaration?.Variables.FirstOrDefault();
if (firstVariable != null)
{
declaredSymbol = context.SemanticModel.GetDeclaredSymbol(firstVariable, context.CancellationToken);
Expand Down Expand Up @@ -206,15 +226,57 @@ private static void HandleMemberDeclaration(SyntaxNodeAnalysisContext context)
}
}

/// <summary>
/// Method compares a <paramref name="constructorMethodSymbol">constructor method</paramref> signature against its
/// <paramref name="baseConstructorSymbols">base type constructors</paramref> to find if there is a method signature match.
/// </summary>
/// <returns><see langword="true"/> if any base type constructor's signature matches the signature of <paramref name="constructorMethodSymbol"/>, <see langword="false"/> otherwise.</returns>
private static bool HasMatchingSignature(ImmutableArray<IMethodSymbol> baseConstructorSymbols, IMethodSymbol constructorMethodSymbol)
{
bool found = false;

foreach (IMethodSymbol baseConstructorMethod in baseConstructorSymbols)
{
// Constructors must have the same number of parameters.
if (constructorMethodSymbol.Parameters.Count() != baseConstructorMethod.Parameters.Count())
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
{
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)
{
found = true;
break;
MartyIX marked this conversation as resolved.
Show resolved Hide resolved
}
}

return found;
}

private static bool HasXmlCrefAttribute(XmlNodeSyntax inheritDocElement)
{
XmlElementSyntax xmlElementSyntax = inheritDocElement as XmlElementSyntax;
XmlElementSyntax? xmlElementSyntax = inheritDocElement as XmlElementSyntax;
if (xmlElementSyntax?.StartTag?.Attributes.Any(SyntaxKind.XmlCrefAttribute) ?? false)
{
return true;
}

XmlEmptyElementSyntax xmlEmptyElementSyntax = inheritDocElement as XmlEmptyElementSyntax;
XmlEmptyElementSyntax? xmlEmptyElementSyntax = inheritDocElement as XmlEmptyElementSyntax;
if (xmlEmptyElementSyntax?.Attributes.Any(SyntaxKind.XmlCrefAttribute) ?? false)
{
return true;
Expand Down