Skip to content

Commit 816dc8f

Browse files
Updated PropertySummaryDocumentationAnalyzer to always report SA1623 for a property with a lone getter.
1 parent c8d2158 commit 816dc8f

3 files changed

Lines changed: 31 additions & 48 deletions

File tree

StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1623UnitTests.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,14 @@ public int Property
188188
[Theory]
189189
[InlineData("public", "int", "{ get; set; }", "Gets", "Gets or sets")] // Regression test for #2098
190190
[InlineData("public", "int", "{ get; set; }", "Sets", "Gets or sets")] // Regression test for #2098
191+
[InlineData("public", "int", "{ get; }", "Sets", "Gets")] // Regression test for #2253
192+
[InlineData("public", "int", "{ get; }", "Gets or sets", "Gets")] // Regression test for #2253
193+
[InlineData("public", "int", "=> 0;", "Sets", "Gets")] // Regression test for #2253
194+
[InlineData("public", "int", "=> 0;", "Gets or sets", "Gets")] // Regression test for #2253
195+
[InlineData("public", "bool", "=> false;", "Gets or sets a value indicating whether", "Gets a value indicating whether")] // Regression test for #2253
196+
[InlineData("protected", "int", "=> 0;", "Gets or sets", "Gets")] // Regression test for #2253
197+
[InlineData("protected internal", "int", "=> 0;", "Gets or sets", "Gets")] // Regression test for #2253
198+
[InlineData("internal", "int", "=> 0;", "Gets or sets", "Gets")] // Regression test for #2253
191199
public async Task IncorrectSummaryTagWithKnownPrefixShouldBeFixedCorrectlyAsync(string accessibility, string type, string accessors, string summaryPrefix, string expectedArgument)
192200
{
193201
var testCode = $@"

StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1624UnitTests.cs

Lines changed: 1 addition & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public class TestClass
7878
}
7979

8080
/// <summary>
81-
/// Verifies that documentation that starts with the proper text for multiple accessors will not produce this
81+
/// Verifies that documentation that starts with the proper text for a lone getter will not produce this
8282
/// diagnostic when the property has an expression body.
8383
/// </summary>
8484
/// <param name="accessibility">The accessibility of the property.</param>
@@ -107,52 +107,6 @@ public class TestClass
107107
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
108108
}
109109

110-
/// <summary>
111-
/// Verifies that documentation that starts with the proper text for multiple accessors will produce a diagnostic for expression body properties.
112-
/// </summary>
113-
/// <param name="accessibility">The accessibility of the property.</param>
114-
/// <param name="type">The type to use for the property.</param>
115-
/// <param name="summaryPrefix">The prefix to use in the summary text.</param>
116-
/// <param name="expectedArgument1">The first expected argument for the diagnostic.</param>
117-
/// <param name="expectedArgument2">The second expected argument for the diagnostic.</param>
118-
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
119-
[Theory(DisplayName = "ExpressionBody Gets&Sets")]
120-
[InlineData("public", "int", "Gets or sets", "get", "Gets")]
121-
[InlineData("public", "bool", "Gets or sets a value indicating whether", "get", "Gets a value indicating whether")]
122-
[InlineData("protected", "int", "Gets or sets", "get", "Gets")]
123-
[InlineData("protected internal", "int", "Gets or sets", "get", "Gets")]
124-
[InlineData("internal", "int", "Gets or sets", "get", "Gets")]
125-
public async Task VerifyThatInvalidDocumentationWillReportDiagnosticForExpressionBodyAsync(string accessibility, string type, string summaryPrefix, string expectedArgument1, string expectedArgument2)
126-
{
127-
var testCode = $@"
128-
public class TestClass
129-
{{
130-
/// <summary>
131-
/// {summaryPrefix} the test property.
132-
/// </summary>
133-
{accessibility} {type} TestProperty =>
134-
default({type});
135-
}}
136-
";
137-
138-
var fixedTestCode = $@"
139-
public class TestClass
140-
{{
141-
/// <summary>
142-
/// {expectedArgument2} the test property.
143-
/// </summary>
144-
{accessibility} {type} TestProperty =>
145-
default({type});
146-
}}
147-
";
148-
149-
var expected = this.CSharpDiagnostic(PropertySummaryDocumentationAnalyzer.SA1624Descriptor).WithLocation(7, 7 + accessibility.Length + type.Length).WithArguments(expectedArgument1, expectedArgument2);
150-
151-
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
152-
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
153-
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
154-
}
155-
156110
/// <summary>
157111
/// Verify that no diagnostic will be reported when a public and a private accessor are present within a property that is defined in a contained class of a private class.
158112
/// </summary>

StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/PropertySummaryDocumentationAnalyzer.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ private static void AnalyzeSummaryElement(SyntaxNodeAnalysisContext context, Xml
228228
{
229229
if (setterVisible)
230230
{
231+
// Both getter and setter are visible.
231232
if (!startsWithGetsOrSets)
232233
{
233234
diagnosticProperties.Add(ExpectedTextKey, startingTextGetsOrSets);
@@ -244,8 +245,9 @@ private static void AnalyzeSummaryElement(SyntaxNodeAnalysisContext context, Xml
244245
context.ReportDiagnostic(Diagnostic.Create(SA1623Descriptor, diagnosticLocation, diagnosticProperties.ToImmutable(), startingTextGetsOrSets));
245246
}
246247
}
247-
else
248+
else if (setter != null)
248249
{
250+
// Both getter and setter exist, but only getter is visible.
249251
if (startsWithGetsOrSets)
250252
{
251253
diagnosticProperties.Add(ExpectedTextKey, startingTextGets);
@@ -258,6 +260,25 @@ private static void AnalyzeSummaryElement(SyntaxNodeAnalysisContext context, Xml
258260
context.ReportDiagnostic(Diagnostic.Create(SA1623Descriptor, diagnosticLocation, diagnosticProperties.ToImmutable(), startingTextGets));
259261
}
260262
}
263+
else
264+
{
265+
// Getter exists and is visible. Setter does not exist.
266+
if (!startsWithGets || startsWithGetsOrSets)
267+
{
268+
diagnosticProperties.Add(ExpectedTextKey, startingTextGets);
269+
270+
if (startsWithSets)
271+
{
272+
diagnosticProperties.Add(TextToRemoveKey, text.Substring(0, startingTextSets.Length));
273+
}
274+
else if (startsWithGetsOrSets)
275+
{
276+
diagnosticProperties.Add(TextToRemoveKey, text.Substring(0, startingTextGetsOrSets.Length));
277+
}
278+
279+
context.ReportDiagnostic(Diagnostic.Create(SA1623Descriptor, diagnosticLocation, diagnosticProperties.ToImmutable(), startingTextGets));
280+
}
281+
}
261282
}
262283
else if (setterVisible)
263284
{

0 commit comments

Comments
 (0)