-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Analyzer] Usage of ToString in logging source generator method call sites #78402
[Analyzer] Usage of ToString in logging source generator method call sites #78402
Comments
Tagging subscribers to this area: @dotnet/area-extensions-logging Issue DetailsWe should consider an analyzer that flags use of [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Excessively large timeout `{timeout}`")]
public static partial void ExcessivelyLargeTimeout(this ILogger logger, string timeout);
...
TimeSpan timeout = ...;
logger.ExcessivelyLargeTimeout(timeout.ToString()); the analyzer would flag the
|
CC @Youssef1313 |
Happy to implement once the analyzer is approved. |
Analyzer would detect when "work" is being done in an argument call to an Note that this list includes identifying any parameters being wrapped in a new array to a The analyzer should not fire if it identifies that it is in a scope that has already checked on log levels. A logging method is any of:
Category: Performance |
@buyaa-n I did a quick (not yet tested) prototype (dotnet/roslyn-analyzers#6643). For now, I don't think I'll be able to complete it soon. If anyone wants to pick it and continue on the code I wrote, this is totally fine. Currently, the PR is missing:
|
@Youssef1313 thanks for letting us know and having a draft PR we can use to build this analyzer. |
I'd like to finish this analyzer :) |
@mpidash thanks for willing to help with that. Please ensure doing it as described in #78402 (comment). |
We should consider an analyzer that flags use of
ToString()
in call sites to logging methods and suggests reconsidering the shape of the logging method such that it takes a strongly-typed value. In many situations, logging is disabled or set at a log level that will result in a particular event being disabled, and doing theToString()
at the call site is then often unnecessary allocation / expense. For example, given:the analyzer would flag the
.ToString()
and suggest theExcessivelyLargeTimeout
method should be changed to take aTimeSpan
instead of astring
, with the.ToString()
removed from the call site.Performance rules Category
Severity = suggestion
The text was updated successfully, but these errors were encountered: