microsoft / microsoft/microsoft-ui-reactor
[Test-gate] ReadCleanElementCommonBlock matches over comments — false-green in one consumer, false-red in the other
- Dominant language
- C#
- Stars
- 646
- Forks
- 54
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 84
Description
## Summary
`ReadCleanElementCommonBlock` returns the raw source text of `CleanElement`'s FrameworkElement-common block with **comments intact**, and both consumers pattern-match over it. A comment containing an example call therefore participates in the gate as if it were code.
**Currently latent — 0 occurrences today — but there are 43 comment lines inside the block, and the block's comments are actively being reworded.**
```
OBSERVED_AT 2026-08-01T12:36:10Z main = 1ed2644e
src/Reactor/Core/ElementPool.cs — CleanElement FE-common block: 118 lines, 7671 chars
comment lines in block: 43
```
## The mechanism
`tests/Reactor.Tests/AnalyzerTests/ModifierUnsetClearValueTests.cs:509-534`
```csharp
private static string ReadCleanElementCommonBlock(out string paramName)
{
var source = File.ReadAllText(file);
...
return source.Substring(braceStart, switchStart.Index); // raw text — no comment stripping
}
```
The method already anchors the `switch` boundary to line-start specifically so *"a comment mentioning the type dispatch cannot masquerade as the boundary"* (`:524-526`) — **so comment interference was anticipated for the boundary, but not for the body.**
## The interesting part: one root cause, opposite polarity in the two consumers
| consumer | pattern | a comment adds… | effect |
|---|---|---|---|
| `ModifierUnsetClearValueTests.cs:366` | `\b\w+\.ClearValue\(…Property\)` → **satisfied-pin set** | a phantom **pin** | **FALSE-GREEN** |
| `ModifierUnsetClearValueTests.cs:176` | `\bfe\.(\w+)\s*=[^=]` → **offender set** | a phantom **offender** | **FALSE-RED** |
```
measured on main:
:366 ClearValue( in code = 57 in comment = 0
:176 fe.X = in code = 1 in comment = 0
```
**The consequences are not symmetric:**
- **False-green** means a `ClearValue` call can be *removed from the code* and the pin still satisfied by a comment that mentions it — **precisely the failure this gate exists to prevent.**
- **False-red** means a spurious offender — loud, immediate, self-correcting.
> **The loud direction is the one that will get reported first, and fixing it in isolation — without noticing both consumers share `ReadCleanElementCommonBlock` — leaves the dangerous half live.**
## Suggested fix
Strip comments in `ReadCleanElementCommonBlock` before returning, so both consumers inherit it:
```csharp
var block = source.Substring(braceStart, switchStart.Index);
block = Regex.Replace(block, @"/\*.*?\*/", string.Empty, RegexOptions.Singleline);
block = Regex.Replace(block, @"//[^\n]*", string.Empty);
return block;
```
`PoolResetSetConsistencyTests` has a sibling `ReadCleanElementCommonBlock` and needs the same treatment.
## Non-vacuous test for the fix
Assert that a comment mentioning a clear does **not** satisfy its pin — i.e. feed the reader a block containing only `// fe.ClearValue(FrameworkElement.MarginProperty)` and assert the extracted set is **empty**. That assertion fails if the strip is removed, and cannot be satisfied by the shape of the gate alone.
**Positive control required:** the same test must show a real `fe.ClearValue(FrameworkElement.MarginProperty);` **does** register, otherwise a stripper that deletes everything would pass.
---
Found by the #1015 session during merge rehearsal; verified independently here. **Not blocking #1015** — the defect predates it, is latent today, and the PR is approved and green.
Contributor guide
Research direction
Start with ReadCleanElementCommonBlock in tests/Reactor.Tests/AnalyzerTests/ModifierUnsetClearValueTests.cs:509-534, then inspect both consumers at lines 176 and 366. Check the sibling reader in PoolResetSetConsistencyTests as well. Done means comment text no longer affects either extracted set, while real ClearValue calls still register; add the stated negative and positive regression coverage and run the relevant analyzer tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100