Skip to content

Commit 2959cac

Browse files
sharwellsharwell
committed
Update SA1600 for default interface members
Fixes #3551 Fixes #3717
1 parent d76aeef commit 2959cac

File tree

5 files changed

+183
-8
lines changed

5 files changed

+183
-8
lines changed

‎StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp8/DocumentationRules/SA1600CSharp8UnitTests.cs‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,121 @@
33

44
namespace StyleCop.Analyzers.Test.CSharp8.DocumentationRules
55
{
6+
using System.Threading;
7+
using System.Threading.Tasks;
68
using Microsoft.CodeAnalysis.CSharp;
9+
using Microsoft.CodeAnalysis.Testing;
710
using StyleCop.Analyzers.Test.CSharp7.DocumentationRules;
11+
using Xunit;
12+
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
13+
StyleCop.Analyzers.DocumentationRules.SA1600ElementsMustBeDocumented,
14+
StyleCop.Analyzers.DocumentationRules.SA1600CodeFixProvider>;
815

916
public partial class SA1600CSharp8UnitTests : SA1600CSharp7UnitTests
1017
{
1118
// Using 'Default' here makes sure that later test projects also run these tests with their own language version, without having to override this property
1219
protected override LanguageVersion LanguageVersion => LanguageVersion.Default;
20+
21+
[Fact]
22+
[WorkItem(3002, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3002")]
23+
public async Task TestDefaultInterfaceMemberRequiresDocumentationAsync()
24+
{
25+
var testCode = @"using System;
26+
/// <summary>Summary.</summary>
27+
public interface ITest
28+
{
29+
public static int [|field1|];
30+
static int [|field2|];
31+
32+
public int [|Prop1|] { get => 0; }
33+
int [|Prop2|] { get => 0; }
34+
35+
int [|this|][int index] { get => 0; }
36+
37+
public event EventHandler [|Event1|];
38+
event EventHandler [|Event2|];
39+
40+
public event EventHandler [|Event3|] { add { } remove { } }
41+
event EventHandler [|Event4|] { add { } remove { } }
42+
43+
public void [|Method1|]() { }
44+
void [|Method2|]() { }
45+
46+
public delegate void [|Del1|]();
47+
delegate void [|Del2|]();
48+
49+
public class [|Class1|] { }
50+
class [|Class2|] { }
51+
52+
public struct [|Struct1|] { }
53+
struct [|Struct2|] { }
54+
55+
public interface [|Interface1|] { }
56+
interface [|Interface2|] { }
57+
}
58+
";
59+
60+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
61+
}
62+
63+
[Fact]
64+
[WorkItem(3002, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3002")]
65+
public async Task TestDefaultInterfaceFieldRequiresDocumentationAsync()
66+
{
67+
var testCode = @"
68+
/// <summary>Summary.</summary>
69+
public interface ITest
70+
{
71+
public static int [|field1|];
72+
static int [|field2|];
73+
}
74+
";
75+
76+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
77+
}
78+
79+
[Fact]
80+
[WorkItem(3002, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3002")]
81+
public async Task TestPrivateDefaultInterfaceMethodDoesNotRequireDocumentationByDefaultAsync()
82+
{
83+
var testCode = @"
84+
/// <summary>Summary.</summary>
85+
public interface ITest
86+
{
87+
private void M()
88+
{
89+
}
90+
}
91+
";
92+
93+
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
94+
}
95+
96+
[Fact]
97+
[WorkItem(3002, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3002")]
98+
public async Task TestPrivateDefaultInterfaceMethodHonorsDocumentPrivateElementsAsync()
99+
{
100+
var testCode = @"
101+
/// <summary>Summary.</summary>
102+
public interface ITest
103+
{
104+
private void [|M|]()
105+
{
106+
}
107+
}
108+
";
109+
110+
var settings = @"
111+
{
112+
""settings"": {
113+
""documentationRules"": {
114+
""documentPrivateElements"": true
115+
}
116+
}
117+
}
118+
";
119+
120+
await VerifyCSharpDiagnosticAsync(testCode, settings, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
121+
}
13122
}
14123
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1600UnitTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public async Task TestDelegateWithDocumentationAsync()
9999
public async Task TestMethodWithoutDocumentationAsync()
100100
{
101101
await this.TestMethodDeclarationDocumentationAsync(string.Empty, false, false, false).ConfigureAwait(false);
102-
await this.TestMethodDeclarationDocumentationAsync(string.Empty, true, true, false).ConfigureAwait(false);
102+
await this.TestMethodDeclarationDocumentationAsync(string.Empty, true, false, false).ConfigureAwait(false);
103103
await this.TestMethodDeclarationDocumentationAsync("private", false, false, false).ConfigureAwait(false);
104104
await this.TestMethodDeclarationDocumentationAsync("protected", false, true, false).ConfigureAwait(false);
105105
await this.TestMethodDeclarationDocumentationAsync("internal", false, true, false).ConfigureAwait(false);
@@ -291,7 +291,7 @@ public async Task TestFieldWithDocumentationAsync()
291291
public async Task TestPropertyWithoutDocumentationAsync()
292292
{
293293
await this.TestPropertyDeclarationDocumentationAsync(string.Empty, false, false, false).ConfigureAwait(false);
294-
await this.TestPropertyDeclarationDocumentationAsync(string.Empty, true, true, false).ConfigureAwait(false);
294+
await this.TestPropertyDeclarationDocumentationAsync(string.Empty, true, false, false).ConfigureAwait(false);
295295
await this.TestPropertyDeclarationDocumentationAsync("private", false, false, false).ConfigureAwait(false);
296296
await this.TestPropertyDeclarationDocumentationAsync("protected", false, true, false).ConfigureAwait(false);
297297
await this.TestPropertyDeclarationDocumentationAsync("internal", false, true, false).ConfigureAwait(false);
@@ -319,7 +319,7 @@ public async Task TestPropertyWithDocumentationAsync()
319319
public async Task TestIndexerWithoutDocumentationAsync()
320320
{
321321
await this.TestIndexerDeclarationDocumentationAsync(string.Empty, false, false, false).ConfigureAwait(false);
322-
await this.TestIndexerDeclarationDocumentationAsync(string.Empty, true, true, false).ConfigureAwait(false);
322+
await this.TestIndexerDeclarationDocumentationAsync(string.Empty, true, false, false).ConfigureAwait(false);
323323
await this.TestIndexerDeclarationDocumentationAsync("private", false, false, false).ConfigureAwait(false);
324324
await this.TestIndexerDeclarationDocumentationAsync("protected", false, true, false).ConfigureAwait(false);
325325
await this.TestIndexerDeclarationDocumentationAsync("internal", false, true, false).ConfigureAwait(false);
@@ -347,7 +347,7 @@ public async Task TestIndexerWithDocumentationAsync()
347347
public async Task TestEventWithoutDocumentationAsync()
348348
{
349349
await this.TestEventDeclarationDocumentationAsync(string.Empty, false, false, false).ConfigureAwait(false);
350-
await this.TestEventDeclarationDocumentationAsync(string.Empty, true, true, false).ConfigureAwait(false);
350+
await this.TestEventDeclarationDocumentationAsync(string.Empty, true, false, false).ConfigureAwait(false);
351351
await this.TestEventDeclarationDocumentationAsync("private", false, false, false).ConfigureAwait(false);
352352
await this.TestEventDeclarationDocumentationAsync("protected", false, true, false).ConfigureAwait(false);
353353
await this.TestEventDeclarationDocumentationAsync("internal", false, true, false).ConfigureAwait(false);

‎StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/SA1600ElementsMustBeDocumented.cs‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ internal class SA1600ElementsMustBeDocumented : DiagnosticAnalyzer
5959

6060
public static bool NeedsComment(DocumentationSettings documentationSettings, SyntaxKind syntaxKind, SyntaxKind parentSyntaxKind, Accessibility declaredAccessibility, Accessibility effectiveAccessibility)
6161
{
62-
if (syntaxKind == SyntaxKind.InterfaceDeclaration || parentSyntaxKind == SyntaxKind.InterfaceDeclaration)
62+
if (syntaxKind == SyntaxKind.InterfaceDeclaration)
6363
{
6464
switch (documentationSettings.DocumentInterfaces)
6565
{
@@ -75,6 +75,27 @@ public static bool NeedsComment(DocumentationSettings documentationSettings, Syn
7575
}
7676
}
7777

78+
if (parentSyntaxKind == SyntaxKind.InterfaceDeclaration)
79+
{
80+
bool isPublicInterfaceMember = declaredAccessibility is Accessibility.Public or Accessibility.NotApplicable;
81+
if (!isPublicInterfaceMember)
82+
{
83+
return documentationSettings.DocumentPrivateElements;
84+
}
85+
86+
// At this point we know the declared accessibility is public
87+
switch (documentationSettings.DocumentInterfaces)
88+
{
89+
case InterfaceDocumentationMode.All:
90+
case InterfaceDocumentationMode.Exposed:
91+
return true;
92+
93+
default:
94+
// DocumentInterfaces => no interface members should be documented
95+
return false;
96+
}
97+
}
98+
7899
if (syntaxKind == SyntaxKind.FieldDeclaration && documentationSettings.DocumentPrivateFields)
79100
{
80101
// DocumentPrivateFields => all fields should be documented
@@ -169,6 +190,10 @@ public static void HandleMethodDeclaration(SyntaxNodeAnalysisContext context, St
169190
}
170191

171192
MethodDeclarationSyntax declaration = (MethodDeclarationSyntax)context.Node;
193+
if (declaration.ExplicitInterfaceSpecifier != null)
194+
{
195+
return;
196+
}
172197

173198
Accessibility declaredAccessibility = declaration.GetDeclaredAccessibility(context.SemanticModel, context.CancellationToken);
174199
Accessibility effectiveAccessibility = declaration.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken);
@@ -229,6 +254,10 @@ public static void HandlePropertyDeclaration(SyntaxNodeAnalysisContext context,
229254
}
230255

231256
PropertyDeclarationSyntax declaration = (PropertyDeclarationSyntax)context.Node;
257+
if (declaration.ExplicitInterfaceSpecifier != null)
258+
{
259+
return;
260+
}
232261

233262
Accessibility declaredAccessibility = declaration.GetDeclaredAccessibility(context.SemanticModel, context.CancellationToken);
234263
Accessibility effectiveAccessibility = declaration.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken);
@@ -249,6 +278,10 @@ public static void HandleIndexerDeclaration(SyntaxNodeAnalysisContext context, S
249278
}
250279

251280
IndexerDeclarationSyntax declaration = (IndexerDeclarationSyntax)context.Node;
281+
if (declaration.ExplicitInterfaceSpecifier != null)
282+
{
283+
return;
284+
}
252285

253286
Accessibility declaredAccessibility = declaration.GetDeclaredAccessibility(context.SemanticModel, context.CancellationToken);
254287
Accessibility effectiveAccessibility = declaration.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken);
@@ -314,6 +347,10 @@ public static void HandleEventDeclaration(SyntaxNodeAnalysisContext context, Sty
314347
}
315348

316349
EventDeclarationSyntax declaration = (EventDeclarationSyntax)context.Node;
350+
if (declaration.ExplicitInterfaceSpecifier != null)
351+
{
352+
return;
353+
}
317354

318355
Accessibility declaredAccessibility = declaration.GetDeclaredAccessibility(context.SemanticModel, context.CancellationToken);
319356
Accessibility effectiveAccessibility = declaration.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken);

‎StyleCop.Analyzers/StyleCop.Analyzers/Helpers/AccessLevelHelper.cs‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,11 @@ internal static Accessibility GetDeclaredAccessibility(this BaseTypeDeclarationS
158158

159159
if (!syntax.Modifiers.Any(SyntaxKind.PartialKeyword))
160160
{
161+
if (syntax.Parent.IsKind(SyntaxKind.InterfaceDeclaration))
162+
{
163+
return Accessibility.Public;
164+
}
165+
161166
return !(syntax.Parent is BaseTypeDeclarationSyntax) ? Accessibility.Internal : Accessibility.Private;
162167
}
163168

@@ -181,6 +186,11 @@ internal static Accessibility GetDeclaredAccessibility(this BaseMethodDeclaratio
181186
return Accessibility.Private;
182187
}
183188

189+
if (syntax.Parent.IsKind(SyntaxKind.InterfaceDeclaration))
190+
{
191+
return Accessibility.Public;
192+
}
193+
184194
if (syntax is MethodDeclarationSyntax methodDeclarationSyntax)
185195
{
186196
if (methodDeclarationSyntax.ExplicitInterfaceSpecifier == null)
@@ -217,7 +227,7 @@ internal static Accessibility GetDeclaredAccessibility(this BasePropertyDeclarat
217227
{
218228
if (propertyDeclarationSyntax.ExplicitInterfaceSpecifier == null)
219229
{
220-
return Accessibility.Private;
230+
return syntax.Parent.IsKind(SyntaxKind.InterfaceDeclaration) ? Accessibility.Public : Accessibility.Private;
221231
}
222232
else
223233
{
@@ -229,7 +239,7 @@ internal static Accessibility GetDeclaredAccessibility(this BasePropertyDeclarat
229239
{
230240
if (indexerDeclarationSyntax.ExplicitInterfaceSpecifier == null)
231241
{
232-
return Accessibility.Private;
242+
return syntax.Parent.IsKind(SyntaxKind.InterfaceDeclaration) ? Accessibility.Public : Accessibility.Private;
233243
}
234244
else
235245
{
@@ -241,7 +251,7 @@ internal static Accessibility GetDeclaredAccessibility(this BasePropertyDeclarat
241251
{
242252
if (eventDeclarationSyntax.ExplicitInterfaceSpecifier == null)
243253
{
244-
return Accessibility.Private;
254+
return syntax.Parent.IsKind(SyntaxKind.InterfaceDeclaration) ? Accessibility.Public : Accessibility.Private;
245255
}
246256
else
247257
{
@@ -281,6 +291,11 @@ internal static Accessibility GetDeclaredAccessibility(this BaseFieldDeclaration
281291

282292
if (syntax.IsKind(SyntaxKind.FieldDeclaration) || syntax.IsKind(SyntaxKind.EventFieldDeclaration))
283293
{
294+
if (syntax.Parent.IsKind(SyntaxKind.InterfaceDeclaration))
295+
{
296+
return Accessibility.Public;
297+
}
298+
284299
return Accessibility.Private;
285300
}
286301

@@ -316,6 +331,11 @@ internal static Accessibility GetDeclaredAccessibility(this DelegateDeclarationS
316331
return accessLevel.ToAccessibility();
317332
}
318333

334+
if (syntax.Parent.IsKind(SyntaxKind.InterfaceDeclaration))
335+
{
336+
return Accessibility.Public;
337+
}
338+
319339
return !(syntax.Parent is BaseTypeDeclarationSyntax) ? Accessibility.Internal : Accessibility.Private;
320340
}
321341

‎documentation/SA1600.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ C# syntax provides a mechanism for inserting documentation for classes and eleme
2525

2626
A violation of this rule occurs if an element is completely missing a documentation header, or if the header is empty. In C# the following types of elements can have documentation headers: classes, constructors, delegates, enums, events, finalizers, indexers, interfaces, methods, properties, records, and structs.
2727

28+
### Default interface members
29+
30+
Interface members are implicitly `public` unless otherwise specified. Default interface members (methods, properties, indexers, events, etc. that include implementations) follow these rules:
31+
32+
* Public interface members still require documentation when interfaces are configured to be documented (the default).
33+
* Non-public interface members (such as `private` default implementations) follow the `documentPrivateElements` setting. When `documentPrivateElements` is `false` (default), these members do not require documentation.
34+
35+
Explicit interface implementations in classes do not require separate documentation; they inherit documentation from the interface definition.
36+
2837
## How to fix violations
2938

3039
To fix a violation of this rule, add or fill-in a documentation header for the element.

0 commit comments

Comments
 (0)