Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Controls/src/SourceGen/SourceGenContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ public void AddLocalMethod(string code)
}
}

readonly HashSet<string> _emittedTemplateMethods = new HashSet<string>();

// 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<ITypeSymbol, (ConverterDelegate, ITypeSymbol)>? knownSGTypeConverters;
internal Dictionary<ITypeSymbol, IKnownMarkupValueProvider>? knownSGValueProviders;
internal Dictionary<ITypeSymbol, ProvideValueDelegate>? knownSGEarlyMarkupExtensions;
Expand Down
4 changes: 2 additions & 2 deletions src/Controls/src/SourceGen/Visitors/CreateValuesVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
80 changes: 42 additions & 38 deletions src/Controls/src/SourceGen/Visitors/SetPropertiesVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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")!;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = """
<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:local="clr-namespace:TestApp"
x:Class="TestApp.MainPage">
<local:TemplateHost>
<local:TemplateHost.Template>
<DataTemplate>
<Label Text="Hi" TextColor="Black" />
</DataTemplate>
</local:TemplateHost.Template>
</local:TemplateHost>
</ContentPage>
""";

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<XamlGenerator>(
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 = """
<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:local="clr-namespace:TestApp"
x:Class="TestApp.MainPage"
x:Name="ThePage">
<local:TemplateHost>
<local:TemplateHost.Template>
<DataTemplate>
<Label HeightRequest="{Binding Source={x:Reference ThePage}, Path=Height}" />
</DataTemplate>
</local:TemplateHost.Template>
</local:TemplateHost>
</ContentPage>
""";

XamlHotReloadState.Reset();
var compilation = CreateCompilation().AddSyntaxTrees(CSharpSyntaxTree.ParseText(host));

var result = SourceGeneratorDriver.RunGenerator<XamlGenerator>(
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()
{
Expand Down Expand Up @@ -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 = """
<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
Expand All @@ -1475,7 +1587,7 @@ public void ResourceWithConverters_UCDoesNotRegisterUnencodableKeys()
<Label Text="Hello" />
</ContentPage>
""";
const string xamlV2 = """
const string xamlV2 = """
<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
Expand All @@ -1488,17 +1600,17 @@ public void ResourceWithConverters_UCDoesNotRegisterUnencodableKeys()
</ContentPage>
""";

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()
Expand Down
Loading