Skip to content

Conversation

@grokys
Copy link
Member

@grokys grokys commented Jan 9, 2026

Summary

This PR introduces a new public API for creating compiled bindings directly from LINQ expressions, along with significant internal refactoring to unify the compiled binding infrastructure.

Depends on: #20439 (Move compiled bindings to Avalonia.Base)

New Public API

Adds two static factory methods to CompiledBinding:

// Without explicit source (uses DataContext)
var binding = CompiledBinding.Create<MyViewModel, string>(vm => vm.Title);
textBlock.Bind(TextBlock.TextProperty, binding);

// With explicit source
var binding = CompiledBinding.Create(viewModel, vm => vm.Title, 
    mode: BindingMode.TwoWay);
textBlock.Bind(TextBlock.TextProperty, binding);

Benefits:

  • Type-safe binding creation without string-based paths
  • Compile-time validation of binding expressions
  • Full IntelliSense support
  • Better performance than reflection-based bindings
  • Consistent with existing observable-to-binding patterns

Supported expressions:

  • Property access: x => x.Property
  • Nested properties: x => x.Property.Nested
  • Indexers: x => x.Items[0]
  • Type casts: x => ((DerivedType)x).Property
  • Logical NOT: x => !x.BoolProperty
  • Stream bindings: x => x.TaskProperty (Task/Observable)
  • AvaloniaProperty access: x => x[MyProperty]

Internal Refactoring

  1. Unified Binding Path Construction

    • Refactored BindingExpressionVisitor to use CompiledBindingPathBuilder instead of directly creating ExpressionNode instances
    • Both compile-time XAML bindings and runtime expression bindings now use the same infrastructure
    • Removed duplicate code path (CompiledBindingPathFromExpressionBuilder)
  2. Architecture Improvements

    • Moved CompiledBindingPath from Avalonia.Markup.Xaml to Avalonia.Base
    • Simplified CompiledBindingExtension by ~180 lines
    • Better separation between compile-time XAML and runtime expression binding
  3. Cast Handling Fixes

    • Fixed backwards IsAssignableFrom check in cast validation
    • Now allows all valid reference type casts (both up-casts and down-casts)
    • Fixed cast transparency issue - casts now properly create nodes instead of being transparent
    • Uses compiled cast functions for better performance
  4. Test-Only Code Cleanup

    • Moved BindingExpression.Create and BindingExpressionVisitor.BuildNodes from production to test extensions
    • Keeps production assemblies smaller and API surface cleaner
    • Added comprehensive tests (36+ for BindingExpressionVisitor, 9 for CompiledBinding.Create)

Changes

Production Code

  • src/Avalonia.Base/Data/CompiledBinding.cs - Added Create factory methods
  • src/Avalonia.Base/Data/CompiledBindingPath.cs - Moved from Markup project
  • src/Avalonia.Base/Data/Core/Parsers/BindingExpressionVisitor.cs - Major refactoring to use builder pattern
  • src/Avalonia.Base/Data/Core/BindingExpression.cs - Removed test-only Create method
  • src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/CompiledBindingExtension.cs - Simplified (~180 lines removed)

Test Code

  • tests/Avalonia.Base.UnitTests/Data/CompiledBindingTests_Create.cs - New tests for Create API
  • tests/Avalonia.Base.UnitTests/Data/Core/Parsers/BindingExpressionVisitorTests.cs - Comprehensive test suite
  • tests/Avalonia.Base.UnitTests/Data/Core/Parsers/BindingExpressionVisitorExtensions.cs - Test helper
  • tests/Avalonia.LeakTests/BindingExpressionExtensions.cs - Test helper
  • Removed CompiledBindingPathFromExpressionBuilder.cs (now redundant)

Test Plan

  • ✅ All 36 BindingExpressionVisitor tests pass
  • ✅ All 9 CompiledBinding.Create tests pass (including integration tests)
  • ✅ All existing tests pass (no regressions)
  • ✅ Production code builds cleanly
  • ✅ Verified path string representations are correct

Breaking Changes

None. All changes are additions or internal refactoring. Existing APIs remain unchanged.

🤖 Generated with Claude Code

