Skip to content

Commit c0bb52b

Browse files
alban-auzeillalban-auzeill
authored
SONARJAVA-4555 Update rules metadata (#4423)
1 parent c6c9fc7 commit c0bb52b

File tree

7 files changed

+219
-58
lines changed

7 files changed

+219
-58
lines changed

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S2154.html‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,49 @@
11
<h2>Why is this an issue?</h2>
2-
<p>If wrapped primitive values (e.g. <code>Integers</code> and <code>Floats</code>) are used in a ternary operator (e.g. <code>a?b:c</code>), both
3-
values will be unboxed and coerced to a common type, potentially leading to unexpected results. To avoid this, add an explicit cast to a compatible
4-
type.</p>
5-
<h3>Noncompliant code example</h3>
6-
<pre>
2+
<p>Using boxed values in a ternary operator does not simply return one operand or the other based on the condition. Instead, the values are unboxed
3+
and coerced to a common type, which can result in a loss of precision when converting one operand from <code>int</code> to <code>float</code> or from
4+
<code>long</code> to <code>double</code>.</p>
5+
<p>While this behavior is expected for arithmetic operations, it may be unexpected for the ternary operator. To avoid confusion or unexpected
6+
behavior, cast to a compatible type explicitly.</p>
7+
<h2>How to fix it</h2>
8+
<h3>Code examples</h3>
9+
<h4>Noncompliant code example</h4>
10+
<p>Cast one of both operands to a common supertype (e.g., <code>Number</code>) to prevent auto-unboxing and, thus, type coercion.</p>
11+
<pre data-diff-id="1" data-diff-type="noncompliant">
712
Integer i = 123456789;
813
Float f = 1.0f;
9-
Number n = condition ? i : f; // Noncompliant; i is coerced to float. n = 1.23456792E8
14+
Number n1 = condition ? i : f; // Noncompliant, unexpected precision loss, n1 = 1.23456792E8
1015
</pre>
11-
<h3>Compliant solution</h3>
12-
<pre>
16+
<h4>Compliant solution</h4>
17+
<pre data-diff-id="1" data-diff-type="compliant">
1318
Integer i = 123456789;
1419
Float f = 1.0f;
15-
Number n = condition ? (Number) i : f; // n = 123456789
20+
Number n1 = condition ? (Number) i : f; // Compliant, cast to Number prevents unboxing
21+
Number n2 = condition ? i : (Number) f; // Compliant, cast to Number prevents unboxing
1622
</pre>
23+
<h4>Noncompliant code example</h4>
24+
<p>If type coercion was your intention, clarify this by casting the operand that would be coerced to the corresponding type explicitly.</p>
25+
<pre data-diff-id="2" data-diff-type="noncompliant">
26+
Integer i = 123456789;
27+
Float f = 1.0f;
28+
Number n1 = condition ? i : f; // Noncompliant, unexpected precision loss, n1 = 1.23456792E8
29+
</pre>
30+
<h4>Compliant solution</h4>
31+
<pre data-diff-id="2" data-diff-type="compliant">
32+
Integer i = 123456789;
33+
Float f = 1.0f;
34+
Number n = condition ? (float) i : f; // Compliant, intentional type coercion with precision loss
35+
</pre>
36+
<h2>Resources</h2>
37+
<h3>Documentation</h3>
38+
<ul>
39+
<li> <a href="https://docs.oracle.com/javase/tutorial/java/nutsandbolts/op2.html">The Java Tutorials: Equality, Relational, and Conditional
40+
Operators</a> </li>
41+
<li> <a href="https://docs.oracle.com/javase/tutorial/java/data/autoboxing.html">The Java Tutorials: Autoboxing and Unboxing</a> </li>
42+
<li> <a href="https://docs.oracle.com/javase/specs/jls/se7/html/jls-5.html">The Java® Language Specification Java SE 7 Edition: Chapter 5.
43+
Conversions and Promotions</a> </li>
44+
</ul>
45+
<h3>Articles &amp; blog posts</h3>
46+
<ul>
47+
<li> <a href="https://www.geeksforgeeks.org/coercion-in-java/">GeeksforGeeks: Coercion in Java</a> </li>
48+
</ul>
1749

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S2167.html‎

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,65 @@
11
<h2>Why is this an issue?</h2>
2-
<p>It is the sign, rather than the magnitude of the value returned from <code>compareTo</code> that matters. Returning <code>Integer.MIN_VALUE</code>
3-
does <em>not</em> convey a higher degree of inequality, and doing so can cause errors because the return value of <code>compareTo</code> is sometimes
4-
inversed, with the expectation that negative values become positive. However, inversing <code>Integer.MIN_VALUE</code> yields
5-
<code>Integer.MIN_VALUE</code> rather than <code>Integer.MAX_VALUE</code>.</p>
6-
<h3>Noncompliant code example</h3>
7-
<pre>
8-
public int compareTo(MyClass) {
2+
<p>The <code>Comparable.compareTo</code> method returns a negative integer, zero, or a positive integer to indicate whether the object is less than,
3+
equal to, or greater than the parameter. The sign of the return value or whether it is zero is what matters, not its magnitude.</p>
4+
<p>Returning a positive or negative constant value other than the basic ones (-1, 0, or 1) provides no additional information to the caller. Moreover,
5+
it could potentially confuse code readers who are trying to understand its purpose.</p>
6+
<h2>How to fix it</h2>
7+
<p>Replace any positive constant return value with 1. Replace any negative constant return value with -1.</p>
8+
<h3>Code examples</h3>
9+
<h4>Noncompliant code example</h4>
10+
<pre data-diff-id="1" data-diff-type="noncompliant">
11+
public int compareTo(Name name) {
912
if (condition) {
10-
return Integer.MIN_VALUE; // Noncompliant
13+
return Integer.MIN_VALUE; // Noncompliant
1114
}
15+
}
1216
</pre>
13-
<h3>Compliant solution</h3>
14-
<pre>
15-
public int compareTo(MyClass) {
17+
<h4>Compliant solution</h4>
18+
<pre data-diff-id="1" data-diff-type="compliant">
19+
public int compareTo(Name name) {
1620
if (condition) {
17-
return -1;
21+
return -1; // Compliant
1822
}
23+
}
1924
</pre>
25+
<h4>Noncompliant code example</h4>
26+
<pre data-diff-id="2" data-diff-type="noncompliant">
27+
public int compareTo(Name name) {
28+
if (condition) {
29+
return 42; // Noncompliant
30+
}
31+
}
32+
</pre>
33+
<h4>Compliant solution</h4>
34+
<pre data-diff-id="2" data-diff-type="compliant">
35+
public int compareTo(Name name) {
36+
if (condition) {
37+
return 1; // Compliant
38+
}
39+
}
40+
</pre>
41+
<h4>Noncompliant code example</h4>
42+
<p>It is compliant to return other values than -1, 0 or 1 if they are not constants.</p>
43+
<pre data-diff-id="3" data-diff-type="noncompliant">
44+
public int compareTo(Name name) {
45+
if (condition) {
46+
return 42; // Noncompliant
47+
}
48+
}
49+
</pre>
50+
<h4>Compliant solution</h4>
51+
<pre data-diff-id="3" data-diff-type="compliant">
52+
public int compareTo(Name name) {
53+
if (condition) {
54+
return hashCode() - name.hashCode(); // Compliant, not a constant
55+
}
56+
}
57+
</pre>
58+
<h2>Resources</h2>
59+
<h3>Documentation</h3>
60+
<ul>
61+
<li> <a href="https://docs.oracle.com/javase/8/docs/api/java/lang/Comparable.html#compareTo-T-">Java SE 8 API Specification:
62+
Comparable.compareTo</a> </li>
63+
<li> <a href="https://docs.oracle.com/javase/8/docs/api/java/lang/Integer.html#MIN_VALUE">Java SE 8 API Specification: Integer.MIN_VALUE</a> </li>
64+
</ul>
2065

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S2229.html‎

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
<h2>Why is this an issue?</h2>
2-
<p>When using Spring proxies, calling a method in the same class (e.g. <code>this.aMethod()</code>) with an incompatible <code>@Transactional</code>
3-
requirement will result in runtime exceptions because Spring only "sees" the caller and makes no provisions for properly invoking the callee.</p>
4-
<p>Therefore, certain calls should never be made within the same class:</p>
2+
<p>Transactional methods have a propagation type parameter in the @Transaction annotation that specifies the requirements about the transactional
3+
context in which the method can be called and how it creates, appends, or suspends an ongoing transaction.</p>
4+
<p>When an instance that contains transactional methods is injected, Spring uses proxy objects to wrap these methods with the actual transaction
5+
code.</p>
6+
<p>However, if a transactional method is called from another method in the same class, the <code>this</code> argument is used as the receiver instance
7+
instead of the injected proxy object, which bypasses the wrapper code. This results in specific transitions from one transactional method to another,
8+
which are not allowed:</p>
59
<table>
610
<colgroup>
711
<col style="width: 50%;">
@@ -48,18 +52,73 @@ <h2>Why is this an issue?</h2>
4852
</tr>
4953
</tbody>
5054
</table>
51-
<h3>Noncompliant code example</h3>
52-
<pre>
53-
@Override
55+
<h2>How to fix it</h2>
56+
<p>Change the corresponding functions into a compatible propagation type.</p>
57+
<h3>Code examples</h3>
58+
<h4>Noncompliant code example</h4>
59+
<pre data-diff-id="1" data-diff-type="noncompliant">
5460
public void doTheThing() {
5561
// ...
56-
actuallyDoTheThing(); // Noncompliant
62+
actuallyDoTheThing(); // Noncompliant, call from non-transactional to transactional
5763
}
5864

59-
@Override
6065
@Transactional
6166
public void actuallyDoTheThing() {
6267
// ...
6368
}
6469
</pre>
70+
<h4>Compliant solution</h4>
71+
<pre data-diff-id="1" data-diff-type="compliant">
72+
@Transactional
73+
public void doTheThing() {
74+
// ...
75+
actuallyDoTheThing(); // Compliant
76+
}
77+
78+
@Transactional
79+
public void actuallyDoTheThing() {
80+
// ...
81+
}
82+
</pre>
83+
<h4>Noncompliant code example</h4>
84+
<pre data-diff-id="2" data-diff-type="noncompliant">
85+
@Transactional
86+
public void doTheThing() {
87+
// ...
88+
actuallyDoTheThing(); // Noncompliant, call from REQUIRED to REQUIRES_NEW
89+
}
90+
91+
@Transactional(propagation = Propagation.REQUIRES_NEW)
92+
public void actuallyDoTheThing() {
93+
// ...
94+
}
95+
</pre>
96+
<h4>Compliant solution</h4>
97+
<pre data-diff-id="2" data-diff-type="compliant">
98+
@Transactional
99+
public void doTheThing() {
100+
// ...
101+
actuallyDoTheThing(); // Compliant, call from REQUIRED to MANDATORY
102+
}
103+
104+
@Transactional(propagation = Propagation.MANDATORY)
105+
public void actuallyDoTheThing() {
106+
// ...
107+
}
108+
</pre>
109+
<h2>Resources</h2>
110+
<ul>
111+
<li> <a href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/transaction/annotation/Propagation.html">Spring
112+
Framework 6 API: Enum Class propagation</a> </li>
113+
<li> <a href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/transaction/annotation/Transactional.html">Spring
114+
Framework 6 API: Annotation Interface Transactional</a> </li>
115+
<li> <a href="https://docs.spring.io/spring-framework/reference/data-access/transaction/declarative/tx-propagation.html">Spring 6 Documentation:
116+
Transaction Propagation</a> </li>
117+
</ul>
118+
<h3>Articles &amp; blog posts</h3>
119+
<ul>
120+
<li> <a href="https://www.baeldung.com/spring-transactional-propagation-isolation">Baeldung: Transaction Propagation and Isolation in Spring
121+
@Transactional</a> </li>
122+
<li> <a href="https://dzone.com/articles/spring-transaction-propagation">DZone: Spring Transaction Propagation in a Nutshell</a> </li>
123+
</ul>
65124

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S2232.html‎

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,41 @@
11
<h2>Why is this an issue?</h2>
2-
<p>There are several reasons to avoid <code>ResultSet.isLast()</code>. First, support for this method is optional for <code>TYPE_FORWARD_ONLY</code>
3-
result sets. Second, it can be expensive (the driver may need to fetch the next row to answer the question). Finally, the specification is not clear
4-
on what should be returned when the <code>ResultSet</code> is empty, so some drivers may return the opposite of what is expected.</p>
5-
<h3>Noncompliant code example</h3>
6-
<pre>
7-
stmt.executeQuery("SELECT name, address FROM PERSON");
8-
ResultSet rs = stmt.getResultSet();
9-
while (! rs.isLast()) { // Noncompliant
10-
// process row
2+
<p>There are several reasons to avoid using this method:</p>
3+
<ol>
4+
<li> It is optionally available only for result sets of type <code>ResultSet.TYPE_FORWARD_ONLY</code>. Database drivers will throw an exception if
5+
not supported. </li>
6+
<li> The method can be expensive to execute as the database driver may need to fetch ahead one row to determine whether the current row is the last
7+
in the result set. The documentation of the method explicitly mentions this fact. </li>
8+
<li> What "the cursor is on the last row" means for an empty <code>ResultSet</code> is unclear. Database drivers may return <code>true</code> or
9+
<code>false</code> in this case . </li>
10+
</ol>
11+
<p><code>ResultSet.next()</code> is a good alternative to <code>ResultSet.isLast()</code> as it does not have the mentioned issues. It is always
12+
supported and, as per specification, returns <code>false</code> for empty result sets.</p>
13+
<h2>How to fix it</h2>
14+
<p>Refactor your code to use <code>ResultSet.next()</code> instead of <code>ResultSet.isLast()</code>. Be cautious of its different semantics and side
15+
effects on cursor positioning in the result set. Verify that your program logic is still valid under these side effects and otherwise adjust it.</p>
16+
<h3>Code examples</h3>
17+
<h4>Noncompliant code example</h4>
18+
<pre data-diff-id="1" data-diff-type="noncompliant">
19+
ResultSet results = stmt.executeQuery("SELECT name, address FROM PERSON");
20+
StringBuilder sb = new StringBuilder();
21+
while (results.next() &amp;&amp; !results.isLast()) { // Noncompliant
22+
sb.append(results.getString("name") + ", ");
1123
}
24+
sb.append(results.getString("name"));
25+
String formattedNames = sb.toString();
1226
</pre>
13-
<h3>Compliant solution</h3>
14-
<pre>
15-
ResultSet rs = stmt.executeQuery("SELECT name, address FROM PERSON");
16-
while (rs.next()) {
17-
// process row
27+
<h4>Compliant solution</h4>
28+
<pre data-diff-id="1" data-diff-type="compliant">
29+
ResultSet results = stmt.executeQuery("SELECT name, address FROM PERSON");
30+
List&lt;String&gt; names = new ArrayList&lt;&gt;();
31+
while (results.next()) { // Compliant, and program logic refactored
32+
names.add(results.getString("name"));
1833
}
34+
String formattedNames = names.stream().collect(Collectors.joining(", "));
1935
</pre>
36+
<h2>Resources</h2>
37+
<h3>Documentation</h3>
38+
<ul>
39+
<li> <a href="https://docs.oracle.com/javase/8/docs/api/java/sql/ResultSet.html#isLast--">Java SE 8 API Specification: ResultSet.isLast()</a> </li>
40+
</ul>
2041

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S2254.html‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,20 @@ <h2>Why is this an issue?</h2>
66
</blockquote>
77
<p>The session ID it returns is either transmitted in a cookie or a URL parameter so by definition, nothing prevents the end-user from manually
88
updating the value of this session ID in the HTTP request.</p>
9-
<p>Here is an example of a updated HTTP header:</p>
9+
<p>Here is an example of an updated HTTP header:</p>
1010
<pre>
1111
GET /pageSomeWhere HTTP/1.1
1212
Host: webSite.com
1313
User-Agent: Mozilla/5.0
1414
Cookie: JSESSIONID=Hacked_Session_Value'''"&gt;
1515
</pre>
16-
<p>Due to the ability of the end-user to manually change the value, the session ID in the request should only be used by a servlet container (E.G.
17-
Tomcat or Jetty) to see if the value matches the ID of an an existing session. If it does not, the user should be considered unauthenticated.
18-
Moreover, this session ID should never be logged as is but using a one-way hash to prevent hijacking of active sessions.</p>
16+
<p>Due to the ability of the end-user to manually change the value, the session ID in the request should only be used by a servlet container (e.g.
17+
Tomcat or Jetty) to see if the value matches the ID of an existing session. If it does not, the user should be considered unauthenticated. Moreover,
18+
this session ID should never be logged as is but logged using a one-way hash to prevent hijacking of active sessions.</p>
1919
<h3>Noncompliant code example</h3>
2020
<pre>
21-
if(isActiveSession(request.getRequestedSessionId()) ){
22-
...
21+
if (isActiveSession(request.getRequestedSessionId())) {
22+
// ...
2323
}
2424
</pre>
2525
<h2>Resources</h2>

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S2272.html‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,30 @@ <h2>Why is this an issue?</h2>
44
<code>Iterator</code>.</p>
55
<h3>Noncompliant code example</h3>
66
<pre>
7-
public class MyIterator implements Iterator&lt;String&gt;{
7+
public class MyIterator implements Iterator&lt;String&gt; {
88
...
9-
public String next(){
10-
if(!hasNext()){
9+
public String next() {
10+
if (!hasNext()) {
1111
return null;
1212
}
13-
...
13+
// ...
1414
}
1515
}
1616
</pre>
1717
<h3>Compliant solution</h3>
1818
<pre>
19-
public class MyIterator implements Iterator&lt;String&gt;{
19+
public class MyIterator implements Iterator&lt;String&gt; {
2020
...
21-
public String next(){
22-
if(!hasNext()){
21+
public String next() {
22+
if (!hasNext()) {
2323
throw new NoSuchElementException();
2424
}
25-
...
25+
// ...
2626
}
2727
}
2828
</pre>
29+
<h2>Resources</h2>
30+
<ul>
31+
<li> <a href="https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html#next--">JavaDoc 8 - java.util.Iterator</a> </li>
32+
</ul>
2933

‎sonarpedia.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"languages": [
44
"JAVA"
55
],
6-
"latest-update": "2023-07-21T13:20:27.121561Z",
6+
"latest-update": "2023-07-24T15:48:48.266201Z",
77
"options": {
88
"no-language-in-filenames": true,
99
"preserve-filenames": false

0 commit comments

Comments
 (0)