Skip to content

[Binding SG] Dogfood binding source generator in the MAUI codebase#23393

Merged
simonrozsival merged 9 commits intodotnet:net9.0from
simonrozsival:use-binding-source-generator-in-product
Jul 23, 2024
Merged

[Binding SG] Dogfood binding source generator in the MAUI codebase#23393
simonrozsival merged 9 commits intodotnet:net9.0from
simonrozsival:use-binding-source-generator-in-product

Conversation

@simonrozsival
Copy link
Copy Markdown
Member

@simonrozsival simonrozsival commented Jul 2, 2024

Description of Change

This PR replaces as many uses of TypedBinding.ForSingleNestingLevel(...) (introduced in #20567) with the source generated interceptors where possible.

Some observations:

Issues Fixed

Contributes to #22384

/cc @jkurdek @vitek-karas @jonathanpeppers @StephaneDelcroix

@simonrozsival simonrozsival requested a review from a team as a code owner July 2, 2024 09:33
Comment on lines -8 to -12
<!--
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>
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

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.

</ItemGroup>

<PropertyGroup>
<InterceptorsPreviewNamespaces>$(InterceptorsPreviewNamespaces);Microsoft.Maui.Controls.Generated</InterceptorsPreviewNamespaces>
Copy link
Copy Markdown
Member

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.

@simonrozsival
Copy link
Copy Markdown
Member Author

simonrozsival commented Jul 2, 2024

There is a problem on Windows in the XamlPreCompile target because it doesn't pass InterceptorsPreviewNamespaces to the Csc task.

This is a known bug: dotnet/msbuild#9785

Comment on lines +49 to +70
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; }
}
}

Copy link
Copy Markdown
Member Author

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

@simonrozsival
Copy link
Copy Markdown
Member Author

@StephaneDelcroix I had to update the PR to fix failing CI, please re-approve.

@simonrozsival simonrozsival enabled auto-merge (squash) July 20, 2024 11:17
@simonrozsival simonrozsival merged commit a85362c into dotnet:net9.0 Jul 23, 2024
@samhouts samhouts added the fixed-in-net9.0-nightly This may be available in a nightly release! label Aug 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 13, 2024
@samhouts samhouts added the fixed-in-net8.0-nightly This may be available in a nightly release! label Dec 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

fixed-in-net8.0-nightly This may be available in a nightly release! fixed-in-net9.0-nightly This may be available in a nightly release!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants