microsoft / microsoft/microsoft-ui-reactor

Gallery snippet lint doesn't catch no-op handlers in a snippet whose live card wires real ones

Open
#1,028 2 comments 0 reactions 0 assignees View on GitHub
bug samples
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.