grokys and others added 18 commits January 9, 2026 02:37
And documented public (internal) API.
Tests cover all supported features including property access, indexers,
AvaloniaProperty access, logical NOT, stream bindings, and type operators.
Also includes tests for unsupported operations that should throw exceptions.

Discovered bug: IsAssignableFrom check at line 139 is backwards, causing
upcasts to incorrectly throw and downcasts to be incorrectly ignored.
Bug is documented with skipped tests showing expected behavior and passing
tests documenting current broken behavior.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Fixed the inheritance cast check which was inverted, causing:
- Upcasts (derived→base) to incorrectly throw exceptions
- Downcasts (base→derived) to be incorrectly ignored

Changed line 139 from:
  node.Operand.Type.IsAssignableFrom(node.Type)
to:
  node.Type.IsAssignableFrom(node.Operand.Type)

This correctly identifies safe upcasts (which are ignored) vs unsafe
downcasts (which throw exceptions).

Updated tests to remove skip attributes and removed the temporary tests
that documented the broken behavior. All 33 tests now pass.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Changed the cast handling to allow both upcasts and downcasts for reference
types. This makes casts actually useful in binding expressions while still
rejecting value type conversions that would require actual conversion logic.

The logic now checks:
- Both types must be reference types (not value types)
- One type must be assignable to/from the other (either direction)

This allows practical scenarios like:
- Upcasts: (BaseClass)derived
- Downcasts: (DerivedClass)baseInstance
- Casting through object: (TargetType)(object)source

The binding system will gracefully handle any runtime type mismatches.

Updated tests to verify downcasts are allowed and added test for casting
through object pattern.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Added test showing that casts are transparent in property chains.
For example, ((TestClass)x).Property produces just one node for the
property access - the cast doesn't create additional nodes.

This clarifies that empty nodes for (TestClass)x is correct behavior:
- Empty nodes = bind to source directly
- The cast is just a type annotation, transparent to the binding path

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Casts were incorrectly being treated as transparent (not creating nodes),
but CompiledBindingPath uses TypeCastPathElement which creates FuncTransformNode.
For consistency and correctness, BindingExpressionVisitor should also create
cast nodes using ReflectionTypeCastNode.

Changes:
- Convert expressions now create ReflectionTypeCastNode
- TypeAs expressions now create ReflectionTypeCastNode
- Both upcasts and downcasts create nodes (runtime checks handle failures)

Examples:
- x => (TestClass)x → 1 node (cast)
- x => ((TestClass)x).Prop → 2 nodes (cast + property)
- x => x.Child as object → 2 nodes (property + cast)

This matches the behavior of CompiledBindingPathBuilder.TypeCast<T>().

Updated all related tests to verify cast nodes are created.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Changed from ReflectionTypeCastNode (which uses Type.IsInstanceOfType) to
FuncTransformNode with a compiled cast function. This matches how
CompiledBindingPath handles TypeCastPathElement and provides better
performance by avoiding reflection.

The CreateCastFunc method compiles an expression `(object? obj) => obj as T`
which generates efficient IL similar to the 'is T' pattern used in
TypeCastPathElement<T>, rather than using reflection-based type checks.

Performance improvement:
- Before: Type.IsInstanceOfType() reflection call for each cast
- After: Compiled IL using 'as' operator (same as TypeCastPathElement<T>)

Updated tests to expect FuncTransformNode instead of ReflectionTypeCastNode.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Instead of compiling our own cast expression, extract the Cast delegate
directly from TypeCastPathElement<T>. This ensures we use the exact same
code path as CompiledBindingPath, avoiding any potential behavioral
differences and code duplication.

Benefits:
- Code reuse - single implementation of cast logic
- Consistency - same behavior as CompiledBindingPathBuilder.TypeCast<T>()
- No duplicate expression compilation logic

Implementation uses reflection to create the closed generic type and
extract the pre-compiled Cast delegate, which is still more efficient
than reflection-based type checks at runtime.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Reverted from using TypeCastPathElement<T> back to compiling lambda
expressions directly. The lambda approach is cleaner and more straightforward:

Benefits of lambda compilation:
- No Activator.CreateInstance call (avoids reflection for construction)
- More direct - creates exactly what we need
- No coupling to TypeCastPathElement internal implementation
- Simpler code flow

