540bcd001c
## Review A dedicated reviewer checked the rule description successfully for: - [ ] logical errors and incorrect information - [ ] information gaps and missing content - [ ] text style and tone - [ ] PR summary and labels follow [the guidelines](https://github.com/SonarSource/rspec/#to-modify-an-existing-rule)
116 lines
3.7 KiB
Plaintext
116 lines
3.7 KiB
Plaintext
== Why is this an issue?
|
|
|
|
Utility classes, which are collections of https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/static[`static`] members, are not meant to be instantiated.
|
|
|
|
C# adds an implicit https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/public[`public`] https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/constructors[`constructor`] to every class which does not explicitly define at least one constructor. Hence, at least one https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/protected[`protected`] constructor should be defined if you wish to inherit this utility class. Or the `static` keyword should be added to the class declaration to prevent inheriting it.
|
|
|
|
== How to fix it
|
|
|
|
=== Code examples
|
|
|
|
==== Noncompliant code example
|
|
|
|
[source,csharp,diff-id=1,diff-type=noncompliant]
|
|
----
|
|
public class StringUtils // Noncompliant: implicit public constructor
|
|
{
|
|
public static string Concatenate(string s1, string s2)
|
|
{
|
|
return s1 + s2;
|
|
}
|
|
}
|
|
----
|
|
|
|
or
|
|
|
|
[source,csharp,diff-id=2,diff-type=noncompliant]
|
|
----
|
|
public class StringUtils // Noncompliant: explicit public constructor
|
|
{
|
|
public StringUtils()
|
|
{
|
|
}
|
|
|
|
public static string Concatenate(string s1, string s2)
|
|
{
|
|
return s1 + s2;
|
|
}
|
|
}
|
|
----
|
|
|
|
==== Compliant solution
|
|
|
|
[source,csharp,diff-id=1,diff-type=compliant]
|
|
----
|
|
public static class StringUtils
|
|
{
|
|
public static string Concatenate(string s1, string s2)
|
|
{
|
|
return s1 + s2;
|
|
}
|
|
}
|
|
----
|
|
|
|
or
|
|
|
|
[source,csharp,diff-id=2,diff-type=compliant]
|
|
----
|
|
public class StringUtils
|
|
{
|
|
protected StringUtils()
|
|
{
|
|
}
|
|
|
|
public static string Concatenate(string s1, string s2)
|
|
{
|
|
return s1 + s2;
|
|
}
|
|
}
|
|
----
|
|
|
|
== Resources
|
|
|
|
=== Documentation
|
|
|
|
* https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/static[Static keyword]
|
|
* https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/constructors[Constructors]
|
|
* https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/using-constructors[Using Constructors]
|
|
* https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/access-modifiers[Access Modifiers]
|
|
|
|
ifdef::env-github,rspecator-view[]
|
|
|
|
'''
|
|
== Implementation Specification
|
|
(visible only on this page)
|
|
|
|
=== Message
|
|
|
|
Hide this public constructor by making it ["protected"|"private"].
|
|
|
|
Add a ["protected"|"private"] constructor or the "static" keyword to the class declaration.
|
|
|
|
'''
|
|
== Comments And Links
|
|
(visible only on this page)
|
|
|
|
=== on 24 Jun 2015, 14:09:33 Ann Campbell wrote:
|
|
very minor edits [~tamas.vajk]. Assigned to you for double-check
|
|
|
|
=== on 25 Jun 2015, 06:23:47 Tamas Vajk wrote:
|
|
\[~ann.campbell.2] thanks, it looks good.
|
|
|
|
=== on 25 Jun 2015, 17:39:51 Ann Campbell wrote:
|
|
\[~tamas.vajk] I noticed you overrode the SQALE remediation cost. Is this truly a language-specific difference, or do you think it's wrong in the main task?
|
|
|
|
=== on 26 Jun 2015, 06:24:39 Tamas Vajk wrote:
|
|
\[~ann.campbell.2] In Java you can write call a utility method through an instance, like ``++(new StringUtils()).Concatenate("a", "b")++``, so if you change the visibility of the constructor, then you'll need to change all these calls to ``++StringUtils.Concatenate(...)++``. However, in C# you can't use a static method through an instance, so changing the visibility of the constructor won't affect the code as much as in Java.
|
|
|
|
But thinking about it a bit, the 2 minutes is a bit too short, I'll modify it to 10mn, because you might have useless ``++new StringUtils()++`` calls in the code.
|
|
|
|
=== on 26 Jun 2015, 11:16:22 Ann Campbell wrote:
|
|
Okay [~tamas.vajk]
|
|
|
|
include::../comments-and-links.adoc[]
|
|
|
|
endif::env-github,rspecator-view[]
|