unoplatform / uno.extensions

Libraries to ease common developer tasks associated with building multi-platform mobile, desktop and web applications using Uno Platform or WinAppSDK.
https://platform.uno/
Other
72 stars 46 forks source link

[MVUX] Fix missing 'out' keyword in ICommand binding generation in UNO's MVU #2119

Open lindexi opened 9 months ago

lindexi commented 9 months ago

Current behavior

Can not compile

Expected behavior

Build successfully

How to reproduce it (as minimally and precisely as possible)

In the generated binding code in UNO's MVU, specifically in BindableXxx.cs, ICommand command binding properties are generated for the XxxModel layer. For example, for the following method in XxxModel:

public void Foo(out int n)
{
    n = 10;
}

A corresponding public global::Uno.Extensions.Reactive.IAsyncCommand Foo { get; private set; } property is generated in BindableXxx.cs.

However, when generating ICommand command bindings, the 'out' keyword is not added for 'out' parameters. As in the above code, the current code generation content is as follows:

          var n = reactive_arguments;

          model.Foo(n);

This generated code does not comply with C# syntax and will cause the build to fail with error code CS1620.

The simplest demo to reproduce this issue can be found at: https://github.com/lindexi/lindexi_gd/tree/eeeb023df2f7ab638bafc29d56e1e62a3445cd29/UnoKearqeljikay

Workaround

No response

Works on UWP/WinUI

None

Environment

No response

NuGet package version(s)

<Project ToolsVersion="15.0">
  <ItemGroup>
    <PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
    <PackageVersion Include="Microsoft.Windows.SDK.BuildTools" Version="10.0.22621.756" />
    <PackageVersion Include="Microsoft.WindowsAppSDK" Version="1.4.231008000" />
    <PackageVersion Include="SkiaSharp.Skottie" Version="2.88.6" />
    <PackageVersion Include="SkiaSharp.Views.Uno.WinUI" Version="2.88.6" />
    <PackageVersion Include="Uno.Core.Extensions.Logging.Singleton" Version="4.0.1" />
    <PackageVersion Include="Uno.Extensions.Core.WinUI" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Configuration" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Hosting" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Hosting.WinUI" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Localization" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Localization.WinUI" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Logging.WinUI" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Navigation" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Navigation.WinUI" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Navigation.Toolkit.WinUI" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Reactive" Version="3.0.19" />
    <PackageVersion Include="Uno.Extensions.Reactive.WinUI" Version="3.0.19" />
    <PackageVersion Include="Uno.Material.WinUI" Version="4.0.6" />
    <PackageVersion Include="Uno.Dsp.Tasks" Version="1.2.8" />
    <PackageVersion Include="Uno.Toolkit.WinUI" Version="5.0.19" />
    <PackageVersion Include="Uno.Toolkit.WinUI.Material" Version="5.0.19" />
    <PackageVersion Include="Uno.Resizetizer" Version="1.2.1" />
    <PackageVersion Include="Uno.UI.Adapter.Microsoft.Extensions.Logging" Version="5.0.118" />
    <PackageVersion Include="Uno.UniversalImageLoader" Version="1.9.36" />
    <PackageVersion Include="Uno.WinUI" Version="5.0.118" />
    <PackageVersion Include="Uno.WinUI.Lottie" Version="5.0.118" />
    <PackageVersion Include="Uno.WinUI.DevServer" Version="5.0.118" />
    <PackageVersion Include="Uno.WinUI.Skia.Gtk" Version="5.0.118" />
    <PackageVersion Include="Uno.WinUI.Skia.Wpf" Version="5.0.118" />
  </ItemGroup>
</Project>

Affected platforms

Build tasks

IDE

Visual Studio 2022

IDE version

No response

Relevant plugins

No response

Anything else we need to know?

No response

Youssef1313 commented 9 months ago

Moving to uno.extensions.

cc @dr1rrb

lindexi commented 9 months ago

Thank you @Youssef1313

And sorry I do not open the issues in the right repo.

dr1rrb commented 8 months ago

Hummm didn't saw that, interesting case.

I assume that since the out parameter won't be used, you actually don't need to have a command for that Foo method and it is public only to be consumed by someone else. If so you can add the [Uno.Extensions.Reactive.Commands.Command(false)] attribute on your method to prevent generation (and if you still need it as a command, you should be able to add an overload that does not have that out parameter).

lindexi commented 8 months ago

Thank you @dr1rrb

I think it would be helpful to have more tips.

dr1rrb commented 8 months ago

Ok my suggestion was only to do that:

public void Foo() => Foo(out _); // Generates the command

[Uno.Extensions.Reactive.Commands.Command(false)]
public void Foo(out int n) => n = 10; // Only use the method

But I tested and it actually doesn't work neither, it will still generate invalid code :(

Then unfortunately for now I can only suggest you to avoid usage of the out keyword (as well as in and ref) on public and internal methods (reminder: if your method is private, nothing is generated for it).

So to workaround that issue in your example, you should do something like:

[Uno.Extensions.Reactive.Commands.Command(false)]
public int Foo()
{
    return 10;
}

And if you need more than one return value, then you should use value tuples, e..g:

[Uno.Extensions.Reactive.Commands.Command(false)]
public (int myInt, string myStr) Foo()
{
    return (10, "hello world");
}