diff --git a/docs/_pipeline/templates/analyzer-architecture.md.dt b/docs/_pipeline/templates/analyzer-architecture.md.dt index 0687858f5..f8f5ab030 100644 --- a/docs/_pipeline/templates/analyzer-architecture.md.dt +++ b/docs/_pipeline/templates/analyzer-architecture.md.dt @@ -124,6 +124,7 @@ via `.editorconfig`. | `REACTOR_DOC_001` | Warning | Public API missing XML doc summary | `XmlDocSummaryAnalyzer.cs` | | `REACTOR_POOL_001` | Warning | `.Set` writes to a property that pool reset clears | `PoolResetSetAnalyzer.cs` | | `REACTOR0050` | Warning | Optional OneWay descriptor entry has no `dp:` ClearValue fallback | `OneWayClearValueAnalyzer.cs` | +| `REACTOR_MEMO_001` | Info | Modifiers on a keyed Memo(key,factory) wrapper opt the row out of the recycle cache | `MemoWrapperModifierAnalyzer.cs` | `REACTOR_HOOKS_002` and `_003` are reserved for future control-flow / data-flow analyses (variable hook counts across early returns, async diff --git a/plugins/reactor/skills/reactor-build-and-check/SKILL.md b/plugins/reactor/skills/reactor-build-and-check/SKILL.md index 4b5844666..8ca768473 100644 --- a/plugins/reactor/skills/reactor-build-and-check/SKILL.md +++ b/plugins/reactor/skills/reactor-build-and-check/SKILL.md @@ -88,6 +88,7 @@ Additional flags: | `REACTOR_A11Y_001` | warning | Icon-only button missing accessible name | Add `.AutomationName("Delete")` (or similar). | | `REACTOR_A11Y_002` | warning | Image missing alt text | Add `.AutomationName(...)` or `.AccessibilityHidden(true)` for decorative images. | | `REACTOR_A11Y_003` | warning | Form field missing label | Wrap in `FormField(input, label: "Email", required: true)`. | +| `REACTOR_MEMO_001` | info | A fluent modifier is applied to a keyed `Memo(key, factory)` wrapper, so the row opts out of the virtualized cross-recycle cache | Move the modifier(s) inside the factory: `Memo(id, () => Row(item).Padding(8))` instead of `Memo(id, () => Row(item)).Padding(8)`. Only a bare keyed-Memo wrapper is cached — fold any state the moved modifiers read into the key (e.g. `Memo((id, isSelected), …)`) or a cache hit can serve stale content. | | `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`. | | `CS1061` | error | "'X' does not contain a definition for 'Y'" | Modifier order — type-specific sugar (`.Bold()`, `.Foreground()` on `TextBlockElement`) must come before generic modifiers (`.Margin()`, `.Padding()`) that return base `Element`. | | `CS0117` | error | "'Element' does not contain a definition for X" | Same root cause as CS1061 — modifier order, OR you're calling a factory that doesn't exist. Confirm against `reactor-dsl/references/reactor.api.txt`. | diff --git a/src/Reactor.Analyzers/AnalyzerReleases.Unshipped.md b/src/Reactor.Analyzers/AnalyzerReleases.Unshipped.md index ca1269ec6..eb5d8c3c3 100644 --- a/src/Reactor.Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Reactor.Analyzers/AnalyzerReleases.Unshipped.md @@ -20,3 +20,4 @@ REACTOR_DSL_001 | Reactor.Dsl | Warning | MissingWithKeyAnalyzer - Dynamic list REACTOR_DOCK_001 | Reactor.Docking | Warning | OnLiveLayoutRoundTripAnalyzer - OnLiveLayoutChanged feeds the live layout back into state REACTOR_POOL_001 | Reactor.Pool | Warning | PoolResetSetAnalyzer - .Set assigns to a property reset on pool return; use the surviving Reactor modifier REACTOR0050 | Reactor.Descriptor | Warning | OneWayClearValueAnalyzer - Optional OneWay descriptor entries should provide dp: for ClearValue fallback +REACTOR_MEMO_001 | Reactor.Performance | Info | MemoWrapperModifierAnalyzer - Modifiers on a keyed Memo(key,factory) wrapper opt the row out of the recycle cache diff --git a/src/Reactor.Analyzers/MemoWrapperModifierAnalyzer.cs b/src/Reactor.Analyzers/MemoWrapperModifierAnalyzer.cs new file mode 100644 index 000000000..6dc5540ef --- /dev/null +++ b/src/Reactor.Analyzers/MemoWrapperModifierAnalyzer.cs @@ -0,0 +1,215 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.UI.Reactor.Analyzers; + +/// +/// REACTOR_MEMO_001 — a fluent modifier applied to a keyed +/// Memo(key, factory) wrapper silently opts the row out of the virtualized +/// cross-recycle cache. +/// +/// +/// The opt-in keyed memo (Factories.Memo<TKey>(TKey key, Func<Element> factory) → +/// ) is only served from the owning +/// ElementFactory's bounded cross-recycle LRU when the wrapper is bare: +/// km.Modifiers is null && km.Key is null && km.Extensions is null +/// (ElementFactory.BuildOrCache). Decorating the wrapper — e.g. +/// Memo(item.Id, () => Row(item)).Padding(8) — sets Modifiers (or +/// Key/Extensions), so the row falls through the cache and is rebuilt + +/// re-diffed on every recycle, quietly losing the perf benefit. Because the reconciler +/// treats the wrapper transparently (it re-applies the wrapper's modifiers to the inner +/// element on mount/update), moving those modifiers onto the element the factory returns +/// (Memo(item.Id, () => Row(item).Padding(8))) is behaviorally identical and +/// keeps the wrapper cacheable. +/// +/// Distinguishing the keyed overload from the non-keyed +/// Memo(Func<RenderContext, Element>, params object?[]) (→ MemoElement) is +/// done semantically, by resolving the receiver call's return type — the two share +/// the Memo name but only the keyed one participates in the cross-recycle cache. +/// +/// Conservative — fires only when a mechanical fix is possible: the factory is a +/// parameterless lambda with an expression body (or a single-return block), so its +/// body can carry the moved modifiers. Method-group / multi-statement / non-keyed receivers +/// do not fire. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class MemoWrapperModifierAnalyzer : DiagnosticAnalyzer +{ + public const string DiagnosticId = "REACTOR_MEMO_001"; + + private const string ElementTypeName = "Element"; + private const string KeyedMemoTypeName = "KeyedMemoElement"; + private const string ReactorCoreNamespace = "Microsoft.UI.Reactor.Core"; + + private static readonly LocalizableString Title = + "Modifier on a keyed Memo wrapper opts out of the recycle cache"; + + private static readonly LocalizableString MessageFormat = + "Modifier '{0}' is applied to the keyed 'Memo(key, factory)' wrapper, which opts the row out of the cross-recycle cache. Move the modifier(s) inside the factory body instead."; + + private static readonly LocalizableString Description = + "A keyed 'Memo(key, factory)' row is only served from the virtualized-list cross-recycle " + + "cache while the wrapper is bare (no modifiers, key, or attached state). Decorating the " + + "wrapper — e.g. 'Memo(id, () => Row(item)).Padding(8)' — makes the row bypass the cache and " + + "rebuild + re-diff on every recycle, silently losing the perf benefit. Move the modifiers " + + "onto the element the factory returns — 'Memo(id, () => Row(item).Padding(8))' — to keep the " + + "wrapper cacheable. Because the memo then caches the modified element per key, make sure every " + + "input the moved modifiers read is captured by the key (fold selection/theme flags into the " + + "key, e.g. 'Memo((id, isSelected), () => ...)'); otherwise a cache hit can serve stale content."; + + private static readonly DiagnosticDescriptor Rule = new( + DiagnosticId, + Title, + MessageFormat, + "Reactor.Performance", + DiagnosticSeverity.Info, + isEnabledByDefault: true, + description: Description); + + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(AnalyzeInvocation, SyntaxKind.InvocationExpression); + } + + private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) + { + var modifierInvocation = (InvocationExpressionSyntax)context.Node; + + // Shape: `.Modifier(args)`. + if (modifierInvocation.Expression is not MemberAccessExpressionSyntax modifierAccess) + return; + + // The receiver must be *directly* a call — the raw `Memo(...)` invocation. This is + // what makes the rule fire exactly once per chain: for `Memo(k, f).Padding(8).Margin(4)` + // only `.Padding(8)`'s receiver is the bare `Memo(...)` call; `.Margin(4)`'s receiver is + // the `Memo(...).Padding(8)` sub-chain, whose callee is `Padding`, not `Memo`. + if (modifierAccess.Expression is not InvocationExpressionSyntax memoInvocation) + return; + + // Cheap syntactic gate before any semantic work: the receiver's callee is named `Memo`. + if (!IsCalleeNamed(memoInvocation.Expression, "Memo")) + return; + + // Only fire when a mechanical fix is possible: the factory is a movable lambda. This also + // enforces the keyed overload's 2-argument (key, factory) shape syntactically, so most + // non-keyed `Memo(...)` calls are rejected before the semantic model is touched. + if (!TryGetMovableFactory(memoInvocation, out _, out _, out _)) + return; + + // Semantic confirm: the receiver resolves to the KEYED overload. The keyed + // `Memo(TKey, Func)` returns KeyedMemoElement; the non-keyed + // `Memo(Func, params object?[])` returns MemoElement and must not fire. + if (context.SemanticModel.GetSymbolInfo(memoInvocation, context.CancellationToken).Symbol + is not IMethodSymbol memoSymbol + || memoSymbol.Name != "Memo" + || !IsNamedType(memoSymbol.ReturnType, KeyedMemoTypeName, ReactorCoreNamespace)) + return; + + // The trailing call must be a real Element modifier (returns an Element), not e.g. + // `.ToString()` / `.GetHashCode()` on the wrapper. + if (context.SemanticModel.GetSymbolInfo(modifierInvocation, context.CancellationToken).Symbol + is not IMethodSymbol modifierSymbol + || !DerivesFromElement(modifierSymbol.ReturnType)) + return; + + context.ReportDiagnostic(Diagnostic.Create( + Rule, + modifierAccess.Name.GetLocation(), + modifierAccess.Name.Identifier.ValueText)); + } + + /// The callee expression's simple name equals . + private static bool IsCalleeNamed(ExpressionSyntax callee, string name) => callee switch + { + IdentifierNameSyntax id => id.Identifier.ValueText == name, + MemberAccessExpressionSyntax member => member.Name.Identifier.ValueText == name, + MemberBindingExpressionSyntax binding => binding.Name.Identifier.ValueText == name, + _ => false, + }; + + /// + /// The keyed Memo(key, factory) overload takes exactly two arguments; the factory is a + /// parameterless lambda (Func<Element>). A movable factory is one whose body the + /// code-fix can carry the moved modifiers onto: an expression body, or a single-return + /// block. Anything else (method group, multi-statement block, non-lambda) is opaque → no fire. + /// The factory is located by shape (the parameterless lambda argument), not by + /// position, so named / out-of-order argument calls (Memo(factory: () => …, key: id)) + /// are handled uniformly. The keyed overload's TKey key can never be a bare lambda (a + /// lambda has no natural type for TKey inference), so exactly one of the two arguments is + /// the parameterless-lambda factory; anything ambiguous bails. + /// + internal static bool TryGetMovableFactory( + InvocationExpressionSyntax memoInvocation, + out ArgumentSyntax factoryArgument, + out ParenthesizedLambdaExpressionSyntax lambda, + out ExpressionSyntax body) + { + factoryArgument = null!; + lambda = null!; + body = null!; + + var arguments = memoInvocation.ArgumentList.Arguments; + if (arguments.Count != 2) + return false; + + foreach (var argument in arguments) + { + if (argument.Expression is not ParenthesizedLambdaExpressionSyntax candidate + || candidate.ParameterList.Parameters.Count != 0 + || !TryGetMovableBody(candidate, out var candidateBody)) + continue; + + // A second movable parameterless lambda would make the factory ambiguous — bail. + if (factoryArgument is not null) + return false; + + factoryArgument = argument; + lambda = candidate; + body = candidateBody; + } + + return factoryArgument is not null; + } + + private static bool TryGetMovableBody(ParenthesizedLambdaExpressionSyntax lambda, out ExpressionSyntax body) + { + if (lambda.ExpressionBody is { } expressionBody) + { + body = expressionBody; + return true; + } + + if (lambda.Block is { Statements.Count: 1 } block + && block.Statements[0] is ReturnStatementSyntax { Expression: { } returnExpr }) + { + body = returnExpr; + return true; + } + + body = null!; + return false; + } + + internal static bool DerivesFromElement(ITypeSymbol? type) + { + for (var current = type; current is not null; current = current.BaseType) + { + if (IsNamedType(current, ElementTypeName, ReactorCoreNamespace)) + return true; + } + return false; + } + + private static bool IsNamedType(ITypeSymbol? type, string name, string containingNamespace) => + type is { } t + && t.Name == name + && t.ContainingNamespace?.ToDisplayString() == containingNamespace; +} diff --git a/src/Reactor.Analyzers/MemoWrapperModifierCodeFix.cs b/src/Reactor.Analyzers/MemoWrapperModifierCodeFix.cs new file mode 100644 index 000000000..1ba8a4d53 --- /dev/null +++ b/src/Reactor.Analyzers/MemoWrapperModifierCodeFix.cs @@ -0,0 +1,141 @@ +using System.Collections.Immutable; +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; + +namespace Microsoft.UI.Reactor.Analyzers; + +/// +/// Code fix for (REACTOR_MEMO_001) — moves the +/// fluent modifier chain off the keyed Memo(key, factory) wrapper and onto the element the +/// factory returns, so the wrapper stays bare and cacheable: +/// Memo(id, () => Row(item)).Padding(8) → Memo(id, () => Row(item).Padding(8)). +/// +/// +/// The transform re-roots the modifier chain: within the full decorated expression +/// (Memo(...).Padding(8).Margin(4)) the Memo(...) sub-node is replaced by the +/// factory body, yielding the new body (Row(item).Padding(8).Margin(4)), which is then +/// dropped back into the factory lambda. Only movable factories reach here — the analyzer already +/// guaranteed a parameterless lambda with an expression body or a single-return block — so +/// the fix never has to reason about captures or multi-statement bodies. +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MemoWrapperModifierCodeFix))] +[Shared] +public sealed class MemoWrapperModifierCodeFix : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds => + ImmutableArray.Create(MemoWrapperModifierAnalyzer.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; + + var model = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (model is null) return; + + foreach (var diagnostic in context.Diagnostics) + { + // The diagnostic is reported on the modifier's name token; the enclosing invocation is + // the innermost modifier call whose receiver is the raw keyed Memo(...) call. + var node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); + if (node.FirstAncestorOrSelf() is not { } innerModifier) + continue; + + if (innerModifier.Expression is not MemberAccessExpressionSyntax { Expression: InvocationExpressionSyntax memoInvocation }) + continue; + + if (!MemoWrapperModifierAnalyzer.TryGetMovableFactory(memoInvocation, out var factoryArgument, out var lambda, out var body)) + continue; + + // Walk up the modifier chain to the LAST Element-returning modifier — but no further. + // A trailing non-Element call (e.g. `.ToString()`) is NOT part of the wrapper's modifier + // set and must stay outside the factory, otherwise the moved body stops satisfying + // Func and the fix produces non-compiling code. + var outermost = innerModifier; + while (outermost.Parent is MemberAccessExpressionSyntax parentAccess + && parentAccess.Expression == outermost + && parentAccess.Parent is InvocationExpressionSyntax parentInvocation + && model.GetSymbolInfo(parentInvocation, context.CancellationToken).Symbol is IMethodSymbol parentSymbol + && MemoWrapperModifierAnalyzer.DerivesFromElement(parentSymbol.ReturnType)) + { + outermost = parentInvocation; + } + + var capturedOutermost = outermost; + var capturedFactoryArgument = factoryArgument; + context.RegisterCodeFix( + CodeAction.Create( + "Move modifiers inside the Memo factory (ensure their inputs are in the key)", + _ => Task.FromResult(MoveModifiersIntoFactory(context.Document, root, capturedOutermost, memoInvocation, capturedFactoryArgument, lambda, body)), + equivalenceKey: MemoWrapperModifierAnalyzer.DiagnosticId), + diagnostic); + } + } + + private static Document MoveModifiersIntoFactory( + Document document, + SyntaxNode root, + InvocationExpressionSyntax outermost, + InvocationExpressionSyntax memoInvocation, + ArgumentSyntax factoryArgument, + ParenthesizedLambdaExpressionSyntax lambda, + ExpressionSyntax factoryBody) + { + // Re-root the modifier chain on the factory body: replacing the Memo(...) sub-node with the + // body turns `Memo(k, f).Padding(8).Margin(4)` into `body.Padding(8).Margin(4)`. + var substituteBody = NeedsParentheses(factoryBody) + ? SyntaxFactory.ParenthesizedExpression(factoryBody.WithoutTrivia()) + : factoryBody.WithoutTrivia(); + + var newFactoryBody = outermost.ReplaceNode(memoInvocation, substituteBody); + + // Drop the re-rooted chain back into the lambda, preserving the lambda's original shape. + ParenthesizedLambdaExpressionSyntax newLambda = lambda.ExpressionBody is not null + ? lambda.WithExpressionBody(newFactoryBody) + : lambda.WithBlock(((BlockSyntax)lambda.Block!).ReplaceNode(factoryBody, newFactoryBody)); + + // Rebuild the Memo(...) call with the rewritten factory (located by identity, so named / + // out-of-order arguments are handled) and no trailing modifiers. + var newArguments = memoInvocation.ArgumentList.Arguments.Replace( + factoryArgument, + factoryArgument.WithExpression(newLambda)); + var newMemoInvocation = memoInvocation + .WithArgumentList(memoInvocation.ArgumentList.WithArguments(newArguments)) + .WithTriviaFrom(outermost); + + var newRoot = root.ReplaceNode(outermost, newMemoInvocation); + return document.WithSyntaxRoot(newRoot); + } + + /// + /// When the factory body takes the receiver position of the moved modifier chain, wrap it in + /// parentheses unless it is already a primary/postfix expression — otherwise operator + /// precedence would rebind the modifier (e.g. cond ? a : b.Padding()). + /// + private static bool NeedsParentheses(ExpressionSyntax expression) => expression switch + { + InvocationExpressionSyntax => false, + MemberAccessExpressionSyntax => false, + ElementAccessExpressionSyntax => false, + IdentifierNameSyntax => false, + GenericNameSyntax => false, + ParenthesizedExpressionSyntax => false, + ObjectCreationExpressionSyntax => false, + ImplicitObjectCreationExpressionSyntax => false, + ThisExpressionSyntax => false, + BaseExpressionSyntax => false, + LiteralExpressionSyntax => false, + MemberBindingExpressionSyntax => false, + ConditionalAccessExpressionSyntax => false, + PostfixUnaryExpressionSyntax => false, + TupleExpressionSyntax => false, + _ => true, + }; +} diff --git a/tests/Reactor.Tests/AnalyzerTests/MemoWrapperModifierAnalyzerTests.cs b/tests/Reactor.Tests/AnalyzerTests/MemoWrapperModifierAnalyzerTests.cs new file mode 100644 index 000000000..cad20d35c --- /dev/null +++ b/tests/Reactor.Tests/AnalyzerTests/MemoWrapperModifierAnalyzerTests.cs @@ -0,0 +1,322 @@ +using Microsoft.UI.Reactor.Analyzers; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.UI.Reactor.Tests.AnalyzerTests; + +/// +/// Tests for (REACTOR_MEMO_001) and its +/// . Stubs both Memo overloads — the keyed +/// Memo<TKey>(TKey, Func<Element>) → KeyedMemoElement and the non-keyed +/// Memo(Func<RenderContext, Element>, params object[]) → MemoElement — so the +/// analyzer's semantic keyed-overload discrimination is exercised for real. +/// +public class MemoWrapperModifierAnalyzerTests +{ + // `IsExternalInit` lets the stub sources use `record`. The two Memo overloads plus a couple of + // fluent modifiers reproduce the real DSL shape well enough for overload resolution to pick the + // keyed vs non-keyed method exactly as the framework would. + private const string Stubs = @" +namespace System.Runtime.CompilerServices +{ + public static class IsExternalInit { } +} + +namespace Microsoft.UI.Reactor.Core +{ + public sealed class RenderContext { } + + public abstract record Element { } + public sealed record MemoElement(System.Func Render) : Element { } + public sealed record KeyedMemoElement(object MemoKey, System.Func Factory) : Element { } + + public static class Factories + { + public static MemoElement Memo(System.Func render, params object[] dependencies) => null!; + public static KeyedMemoElement Memo(TKey key, System.Func factory) => null!; + // Synthetic non-keyed look-alike: same (value, () => Element) call shape as the keyed + // overload but returns MemoElement, so only the analyzer's return-type check distinguishes + // it. A `bool` key binds this non-generic overload ahead of the generic keyed one. + public static MemoElement Memo(bool flag, System.Func factory) => null!; + public static Element Text(string s) => null!; + } + + public static class ElementExtensions + { + public static T Padding(this T el, int value) where T : Element => el; + public static T Margin(this T el, int value) where T : Element => el; + } +} +"; + + private static string Program(string body) => Stubs + @" +namespace TestApp +{ + using System; + using Microsoft.UI.Reactor.Core; + using static Microsoft.UI.Reactor.Core.Factories; + + public record Item(string Id); + + public static class C + { + static Element Row(Item item) => Text(item.Id); + static Element BuildRow() => Text(""x""); + +" + body + @" + } +}"; + + // ── Analyzer positive ────────────────────────────────────────────── + + [Fact] + public async Task Fires_On_Modifier_Applied_To_Keyed_Memo_Wrapper() + { + var source = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => Row(item)).{|REACTOR_MEMO_001:Padding|}(8);"); + + await new CSharpAnalyzerTest + { + TestCode = source, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Fires_Once_On_Innermost_Modifier_Of_A_Chain() + { + // Only the modifier whose receiver is the bare Memo(...) call fires — not each link. + var source = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => Row(item)).{|REACTOR_MEMO_001:Padding|}(8).Margin(4);"); + + await new CSharpAnalyzerTest + { + TestCode = source, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Fires_On_Fully_Qualified_Factories_Memo_Call() + { + // Member-access callee form `Factories.Memo(...)` (not the `using static` identifier form). + var source = Program(@" + public static Element Build(Item item) + => Factories.Memo(item.Id, () => Row(item)).{|REACTOR_MEMO_001:Padding|}(8);"); + + await new CSharpAnalyzerTest + { + TestCode = source, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Fires_On_Keyed_Memo_With_OutOfOrder_Named_Arguments() + { + // The factory is located by shape, not position, so named/out-of-order args still fire. + var source = Program(@" + public static Element Build(Item item) + => Memo(factory: () => Row(item), key: item.Id).{|REACTOR_MEMO_001:Padding|}(8);"); + + await new CSharpAnalyzerTest + { + TestCode = source, + }.RunAsync(TestContext.Current.CancellationToken); + } + + // ── Analyzer negatives ───────────────────────────────────────────── + + [Fact] + public async Task No_Diagnostic_On_NonKeyed_Memo_Overload() + { + // The non-keyed Memo(ctx => ...) overload returns MemoElement and never participates in the + // cross-recycle cache, so decorating it is fine. This is the critical semantic near-miss: + // syntactically identical `Memo(...).Padding(8)`, but the wrong overload. + var source = Program(@" + public static Element Build(Item item) + => Memo(ctx => Row(item)).Padding(8);"); + + await new CSharpAnalyzerTest + { + TestCode = source, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task No_Diagnostic_On_Bare_Keyed_Memo_Without_Modifier() + { + var source = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => Row(item));"); + + await new CSharpAnalyzerTest + { + TestCode = source, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task No_Diagnostic_When_TwoArg_Memo_Resolves_To_NonKeyed_Return() + { + // Exercises the SEMANTIC return-type guard specifically: this call passes every syntactic + // gate (2 args, parameterless-lambda factory, trailing Element modifier) yet resolves to the + // non-keyed look-alike returning MemoElement, so only the KeyedMemoElement check stops it. + var source = Program(@" + public static Element Build(Item item) + => Memo(true, () => Row(item)).Padding(8);"); + + await new CSharpAnalyzerTest + { + TestCode = source, + }.RunAsync(TestContext.Current.CancellationToken); + } + + // ── Analyzer near-misses (almost trip the syntactic fast path) ───── + + [Fact] + public async Task No_Diagnostic_When_Factory_Is_A_Method_Group() + { + // Opaque factory — there is no lambda body to move the modifier into, so the rule bails + // rather than fire an unfixable Info. + var source = Program(@" + public static Element Build(Item item) + => Memo(item.Id, BuildRow).Padding(8);"); + + await new CSharpAnalyzerTest + { + TestCode = source, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task No_Diagnostic_When_Trailing_Call_Is_Not_An_Element_Modifier() + { + // `.ToString()` returns a string, not an Element — not a wrapper-decorating modifier. + var source = Program(@" + public static string Build(Item item) + => Memo(item.Id, () => Row(item)).ToString();"); + + await new CSharpAnalyzerTest + { + TestCode = source, + }.RunAsync(TestContext.Current.CancellationToken); + } + + // ── Code-fix round-trips ─────────────────────────────────────────── + + [Fact] + public async Task CodeFix_Moves_Single_Modifier_Into_Factory() + { + var before = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => Row(item)).{|REACTOR_MEMO_001:Padding|}(8);"); + + var after = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => Row(item).Padding(8));"); + + await new CSharpCodeFixTest + { + TestCode = before, + FixedCode = after, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task CodeFix_Moves_Chained_Modifiers_Into_Factory() + { + var before = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => Row(item)).{|REACTOR_MEMO_001:Padding|}(8).Margin(4);"); + + var after = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => Row(item).Padding(8).Margin(4));"); + + await new CSharpCodeFixTest + { + TestCode = before, + FixedCode = after, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task CodeFix_Moves_Modifier_Into_A_Block_Body_Factory() + { + var before = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => { return Row(item); }).{|REACTOR_MEMO_001:Padding|}(8);"); + + var after = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => { return Row(item).Padding(8); });"); + + await new CSharpCodeFixTest + { + TestCode = before, + FixedCode = after, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task CodeFix_Parenthesizes_A_NonPrimary_Factory_Body() + { + // A conditional factory body must be wrapped so the moved modifier binds to the whole + // expression, not just the else-branch (operator precedence). + var before = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => item.Id.Length > 0 ? Row(item) : Text(""e"")).{|REACTOR_MEMO_001:Padding|}(8);"); + + var after = Program(@" + public static Element Build(Item item) + => Memo(item.Id, () => (item.Id.Length > 0 ? Row(item) : Text(""e"")).Padding(8));"); + + await new CSharpCodeFixTest + { + TestCode = before, + FixedCode = after, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task CodeFix_Leaves_A_Trailing_NonElement_Call_Outside_The_Factory() + { + // The walk must stop at the last Element modifier: `.ToString()` returns a string, so pulling + // it into the factory would break Func. It stays on the (now bare) wrapper. + var before = Program(@" + public static string Build(Item item) + => Memo(item.Id, () => Row(item)).{|REACTOR_MEMO_001:Padding|}(8).ToString();"); + + var after = Program(@" + public static string Build(Item item) + => Memo(item.Id, () => Row(item).Padding(8)).ToString();"); + + await new CSharpCodeFixTest + { + TestCode = before, + FixedCode = after, + }.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task CodeFix_Targets_The_Factory_Argument_By_Identity_With_Named_Args() + { + // Out-of-order named args: the fix must rewrite the `factory:` argument (located by + // identity), leaving the `key:` argument untouched. + var before = Program(@" + public static Element Build(Item item) + => Memo(factory: () => Row(item), key: item.Id).{|REACTOR_MEMO_001:Padding|}(8);"); + + var after = Program(@" + public static Element Build(Item item) + => Memo(factory: () => Row(item).Padding(8), key: item.Id);"); + + await new CSharpCodeFixTest + { + TestCode = before, + FixedCode = after, + }.RunAsync(TestContext.Current.CancellationToken); + } +}