Skip to content

Commit fd402ad

Browse files
committed
Address review comments
1 parent b7d10a8 commit fd402ad

2 files changed

Lines changed: 75 additions & 37 deletions

File tree

StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1648UnitTests.cs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,30 @@ public Test(string s, int b)
5454
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
5555
}
5656

57+
[Fact]
58+
public async Task TestConstructorInheritsImplicitlyFromSystemObjectAsync()
59+
{
60+
var testCode = @"class Test
61+
{
62+
/// <inheritdoc />
63+
public Test() { }
64+
}";
65+
66+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
67+
}
68+
69+
[Fact]
70+
public async Task TestConstructorInheritsExplicitlyFromSystemObjectAsync()
71+
{
72+
var testCode = @"class Test : System.Object
73+
{
74+
/// <inheritdoc />
75+
public Test() { }
76+
}";
77+
78+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
79+
}
80+
5781
[Fact]
5882
public async Task TestClassOverridesClassAsync()
5983
{
@@ -126,7 +150,6 @@ public async Task TestTypeWithEmptyBaseListAndCrefAttributeAsync(string declarat
126150
}
127151

128152
[Theory]
129-
[InlineData("Test() { }")]
130153
[InlineData("void Foo() { }")]
131154
[InlineData("string foo;")]
132155
[InlineData("string Foo { get; set; }")]

StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1648InheritDocMustBeUsedWithInheritingClass.cs

Lines changed: 51 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -148,51 +148,24 @@ private static void HandleMemberDeclaration(SyntaxNodeAnalysisContext context)
148148

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

151-
if (memberSyntax is ConstructorDeclarationSyntax constructorDeclarationSyntax)
151+
if (memberSyntax is ConstructorDeclarationSyntax constructorDeclarationSyntax && declaredSymbol is IMethodSymbol constructorMethodSymbol)
152152
{
153-
if (declaredSymbol is IMethodSymbol constructorMethodSymbol && constructorMethodSymbol.ContainingType is INamedTypeSymbol enclosingNamedTypeSymbol)
153+
if (constructorMethodSymbol.ContainingType is INamedTypeSymbol enclosingNamedTypeSymbol)
154154
{
155155
INamedTypeSymbol baseType = enclosingNamedTypeSymbol.BaseType;
156156

157-
if (baseType.SpecialType != SpecialType.System_Object)
157+
if (baseType.SpecialType == SpecialType.System_Object)
158158
{
159-
bool foundMatchingConstructorsToInheritFrom = false;
160-
161-
foreach (IMethodSymbol baseConstructorMethod in baseType.Constructors)
162-
{
163-
// Constructors must have the same number of parameters.
164-
if (constructorMethodSymbol.Parameters.Count() != baseConstructorMethod.Parameters.Count())
165-
{
166-
continue;
167-
}
168-
169-
// Our constructor and the base constructor must have the same signature. But variable names can be different.
170-
bool success = true;
171-
172-
for (int i = 0; i < constructorMethodSymbol.Parameters.Length; i++)
173-
{
174-
IParameterSymbol constructorParameter = constructorMethodSymbol.Parameters[i];
175-
IParameterSymbol baseParameter = baseConstructorMethod.Parameters[i];
176-
177-
if (!constructorParameter.Type.Equals(baseParameter.Type))
178-
{
179-
success = false;
180-
break;
181-
}
182-
}
183-
184-
if (success)
185-
{
186-
foundMatchingConstructorsToInheritFrom = true;
187-
break;
188-
}
189-
}
190-
191-
if (foundMatchingConstructorsToInheritFrom)
159+
// Exception: If the base type is System.Object, then we can use <inheritdoc/> if our constructor has zero parameters.
160+
if (constructorMethodSymbol.Parameters.Length == 0)
192161
{
193162
return;
194163
}
195164
}
165+
else if (HasMatchingSignature(baseType.Constructors, constructorMethodSymbol))
166+
{
167+
return;
168+
}
196169
}
197170
}
198171

@@ -253,6 +226,48 @@ private static void HandleMemberDeclaration(SyntaxNodeAnalysisContext context)
253226
}
254227
}
255228

229+
/// <summary>
230+
/// Method compares a <paramref name="constructorMethodSymbol">constructor method</paramref> signature against its
231+
/// <paramref name="baseConstructorSymbols">base type constructors</paramref> to find if there is a method signature match.
232+
/// </summary>
233+
/// <returns><see langword="true"/> if any base type constructor's signature matches the signature of <paramref name="constructorMethodSymbol"/>, <see langword="false"/> otherwise.</returns>
234+
private static bool HasMatchingSignature(ImmutableArray<IMethodSymbol> baseConstructorSymbols, IMethodSymbol constructorMethodSymbol)
235+
{
236+
bool found = false;
237+
238+
foreach (IMethodSymbol baseConstructorMethod in baseConstructorSymbols)
239+
{
240+
// Constructors must have the same number of parameters.
241+
if (constructorMethodSymbol.Parameters.Count() != baseConstructorMethod.Parameters.Count())
242+
{
243+
continue;
244+
}
245+
246+
// Our constructor and the base constructor must have the same signature. But variable names can be different.
247+
bool success = true;
248+
249+
for (int i = 0; i < constructorMethodSymbol.Parameters.Length; i++)
250+
{
251+
IParameterSymbol constructorParameter = constructorMethodSymbol.Parameters[i];
252+
IParameterSymbol baseParameter = baseConstructorMethod.Parameters[i];
253+
254+
if (!constructorParameter.Type.Equals(baseParameter.Type))
255+
{
256+
success = false;
257+
break;
258+
}
259+
}
260+
261+
if (success)
262+
{
263+
found = true;
264+
break;
265+
}
266+
}
267+
268+
return found;
269+
}
270+
256271
private static bool HasXmlCrefAttribute(XmlNodeSyntax inheritDocElement)
257272
{
258273
XmlElementSyntax? xmlElementSyntax = inheritDocElement as XmlElementSyntax;

0 commit comments

Comments
 (0)