XAMLC is failing on valid StringFormat

[davidortinau]

Description

I upgraded the FormsGallery from 3.4.0 to 3.6-pre1 and it now throws the XAMLC error:

This is happening everywhere StringFormat is being used, such as:

<Label Text="{Binding Birthday, StringFormat='Born {0:d}'}" />

StringFormat from the docs:
https://docs.microsoft.com/en-us/xamarin/xamarin-forms/app-fundamentals/data-binding/string-formatting#the-stringformat-property

When I disabled XAMLC the app runs fine.

Expected Behavior

XAMLC passes

Actual Behavior

XAMLC produces the above error.

Basic Information

• Version with issue: 3.6-pre1
• Last known good version: 3.4.0 (I didn't test 3.5)

Reproduction Link

https://developer.xamarin.com/samples/xamarin-forms/FormsGallery/

[StephaneDelcroix]

This is indeed a breaking change, introduced at #4723, to fix #2756. Your StringFormat is not valid

[samhouts]

I believe we should only be throwing an exception if the StringFormat starts with a curly brace, not if it contains one.

[stevehurcombe]

Would this change allow this format, contrived I know, but still valid?

<Label class="RadioText" Text="{Binding OrderNumber, StringFormat='{0} is your order number.'}"></Label>

[StephaneDelcroix]

@stevehurcombe doesn't look valid to me

[stevehurcombe]

What's wrong with it exactly? This is an example from the documentation:

<Label Text="{Binding Source={x:Reference slider},
                              Path=Value,
                              StringFormat='The slider value is {0:F2}'}" />

Are you saying that this isn't valid?

<Label Text="{Binding Source={x:Reference slider},
                              Path=Value,
                              StringFormat='{0:F2} is the value of the slider.'}" />

[stevehurcombe]

OK, so I've been reading into the background of this. The solution doesn't seem like a great one, it's non-obvious and smells. People are going to be tripping up over this error for years to come for little perceived value, even if it is technically correct.

Surely the fact that it's part of an inner single quoted string mitigates the issue? The start of the string is surely Text=". Is there ever a case where this could be considered a markup extension in this location, in the middle of the attribute? Clearly it's not being interpreted as a markup extension by the compiler.

There must be a better way...

[sushihangover]

@StephaneDelcroix @stevehurcombe There must be a better way... I agree, there has to be a better way... Are we really saying that this is the solution in Forms' 4+, `StringFormat='{}{0:N0}'.

Now Invalid:

        <Label Text="{Binding Path=Ticks, StringFormat='{0:N0} ticks since 1/1/1'}" />

Valid:

        <Label Text="{Binding Path=Ticks, StringFormat='{}{0:N0} ticks since 1/1/1'}" />

[jalbertSyncroTech]

This issue shouldn't have been closed. The fix in #5169 Improves the situation, but it still forces edits to StringFormats that are valid according to the XAML spec.

The Microsoft documentation on this is unambiguous:
https://docs.microsoft.com/en-us/openspecs/microsoft_domain_specific_languages/ms-xaml/f8906664-a3d7-4c7f-8f63-bf8041a91e94

The text value becomes the characters up to but not including the next matching quote

Further, the documentation for StringFormat in WPF clearly shows examples both with and without the braces, where the opening empty braces are only used to break out of the markup extension:
https://docs.microsoft.com/en-us/dotnet/api/system.windows.data.bindingbase.stringformat?view=netframework-4.7.2

• DisplayMemberBinding="{Binding Path=Price, StringFormat=Now {0:c}!}"
• StringFormat="{}{0} -- Now only {1:C}!"

I've left a corresponding comment on the original issue reported to Visual Studio
https://developercommunity.visualstudio.com/content/problem/252490/error-in-xamarin-stringformat-intellisense.html

[StephaneDelcroix]

@jalbertSyncroTech that doc doesn't mention escape sequence

• the first WPF example doesn't need the brace, as the string doesn't start with { => same in XF
• the second one requires it => same in XF

I understand the frustration, because some strings used to get correctly parsed by XF and no longer are, but I'm happy that the current XF is more correct than before.

[jalbertSyncroTech]

@StephaneDelcroix I realize this is a complex issue, but the reading of the XAML Markup Extension Parsing doc is unambiguous:
https://docs.microsoft.com/en-us/openspecs/microsoft_domain_specific_languages/ms-xaml/f8906664-a3d7-4c7f-8f63-bf8041a91e94

The text value becomes the characters up to but not including the next matching quote

As an example:
in {Binding StringFormat='FooBar'}, the content FooBar, because it is quoted, is the text assigned to StringFormat. It can never be a markup extension and therefore never needs the opening empty curly braces to qualify it.

[jalbertSyncroTech]

@stevehurcombe I think it does:

I just tested calling string.Format("{}FooBar") and it triggers a FormatException because {} isn't a valid format string. So in fact, when following the spec, StringFormat='{}FooBar' is incorrect and not the other way around.

However, StringFormat={}FooBar is correct as it always was because the opening {} isn't part of the string content.

It all comes down to the single-quote behavior follow the spec.

[jalbertSyncroTech]

I read through the full parser documentation again, and it's even more confusing than the above discussion:

The "Markup Extension Parsing" doesn't define anything about recursive markup extensions when assigning member values, it seems like that behavior is entirely up to parent extension, BindingExtension in this case.

When it comes to single vs. no-quotes, for the no-quotes variation the {} is part of the final text value, so even for StringFormat some further processing must be happening in WPF, and also '{}' is a valid (though obviously not required) format.

The question comes down to how BindingExtension determines how to assign the text value from the parser to the StringFormat member.

[mgoertz-msft]

Yeah, I can kind of see the argument for both but like you said, the spec does not address nested markup extensions, and the WPF compiler produces a build error for the following (with or without quotes):

{Binding StringFormat='{0} Height' ...}

The only way to avoid the build error is to prefix it with the {} escape sequence. That’s why we have always marked it as an error.

[stevehurcombe]

So you're saying this is an edge case issue in the WPF compiler? :-)

This is surely the preferred syntax:
{Binding StringFormat='{0} Height' } and the spec seems to allow it.

[mgoertz-msft]

I choose to think of it more as the WPF compiler setting a precedent. :-)

That said, we don't want to be stuck in the past. If the consensus for Xamarin Forms is to treat any nested "MarkupExtensions" as literal text if they are surrounded by quotes then we will need to make the changes in the language service to no longer try to interpret those as MarkupExtensions either (for Xamarin Forms).

[jalbertSyncroTech]

So you're saying this is an edge case issue in the WPF compiler? :-)

This is surely the preferred syntax:
{Binding StringFormat='{0} Height' } and the spec seems to allow it.

@stevehurcombe I think rather that the spec doesn't make any distinction between quotes and no quotes in the final parsed text - it seems like quotes really just get you the ability to use commas and equal signs without tripping up the parser.

The issue for my team is that the behavior of Xamarin.Forms both before and after this change were different from WPF. Before you didn't need the opening {}, and in fact we incorrectly have a number of StringFormat's that are just a traditional format string without the opening braces. After the original change you needed the opening {} even if the format string's placeholder didn't appear at the beginning, which is also different from WPF.

I think we're good now, since at least we match both the spec and the old WPF behavior, but the ongoing argument is about if Xamarin.Forms can unambiguously identify a string from a markup extension without needing the opening braces then why is it requiring them? In other words, if it's a nested MarkupExtension then you never need the quotes and the convention is to not use them, so why not just make the use of quotes always mean the value should be interpreted as a literal string.

[stevehurcombe]

I think there are two arguments. I, at least, am petitioning for this format to be supported:

{Binding StringFormat='{0} Height' }

We have a chance here to either be famous for 15 minutes on this thread, or be infamous for the next 10 years as newbie XF developers trip over this syntax and end up here!

It might be to the spec, but it's not a good solution IMHO. There is an opportunity to correct that before it goes out into the main releases.