Fixed LoggerMessageGenerator message escaping.#123787
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes LoggerMessageGenerator emitting malformed C# string literals by escaping backslashes and all non-printable characters (< 0x20) when generating source, and adds a regression test covering these escaping scenarios.
Changes:
- Update the emitter to escape
\and all control characters (< 0x20) using\xNN(while keeping\r/\nas named escapes). - Add a new theory test validating the generated source is syntactically valid and contains the expected escaped message literal.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs | Extends message escaping to include backslashes and all control characters so generated string literals remain well-formed. |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs | Adds a theory-based regression test for message escaping (backslash, quotes, CR/LF, and \xNN escapes). |
...ns/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs
Outdated
Show resolved
Hide resolved
...ns/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
…Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Moved the diagnostics retrieval after the source text assertion to ensure proper validation of generated diagnostics.
src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
We should get rid of this method and just call Refers to: src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs:580 in 2f6a0f4. [](commit_id = 2f6a0f4, deletion_comment = False) |
|
@tarekgh, pushed your suggested changes and added some additional test coverage. |
tarekgh
left a comment
There was a problem hiding this comment.
Thanks @John-Leitch. I pushed a small test fix, LGTM otherwise.
This PR addresses bug #123786 by escaping all non-printable characters (
< 0x20) in the emitted message.\rand\nare emitted using the same escape sequence, while other non-printable characters are emitted as hex escape sequences e.g.\x00. Escaping of\is also addressed. Finally, an additional test was added to cover these behaviors.