Modify rule S3416: add C# and VB.NET (#2528)
* Modify rule S3416: add C# and VB.NET * Add logging frameworks to the list of allowed frameworks * Reverse adding logging frameworks * Fix issues from adoc validation * Review 1 * Fix tabbing * Review 2 * Add list of supported frameworks * Missed renames * Add 'logging' tag * Remove VB.NET * Fix a minor typo --------- Co-authored-by: Gregory Paidis <gregory.paidis@sonarsource.com> Co-authored-by: Gregory Paidis <115458417+gregory-paidis-sonarsource@users.noreply.github.com>
This commit is contained in:
parent
3c1d615467
commit
951c7da4b7
10
rules/S3416/comments-and-links.adoc
Normal file
10
rules/S3416/comments-and-links.adoc
Normal file
@ -0,0 +1,10 @@
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
=== on 25 Nov 2015, 09:22:38 Freddy Mallet wrote:
|
||||
See my comment on relating Google Group thread [~ann.campbell.2]: \https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/sonarqube/7DGhtbqRsHk/j2rDIp6dAwAJ
|
||||
|
||||
=== on 7 Jun 2018, 14:17:52 Andrei Epure wrote:
|
||||
Also requested in \https://groups.google.com/forum/?utm_source=digest&utm_medium=email#!topic/sonarqube/qCK_pZJ7G_Q
|
||||
|
||||
|
58
rules/S3416/csharp/how.adoc
Normal file
58
rules/S3416/csharp/how.adoc
Normal file
@ -0,0 +1,58 @@
|
||||
== How to fix it
|
||||
|
||||
When the logger name is defined by a generic type parameter:
|
||||
|
||||
[source,csharp]
|
||||
----
|
||||
class EnclosingType
|
||||
{
|
||||
private readonly ILogger logger;
|
||||
|
||||
public EnclosingType(ILoggerFactory loggerFactory)
|
||||
{
|
||||
logger = loggerFactory.CreateLogger<AnotherType>(); // Noncompliant
|
||||
logger = loggerFactory.CreateLogger<EnclosingType>(); // Compliant
|
||||
}
|
||||
}
|
||||
----
|
||||
|
||||
When the logger name is defined by an input parameter of type `Type`:
|
||||
|
||||
[source,csharp]
|
||||
----
|
||||
class EnclosingType
|
||||
{
|
||||
private readonly ILogger logger;
|
||||
|
||||
public EnclosingType(ILoggerFactory loggerFactory)
|
||||
{
|
||||
logger = loggerFactory.CreateLogger(typeof(AnotherType)); // Noncompliant
|
||||
logger = loggerFactory.CreateLogger(typeof(EnclosingType)); // Compliant
|
||||
logger = loggerFactory.CreateLogger(GetType()); // Compliant
|
||||
}
|
||||
}
|
||||
----
|
||||
|
||||
When the logger name is a string, derived from a `Type`:
|
||||
|
||||
[source,csharp]
|
||||
----
|
||||
class EnclosingType
|
||||
{
|
||||
private readonly ILogger logger;
|
||||
|
||||
public EnclosingType(ILoggerFactory loggerFactory)
|
||||
{
|
||||
logger = loggerFactory.CreateLogger(typeof(AnotherType).Name); // Noncompliant
|
||||
logger = loggerFactory.CreateLogger(typeof(AnotherType).FullName); // Noncompliant
|
||||
logger = loggerFactory.CreateLogger(nameof(AnotherType)); // Noncompliant
|
||||
// Fix by referring to the right type
|
||||
logger = loggerFactory.CreateLogger(typeof(EnclosingType).Name); // Compliant
|
||||
logger = loggerFactory.CreateLogger(typeof(EnclosingType).FullName); // Compliant
|
||||
logger = loggerFactory.CreateLogger(nameof(EnclosingType)); // Compliant
|
||||
// or by retrieving the right type dynamically
|
||||
logger = loggerFactory.CreateLogger(GetType().FullName); // Compliant
|
||||
}
|
||||
}
|
||||
----
|
||||
|
4
rules/S3416/csharp/metadata.json
Normal file
4
rules/S3416/csharp/metadata.json
Normal file
@ -0,0 +1,4 @@
|
||||
{
|
||||
"title": "Loggers should be named for their enclosing types",
|
||||
"quickfix": "targeted"
|
||||
}
|
3
rules/S3416/csharp/rule.adoc
Normal file
3
rules/S3416/csharp/rule.adoc
Normal file
@ -0,0 +1,3 @@
|
||||
:language: csharp
|
||||
|
||||
include::../rule-dotnet.adoc[]
|
13
rules/S3416/csharp/why-code-example.adoc
Normal file
13
rules/S3416/csharp/why-code-example.adoc
Normal file
@ -0,0 +1,13 @@
|
||||
[source,csharp]
|
||||
----
|
||||
class EnclosingType
|
||||
{
|
||||
private readonly ILogger logger;
|
||||
|
||||
public EnclosingType(ILoggerFactory loggerFactory)
|
||||
{
|
||||
logger = loggerFactory.CreateLogger<AnotherType>(); // Noncompliant
|
||||
logger = loggerFactory.CreateLogger<EnclosingType>(); // Compliant
|
||||
}
|
||||
}
|
||||
----
|
13
rules/S3416/csharp/why-exception-example.adoc
Normal file
13
rules/S3416/csharp/why-exception-example.adoc
Normal file
@ -0,0 +1,13 @@
|
||||
[source,csharp]
|
||||
----
|
||||
class EnclosingType
|
||||
{
|
||||
private readonly ILogger logger;
|
||||
|
||||
EnclosingType(ILoggerFactory loggerFactory)
|
||||
{
|
||||
logger = loggerFactory.CreateLogger("My cross-type logging category"); // Compliant
|
||||
logger = loggerFactory.CreateLogger(AComplexLogicToFindTheRightType()); // Compliant
|
||||
}
|
||||
}
|
||||
----
|
@ -1,34 +1,2 @@
|
||||
{
|
||||
"title": "Loggers should be named for their enclosing classes",
|
||||
"type": "CODE_SMELL",
|
||||
"code": {
|
||||
"impacts": {
|
||||
"MAINTAINABILITY": "LOW"
|
||||
},
|
||||
"attribute": "IDENTIFIABLE"
|
||||
},
|
||||
"status": "ready",
|
||||
"remediation": {
|
||||
"func": "Constant\/Issue",
|
||||
"constantCost": "5min"
|
||||
},
|
||||
"tags": [
|
||||
"confusing"
|
||||
],
|
||||
"extra": {
|
||||
"replacementRules": [
|
||||
|
||||
],
|
||||
"legacyKeys": [
|
||||
|
||||
]
|
||||
},
|
||||
"defaultSeverity": "Minor",
|
||||
"ruleSpecification": "RSPEC-3416",
|
||||
"sqKey": "S3416",
|
||||
"scope": "Main",
|
||||
"defaultQualityProfiles": [
|
||||
"Sonar way"
|
||||
],
|
||||
"quickfix": "unknown"
|
||||
}
|
@ -36,20 +36,12 @@ ifdef::env-github,rspecator-view[]
|
||||
|
||||
Update this logger to use the current class.
|
||||
|
||||
|
||||
=== Highlighting
|
||||
|
||||
Xxx.class
|
||||
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
=== on 25 Nov 2015, 09:22:38 Freddy Mallet wrote:
|
||||
See my comment on relating Google Group thread [~ann.campbell.2]: \https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/sonarqube/7DGhtbqRsHk/j2rDIp6dAwAJ
|
||||
|
||||
=== on 7 Jun 2018, 14:17:52 Andrei Epure wrote:
|
||||
Also requested in \https://groups.google.com/forum/?utm_source=digest&utm_medium=email#!topic/sonarqube/qCK_pZJ7G_Q
|
||||
include::../comments-and-links.adoc[]
|
||||
|
||||
endif::env-github,rspecator-view[]
|
||||
|
@ -1,2 +1,33 @@
|
||||
{
|
||||
"title": "Loggers should be named for their enclosing classes",
|
||||
"type": "CODE_SMELL",
|
||||
"status": "ready",
|
||||
"code": {
|
||||
"impacts": {
|
||||
"MAINTAINABILITY": "LOW"
|
||||
},
|
||||
"attribute": "IDENTIFIABLE"
|
||||
},
|
||||
"remediation": {
|
||||
"func": "Constant\/Issue",
|
||||
"constantCost": "5min"
|
||||
},
|
||||
"tags": [
|
||||
"confusing",
|
||||
"logging"
|
||||
],
|
||||
"extra": {
|
||||
"replacementRules": [
|
||||
],
|
||||
"legacyKeys": [
|
||||
]
|
||||
},
|
||||
"defaultSeverity": "Minor",
|
||||
"ruleSpecification": "RSPEC-3416",
|
||||
"sqKey": "S3416",
|
||||
"scope": "Main",
|
||||
"defaultQualityProfiles": [
|
||||
"Sonar way"
|
||||
],
|
||||
"quickfix": "unknown"
|
||||
}
|
||||
|
21
rules/S3416/rspecator-dotnet.adoc
Normal file
21
rules/S3416/rspecator-dotnet.adoc
Normal file
@ -0,0 +1,21 @@
|
||||
ifdef::env-github,rspecator-view[]
|
||||
|
||||
'''
|
||||
== Implementation Specification
|
||||
(visible only on this page)
|
||||
|
||||
=== Message
|
||||
|
||||
Update this logger to use its enclosing type.
|
||||
|
||||
=== Highlighting
|
||||
|
||||
XXX | typeof(XXX) | typeof(XXX).FullName
|
||||
|
||||
'''
|
||||
== Comments And Links
|
||||
(visible only on this page)
|
||||
|
||||
include::comments-and-links.adoc[]
|
||||
|
||||
endif::env-github,rspecator-view[]
|
57
rules/S3416/rule-dotnet.adoc
Normal file
57
rules/S3416/rule-dotnet.adoc
Normal file
@ -0,0 +1,57 @@
|
||||
== Why is this an issue?
|
||||
|
||||
It is a well-established convention to name each logger after its enclosing type. This rule raises an issue when the convention is not respected.
|
||||
|
||||
include::{language}/why-code-example.adoc[]
|
||||
|
||||
Not following such a convention can result in confusion and logging misconfiguration.
|
||||
|
||||
For example, the person configuring the log may attempt to change the logging behavior for the `MyNamespace.EnclosingType` type, by overriding defaults for the logger named after that type.
|
||||
|
||||
[source,json]
|
||||
----
|
||||
{
|
||||
"Logging": {
|
||||
"LogLevel": {
|
||||
"Default": "Error",
|
||||
"MyNamespace.EnclosingType": "Debug"
|
||||
}
|
||||
}
|
||||
}
|
||||
----
|
||||
|
||||
However, if the convention is not in place, the override would not affect logs from `MyNamespace.EnclosingType`, since they are made via a logger with a different name.
|
||||
|
||||
Moreover, using the same logger name for multiple types prevents the granular configuration of each type's logger, since there is no way to distinguish them in configuration.
|
||||
|
||||
The rule targets the following logging frameworks:
|
||||
* https://learn.microsoft.com/en-us/dotnet/core/extensions/logging[Microsoft Extensions Logging]
|
||||
* https://logging.apache.org/log4net/[Apache log4net]
|
||||
* https://nlog-project.org/[NLog]
|
||||
|
||||
=== Exceptions
|
||||
|
||||
The rule doesn't raise issues when custom handling of logging names is in place, and the logger name is not derived from a `Type`.
|
||||
|
||||
include::{language}/why-exception-example.adoc[]
|
||||
|
||||
include::{language}/how.adoc[]
|
||||
|
||||
== Resources
|
||||
|
||||
=== Documentation
|
||||
|
||||
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/core/diagnostics/logging-tracing[.NET logging and tracing]
|
||||
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/core/extensions/logging?tabs=command-line#log-category[Logging in C# and .NET - Log category]
|
||||
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/core/extensions/logging?tabs=command-line#configure-logging[Logging in C# and .NET - Configure logging]
|
||||
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.logging.ilogger-1[ILogger<TCategoryName> Interface]
|
||||
* Apache Logging - https://logging.apache.org/log4net/[Apache log4net]
|
||||
* NLog - https://nlog-project.org/[Flexible & free open-source logging for .NET]
|
||||
|
||||
=== Articles & blog posts
|
||||
|
||||
* Raygun Blog - https://raygun.com/blog/c-sharp-logging-best-practices/[C# logging: Best practices in 2023 with examples and tools]
|
||||
* Apache Logging - https://logging.apache.org/log4net/release/manual/configuration.html[Apache log4net Manual - Configuration]
|
||||
* GitHub NLog repository - https://github.com/nlog/nlog/wiki/Tutorial#best-practices-for-using-nlog[Best practices for using NLog]
|
||||
|
||||
include::rspecator-dotnet.adoc[]
|
Loading…
x
Reference in New Issue
Block a user