Skip to content

Commit 6c1c9f0

Browse files
committed
Merge pull request #1825 from sharwell/fix-nre
Fix NullReferenceException in SA1623/SA1624
2 parents 995e93d + 370a880 commit 6c1c9f0

5 files changed

Lines changed: 58 additions & 13 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,15 @@ public class SA1623UnitTests : CodeFixVerifier
3232
[InlineData("public", "int", "{ get; internal set; }", "Gets")]
3333
[InlineData("public", "int", "{ get; private set; }", "Gets")]
3434
[InlineData("public", "int", "{ get; }", "Gets")]
35+
[InlineData("public", "int", "{ get; } = 0;", "Gets")]
36+
[InlineData("public", "int", "=> 0;", "Gets")]
3537
[InlineData("public", "int", "{ set { } }", "Sets")]
3638
[InlineData("public", "int", "{ internal get { return 0; } set { } }", "Sets")]
3739
[InlineData("public", "int", "{ private get { return 0; } set { } }", "Sets")]
3840
[InlineData("public", "bool", "{ get; set; }", "Gets or sets a value indicating whether")]
3941
[InlineData("public", "bool", "{ get; }", "Gets a value indicating whether")]
42+
[InlineData("public", "bool", "{ get; } = true;", "Gets a value indicating whether")]
43+
[InlineData("public", "bool", "=> true;", "Gets a value indicating whether")]
4044
[InlineData("public", "bool", "{ get; private set; }", "Gets a value indicating whether")]
4145
[InlineData("public", "bool", "{ set { } }", "Sets a value indicating whether")]
4246
[InlineData("public", "bool", "{ private get { return false; } set { } }", "Sets a value indicating whether")]

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,36 @@ public class TestClass
7777
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
7878
}
7979

80+
/// <summary>
81+
/// Verifies that documentation that starts with the proper text for multiple accessors will not produce this
82+
/// diagnostic when the property has an expression body.
83+
/// </summary>
84+
/// <param name="accessibility">The accessibility of the property.</param>
85+
/// <param name="type">The type to use for the property.</param>
86+
/// <param name="summaryPrefix">The prefix to use in the summary text.</param>
87+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
88+
[Theory]
89+
[InlineData("public", "int", "Gets or sets")]
90+
[InlineData("public", "bool", "Gets or sets a value indicating whether")]
91+
[InlineData("protected", "int", "Gets or sets")]
92+
[InlineData("protected internal", "int", "Gets or sets")]
93+
[InlineData("internal", "int", "Gets or sets")]
94+
public async Task VerifyThatInvalidDocumentationWillReportDiagnosticForExpressionBodyAsync(string accessibility, string type, string summaryPrefix)
95+
{
96+
var testCode = $@"
97+
public class TestClass
98+
{{
99+
/// <summary>
100+
/// {summaryPrefix} the test property.
101+
/// </summary>
102+
{accessibility} {type} TestProperty =>
103+
default({type});
104+
}}
105+
";
106+
107+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
108+
}
109+
80110
/// <summary>
81111
/// 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.
82112
/// </summary>
@@ -109,6 +139,12 @@ public int TestProperty
109139
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
110140
}
111141

142+
/// <inheritdoc/>
143+
protected override IEnumerable<string> GetDisabledDiagnostics()
144+
{
145+
yield return PropertySummaryDocumentationAnalyzer.SA1623Descriptor.Id;
146+
}
147+
112148
/// <inheritdoc/>
113149
protected override CodeFixProvider GetCSharpCodeFixProvider()
114150
{

StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/DocumentationResources.Designer.cs

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

StyleCop.Analyzers/StyleCop.Analyzers/DocumentationRules/DocumentationResources.resx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@
148148
<value>The property's documentation summary text must begin with: '{0}'</value>
149149
</data>
150150
<data name="SA1623Title" xml:space="preserve">
151-
<value>Property summary documentation must match accessors.</value>
151+
<value>Property summary documentation must match accessors</value>
152152
</data>
153153
<data name="SA1624Description" xml:space="preserve">
154154
<value>The documentation text within a C# property’s &lt;summary&gt; tag takes into account all of the accessors within the property, but one of the accessors has limited access.</value>
@@ -157,7 +157,7 @@
157157
<value>Because the property only contains a visible {0} accessor, the documentation summary text must begin with '{1}'.</value>
158158
</data>
159159
<data name="SA1624Title" xml:space="preserve">
160-
<value>Property summary documentation must omit accessor with restricted access.</value>
160+
<value>Property summary documentation must omit accessor with restricted access</value>
161161
</data>
162162
<data name="SA1626CodeFix" xml:space="preserve">
163163
<value>Convert to line comment</value>

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

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,20 +80,24 @@ protected override void HandleXmlElement(SyntaxNodeAnalysisContext context, XmlN
8080
private static void AnalyzeSummaryElement(SyntaxNodeAnalysisContext context, XmlNodeSyntax syntax, Location diagnosticLocation, PropertyDeclarationSyntax propertyDeclaration, string startingTextGets, string startingTextSets, string startingTextGetsOrSets)
8181
{
8282
var diagnosticProperties = ImmutableDictionary.CreateBuilder<string, string>();
83+
ArrowExpressionClauseSyntax expressionBody = propertyDeclaration.ExpressionBody;
8384
AccessorDeclarationSyntax getter = null;
8485
AccessorDeclarationSyntax setter = null;
8586

86-
foreach (var accessor in propertyDeclaration.AccessorList.Accessors)
87+
if (propertyDeclaration.AccessorList != null)
8788
{
88-
switch (accessor.Keyword.Kind())
89+
foreach (var accessor in propertyDeclaration.AccessorList.Accessors)
8990
{
90-
case SyntaxKind.GetKeyword:
91-
getter = accessor;
92-
break;
91+
switch (accessor.Keyword.Kind())
92+
{
93+
case SyntaxKind.GetKeyword:
94+
getter = accessor;
95+
break;
9396

94-
case SyntaxKind.SetKeyword:
95-
setter = accessor;
96-
break;
97+
case SyntaxKind.SetKeyword:
98+
setter = accessor;
99+
break;
100+
}
97101
}
98102
}
99103

@@ -107,12 +111,13 @@ private static void AnalyzeSummaryElement(SyntaxNodeAnalysisContext context, Xml
107111
var textElement = summaryElement.Content.FirstOrDefault() as XmlTextSyntax;
108112
var text = textElement == null ? string.Empty : XmlCommentHelper.GetText(textElement, true).TrimStart();
109113

110-
if (getter != null)
114+
if (getter != null || expressionBody != null)
111115
{
112116
bool startsWithGetOrSet = text.StartsWith(startingTextGetsOrSets, StringComparison.Ordinal);
113117

114118
if (setter != null)
115119
{
120+
// There is no way getter is null (can't have expression body and accessor list)
116121
var getterAccessibility = getter.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken);
117122
var setterAccessibility = setter.GetEffectiveAccessibility(context.SemanticModel, context.CancellationToken);
118123
bool getterVisible;

0 commit comments

Comments
 (0)