T-SQL convention guard labels offenders with a type name, a local variable, or a bare keyword instead of the enclosing member #3094

TsqlConventionGuardTests.EnclosingMember builds the member half of every offender string the T-SQL convention guard prints:

offenders.Add($"{name}:{EnclosingMember(text, start)} [{rule}] {detail}");

It takes "the nearest declaration above the literal" from a two-alternative regex over raw source:

(?:const\s+string|static\s+string|string)\s+(?<name>[A-Za-z_][A-Za-z0-9_]*)\s*(?:=|=>)
|(?<name2>[A-Za-z_][A-Za-z0-9_]*)\s*\([^)]*\)\s*(?:=>|\{)

The second alternative asks for an identifier, a parenthesised anything, and an opening brace — which is a description of most C# statements, not of a member declaration. There is no containment test anywhere: whatever matched last before the offset wins, whether or not the literal is inside it.

Measured, over the corpus the guard actually scans

Driving the shipped helper at the offset of every T-SQL literal the scan reads (160 literals across the three scanned trees):

48 of 160 are labelled with a bare C# statement keyword or with nothing at all — 29 if, 11 catch, 7 foreach, and 1 <unknown>. A further 3 are labelled SqlConnectionStringBuilder, which is a type being constructed and not a member of the file at all, and 7 more with a local variable (query x3, sql x2, optimizeForSequentialKey, replicaGroupKey).

The five sites worth naming individually, each reproduced before anything was changed:

Two smaller notes on the table above. IndexObjectStatsCollector.cs:437 also reports if (BuildEnumerationQuery), and .FinOps.Recommendations.cs reports if at two more sites and catch at two others — all five of those are GetRecommendationsAsync. So the file-level picture is worse than any single row: LocalDataService.FinOps.Recommendations.cs has seven T-SQL literals and labels four of them with a keyword.

Detection is unaffected. That is the point, not a mitigation

The same literals are read, the same rules fire, and the offender count is identical — the guard finds zero violations on the tree today either way. Nothing about what this guard detects is wrong.

But the offender list is the guard's entire output at the moment it reds. When #3078's COUNT(*) lands again, what a reader gets is a filename and a member name, and today that member name is a type, a local, or the word if. A reader has no way to tell a correct label from a wrong one — so the wrong one costs more than no label at all, because it is trusted. A guard that detects correctly and then misdirects sends the reader to look for a member that does not exist in the file, and the natural next conclusion is that the guard is broken rather than that the SQL is.

There is also a second-order cost: if appears 29 times as a member name. Two different violations in two different files can produce offender lines that differ only by filename, so the list stops being usable as a work queue at exactly the moment it has more than one entry.

Two things the fix has to respect

• The helper is shared by every rule the guard emits, so this is one change to all six covered rules' messages at once.
• CSharpSourceWalker is in the same directory and provides the code mask (CodeMask / StripCommentsAndStrings) that The shared source-walker blanks interpolated-string holes, so a call inside an interpolation is invisible to every scan built on it #2913 extracted precisely so that five copies of a hand-rolled comment/string masker could stop being five. The current regex reads raw source, so it can match a modifier inside a comment or an identifier inside a string literal. The member lookup should be built on the walker rather than on raw text — and not on a new hand-rolled mask: the two failure modes cited in that class's doc comment (a block-comment continuation with no leading asterisk, which is this repo's style, and a /* inside a string literal read as a real opener) are exactly the two that got hand-rolled wrong in opposite directions by separate parties on one day.

Shape of the fix

#3089 solved the same problem for PgPanelTabOwnershipTests hours ago — resolving which member an offset belongs to, derived from MethodDeclaration match offsets over walked source, with a next-declaration bound so a range cannot silently extend past its own member, plus a brace-balance arm for the end of the file where that bound is structurally blind. That is the design to reuse rather than reinvent, including its two findings: a re-check that reads the same walked text and repeats the same walk is a tautology, and a detector that re-implements the rule's predicate inline cannot fail.

The pin needs to be a fixture table of (file, literal, expected member) drawn from the real tree and red-proofed by reverting the fix — and located by a distinctive anchor substring rather than a literal offset. All five files above are live source that changes most weeks; a pinned integer would go red for a reason unrelated to attribution, and the next reader would repair it by updating the number, which silently retargets the pin at whatever literal now sits at that offset.