Allow @@ as a fallback #10752
Allow @@ as a fallback #10752
Conversation
… the new lexer, these will once again be disallowed; attempting to use runtime compilation will necessitate using the native lexer.
| AssertDocumentNodeMatchesBaseline(generated.CodeDocument); | ||
| AssertCSharpDocumentMatchesBaseline(generated.CodeDocument); | ||
| CompileToAssembly(generated, | ||
| // x:\dir\subdir\Test\TestComponent.cshtml(1,28): error CS0103: The name 'Html' does not exist in the current context |
There was a problem hiding this comment.
Consider marking the input as cshtml to avoid this error (by default in ComponentCodeGenerationTestBase you are testing in .razor mode I think)
There was a problem hiding this comment.
That produces different errors with our test host setup that I don't feel are worth chasing down. I'll just leave this as is.
| CSharpCode - (2:0,2 [67] x:\dir\subdir\Test\TestComponent.cshtml) | ||
| LazyIntermediateToken - (2:0,2 [67] x:\dir\subdir\Test\TestComponent.cshtml) - CSharp - var validationMessage = @Html.ValidationMessage("test", "", new { | ||
| CSharpCode - (70:0,70 [39] x:\dir\subdir\Test\TestComponent.cshtml) | ||
| LazyIntermediateToken - (70:0,70 [39] x:\dir\subdir\Test\TestComponent.cshtml) - CSharp - @class = "invalid-feedback" }, "div"); |
There was a problem hiding this comment.
Shouldn't the whole statement be turned into one CSharpCode node rather than two?
There was a problem hiding this comment.
This is the crux of the change. It's best shown by this diff: 8663ad3#diff-a89dc29d4e0f7df972485727da00b99a98e3fe77929e68f75d1bc6cb71f621cc. We go from one statement to 2 statements, with an ephemeral node splitting them.
There was a problem hiding this comment.
Yes, I see. I'm just wondering whether that's needed, or it would be possible and better to have a single statement rather than two. E.g., aren't there going to be problems with the statements split into two like that (e.g., what if some diagnostic spans both of the generated statements)? But it's just a question / nit considering the change is a backwards-compat thing only.
There was a problem hiding this comment.
This is essentially replicating what the old code would have here; I'd rather keep it as is, since this is a backcompat issue.
| Debug.Assert(identifier.Kind is SyntaxKind.Identifier or SyntaxKind.Keyword); | ||
| // We special case @@identifier because the old compiler behavior was to simply accept it and treat it as if it was @identifier. While | ||
| // this isn't legal, the runtime compiler doesn't handle @identifier correctly. We'll continue to accept this for now, but will potentially | ||
| // break it in the future when we move to the roslyn lexer and the runtime/compiletime split is much greater. |
There was a problem hiding this comment.
Did we want to add a warning for @@?
There was a problem hiding this comment.
I believe we said we'd look at that as a followup when Dan was back.
|
@chsienki @dotnet/razor-compiler for a second review. |
* upstream/main: (71 commits) Fix after merge PR feedback Bump Roslyn version Moving formatting service to common layer (dotnet#10761) Move GetSyntaxTree to document snapshot Inject file path service into the document snapshot Remove code document parameter and just use document snapshot Update NOTICE.txt (dotnet#10768) Allow @@ as a fallback (dotnet#10752) Rework how we get generated documents Directly test the component definition service in cohosting Add missing test case Defer to C# for component attribute GTD in cohosting Allow LSP and cohosting to provide specialized methods to get a syntax tree for a C# document Dev Container (dotnet#10751) Use a proper Try pattern Add tests for co-hosted GTD Rework IDocumentPositionInfoStrategy and use correctly in co-hosted GTD Add DocumentMappingSerice to RazorDocumentServiceBase Move IDocumentPositionInfoStrategy and friends to Workspaces layer ...
Previously, #10232 unified our handling of the @ character. While we accepted that there would be some breaking changes, we've decided to partially support one of the things that breaks, @@, in order to make lessen the severity of the breaking change, particularly as there are some scenarios with runtime compilation that become unable to compile and satisfy both versions. We will break this again in the future with the new lexer, and using runtime compilation will require use of the older lexer, which will eventually be deprecated. Fixes dotnet/sdk#42730.