The compiled lambda generates the same efficient IL code (using 'as' operator)
as TypeCastPathElement<T> does, just without the indirection.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Changed CreateCastFunc XML docs to use <remarks> tag for better formatting
- Removed unused 'using Avalonia.Data;' from tests
- Removed redundant '#nullable enable' from tests

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Changes BindingExpressionVisitor to use CompiledBindingPathBuilder instead of
directly creating ExpressionNode instances, unifying the approach with
compile-time XAML bindings. Adds new BuildPath() method that returns
CompiledBindingPath, while maintaining backwards compatibility through the
existing BuildNodes() wrapper method.

Key changes:
- Replace internal List<ExpressionNode> with CompiledBindingPathBuilder
- Refactor all visitor methods to call builder methods
- Add accessor factory methods and implementations for property access
- Support AvaloniaProperty, CLR properties, arrays, indexers, streams, casts
- Update tests to expect PropertyAccessorNode, StreamNode, ArrayIndexerNode

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
…pressionVisitor.

Now that BindingExpressionVisitor has been refactored to use
CompiledBindingPathBuilder and provides a BuildPath() method, the test-only
CompiledBindingPathFromExpressionBuilder class is redundant and can be removed.

Changes:
- Replace CompiledBindingPathFromExpressionBuilder.Build() with
  BindingExpressionVisitor<TIn>.BuildPath() in BindingExpressionTests
- Delete CompiledBindingPathFromExpressionBuilder.cs test file
- All 122 tests in BindingExpressionTests.Compiled continue to pass

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Removes BindingExpression.Create and BindingExpressionVisitor.BuildNodes
from production code since they were only used by unit tests.

Changes:
- Remove BindingExpression.Create<TIn, TOut> method
- Remove BindingExpressionVisitor.BuildNodes method
- Add BindingExpressionVisitorExtensions in Base.UnitTests
- Add BindingExpressionExtensions in LeakTests
- Add static helper methods in test classes to reduce noise

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Adds two static factory methods to create CompiledBinding instances from
lambda expressions using BindingExpressionVisitor.BuildPath():

- Create<TIn, TOut>(expression, converter, mode)
  Creates binding without explicit source (uses DataContext)

- Create<TIn, TOut>(source, expression, converter, mode)
  Creates binding with explicit source

This provides a type-safe, ergonomic API for creating compiled bindings
from code without string-based paths.

