Skip to content

Commit

Permalink
SONARJAVA-2376 Update rule descriptions (#1597)
Browse files Browse the repository at this point in the history
  • Loading branch information
benzonico authored and Wohops committed Jul 21, 2017
1 parent c902365 commit fa9fe66
Show file tree
Hide file tree
Showing 29 changed files with 116 additions and 45 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<p>Proper indentation is a simple and effective way to improve the code's readability. Consistent indentation among the developers within a team also
reduces the differences that are committed to source control systems, making code reviews easier. </p>
<p>By default this rule checks that each block of code is indented, although it does not check the size of the indent. Parameter "indentSize" allows
the expected indent size to be defined. Only the first line of a badly indented section is reported.</p>
<p>This rule raises an issue when indentation does not match the configured value. Only the first line of a badly indented section is reported.</p>
<h2>Noncompliant Code Example</h2>
<p>With an indent size of 2:</p>
<pre>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@ <h2>Compliant Solution</h2>
}
</pre>
<h2>Exceptions</h2>
<p>Override and implementation methods are excluded, as are parameters annotated with <code>@Observes</code>, and methods that are intended to be
overridden.</p>
<p>The rule will not raise issues for unused parameters:</p>
<ul>
<li> that are annotated with <code>@javax.enterprise.event.Observes</code> </li>
<li> in overrides and implementation methods </li>
<li> in interface <code>default</code> methods </li>
<li> in non-private methods that only <code>throw</code> or that have empty bodies </li>
<li> in annotated methods, unless the annotation is <code>@SuppressWarning("unchecked")</code> or <code>@SuppressWarning("rawtypes")</code>, in
which case the annotation will be ignored </li>
</ul>
<pre>
@Override
void doSomething(int a, int b) { // no issue reported on b
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"title": "Locale should be used in String operations",
"type": "BUG",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"title": "Jump statements should not be used unconditionally",
"type": "BUG",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"title": "Fields in a \"Serializable\" class should either be transient or serializable",
"type": "BUG",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<p>Java's garbage collection cannot be relied on to clean up everything. Specifically, connections, streams, files and other classes that implement
the <code>Closeable</code> interface or its super-interface, <code>AutoCloseable</code>, must be manually closed after creation. Further, that
<code>close</code> call must be made in a <code>finally</code> block, otherwise an exception could keep the call from being made. </p>
<p>Connections, streams, files, and other classes that implement the <code>Closeable</code> interface or its super-interface,
<code>AutoCloseable</code>, needs to be closed after use. Further, that <code>close</code> call must be made in a <code>finally</code> block otherwise
an exception could keep the call from being made. Preferably, when class implements <code>AutoCloseable</code>, resource should be created using
"try-with-resources" pattern and will be closed automatically.</p>
<p>Failure to properly close resources will result in a resource leak which could bring first the application and then perhaps the box it's on to
their knees.</p>
<h2>Noncompliant Code Example</h2>
Expand All @@ -10,6 +11,8 @@ <h2>Noncompliant Code Example</h2>
BufferedReader reader = Files.newBufferedReader(path, this.charset)) {
// ...
reader.close(); // Noncompliant
// ...
Files.lines("input.txt").forEach(System.out::println); // Noncompliant: The stream needs to be closed
}

