From e0d89660284a8540f0cd6a91c155546d93aa25b0 Mon Sep 17 00:00:00 2001
From: "erwan.serandour" <erwan.serandour@sonarsource.com>
Date: Tue, 27 May 2025 12:07:46 +0200
Subject: [PATCH 1/4] SONARJAVA-5581 modify S3033: add performance benchmark
 table

---
 rules/S3033/java/rule.adoc | 75 +++++++++++++++++++++++++++++++++++---
 1 file changed, 70 insertions(+), 5 deletions(-)

diff --git a/rules/S3033/java/rule.adoc b/rules/S3033/java/rule.adoc
index 0fdaf09320b..e87f73ee4c6 100644
--- a/rules/S3033/java/rule.adoc
+++ b/rules/S3033/java/rule.adoc
@@ -1,31 +1,96 @@
 == Why is this an issue?
 
-Assembling a ``++StringBuilder++`` or ``++StringBuffer++`` into a ``++String++`` merely to see if it's empty is a waste of cycles. Instead, jump right to the heart of the matter and get its ``++.length()++`` instead.
+Assembling a ``++StringBuilder++`` or ``++StringBuffer++`` into a ``++String++`` merely to see if it's empty is a waste of cycles. Instead, jump right to the heart of the matter and use ``++.isEmpty++`` instead.
 
 
 === Noncompliant code example
 
