Skip to content

Commit 3dfec88

Browse files
committed
Add SSS004 analyzer: flag if/else-if and if-{return/throw} chains with repeated '<sameScrutinee> is <SameType> { <SameProperty>: ... }' conditions, which collapse to a single switch with one isinst + one property read instead of N of each; refactors two ASC/DESC chains in Selection / WindowExpression that the rule found.
1 parent 6fd3f63 commit 3dfec88

5 files changed

Lines changed: 690 additions & 14 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ The number lands in `Data["HelpLink.EvtID"]` for tests to assert. When adding er
118118
- **SSS001** (custom analyzer): non-public types may not have auto-properties or trivial wrappers over same-type fields. Expose the field directly: `public readonly T Foo = expr;`. Overrides, abstracts, statics, and interface implementations (both explicit and implicit — a property whose name and signature satisfy a member of an implemented interface) are exempt; the interface contract dictates the property shape. Lives in `SqlServerSimulator.Analyzers/`.
119119
- **SSS002** (custom analyzer): a `readonly` field in a non-public-API type whose declared type is a strict supertype of its immediately-assigned initializer should be declared as the concrete type. Same-assembly callers gain no API-stability benefit from the abstraction; the concrete declaration exposes more members directly and avoids virtual dispatch. Public types are exempt; value-typed initializers (boxing) are exempt; const fields and fields without initializers don't apply. After applying the rule, switch / conditional expressions that previously inferred the (now-shed) base type may need an explicit base-type annotation — extract to a helper method with an explicit return type (`SqlType.ResolveSimpleKeyword`), declare the destination variable explicitly (`SqlType resultType = ... ? ...`), or cast a `null` arm.
120120
- **SSS003** (custom analyzer): `string.ToUpperInvariant()` / `string.ToLowerInvariant()` whose result is the *governing expression* of a `switch` statement or `switch` expression allocates a temporary string only to throw it away after dispatch. Use the `Span<char>` overload — `Span<char> buf = stackalloc char[s.Length]; s.AsSpan().ToUpperInvariant(buf)` — and switch on the resulting count (which lets the parser dispatch by length first; matches the established `Parser/Expression.cs:ResolveBuiltIn` and `Storage/SqlType.cs:GetByName` pattern). The rule is intentionally narrow: only the switch-governing case is flagged. Allocating uses where the upper/lower string itself is the function's returned value (e.g. SQL `UPPER`, GUID-to-string casts) don't trip — their result isn't a switch governing expression.
121+
- **SSS004** (custom analyzer): two or more `if` / `else if` branches (or a run of consecutive `if (cond) { return/throw; }` early-exit statements) whose conditions all have the shape `<sameScrutinee> is <SameType> { <SameProperty>: ... }` should be a single `switch`. The C# compiler emits one `isinst` and one property/field read per arm in the `if`-chain form; the equivalent `switch` fuses both — one `isinst` + one `ldfld` for the whole dispatch, with each arm reduced to a `ldloc` + `beq`/`bne` against the cached discriminant (verified via `ilspycmd -il`). Token-dispatch hot paths in this codebase use this pattern pervasively. The rule restricts the scrutinee to syntactically simple chains (locals, parameters, `this`, dotted member access) to avoid silently changing semantics for side-effecting expressions; method calls / indexers / casts in the scrutinee position skip the diagnostic. Property-pattern designations (`is T { P: x } v`) and multiple subpatterns (`is T { P: a, Q: b }`) are also skipped — the rewrite isn't mechanical for those.
121122
- **MSTEST0049**: async tests must thread `TestContext.CancellationToken`. Pattern: `public TestContext TestContext { get; set; } = null!;` plus a helper that uses `this.TestContext.CancellationToken`.
122123
- **MSTEST0037**: prefer `Assert.IsEmpty(values)` over `Assert.AreEqual(0, values.Count)`; use typed asserts over generic `Assert.AreEqual` on known types.
123124
- **AssemblyHooks**: each test project has `AssemblyHooks.cs` with `static [TestClass]` hosting `[AssemblyInitialize]`. The analyzer-tests warm-up specifically prevents Roslyn cache contention under parallel execution (~3x slowdown without it).

0 commit comments

Comments
 (0)