-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[Binding SG] Dogfood binding source generator in the MAUI codebase #23393
[Binding SG] Dogfood binding source generator in the MAUI codebase #23393
Conversation
<!-- | ||
NOTE: Keep this project on C# 9 to avoid changes in overload resolution: | ||
src/Controls/tests/Core.UnitTests/TemplatedItemsListTests.cs(543,11): error CS0121: The call is ambiguous between the following methods or properties: 'Assert.That(TestDelegate, IResolveConstraint)' and 'Assert.That<TActual>(TActual, IResolveConstraint)' | ||
--> | ||
<LangVersion>9.0</LangVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did the problem mentioned here, go away somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find any Assert.That
call in that file. Also the tests passed just fine. This seems outdated to me.
@@ -52,6 +52,16 @@ | |||
<ItemGroup Condition="$(TargetFramework.Contains('-windows')) == true "> | |||
</ItemGroup> | |||
|
|||
<PropertyGroup> | |||
<InterceptorsPreviewNamespaces>$(InterceptorsPreviewNamespaces);Microsoft.Maui.Controls.Generated</InterceptorsPreviewNamespaces> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, if an interceptor is declared outside of a namespace listed here, it will get a C# compiler error.
It sounds like they will rename this to $(InterceptorsNamespaces)
one day:
No problem here, just fyi for later.
There is a problem on Windows in the This is a known bug: dotnet/msbuild#9785 |
…ing-source-generator-in-product
namespace System.Runtime.CompilerServices | ||
{ | ||
using System; | ||
using System.CodeDom.Compiler; | ||
|
||
{{GeneratedCodeAttribute}} | ||
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)] | ||
file sealed class InterceptsLocationAttribute : Attribute | ||
{ | ||
public InterceptsLocationAttribute(string filePath, int line, int column) | ||
{ | ||
FilePath = filePath; | ||
Line = line; | ||
Column = column; | ||
} | ||
|
||
public string FilePath { get; } | ||
public int Line { get; } | ||
public int Column { get; } | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might seem like a weird change but we need to make the attribute file-scoped so that there aren't any collisions when there are assemblies with reference between them and they see each others internals (as in the case of Microsoft.Maui.Controls.Core.dll
and Microsoft.Maui.Controls.Compatibility.dll
…ing-source-generator-in-product
@StephaneDelcroix I had to update the PR to fix failing CI, please re-approve. |
Description of Change
This PR replaces as many uses of
TypedBinding.ForSingleNestingLevel(...)
(introduced in #20567) with the source generated interceptors where possible.Some observations:
public
orinternal
. It is not possible to use theSetBinding<TS, TP>()
method with private types (see the change toPersonViewModel
in RelativeSourceBindingTests.cs)ForSingleNestingLevel
in BaseShellItem.cs and MultiBindingTests.cs because they need [Binding SG] Add an API for creating instances of TypedBinding #23239Issues Fixed
Contributes to #22384
/cc @jkurdek @vitek-karas @jonathanpeppers @StephaneDelcroix