private void doSomething() {
Expand All @@ -28,17 +31,16 @@ <h2>Noncompliant Code Example</h2>
</pre>
<h2>Compliant Solution</h2>
<pre>
private void readTheFile() throws IOException {
Path path = Paths.get(this.fileName);
BufferedReader reader = null;
try {
reader = Files.newBufferedReader(path, this.charset)) {
// ...
} finally {
if (reader != null) {
reader.close();
private void readTheFile(String fileName) throws IOException {
Path path = Paths.get(fileName);
try (BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
reader.readLine();
// ...
}
// ..
try (Stream&lt;String&gt; input = Files.lines("input.txt")) {
input.forEach(System.out::println);
}
}
}

private void doSomething() {
Expand Down Expand Up @@ -71,5 +73,6 @@ <h2>See</h2>
<li> <a href="http://cwe.mitre.org/data/definitions/459.html">MITRE, CWE-459</a> - Incomplete Cleanup </li>
<li> <a href="https://www.securecoding.cert.org/confluence/x/9gFqAQ">CERT, FIO04-J.</a> - Release resources when they are no longer needed </li>
<li> <a href="https://www.securecoding.cert.org/confluence/x/GAGQBw">CERT, FIO42-C.</a> - Close files when they are no longer needed </li>
<li> <a href="https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html">Try With Resources</a> </li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,17 @@ <h2>Noncompliant Code Example</h2>
public void doSometing() {...}
}
</pre>
<h2>Exceptions</h2>
<p>If <code>run()</code> is not overridden in a class extending <code>Thread</code>, it means that starting the thread will actually call
<code>Thread.run()</code>. However, <code>Thread.run()</code> does nothing if it has not been fed with a target <code>Runnable</code>. The rule
consequently ignore classes extending <code>Thread</code> if they are calling, in their constructors, the <code>super(...)</code> constructor with a
proper <code>Runnable</code> target.</p>
<pre>
class MyThread extends Thread { // Compliant - calling super constructor with a Runnable
MyThread(Runnable target) {
super(target); // calling super constructor with a Runnable, which will be used for when Thread.run() is executed
// ...
}
}
</pre>

Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,7 @@ <h2>Compliant Solution</h2>
<h2>See</h2>
<ul>
<li> <a href="http://cwe.mitre.org/data/definitions/476.html">MITRE CWE-476</a> - NULL Pointer Dereference </li>
<li> <a href="https://www.securecoding.cert.org/confluence/x/ZwDOAQ">CERT, EXP01-J.</a> - Do not use a null in a case where an object is required
</li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
"constantCost": "5min"
},
"tags": [
"cwe"
"cwe",
"cert"
],
"standards": [
"CWE"
"CWE",
"CERT"
],
"defaultSeverity": "Major"
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
<p>When a parent class references a static member of a subclass during its own initialization, the results will not be what you expect because the
child class won't exist yet. </p>
<p>In a best-case scenario, you'll see immediate failures in the code as a result. Worst-case, the damage will be more insidious and difficult to
track down.</p>
<p>When a parent class references a member of a subclass during its own initialization, the results might not be what you expect because the child
class might not have been initialized yet. This could create what is known as an "initialisation cycle", or even a deadlock in some extreme cases.</p>
<p>To make things worse, these issues are very hard to diagnose so it is highly recommended you avoid creating this kind of dependencies.</p>
<h2>Noncompliant Code Example</h2>
<pre>
class Parent {
public static final int childVersion = Child.version;
static int field1 = Child.method(); // Noncompliant
static int field2 = 42;

public static void main(String[] args) {
System.out.println(Parent.field1); // will display "0" instead of "42"
}
}

class Child extends Parent {
public static final int version = 6;
static int method() {
return Parent.field2;
}
}
</pre>
<h2>See</h2>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"title": "Classes should not access static members of their own subclasses during initialization",
"type": "BUG",
"title": "Classes should not access their own subclasses during initialization",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand All @@ -12,5 +12,5 @@
"standards": [
"CERT"
],
"defaultSeverity": "Major"
"defaultSeverity": "Critical"
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"title": "Lazy initialization of \"static\" fields should be \"synchronized\"",
"type": "BUG",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand All @@ -9,5 +9,5 @@
"tags": [
"multi-threading"
],
"defaultSeverity": "Major"
"defaultSeverity": "Critical"
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,10 @@ <h2>Noncompliant Code Example</h2>
return null; // Noncompliant
}
</pre>
<h2>See</h2>
<ul>
<li> <a href="http://cwe.mitre.org/data/definitions/476.html">MITRE CWE-476</a> - NULL Pointer Dereference </li>
<li> <a href="https://www.securecoding.cert.org/confluence/x/ZwDOAQ">CERT, EXP01-J.</a> - Do not use a null in a case where an object is required
</li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
"constantCost": "20min"
},
"tags": [
"cwe",
"cert",
"pitfall"
],
"standards": [
"CWE",
"CERT"
],
"defaultSeverity": "Critical"
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ <h2>Noncompliant Code Example</h2>
<pre>
logger.log(Level.DEBUG, "Something went wrong: " + message); // Noncompliant; string concatenation performed even when log level too high to show DEBUG messages

logger.fine("An exception occurred with message: " + message); // Noncompliant

LOG.error("Unable to open file " + csvPath, e); // Noncompliant

Preconditions.checkState(a &gt; 0, "Arg must be positive, but got " + a); // Noncompliant. String concatenation performed even when a &gt; 0
Expand All @@ -22,6 +24,8 @@ <h2>Compliant Solution</h2>
<pre>
logger.log(Level.SEVERE, "Something went wrong: %s ", message); // String formatting only applied if needed

logger.log(Level.FINE, "An exception occurred with message: {}", message);

logger.log(Level.SEVERE, () -&gt; "Something went wrong: " + message); // since Java 8, we can use Supplier , which will be evaluated lazily

LOG.error("Unable to open file {}", csvPath, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,10 @@ <h2>Noncompliant Code Example</h2>
return mix; // Noncompliant; return value is Nonnull, but null is returned.}}
}
</pre>
<h2>See</h2>
<ul>
<li> <a href="http://cwe.mitre.org/data/definitions/476.html">MITRE CWE-476</a> - NULL Pointer Dereference </li>
<li> <a href="https://www.securecoding.cert.org/confluence/x/ZwDOAQ">CERT, EXP01-J.</a> - Do not use a null in a case where an object is required
</li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@
"constantCost": "15min"
},
"tags": [

"cwe",
"cert"
],
"standards": [
"CWE",
"CERT"
],
"defaultSeverity": "Minor"
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,9 @@ <h2>Noncompliant Code Example</h2>
}
}
</pre>
<h2>See</h2>
<ul>
<li> <a href="https://www.securecoding.cert.org/confluence/x/ZQIRAg">CERT, TSM02-J.</a> - Do not use background threads during class initialization
</li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@
},
"tags": [
"multi-threading",
"cert",
"pitfall"
],
"standards": [
"CERT"
],
"defaultSeverity": "Blocker"
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
<li> Rest-assured 2.0 </li>
<li> AssertJ </li>
<li> Hamcrest </li>
<li> Mockito </li>
<li> Spring's <code>org.springframework.test.web.servlet.ResultActions.andExpect()</code> </li>
<li> EasyMock </li>
<li> Truth Framework </li>
<li> Mockito </li>
<li> EasyMock </li>
<li> JMock </li>
<li> WireMock </li>
</ul>
<h2>Noncompliant Code Example</h2>
<pre>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"title": "Static fields should not be updated in constructors",
"type": "BUG",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
<p>This rule raises an issue if on methods that contain only one statement: the <code>return</code> of a constant value. </p>
<h2>Noncompliant Code Example</h2>
<pre>
public int getBestNumber() {
int getBestNumber() {
return 12; // Noncompliant
}
</pre>
<h2>Compliant Solution</h2>
<pre>
public static int bestNumber = 12;
static int bestNumber = 12;
</pre>
<h2>Exceptions</h2>
<p>Methods with annotations, such as <code>@Override</code> and Spring's <code>@RequestMapping</code>, are ignored.</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
value types are introduced in the language.</p>
<p>This rule raises an issue when a <code>Serializable</code> class defines a non-transient, non-static field field whose type is a known serializable
value-based class. Known serializable value-based classes are: all the classes in the <code>java.time</code> package except <code>Clock</code>; the
date classes for alternate calendars: <code>HijrahDate</code>, <code>JapaneseDate</code>, <code>MinguaDate</code>, <code>ThaiBuddhistDate</code>.</p>
date classes for alternate calendars: <code>HijrahDate</code>, <code>JapaneseDate</code>, <code>MinguoDate</code>, <code>ThaiBuddhistDate</code>.</p>
<h2>Noncompliant Code Example</h2>
<pre>
class MyClass implements Serializable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ <h2>Compliant Solution</h2>
</pre>
<h2>See</h2>
<p><a href="https://docs.oracle.com/javase/8/docs/api/java/util/stream/package-summary.html#StreamOps">Stream Operations</a></p>

Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@
"java8"
],
"defaultSeverity": "Major"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ <h2>Noncompliant Code Example</h2>
</pre>
<h2>See</h2>
<p><a href="https://docs.oracle.com/javase/8/docs/api/java/util/stream/package-summary.html#StreamOps">Stream Operations</a></p>

Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@
"java8"
],
"defaultSeverity": "Major"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ <h2>Compliant Solution</h2>
<pre>
boolean hasRed = widgets.stream().anyMatch(w -&gt; w.getColor() == RED);
</pre>

Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@
"clumsy"
],
"defaultSeverity": "Minor"
}
}

0 comments on commit fa9fe66

Please sign in to comment.