microsoft / microsoft/microsoft-ui-reactor
Gallery snippet lint doesn't catch no-op handlers in a snippet whose live card wires real ones
- Dominant language
- C#
- Stars
- 646
- Forks
- 54
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 84
Description
## Summary
No gate in the repo detects a `SampleCard` snippet that shows **no-op handlers** (`() => {}`) or omits a readout, while the **live card** next to it wires real handlers and renders one. Copy-pasting such a snippet yields a demo where clicking does nothing — the opposite of what the card demonstrates on screen.
This is measured, not inferred. Both directions below were run on a clean tree at `99fc4416`.
## Measurement
**Probe 1 — defect in a *second* card's snippet (not sampled by the search index):**
Reverted `MenuFlyoutPage.cs` card 2's snippet to three `() => {}` no-ops while the live card kept its three real `setFormatAction(...)` handlers and its `TextBlock($"Last action: {formatAction}")` readout.
```
dotnet test tests/Reactor.Tests --filter "FullyQualifiedName~Tooling"
Passed! - Failed: 0, Passed: 139, Total: 139
```
**Nothing fired.** `GallerySnippetAgreementTests`, `GallerySampleLintTests`, and `GalleryCardIndependenceTests` are all green on a snippet that contradicts the card beside it.
**Probe 2 — same defect in a *first* card's snippet:**
```
Microsoft.UI.Reactor.Tests.Tooling.SearchIndexGeneratorTests.Index_IsUpToDate [FAIL]
Failed! - Failed: 1, Passed: 138, Total: 139
```
Only the **index** gate fired — and it is *not* detecting the defect. It fired because the indexed text changed at all, and it is satisfied by regenerating, which simply **bakes the defective snippet into the index**. The snippet-agreement gate stayed green in both probes.
**Positive control** (so the above isn't a dead-suite artifact): flipping one byte at offset 100 of the 86,246-byte index makes `Index_IsUpToDate` fail with *"stale (content or BOM/encoding)"*. The suite can still convict.
## Why the existing gate misses it
`GallerySnippetAgreementTests` checks snippet → live in **one direction**: identifiers appearing in the snippet must be plausible against the live sample. A snippet that *omits* or *hollows out* something the live card has is invisible to it, because it never asks the live→snippet question. Two related consequences already observed:
- **Agreement ≠ binding.** `CommandBarPage.cs` had a snippet calling `setLastAction(...)` with no declaring `UseState` — a free identifier the "self-contained and compilable" rule is meant to reject. It shipped green.
- **Abridgement is invisible.** `MenuBarPage.cs`'s first snippet omitted `Exit`, `Redo`, `Paste` and the entire `View` menu (filed as #1024, fixed in #1005).
## Remaining instances
Five snippets still carry no-op handlers, in pages outside #1005's scope:
| file | line |
|---|---|
| `ControlPages/Collections/RefreshContainerPage.cs` | 48 |
| `ControlPages/Collections/SwipeControlPage.cs` | 27 |
| `ControlPages/DesignGuidance/GeometryPage.cs` | 156 |
| `ControlPages/DesignGuidance/GeometryPage.cs` | 314 |
| `ControlPages/Media/AnimatedIconPage.cs` | 39 |
The four in `MenuBarPage`, `CommandBarPage`, `MenuFlyoutPage` and `CommandBarFlyoutPage` are fixed in #1005.
Note that a no-op handler is not *always* wrong — a snippet illustrating pure structure may legitimately use `() => {}`. The rule wants to be "the snippet's handlers are no-ops **while the live card's are not**", not a blanket ban.
## Suggested fix
Extend the snippet lint with a live→snippet direction: for each `SampleCard`, if the live body invokes a state setter inside a handler lambda, the snippet must too. Enabling it will redden the five pages above, so they should be fixed in the same PR.
`AnimatedIconPage.cs` is currently being edited by #1009 — worth sequencing after it lands to avoid a conflict.
/cc #980 #1024
Contributor guide
Research direction
Start with GallerySnippetAgreementTests, GallerySampleLintTests, and the SampleCard linting path, then reproduce the two probes with the Tooling test filter. Add the live-to-snippet handler check described in the issue and update the five listed page locations it exposes, while preserving valid pure-structure no-op examples. Done means the Tooling tests catch the mismatch and pass after those snippets are corrected.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- testing, tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100