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

Avoid compiler error when using init properties with BindingSourceGenerator #27655

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

rabuckley
Copy link
Contributor

Always emit a throwing setter action for init properties.

Closes #27654

@rabuckley rabuckley requested a review from a team as a code owner February 9, 2025 15:59
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Feb 9, 2025
@jsuarezruiz jsuarezruiz added the area-xaml XAML, CSS, Triggers, Behaviors label Feb 10, 2025
@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@StephaneDelcroix
Copy link
Contributor

isn't there anything diagnostic we can send at sourceGen time to avoid runtime exception ?

@rabuckley
Copy link
Contributor Author

isn't there anything diagnostic we can send at sourceGen time to avoid runtime exception ?

This implementing the equivalent behaviour to get-only properties.

If we wanted to emit diagnostics it should be for all the cases where the target property isn't writable and ShouldUseSetter will return true. It can be done, but that's a more involved change because the generator doesn't currently inspect the BindingMode IIRC.

@simonrozsival
Copy link
Member

It can be done, but that's a more involved change because the generator doesn't currently inspect the BindingMode IIRC.

Yes, that's correct. Inspecting the binding mode is tricky because there are several options:

  • the SetBinding/Binding.Create method can be called with a constant (mode: BindingMode.TwoWay) in which case we could statically analyze if that mode is compatible with the property
  • in the case when it is BindingMode.Default, we would have to analyze the BindableProperty and check its default binding mode (probably still doable)
  • but when the mode is passed as a variable, we don't know the mode at compile time.

So we could do a best effort implementation that will produce a warning when it is clear that the binding mode is not compatible with the target property.

Then there is also the option to just generate empty setters without throwing any exception when the property isn't writable. That might make it easier to adopt this feature.

/cc @jkurdek

@StephaneDelcroix
Copy link
Contributor

  • but when the mode is passed as a variable, we don't know the mode at compile time.

that almost never happen, and we could warn on those cases

@PureWeen PureWeen added this to the .NET 9 SR4 milestone Feb 10, 2025
@rmarinho
Copy link
Member

Failing tests not related

@rmarinho rmarinho merged commit 8e5ef59 into dotnet:main Feb 10, 2025
120 of 123 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-xaml XAML, CSS, Triggers, Behaviors community ✨ Community Contribution
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

BindingSourceGenerator should emit throwing setter when target property is init only
7 participants