Add XML documentation to LoggerMessage generated methods#124258
Add XML documentation to LoggerMessage generated methods#124258
Conversation
Co-authored-by: cincuranet <4540597+cincuranet@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-extensions-logging |
src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
…l mapping Co-authored-by: cincuranet <4540597+cincuranet@users.noreply.github.com>
|
@copilot fix the errors: |
|
@cincuranet is it really worth it to fix? Personally, I am not seeing this adding not adding much value. Also, did you check the extensions source gen if it does the same? |
Co-authored-by: tarekgh <10833894+tarekgh@users.noreply.github.com>
... Fixed. The issue was caused by |
I think it is nice, small quality of life improvement. But if you object, feel free to close the PR.
|
|
@copilot Your "Removed the escaping call since XML documentation comments in C# don't require explicit escaping." was a wrong move. You need to at least escape '\n', '\r', '<', '>', I think. |
I'll leave it to you to decide. /// <summary>
/// <para><b>Message:</b> </para>
/// </summary>
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")]
public static partial void M0(global::Microsoft.Extensions.Logging.ILogger logger, global::Microsoft.Extensions.Logging.LogLevel level)
{
if (logger.IsEnabled(level))
{
logger.Log(
level,
new global::Microsoft.Extensions.Logging.EventId(316638712, nameof(M0)),
new __M0Struct(),
null,
__M0Struct.Format);
}
} |
Co-authored-by: cincuranet <4540597+cincuranet@users.noreply.github.com>
Fixed. Implemented proper XML escaping function that handles Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Code review completed. ✅ LGTM - No blocking issues found. The implementation correctly generates XML documentation, properly escapes all special characters (including |
|
(I'm not sure why the full code-review results weren't posted. If you look at the session log via the "View session" button, you can see it there.) |
src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs
Show resolved
Hide resolved
I think because I didn't include |
…lation Co-authored-by: tarekgh <10833894+tarekgh@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds XML documentation to LoggerMessage source-generated methods to improve the developer experience by showing the log message template and level in IntelliSense tooltips when hovering over generated logging methods.
Changes:
- Added GenMethodDocumentation method to emit XML summary tags with message template and log level
- Implemented EscapeXml method to properly escape XML special characters in message templates
- Refactored GetLogLevelFullName to be a class-level method and created GetLogLevelName for documentation-friendly level names
- Updated all baseline test files to include the new XML documentation output
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs | Main implementation adding GenMethodDocumentation, EscapeXml, and GetLogLevelName methods; refactored GetLogLevelFullName |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithTwoParams.generated.txt | Updated baseline with XML documentation for two-parameter test case |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithSkipEnabledCheck.generated.txt | Updated baseline with XML documentation for skip-enabled-check test case |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClassWithGenericTypesWithAttributes.generated.txt | Updated baseline with XML documentation for nested generic class test case |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClass.generated.txt | Updated baseline with XML documentation for nested class test case |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithMultipleClassesStableOrder.generated.txt | Updated baseline with XML documentation for multiple classes test case |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithMoreThan6Params.generated.txt | Updated baseline with XML documentation for methods with more than 6 parameters |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithLoggerInFieldAndFromPrimaryConstructor.generated.txt | Updated baseline with XML documentation for logger in field test case |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithLoggerFromPrimaryConstructorWithParameterUsedInMethod.generated.txt | Updated baseline with XML documentation for primary constructor with parameter test case |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithLoggerFromPrimaryConstructor.generated.txt | Updated baseline with XML documentation for primary constructor test case |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithDynamicLogLevel.generated.txt | Updated baseline with XML documentation (without level) for dynamic log level test case |
| src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithDefaultValues.generated.txt | Updated baseline with XML documentation for default values test case |
src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: tarekgh <10833894+tarekgh@users.noreply.github.com>
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.