Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Behaviour of !! operator has changed in 11.1 #16071

Closed
stogle opened this issue Jun 19, 2024 · 6 comments · Fixed by #16101
Closed

Behaviour of !! operator has changed in 11.1 #16071

stogle opened this issue Jun 19, 2024 · 6 comments · Fixed by #16101

Comments

@stogle
Copy link
Contributor

stogle commented Jun 19, 2024

Describe the bug

The behaviour of the double-negation operator (!!) has changed between 11.0.10 and 11.1.0-rc1.

Consider the binding {Binding !!Value}, where Value is null. In 11.0.10 this binding yields false. In 11.1.0-rc1 the same binding yields null. This is a breaking change for my application.

To Reproduce

MainWindow.axaml
<Window xmlns="https://github.com/avaloniaui"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:vm="using:MultiBindingBug.ViewModels"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        mc:Ignorable="d" d:DesignWidth="800" d:DesignHeight="450"
        x:Class="MultiBindingBug.Views.MainWindow"
        x:DataType="vm:MainWindowViewModel">
    <StackPanel>
        <StackPanel Orientation="Horizontal">
            <TextBlock Text="Value1: " />
            <TextBlock Text="{Binding Value1}" />
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <TextBlock Text="!!Value1: " />
            <TextBlock Text="{Binding !!Value1}" />
        </StackPanel>

        <StackPanel Orientation="Horizontal">
            <TextBlock Text="Value2: " />
            <TextBlock Text="{Binding Value2}" />
        </StackPanel>
        <StackPanel Orientation="Horizontal">
            <TextBlock Text="!!Value2: " />
            <TextBlock Text="{Binding !!Value2}" />
        </StackPanel>
    </StackPanel>
</Window>
MainWindowViewModel.cs
namespace MultiBindingBug.ViewModels;

public partial class MainWindowViewModel
{
    public object? Value1 { get; } = 0;
    public object? Value2 { get; } = null;
}

Steps to reproduce:

  1. Run the application. Avalonia 11.1.0-rc1 displays:
    image

Expected behavior

Avalonia 11.0.10 displays:
image

Avalonia version

11.1.0-rc1

OS

Windows

Additional context

No response

@stogle stogle added the bug label Jun 19, 2024
@maxkatz6
Copy link
Member

"!!" wasn't a defined and documented feature before. I would recommend using more explicit ObjectConverters.IsNotNull.

@stogle
Copy link
Contributor Author

stogle commented Jun 20, 2024

"!!" wasn't a defined and documented feature before. I would recommend using more explicit ObjectConverters.IsNotNull.

It was documented. From the link I posted above:

"You can also use the negation operator twice. For example, where you want to perform the conversion from integer to Boolean, and then negate that value."

This is not equivalent to any of the ObjectConverters.

@maxkatz6
Copy link
Member

It was documented

TIL, thanks. Either way, I agree that it's a regression.

This is not equivalent to any of the ObjectConverters.

True, I was referring specifically to the "null to boolean" conversion.

@grokys
Copy link
Member

grokys commented Jun 21, 2024

Definitely looks like a regression introduced by the binding system refactor, will investigate.

Not sure if this is a showstopper and needs to be fixed for 11.0.0 or whether it can wait for 11.0.1 though.

@grokys
Copy link
Member

grokys commented Jun 21, 2024

We've decided that this issue is a serious enough regression that it necessitates an rc2.

grokys added a commit that referenced this issue Jun 24, 2024
The previous design assumed that a source of `null` was an invalid input to all types of expression nodes. It turned out that there was a single exception to this rule: the `!` operator can in fact operate on a null value. With this new design we instead have to explicitly check for a null value in every override of `OnSourceChanged ` except in `LogicalNotNode`.

Due to this change, we also have to distinguish between `null` and `(unset)` in `ExpressionNode.SetSource` as well.

Fixes #16071
maxkatz6 pushed a commit that referenced this issue Jun 26, 2024
* Add tests for binding negation operator.

Expected results come from 11.0.x branch.

* Unset and null need to be distinct.

Also remove `ExpressionNode.Reset` as it was doing unneeded stuff and should just be the same as `SetSource(AvaloniaProperty.UnsetValue, null);`.

* Make ExpressionNode.OnSourceChanged accept null.

The previous design assumed that a source of `null` was an invalid input to all types of expression nodes. It turned out that there was a single exception to this rule: the `!` operator can in fact operate on a null value. With this new design we instead have to explicitly check for a null value in every override of `OnSourceChanged ` except in `LogicalNotNode`.

Due to this change, we also have to distinguish between `null` and `(unset)` in `ExpressionNode.SetSource` as well.

Fixes #16071

* Fix comment.
grokys added a commit that referenced this issue Jun 28, 2024
* Add tests for binding negation operator.

Expected results come from 11.0.x branch.

* Unset and null need to be distinct.

Also remove `ExpressionNode.Reset` as it was doing unneeded stuff and should just be the same as `SetSource(AvaloniaProperty.UnsetValue, null);`.

* Make ExpressionNode.OnSourceChanged accept null.

The previous design assumed that a source of `null` was an invalid input to all types of expression nodes. It turned out that there was a single exception to this rule: the `!` operator can in fact operate on a null value. With this new design we instead have to explicitly check for a null value in every override of `OnSourceChanged ` except in `LogicalNotNode`.

Due to this change, we also have to distinguish between `null` and `(unset)` in `ExpressionNode.SetSource` as well.

Fixes #16071

* Fix comment.
@stogle
Copy link
Contributor Author

stogle commented Jul 8, 2024

I've confirmed that this issue is fixed for me in 11.1.0-RC2.

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

Successfully merging a pull request may close this issue.

3 participants