Skip to content

Commit 7ceff21

Browse files
committed
Added two new code analysis rules: SSS005 (sort simple switches) and SSS006 (chain StringBuilder APIs where possible).
1 parent fa2e876 commit 7ceff21

17 files changed

Lines changed: 980 additions & 53 deletions

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ Six scopes, one home each. **Add new state to whichever class matches its true s
8181
- **SSS002**: a `readonly` field in a non-public-API type whose declared type is a strict supertype of its initializer should be declared as the concrete type. Public types, value-typed initializers (boxing), const fields, and uninitialized fields exempt.
8282
- **SSS003**: `string.ToUpperInvariant()` / `ToLowerInvariant()` whose result is the *governing expression* of a `switch` allocates a temporary string. Use the `Span<char>` overload — `Span<char> buf = stackalloc char[s.Length]; s.AsSpan().ToUpperInvariant(buf)` — and switch on the resulting count.
8383
- **SSS004**: two or more `if`/`else if` branches with conditions of the shape `<sameScrutinee> is <SameType> { <SameProperty>: ... }` should be a single `switch`. The `switch` form fuses isinst + ldfld; the if-chain repeats both per arm.
84+
- **SSS005**: a `switch` (expr or statement) whose arms are all single string/numeric compile-time constants must list them sorted — strings ordinal, numbers numerically (`_`/`null` excluded). Exempts any arm that's a guard, or/relational/recursive/`var` pattern, or an **enum/`char`/`bool`** constant (those order by meaning, not value). A switch deliberately ordered by meaning (time-unit magnitude, host level, …): wrap in `#pragma warning disable SSS005` + one-line rationale rather than sorting.
85+
- **SSS006**: 2+ consecutive statements that each call a self-returning `StringBuilder` method (containing-type and return-type both `StringBuilder``Append`/`Insert`/`Replace`/… ) on the same builder and discard the result (bare or `_ =`) should be one fluent chain. An already-chained statement is peeled to its base receiver, so `sb.Append(a).Append(b)` beside `sb.Append(c)` merges. Only fires when that base is side-effect-free (identifier / `this.field` / dotted); call-valued roots exempt. Comments between the statements don't exempt — slot them between the chained calls.
8486
- **MSTEST0049**: async tests must thread `TestContext.CancellationToken`. Pattern: `public TestContext TestContext { get; set; } = null!;`.
8587
- **MSTEST0037**: prefer `Assert.IsEmpty(values)` over `Assert.AreEqual(0, values.Count)`; typed asserts over generic.
8688

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
using Microsoft.CodeAnalysis.CSharp.Testing;
2+
using Microsoft.CodeAnalysis.Testing;
3+
4+
namespace SqlServerSimulator.Analyzers;
5+
6+
[TestClass]
7+
public sealed class ChainableStringBuilderCallsAnalyzerTests
8+
{
9+
public TestContext TestContext { get; set; } = null!;
10+
11+
private Task RunAsync(string source) =>
12+
new CSharpAnalyzerTest<ChainableStringBuilderCallsAnalyzer, DefaultVerifier>
13+
{ TestCode = source }.RunAsync(this.TestContext.CancellationToken);
14+
15+
private const string Preamble = "using System.Text;\n";
16+
17+
[TestMethod]
18+
public Task TwoDiscardedAppends_Reports() =>
19+
RunAsync(Preamble + """
20+
internal sealed class C
21+
{
22+
void M(StringBuilder sb)
23+
{
24+
{|SSS006:_ = sb.Append("a");|}
25+
_ = sb.Append("b");
26+
}
27+
}
28+
""");
29+
30+
[TestMethod]
31+
public Task ThreeAppends_ExampleShape_Reports() =>
32+
RunAsync(Preamble + """
33+
internal sealed class C
34+
{
35+
void M(StringBuilder sb, byte[] bytes)
36+
{
37+
{|SSS006:_ = sb.Append('"');|}
38+
_ = sb.Append(System.Convert.ToBase64String(bytes));
39+
_ = sb.Append('"');
40+
}
41+
}
42+
""");
43+
44+
[TestMethod]
45+
public Task TwoBareAppends_Reports() =>
46+
RunAsync(Preamble + """
47+
internal sealed class C
48+
{
49+
void M(StringBuilder sb)
50+
{
51+
{|SSS006:sb.Append("a");|}
52+
sb.Append("b");
53+
}
54+
}
55+
""");
56+
57+
[TestMethod]
58+
public Task AppendLineAndInsert_SelfReturningMethods_Report() =>
59+
RunAsync(Preamble + """
60+
internal sealed class C
61+
{
62+
void M(StringBuilder sb)
63+
{
64+
{|SSS006:_ = sb.AppendLine("a");|}
65+
_ = sb.Insert(0, "b");
66+
}
67+
}
68+
""");
69+
70+
[TestMethod]
71+
public Task FieldReceiver_Reports() =>
72+
RunAsync(Preamble + """
73+
internal sealed class C
74+
{
75+
private readonly StringBuilder sb = new();
76+
void M()
77+
{
78+
{|SSS006:_ = this.sb.Append("a");|}
79+
_ = this.sb.Append("b");
80+
}
81+
}
82+
""");
83+
84+
[TestMethod]
85+
public Task CommentBetween_StillReports() =>
86+
RunAsync(Preamble + """
87+
internal sealed class C
88+
{
89+
void M(StringBuilder sb)
90+
{
91+
{|SSS006:_ = sb.Append("a");|}
92+
// a documented step
93+
_ = sb.Append("b");
94+
}
95+
}
96+
""");
97+
98+
[TestMethod]
99+
public Task SingleAppend_DoesNotReport() =>
100+
RunAsync(Preamble + """
101+
internal sealed class C
102+
{
103+
void M(StringBuilder sb) => sb.Append("a");
104+
}
105+
""");
106+
107+
[TestMethod]
108+
public Task DifferentBuildersInterleaved_DoesNotReport() =>
109+
RunAsync(Preamble + """
110+
internal sealed class C
111+
{
112+
void M(StringBuilder sb, StringBuilder other)
113+
{
114+
_ = sb.Append("a");
115+
_ = other.Append("b");
116+
_ = sb.Append("c");
117+
}
118+
}
119+
""");
120+
121+
[TestMethod]
122+
public Task NonBuilderStatementBetween_DoesNotReport() =>
123+
RunAsync(Preamble + """
124+
internal sealed class C
125+
{
126+
void M(StringBuilder sb)
127+
{
128+
_ = sb.Append("a");
129+
System.Console.WriteLine();
130+
_ = sb.Append("b");
131+
}
132+
}
133+
""");
134+
135+
[TestMethod]
136+
public Task MethodCallReceiver_NotSimple_DoesNotReport() =>
137+
RunAsync(Preamble + """
138+
internal sealed class C
139+
{
140+
StringBuilder Get() => new();
141+
void M()
142+
{
143+
_ = Get().Append("a");
144+
_ = Get().Append("b");
145+
}
146+
}
147+
""");
148+
149+
[TestMethod]
150+
public Task CapturedReturnThenAppend_DoesNotReport() =>
151+
RunAsync(Preamble + """
152+
internal sealed class C
153+
{
154+
void M(StringBuilder sb)
155+
{
156+
var captured = sb.Append("a");
157+
_ = sb.Append("b");
158+
}
159+
}
160+
""");
161+
162+
[TestMethod]
163+
public Task BareAppendAfterExistingChain_Reports() =>
164+
RunAsync(Preamble + """
165+
internal sealed class C
166+
{
167+
void M(StringBuilder sb)
168+
{
169+
{|SSS006:_ = sb.Append("a").Append("b");|}
170+
_ = sb.Append("c");
171+
}
172+
}
173+
""");
174+
175+
[TestMethod]
176+
public Task LoneExistingChain_DoesNotReport() =>
177+
RunAsync(Preamble + """
178+
internal sealed class C
179+
{
180+
void M(StringBuilder sb)
181+
{
182+
_ = sb.Append("a").Append("b");
183+
}
184+
}
185+
""");
186+
187+
[TestMethod]
188+
public Task ChainsOnCallValuedRoot_DoesNotReport() =>
189+
RunAsync(Preamble + """
190+
internal sealed class C
191+
{
192+
StringBuilder Get() => new();
193+
void M()
194+
{
195+
_ = Get().Append("a").Append("b");
196+
_ = Get().Append("c");
197+
}
198+
}
199+
""");
200+
201+
[TestMethod]
202+
public Task RunFollowedByToString_ReportsOnlyTheAppendRun() =>
203+
RunAsync(Preamble + """
204+
internal sealed class C
205+
{
206+
string M(StringBuilder sb)
207+
{
208+
{|SSS006:_ = sb.Append("a");|}
209+
_ = sb.Append("b");
210+
return sb.ToString();
211+
}
212+
}
213+
""");
214+
}

0 commit comments

Comments
 (0)