[XAML SourceGen] Make compiled bindings null-tolerant for intermediate members by Copilot · Pull Request #36444 · dotnet/maui

Copilot · 2026-07-08T08:48:02Z

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description of Change

XAML Source Generation emitted direct member access for binding paths, so a null intermediate (for example, {Binding A.B.C.Text} where A or B is null at runtime) threw NullReferenceException. This diverged from runtime and XamlC bindings, which tolerate null intermediates.

The root cause was that the generator based null-conditional access on nullable reference-type annotations. Those annotations are not enforced at runtime: a reference declared non-nullable can still be null.

This change:

Uses runtime nullability for member, array-index, and indexer path parts, and propagates the resulting nullable type through generated getters, setters, handlers, and TypedBinding<,> arguments.
Keeps two-way bindings able to write null through null-tolerant paths. Reference-type endpoints assign with the null-forgiving operator to remain warning-free; non-nullable value types retain the existing null guard.
Handles value-type property and indexer intermediates with read-modify-write generation. The setter updates a captured copy and assigns it back through each property/indexer in reverse order, avoiding both CS1612 and silently dropped writes.
Identifies array elements from the receiver type. Array elements and value-type fields remain direct lvalues and are not captured.

Getter output, before and after:

// A, B, and C are non-nullable-declared reference types
getter: source => (source.A.B.C.Text, true),
getter: source => (source.A?.B?.C?.Text, true),

The first hop remains non-conditional (source.Foo, not source?.Foo) because TypedBinding.ApplyCore invokes the getter only with a non-null source. Non-nullable value-type endpoints retain their ?? default fallback so their binding property types do not change.

Tests

Updated exact generated-code expectations for compiled bindings, indexers, and Maui13856.
Added coverage for non-nullable-declared reference intermediates.
Added coverage proving two-way paths still write null.
Added setter coverage for value-type properties, indexers, arrays, fields, and nested reverse-order copy-back.

Issues Fixed

Fixes #36313

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

…paths Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

