CommunityToolkit/Maui.Markup

[BUG] TextAlignment Extension Throwing Compiler Error

danielftz opened this issue · 11 comments

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

if I extend the base Label class and implement the ILabel interface,

public class MyLabel : Label, ILabel
{

}

if I try to use the extended MyLabel class, text alignment extension will throw this exception

public MainPage()
{
	Content = new MyLabel
	{

	}.Center().TextCenter();
}
[CS0121](classifiedtextelementview-invokeaction:0): The call is ambiguous between the following methods or properties:
 'TextAlignmentExtensions_MyLabel.TextCenter<TAssignable>(TAssignable)' and 
'TextAlignmentExtensions_Label.TextCenter<TAssignable>(TAssignable)'

Expected Behavior

No compiler Error

Steps To Reproduce

See above

Link to public reproduction project repository

https://github.com/danielftz/BugExample.MCTMarkupTextAlignment

Environment

- .NET MAUI C# Markup CommunityToolkit:
- OS: MacOS
- .NET MAUI: Latest CUrrent

Anything else?

No response

I'm confused by the example. Why are you inheriting from Label and also implementing ILabel? Label already implements ILabel; there's no need to declare this interface in your custom control.

Label implements ILabel, and subsequently has already implemented ITextAlignment, so it doesn't make much sense to inherit from Label and explicitly implement ILabel.

You'll achieve the same UI control if you only inherit from Label and remove ILabel, which is the better, more pragmatic, approach.

Does this error only happen when you inherit from the ITextAlignment interface (ILabel inherits from ITextAlignment) on a control that has already implemented ITextAlignment?

I agree that this not a scenario we planned for when creating the ITextAlignment source generator, but it shouldn't be a scenario implemented in any code base.

I'm confused by the example. Why are you inheriting from Label and also implementing ILabel? Label already implements ILabel; there's no need to declare this interface in your custom control.

Label implements ILabel, and subsequently has already implemented ITextAlignment, so it doesn't make much sense to inherit from Label and explicitly implement ILabel.

You'll achieve the same UI control if you only inherit from Label and remove ILabel, which is the better, more pragmatic, approach.

Does this error only happen when you inherit from the ITextAlignment interface (ILabel inherits from ITextAlignment) on a control that has already implemented ITextAlignment?

I agree that this not a scenario we planned for when creating the ITextAlignment source generator, but it shouldn't be a scenario implemented in any code base.

my actual use case is more like

public class MyLabel : Label, IMyLabel
{
    //Bindable property, etc.
    public bool MyProperty { get; set; }
}

public interface IMyLabel : ILabel
{
     public bool MyProperty { get; }
}

which has the same error.

I believe this is written the same way Maui source code is written in order to implement a Handler architecture.

@danielftz Can you share a link with a sample project?
Friendly reminder that we don't accept zip files, just repos on GitHub or GitLab, more info here

It looks like we're generating code for classes that extends from base types that already have code generated, I guess it's the issue that I mentioned here. But we can just confirm with a reproduction sample

Hi @danielftz. We have added the "needs reproduction" label to this issue, which indicates that we cannot take further action. This issue will be closed automatically in 5 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 3 days. It will be closed if no further activity occurs within 2 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

We haven't received a reproduction sample from you. The issue is closed.

Here is the reproduction sample.

https://github.com/danielftz/BugExample.MCTMarkupTextAlignment

The issue exists for Entry as well, as demonstrated

pictos commented

I could confirm, it's a regression. It worked on version 2.X.0

pictos commented

@Youssef1313 do you want to see if there's a good way to fix that on your implementation? I did some work and fix, but didn't like the solution. I know that you want to avoid Roslyn objects to live more than one pipeline, but I can't see another elegant way to implement the SG. You can see what I did here

@pictos Aside from implementation details, one fundamental problem here is really what the expected behavior is.

If I understand correctly, the whole idea around this generator is to provide extensions in a fluent API shape (ie, myControl.TextCenter() should produce the same type as myControl)

If we no longer generate the extensions when it was generated already for a base type, then (for the example in the issue), the return type of myLabel.TextCenter() is going to be Label instead of MyLabel.

Assuming you're okay with not generating for MyLabel and use the extension generated for the base type, then #205 is my suggestion regarding the fix implementation

My assumption was wrong.

My assumption in the previous comment may be wrong.