-[source,java]
+[source,java,diff-id=1,diff-type=nonCompliant]
 ----
 StringBuilder sb = new StringBuilder();
 // ...
 if ("".equals(sb.toString()) { // Noncompliant
   // ...
 }
+if (sb.toString().isEmpty()) { // Noncompliant
+  // ...
+}
 ----
 
-
 === Compliant solution
 
-[source,java]
+[source,java,diff-id=1,diff-type=compliant]
 ----
 StringBuilder sb = new StringBuilder();
 // ...
-if (sb.length() == 0) {
+if (sb.isEmpty()) {
+  // ...
+}
+if (sb.isEmpty()) {
   // ...
 }
 ----
 
+=== Benchmarks
+
+[options="header"]
+|===
+| Method| Runtime| Average time| Error margin
+| isEmpty| Temurin 21| 0.74 ns/op| ±0.29 ns/op
+| length| Temurin 21| 0.71 ns/op| ±0.10 ns/op
+| toStringIsEmpty| Temurin 21| 3.94 ns/op| ±0.59 ns/op
+| toStringEquals| Temurin 21| 4.80 ns/op| ±0.78 ns/op
+|===
+
+
+The results were generated by running the following snippet with https://github.com/openjdk/jmh[jmh]:
+
+[source,java]
+----
+private String[] str = {"a", "b", "c", "d", "e"};
+private StringBuilder sb = new StringBuilder();
+
+@Setup(Level.Iteration)
+public void setup() {
+  StringBuilder sb = new StringBuilder();
+  for (String s : str) {
+    sb.append(s);
+  }
+}
+
+@Benchmark
+public StringBuilder toStringEquals() {
+  if ("".equals((sb.toString()))) {
+    return sb;
+  }
+  return new StringBuilder();
+}
+
+@Benchmark
+public StringBuilder toStringIsEmpty() {
+  if (sb.toString().isEmpty()) {
+    return sb;
+  }
+  return new StringBuilder();
+}
+
+@Benchmark
+public StringBuilder length() {
+  if (sb.length() == 0) {
+    return sb;
+  }
+  return new StringBuilder();
+}
+
+@Benchmark
+public StringBuilder isEmpty() {
+  if (sb.isEmpty()) {
+    return sb;
+  }
+  return new StringBuilder();
+}
+----
+
 
 ifdef::env-github,rspecator-view[]
 

From 5c245ec36d2818be42b061744bb27e3494e3ad92 Mon Sep 17 00:00:00 2001
From: "erwan.serandour" <erwan.serandour@sonarsource.com>
Date: Tue, 27 May 2025 14:44:48 +0200
Subject: [PATCH 2/4] update benchmark results, because benchmarking code is
 wrong

---
 rules/S3033/java/rule.adoc | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/rules/S3033/java/rule.adoc b/rules/S3033/java/rule.adoc
index e87f73ee4c6..26f208a2066 100644
--- a/rules/S3033/java/rule.adoc
+++ b/rules/S3033/java/rule.adoc
@@ -36,10 +36,10 @@ if (sb.isEmpty()) {
 [options="header"]
 |===
 | Method| Runtime| Average time| Error margin
-| isEmpty| Temurin 21| 0.74 ns/op| ±0.29 ns/op
-| length| Temurin 21| 0.71 ns/op| ±0.10 ns/op
-| toStringIsEmpty| Temurin 21| 3.94 ns/op| ±0.59 ns/op
-| toStringEquals| Temurin 21| 4.80 ns/op| ±0.78 ns/op
+| isEmpty| Temurin 21| 6.60 ns/op| ±0.58 ns/op
+| length| Temurin 21| 6.89 ns/op| ±0.22 ns/op
+| toStringEquals| Temurin 21| 10.90 ns/op| ±0.62 ns/op
+| toStringIsEmpty| Temurin 21| 11.07 ns/op| ±0.94 ns/op
 |===
 
 
@@ -52,7 +52,7 @@ private StringBuilder sb = new StringBuilder();
 
 @Setup(Level.Iteration)
 public void setup() {
-  StringBuilder sb = new StringBuilder();
+  sb = new StringBuilder();
   for (String s : str) {
     sb.append(s);
   }

From c4b82fa553784c68bb2312333bd83c3435a1c552 Mon Sep 17 00:00:00 2001
From: "erwan.serandour" <erwan.serandour@sonarsource.com>
Date: Tue, 27 May 2025 17:14:42 +0200
Subject: [PATCH 3/4] update after code review

---
 rules/S3033/java/rule.adoc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rules/S3033/java/rule.adoc b/rules/S3033/java/rule.adoc
index 26f208a2066..88eb545270f 100644
--- a/rules/S3033/java/rule.adoc
+++ b/rules/S3033/java/rule.adoc
@@ -1,6 +1,6 @@
 == Why is this an issue?
 
-Assembling a ``++StringBuilder++`` or ``++StringBuffer++`` into a ``++String++`` merely to see if it's empty is a waste of cycles. Instead, jump right to the heart of the matter and use ``++.isEmpty++`` instead.
+It is inefficient to build a ``++String++`` from a ``++StringBuilder++`` or ``++StringBuffer++`` just to check if it's empty. Instead, directly use the ``++.isEmpty++`` method.
 
 
 === Noncompliant code example
@@ -43,7 +43,7 @@ if (sb.isEmpty()) {
 |===
 
 
-The results were generated by running the following snippet with https://github.com/openjdk/jmh[jmh]:
+The results were generated by running the following snippet with https://github.com/openjdk/jmh[JMH]:
 
 [source,java]
 ----

From f6352e7a9fa4aaa9e93c81d2580d8e130c51a367 Mon Sep 17 00:00:00 2001
From: "erwan.serandour" <erwan.serandour@sonarsource.com>
Date: Wed, 28 May 2025 09:20:05 +0200
Subject: [PATCH 4/4] fix typo: nonCompliant -> noncompliant

---
 rules/S3033/java/rule.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rules/S3033/java/rule.adoc b/rules/S3033/java/rule.adoc
index 88eb545270f..c0e553e73b1 100644
--- a/rules/S3033/java/rule.adoc
+++ b/rules/S3033/java/rule.adoc
@@ -5,7 +5,7 @@ It is inefficient to build a ``++String++`` from a ``++StringBuilder++`` or ``++
 
 === Noncompliant code example
 
-[source,java,diff-id=1,diff-type=nonCompliant]
+[source,java,diff-id=1,diff-type=noncompliant]
 ----
 StringBuilder sb = new StringBuilder();
 // ...