diff --git a/CHANGELOG.md b/CHANGELOG.md index 99ad437fa..49b9ed7b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,18 @@ Conventions for contributors: ### Added +- **`REACTOR_DYM_006`: color strings that render gray** (issue #1375, spec 061 §7.3). A new + default-on **Warning** flags compile-time-constant color strings that `BrushHelper` can't + parse and therefore silently fall back to gray: `.Background("orange")`, `.Foreground("#888")`, + or a font name passed to `ResourceBuilder.Set(key, string)` under a `*FontFamily` key. It covers + `BrushHelper.Parse`, `ResourceBuilder.Set(string, string)` and the `string` overloads of + `Background` / `Foreground` / `BorderBrush` / `WithBorder`, suggests the exact hex (or + `new FontFamily(...)` once that `Set` overload exists), and ships a code fix; `mur check`'s new + `ColorStringLiteralRule` repeats the rewrite as a `→ try:` hint. Unlike `REACTOR_DYM_001`–`005` + it fires on code that compiles, so a project with `TreatWarningsAsErrors` can stop building on + an existing gray color. Fix the value, or adjust the severity with + `dotnet_diagnostic.REACTOR_DYM_006.severity` in `.editorconfig`. + - **Inspector diagnostics: call-site column and a per-component render event** (issue #1326, spec 010 §1.1): - `SourceLocation.ColumnNumber` (1-based, `0` = unknown), a third positional diff --git a/docs/_pipeline/templates/analyzer-architecture.md.dt b/docs/_pipeline/templates/analyzer-architecture.md.dt index a58fe08c2..09ba2e011 100644 --- a/docs/_pipeline/templates/analyzer-architecture.md.dt +++ b/docs/_pipeline/templates/analyzer-architecture.md.dt @@ -179,6 +179,7 @@ sibling `src/Reactor.Analyzers.Internal/` project instead; see | `REACTOR_DYM_003` | Warning | Mistyped Reactor factory name in call position (e.g. `Buton(...)`) | `FuzzyFactoryNameAnalyzer.cs` | | `REACTOR_DYM_004` | Warning | Reactor factory called with too few arguments (CS7036), single unique overload — suggests the parameter shape | `MissingFactoryArgumentAnalyzer.cs` | | `REACTOR_DYM_005` | Warning | String passed where a Reactor `Element` is expected (CS1503) — wrap it in a text factory | `StringForElementArgumentAnalyzer.cs` | +| `REACTOR_DYM_006` | Warning | Constant color string `BrushHelper` can't parse, so it renders gray (e.g. `.Background("orange")`, `.Foreground("#888")`, a font name under a `*FontFamily` resource key) — suggests the hex value (or `new FontFamily(...)` once that `ResourceBuilder.Set` overload exists) | `ColorStringLiteralAnalyzer.cs` | `REACTOR_HOOKS_002` and `_003` were the reserved control-flow / data-flow slots (variable hook counts across early returns, async boundaries inside `UseEffect`); diff --git a/docs/guide/analyzer-architecture.md b/docs/guide/analyzer-architecture.md index 073fb2cf3..17d4a8e8a 100644 --- a/docs/guide/analyzer-architecture.md +++ b/docs/guide/analyzer-architecture.md @@ -205,6 +205,7 @@ sibling `src/Reactor.Analyzers.Internal/` project instead; see | `REACTOR_DYM_003` | Warning | Mistyped Reactor factory name in call position (e.g. `Buton(...)`) | `FuzzyFactoryNameAnalyzer.cs` | | `REACTOR_DYM_004` | Warning | Reactor factory called with too few arguments (CS7036), single unique overload — suggests the parameter shape | `MissingFactoryArgumentAnalyzer.cs` | | `REACTOR_DYM_005` | Warning | String passed where a Reactor `Element` is expected (CS1503) — wrap it in a text factory | `StringForElementArgumentAnalyzer.cs` | +| `REACTOR_DYM_006` | Warning | Constant color string `BrushHelper` can't parse, so it renders gray (e.g. `.Background("orange")`, `.Foreground("#888")`, a font name under a `*FontFamily` resource key) — suggests the hex value (or `new FontFamily(...)` once that `ResourceBuilder.Set` overload exists) | `ColorStringLiteralAnalyzer.cs` | `REACTOR_HOOKS_002` and `_003` were the reserved control-flow / data-flow slots (variable hook counts across early returns, async boundaries inside `UseEffect`); diff --git a/docs/reference/mur-check-did-you-mean.md b/docs/reference/mur-check-did-you-mean.md index 51052ef99..8b060bed2 100644 --- a/docs/reference/mur-check-did-you-mean.md +++ b/docs/reference/mur-check-did-you-mean.md @@ -109,8 +109,8 @@ mur check │ fuzzy-match against the real Reactor API surface. │ │ Tier 3 — induced + vocabulary rules (SHIPPED in part, Phase 3) - │ Small symbol-bound rewriters. Six rules live today - │ (four Class-A induced + two Class-B vocabulary). + │ Small symbol-bound rewriters. Seven rules live today + │ (4 Class-A + 2 Class-B + 1 analyzer mirror). │ Rules can cover diagnostic codes Tier-2 doesn't │ (e.g. CS1955). Rules WIN over Tier-2 when both match. │ @@ -168,7 +168,7 @@ Each rule is a small file under `src/Reactor.Cli/Check/Rules/`, registered into Each rule passes a six-bar **Validation Gate** before merge (frequency, cross-agent reproducibility, positive fixtures, negative counter-examples, independent reviewer signoff, kill-switch). The gate exists because a bad rule is worse than no rule — it contaminates every downstream agent session. Class-B (vocabulary-translation) rules waive bar #1 (frequency) — their justification is the documented prior-framework citation rather than a corpus cluster. The audit at `docs/specs/tasks/038-tuning-reports/2026-05-11-cross-agent-audit.md` is where bar #2 (cross-agent) gets formally cleared per cluster. -**Six rules shipped to date** (four Class-A induced + two Class-B vocabulary): +**Seven rules shipped to date** (four Class-A induced + two Class-B vocabulary + one analyzer mirror): | Rule | Class | Code(s) | Receiver | What it suggests | Cross-agent events | |---|---|---|---|---|---| @@ -178,6 +178,7 @@ Each rule passes a six-bar **Validation Gate** before merge (frequency, cross-ag | `GridSizePxRenameRule` | A | CS0117 | `GridSize` | `GridSize.Px(...)` for `Pixel`/`Pixels`/`Fixed` legacy names | 9 | | `TextBlockStyleHintRule` | A | CS1061 / CS0117 | `TextBlockElement` | fluent text helpers (`.FontSize`, `.SemiBold`, …); Reactor has no `Style` member | 5 across both `.Style(...)` and `with { Style = ... }` shapes | | `ThemeRawResourceKeyRule` | A | CS0117 | `Theme` | real `Theme.*` aliases (`Theme.LayerFill`, `Theme.Accent`, …) for WinUI `FillColor` ThemeResource key names | run5 opus-4.7 only; bar #2 partial | +| `ColorStringLiteralRule` | analyzer mirror | `REACTOR_DYM_006` | `BrushHelper` / `ResourceBuilder` / `ElementExtensions` | the hex for an unparseable constant color string (`"orange"` → `"#FFA500"`, `"#888"` → `"#888888"`), or `new FontFamily("…")` under a `*FontFamily` resource key when that `ResourceBuilder.Set` overload exists. **First rule anchored on an analyzer ID**: the mistake compiles, so there is no CS code to hang it on (issue #1375); the rewrite is read from the analyzer's message | n/a — mirrors the in-build analyzer | ### Suggest-gate carve-out for Tier-3 rules @@ -503,7 +504,7 @@ Branch: `eval/spec-038-ec3-2026-05-11` at commit `2b7090f`. Six rules + the two - `RuleSymbolResolver.cs` — `ResolveType(string)`, `ResolveMethod(INamedTypeSymbol, string)`, `ResolveMember(INamedTypeSymbol, string)`. Cached via `ConditionalWeakTable` so two callers with the same compilation reference share caches; per-compilation resolver identity is test-locked. - CLI: `--disable-rule ` (repeatable), `--list-rules` (prints name/provenance/status table; short-circuits before `dotnet build` runs). Unknown `--disable-rule` names warn instead of error. -**The six rules** — see the table in §3 for the full inventory. Four Class-A (induced from the cross-corpus audit) + two Class-B (vocabulary-translation, structurally justified). All six bind through `RuleSymbolResolver`; no string-matching. +**The six rules** — see the table in §3 for the full inventory. Four Class-A (induced from the cross-corpus audit) + two Class-B (vocabulary-translation, structurally justified). All six bind through `RuleSymbolResolver`; no string-matching. A seventh, `ColorStringLiteralRule` (issue #1375), landed later on `main`: it mirrors the in-build `REACTOR_DYM_006` analyzer rather than a corpus cluster, anchors on that analyzer ID, and reads its rewrite from the analyzer's message. **Two critical correctness fixes** (both blocked any real-world rule firing prior; surfaced by end-to-end smoke testing during Phase 3): diff --git a/docs/specs/061-in-build-did-you-mean.md b/docs/specs/061-in-build-did-you-mean.md index 9a0254676..b2021bd0b 100644 --- a/docs/specs/061-in-build-did-you-mean.md +++ b/docs/specs/061-in-build-did-you-mean.md @@ -5,8 +5,9 @@ **Implemented (in part) — 2026-07-07.** Phase 1 (`REACTOR_DYM_001`) and Phase 2 (`REACTOR_DYM_002`) have shipped, along with the first fuzzy analyzer (`REACTOR_DYM_003`, the CS0103 mistyped-factory-name case) and the argument-shape analyzers (`REACTOR_DYM_004`, CS7036 missing -argument; `REACTOR_DYM_005`, the CS1503 string-for-Element case — see §7.2); the remaining fuzzy -member-resolution phases and the shared match-engine extraction remain design-only. This spec +argument; `REACTOR_DYM_005`, the CS1503 string-for-Element case — see §7.2), and the first did-you-mean +that rides **no** compiler error (`REACTOR_DYM_006`, unparseable color-string literals — see §7.3); the +remaining fuzzy member-resolution phases and the shared match-engine extraction remain design-only. This spec is the design of record for bringing Reactor's "did you mean" suggestions — today reachable only through the `mur check` CLI — into a plain `dotnet build` and the IDE, for consumers of the `Microsoft.UI.Reactor` NuGet package. @@ -321,14 +322,72 @@ produces uncompilable code is worse than none, so neither ships one. and the IDE. Tunable via `.editorconfig`. The shared gating lives in one `ArgumentShapeGate` helper so the two analyzers can't drift. +### §7.3 `REACTOR_DYM_006` — unparseable color-string literal (shipped) + +Issue #1375. Every Reactor API that takes a color *string* goes through `BrushHelper.ParseColor`, which +knows eight names (`red`, `green`, `blue`, `white`, `black`, `gray`/`grey`, `lightgray`/`lightgrey`, +`transparent`) plus `#RRGGBB` / `#AARRGGBB`. Anything else silently becomes a **gray** brush. So +`.Background("orange")`, `.Foreground("#888")`, and — once `ResourceBuilder.Set(string, FontFamily)` exists +(PR #1370) — `.Resources(r => r.Set("ContentControlThemeFontFamily", "Consolas"))` all compile and render +wrong. `ColorStringLiteralAnalyzer` + `ColorStringLiteralCodeFix` catch the constant cases. + +**The first DYM that rides no compiler error.** DYM_001–005 always co-occur with a CS error, which is what +made Warning safe under `TreatWarningsAsErrors`. This one fires on code that compiles. It is still a +**Warning**: the result is always a wrong brush, never a style preference (the same reasoning as +`REACTOR_MOD_003`). In-repo hits were fixed in the PR that introduced it rather than suppressed. + +- **Targets (symbol identity, resolved once per compilation).** `BrushHelper.Parse(string)`, + `ResourceBuilder.Set(string key, string color)`, and the `string color` overloads of `Background` / + `Foreground` / `BorderBrush` / `WithBorder` on `ElementExtensions` (generic and RichText). A look-alike API + in another namespace never matches. `D3Charts.Brush(string)` is out of scope: it parses with `D3Color`. +- **Values.** Only compile-time constants: literals, `const`s, constant concatenation, and each constant + branch of a `?:` / `??` / switch expression. A wholly-constant expression is checked as its single value, + and the walk also prunes branches the compiler doesn't fold but a constant rules out (a `const bool` + condition, a constant non-null `??` left side, a constant-valued switch over constant patterns), so a dead + branch is never reported. Runtime strings are out of reach; a runtime throw was considered and + left as a separate decision (it would change behaviour for apps relying on the fallback). +- **Grammar parity.** The analyzer can't reference `src/Reactor`, so `ColorStringClassifier` carries a + netstandard2.0 copy of the grammar, including its quirks (`TrimStart('#')` accepts `##FF0000`; + `NumberStyles.HexNumber` tolerates whitespace inside a byte pair). `BrushHelper` exposes an internal + `TryParseColor` (no behaviour change) so `ColorStringParityTests` can compare exactly — a plain + "result is gray" oracle can't tell an unparseable string from a legitimate `"gray"`. +- **Did-you-mean, in priority order.** (1) a `ResourceBuilder.Set` key ending in `FontFamily` suggests + `new FontFamily("…")`, but **only when** `Set(string, FontFamily)` is present in the compilation and the + value is the whole argument (it can't replace one branch of a `?:`); otherwise the message says the overload + only stores color brushes, points at the `.FontFamily("…")` element modifier, and offers no fix. (2) `#RGB` / `#ARGB` + shorthand → expanded `#RRGGBB` / `#AARRGGBB`. (3) six or eight hex digits without `#` → add it. (4) other + `#…` → "must be `#RRGGBB` or `#AARRGGBB`". (5) a standard CSS / WinUI named color (139 entries, including + the `grey` spellings) → its exact hex, cross-checked against `System.Drawing.KnownColor`. (6) anything else + → the supported list, hex, or a `Theme` token. +- **Fix.** Cases 1–3 and 5 carry the replacement in the property bag. Literal fixes rewrite the string + literal (including one inside a `?:` branch); the FontFamily fix wraps the whole argument (a literal or a + `const` reference) in a `Simplifier`-annotated `new FontFamily(...)`, renaming a named `color:` argument to + the FontFamily overload's parameter so the call still binds. + +**`mur check` mirror.** `ColorStringLiteralRule` is the first Tier-3 rule anchored on an **analyzer** ID +rather than a CS code — there is no compiler diagnostic to augment. `mur check` wraps `dotnet build`, which +runs the analyzer bundle, so `REACTOR_DYM_006` appears in the parsed output. Every fixable message ends in +`did you mean X?`, computed for the exact value the diagnostic points at, so the rule lifts `X` out of the +message rather than re-deriving it. It has no grammar of its own and can't disagree with the code fix, and +the `→ try:` suffix keeps the rewrite visible after `mur check` truncates the message. A `*FontFamily` +key with no rewrite surfaces the message's `.FontFamily("…")` modifier advice the same way; messages with no +concrete next step (unknown name, malformed hex) stay silent. There is deliberately no Tier-1 `HintFor` entry, which would mask the suggestion. +`DidYouMeanAnalyzerParityTests` feeds real analyzer diagnostics to the rule and asserts it suggests the +code fix's rewrite; `ColorStringParityTests` pins the `did you mean` suffix. + +**Overlap with `REACTOR_THEME_001`.** THEME_001 already flags every string literal passed to `.Background` / +`.Foreground` / `.WithBorder` as "use a Theme token". That is a style nudge (and is downgraded to +`suggestion` across `samples/`). DYM_006 is the correctness subset: the literal doesn't just ignore the +theme, it renders gray. Both can fire on one literal. + ## §8 Risks and guardrails | Risk | Guardrail | |---|---| | False positives under cascading / partial-edit errors | Bail when the receiver type is unresolved or an error type; only fire when the invocation didn't bind; gate to Reactor symbols; (fuzzy phases) require high similarity thresholds. | | IDE typing latency | Use the provided `context.SemanticModel` (never `Compilation.GetSemanticModel`, RS1030); narrow syntax-node actions; cheap symbol pre-checks before any fuzzy match. | -| Divergence between analyzer and `mur check` | Phase 2: local maps locked to the CLI Tier-3 rules by a cross-check parity test (`DidYouMeanAnalyzerParityTests`). Argument-shape phase (§7.2): the same test asserts `REACTOR_DYM_004`/`_005` and the CLI `SymbolSuggester` both fire on the representative CS7036 / CS1503 shapes. Fuzzy phases: one shared match library + shared fixtures (§7 item 5). | -| Noise (a second diagnostic atop the CS error) | Concise message; `REACTOR_DYM_*` only ever accompanies a genuine compile error; consumers can tune severity via `.editorconfig`. | +| Divergence between analyzer and `mur check` | Phase 2: local maps locked to the CLI Tier-3 rules by a cross-check parity test (`DidYouMeanAnalyzerParityTests`). Argument-shape phase (§7.2): the same test asserts `REACTOR_DYM_004`/`_005` and the CLI `SymbolSuggester` both fire on the representative CS7036 / CS1503 shapes. `REACTOR_DYM_006` (§7.3): `ColorStringParityTests` locks `BrushHelper.TryParseColor` and the analyzer's classifier together; the rule reads the analyzer's own message, and a live-assembly test pins the analyzer's targets to the real Reactor surface. Fuzzy phases: one shared match library + shared fixtures (§7 item 5). | +| Noise (a second diagnostic atop the CS error) | Concise message; `REACTOR_DYM_001`–`_005` only ever accompany a genuine compile error; consumers can tune severity via `.editorconfig`. `REACTOR_DYM_006` is the exception (§7.3) and fires only on a value that provably renders gray. | ## §9 Relationship to `mur check` diff --git a/plugins/reactor/skills/reactor-build-and-check/SKILL.md b/plugins/reactor/skills/reactor-build-and-check/SKILL.md index 03022c386..18294109b 100644 --- a/plugins/reactor/skills/reactor-build-and-check/SKILL.md +++ b/plugins/reactor/skills/reactor-build-and-check/SKILL.md @@ -142,6 +142,7 @@ Additional flags: | `REACTOR_DYM_003` | warning | An unresolved bare call closely matches a Reactor factory name (e.g. `Buton(...)`) — pairs with compiler `CS0103` | Rename to the suggested factory: `Button(...)`. Fires only on a close, unambiguous factory match (the typo flavour of `CS0103`); the IDE offers a one-click rename. | | `REACTOR_DYM_004` | warning | A Reactor factory is called with too few arguments (e.g. `ScrollViewer()`) — pairs with compiler `CS7036` | Supply the missing argument(s); the message lists the factory's full parameter shape as named arguments. Fires only when a single overload uniquely matches, so multi-overload factories (e.g. `Button()`) are deliberately left to the raw compiler error. Message only — no code fix (the `` placeholder wouldn't compile). | | `REACTOR_DYM_005` | warning | A `string` is passed where a Reactor `Element` is expected (e.g. `ScrollViewer("hi")`) — pairs with compiler `CS1503` | Wrap the string in a text factory: `ScrollViewer(TextBlock("hi"))` (or `Heading`/`Caption`). Narrow, high-confidence special case only; general type mismatches degrade to the raw `CS1503`. Message only — no code fix. | +| `REACTOR_DYM_006` | warning | A constant color string `BrushHelper` can't parse (e.g. `.Background("orange")`, `.Foreground("#888")`, `.Resources(r => r.Set("ContentControlThemeFontFamily", "Consolas"))`). It compiles and silently renders **gray**. | Color strings accept only `red`, `green`, `blue`, `white`, `black`, `gray`/`grey`, `lightgray`/`lightgrey`, `transparent`, `#RRGGBB` and `#AARRGGBB`. Use the suggested hex (`"orange"` → `"#FFA500"`, `"#888"` → `"#888888"`, `"FF0000"` → `"#FF0000"`) or a `Theme` token; the IDE fix and `mur check`'s `→ try:` apply the same rewrite. A font name under a `*FontFamily` resource key is never a color: set the font with the `.FontFamily("…")` modifier on the element instead. | | `CS0103` | error | "The name 'X' does not exist in the current context" | Missing `using` — most often `Microsoft.UI.Reactor.Layout` (FlexAlign), `Microsoft.UI.Xaml.Controls` (InfoBarSeverity, Orientation), or `static Microsoft.UI.Reactor.Factories`. (A close-but-wrong factory name instead raises `REACTOR_DYM_003` above.) | | `CS1061` | error | "'X' does not contain a definition for 'Y'" | A type-specific modifier called on an element type that does not declare it — `.Bold()` / `.FontSize()` / `.TextWrapping()` are `TextBlockElement`-only, so `Button("Go").Bold()` fails. This is **not** an ordering problem: modifiers are generic (` where T : Element`) and preserve the concrete type, so `.Margin(16).Bold()` and `.Bold().Margin(16)` compile alike. Check the receiver's element type against `reactor-dsl/references/reactor.api.txt`. | | `CS0117` | error | "'Element' does not contain a definition for X" | You're calling a factory or static member that doesn't exist — confirm the name against `reactor-dsl/references/reactor.api.txt`. (For a missing *instance* modifier see `CS1061` above.) | diff --git a/samples/ReactorHostControlDemo/MainWindow.xaml.cs b/samples/ReactorHostControlDemo/MainWindow.xaml.cs index adc4f8a64..ff74f7143 100644 --- a/samples/ReactorHostControlDemo/MainWindow.xaml.cs +++ b/samples/ReactorHostControlDemo/MainWindow.xaml.cs @@ -47,9 +47,9 @@ public MainWindow() ).Margin(16, 8, 16, 16), items.Count == 0 - ? TextBlock("All done!").Foreground("#888").HAlign(HorizontalAlignment.Center).Margin(16) + ? TextBlock("All done!").Foreground("#888888").HAlign(HorizontalAlignment.Center).Margin(16) : TextBlock($"{items.Count} item{(items.Count == 1 ? "" : "s")} remaining") - .Foreground("#888").HAlign(HorizontalAlignment.Center).Margin(0, 0, 0, 16) + .Foreground("#888888").HAlign(HorizontalAlignment.Center).Margin(0, 0, 0, 16) ); }); TodoPanel.Child = todoHost; diff --git a/samples/apps/wct-controls/Program.cs b/samples/apps/wct-controls/Program.cs index 46179b8ef..2d705bcf9 100644 --- a/samples/apps/wct-controls/Program.cs +++ b/samples/apps/wct-controls/Program.cs @@ -339,7 +339,7 @@ public override Element Render() => private static Element Pane(string label) => Border(TextBlock(label).Center()) - .Background("AliceBlue") + .Background("#F0F8FF") .CornerRadius(8); } @@ -426,7 +426,7 @@ public override Element Render() => VStack(6, TextBlock("1 Microsoft Way"), TextBlock("Redmond, WA 98052"))) - .Background("AliceBlue").CornerRadius(8).Margin(0)) + .Background("#F0F8FF").CornerRadius(8).Margin(0)) .Width(360)); } @@ -484,7 +484,7 @@ public override Element Render() => "A ribbon-style NavigationView whose tabs are TabbedCommandBarItem (a CommandBar) objects in MenuItems — a WinUI control subtree the prop/event wrapper can't express declaratively, so the tabs are built once through the imperative Setters escape hatch (legitimate here: it's controls, not data). The document body is the declarative content slot.", (TabbedCommandBar( content: Border(TextBlock("Document body — pick a ribbon tab above.").Center()) - .Background("AliceBlue").CornerRadius(8).Height(150)) + .Background("#F0F8FF").CornerRadius(8).Height(150)) .Set(BuildTabs)) .Height(260)); diff --git a/src/Reactor.Analyzers/AnalyzerReleases.Unshipped.md b/src/Reactor.Analyzers/AnalyzerReleases.Unshipped.md index e71c30f3a..3a2d39aa4 100644 --- a/src/Reactor.Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Reactor.Analyzers/AnalyzerReleases.Unshipped.md @@ -64,5 +64,6 @@ REACTOR_DYM_002 | Reactor.DidYouMean | Warning | ThemeBackgroundSuffixAnalyzer - REACTOR_DYM_003 | Reactor.DidYouMean | Warning | FuzzyFactoryNameAnalyzer - Mistyped Reactor factory name in call position (e.g. Buton(...)); did-you-mean the closest factory REACTOR_DYM_004 | Reactor.DidYouMean | Warning | MissingFactoryArgumentAnalyzer - Reactor factory called with too few arguments (CS7036); suggests the full parameter shape (single unique overload only) REACTOR_DYM_005 | Reactor.DidYouMean | Warning | StringForElementArgumentAnalyzer - String passed where a Reactor Element is expected (CS1503); wrap it in a text factory (e.g. TextBlock) +REACTOR_DYM_006 | Reactor.DidYouMean | Warning | ColorStringLiteralAnalyzer - Constant color string BrushHelper cannot parse (renders gray); suggests hex, #RRGGBB expansion, or new FontFamily(...) for a *FontFamily resource key REACTOR_ITEMS_002 | Reactor.Collections | Warning | ItemsViewContainerRootAnalyzer - ItemsView viewBuilder returns a non-ItemContainer root (mount-time InvalidOperationException); wrap it with ItemContainer(...) REACTOR_ICON_001 | Reactor.Windowing | Warning | WindowIconSurfaceAnalyzer - WindowIcon source kind the target surface silently skips (FromResource on tray/overlay/thumbnail, which need an HICON; FromBytes/FromRgba on the window caption or a jump list, which need a path or Uri); only fires when the kind is provably known at the use site diff --git a/src/Reactor.Analyzers/ColorStringClassifier.cs b/src/Reactor.Analyzers/ColorStringClassifier.cs new file mode 100644 index 000000000..68d7f3fda --- /dev/null +++ b/src/Reactor.Analyzers/ColorStringClassifier.cs @@ -0,0 +1,348 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Globalization; + +namespace Microsoft.UI.Reactor.Analyzers; + +/// What is wrong with a color string that BrushHelper cannot parse. +internal enum ColorStringIssue +{ + /// A font name passed to ResourceBuilder.Set under a *FontFamily key. + FontFamilyKey, + /// #RGB / #ARGB shorthand, which BrushHelper does not expand. + HexShorthand, + /// Six or eight hex digits without the leading #. + MissingHash, + /// Starts with # but is not #RRGGBB / #AARRGGBB. + MalformedHex, + /// A standard CSS / WinUI named color that BrushHelper's 8-name table lacks. + NamedColor, + /// Anything else. + Unknown, +} + +/// +/// The did-you-mean for one unparseable color string. is the new string +/// literal value for the literal fixes, the family name for +/// (only when the FontFamily overload exists), or when there is no fix. +/// +internal readonly struct ColorStringFinding +{ + public ColorStringFinding(ColorStringIssue issue, string? replacement, string detail) + { + Issue = issue; + Replacement = replacement; + Detail = detail; + } + + public ColorStringIssue Issue { get; } + public string? Replacement { get; } + public string Detail { get; } +} + +/// +/// Classifies color strings for REACTOR_DYM_006 (). +/// +/// +/// is a netstandard2.0 copy of Microsoft.UI.Reactor.BrushHelper.TryParseColor +/// (the analyzer cannot reference src/Reactor); ColorStringParityTests locks the two +/// together — edit them in lockstep. Every fixable ends in +/// did you mean X?: the mur check ColorStringLiteralRule lifts X out of the +/// diagnostic message, so keep that suffix stable. +/// +internal static class ColorStringClassifier +{ + /// The names BrushHelper's switch recognizes (case-insensitive). + public const string SupportedNames = "red, green, blue, white, black, gray/grey, lightgray/lightgrey, transparent"; + + /// The suffix that marks a WinUI FontFamily resource key (e.g. ContentControlThemeFontFamily). + public const string FontFamilyKeySuffix = "FontFamily"; + + /// + /// Standard CSS / WinUI (Microsoft.UI.Colors) named colors that BrushHelper does not + /// recognize, mapped to their exact #RRGGBB. The 8 names BrushHelper does support are excluded. + /// + public static readonly ImmutableDictionary NamedColorHex = + ImmutableDictionary.CreateRange(StringComparer.OrdinalIgnoreCase, new[] + { + new KeyValuePair("aliceblue", "#F0F8FF"), + new KeyValuePair("antiquewhite", "#FAEBD7"), + new KeyValuePair("aqua", "#00FFFF"), + new KeyValuePair("aquamarine", "#7FFFD4"), + new KeyValuePair("azure", "#F0FFFF"), + new KeyValuePair("beige", "#F5F5DC"), + new KeyValuePair("bisque", "#FFE4C4"), + new KeyValuePair("blanchedalmond", "#FFEBCD"), + new KeyValuePair("blueviolet", "#8A2BE2"), + new KeyValuePair("brown", "#A52A2A"), + new KeyValuePair("burlywood", "#DEB887"), + new KeyValuePair("cadetblue", "#5F9EA0"), + new KeyValuePair("chartreuse", "#7FFF00"), + new KeyValuePair("chocolate", "#D2691E"), + new KeyValuePair("coral", "#FF7F50"), + new KeyValuePair("cornflowerblue", "#6495ED"), + new KeyValuePair("cornsilk", "#FFF8DC"), + new KeyValuePair("crimson", "#DC143C"), + new KeyValuePair("cyan", "#00FFFF"), + new KeyValuePair("darkblue", "#00008B"), + new KeyValuePair("darkcyan", "#008B8B"), + new KeyValuePair("darkgoldenrod", "#B8860B"), + new KeyValuePair("darkgray", "#A9A9A9"), + new KeyValuePair("darkgreen", "#006400"), + new KeyValuePair("darkgrey", "#A9A9A9"), + new KeyValuePair("darkkhaki", "#BDB76B"), + new KeyValuePair("darkmagenta", "#8B008B"), + new KeyValuePair("darkolivegreen", "#556B2F"), + new KeyValuePair("darkorange", "#FF8C00"), + new KeyValuePair("darkorchid", "#9932CC"), + new KeyValuePair("darkred", "#8B0000"), + new KeyValuePair("darksalmon", "#E9967A"), + new KeyValuePair("darkseagreen", "#8FBC8F"), + new KeyValuePair("darkslateblue", "#483D8B"), + new KeyValuePair("darkslategray", "#2F4F4F"), + new KeyValuePair("darkslategrey", "#2F4F4F"), + new KeyValuePair("darkturquoise", "#00CED1"), + new KeyValuePair("darkviolet", "#9400D3"), + new KeyValuePair("deeppink", "#FF1493"), + new KeyValuePair("deepskyblue", "#00BFFF"), + new KeyValuePair("dimgray", "#696969"), + new KeyValuePair("dimgrey", "#696969"), + new KeyValuePair("dodgerblue", "#1E90FF"), + new KeyValuePair("firebrick", "#B22222"), + new KeyValuePair("floralwhite", "#FFFAF0"), + new KeyValuePair("forestgreen", "#228B22"), + new KeyValuePair("fuchsia", "#FF00FF"), + new KeyValuePair("gainsboro", "#DCDCDC"), + new KeyValuePair("ghostwhite", "#F8F8FF"), + new KeyValuePair("gold", "#FFD700"), + new KeyValuePair("goldenrod", "#DAA520"), + new KeyValuePair("greenyellow", "#ADFF2F"), + new KeyValuePair("honeydew", "#F0FFF0"), + new KeyValuePair("hotpink", "#FF69B4"), + new KeyValuePair("indianred", "#CD5C5C"), + new KeyValuePair("indigo", "#4B0082"), + new KeyValuePair("ivory", "#FFFFF0"), + new KeyValuePair("khaki", "#F0E68C"), + new KeyValuePair("lavender", "#E6E6FA"), + new KeyValuePair("lavenderblush", "#FFF0F5"), + new KeyValuePair("lawngreen", "#7CFC00"), + new KeyValuePair("lemonchiffon", "#FFFACD"), + new KeyValuePair("lightblue", "#ADD8E6"), + new KeyValuePair("lightcoral", "#F08080"), + new KeyValuePair("lightcyan", "#E0FFFF"), + new KeyValuePair("lightgoldenrodyellow", "#FAFAD2"), + new KeyValuePair("lightgreen", "#90EE90"), + new KeyValuePair("lightpink", "#FFB6C1"), + new KeyValuePair("lightsalmon", "#FFA07A"), + new KeyValuePair("lightseagreen", "#20B2AA"), + new KeyValuePair("lightskyblue", "#87CEFA"), + new KeyValuePair("lightslategray", "#778899"), + new KeyValuePair("lightslategrey", "#778899"), + new KeyValuePair("lightsteelblue", "#B0C4DE"), + new KeyValuePair("lightyellow", "#FFFFE0"), + new KeyValuePair("lime", "#00FF00"), + new KeyValuePair("limegreen", "#32CD32"), + new KeyValuePair("linen", "#FAF0E6"), + new KeyValuePair("magenta", "#FF00FF"), + new KeyValuePair("maroon", "#800000"), + new KeyValuePair("mediumaquamarine", "#66CDAA"), + new KeyValuePair("mediumblue", "#0000CD"), + new KeyValuePair("mediumorchid", "#BA55D3"), + new KeyValuePair("mediumpurple", "#9370DB"), + new KeyValuePair("mediumseagreen", "#3CB371"), + new KeyValuePair("mediumslateblue", "#7B68EE"), + new KeyValuePair("mediumspringgreen", "#00FA9A"), + new KeyValuePair("mediumturquoise", "#48D1CC"), + new KeyValuePair("mediumvioletred", "#C71585"), + new KeyValuePair("midnightblue", "#191970"), + new KeyValuePair("mintcream", "#F5FFFA"), + new KeyValuePair("mistyrose", "#FFE4E1"), + new KeyValuePair("moccasin", "#FFE4B5"), + new KeyValuePair("navajowhite", "#FFDEAD"), + new KeyValuePair("navy", "#000080"), + new KeyValuePair("oldlace", "#FDF5E6"), + new KeyValuePair("olive", "#808000"), + new KeyValuePair("olivedrab", "#6B8E23"), + new KeyValuePair("orange", "#FFA500"), + new KeyValuePair("orangered", "#FF4500"), + new KeyValuePair("orchid", "#DA70D6"), + new KeyValuePair("palegoldenrod", "#EEE8AA"), + new KeyValuePair("palegreen", "#98FB98"), + new KeyValuePair("paleturquoise", "#AFEEEE"), + new KeyValuePair("palevioletred", "#DB7093"), + new KeyValuePair("papayawhip", "#FFEFD5"), + new KeyValuePair("peachpuff", "#FFDAB9"), + new KeyValuePair("peru", "#CD853F"), + new KeyValuePair("pink", "#FFC0CB"), + new KeyValuePair("plum", "#DDA0DD"), + new KeyValuePair("powderblue", "#B0E0E6"), + new KeyValuePair("purple", "#800080"), + new KeyValuePair("rebeccapurple", "#663399"), + new KeyValuePair("rosybrown", "#BC8F8F"), + new KeyValuePair("royalblue", "#4169E1"), + new KeyValuePair("saddlebrown", "#8B4513"), + new KeyValuePair("salmon", "#FA8072"), + new KeyValuePair("sandybrown", "#F4A460"), + new KeyValuePair("seagreen", "#2E8B57"), + new KeyValuePair("seashell", "#FFF5EE"), + new KeyValuePair("sienna", "#A0522D"), + new KeyValuePair("silver", "#C0C0C0"), + new KeyValuePair("skyblue", "#87CEEB"), + new KeyValuePair("slateblue", "#6A5ACD"), + new KeyValuePair("slategray", "#708090"), + new KeyValuePair("slategrey", "#708090"), + new KeyValuePair("snow", "#FFFAFA"), + new KeyValuePair("springgreen", "#00FF7F"), + new KeyValuePair("steelblue", "#4682B4"), + new KeyValuePair("tan", "#D2B48C"), + new KeyValuePair("teal", "#008080"), + new KeyValuePair("thistle", "#D8BFD8"), + new KeyValuePair("tomato", "#FF6347"), + new KeyValuePair("turquoise", "#40E0D0"), + new KeyValuePair("violet", "#EE82EE"), + new KeyValuePair("wheat", "#F5DEB3"), + new KeyValuePair("whitesmoke", "#F5F5F5"), + new KeyValuePair("yellow", "#FFFF00"), + new KeyValuePair("yellowgreen", "#9ACD32"), + }); + + /// + /// Mirrors BrushHelper.TryParseColor: iff the string does not fall + /// back to gray. + /// + public static bool TryParse(string color) + { + switch (color.ToLowerInvariant()) + { + case "red": + case "green": + case "blue": + case "white": + case "black": + case "gray": + case "grey": + case "lightgray": + case "lightgrey": + case "transparent": + return true; + } + return color.StartsWith("#", StringComparison.Ordinal) && TryParseHex(color); + } + + private static bool TryParseHex(string hex) + { + hex = hex.TrimStart('#'); + if (hex.Length == 6) + return IsByte(hex, 0) && IsByte(hex, 2) && IsByte(hex, 4); + if (hex.Length == 8) + return IsByte(hex, 0) && IsByte(hex, 2) && IsByte(hex, 4) && IsByte(hex, 6); + return false; + } + + // Same call as BrushHelper (`byte.TryParse(hex[i..i+2], NumberStyles.HexNumber, null, out _)`), + // including HexNumber's leading/trailing-whitespace allowance. + private static bool IsByte(string hex, int start) => + byte.TryParse(hex.Substring(start, 2), NumberStyles.HexNumber, null, out _); + + /// + /// Returns the finding for , or when BrushHelper + /// parses it. is the constant key of a ResourceBuilder.Set + /// call (otherwise ); says whether + /// ResourceBuilder.Set(string, FontFamily) is available to rewrite to; + /// is that overload's parameter name when the flagged argument is named (color: "…") and must be renamed. + /// + public static ColorStringFinding? Classify(string value, string? resourceKey, bool fontFamilyOverloadExists, string? fontFamilyArgumentName = null) + { + if (TryParse(value)) + return null; + + if (resourceKey is not null && resourceKey.EndsWith(FontFamilyKeySuffix, StringComparison.Ordinal)) + { + return fontFamilyOverloadExists + ? new ColorStringFinding( + ColorStringIssue.FontFamilyKey, + value, + $"'{resourceKey}' is a FontFamily resource; did you mean {FontFamilySuggestion(value, fontFamilyArgumentName)}?") + : new ColorStringFinding( + ColorStringIssue.FontFamilyKey, + null, + $"'{resourceKey}' is a FontFamily resource, but this Set overload only stores color brushes; to change the font, use .FontFamily({Literal(value)}) on the element"); + } + + if (value.StartsWith("#", StringComparison.Ordinal)) + { + var digits = value.Substring(1); + if ((digits.Length == 3 || digits.Length == 4) && IsAllHex(digits)) + { + var expanded = Expand(digits); + return new ColorStringFinding( + ColorStringIssue.HexShorthand, + expanded, + $"#RGB/#ARGB shorthand isn't supported; did you mean \"{expanded}\"?"); + } + return new ColorStringFinding( + ColorStringIssue.MalformedHex, + null, + "hex colors must be #RRGGBB or #AARRGGBB"); + } + + if ((value.Length == 6 || value.Length == 8) && IsAllHex(value)) + { + var hashed = "#" + value; + return new ColorStringFinding( + ColorStringIssue.MissingHash, + hashed, + $"hex colors need a leading '#'; did you mean \"{hashed}\"?"); + } + + if (NamedColorHex.TryGetValue(value, out var hex)) + { + return new ColorStringFinding( + ColorStringIssue.NamedColor, + hex, + $"the only built-in names are {SupportedNames}; did you mean \"{hex}\"?"); + } + + return new ColorStringFinding( + ColorStringIssue.Unknown, + null, + $"use #RRGGBB or #AARRGGBB, a Theme token, or one of {SupportedNames}"); + } + + /// + /// The C# source the FontFamily rewrite produces, e.g. new Microsoft.UI.Xaml.Media.FontFamily("Consolas"), + /// prefixed with name: for a renamed argument. The type is namespace-qualified so the text compiles + /// without a using Microsoft.UI.Xaml.Media; (mur check shows it verbatim), and the value is escaped + /// as a regular C# string literal. + /// + public static string FontFamilySuggestion(string value, string? argumentName) => + (argumentName is null ? "" : argumentName + ": ") + "new Microsoft.UI.Xaml.Media.FontFamily(" + Literal(value) + ")"; + + // A regular C# string literal for the value (quotes, backslashes and control characters escaped). + private static string Literal(string value) => + Microsoft.CodeAnalysis.CSharp.SymbolDisplay.FormatLiteral(value, quote: true); + + private static bool IsAllHex(string s) + { + foreach (var c in s) + { + var isHex = (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'); + if (!isHex) + return false; + } + return true; + } + + private static string Expand(string digits) + { + var chars = new char[1 + digits.Length * 2]; + chars[0] = '#'; + for (var i = 0; i < digits.Length; i++) + { + chars[1 + i * 2] = digits[i]; + chars[2 + i * 2] = digits[i]; + } + return new string(chars); + } +} diff --git a/src/Reactor.Analyzers/ColorStringLiteralAnalyzer.cs b/src/Reactor.Analyzers/ColorStringLiteralAnalyzer.cs new file mode 100644 index 000000000..568cf95ff --- /dev/null +++ b/src/Reactor.Analyzers/ColorStringLiteralAnalyzer.cs @@ -0,0 +1,378 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.UI.Reactor.Analyzers; + +/// +/// REACTOR_DYM_006 — a compile-time-constant color string that BrushHelper cannot parse, so +/// it silently renders gray (e.g. .Background("orange"), .Foreground("#888"), or a font +/// name stored through ResourceBuilder.Set(key, "Consolas")). Paired with +/// . +/// +/// +/// +/// Covers every Reactor API whose color string goes through BrushHelper.Parse: +/// BrushHelper.Parse itself, ResourceBuilder.Set(string, string), and the +/// string overloads of the Background / Foreground / BorderBrush / +/// WithBorder modifiers on ElementExtensions. Targets are resolved once per +/// compilation and matched by symbol identity, so a look-alike API in another namespace never fires. +/// +/// +/// Unlike REACTOR_DYM_001–005 this does not ride a compiler error — the code compiles — but +/// the result is always a wrong (gray) brush, so it is a Warning. Only constant values are checked +/// (literals, consts, constant concatenation, and each constant branch of a ?: / +/// ?? / switch expression); runtime strings are out of reach. The mur check +/// ColorStringLiteralRule anchors on this diagnostic's ID and repeats the did-you-mean. +/// +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ColorStringLiteralAnalyzer : DiagnosticAnalyzer +{ + public const string DiagnosticId = "REACTOR_DYM_006"; + + public const string BrushHelperMetadataName = "Microsoft.UI.Reactor.BrushHelper"; + public const string ResourceBuilderMetadataName = "Microsoft.UI.Reactor.Elements.ResourceBuilder"; + public const string ElementExtensionsMetadataName = "Microsoft.UI.Reactor.ElementExtensions"; + public const string FontFamilyMetadataName = "Microsoft.UI.Xaml.Media.FontFamily"; + + /// The modifier names whose string color overloads parse through BrushHelper. + public static readonly ImmutableHashSet ModifierNames = + ImmutableHashSet.Create(StringComparer.Ordinal, "Background", "Foreground", "BorderBrush", "WithBorder"); + + /// The parameter name every covered API uses for its color string. + public const string ColorParameterName = "color"; + + /// The resource-key parameter name of ResourceBuilder.Set. + public const string KeyParameterName = "key"; + + /// Property-bag key: the name. + public const string IssueProperty = "Issue"; + + /// Property-bag key: the replacement value (literal text, or family name). Absent when there is no fix. + public const string ReplacementProperty = "Replacement"; + + /// + /// Property-bag key (FontFamily fix only): the value parameter name of + /// ResourceBuilder.Set(string, FontFamily), so a named color: argument can be renamed. + /// + public const string FontFamilyParameterProperty = "FontFamilyParameter"; + + private static readonly LocalizableString Title = + "Unrecognized color string renders gray"; + private static readonly LocalizableString MessageFormat = + "'{0}' is not a color string Reactor can parse, so it renders gray — {1}"; + private static readonly LocalizableString Description = + "BrushHelper.Parse understands only red, green, blue, white, black, gray/grey, lightgray/lightgrey, transparent, #RRGGBB and #AARRGGBB. Anything else silently falls back to a gray brush. This covers BrushHelper.Parse, ResourceBuilder.Set(string, string), and the string overloads of the Background/Foreground/BorderBrush/WithBorder modifiers."; + + private static readonly DiagnosticDescriptor Rule = new( + DiagnosticId, + Title, + MessageFormat, + "Reactor.DidYouMean", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: Description); + + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(start => + { + var targets = Targets.Resolve(start.Compilation); + if (targets is null) + return; // No Reactor color-string API referenced. + + start.RegisterSyntaxNodeAction( + ctx => AnalyzeInvocation(ctx, targets), + SyntaxKind.InvocationExpression); + }); + } + + /// The resolved color-string APIs for one compilation. + internal sealed class Targets + { + private Targets(ImmutableHashSet methods, IMethodSymbol? resourceSet, string? fontFamilyParameterName) + { + Methods = methods; + ResourceSet = resourceSet; + FontFamilyParameterName = fontFamilyParameterName; + } + + public ImmutableHashSet Methods { get; } + public IMethodSymbol? ResourceSet { get; } + + /// The value parameter of ResourceBuilder.Set(string, FontFamily), or null when that overload doesn't exist. + public string? FontFamilyParameterName { get; } + + public static Targets? Resolve(Compilation compilation) + { + var methods = ImmutableHashSet.CreateBuilder(SymbolEqualityComparer.Default); + var str = compilation.GetSpecialType(SpecialType.System_String); + + var brushHelper = compilation.GetTypeByMetadataName(BrushHelperMetadataName); + if (brushHelper is not null) + { + foreach (var m in brushHelper.GetMembers("Parse").OfType().Where(m => HasStringColorParameter(m, str))) + methods.Add(m); + } + + IMethodSymbol? resourceSet = null; + string? fontFamilyParameter = null; + var resourceBuilder = compilation.GetTypeByMetadataName(ResourceBuilderMetadataName); + if (resourceBuilder is not null) + { + foreach (var m in resourceBuilder.GetMembers("Set").OfType().Where(m => m.Parameters.Length == 2)) + { + if (HasStringColorParameter(m, str) && SymbolEqualityComparer.Default.Equals(m.Parameters[0].Type, str)) + { + resourceSet = m; + methods.Add(m); + } + else if (m.Parameters[1].Type.ToDisplayString() == FontFamilyMetadataName) + { + fontFamilyParameter = m.Parameters[1].Name; + } + } + } + + var extensions = compilation.GetTypeByMetadataName(ElementExtensionsMetadataName); + if (extensions is not null) + { + foreach (var name in ModifierNames) + foreach (var m in extensions.GetMembers(name).OfType().Where(m => m.IsExtensionMethod && HasStringColorParameter(m, str))) + methods.Add(m); + } + + return methods.Count == 0 ? null : new Targets(methods.ToImmutable(), resourceSet, fontFamilyParameter); + } + + private static bool HasStringColorParameter(IMethodSymbol method, ITypeSymbol str) + { + foreach (var p in method.Parameters) + if (p.Name == ColorParameterName && SymbolEqualityComparer.Default.Equals(p.Type, str)) + return true; + return false; + } + } + + private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context, Targets targets) + { + var invocation = (InvocationExpressionSyntax)context.Node; + + // Cheap syntactic pre-filter before any binding. + var calledName = invocation.Expression switch + { + MemberAccessExpressionSyntax m => m.Name.Identifier.ValueText, + IdentifierNameSyntax id => id.Identifier.ValueText, + GenericNameSyntax g => g.Identifier.ValueText, + MemberBindingExpressionSyntax b => b.Name.Identifier.ValueText, + _ => null, + }; + if (calledName is null || !(calledName == "Parse" || calledName == "Set" || ModifierNames.Contains(calledName))) + return; + // The ubiquitous `.Set(c => …)` setter modifier has one argument; ResourceBuilder.Set has two. + if (calledName == "Set" && invocation.ArgumentList.Arguments.Count != 2) + return; + + if (context.SemanticModel.GetOperation(invocation, context.CancellationToken) is not IInvocationOperation operation) + return; + + var target = operation.TargetMethod; + var method = (target.ReducedFrom ?? target).OriginalDefinition; + if (!targets.Methods.Contains(method)) + return; + + IArgumentOperation? colorArg = null; + IArgumentOperation? keyArg = null; + foreach (var arg in operation.Arguments) + { + if (arg.Parameter is null) + continue; + if (arg.Parameter.Name == ColorParameterName) + colorArg = arg; + else if (arg.Parameter.Name == KeyParameterName) + keyArg = arg; + } + if (colorArg is null || colorArg.ArgumentKind != ArgumentKind.Explicit) + return; + // Through a user-defined conversion (e.g. a `ColorToken` with `implicit operator string`) the + // source strings never reach BrushHelper — the operator's result does — so the syntax says nothing. + if (colorArg.Value is IConversionOperation { Conversion.IsUserDefined: true }) + return; + + string? resourceKey = null; + if (targets.ResourceSet is not null + && SymbolEqualityComparer.Default.Equals(method, targets.ResourceSet) + && keyArg?.Value.ConstantValue is { HasValue: true, Value: string key }) + { + resourceKey = key; + } + + foreach (var (expression, value) in ConstantStrings(colorArg.Value.Syntax as ExpressionSyntax, context.SemanticModel, context.CancellationToken)) + { + // new FontFamily(...) can only replace the whole argument: inside a ?: branch the other + // branch would still be a string and the expression would stop type-checking. + var canUseFontFamily = targets.FontFamilyParameterName is not null && expression.Parent is ArgumentSyntax; + // A named `color:` argument must be renamed too, or the rewritten call won't bind. + var renamedArgument = canUseFontFamily && expression.Parent is ArgumentSyntax { NameColon: not null } + ? targets.FontFamilyParameterName + : null; + var finding = ColorStringClassifier.Classify(value, resourceKey, canUseFontFamily, renamedArgument); + if (finding is not { } f) + continue; + + var properties = ImmutableDictionary.CreateBuilder(); + properties.Add(IssueProperty, f.Issue.ToString()); + if (f.Replacement is not null) + properties.Add(ReplacementProperty, f.Replacement); + if (f.Issue == ColorStringIssue.FontFamilyKey && f.Replacement is not null) + properties.Add(FontFamilyParameterProperty, targets.FontFamilyParameterName); + + context.ReportDiagnostic(Diagnostic.Create( + Rule, + expression.GetLocation(), + properties.ToImmutable(), + value, + f.Detail)); + } + } + + /// + /// The constant strings an argument can evaluate to, each with the expression that produced it. + /// A wholly-constant expression yields itself; otherwise only value-selecting shapes are + /// walked (parentheses, ?:, ??, switch arms) — never into nested calls. Branches a + /// constant condition or a constant non-null ?? left side rules out are skipped, so dead + /// code (DebugBuild ? "magenta" : color with a const bool) is never reported. + /// + internal static IEnumerable<(ExpressionSyntax Expression, string Value)> ConstantStrings( + ExpressionSyntax? expression, SemanticModel model, CancellationToken cancellationToken) + { + if (expression is null) + yield break; + + var constant = model.GetConstantValue(expression, cancellationToken); + if (constant.HasValue) + { + if (constant.Value is string s) + yield return (expression, s); + yield break; + } + + if (!IsPlainString(expression, model, cancellationToken)) + yield break; + + foreach (var child in LiveChildren(expression, model, cancellationToken)) + foreach (var item in ConstantStrings(child, model, cancellationToken)) + yield return item; + } + + /// + /// Whether a non-constant sub-expression is itself a string value that flows on unchanged. A branch + /// of another type (e.g. token ?? "orange" where token is a ColorToken with + /// implicit operator string) or one reaching string through a user-defined conversion is a + /// boundary: whatever strings it contains, the operator's result is what gets parsed. + /// + private static bool IsPlainString(ExpressionSyntax expression, SemanticModel model, CancellationToken cancellationToken) + { + var typeInfo = model.GetTypeInfo(expression, cancellationToken); + if ((typeInfo.Type ?? typeInfo.ConvertedType)?.SpecialType != SpecialType.System_String) + return false; + return !model.GetConversion(expression, cancellationToken).IsUserDefined; + } + + /// + /// The sub-expressions a value-selecting shape can actually yield. Empty for any other shape. + /// + private static IEnumerable LiveChildren(ExpressionSyntax expression, SemanticModel model, CancellationToken cancellationToken) => + expression switch + { + ParenthesizedExpressionSyntax p => new[] { p.Expression }, + ConditionalExpressionSyntax c => model.GetConstantValue(c.Condition, cancellationToken) is { HasValue: true, Value: bool taken } + ? new[] { taken ? c.WhenTrue : c.WhenFalse } + : new[] { c.WhenTrue, c.WhenFalse }, + BinaryExpressionSyntax b when b.IsKind(SyntaxKind.CoalesceExpression) => + IsKnownNonNull(b.Left, model, cancellationToken) + ? new[] { b.Left } + : new[] { b.Left, b.Right }, + // Arms are only pruned when every pattern is a plain constant and the governing value is a + // constant; anything fancier (relational, type, `when`) keeps every arm. + SwitchExpressionSyntax sw => LiveArms(sw, model, cancellationToken), + _ => Array.Empty(), + }; + + /// + /// Whether the expression provably evaluates to a non-null value: a non-null constant, a + /// throw (it never yields at all), or a value-selecting shape whose every live branch is one + /// of those. Unknown (e.g. a variable) is . + /// + private static bool IsKnownNonNull(ExpressionSyntax expression, SemanticModel model, CancellationToken cancellationToken) + { + if (expression is ThrowExpressionSyntax) + return true; + if (!IsPlainString(expression, model, cancellationToken)) + return false; + + var constant = model.GetConstantValue(expression, cancellationToken); + if (constant.HasValue) + return constant.Value is not null; + + if (expression is BinaryExpressionSyntax b && b.IsKind(SyntaxKind.CoalesceExpression)) + { + // A non-null left value can still become null through a user-defined conversion + // (`token ?? "red"` with an `implicit operator string` that returns null), so such a + // coalesce is never provably non-null, whatever its right side is. + if (model.GetOperation(b, cancellationToken) is ICoalesceOperation { ValueConversion.IsUserDefined: true }) + return false; + return IsKnownNonNull(b.Left, model, cancellationToken) || IsKnownNonNull(b.Right, model, cancellationToken); + } + + var any = false; + foreach (var child in LiveChildren(expression, model, cancellationToken)) + { + if (!IsKnownNonNull(child, model, cancellationToken)) + return false; + any = true; + } + return any; + } + + private static IEnumerable LiveArms(SwitchExpressionSyntax sw, SemanticModel model, CancellationToken cancellationToken) + { + var governing = model.GetConstantValue(sw.GoverningExpression, cancellationToken); + if (!governing.HasValue) + return sw.Arms.Select(a => a.Expression); + + foreach (var arm in sw.Arms) + { + if (arm.WhenClause is not null) + return sw.Arms.Select(a => a.Expression); + switch (arm.Pattern) + { + case DiscardPatternSyntax: + return new[] { arm.Expression }; + // Compare the pattern constant *converted to the governing type* (`1` against a + // `const long` is a long 1), not the boxed source literal. + case ConstantPatternSyntax when model.GetOperation(arm.Pattern, cancellationToken) is IConstantPatternOperation { Value.ConstantValue: { HasValue: true } armValue }: + if (Equals(armValue.Value, governing.Value)) + return new[] { arm.Expression }; + break; + default: + return sw.Arms.Select(a => a.Expression); + } + } + return Array.Empty(); + } +} diff --git a/src/Reactor.Analyzers/ColorStringLiteralCodeFix.cs b/src/Reactor.Analyzers/ColorStringLiteralCodeFix.cs new file mode 100644 index 000000000..7296e9676 --- /dev/null +++ b/src/Reactor.Analyzers/ColorStringLiteralCodeFix.cs @@ -0,0 +1,107 @@ +using System.Collections.Immutable; +using System.Linq; +using System.Composition; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Simplification; + +namespace Microsoft.UI.Reactor.Analyzers; + +/// +/// Code fix for (REACTOR_DYM_006). Rewrites a string literal +/// to the suggested color ("#888" → "#888888", "FF0000" → "#FF0000", +/// "orange" → "#FFA500"), or — for a font name stored under a *FontFamily +/// resource key — rewrites the argument to new FontFamily("…") so it binds to the +/// ResourceBuilder.Set(string, FontFamily) overload. The replacement travels in the +/// diagnostic's property bag; no fix is offered when the analyzer attached none. The literal fixes +/// need a plain string literal (a flagged const reference is fixed at its declaration instead). +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ColorStringLiteralCodeFix))] +[Shared] +public sealed class ColorStringLiteralCodeFix : CodeFixProvider +{ + private const string LiteralEquivalenceKey = "Reactor_ColorStringLiteral"; + private const string FontFamilyEquivalenceKey = "Reactor_ColorStringFontFamily"; + + public override ImmutableArray FixableDiagnosticIds => + ImmutableArray.Create(ColorStringLiteralAnalyzer.DiagnosticId); + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) return; + + // Only diagnostics the analyzer attached a rewrite to are fixable. + foreach (var diagnostic in context.Diagnostics.Where(d => + d.Properties.TryGetValue(ColorStringLiteralAnalyzer.ReplacementProperty, out var r) && r is not null + && d.Properties.ContainsKey(ColorStringLiteralAnalyzer.IssueProperty))) + { + var replacement = diagnostic.Properties[ColorStringLiteralAnalyzer.ReplacementProperty]!; + var issue = diagnostic.Properties[ColorStringLiteralAnalyzer.IssueProperty]; + + var node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); + if (node is ArgumentSyntax argument) + node = argument.Expression; + if (node is not ExpressionSyntax expression) + continue; + + if (issue == nameof(ColorStringIssue.FontFamilyKey)) + { + // Only when the flagged value *is* the argument — inside a ?: branch the other branch + // would still be a string, and the expression would no longer type-check. A const + // reference is fine to wrap as-is. + if (expression.Parent is not ArgumentSyntax) + continue; + + var creation = SyntaxFactory.ObjectCreationExpression( + SyntaxFactory.ParseTypeName("global::" + ColorStringLiteralAnalyzer.FontFamilyMetadataName) + .WithAdditionalAnnotations(Simplifier.Annotation)) + .WithArgumentList(SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList( + SyntaxFactory.Argument(expression.WithoutTrivia())))) + .WithTriviaFrom(expression); + + // `color: "Consolas"` must become `: new FontFamily(...)`, or the call + // would no longer bind to the FontFamily overload. + var oldArgument = (ArgumentSyntax)expression.Parent!; + var newArgument = oldArgument.WithExpression(creation); + if (oldArgument.NameColon is { } nameColon) + { + if (!diagnostic.Properties.TryGetValue(ColorStringLiteralAnalyzer.FontFamilyParameterProperty, out var parameterName) + || string.IsNullOrEmpty(parameterName)) + continue; + newArgument = newArgument.WithNameColon( + nameColon.WithName(SyntaxFactory.IdentifierName(parameterName!).WithTriviaFrom(nameColon.Name))); + } + + context.RegisterCodeFix( + CodeAction.Create( + $"Use {ColorStringClassifier.FontFamilySuggestion(replacement, null)}", + _ => Task.FromResult(context.Document.WithSyntaxRoot(root.ReplaceNode(oldArgument, newArgument))), + equivalenceKey: FontFamilyEquivalenceKey), + diagnostic); + continue; + } + + if (expression is not LiteralExpressionSyntax literal || !literal.IsKind(SyntaxKind.StringLiteralExpression)) + continue; + + var newLiteral = SyntaxFactory.LiteralExpression( + SyntaxKind.StringLiteralExpression, + SyntaxFactory.Literal(replacement!)) + .WithTriviaFrom(literal); + + context.RegisterCodeFix( + CodeAction.Create( + $"Change to \"{replacement}\"", + _ => Task.FromResult(context.Document.WithSyntaxRoot(root.ReplaceNode(literal, newLiteral))), + equivalenceKey: LiteralEquivalenceKey), + diagnostic); + } + } +} diff --git a/src/Reactor.Cli/Check/Rules/ColorStringLiteralRule.cs b/src/Reactor.Cli/Check/Rules/ColorStringLiteralRule.cs new file mode 100644 index 000000000..5cd061423 --- /dev/null +++ b/src/Reactor.Cli/Check/Rules/ColorStringLiteralRule.cs @@ -0,0 +1,77 @@ +// Mirror of the REACTOR_DYM_006 analyzer (ColorStringLiteralAnalyzer) — issue #1375. +// +// A compile-time-constant color string that BrushHelper can't parse compiles +// fine and silently renders gray, so unlike every other Tier-3 rule there is +// no compiler error to hang this on. The rule therefore anchors on the +// analyzer's own diagnostic ID: `dotnet build` (which `mur check` wraps) runs +// the shipped Reactor.Analyzers bundle and REACTOR_DYM_006 lands in the parsed +// output. +// +// The analyzer has already done the work: every fixable REACTOR_DYM_006 +// message ends in `did you mean X?`, computed for the exact literal the +// diagnostic points at. The rule lifts X out of that message rather than +// re-deriving it, so it can never disagree with the IDE code fix — and since +// CheckCommand truncates messages to 100 chars, the `→ try:` suffix is what +// keeps the rewrite visible. A *FontFamily key without the FontFamily overload +// has no rewrite of the flagged argument, but its message names the +// `.FontFamily("…")` element modifier as the way to set the font; the rule +// surfaces that too, because truncation would otherwise hide it. Messages with +// no concrete next step (unknown name, malformed hex) stay silent. +// It deliberately has NO Tier-1 HintFor entry in CheckCommand — that table +// wins ties and would mask the suggestion. +// +// DidYouMeanAnalyzerParityTests feeds real analyzer diagnostics through this +// rule and asserts it suggests the code fix's rewrite. + +using System.Text.RegularExpressions; + +namespace Microsoft.UI.Reactor.Cli.Check.Rules; + +internal sealed partial class ColorStringLiteralRule : IRulePattern +{ + internal const string DiagnosticId = "REACTOR_DYM_006"; + const string BrushHelperFqn = "Microsoft.UI.Reactor.BrushHelper"; + const string ResourceBuilderFqn = "Microsoft.UI.Reactor.Elements.ResourceBuilder"; + const string ElementExtensionsFqn = "Microsoft.UI.Reactor.ElementExtensions"; + + public string Name => "ColorStringLiteralRule"; + public string Provenance => "analyzer:REACTOR_DYM_006"; + public IReadOnlyList DiagnosticCodes { get; } = new[] { DiagnosticId }; + public IReadOnlyList DeclaredTargets { get; } = new[] { BrushHelperFqn, ResourceBuilderFqn, ElementExtensionsFqn }; + + // `'' is not a color string Reactor can parse, so it renders gray — ; did you mean ?` + [GeneratedRegex(@"^'(?.*?)' is not a color string Reactor can parse\b.*; did you mean (?.+)\?$", RegexOptions.Singleline)] + private static partial Regex MessagePattern(); + + // `… — '' is a FontFamily resource, but this Set overload only stores color brushes; to change the font, use on the element` + [GeneratedRegex(@"^'.*?' is not a color string Reactor can parse\b.*; to change the font, use (?\.FontFamily\(.+\)) on the element$", RegexOptions.Singleline)] + private static partial Regex FontModifierPattern(); + + public RuleSuggestion TryMatch(in RuleContext ctx) + { + if (ctx.Diagnostic.Id != DiagnosticId) return RuleSuggestion.Silent; + + var message = ctx.Diagnostic.GetMessage(); + var match = MessagePattern().Match(message); + if (!match.Success) + { + var modifier = FontModifierPattern().Match(message); + return modifier.Success + ? new RuleSuggestion( + Text: modifier.Groups["modifier"].Value, + Confidence: 0.9, + Evidence: "*FontFamily key: Set(key, string) only stores brushes") + : RuleSuggestion.Silent; + } + + var suggestion = match.Groups["suggestion"].Value; + var value = match.Groups["value"].Value; + var evidence = suggestion.Contains(".FontFamily(", StringComparison.Ordinal) + ? "*FontFamily key → FontFamily overload" + : $"'{Truncate(value)}' renders gray"; + + return new RuleSuggestion(Text: suggestion, Confidence: 0.95, Evidence: evidence); + } + + static string Truncate(string s) => s.Length <= 24 ? s : s[..21] + "..."; +} diff --git a/src/Reactor.Cli/Check/Rules/RuleRegistry.cs b/src/Reactor.Cli/Check/Rules/RuleRegistry.cs index 22a2cfefe..81aa5f537 100644 --- a/src/Reactor.Cli/Check/Rules/RuleRegistry.cs +++ b/src/Reactor.Cli/Check/Rules/RuleRegistry.cs @@ -150,6 +150,7 @@ public IReadOnlyList Statuses(CSharpCompilation? compilation, ISet BuiltInRules() => [ new ButtonOnClickFactoryMoveRule(), + new ColorStringLiteralRule(), new GridSizeFactoryParensRule(), new GridSizePxRenameRule(), new TextBlockStyleHintRule(), diff --git a/src/Reactor/Elements/BrushHelper.cs b/src/Reactor/Elements/BrushHelper.cs index 3795add41..f57f2811d 100644 --- a/src/Reactor/Elements/BrushHelper.cs +++ b/src/Reactor/Elements/BrushHelper.cs @@ -26,7 +26,8 @@ public static class BrushHelper /// caller. Supports named colors (red, green, blue, white, black, gray, lightgray, /// transparent) and hex codes (#RRGGBB or #AARRGGBB). The parsed color is cached, /// but a new brush instance is returned on every call, so callers may safely mutate - /// it and a brush is never shared between controls. + /// it and a brush is never shared between controls. Any other string silently + /// yields a gray brush; the REACTOR_DYM_006 analyzer flags constant strings that do. /// public static SolidColorBrush Parse(string color) => new(ParseColor(color)); @@ -36,21 +37,46 @@ public static class BrushHelper /// internal static global::Windows.UI.Color ParseColor(string color) => _colorCache.GetOrAdd(color, static c => - c.ToLowerInvariant() switch - { - "red" => global::Windows.UI.Color.FromArgb(255, 255, 0, 0), - "green" => global::Windows.UI.Color.FromArgb(255, 0, 128, 0), - "blue" => global::Windows.UI.Color.FromArgb(255, 0, 0, 255), - "white" => global::Windows.UI.Color.FromArgb(255, 255, 255, 255), - "black" => global::Windows.UI.Color.FromArgb(255, 0, 0, 0), - "gray" or "grey" => global::Windows.UI.Color.FromArgb(255, 128, 128, 128), - "lightgray" or "lightgrey" => global::Windows.UI.Color.FromArgb(255, 211, 211, 211), - "transparent" => global::Windows.UI.Color.FromArgb(0, 0, 0, 0), - _ when c.StartsWith('#') => ParseHex(c), - _ => global::Windows.UI.Color.FromArgb(255, 128, 128, 128), - }); + TryParseColor(c, out var parsed) ? parsed : FallbackGray); - internal static global::Windows.UI.Color ParseHex(string hex) + /// + /// The color every unrecognized string silently falls back to. + /// + internal static readonly global::Windows.UI.Color FallbackGray = global::Windows.UI.Color.FromArgb(255, 128, 128, 128); + + /// + /// Parses a color string without the gray fallback, so callers can tell a genuine + /// "gray" from an unrecognized string. Uncached. + /// + /// + /// The REACTOR_DYM_006 analyzer (ColorStringClassifier in Reactor.Analyzers) and + /// the mur check ColorStringLiteralRule carry byte-for-byte copies of this + /// grammar; ColorStringParityTests locks all three together. + /// + internal static bool TryParseColor(string color, out global::Windows.UI.Color parsed) + { + switch (color.ToLowerInvariant()) + { + case "red": parsed = global::Windows.UI.Color.FromArgb(255, 255, 0, 0); return true; + case "green": parsed = global::Windows.UI.Color.FromArgb(255, 0, 128, 0); return true; + case "blue": parsed = global::Windows.UI.Color.FromArgb(255, 0, 0, 255); return true; + case "white": parsed = global::Windows.UI.Color.FromArgb(255, 255, 255, 255); return true; + case "black": parsed = global::Windows.UI.Color.FromArgb(255, 0, 0, 0); return true; + case "gray" or "grey": parsed = global::Windows.UI.Color.FromArgb(255, 128, 128, 128); return true; + case "lightgray" or "lightgrey": parsed = global::Windows.UI.Color.FromArgb(255, 211, 211, 211); return true; + case "transparent": parsed = global::Windows.UI.Color.FromArgb(0, 0, 0, 0); return true; + } + if (color.StartsWith('#')) + return TryParseHex(color, out parsed); + parsed = FallbackGray; + return false; + } + + internal static global::Windows.UI.Color ParseHex(string hex) => + // Fallback to gray for malformed hex, consistent with named color fallback + TryParseHex(hex, out var parsed) ? parsed : FallbackGray; + + private static bool TryParseHex(string hex, out global::Windows.UI.Color parsed) { hex = hex.TrimStart('#'); if (hex.Length == 6 @@ -58,7 +84,8 @@ _ when c.StartsWith('#') => ParseHex(c), && byte.TryParse(hex[2..4], global::System.Globalization.NumberStyles.HexNumber, null, out var g6) && byte.TryParse(hex[4..6], global::System.Globalization.NumberStyles.HexNumber, null, out var b6)) { - return global::Windows.UI.Color.FromArgb(255, r6, g6, b6); + parsed = global::Windows.UI.Color.FromArgb(255, r6, g6, b6); + return true; } if (hex.Length == 8 && byte.TryParse(hex[0..2], global::System.Globalization.NumberStyles.HexNumber, null, out var a8) @@ -66,9 +93,10 @@ _ when c.StartsWith('#') => ParseHex(c), && byte.TryParse(hex[4..6], global::System.Globalization.NumberStyles.HexNumber, null, out var g8) && byte.TryParse(hex[6..8], global::System.Globalization.NumberStyles.HexNumber, null, out var b8)) { - return global::Windows.UI.Color.FromArgb(a8, r8, g8, b8); + parsed = global::Windows.UI.Color.FromArgb(a8, r8, g8, b8); + return true; } - // Fallback to gray for malformed hex, consistent with named color fallback - return global::Windows.UI.Color.FromArgb(255, 128, 128, 128); + parsed = FallbackGray; + return false; } } diff --git a/src/Reactor/Elements/ResourceBuilder.cs b/src/Reactor/Elements/ResourceBuilder.cs index c45a049db..bb43f643c 100644 --- a/src/Reactor/Elements/ResourceBuilder.cs +++ b/src/Reactor/Elements/ResourceBuilder.cs @@ -43,7 +43,12 @@ public sealed class ResourceBuilder private readonly Dictionary _literals = new(); private readonly Dictionary _themeRefs = new(); - /// Sets a color resource override from a hex string or named color. + /// + /// Sets a color resource override from a hex string (#RRGGBB / #AARRGGBB) or named color + /// (see ). Anything else — including a font name under a + /// *FontFamily key — silently stores a gray brush; the REACTOR_DYM_006 analyzer + /// flags constant values that do. + /// public ResourceBuilder Set(string key, string color) { _literals[key] = BrushHelper.Parse(color); diff --git a/tests/Reactor.Tests/AnalyzerTests/ColorStringLiteralAnalyzerTests.cs b/tests/Reactor.Tests/AnalyzerTests/ColorStringLiteralAnalyzerTests.cs new file mode 100644 index 000000000..4b1662936 --- /dev/null +++ b/tests/Reactor.Tests/AnalyzerTests/ColorStringLiteralAnalyzerTests.cs @@ -0,0 +1,498 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.UI.Reactor.Analyzers; +using Xunit; + +namespace Microsoft.UI.Reactor.Tests.AnalyzerTests; + +/// +/// Tests for REACTOR_DYM_006 ( + +/// ): a constant color string that BrushHelper can't parse and +/// so silently renders gray. Unlike DYM_001–005 the flagged code compiles, so compiler-error +/// verification stays ON — every fixed snippet must still compile. +/// +public class ColorStringLiteralAnalyzerTests +{ + // The color-string API surface, minus the FontFamily overload of ResourceBuilder.Set (PR #1370). + internal const string CommonStubs = @" +namespace Microsoft.UI.Xaml.Media +{ + public class Brush { } + public class SolidColorBrush : Brush { } + public class FontFamily { public FontFamily(string familyName) { } } +} +namespace Microsoft.UI.Reactor.Core +{ + public abstract class Element { } + public sealed class ThemeRef { } + public sealed class TextBlockElement : Element { } +} +namespace Microsoft.UI.Reactor +{ + using Microsoft.UI.Reactor.Core; + using Microsoft.UI.Xaml.Media; + public sealed class RichTextRun { } + public static class BrushHelper + { + public static SolidColorBrush Parse(string color) => null!; + } + public static partial class ElementExtensions + { + public static T Background(this T el, string color) where T : Element => el; + public static T Background(this T el, Brush brush) where T : Element => el; + public static T Background(this T el, ThemeRef theme) where T : Element => el; + public static T Foreground(this T el, string color) where T : Element => el; + public static T BorderBrush(this T el, string color) where T : Element => el; + public static T WithBorder(this T el, string color, double thickness = 1) where T : Element => el; + public static RichTextRun Foreground(this RichTextRun run, string color) => run; + public static T Margin(this T el, string label) where T : Element => el; + } + public static class Factories + { + public static TextBlockElement TextBlock(string text) => null!; + } +} +namespace Acme +{ + using Microsoft.UI.Reactor.Core; + public static class ElementExtensions + { + public static T Paint(this T el, string color) where T : Element => el; + } + public static class BrushHelper + { + public static object Parse(string color) => null!; + } +} +"; + + internal const string ResourceBuilderWithoutFontFamily = @" +namespace Microsoft.UI.Reactor.Elements +{ + using Microsoft.UI.Xaml.Media; + public sealed class ResourceBuilder + { + public ResourceBuilder Set(string key, string color) => this; + public ResourceBuilder Set(string key, Brush brush) => this; + public ResourceBuilder Set(string key, double value) => this; + } +} +"; + + internal const string ResourceBuilderWithFontFamily = @" +namespace Microsoft.UI.Reactor.Elements +{ + using Microsoft.UI.Xaml.Media; + public sealed class ResourceBuilder + { + public ResourceBuilder Set(string key, string color) => this; + public ResourceBuilder Set(string key, Brush brush) => this; + public ResourceBuilder Set(string key, double value) => this; + public ResourceBuilder Set(string key, FontFamily fontFamily) => this; + } +} +"; + + private const string Usings = @" +namespace App +{ + using Microsoft.UI.Reactor; + using Microsoft.UI.Reactor.Core; + using Microsoft.UI.Reactor.Elements; + using Microsoft.UI.Xaml.Media; + using static Microsoft.UI.Reactor.Factories; +"; + + private static string Wrap(string members, bool fontFamilyOverload = false) => + CommonStubs + + (fontFamilyOverload ? ResourceBuilderWithFontFamily : ResourceBuilderWithoutFontFamily) + + Usings + + " static class C\n {\n" + members + "\n }\n}\n"; + + private static CSharpAnalyzerTest MakeTest(string code) + { + var test = new CSharpAnalyzerTest { TestCode = code }; + test.DisabledDiagnostics.Add("CS1591"); + return test; + } + + private static Task RunAsync(string members, bool fontFamilyOverload = false) => + MakeTest(Wrap(members, fontFamilyOverload)).RunAsync(TestContext.Current.CancellationToken); + + private static DiagnosticResult Dym006(int markup) => + new DiagnosticResult(ColorStringLiteralAnalyzer.DiagnosticId, Microsoft.CodeAnalysis.DiagnosticSeverity.Warning) + .WithLocation(markup); + + // ── Positives: every covered API ───────────────────────────────────────────────────────────── + + [Fact] + public async Task Flags_Background_with_css_name_and_suggests_hex() + { + var test = MakeTest(Wrap(@" static object M() => TextBlock(""hi"").Background({|#0:""orange""|});")); + test.ExpectedDiagnostics.Add(Dym006(0).WithArguments( + "orange", + "the only built-in names are red, green, blue, white, black, gray/grey, lightgray/lightgrey, transparent; did you mean \"#FFA500\"?")); + await test.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public Task Flags_Foreground_hex_shorthand() => + RunAsync(@" static object M() => TextBlock(""hi"").Foreground({|REACTOR_DYM_006:""#888""|});"); + + [Fact] + public Task Flags_BorderBrush_missing_hash() => + RunAsync(@" static object M() => TextBlock(""hi"").BorderBrush({|REACTOR_DYM_006:""FF0000""|});"); + + [Fact] + public Task Flags_WithBorder_malformed_hex() => + RunAsync(@" static object M() => TextBlock(""hi"").WithBorder({|REACTOR_DYM_006:""#12345""|}, 2);"); + + [Fact] + public Task Flags_RichTextRun_Foreground() => + RunAsync(@" static object M(RichTextRun run) => run.Foreground({|REACTOR_DYM_006:""Yellow""|});"); + + [Fact] + public Task Flags_BrushHelper_Parse() => + RunAsync(@" static object M() => BrushHelper.Parse({|REACTOR_DYM_006:""AliceBlue""|});"); + + [Fact] + public Task Flags_ResourceBuilder_Set_color_key() => + RunAsync(@" static object M(ResourceBuilder r) => r.Set(""ButtonBackground"", {|REACTOR_DYM_006:""LightCoral""|});"); + + [Fact] + public Task Flags_named_argument() => + RunAsync(@" static object M() => TextBlock(""hi"").Background(color: {|REACTOR_DYM_006:""orange""|});"); + + [Fact] + public async Task Flags_unknown_name_with_supported_list() + { + var test = MakeTest(Wrap(@" static object M() => TextBlock(""hi"").Background({|#0:""accent""|});")); + test.ExpectedDiagnostics.Add(Dym006(0).WithArguments( + "accent", + "use #RRGGBB or #AARRGGBB, a Theme token, or one of red, green, blue, white, black, gray/grey, lightgray/lightgrey, transparent")); + await test.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public Task Flags_empty_string() => + RunAsync(@" static object M() => TextBlock(""hi"").Background({|REACTOR_DYM_006:""""|});"); + + // ── Positives: constant shapes ─────────────────────────────────────────────────────────────── + + [Fact] + public Task Flags_const_reference() => + RunAsync(@" const string Brand = ""orange""; + static object M() => TextBlock(""hi"").Background({|REACTOR_DYM_006:Brand|});"); + + [Fact] + public Task Flags_constant_concatenation() => + RunAsync(@" const string Hash = ""#""; + static object M() => TextBlock(""hi"").Background({|REACTOR_DYM_006:Hash + ""888""|});"); + + [Fact] + public Task Flags_only_the_unparseable_conditional_branch() => + RunAsync(@" static object M(bool on) => TextBlock(""hi"").Background(on ? ""red"" : {|REACTOR_DYM_006:""orange""|});"); + + [Fact] + public Task Flags_coalesce_and_switch_branches() => + RunAsync(@" static object M(string s, int i) => TextBlock(""hi"") + .Background(s ?? {|REACTOR_DYM_006:""teal""|}) + .Foreground(i switch { 0 => ""black"", _ => {|REACTOR_DYM_006:""navy""|} });"); + + // ── FontFamily key ────────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task Flags_FontFamily_key_and_suggests_FontFamily_when_overload_exists() + { + var test = MakeTest(Wrap( + @" static object M(ResourceBuilder r) => r.Set(""ContentControlThemeFontFamily"", {|#0:""Consolas""|});", + fontFamilyOverload: true)); + test.ExpectedDiagnostics.Add(Dym006(0).WithArguments( + "Consolas", + "'ContentControlThemeFontFamily' is a FontFamily resource; did you mean new Microsoft.UI.Xaml.Media.FontFamily(\"Consolas\")?")); + await test.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Flags_FontFamily_key_without_suggestion_when_overload_missing() + { + var test = MakeTest(Wrap( + @" static object M(ResourceBuilder r) => r.Set(""ContentControlThemeFontFamily"", {|#0:""Consolas""|});")); + test.ExpectedDiagnostics.Add(Dym006(0).WithArguments( + "Consolas", + "'ContentControlThemeFontFamily' is a FontFamily resource, but this Set overload only stores color brushes; to change the font, use .FontFamily(\"Consolas\") on the element")); + await test.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task FontFamily_key_inside_a_branch_gets_no_FontFamily_suggestion() + { + // new FontFamily(...) can't replace one branch of a ?: — the other branch is still a string. + var test = MakeTest(Wrap( + @" static object M(ResourceBuilder r, bool on) => r.Set(""ContentControlThemeFontFamily"", on ? {|#0:""Consolas""|} : ""red"");", + fontFamilyOverload: true)); + test.ExpectedDiagnostics.Add(Dym006(0).WithArguments( + "Consolas", + "'ContentControlThemeFontFamily' is a FontFamily resource, but this Set overload only stores color brushes; to change the font, use .FontFamily(\"Consolas\") on the element")); + await test.RunAsync(TestContext.Current.CancellationToken); + } + + // ── Negatives ─────────────────────────────────────────────────────────────────────────────── + + [Fact] + public Task Silent_on_every_supported_name_in_any_casing() => + RunAsync(@" static object M() => TextBlock(""hi"") + .Background(""red"").Background(""Green"").Background(""BLUE"").Background(""White"") + .Background(""black"").Background(""Gray"").Background(""grey"").Background(""LightGray"") + .Background(""lightgrey"").Background(""Transparent"");"); + + [Fact] + public Task Silent_on_valid_hex() => + RunAsync(@" static object M() => TextBlock(""hi"") + .Background(""#FF0000"").Foreground(""#80ff0000"").BorderBrush(""#aAbBcC"").WithBorder(""#00000000"");"); + + [Fact] + public Task Silent_on_non_constant_values() => + RunAsync(@" static object M(string c, int n) => TextBlock(""hi"") + .Background(c).Foreground($""#{n:X6}"").BorderBrush(c + ""00"");"); + + [Fact] + public Task Silent_on_brush_and_theme_overloads_and_non_color_params() => + RunAsync(@" static object M(Brush b, ThemeRef t, ResourceBuilder r) + { + r.Set(""ButtonBorderThemeThickness"", 2.0); + r.Set(""ButtonBackground"", ""#0078D4""); + r.Set(""ContentControlThemeFontFamily"", b); + return TextBlock(""orange"").Background(b).Background(t).Margin(""orange""); + }"); + + [Fact] + public Task Silent_on_lookalike_apis_outside_Reactor() => + RunAsync(@" static object M() => Acme.BrushHelper.Parse(""orange"") ?? Acme.ElementExtensions.Paint(TextBlock(""hi""), ""orange"");"); + + [Fact] + public Task Silent_on_FontFamily_key_with_a_valid_color() => + RunAsync(@" static object M(ResourceBuilder r) => r.Set(""SomethingFontFamily"", ""red"");", fontFamilyOverload: true); + + [Fact] + public Task Silent_on_unreachable_constant_branch() => + // The whole expression is a constant ("red"), so the dead "orange" branch is never parsed. + RunAsync(@" static object M() => TextBlock(""hi"").Background(true ? ""red"" : ""orange"");"); + + [Fact] + public Task Silent_on_dead_branches_the_compiler_does_not_fold() => + // Each expression as a whole is non-constant, so only the constant condition / left side + // proves which branch runs. The live branches are still checked. + RunAsync(@" const bool DebugBuild = false; + const string Mode = ""dark""; + static object M(string c) => TextBlock(""hi"") + .Background(DebugBuild ? ""magenta"" : c) + .Foreground(!DebugBuild ? c : ""magenta"") + .BorderBrush(""red"" ?? c) + .WithBorder(Mode switch { ""light"" => ""orange"", ""dark"" => c, _ => ""teal"" }) + .Background(DebugBuild ? c : {|REACTOR_DYM_006:""navy""|});"); + + [Fact] + public Task Non_constant_switch_keeps_every_arm() => + RunAsync(@" static object M(string mode, string c) => TextBlock(""hi"") + .Background(mode switch { ""light"" => {|REACTOR_DYM_006:""orange""|}, _ => c });"); + + [Fact] + public Task Coalesce_fallback_is_dead_when_a_nested_selector_is_provably_non_null() => + RunAsync(@" const bool DebugBuild = false; + static object M(string c, bool on) => TextBlock(""hi"") + .Background((DebugBuild ? c : ""red"") ?? ""orange"") + .Foreground((on ? ""red"" : ""white"") ?? ""teal"") + .BorderBrush((on ? c : ""red"") ?? {|REACTOR_DYM_006:""navy""|});"); + + [Fact] + public Task Silent_through_a_user_defined_conversion() => + // `token ?? "orange"` is a ColorToken; its operator decides the string BrushHelper receives, + // so "orange" is never parsed. Positive control: once the coalesce is over strings, it is. + RunAsync(@" sealed class ColorToken + { + public static implicit operator ColorToken(string s) => new ColorToken(); + public static implicit operator string(ColorToken t) => ""red""; + } + static object M(ColorToken token, bool on) => BrushHelper.Parse(token ?? ""orange"") + ?? BrushHelper.Parse(on switch { true => token ?? ""orange"", _ => ""red"" }) + ?? BrushHelper.Parse((string)token ?? {|REACTOR_DYM_006:""orange""|});"); + + [Fact] + public Task Coalesce_over_a_one_way_user_defined_conversion_is_not_provably_non_null() => + // `token ?? "red"` is a string, but a non-null token converts through an operator that may + // return null, so the outer "orange" fallback is reachable. + RunAsync(@" sealed class NullableToken + { + public static implicit operator string(NullableToken t) => null!; + } + static object M(NullableToken token) => TextBlock(""hi"").Background((token ?? ""red"") ?? {|REACTOR_DYM_006:""orange""|});"); + + [Fact] + public Task Coalesce_fallback_is_dead_when_the_other_branch_throws() => + // Execution either yields "red" or throws, so "orange" is unreachable. + RunAsync(@" static object M(string c, bool on) => TextBlock(""hi"") + .Background((on ? ""red"" : throw new global::System.InvalidOperationException()) ?? ""orange"") + .Foreground((on ? c : throw new global::System.InvalidOperationException()) ?? {|REACTOR_DYM_006:""teal""|});"); + + [Fact] + public Task Switch_pruning_compares_the_converted_pattern_constant() => + // `1` is an int literal but the pattern converts it to the governing `long`. + RunAsync(@" const long Mode = 1; + static object M(string c) => TextBlock(""hi"") + .Background(Mode switch { 1 => c, _ => ""orange"" }) + .Foreground(Mode switch { 2 => c, _ => {|REACTOR_DYM_006:""teal""|} });"); + + [Fact] + public async Task FontFamily_suggestion_renames_a_named_argument_and_escapes_the_value() + { + var test = MakeTest(Wrap( + @" static object M(ResourceBuilder r) => r.Set(key: ""ContentControlThemeFontFamily"", color: {|#0:@""C:\Fonts\Mono.ttf#Mono""|});", + fontFamilyOverload: true)); + test.ExpectedDiagnostics.Add(Dym006(0).WithArguments( + @"C:\Fonts\Mono.ttf#Mono", + @"'ContentControlThemeFontFamily' is a FontFamily resource; did you mean fontFamily: new Microsoft.UI.Xaml.Media.FontFamily(""C:\\Fonts\\Mono.ttf#Mono"")?")); + await test.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Silent_without_any_Reactor_color_api() + { + var test = MakeTest(@"static class C { static string M() => ""orange""; }"); + await test.RunAsync(TestContext.Current.CancellationToken); + } + + // ── Live Reactor surface ──────────────────────────────────────────────────────────────────── + // Every other test runs against stubs that hardcode `string color`. This one binds to the real + // Reactor assembly, so renaming a `color` parameter or moving a type (which would silently turn + // the analyzer off) fails here. + + [Fact] + public async Task Fires_on_every_covered_api_of_the_live_Reactor_assembly() + { + var live = Microsoft.UI.Reactor.Tests.CheckCommandTests.Rules.RuleTargetResolutionTests.BuildLiveReactorCompilation(); + + var targets = ColorStringLiteralAnalyzer.Targets.Resolve(live); + Assert.NotNull(targets); + var covered = targets!.Methods + .Select(m => $"{m.ContainingType.Name}.{m.Name}({string.Join(", ", m.Parameters.Select(p => p.Type.Name + " " + p.Name))})") + .OrderBy(s => s, global::System.StringComparer.Ordinal) + .ToArray(); + Assert.Equal(new[] + { + "BrushHelper.Parse(String color)", + "ElementExtensions.Background(T el, String color)", + "ElementExtensions.BorderBrush(T el, String color)", + "ElementExtensions.Foreground(RichTextBlockElement el, String color)", + "ElementExtensions.Foreground(RichTextHyperlink link, String color)", + "ElementExtensions.Foreground(RichTextParagraph paragraph, String color)", + "ElementExtensions.Foreground(RichTextRun run, String color)", + "ElementExtensions.Foreground(T el, String color)", + "ElementExtensions.WithBorder(T el, String color, Double thickness)", + "ResourceBuilder.Set(String key, String color)", + }, covered); + + var source = Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTree.ParseText(@" +using Microsoft.UI.Reactor; +using Microsoft.UI.Reactor.Elements; +using static Microsoft.UI.Reactor.Factories; +static class LiveProbe +{ + static object A() => TextBlock(""x"").Background(""orange""); + static object B() => new ResourceBuilder().Set(""ButtonBackground"", ""#888""); + static object C() => BrushHelper.Parse(""FF0000""); + static object D() => TextBlock(""x"").Foreground(""red"").BorderBrush(""#80FF0000""); +}", cancellationToken: TestContext.Current.CancellationToken); + var compilation = live.AddSyntaxTrees(source); + Assert.Empty(compilation.GetDiagnostics(TestContext.Current.CancellationToken) + .Where(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error && d.Location.SourceTree == source)); + + var diagnostics = await compilation + .WithAnalyzers(global::System.Collections.Immutable.ImmutableArray.Create(new ColorStringLiteralAnalyzer())) + .GetAnalyzerDiagnosticsAsync(TestContext.Current.CancellationToken); + Assert.Equal( + new[] { "orange", "#888", "FF0000" }, + diagnostics.Where(d => d.Id == ColorStringLiteralAnalyzer.DiagnosticId) + .OrderBy(d => d.Location.SourceSpan.Start) + .Select(d => d.Properties.ContainsKey(ColorStringLiteralAnalyzer.ReplacementProperty) ? source.GetText(TestContext.Current.CancellationToken).ToString(d.Location.SourceSpan).Trim('"') : "?")); + } + + // ── Code fixes ────────────────────────────────────────────────────────────────────────────── + + private static Task RunFixAsync(string before, string after, bool fontFamilyOverload = false) + { + var test = new CSharpCodeFixTest + { + TestCode = Wrap(before, fontFamilyOverload), + FixedCode = Wrap(after, fontFamilyOverload), + }; + test.DisabledDiagnostics.Add("CS1591"); + return test.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public Task Fix_expands_hex_shorthand() => + RunFixAsync( + @" static object M() => TextBlock(""hi"").Foreground({|REACTOR_DYM_006:""#888""|}).Background({|REACTOR_DYM_006:""#8abc""|});", + @" static object M() => TextBlock(""hi"").Foreground(""#888888"").Background(""#88aabbcc"");"); + + [Fact] + public Task Fix_adds_missing_hash() => + RunFixAsync( + @" static object M() => TextBlock(""hi"").Background({|REACTOR_DYM_006:""FF0000""|});", + @" static object M() => TextBlock(""hi"").Background(""#FF0000"");"); + + [Fact] + public Task Fix_maps_css_name_to_hex_including_inside_a_branch() => + RunFixAsync( + @" static object M(bool on) => BrushHelper.Parse(on ? {|REACTOR_DYM_006:""AliceBlue""|} : ""white"");", + @" static object M(bool on) => BrushHelper.Parse(on ? ""#F0F8FF"" : ""white"");"); + + [Fact] + public Task Fix_rewrites_FontFamily_key_to_FontFamily_overload() => + RunFixAsync( + @" static object M(ResourceBuilder r) => r.Set(""ContentControlThemeFontFamily"", {|REACTOR_DYM_006:""Consolas""|});", + @" static object M(ResourceBuilder r) => r.Set(""ContentControlThemeFontFamily"", new FontFamily(""Consolas""));", + fontFamilyOverload: true); + + [Fact] + public Task Fix_wraps_const_FontFamily_reference() => + RunFixAsync( + @" const string Mono = ""Consolas""; + static object M(ResourceBuilder r) => r.Set(""ContentControlThemeFontFamily"", {|REACTOR_DYM_006:Mono|});", + @" const string Mono = ""Consolas""; + static object M(ResourceBuilder r) => r.Set(""ContentControlThemeFontFamily"", new FontFamily(Mono));", + fontFamilyOverload: true); + + [Fact] + public Task Fix_renames_named_color_argument_to_the_FontFamily_parameter() => + RunFixAsync( + @" static object M(ResourceBuilder r) => r.Set(key: ""ContentControlThemeFontFamily"", color: {|REACTOR_DYM_006:""Consolas""|});", + @" static object M(ResourceBuilder r) => r.Set(key: ""ContentControlThemeFontFamily"", fontFamily: new FontFamily(""Consolas""));", + fontFamilyOverload: true); + + [Fact] + public Task No_FontFamily_fix_inside_a_branch() => + RunFixAsync( + @" static object M(ResourceBuilder r, bool on) => r.Set(""ContentControlThemeFontFamily"", on ? {|REACTOR_DYM_006:""Consolas""|} : ""red"");", + @" static object M(ResourceBuilder r, bool on) => r.Set(""ContentControlThemeFontFamily"", on ? {|REACTOR_DYM_006:""Consolas""|} : ""red"");", + fontFamilyOverload: true); + + [Fact] + public Task No_fix_without_FontFamily_overload_or_for_unknown_or_const_color() => + RunFixAsync( + @" const string Brand = ""orange""; + static object M(ResourceBuilder r) + { + r.Set(""ContentControlThemeFontFamily"", {|REACTOR_DYM_006:""Consolas""|}); + return TextBlock(""hi"").Background({|REACTOR_DYM_006:""accent""|}).Foreground({|REACTOR_DYM_006:Brand|}).BorderBrush({|REACTOR_DYM_006:""#12345""|}); + }", + @" const string Brand = ""orange""; + static object M(ResourceBuilder r) + { + r.Set(""ContentControlThemeFontFamily"", {|REACTOR_DYM_006:""Consolas""|}); + return TextBlock(""hi"").Background({|REACTOR_DYM_006:""accent""|}).Foreground({|REACTOR_DYM_006:Brand|}).BorderBrush({|REACTOR_DYM_006:""#12345""|}); + }"); +} diff --git a/tests/Reactor.Tests/AnalyzerTests/ColorStringParityTests.cs b/tests/Reactor.Tests/AnalyzerTests/ColorStringParityTests.cs new file mode 100644 index 000000000..27851f303 --- /dev/null +++ b/tests/Reactor.Tests/AnalyzerTests/ColorStringParityTests.cs @@ -0,0 +1,111 @@ +using AnalyzerClassifier = Microsoft.UI.Reactor.Analyzers.ColorStringClassifier; +using Xunit; + +namespace Microsoft.UI.Reactor.Tests.AnalyzerTests; + +/// +/// REACTOR_DYM_006 parity lock (issue #1375). Two copies of one color grammar must agree: +/// BrushHelper.TryParseColor (the runtime truth) and the netstandard2.0 analyzer's +/// ColorStringClassifier. If they drift, the analyzer either flags colors that render fine or +/// misses ones that render gray. (The mur check rule reads the analyzer's message, so it has no +/// grammar of its own; DidYouMeanAnalyzerParityTests covers it.) +/// +public class ColorStringParityTests +{ + // Covers every branch of BrushHelper's grammar, including its quirks: case folding, the + // '#'-only hex route, TrimStart('#') swallowing repeated hashes, and NumberStyles.HexNumber + // accepting leading/trailing whitespace inside a byte pair. + public static TheoryData Battery => new() + { + "", " ", "red", "RED", "Red", "green", "blue", "white", "black", "gray", "GREY", "lightgray", + "LightGrey", "transparent", "Transparent", " red", "red ", "reds", "orange", "AliceBlue", + "aliceblue", "LightCoral", "Yellow", "slategrey", "rebeccapurple", "accent", "Consolas", + "Segoe UI", "#", "##", "#FF0000", "#ff0000", "#aAbBcC", "#80FF0000", "#00000000", "##FF0000", + "###FF0000", "#FFF", "#ffff", "#8abc", "#12345", "#1234567", "#123456789", "#GGHHII", + "#FF00GG", "#0 0000", "# 00000", "#00000 ", "# FF0000", "#-10000", "#+10000", "#0x1234", + "FF0000", "80FF0000", "ff00ff", "FFF", "decade", "#FF0000\n", "\u0130", "#\u0661\u0662\u0663\u0664\u0665\u0666", + }; + + [Theory] + [MemberData(nameof(Battery))] + public void Analyzer_TryParse_matches_BrushHelper(string value) + { + var runtime = BrushHelper.TryParseColor(value, out _); + Assert.Equal(runtime, AnalyzerClassifier.TryParse(value)); + // Classify must agree too: a finding exactly when BrushHelper falls back to gray. + Assert.Equal(!runtime, AnalyzerClassifier.Classify(value, null, false).HasValue); + } + + [Fact] + public void Every_fixable_detail_ends_in_the_did_you_mean_the_cli_rule_reads() + { + // ColorStringLiteralRule lifts the rewrite out of "...; did you mean X?". Keep the suffix stable. + foreach (var (value, key, overload) in new (string, string?, bool)[] + { + ("#888", null, false), ("FF0000", null, false), ("orange", null, false), + ("Consolas", "ContentControlThemeFontFamily", true), + }) + { + var finding = AnalyzerClassifier.Classify(value, key, overload)!.Value; + Assert.NotNull(finding.Replacement); + Assert.EndsWith("; did you mean " + (finding.Issue.ToString() == "FontFamilyKey" + ? AnalyzerClassifier.FontFamilySuggestion(finding.Replacement!, null) + : $"\"{finding.Replacement}\"") + "?", finding.Detail, StringComparison.Ordinal); + } + } + + [Theory] + [InlineData("Consolas", null, "new Microsoft.UI.Xaml.Media.FontFamily(\"Consolas\")")] + [InlineData("Consolas", "fontFamily", "fontFamily: new Microsoft.UI.Xaml.Media.FontFamily(\"Consolas\")")] + [InlineData(@"C:\Fonts\a""b.ttf#A", null, @"new Microsoft.UI.Xaml.Media.FontFamily(""C:\\Fonts\\a\""b.ttf#A"")")] + public void FontFamily_suggestion_is_compilable_CSharp(string value, string? argumentName, string expected) + { + Assert.Equal(expected, AnalyzerClassifier.FontFamilySuggestion(value, argumentName)); + // And it round-trips: the literal parses back to the original value. + var creation = (Microsoft.CodeAnalysis.CSharp.Syntax.ObjectCreationExpressionSyntax) + Microsoft.CodeAnalysis.CSharp.SyntaxFactory.ParseExpression(AnalyzerClassifier.FontFamilySuggestion(value, null)); + Assert.Empty(creation.GetDiagnostics()); + var literal = (Microsoft.CodeAnalysis.CSharp.Syntax.LiteralExpressionSyntax)creation.ArgumentList!.Arguments[0].Expression; + Assert.Equal(value, literal.Token.ValueText); + } + + [Fact] + public void Supported_names_list_matches_BrushHelper() + { + foreach (var name in AnalyzerClassifier.SupportedNames.Split(", ").SelectMany(n => n.Split('/'))) + Assert.True(BrushHelper.TryParseColor(name, out _), $"'{name}' is listed as supported but BrushHelper rejects it."); + } + + [Fact] + public void Every_named_color_is_unsupported_by_BrushHelper_and_maps_to_its_exact_hex() + { + Assert.True(AnalyzerClassifier.NamedColorHex.Count >= 130, $"Only {AnalyzerClassifier.NamedColorHex.Count} names."); + foreach (var (name, hex) in AnalyzerClassifier.NamedColorHex) + { + // A name BrushHelper already parses would be a false positive. + Assert.False(BrushHelper.TryParseColor(name, out _), $"BrushHelper already parses '{name}'."); + + // The suggested hex must parse, and must be the color the name means. System.Drawing's + // KnownColor table is an independent oracle (it spells the grey aliases 'gray'). + Assert.True(BrushHelper.TryParseColor(hex, out var parsed), $"'{hex}' for '{name}' does not parse."); + var known = global::System.Drawing.Color.FromName(name.Replace("grey", "gray", StringComparison.Ordinal)); + Assert.True(known.IsKnownColor, $"'{name}' is not a known color."); + Assert.Equal((known.R, known.G, known.B), (parsed.R, parsed.G, parsed.B)); + Assert.Equal(255, parsed.A); + } + } + + [Theory] + [InlineData("#888", "#888888")] + [InlineData("#8abc", "#88aabbcc")] + [InlineData("FF0000", "#FF0000")] + [InlineData("80FF0000", "#80FF0000")] + [InlineData("orange", "#FFA500")] + public void Every_literal_replacement_actually_parses(string value, string expected) + { + var finding = AnalyzerClassifier.Classify(value, null, false); + Assert.NotNull(finding); + Assert.Equal(expected, finding!.Value.Replacement); + Assert.True(BrushHelper.TryParseColor(expected, out _)); + } +} diff --git a/tests/Reactor.Tests/AnalyzerTests/DidYouMeanAnalyzerParityTests.cs b/tests/Reactor.Tests/AnalyzerTests/DidYouMeanAnalyzerParityTests.cs index 9a930e8cc..920d7edfe 100644 --- a/tests/Reactor.Tests/AnalyzerTests/DidYouMeanAnalyzerParityTests.cs +++ b/tests/Reactor.Tests/AnalyzerTests/DidYouMeanAnalyzerParityTests.cs @@ -10,6 +10,7 @@ using Microsoft.UI.Reactor.Cli.Check.Rules; using Microsoft.UI.Reactor.Cli.Check.Suggesters; using Microsoft.UI.Reactor.Tests.CheckCommandTests; +using Microsoft.UI.Reactor.Tests.CheckCommandTests.Rules; using Xunit; namespace Microsoft.UI.Reactor.Tests.AnalyzerTests; @@ -167,4 +168,33 @@ private static SuggestionResult RunSymbolSuggester(string userCode, string code) var ctx = new SuggesterContext(comp, diag, pick, receiver, FactoryIndex.Build(comp)); return new SymbolSuggester().Suggest(ctx); } + + // ── REACTOR_DYM_006 (ColorStringLiteralAnalyzer) <-> ColorStringLiteralRule ── + // + // The analyzer's code-fix replacement and the rule's `→ try:` text must name the same rewrite. + // Driven end-to-end: the real analyzer reports, its diagnostic is fed to the rule. + + [Theory] + // `expected` is the source text the IDE code fix writes in place of the flagged argument (the + // code-fix round-trips in ColorStringLiteralAnalyzerTests pin that side); the rule must suggest it. + [InlineData(@"TextBlock(""hi"").Foreground(""#888"")", false, @"""#888888""")] + [InlineData(@"TextBlock(""hi"").Background(""ff00ff"")", false, @"""#ff00ff""")] + [InlineData(@"BrushHelper.Parse(""orange"")", false, @"""#FFA500""")] + [InlineData(@"new ResourceBuilder().Set(""ButtonBackground"", ""Wheat"")", false, @"""#F5DEB3""")] + [InlineData(@"new ResourceBuilder().Set(""ContentControlThemeFontFamily"", ""Consolas"")", true, @"new Microsoft.UI.Xaml.Media.FontFamily(""Consolas"")")] + // A named argument must be renamed to bind to the FontFamily overload. + [InlineData(@"new ResourceBuilder().Set(key: ""ContentControlThemeFontFamily"", color: ""Consolas"")", true, @"fontFamily: new Microsoft.UI.Xaml.Media.FontFamily(""Consolas"")")] + // Backslashes and quotes are escaped as a regular C# literal, whatever form the source used. + [InlineData(@"new ResourceBuilder().Set(""ContentControlThemeFontFamily"", @""Assets\Fonts\Mono.ttf#Mono """"x"""""")", true, @"new Microsoft.UI.Xaml.Media.FontFamily(""Assets\\Fonts\\Mono.ttf#Mono \""x\"""")")] + public void Color_string_analyzer_replacement_matches_cli_rule(string expression, bool fontFamilyOverload, string expected) + { + var (diagnostics, c) = ColorStringLiteralRuleTests.AnalyzeRaw(expression, fontFamilyOverload); + var d = Assert.Single(diagnostics); + Assert.False(string.IsNullOrEmpty(d.Properties[ColorStringLiteralAnalyzer.ReplacementProperty])); + + var s = ColorStringLiteralRuleTests.RunRule(ColorStringLiteralRuleTests.ToCheckDiag(d), c); + Assert.NotNull(s); + Assert.Equal("ColorStringLiteralRule", s!.SuggesterName); + Assert.Equal(expected, s.Text); + } } diff --git a/tests/Reactor.Tests/CheckCommandTests/Rules/ColorStringLiteralRuleTests.cs b/tests/Reactor.Tests/CheckCommandTests/Rules/ColorStringLiteralRuleTests.cs new file mode 100644 index 000000000..ad444f32f --- /dev/null +++ b/tests/Reactor.Tests/CheckCommandTests/Rules/ColorStringLiteralRuleTests.cs @@ -0,0 +1,219 @@ +// Tests for ColorStringLiteralRule — the `mur check` mirror of REACTOR_DYM_006 +// (issue #1375). +// +// There is no compiler error for this mistake, so the rule anchors on the +// analyzer's own diagnostic. These tests therefore run the REAL analyzer over +// a stub compilation, turn each REACTOR_DYM_006 it reports into the +// MSBuild-shaped `CheckCommand.Diag` that `mur check` parses, and drive the +// orchestrator with it — the same path a `dotnet build` line takes. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.UI.Reactor.Analyzers; +using Microsoft.UI.Reactor.Cli.Check; +using Microsoft.UI.Reactor.Cli.Check.Rules; +using Microsoft.UI.Reactor.Tests.AnalyzerTests; +using Xunit; + +namespace Microsoft.UI.Reactor.Tests.CheckCommandTests.Rules; + +public class ColorStringLiteralRuleTests +{ + [Theory] + [InlineData(@"TextBlock(""hi"").Background(""orange"")", "\"#FFA500\"")] + [InlineData(@"TextBlock(""hi"").Foreground(""#888"")", "\"#888888\"")] + [InlineData(@"TextBlock(""hi"").BorderBrush(""FF0000"")", "\"#FF0000\"")] + [InlineData(@"BrushHelper.Parse(""AliceBlue"")", "\"#F0F8FF\"")] + [InlineData(@"new ResourceBuilder().Set(""ButtonBackground"", ""LightCoral"")", "\"#F08080\"")] + [InlineData(@"TextBlock(""hi"").Background(Brand)", "\"#FFA500\"")] + public void Fires_with_the_concrete_rewrite(string expression, string expected) + { + var (diags, c) = Analyze(expression); + var diag = Assert.Single(diags); + var s = RunRule(diag, c); + Assert.NotNull(s); + Assert.Equal("ColorStringLiteralRule", s!.SuggesterName); + Assert.True(s.IsRule); + Assert.Equal(expected, s.Text); + Assert.Contains("analyzer:REACTOR_DYM_006", s.Evidence); + } + + [Fact] + public void Reads_the_rewrite_from_the_diagnostic_message_not_the_source() + { + // The rule trusts the analyzer's message: a message from another source shape still yields its + // own did-you-mean, and a REACTOR_DYM_006 whose message carries none stays silent. + var (diags, c) = Analyze(@"TextBlock(""hi"").Background(""orange"")"); + var diag = Assert.Single(diags); + var forged = diag with { Message = "'teal' is not a color string Reactor can parse, so it renders gray — the only built-in names are red; did you mean \"#008080\"?" }; + Assert.Equal("\"#008080\"", RunRule(forged, c)!.Text); + Assert.Null(RunRule(diag with { Message = "something else entirely" }, c)); + } + + [Fact] + public void Suggests_FontFamily_when_the_overload_exists() + { + var (diags, c) = Analyze(@"new ResourceBuilder().Set(""ContentControlThemeFontFamily"", ""Consolas"")", fontFamilyOverload: true); + var s = RunRule(Assert.Single(diags), c); + Assert.NotNull(s); + Assert.Equal("new Microsoft.UI.Xaml.Media.FontFamily(\"Consolas\")", s!.Text); + } + + [Fact] + public void Without_the_FontFamily_overload_the_modifier_guidance_survives_mur_check_truncation() + { + // No rewrite of the argument exists, but the message's `.FontFamily(...)` workaround must still + // reach the agent: CheckCommand truncates messages to 100 chars, which cuts it off. + var (diags, c) = Analyze(@"new ResourceBuilder().Set(""ContentControlThemeFontFamily"", ""Consolas"")"); + var diag = Assert.Single(diags); + var s = RunRule(diag, c); + Assert.NotNull(s); + Assert.Equal(".FontFamily(\"Consolas\")", s!.Text); + + var line = diag.Format(s); + Assert.DoesNotContain("to change the font", line, StringComparison.Ordinal); // truncated away... + Assert.Contains("→ try: .FontFamily(\"Consolas\")", line, StringComparison.Ordinal); // ...but surfaced here + } + + [Theory] + // No concrete next step exists for these — the analyzer's message carries the advice. + [InlineData(@"TextBlock(""hi"").Background(""accent"")")] + [InlineData(@"TextBlock(""hi"").Background(""#12345"")")] + public void Silent_when_there_is_no_concrete_rewrite(string expression) + { + var (diags, c) = Analyze(expression); + Assert.Null(RunRule(Assert.Single(diags), c)); + } + + [Fact] + public void Each_branch_gets_its_own_rewrite() + { + var (diags, c) = Analyze(@"TextBlock(""hi"").Background(on ? ""orange"" : ""teal"")"); + Assert.Equal(2, diags.Length); + Assert.Equal(new[] { "\"#FFA500\"", "\"#008080\"" }, diags.Select(d => RunRule(d, c)!.Text)); + + // A branch with no rewrite doesn't silence its fixable sibling. + (diags, c) = Analyze(@"TextBlock(""hi"").Background(on ? ""orange"" : ""accent"")"); + Assert.Equal(new[] { "\"#FFA500\"", null }, diags.Select(d => RunRule(d, c)?.Text)); + } + + [Fact] + public void FontFamily_value_inside_a_branch_gets_no_FontFamily_suggestion() + { + // Matches the code fix, which can't wrap one branch of a ?: in new FontFamily(...); the element + // modifier is the remaining advice. + var (diags, c) = Analyze(@"new ResourceBuilder().Set(""ContentControlThemeFontFamily"", on ? ""Consolas"" : ""red"")", fontFamilyOverload: true); + Assert.Equal(".FontFamily(\"Consolas\")", RunRule(Assert.Single(diags), c)?.Text); + } + + [Fact] + public void Silent_on_other_diagnostic_codes() + { + var (diags, c) = Analyze(@"TextBlock(""hi"").Background(""orange"")"); + var other = Assert.Single(diags) with { Code = "REACTOR_THEME_001" }; + Assert.Null(RunRule(other, c)); + } + + [Fact] + public void Silent_when_the_targets_are_missing() + { + // Same source, but the Reactor color APIs were renamed (API churn): the rule self-disables + // instead of guessing. Positive control first so the null below isn't a mis-anchored span. + var (diags, c) = Analyze(@"TextBlock(""hi"").Background(""orange"")"); + var diag = Assert.Single(diags); + Assert.NotNull(RunRule(diag, c)); + + var renamed = c.ReplaceSyntaxTree( + c.SyntaxTrees.First(t => t.FilePath == "Stub.cs"), + CSharpSyntaxTree.ParseText( + ColorStringLiteralAnalyzerTests.CommonStubs + .Replace("public static partial class ElementExtensions", "public static partial class RenamedExtensions", StringComparison.Ordinal) + .Replace("public static class BrushHelper\r\n {\r\n public static SolidColorBrush", "public static class RenamedBrushHelper\r\n {\r\n public static SolidColorBrush", StringComparison.Ordinal) + .Replace("public static class BrushHelper\n {\n public static SolidColorBrush", "public static class RenamedBrushHelper\n {\n public static SolidColorBrush", StringComparison.Ordinal), + path: "Stub.cs", + cancellationToken: TestContext.Current.CancellationToken)); + Assert.Null(renamed.GetTypeByMetadataName("Microsoft.UI.Reactor.ElementExtensions")); + Assert.Null(renamed.GetTypeByMetadataName("Microsoft.UI.Reactor.BrushHelper")); + Assert.Null(RunRule(diag, renamed)); + } + + [Theory] + [InlineData(@"TextBlock(""hi"").Background(""orange"")", false)] + [InlineData(@"TextBlock(""hi"").Foreground(color: ""#888"")", false)] + [InlineData(@"new ResourceBuilder().Set(""ContentControlThemeFontFamily"", ""Consolas"")", true)] + [InlineData(@"new ResourceBuilder().Set(key: ""ContentControlThemeFontFamily"", color: ""Consolas"")", true)] + [InlineData(@"new ResourceBuilder().Set(""ContentControlThemeFontFamily"", @""C:\Fonts\a""""b.ttf#A"")", true)] + public async Task Applying_the_suggestion_compiles_and_clears_the_diagnostic(string expression, bool fontFamilyOverload) + { + // The source under test has no `using Microsoft.UI.Xaml.Media;`, so an unqualified + // `new FontFamily(...)` suggestion would fail with CS0246 here. + var (diagnostics, c) = AnalyzeRaw(expression, fontFamilyOverload); + var d = Assert.Single(diagnostics); + var s = RunRule(ToCheckDiag(d), c); + Assert.NotNull(s); + + // A FontFamily suggestion replaces the whole argument (it may carry a renamed `name:`); a color + // suggestion replaces only the flagged literal, keeping any `color:` name. + var tree = d.Location.SourceTree!; + var flagged = tree.GetRoot(TestContext.Current.CancellationToken).FindNode(d.Location.SourceSpan, getInnermostNodeForTie: true); + var span = s!.Text.Contains(".FontFamily(", StringComparison.Ordinal) + ? flagged.FirstAncestorOrSelf()!.Span + : d.Location.SourceSpan; + var text = tree.GetText(TestContext.Current.CancellationToken).Replace(span, s.Text); + var fixedCompilation = c.ReplaceSyntaxTree(tree, tree.WithChangedText(text)); + + Assert.Empty(fixedCompilation.GetDiagnostics(TestContext.Current.CancellationToken).Where(x => x.Severity == DiagnosticSeverity.Error)); + var after = await fixedCompilation.WithAnalyzers(ImmutableArray.Create(new ColorStringLiteralAnalyzer())) + .GetAnalyzerDiagnosticsAsync(TestContext.Current.CancellationToken); + Assert.DoesNotContain(after, x => x.Id == ColorStringLiteralAnalyzer.DiagnosticId); + } + + internal static (CheckCommand.Diag[] Diags, CSharpCompilation Compilation) Analyze(string expression, bool fontFamilyOverload = false) + { + var (found, c) = AnalyzeRaw(expression, fontFamilyOverload); + return (found.Select(ToCheckDiag).ToArray(), c); + } + + internal static CheckCommand.Diag ToCheckDiag(Diagnostic d) + { + var span = d.Location.GetLineSpan(); + return new CheckCommand.Diag( + span.Path, span.StartLinePosition.Line + 1, span.StartLinePosition.Character + 1, + "warning", d.Id, d.GetMessage()); + } + + internal static (Diagnostic[] Diagnostics, CSharpCompilation Compilation) AnalyzeRaw(string expression, bool fontFamilyOverload = false) + { + var source = $@" +namespace App +{{ + using Microsoft.UI.Reactor; + using Microsoft.UI.Reactor.Elements; + using static Microsoft.UI.Reactor.Factories; + static class C + {{ + const string Brand = ""orange""; + static object M(bool on) => {expression}; + }} +}}"; + var c = TestCompilation.Create(new[] + { + (ColorStringLiteralAnalyzerTests.CommonStubs, "Stub.cs"), + (fontFamilyOverload ? ColorStringLiteralAnalyzerTests.ResourceBuilderWithFontFamily : ColorStringLiteralAnalyzerTests.ResourceBuilderWithoutFontFamily, "ResourceBuilder.cs"), + (source, "Test.cs"), + }); + Assert.Empty(c.GetDiagnostics(TestContext.Current.CancellationToken).Where(d => d.Severity == DiagnosticSeverity.Error)); + + var found = c.WithAnalyzers(ImmutableArray.Create(new ColorStringLiteralAnalyzer())) + .GetAnalyzerDiagnosticsAsync(TestContext.Current.CancellationToken).GetAwaiter().GetResult(); + return (found.Where(d => d.Id == ColorStringLiteralAnalyzer.DiagnosticId).OrderBy(d => d.Location.SourceSpan.Start).ToArray(), c); + } + + internal static Suggestion? RunRule(CheckCommand.Diag diag, CSharpCompilation c) + { + var registry = RuleRegistry.Of(new IRulePattern[] { new ColorStringLiteralRule() }); + return new SuggesterOrchestrator(rules: registry).SuggestAgainst(diag, c); + } +} diff --git a/tests/Reactor.Tests/CheckCommandTests/Rules/RulePerformanceTests.cs b/tests/Reactor.Tests/CheckCommandTests/Rules/RulePerformanceTests.cs index c37edc7aa..1a1b5ff70 100644 --- a/tests/Reactor.Tests/CheckCommandTests/Rules/RulePerformanceTests.cs +++ b/tests/Reactor.Tests/CheckCommandTests/Rules/RulePerformanceTests.cs @@ -66,6 +66,21 @@ public static class Factories { public static ButtonElement Button(string label, Action? onClick = null) => new(label); } + public static class BrushHelper + { + public static object Parse(string color) => null!; + } + public static class ElementExtensions + { + public static T Background(this T el, string color) where T : Element => el; + } +} +namespace Microsoft.UI.Reactor.Elements +{ + public sealed class ResourceBuilder + { + public ResourceBuilder Set(string key, string color) => this; + } }"; const string TestSource = @" diff --git a/tests/Reactor.Tests/CheckCommandTests/Rules/RuleTargetResolutionTests.cs b/tests/Reactor.Tests/CheckCommandTests/Rules/RuleTargetResolutionTests.cs index 51b3643f3..f6d6fe2d4 100644 --- a/tests/Reactor.Tests/CheckCommandTests/Rules/RuleTargetResolutionTests.cs +++ b/tests/Reactor.Tests/CheckCommandTests/Rules/RuleTargetResolutionTests.cs @@ -56,7 +56,7 @@ public void Every_registered_rule_resolves_its_declared_targets_against_live_Rea /// authoritative gate. /// [global::System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("SingleFile", "IL3000", Justification = "Test-only: reads loaded assemblies' on-disk Location to build a live Reactor Roslyn compilation (MetadataReference.CreateFromFile). IL3000 only affects single-file publish (Location is empty there) — already handled by the surrounding try/catch + skip; this host is not single-file-published. Behaviour-neutral.")] - static CSharpCompilation BuildLiveReactorCompilation() + internal static CSharpCompilation BuildLiveReactorCompilation() { // Force Reactor.dll into the AppDomain BEFORE enumerating loaded // assemblies — otherwise xunit's lazy assembly loading can leave diff --git a/tests/Reactor.Tests/PerfDiffSkipPathTests.cs b/tests/Reactor.Tests/PerfDiffSkipPathTests.cs index faba943c7..3b0e53983 100644 --- a/tests/Reactor.Tests/PerfDiffSkipPathTests.cs +++ b/tests/Reactor.Tests/PerfDiffSkipPathTests.cs @@ -616,9 +616,20 @@ public void InterspersedGrid_Builds_Item_And_Separator_Tracks() [Theory] [InlineData("red", 255, 255, 0, 0)] + [InlineData("green", 255, 0, 128, 0)] + [InlineData("blue", 255, 0, 0, 255)] + [InlineData("white", 255, 255, 255, 255)] + [InlineData("black", 255, 0, 0, 0)] + [InlineData("gray", 255, 128, 128, 128)] + [InlineData("grey", 255, 128, 128, 128)] + [InlineData("lightgray", 255, 211, 211, 211)] + [InlineData("lightgrey", 255, 211, 211, 211)] [InlineData("transparent", 0, 0, 0, 0)] [InlineData("#FF8800", 255, 255, 0x88, 0)] [InlineData("#8000FF00", 0x80, 0, 255, 0)] + // Unrecognized input falls back to gray — the behaviour REACTOR_DYM_006 warns about. + [InlineData("orange", 255, 128, 128, 128)] + [InlineData("#888", 255, 128, 128, 128)] public void ParseColor_Resolves_Named_And_Hex(string input, int a, int r, int g, int b) { var color = BrushHelper.ParseColor(input);