Skip to content

Commit 0d248da

Browse files
committed
Added new rule MiKo_2230 that reports if return value documentation should better use a <list> do describe the return values and their meanings
(resolves #822)
1 parent cd09255 commit 0d248da

File tree

6 files changed

+196
-1
lines changed

6 files changed

+196
-1
lines changed

MiKo.Analyzer.Shared/MiKo.Analyzer.Shared.projitems

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@
143143
<Compile Include="$(MSBuildThisFileDirectory)Rules\Documentation\MiKo_2226_DocumentationContainsIntentionallyAnalyzer.cs" />
144144
<Compile Include="$(MSBuildThisFileDirectory)Rules\Documentation\MiKo_2227_DocumentationDoesNotContainReSharperMarkerAnalyzer.cs" />
145145
<Compile Include="$(MSBuildThisFileDirectory)Rules\Documentation\MiKo_2228_DocumentationIsNotNegativeAnalyzer.cs" />
146+
<Compile Include="$(MSBuildThisFileDirectory)Rules\Documentation\MiKo_2230_ReturnValueUsesListAnalyzer.cs" />
146147
<Compile Include="$(MSBuildThisFileDirectory)Rules\Documentation\MiKo_2229_XmlFragmentAnalyzer.cs" />
147148
<Compile Include="$(MSBuildThisFileDirectory)Rules\Documentation\MiKo_2300_MeaninglessCommentAnalyzer.cs" />
148149
<Compile Include="$(MSBuildThisFileDirectory)Rules\Documentation\MiKo_2301_TestArrangeActAssertCommentAnalyzer.cs" />

MiKo.Analyzer.Shared/Resources.Designer.cs

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

MiKo.Analyzer.Shared/Resources.resx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2899,6 +2899,17 @@ Positive wording is much easier to understand as it is straight forward and come
28992899
<data name="MiKo_2229_Title" xml:space="preserve">
29002900
<value>Documentation should not contain left-over XML fragments</value>
29012901
</data>
2902+
<data name="MiKo_2230_Description" xml:space="preserve">
2903+
<value>When return values with specific meanings are documented via "Value Meaning", that documentation should be part of a &lt;list&gt;.
2904+
This makes it easier to read because the XML documentation renderer will place the contents in a list.
2905+
In contrast, when the documentation is just some plain text then that is very difficult to read and understand because all text is place behind each other in a single paragraph.</value>
2906+
</data>
2907+
<data name="MiKo_2230_MessageFormat" xml:space="preserve">
2908+
<value>Use &lt;list&gt; instead for values and their meaning</value>
2909+
</data>
2910+
<data name="MiKo_2230_Title" xml:space="preserve">
2911+
<value>Documentation of return value should use &lt;list&gt; when there are values with specific meanings</value>
2912+
</data>
29022913
<data name="MiKo_2300_Description" xml:space="preserve">
29032914
<value>Comments should explain the deeper reasons behind the code to understand why the code is written in that way.
29042915
They should not describe how it is achieved because that's what the code is for.</value>
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
using System;
2+
using System.Collections.Generic;
3+
4+
using Microsoft.CodeAnalysis;
5+
6+
using Microsoft.CodeAnalysis.CSharp.Syntax;
7+
using Microsoft.CodeAnalysis.Diagnostics;
8+
9+
namespace MiKoSolutions.Analyzers.Rules.Documentation
10+
{
11+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
12+
public sealed class MiKo_2230_ReturnValueUsesListAnalyzer : ReturnsValueDocumentationAnalyzer
13+
{
14+
public const string Id = "MiKo_2230";
15+
16+
public MiKo_2230_ReturnValueUsesListAnalyzer() : base(Id)
17+
{
18+
}
19+
20+
protected override IEnumerable<Diagnostic> AnalyzeReturnType(ISymbol owningSymbol, ITypeSymbol returnType, DocumentationCommentTriviaSyntax comment, string commentXml, string xmlTag)
21+
{
22+
foreach (var token in comment.GetXmlSyntax(xmlTag).GetXmlTextTokens())
23+
{
24+
var text = token.ValueText;
25+
26+
if (text.Contains("Value Meaning", StringComparison.Ordinal))
27+
{
28+
yield return Issue(token);
29+
}
30+
}
31+
}
32+
}
33+
}
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
using Microsoft.CodeAnalysis.Diagnostics;
2+
3+
using NUnit.Framework;
4+
5+
using TestHelper;
6+
7+
//// ncrunch: rdi off
8+
namespace MiKoSolutions.Analyzers.Rules.Documentation
9+
{
10+
[TestFixture]
11+
public sealed class MiKo_2230_ReturnValueUsesListAnalyzerTests : CodeFixVerifier
12+
{
13+
[Test]
14+
public void No_issue_is_reported_for_method_with_no_comment() => No_issue_is_reported_for(@"
15+
public class TestMe
16+
{
17+
public int DoSomething()
18+
{ }
19+
}
20+
");
21+
22+
[Test]
23+
public void No_issue_is_reported_for_empty_returns() => No_issue_is_reported_for(@"
24+
public class TestMe
25+
{
26+
/// <summary></summary>
27+
public int DoSomething()
28+
{ }
29+
}
30+
");
31+
32+
[Test]
33+
public void No_issue_is_reported_for_problematic_text_in_summary() => No_issue_is_reported_for(@"
34+
public class TestMe
35+
{
36+
/// <summary>
37+
/// A value that indicates something being compared. The return value has these meanings: Value Meaning Less than zero Some text here. Zero Some other text here. Greater than zero Some even other text here.
38+
/// </summary>
39+
public int DoSomething()
40+
{ }
41+
}
42+
");
43+
44+
[Test]
45+
public void No_issue_is_reported_for_problematic_text_in_remarks() => No_issue_is_reported_for(@"
46+
public class TestMe
47+
{
48+
/// <remarks>
49+
/// A value that indicates something being compared. The return value has these meanings: Value Meaning Less than zero Some text here. Zero Some other text here. Greater than zero Some even other text here.
50+
/// </remarks>
51+
public int DoSomething()
52+
{ }
53+
}
54+
");
55+
56+
[Test]
57+
public void No_issue_is_reported_for_correct_text_list_in_returns() => No_issue_is_reported_for(@"
58+
public class TestMe
59+
{
60+
/// <returns>
61+
/// A value that indicates something being compared.
62+
/// The return value has these meanings:
63+
/// <list type=""table"">
64+
/// <listheader><term>Value</term><description>Meaning</description></listheader>
65+
/// <item><term>Less than zero</term><description>Some text here.</description></item>
66+
/// <item><term>Zero</term><description>Some other text here.</description></item>
67+
/// <item><term>Greater than zero</term><description>Some even other text here.</description></item>
68+
/// </list>
69+
/// </returns>
70+
public int DoSomething()
71+
{ }
72+
}
73+
");
74+
75+
[Test]
76+
public void No_issue_is_reported_for_correct_text_list_in_value() => No_issue_is_reported_for(@"
77+
public class TestMe
78+
{
79+
/// <value>
80+
/// A value that indicates something being compared.
81+
/// The return value has these meanings:
82+
/// <list type=""table"">
83+
/// <listheader><term>Value</term><description>Meaning</description></listheader>
84+
/// <item><term>Less than zero</term><description>Some text here.</description></item>
85+
/// <item><term>Zero</term><description>Some other text here.</description></item>
86+
/// <item><term>Greater than zero</term><description>Some even other text here.</description></item>
87+
/// </list>
88+
/// </value>
89+
public int Something { get; set; }
90+
}
91+
");
92+
93+
[Test]
94+
public void An_issue_is_reported_for_problematic_text_in_returns() => An_issue_is_reported_for(@"
95+
public class TestMe
96+
{
97+
/// <returns>
98+
/// A value that indicates something being compared. The return value has these meanings: Value Meaning Less than zero Some text here. Zero Some other text here. Greater than zero Some even other text here.
99+
/// </returns>
100+
public int DoSomething()
101+
{ }
102+
}
103+
");
104+
105+
[Test]
106+
public void An_issue_is_reported_for_problematic_text_in_value() => An_issue_is_reported_for(@"
107+
public class TestMe
108+
{
109+
/// <value>
110+
/// A value that indicates something being compared. The return value has these meanings: Value Meaning Less than zero Some text here. Zero Some other text here. Greater than zero Some even other text here.
111+
/// </value>
112+
public int Something { get; set; }
113+
}
114+
");
115+
116+
protected override string GetDiagnosticId() => MiKo_2230_ReturnValueUsesListAnalyzer.Id;
117+
118+
protected override DiagnosticAnalyzer GetObjectUnderTest() => new MiKo_2230_ReturnValueUsesListAnalyzer();
119+
}
120+
}

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ Screenshots on how to use such analyzers can be found [here](https://learn.micro
1515
[![Build history](https://buildstats.info/appveyor/chart/RalfKoban/miko-analyzers)](https://ci.appveyor.com/project/RalfKoban/miko-analyzers/history)
1616

1717
## Available Rules
18-
The following tables lists all the 437 rules that are currently provided by the analyzer.
18+
The following tables lists all the 438 rules that are currently provided by the analyzer.
1919

2020
### Metrics
2121
|ID|Title|Enabled by default|CodeFix available|
@@ -262,6 +262,7 @@ The following tables lists all the 437 rules that are currently provided by the
262262
|MiKo_2227|Documentation should not contain ReSharper suppressions|&#x2713;|\-|
263263
|MiKo_2228|Documentation should use positive wording instead of negative|&#x2713;|\-|
264264
|MiKo_2229|Documentation should not contain left-over XML fragments|&#x2713;|&#x2713;|
265+
|MiKo_2230|Documentation of return value should use &lt;list&gt; when there are values with specific meanings|&#x2713;|&#x2713;|
265266
|MiKo_2300|Comments should explain the 'Why' and not the 'How'|&#x2713;|\-|
266267
|MiKo_2301|Do not use obvious comments in AAA-Tests|&#x2713;|&#x2713;|
267268
|MiKo_2302|Do not keep code that is commented out|&#x2713;|\-|

0 commit comments

Comments
 (0)