Usage:
  var binding = CompiledBinding.Create(viewModel, vm => vm.Title);
  textBlock.Bind(TextBlock.TextProperty, binding);

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@grokys grokys added enhancement area-bindings needs-api-review The PR adds new public APIs that should be reviewed. labels Jan 9, 2026
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 12.0.999-cibuild0061360-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Comment on lines +67 to +68
[RequiresDynamicCode(TrimmingMessages.ExpressionNodeRequiresDynamicCodeMessage)]
[RequiresUnreferencedCode(TrimmingMessages.ExpressionNodeRequiresUnreferencedCodeMessage)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is quite counterintuitive that compiled bindings require dynamic/unreferenced code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, though do note that it's only required to build the compiled binding. We can probably do better with interceptors and source generators, but this API would be useful anyway if we want to e.g. migrate TreeDataGrid from using its current experimental bindings (which are constructed using LINQ expressions).

@MrJul
Copy link
Member

MrJul commented Jan 14, 2026

API diff

Avalonia.Base (net10.0, net8.0)

  namespace Avalonia.Data
  {
      public class CompiledBinding : Avalonia.Data.BindingBase
      {
+         public static CompiledBinding Create<TIn, TOut>(TIn source, Expression<System.Func<TIn, TOut>> expression, IValueConverter? converter = null, BindingMode mode = 0) where TIn : class;
+         public static CompiledBinding Create<TIn, TOut>(Expression<System.Func<TIn, TOut>> expression, IValueConverter? converter = null, BindingMode? mode = 0);
      }
  }

@MrJul
Copy link
Member

MrJul commented Jan 14, 2026

Notes from the API review:
Have a single Create method instead providing every possible property (converterParameter, fallbackValue, etc.)

@MrJul MrJul added api-change-requested The new public APIs need some changes. and removed needs-api-review The PR adds new public APIs that should be reviewed. labels Jan 14, 2026
…arameters.

Consolidates the two Create method overloads into a single method with source
as an optional parameter. Adds optional parameters for all CompiledBinding
properties (priority, converterCulture, converterParameter, fallbackValue,
stringFormat, targetNullValue, updateSourceTrigger, delay) per PR feedback.

Properties that default to AvaloniaProperty.UnsetValue (source, fallbackValue,
targetNullValue) use null-coalescing to convert null parameter values.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@grokys
Copy link
Member Author

grokys commented Jan 15, 2026

Have a single Create method instead providing every possible property (converterParameter, fallbackValue, etc.)

Done. The parameter ordering was based on:

  1. Core parameters first (most commonly used):
    • source - fundamental to binding
    • converter - very common for value transformation
    • mode - commonly specified (OneWay, TwoWay, etc.)
    • priority - important but less frequently changed
  2. Converter-related parameters grouped together:
    - converterCulture, converterParameter - these naturally belong near converter
  3. Value handling parameters:
    - fallbackValue, stringFormat, targetNullValue - all about how values are displayed/handled
  4. Source update behavior (least commonly used):
    - updateSourceTrigger, delay - specialized parameters for controlling when/how source updates happen

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 12.0.999-cibuild0061471-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul MrJul added api-approved The new public APIs have been approved. and removed api-change-requested The new public APIs need some changes. labels Jan 16, 2026
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 12.0.999-cibuild0061628-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Copy link
Member

@MrJul MrJul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left some minor comments. Aside from that, the implementation looks good!

Comment on lines +34 to +35
private static readonly PropertyInfo s_avaloniaObjectIndexer;
private static readonly MethodInfo s_createDelegateMethod;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't a problem in tests, but these static fields should ideally be moved to a non-generic static class to avoid having them duplicated for each generic type.

AvaloniaObjectIndexer = typeof(AvaloniaObject).GetProperty("Item", new[] { typeof(AvaloniaProperty) })!;
CreateDelegateMethod = typeof(MethodInfo).GetMethod("CreateDelegate", new[] { typeof(Type), typeof(object) })!;
s_avaloniaObjectIndexer = typeof(AvaloniaObject).GetProperty("Item", [typeof(AvaloniaProperty)])!;
s_createDelegateMethod = typeof(MethodInfo).GetMethod("CreateDelegate", [typeof(Type), typeof(object)])!;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use nameof(MethodInfo.CreateDelegate)

{
AvaloniaObjectIndexer = typeof(AvaloniaObject).GetProperty("Item", new[] { typeof(AvaloniaProperty) })!;
CreateDelegateMethod = typeof(MethodInfo).GetMethod("CreateDelegate", new[] { typeof(Type), typeof(object) })!;
s_avaloniaObjectIndexer = typeof(AvaloniaObject).GetProperty("Item", [typeof(AvaloniaProperty)])!;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use CommonPropertyNames.IndexerName (already present in this file).

var genericArgs = method.GetGenericArguments();
var genericArg = genericArgs.Length > 0 ? genericArgs[0] : typeof(object);

if (typeof(Task<>).MakeGenericType(genericArg).IsAssignableFrom(instance?.Type) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing is really done with the constructed Task<T> type, we should avoid constructing it at all, which is slower and requires dynamic code (we still have MakeGenericMethod to solve but that's for another time).

Instead, this can be replaced by the following check:
instanceType.IsGenericType && instanceType.GetGenericTypeDefinition() == typeof(Task<>) && genericArg.IsAssignableFrom(instanceType.GetGenericArguments()[0]

Feel free to disregard this comment if you think this it's not worth it.

.MakeGenericMethod(genericArg);
return Add(instance, node, x => builderMethod.Invoke(x, null));
}
else if (typeof(IObservable<>).MakeGenericType(genericArg).IsAssignableFrom(instance?.Type))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above, with IObservable<> instead of Task<>.


public override bool SetValue(object? value, BindingPriority priority)
{
if (!_property.IsReadOnly && _reference.TryGetTarget(out var instance) && instance is not null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: TryGetTarget is guaranteed to return a non-null instance when it returns true (and is annotated correctly), all four null checks in AvaloniaPropertyAccessor can be removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should probably not be part of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-approved The new public APIs have been approved. area-bindings enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants