Tag Helper Rewriting is brittle

[333fred]

As demonstrated in https://github.com/dotnet/razor/issues/10186 (numerous examples) and https://github.com/dotnet/razor/issues/10426, the tag helper attribute value rewriting portion of the razor compiler is very brittle. This is actually a long-standing issue that was more broadly spotlighted by our changes to runtime code generation to make it more specific; there are several examples of attribute values that have been broken for many years. This is due to how tag helper attribute value parsing works: the attributes are interpreted as HTML at first. Then, if the tag is determined to be a tag helper, rather than a pure HTML tag, and if the attribute is mapped to a property of that tag helper, we reinterpret the provided attribute value as if it was implicit C# expression. Unfortunately, this is not really possible to do without reparsing the entire rest of the document, which the Razor compiler does not do; further, it would be a major performance regression if we attempted to do so. Here are a few examples of where this has been broken for several major versions:

<taghelper objectprop='"'/> am I a string value?"' />
<taghelper stringprop='"'"' />
<taghelper objectprop='@"string"' />

The main purpose of this issue is try and figure out how far we want to go in fixing these bugs, vs simply documenting them as errata and leaving them unfixed. The last of these examples seems like it will be fixable without much trouble, but the first two would require major rework of the rewriter and downstream processing stages to work. Thoughts and opinions are welcome.

[jjonescz]

I do not think the first two examples are broken. The current parsing makes sense to me - if you do not use explicit C# transition @(...), we parse as HTML, i.e., if the attribute value starts with quote ', you cannot use that inside. We should spec it like that.

[jaredpar]

. We should spec it like that.

Generally agree. Essentially within a tag helper and you want C# behavior then use the @(...) transition to be explicit. Otherwise your results may vary due to the issues outlined in the text.

[jjonescz]

Note that .razor component parsing behaves equivalently - e.g., given a Component1 with [Parameter] public object Param { get; set; }, then:

<Component1 Param='"'/> am I a string value?"' />

is parsed as Component1 with Param = '"' plus HTML markup am I a string value?"' /> outside of the component. (And it is a compile time error because " is not a valid C# object value.)

[emperador-ming]

As a user, I would like these cases not to compile or, at the very least, raise a warning recommending an explicit @(...) transition.

Thanks so much for the good work, guys.