diff --git a/src/Controls/src/SourceGen/SourceGenContext.cs b/src/Controls/src/SourceGen/SourceGenContext.cs index 562df4f328a3..f79109e0cf45 100644 --- a/src/Controls/src/SourceGen/SourceGenContext.cs +++ b/src/Controls/src/SourceGen/SourceGenContext.cs @@ -77,6 +77,15 @@ public void AddLocalMethod(string code) } } + readonly HashSet _emittedTemplateMethods = new HashSet(); + + // Reserves a generated DataTemplate LoadTemplate method name once per compilation unit, so a + // template whose value is set more than once in the same scope (e.g. a `required` property set + // in the object initializer AND as an assignment) emits the local function only once instead of + // redeclaring it. Returns true the first time a name is seen, false afterwards. See dotnet/maui#36682. + public bool TryReserveTemplateMethod(string name) + => ParentContext != null ? ParentContext.TryReserveTemplateMethod(name) : _emittedTemplateMethods.Add(name); + internal Dictionary? knownSGTypeConverters; internal Dictionary? knownSGValueProviders; internal Dictionary? knownSGEarlyMarkupExtensions; diff --git a/src/Controls/src/SourceGen/Visitors/CreateValuesVisitor.cs b/src/Controls/src/SourceGen/Visitors/CreateValuesVisitor.cs index 168f8e03ef61..8a9a4c156639 100644 --- a/src/Controls/src/SourceGen/Visitors/CreateValuesVisitor.cs +++ b/src/Controls/src/SourceGen/Visitors/CreateValuesVisitor.cs @@ -54,7 +54,7 @@ public static void CreateValue(ElementNode node, IndentedTextWriter writer, IDic if (type.Equals(compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Xaml.ArrayExtension"), SymbolEqualityComparer.Default)) { //we might want to move this to a separate method - var visitor = new SetPropertiesVisitor(Context); + var visitor = new SetPropertiesVisitor(Context, valuePrecomputePass: true); // var children = node.Properties.Values.ToList(); // children.AddRange(node.CollectionItems); foreach (var cn in node.CollectionItems) @@ -220,7 +220,7 @@ public static void CreateValue(ElementNode node, IndentedTextWriter writer, IDic var pType = req is IPropertySymbol prop ? prop.Type : ((IFieldSymbol)req).Type; var pConverter = req.GetAttributes().FirstOrDefault(a => a.AttributeClass!.Equals(compilation.GetTypeByMetadataName("System.ComponentModel.TypeConverterAttribute")!, SymbolEqualityComparer.Default))?.ConstructorArguments[0].Value as ITypeSymbol; - var visitor = new SetPropertiesVisitor(Context); + var visitor = new SetPropertiesVisitor(Context, valuePrecomputePass: true); var children = node.Properties.Values.ToList(); children.AddRange(node.CollectionItems); foreach (var cn in children) diff --git a/src/Controls/src/SourceGen/Visitors/SetPropertiesVisitor.cs b/src/Controls/src/SourceGen/Visitors/SetPropertiesVisitor.cs index 525f65b92dc6..4e4ddbd74f28 100644 --- a/src/Controls/src/SourceGen/Visitors/SetPropertiesVisitor.cs +++ b/src/Controls/src/SourceGen/Visitors/SetPropertiesVisitor.cs @@ -10,7 +10,12 @@ namespace Microsoft.Maui.Controls.SourceGen; using static LocationHelpers; -class SetPropertiesVisitor(SourceGenContext context, bool stopOnResourceDictionary = false) : IXamlNodeVisitor +// valuePrecomputePass: set by CreateValuesVisitor when this visitor is run only to precompute a +// value (e.g. a `required` property's value for the object initializer, or an x:Array element) +// before namescopes are registered. In that pass, DataTemplate LoadTemplate emission under +// Incremental Hot Reload is deferred to the main pass so x:Reference/bindings resolve against the +// outer scope at compile time rather than falling back to runtime resolution. See dotnet/maui#36683. +class SetPropertiesVisitor(SourceGenContext context, bool stopOnResourceDictionary = false, bool valuePrecomputePass = false) : IXamlNodeVisitor { SourceGenContext Context => context; IndentedTextWriter Writer => Context.Writer; @@ -168,7 +173,7 @@ public void Visit(MarkupNode node, INode parentNode) public void Visit(ElementNode node, INode parentNode) { - NodeSGExtensions.GetNodeValueDelegate getNodeValue = (n, type) => + NodeSGExtensions.GetNodeValueDelegate getNodeValue = (n, type) => { if (!context.Variables.TryGetValue(n, out var val)) { @@ -200,7 +205,7 @@ public void Visit(ElementNode node, INode parentNode) { // Find the ResourceDictionary parent ILocalValue? rdVar = null; - + if (parentNode is ElementNode parentElement && Context.Variables.TryGetValue(parentElement, out var pVar)) { var rdType = Context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.ResourceDictionary")!; @@ -239,53 +244,52 @@ public void Visit(ElementNode node, INode parentNode) if (propertyName == XmlName._CreateContent) { + // Under Incremental Hot Reload, defer DataTemplate LoadTemplate emission from a + // value-precompute prepass (required-property/x:Array) to the main SetPropertiesVisitor + // pass. The main pass runs after namescope registration, so it resolves x:Reference and + // bindings against the outer scope at compile time; the prepass runs earlier and would + // emit a slower runtime-resolved body that first-wins dedup would then keep + // (dotnet/maui#36683). Non-HR builds keep their existing prepass behavior. + if (valuePrecomputePass && Context.ProjectItem.EnableIncrementalHotReload) + return; + var variable = Context.Variables[parentNode]; // Under XAML Incremental Hot Reload, emit the template content as a stably-named local - // method rather than an anonymous lambda. On each edit the source generator regenerates + // function rather than an anonymous lambda. On each edit the source generator regenerates // InitializeComponent; an anonymous `LoadTemplate = () => { ... }` lambda has an unstable // synthesized-closure identity across regenerations, so successive edits to a control // inside a DataTemplate produce invalid Edit-and-Continue deltas (deleted/renamed // synthesized closure methods) that crash the app, poison Hot Reload, or kill the - // watcher (dotnet/maui#36482). A named local function gives EnC a stable name anchor - // while preserving capture semantics. Non-HR builds keep the anonymous lambda. + // watcher (dotnet/maui#36482). A named local function gives EnC a stable name anchor. + // + // The function is emitted INLINE at the point of use (not hoisted to the top of the + // method) so its body keeps the exact lexical scope the lambda had — references to + // enclosing locals (the DataTemplate variable, name scopes, resources) resolve the same + // way. It is emitted at most once per template: a template value can be set more than + // once in the same scope (e.g. a `required` property set both in the object initializer + // and as an assignment), and redeclaring the local function would not compile + // (dotnet/maui#36682). Non-HR builds keep the anonymous lambda. if (Context.ProjectItem.EnableIncrementalHotReload) { var methodName = TemplateLoadMethodName(node); - - // Buffer the template body by pointing the child context at a fresh writer. - var bodyBuffer = new System.IO.StringWriter(System.Globalization.CultureInfo.InvariantCulture); - var bodyWriter = new IndentedTextWriter(bodyBuffer, "\t"); - var bufferedContext = new SourceGenContext(bodyWriter, context.Compilation, context.SourceProductionContext, context.XmlnsCache, context.TypeCache, context.RootType!, null, context.ProjectItem) - { - ParentContext = context, - }; - - node.Accept(new CreateValuesVisitor(bufferedContext), null); - node.Accept(new SetNamescopesAndRegisterNamesVisitor(bufferedContext), null); - node.Accept(new SetResourcesVisitor(bufferedContext), null); - node.Accept(new SetPropertiesVisitor(bufferedContext, stopOnResourceDictionary: true), null); - bodyWriter.WriteLine($"return {bufferedContext.Variables[node].ValueAccessor};"); - bodyWriter.Flush(); - - // Wrap the buffered body in a named local function and hoist it to the top of the - // generated method (AddLocalMethod bubbles to the root context). - var methodBuffer = new System.IO.StringWriter(System.Globalization.CultureInfo.InvariantCulture); - var methodWriter = new IndentedTextWriter(methodBuffer, "\t"); - methodWriter.WriteLine($"object {methodName}()"); - methodWriter.WriteLine("{"); - methodWriter.Indent++; - foreach (var line in bodyBuffer.ToString().Split(new[] { "\r\n", "\r", "\n" }, StringSplitOptions.None)) + if (Context.TryReserveTemplateMethod(methodName)) { - if (string.IsNullOrWhiteSpace(line)) - methodWriter.InnerWriter.WriteLine(); - else - methodWriter.WriteLine(line); + Writer.WriteLine($"object {methodName}()"); + using (PrePost.NewBlock(Writer, begin: "{", end: "}")) + { + var templateContext = new SourceGenContext(Writer, context.Compilation, context.SourceProductionContext, context.XmlnsCache, context.TypeCache, context.RootType!, null, context.ProjectItem) + { + ParentContext = context, + }; + + node.Accept(new CreateValuesVisitor(templateContext), null); + node.Accept(new SetNamescopesAndRegisterNamesVisitor(templateContext), null); + node.Accept(new SetResourcesVisitor(templateContext), null); + node.Accept(new SetPropertiesVisitor(templateContext, stopOnResourceDictionary: true), null); + Writer.WriteLine($"return {templateContext.Variables[node].ValueAccessor};"); + } } - methodWriter.Indent--; - methodWriter.WriteLine("}"); - methodWriter.Flush(); - Context.AddLocalMethod(methodBuffer.ToString()); Writer.WriteLine($"{variable.ValueAccessor}.LoadTemplate = {methodName};"); return; diff --git a/src/Controls/tests/SourceGen.UnitTests/XamlIncrementalHotReloadPipelineTests.cs b/src/Controls/tests/SourceGen.UnitTests/XamlIncrementalHotReloadPipelineTests.cs index ded0e270d262..f4601111ec9f 100644 --- a/src/Controls/tests/SourceGen.UnitTests/XamlIncrementalHotReloadPipelineTests.cs +++ b/src/Controls/tests/SourceGen.UnitTests/XamlIncrementalHotReloadPipelineTests.cs @@ -595,7 +595,10 @@ public void DataTemplate_HotReload_EmitsStableNamedMethod_NotAnonymousLambda() var expectedColor = label == "run1" ? "Colors.Black" : "Colors.Green"; Assert.Contains(expectedColor, ic, StringComparison.Ordinal); - if (label == "run1") nameV1 = m.Groups[1].Value; else nameV2 = m.Groups[1].Value; + if (label == "run1") + nameV1 = m.Groups[1].Value; + else + nameV2 = m.Groups[1].Value; } // EnC anchor: the generated method name must be IDENTICAL across the edit, so Edit-and-Continue @@ -634,6 +637,115 @@ public void DataTemplate_HotReload_GeneratedNamedMethod_Compiles() Assert.Contains("LoadTemplate = LoadTemplate_", ic, StringComparison.Ordinal); } + [Fact] + public void DataTemplate_HotReload_SetMultipleTimes_EmitsSingleNamedMethod() + { + // Regression for dotnet/maui#36682 and #36683: a DataTemplate assigned to a `required` + // property is visited more than once by the generator (a value-precompute prepass for the + // object initializer, plus the main pass). Under Incremental Hot Reload the template body is + // a named local function; emitting it twice in the same scope produced two + // `object LoadTemplate_L_P()` declarations -> CS0128 ("already defined") + CS8321 ("declared + // but never used"). The named method must be emitted exactly once and LoadTemplate must be + // wired up. (#36683 also defers the prepass emission to the main pass, so LoadTemplate is now + // assigned a single time from the correctly-scoped pass.) + const string host = """ + namespace TestApp + { + public class TemplateHost : Microsoft.Maui.Controls.View + { + public required Microsoft.Maui.Controls.DataTemplate Template { get; set; } + } + } + """; + const string xaml = """ + + + + + + + + + + """; + + XamlHotReloadState.Reset(); + var compilation = CreateCompilation().AddSyntaxTrees(CSharpSyntaxTree.ParseText(host)); + + // assertNoCompilationErrors: true throws if the generated .xsg.cs has C# errors (e.g. the + // duplicate-method CS0128 this test guards against). + var result = SourceGeneratorDriver.RunGenerator( + compilation, MakeFile(xaml), assertNoCompilationErrors: true); + + var ic = FindSourceByHintSuffix(result, ".xsg.cs"); + Assert.NotNull(ic); + + // Exactly one named LoadTemplate method must be declared (no CS0128 duplicate), and + // LoadTemplate must be wired up to it. + var declarations = System.Text.RegularExpressions.Regex.Matches(ic, @"object LoadTemplate_\d+_\d+\(\)"); + Assert.Single(declarations); + var assignments = System.Text.RegularExpressions.Regex.Matches(ic, @"\.LoadTemplate = LoadTemplate_\d+_\d+;"); + Assert.True(assignments.Count >= 1, "expected LoadTemplate to be assigned the single named method"); + } + + [Fact] + public void DataTemplate_HotReload_RequiredProperty_ResolvesOuterReferenceAtCompileTime() + { + // Regression for dotnet/maui#36683 review: a `required` DataTemplate property whose body + // references an outer named element via {x:Reference} must have its body emitted from the + // main SetPropertiesVisitor pass (which runs after namescope registration), so the reference + // resolves at compile time. Previously the value-precompute prepass emitted the body first, + // before namescopes were registered, and first-wins dedup kept that runtime-resolved + // (XamlServiceProvider fallback) body instead of the optimized one. + const string host = """ + namespace TestApp + { + public class TemplateHost : Microsoft.Maui.Controls.View + { + public required Microsoft.Maui.Controls.DataTemplate Template { get; set; } + } + } + """; + const string xaml = """ + + + + + + + + + + """; + + XamlHotReloadState.Reset(); + var compilation = CreateCompilation().AddSyntaxTrees(CSharpSyntaxTree.ParseText(host)); + + var result = SourceGeneratorDriver.RunGenerator( + compilation, MakeFile(xaml), assertNoCompilationErrors: true); + + var ic = FindSourceByHintSuffix(result, ".xsg.cs"); + Assert.NotNull(ic); + + // Single named method, wired up. + Assert.Single(System.Text.RegularExpressions.Regex.Matches(ic, @"object LoadTemplate_\d+_\d+\(\)")); + Assert.Contains("LoadTemplate = LoadTemplate_", ic, StringComparison.Ordinal); + + // The x:Reference to the outer page must be resolved at compile time (a direct __root + // reference), NOT via the runtime XamlServiceProvider/SimpleValueTargetProvider fallback. + Assert.Contains("= __root;", ic, StringComparison.Ordinal); + Assert.DoesNotContain("SimpleValueTargetProvider", ic, StringComparison.Ordinal); + } + [Fact] public void FirstRun_SeedsHotReloadState() { @@ -1458,13 +1570,13 @@ public void ResourceValueChanged_UCEmitsResourceUpdate() } -[Fact] -public void ResourceWithConverters_UCDoesNotRegisterUnencodableKeys() -{ -// When resources include custom types (converters) that can't be encoded as C# expressions, -// the UC should NOT register those keys — otherwise they get removed on next patch. -XamlHotReloadState.Reset(); -const string xamlV1 = """ + [Fact] + public void ResourceWithConverters_UCDoesNotRegisterUnencodableKeys() + { + // When resources include custom types (converters) that can't be encoded as C# expressions, + // the UC should NOT register those keys — otherwise they get removed on next patch. + XamlHotReloadState.Reset(); + const string xamlV1 = """ """; -const string xamlV2 = """ + const string xamlV2 = """ """; -var (_, run2) = TwoRuns(xamlV1, xamlV2); -var uc = FindUCSource(run2, "uc.xsg"); + var (_, run2) = TwoRuns(xamlV1, xamlV2); + var uc = FindUCSource(run2, "uc.xsg"); -Assert.NotNull(uc); -// Only emittable keys (Color values) should be in RegisterResourceKeys -Assert.Contains("AccentColor", uc, StringComparison.Ordinal); -Assert.Contains("SecondaryColor", uc, StringComparison.Ordinal); -Assert.Contains("RegisterResourceKeys", uc, StringComparison.Ordinal); -// The registered keys should only contain the color keys -Assert.Contains("__version = 1", uc, StringComparison.Ordinal); -} + Assert.NotNull(uc); + // Only emittable keys (Color values) should be in RegisterResourceKeys + Assert.Contains("AccentColor", uc, StringComparison.Ordinal); + Assert.Contains("SecondaryColor", uc, StringComparison.Ordinal); + Assert.Contains("RegisterResourceKeys", uc, StringComparison.Ordinal); + // The registered keys should only contain the color keys + Assert.Contains("__version = 1", uc, StringComparison.Ordinal); + } [Fact] public void ConverterResourceAdded_UCEmitsNewInstance()