Skip to content

Commit 421c07e

Browse files
authored
Enrich recipe descriptions with rationale (#850)
* Enrich recipe descriptions with "why" rationale for RSPEC-tagged rules The RSPEC tags on static analysis recipes used to link to SonarSource's public rule documentation, but those URLs have been decommissioned. Rather than adding broken links, this enriches each recipe's description field with 1-2 sentences explaining why the issue matters — making the recipes self-documenting. Descriptions for 98 recipes were updated; 9 were skipped because they already had thorough rationale. * Reformat description strings to wrap at ~120 characters per line
1 parent c02f72d commit 421c07e

98 files changed

Lines changed: 413 additions & 131 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/main/java/org/openrewrite/staticanalysis/AbstractClassPublicConstructor.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ public class AbstractClassPublicConstructor extends Recipe {
3434

3535
@Getter
3636
final String description = "Constructors of `abstract` classes can only be called in constructors of their subclasses. " +
37-
"Therefore the visibility of `public` constructors are reduced to `protected`.";
37+
"Therefore the visibility of `public` constructors are reduced to `protected`. " +
38+
"Declaring them `public` is misleading since it implies they could be invoked directly, which is never possible.";
3839

3940
@Getter
4041
final Set<String> tags = singleton("RSPEC-S5993");

src/main/java/org/openrewrite/staticanalysis/AddSerialVersionUidToSerializable.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ public class AddSerialVersionUidToSerializable extends Recipe {
5151

5252
String description = "A `serialVersionUID` field is strongly recommended in all `Serializable` classes. If this is not " +
5353
"defined on a `Serializable` class, the compiler will generate this value. If a change is later made " +
54-
"to the class, the generated value will change and attempts to deserialize the class will fail.";
54+
"to the class, the generated value will change and attempts to deserialize the class will fail. " +
55+
"Explicitly declaring this field gives you control over binary compatibility across versions.";
5556

5657
Set<String> tags = singleton("RSPEC-S2057");
5758

src/main/java/org/openrewrite/staticanalysis/AtomicPrimitiveEqualsUsesGet.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,12 @@ public class AtomicPrimitiveEqualsUsesGet extends Recipe {
4949
final String displayName = "Atomic Boolean, Integer, and Long equality checks compare their values";
5050

5151
@Getter
52-
final String description = "`AtomicBoolean#equals(Object)`, `AtomicInteger#equals(Object)` and `AtomicLong#equals(Object)` are only equal to their instance. This recipe converts `a.equals(b)` to `a.get() == b.get()`.";
52+
final String description = "`AtomicBoolean#equals(Object)`, `AtomicInteger#equals(Object)` and " +
53+
"`AtomicLong#equals(Object)` are only equal to their instance. " +
54+
"This recipe converts `a.equals(b)` to `a.get() == b.get()`. " +
55+
"These atomic classes do not override `equals` from `Object`, so calling it " +
56+
"compares object identity rather than the wrapped value, which is almost never " +
57+
"the intended behavior.";
5358

5459
@Getter
5560
final Set<String> tags = singleton("RSPEC-S2204");

src/main/java/org/openrewrite/staticanalysis/AvoidBoxedBooleanExpressions.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,10 @@ public class AvoidBoxedBooleanExpressions extends Recipe {
3333
final String displayName = "Avoid boxed boolean expressions";
3434

3535
@Getter
36-
final String description = "Under certain conditions the `java.lang.Boolean` type is used as an expression, " +
37-
"and it may throw a `NullPointerException` if the value is null.";
36+
final String description = "Under certain conditions the `java.lang.Boolean` type is used as an " +
37+
"expression, and it may throw a `NullPointerException` if the value is null. " +
38+
"Using `Boolean.TRUE.equals(...)` guards against unboxing a `null` reference " +
39+
"in control flow positions like `if` conditions and ternary operators.";
3840

3941
@Getter
4042
final Set<String> tags = singleton("RSPEC-S5411");

src/main/java/org/openrewrite/staticanalysis/BigDecimalDoubleConstructor.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,15 @@
2323

2424
@RecipeDescriptor(
2525
name = "`new BigDecimal(double)` should not be used",
26-
description = "Use of `new BigDecimal(double)` constructor can lead to loss of precision. Use `BigDecimal.valueOf(double)` instead.\n" +
27-
"For example writing `new BigDecimal(0.1)` does not create a `BigDecimal` which is exactly equal to `0.1`, " +
28-
"but it is equal to `0.1000000000000000055511151231257827021181583404541015625`. " +
29-
"This is because `0.1` cannot be represented exactly as a double (or, for that matter, as a binary fraction of any finite length).",
26+
description = "Use of `new BigDecimal(double)` constructor can lead to loss of precision. " +
27+
"Use `BigDecimal.valueOf(double)` instead.\n" +
28+
"For example writing `new BigDecimal(0.1)` does not create a `BigDecimal` " +
29+
"which is exactly equal to `0.1`, but it is equal to " +
30+
"`0.1000000000000000055511151231257827021181583404541015625`. " +
31+
"This is because `0.1` cannot be represented exactly as a double " +
32+
"(or, for that matter, as a binary fraction of any finite length). " +
33+
"`BigDecimal.valueOf` avoids this by converting through a string " +
34+
"representation, preserving the value you actually intended.",
3035
tags = {"RSPEC-S2111"}
3136
)
3237
public class BigDecimalDoubleConstructor {

src/main/java/org/openrewrite/staticanalysis/BigDecimalRoundingConstantsToEnums.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ public class BigDecimalRoundingConstantsToEnums extends Recipe {
4242
final String displayName = "`BigDecimal` rounding constants to `RoundingMode` enums";
4343

4444
@Getter
45-
final String description = "Convert `BigDecimal` rounding constants to the equivalent `RoundingMode` enum.";
45+
final String description = "Convert `BigDecimal` rounding constants to the equivalent " +
46+
"`RoundingMode` enum. The integer-based rounding constants on `BigDecimal` " +
47+
"are deprecated and lack type safety; the `RoundingMode` enum makes the " +
48+
"rounding behavior self-documenting and prevents invalid values.";
4649

4750
@Getter
4851
final Set<String> tags = singleton("RSPEC-S2111");

src/main/java/org/openrewrite/staticanalysis/BooleanChecksNotInverted.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ public class BooleanChecksNotInverted extends Recipe {
3333
final String displayName = "Boolean checks should not be inverted";
3434

3535
@Getter
36-
final String description = "Ensures that boolean checks are not unnecessarily inverted. Also fixes double negative boolean expressions.";
36+
final String description = "Ensures that boolean checks are not unnecessarily inverted. " +
37+
"Also fixes double negative boolean expressions. Negating a comparison and " +
38+
"then inverting it adds cognitive overhead; using the direct operator " +
39+
"(e.g., `>=` instead of `!(... < ...)`) is clearer and easier to reason about.";
3740

3841
@Getter
3942
final Set<String> tags = singleton("RSPEC-S1940");

src/main/java/org/openrewrite/staticanalysis/CaseInsensitiveComparisonsDoNotChangeCase.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ public class CaseInsensitiveComparisonsDoNotChangeCase extends Recipe {
4141
final String displayName = "CaseInsensitive comparisons do not alter case";
4242

4343
@Getter
44-
final String description = "Remove `String#toLowerCase()` or `String#toUpperCase()` from `String#equalsIgnoreCase(..)` comparisons.";
44+
final String description = "Remove `String#toLowerCase()` or `String#toUpperCase()` from `String#equalsIgnoreCase(..)` comparisons. " +
45+
"Changing case before a case-insensitive comparison is redundant and allocates unnecessary intermediate `String` objects.";
4546

4647
@Getter
4748
final Set<String> tags = singleton("RSPEC-S1157");

src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ public class CatchClauseOnlyRethrows extends Recipe {
3939

4040
@Getter
4141
final String description = "A `catch` clause that only rethrows the caught exception is unnecessary. " +
42-
"Letting the exception bubble up as normal achieves the same result with less code.";
42+
"Letting the exception bubble up as normal achieves the same result with less code. " +
43+
"Such catch blocks add visual noise and indentation without changing program behavior.";
4344

4445
@Getter
4546
final Set<String> tags = singleton("RSPEC-S2737");

src/main/java/org/openrewrite/staticanalysis/ChainStringBuilderAppendCalls.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,12 @@ public class ChainStringBuilderAppendCalls extends Recipe {
3939
final String displayName = "Chain `StringBuilder.append()` calls";
4040

4141
@Getter
42-
final String description = "String concatenation within calls to `StringBuilder.append()` causes unnecessary memory allocation. Except for concatenations of String literals, which are joined together at compile time. Replaces inefficient concatenations with chained calls to `StringBuilder.append()`.";
42+
final String description = "String concatenation within calls to `StringBuilder.append()` " +
43+
"causes unnecessary memory allocation. Except for concatenations of String " +
44+
"literals, which are joined together at compile time. Replaces inefficient " +
45+
"concatenations with chained calls to `StringBuilder.append()`. " +
46+
"Using `+` inside `append()` defeats the purpose of the `StringBuilder`, " +
47+
"since the concatenation creates a temporary `String` before appending.";
4348

4449
@Getter
4550
final Set<String> tags = singleton("RSPEC-S3024");

0 commit comments

Comments
 (0)