…eference intermediates (#36313) Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

…list in Setter Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

github-actions · 2026-07-16T15:21:58Z

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36444

Or

Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36444"

azure-pipelines · 2026-07-16T15:22:14Z

Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Pull request overview

This PR updates the MAUI XAML Source Generator’s compiled-binding path emission to be null-tolerant when intermediate members are reference types, aligning generated bindings with the long-standing runtime/XamlC behavior (i.e., avoiding NullReferenceException when an intermediate is null at runtime, even if NRT annotations say otherwise).

Changes:

Generate ?. / ?[...] for intermediate path parts based on runtime nullability (reference types + nullable value types), and propagate resulting nullability so TypedBinding<,> generic arguments compile correctly.
Add ITypeSymbolExtensions.CanBeNullAtRuntime() to decouple binding path null-conditional decisions from nullable reference type annotations.
Update/extend SourceGen unit tests to expect null-conditional member/indexer/array access and add a regression test for non-nullable-declared reference intermediates.

Show a summary per file

File	Description
src/Controls/src/SourceGen/CompiledBindingMarkup.cs	Updates binding path parsing to emit conditional access based on runtime-nullability and propagates binding nullability.
src/Controls/src/BindingSourceGen/ITypeSymbolExtensions.cs	Adds `CanBeNullAtRuntime()` helper for runtime-nullability checks.
src/Controls/src/BindingSourceGen/Setter.cs	Adjusts setter path-walk to avoid `Last()` enumeration and adds a capture to avoid CS1612 for certain conditional-access value-type intermediates.
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs	Updates expectations for null-conditional output and adds a regression test for null-tolerant access through non-nullable reference intermediates.
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/IndexerBindings.cs	Updates expectations for null-conditional indexer/array access and nullable `TypedBinding` type arguments.
src/Controls/tests/SourceGen.UnitTests/Maui13856Tests.cs	Updates expectation for null-conditional enum indexer access.

Copilot's findings

Files reviewed: 6/6 changed files
Comments generated: 0

jonathanpeppers · 2026-07-28T14:49:45Z

/azp run maui-pr-devicetests

jonathanpeppers · 2026-07-28T14:49:51Z

/azp run maui-ui-devicetests

azure-pipelines · 2026-07-28T14:49:57Z

No pipelines are associated with this pull request.

azure-pipelines · 2026-07-28T14:49:59Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Making intermediate members null-tolerant widens the generated TypedBinding's TProperty to a nullable type. That fed into the setter's `value is null` early return, so a two-way binding through any multi-hop path silently dropped null writes - e.g. clearing a selection became a no-op. NRT annotations are not enforced at runtime, and neither of the other two inflators does this: BindingExpression assigns null directly, and XamlC's CompiledBindingGetSetter emits no null check at all. Assign through with the null-forgiving operator instead, keeping the generated code CS8601-clean. The early return is retained for non-nullable value types, which genuinely cannot be assigned null. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 10f057bb-7603-49db-aaf3-707c0cf0ddff

A two-way binding through a value-type indexer result, such as {Binding Items[0].Name} over a List<SomeStruct>, generated 'p0[0].Name = value;'. An indexer returns a copy rather than a variable, so that fails to compile with CS1612. The conditional-access branch already captured value-type *members* into a local for this reason; indexers need the same treatment, in both the conditional and non-conditional positions. Array element access must be excluded: unlike an indexer it yields a genuine variable, so capturing would assign to a copy and silently drop the value. Arrays were previously identified by an empty DefaultMemberName, which only holds for the XAML generator - PathParser names array access 'Item' as well. Make it explicit with IndexAccess.IsArrayElement so both generators agree. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 10f057bb-7603-49db-aaf3-707c0cf0ddff

jonathanpeppers · 2026-07-28T15:20:33Z

I reviewed this and pushed two follow-up commits to the branch. The core change is right — SourceGen was the only inflator dereferencing intermediates unconditionally, and deciding based on NRT annotations is unsound since they aren't enforced at runtime. Worth noting the actual NRE in #36313 comes from Subscribe → PartGetter (TypedBinding.cs:589-602), which has no try/catch, so making the handlers: array null-conditional is the part that really fixes it — the getter's NRE was already being swallowed at TypedBinding.cs:369.

Two things I fixed on top:

1. Two-way bindings had stopped writing `null` (`fd48a94`, first commit)

Setting isNullable = true for conditional hops widened TProperty to a nullable type, which tripped the setter's existing PropertyType.IsNullable && !AcceptsNullValue gate. That's visible in this PR's own golden file — Foo.Bar.Title where Title is a plain public string Title { get; set; } gained:

if (value is null) { return; }

So SelectedItem="{Binding Model.Selected, Mode=TwoWay}" would silently no-op on deselect. Neither of the other inflators does this — BindingExpression.cs:185-190 assigns null directly, and XamlC's CompiledBindingGetSetter (SetPropertiesVisitor.cs:938-1010) emits no null check on value at all.

Reference-type endpoints now assign through with the null-forgiving operator (p1.Title = value!;) so the generated code stays CS8601-clean. The early return is kept for non-nullable value types, which genuinely can't take null.

This also flips NonNullableReferenceTypeAtEndOfConditionalAccessPath, which previously asserted the early return as intended behavior. That was a pre-existing SourceGen-only divergence for explicitly-?-annotated intermediates; since the whole point here is inflator parity, I updated it rather than preserving it. Shout if you'd rather keep that one as-is.

2. `Setter.cs` value-type capture missed indexers → CS1612

{Binding Items[0].Name, Mode=TwoWay} over a List<SomeStruct> generated p0[0].Name = value;, which doesn't compile:

CS1612: Cannot modify the return value of 'List<ItemStruct>.this[int]' because it is not a variable

An indexer returns a copy, so its value-type result is an rvalue — same problem the conditional-access branch already handled for MemberAccess. Extended NeedsValueTypeCapture to cover IndexAccess, and added the missing non-conditional-access branch (reachable when the receiver is itself a struct).

Arrays have to be excluded — array element access is a variable, so capturing would assign to a copy and silently drop the write, which is worse than the compile error. The natural check was DefaultMemberName == "", but that convention only holds in CompiledBindingMarkup; PathParser.GetIndexerName returns "Item" for arrays too, so the x:Bind generator would have misclassified every array. Made it explicit with IndexAccess.IsArrayElement, set in both generators.

Tests

Both new tests were verified to fail without their respective fix:

TwoWayBindingThroughIntermediatesStillWritesNull
TwoWayBindingThroughValueTypeIndexerResultCompiles (fails with the CS1612 above)
TwoWayBindingThroughValueTypeArrayElementIsNotCaptured guards against over-capturing arrays
4 SetterBuilderTests cases covering the Setter.From branches directly

Local runs: SourceGen.UnitTests 198/198, BindingSourceGen.UnitTests 129/129, Xaml.UnitTests 1973 passed / 8 skipped / 0 failed.

Still open — not addressed

The getter reports success: true on a broken path. XamlC emits (default, false) (SetPropertiesVisitor.cs:913-926, the initobj branch) and reflection uses FallbackValue ?? property.GetDefaultValue(target) (BindingExpression.cs:174-175). ApplyCore seeds value with the fallback and only overwrites it if (success), so (expr, true) makes the fallback unreachable. Observable in two cases:

FallbackValue is set — {Binding A.B.C, FallbackValue='n/a'} with a null A yields null instead of 'n/a'
the BindableProperty's default isn't default(T) — IsVisible (default true) becomes false via the ?? default at CompiledBindingMarkup.cs:587, likewise Opacity → 0

Fixable with a statement-bodied getter — ?. up to the last receiver, then __r is null ? (default, false) : (__r.Text, true) — since TryCompileBinding already emits a local method. I left it alone because it's a design call rather than an obvious bug, and it widens the diff. Happy to take it either way.

No runtime parity test. Everything here asserts on generated text, which structurally can't catch either of the two issues above. A [Theory][XamlInflatorData] test in Xaml.UnitTests/Issues/ (pattern: Gh4103.xaml.cs) running the same XAML through Runtime, XamlC and SourceGen would be the real guard — particularly for recovery after a mid-chain object goes from null to non-null, given Subscribe does if (part == null) break; and leaves later handlers stale.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

simonrozsival · 2026-08-20T14:23:01Z

/azp run maui-pr-uitests,maui-pr-devicetests

azure-pipelines · 2026-08-20T14:23:22Z

Azure Pipelines: Successfully started running 2 pipeline(s).

azure-pipelines · 2026-08-21T14:17:13Z

Azure Pipelines: Successfully started running 2 pipeline(s).

simonrozsival · 2026-08-24T10:19:39Z

@kubaflo Could you please take another look?

I investigated the current failing CI lanes, and they appear unrelated to this PR:

The Windows device-test failure is a historically flaky WebView HTML-load timeout in a handler test that does not exercise XAML or generated bindings.
The Android Flyout, SafeArea, and macOS CollectionView failures recur on main, including the same tests and snapshot differences.
The remaining failures are emulator startup, agent-disconnect, and job-timeout infrastructure failures.

The CI build covers the current head (121588c), and I did not find evidence of a regression caused by these changes. I believe the implementation is finished and the PR is ready for another review.

MauiBot

Expert Review — 2 findings

See inline comments for details.

MauiBot · 2026-08-24T15:22:53Z

+				var copyBackTarget = accessAccumulator;
 				accessAccumulator = AccessExpressionBuilder.ExtendExpression(accessAccumulator, part);
 				AddPatternMatchingExpression("{}");
+				copyBackStatements.Add(BuildAssignmentStatement(copyBackTarget, part, accessAccumulator));


🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — The copy-back statement is emitted whenever an intermediate path part is a value-type property/indexer, but nothing verifies that this intermediate member is writable. NeedsValueTypeCapture only inspects IsValueType/Kind/IsArrayElement, and in CompiledBindingMarkup.TryParsePath the IsWritable flag is recomputed and overwritten on every iteration, so only the last part's writability is ever checked.

Concrete failing scenario (two-way binding, Mode=TwoWay):

public class VM { public Options Opts { get; } = new(); } // get-only public struct Options { public bool IsEnabled { get; set; } }

with {Binding Opts.IsEnabled, Mode=TwoWay} now generates source.Opts is {} p0; p0.IsEnabled = value; source.Opts = p0; → CS0200 "Property or indexer 'VM.Opts' cannot be assigned to — it is read-only", i.e. the user's build breaks in generated code. The same applies to the ConditionalAccess branch (line 64) and to a mid-path read-only indexer, e.g. IReadOnlyList<ItemStruct> Items with {Binding Items[0].Name, Mode=TwoWay} → p0[0] = p1; → CS0200. Before this PR these shapes compiled (they silently no-op'd), so this converts a silent no-op into a hard build failure.

