Correctly stitch @ characters to following identifiers when rewriting tag helpers#10331
Merged
Correctly stitch @ characters to following identifiers when rewriting tag helpers#10331
@ characters to following identifiers when rewriting tag helpers#10331Conversation
333fred
commented
May 1, 2024
Comment on lines
-496
to
-504
| #line (26,25)-(26,26) "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ComplexTagHelpers.cshtml" | ||
| @ | ||
|
|
||
| #line default | ||
| #line hidden | ||
| #nullable disable | ||
| #nullable restore | ||
| #line (26,26)-(26,49) "TestFiles/IntegrationTests/CodeGenerationIntegrationTest/ComplexTagHelpers.cshtml" | ||
| DateTimeOffset.Now.Year |
Member
Author
There was a problem hiding this comment.
If we had been validating C# diagnostics, this would have been caught before FUSE was merged. More priority to #10247.
…ng tag helpers Because of how we were rewriting tag helper attribute values, we didn't output a single, unified token when moving an implicit C# transition back to standard C#. This meant that, when we emit `#line` directives, we considered the `@` and the identifier that followed it to be separate tokens; when we added more precise info for FUSE, this then meant that they were emitted on separate lines during runtime codegen and everything fell apart. To fix this, we now unify those implicit transition tokens, rather than trying to keep them as separate tokens. Fixes #10186, for real this time.
69ae12c to
6517c6d
Compare
Member
Author
|
@dotnet/razor-compiler for reviews please |
Member
Author
|
@dotnet/razor-compiler for a second review |
jjonescz
approved these changes
May 2, 2024
333fred
added a commit
that referenced
this pull request
May 9, 2024
* upstream/main: (374 commits) Don't use CancellationTokenSource for disposal Support Linked Editing Ranges in Cohosting. (#10349) Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2445915 Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2445915 Update 17.11 config after VS snap Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20240502.1 Add test Don't think we need this any more Remove Base64 helpers now that they aren't used Whitespace is not significant, but it is important Add ProjectKey APIs for representing unknown projects Fix up serialization tests RazorProjectInfo.SerializationFilePath -> ProjectKey Clean up places were an invalid ProjectKey is used Move ProjectKey to MS.ANC.Razor.ProjectEngineHost Remove ProjectKey static construction methods Protect against exceptions when deleting files Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2443298 Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2443298 Correctly stitch `@` characters to following identifiers when rewriting tag helpers (#10331) ...
333fred
added a commit
to 333fred/razor
that referenced
this pull request
May 20, 2024
…ng tag helpers (dotnet#10331) Because of how we were rewriting tag helper attribute values, we didn't output a single, unified token when moving an implicit C# transition back to standard C#. This meant that, when we emit `#line` directives, we considered the `@` and the identifier that followed it to be separate tokens; when we added more precise info for FUSE, this then meant that they were emitted on separate lines during runtime codegen and everything fell apart. To fix this, we now unify those implicit transition tokens, rather than trying to keep them as separate tokens. Fixes dotnet#10186, for real this time. (cherry picked from commit 26ce354)
333fred
added a commit
that referenced
this pull request
May 21, 2024
* Correctly stitch `@` characters to following identifiers when rewriting tag helpers (#10331) Because of how we were rewriting tag helper attribute values, we didn't output a single, unified token when moving an implicit C# transition back to standard C#. This meant that, when we emit `#line` directives, we considered the `@` and the identifier that followed it to be separate tokens; when we added more precise info for FUSE, this then meant that they were emitted on separate lines during runtime codegen and everything fell apart. To fix this, we now unify those implicit transition tokens, rather than trying to keep them as separate tokens. Fixes #10186, for real this time. (cherry picked from commit 26ce354) * Update baseline for 17.10 code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because of how we were rewriting tag helper attribute values, we didn't output a single, unified token when moving an implicit C# transition back to standard C#. This meant that, when we emit
#linedirectives, we considered the@and the identifier that followed it to be separate tokens; when we added more precise info for FUSE, this then meant that they were emitted on separate lines during runtime codegen and everything fell apart. To fix this, we now unify those implicit transition tokens, rather than trying to keep them as separate tokens. Fixes #10186, for real this time.