Suggested fix: thread the intermediate member's writability into the path part (or into SetterOptions) and, when an intermediate value-type member is not assignable, suppress setter generation and report the existing read-only diagnostic (the same treatment ReadOnlyIndexer already gets for a final part) instead of emitting uncompilable code.

MauiBot · 2026-08-24T15:22:54Z

 	}
+
+	[Fact]
+	public void TwoWayBindingThroughValueTypeIndexerResultCompiles()


🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — The new copy-back branch is only covered for the positive cases (writable indexer, array element, value-type field). There is no test for the negative case that the new branch must not break: an intermediate value-type member that is not assignable — e.g. a get-only struct property (public Options Opts { get; }) or a read-only indexer (IReadOnlyList<ItemStruct> Items) in the middle of a Mode=TwoWay path. Because RunGenerator compiles the generated code, such a test would immediately surface the CS0200 build break described on Setter.cs. Please add a test for a mid-path non-writable value-type member alongside TwoWayBindingThroughValueTypeIndexerResultCompiles.

MauiBot

AI Review Summary

@copilot — new AI review results are available based on commit 121588c.

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ✅ PASSED

Platform: IOS · Base: main · Merge base: 19a18889

✅ Verified (new API / feature) — this PR adds new API and a test that references it in the same project, so reverting the fix un-compiles the test: there is no valid "fails without the fix" baseline to establish (a compile-coupled baseline). The gate instead verified the fix by a clean build + pass with the fix, so this is a real PASS rather than a non-committal INCONCLUSIVE.

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 BindingRepresentationGenTests `BindingRepresentationGenTests`	🛠️ BUILD ERROR	✅ PASS — 24s
🧪 CompiledBindings `CompiledBindings`	✅ FAIL — 16s	✅ PASS — 7s
🧪 IndexerBindings `IndexerBindings`	✅ FAIL — 8s	✅ PASS — 5s
🧪 Maui13856Tests `Maui13856Tests`	✅ FAIL — 7s	✅ PASS — 5s
🧪 SetterBuilderTests `SetterBuilderTests`	🛠️ BUILD ERROR	✅ PASS — 4s

🔴 Without fix — 🧪 BindingRepresentationGenTests: 🛠️ BUILD ERROR · 52s