From 4373bf4a0bb7d2409e478f26c0b27aef62b214c6 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Wed, 14 Oct 2026 15:31:26 +0400 Subject: [PATCH 1/5] SONARJAVA-7188 S7158: Detect zero literals written in any base LiteralUtils.isZero compared the literal's decimal token text, so length() == 0 was flagged but the equivalent length() == 0x0, 00 or 0b0 was not. Compare the parsed literal value instead. Co-Authored-By: Claude Opus 5 (1M context) --- .../java/checks/StringIsEmptyCheckSample.java | 30 +++++++++++++++++++ .../org/sonar/java/model/LiteralUtils.java | 13 +++++++- .../sonar/java/model/LiteralUtilsTest.java | 20 +++++++------ 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/StringIsEmptyCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/StringIsEmptyCheckSample.java index a48f8014bbc..69ab25d6abd 100644 --- a/java-checks-test-sources/default/src/main/java/checks/StringIsEmptyCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/StringIsEmptyCheckSample.java @@ -82,6 +82,36 @@ public boolean sample(String s, String t) { // fix@qf15 {{Replace with "isEmpty()"}} // edit@qf15 [[sc=30;ec=43]]{{isEmpty()}} + // zero written in a non-decimal base + b = s.length() == 0x0; // Noncompliant [[quickfixes=qf16]] + // fix@qf16 {{Replace with "isEmpty()"}} + // edit@qf16 [[sc=11;ec=26]]{{isEmpty()}} + + b = s.length() == 0X0; // Noncompliant [[quickfixes=qf17]] + // fix@qf17 {{Replace with "isEmpty()"}} + // edit@qf17 [[sc=11;ec=26]]{{isEmpty()}} + + b = s.length() == 00; // Noncompliant [[quickfixes=qf18]] + // fix@qf18 {{Replace with "isEmpty()"}} + // edit@qf18 [[sc=11;ec=25]]{{isEmpty()}} + + b = s.length() == 0b0; // Noncompliant [[quickfixes=qf19]] + // fix@qf19 {{Replace with "isEmpty()"}} + // edit@qf19 [[sc=11;ec=26]]{{isEmpty()}} + + b = s.length() == 0B0; // Noncompliant [[quickfixes=qf20]] + // fix@qf20 {{Replace with "isEmpty()"}} + // edit@qf20 [[sc=11;ec=26]]{{isEmpty()}} + + b = 0x0 != s.length(); // Noncompliant [[quickfixes=qf21]] + // fix@qf21 {{Replace with "isEmpty()"}} + // edit@qf21 [[sc=9;ec=16]]{{!}} + // edit@qf21 [[sc=18;ec=26]]{{isEmpty()}} + + b = s.length() == 0x1; + b = s.length() == 01; + b = s.length() == 0b1; + b = s.length() == 1; b = s.length() > 3; b = s.length() <= 10; diff --git a/java-frontend/src/main/java/org/sonar/java/model/LiteralUtils.java b/java-frontend/src/main/java/org/sonar/java/model/LiteralUtils.java index 289694d61cd..64a151d059f 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/LiteralUtils.java +++ b/java-frontend/src/main/java/org/sonar/java/model/LiteralUtils.java @@ -152,8 +152,19 @@ public static boolean isFalse(Tree tree) { return tree.is(Kind.BOOLEAN_LITERAL) && "false".equals(((LiteralTree) tree).value()); } + /** + * @return true when the expression is an int literal whose value is zero, in any base: + * decimal ({@code 0}), hexadecimal ({@code 0x0}), octal ({@code 00}) or binary ({@code 0b0}). + */ public static boolean isZero(ExpressionTree tree) { - return tree.is(Kind.INT_LITERAL) && "0".equals(((LiteralTree) tree).value()); + if (!tree.is(Kind.INT_LITERAL)) { + return false; + } + try { + return intLiteralValue((LiteralTree) tree) == 0; + } catch (NumberFormatException e) { + return false; + } } public static boolean isOne(ExpressionTree tree) { diff --git a/java-frontend/src/test/java/org/sonar/java/model/LiteralUtilsTest.java b/java-frontend/src/test/java/org/sonar/java/model/LiteralUtilsTest.java index 3b9776a052b..c9195a8e8db 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/LiteralUtilsTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/LiteralUtilsTest.java @@ -22,6 +22,8 @@ import java.util.Optional; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.sonar.plugins.java.api.tree.BinaryExpressionTree; import org.sonar.plugins.java.api.tree.ClassTree; import org.sonar.plugins.java.api.tree.CompilationUnitTree; @@ -288,9 +290,10 @@ void isFalse_withExpectedValue_returnsTrue() { assertThat(LiteralUtils.isFalse(falseTree)).isTrue(); } - @Test - void isZero_withExpectedValue() { - ExpressionTree tree = getReturnExpression("int foo(){ return 0; }"); + @ParameterizedTest + @ValueSource(strings = {"0", "00", "0_0", "0x0", "0X0", "0x00", "0b0", "0B0", "0b0_0"}) + void isZero_withExpectedValue(String literal) { + ExpressionTree tree = getReturnExpression("int foo(){ return " + literal + "; }"); assertThat(LiteralUtils.isZero(tree)).isTrue(); } @@ -306,12 +309,11 @@ void isNegOne_withExpectedValue() { assertThat(LiteralUtils.isNegOne(tree)).isTrue(); } - @Test - void isZero_withUnexpectedValues() { - ExpressionTree intTree = getReturnExpression("int foo(){ return 5; }"); - assertThat(LiteralUtils.isZero(intTree)).isFalse(); - ExpressionTree boolTree = getReturnExpression("int foo(){ return true; }"); - assertThat(LiteralUtils.isZero(boolTree)).isFalse(); + @ParameterizedTest + @ValueSource(strings = {"5", "0x1", "01", "0b1", "true", "0xFFFF_FFFF_FFFF_FFFF_F", "0b1111_1111_1111_1111_1111_1111_1111_1111_1"}) + void isZero_withUnexpectedValues(String literal) { + ExpressionTree tree = getReturnExpression("int foo(){ return " + literal + "; }"); + assertThat(LiteralUtils.isZero(tree)).isFalse(); } @Test From b049767d1d3063a5b440b9569d52452bda385581 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Thu, 15 Oct 2026 10:42:49 +0400 Subject: [PATCH 2/5] SONARJAVA-4606 S7158: Suggest "isBlank()" for "trim().length()" comparisons Comparing "s.trim().length()" to zero is a blankness check, so report it with an "isBlank()" message and quick fix instead of "isEmpty()". Gated on Java 11, where "String.isBlank()" was introduced. Co-Authored-By: Claude Opus 5 (1M context) --- .../java/checks/StringIsBlankCheckSample.java | 87 +++++++++++++++++++ .../sonar/java/checks/StringIsEmptyCheck.java | 54 +++++++++--- .../java/checks/StringIsEmptyCheckTest.java | 19 ++++ 3 files changed, 150 insertions(+), 10 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java diff --git a/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java new file mode 100644 index 00000000000..573611abcdb --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java @@ -0,0 +1,87 @@ +package checks; + +public class StringIsBlankCheckSample { + public boolean sample(String s, StringBuilder sb) { + boolean b; + + // test `trim().length() == 0` and equivalent code + + b = s.trim().length() == 0; // Noncompliant {{Use "isBlank()" to check whether a "String" is blank or not.}} [[quickfixes=qf1]] + // fix@qf1 {{Replace with "isBlank()"}} + // edit@qf1 [[sc=11;ec=31]]{{isBlank()}} + + b = s.trim().length() <= 0; // Noncompliant [[quickfixes=qf2]] + // fix@qf2 {{Replace with "isBlank()"}} + // edit@qf2 [[sc=11;ec=31]]{{isBlank()}} + + b = s.trim().length() < 1; // Noncompliant [[quickfixes=qf3]] + // fix@qf3 {{Replace with "isBlank()"}} + // edit@qf3 [[sc=11;ec=30]]{{isBlank()}} + + // test `trim().length() != 0` and equivalent code + + b = s.trim().length() != 0; // Noncompliant [[quickfixes=qf4]] + // fix@qf4 {{Replace with "isBlank()"}} + // edit@qf4 [[sc=9;ec=9]]{{!}} + // edit@qf4 [[sc=11;ec=31]]{{isBlank()}} + + b = s.trim().length() > 0; // Noncompliant [[quickfixes=qf5]] + // fix@qf5 {{Replace with "isBlank()"}} + // edit@qf5 [[sc=9;ec=9]]{{!}} + // edit@qf5 [[sc=11;ec=30]]{{isBlank()}} + + b = s.trim().length() >= 1; // Noncompliant [[quickfixes=qf6]] + // fix@qf6 {{Replace with "isBlank()"}} + // edit@qf6 [[sc=9;ec=9]]{{!}} + // edit@qf6 [[sc=11;ec=31]]{{isBlank()}} + + // reversed order + + b = 0 == s.trim().length(); // Noncompliant [[quickfixes=qf7]] + // fix@qf7 {{Replace with "isBlank()"}} + // edit@qf7 [[sc=9;ec=14]]{{}} + // edit@qf7 [[sc=16;ec=31]]{{isBlank()}} + + b = 0 != s.trim().length(); // Noncompliant [[quickfixes=qf8]] + // fix@qf8 {{Replace with "isBlank()"}} + // edit@qf8 [[sc=9;ec=14]]{{!}} + // edit@qf8 [[sc=16;ec=31]]{{isBlank()}} + + // extra parentheses + + b = (s.trim().length()) == 0; // Noncompliant [[quickfixes=qf9]] + // fix@qf9 {{Replace with "isBlank()"}} + // edit@qf9 [[sc=9;ec=10]]{{}} + // edit@qf9 [[sc=12;ec=33]]{{isBlank()}} + + // the `trim()` call is not directly chained, so only emptiness can be simplified + b = (s.trim()).length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf10]] + // fix@qf10 {{Replace with "isEmpty()"}} + // edit@qf10 [[sc=20;ec=33]]{{isEmpty()}} + + // chained method calls + + b = s.toUpperCase().trim().length() == 0; // Noncompliant [[quickfixes=qf11]] + // fix@qf11 {{Replace with "isBlank()"}} + // edit@qf11 [[sc=25;ec=45]]{{isBlank()}} + + b = sb.toString().trim().length() == 0; // Noncompliant [[quickfixes=qf12]] + // fix@qf12 {{Replace with "isBlank()"}} + // edit@qf12 [[sc=23;ec=43]]{{isBlank()}} + + // only `trim()` is handled + b = s.strip().length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf13]] + // fix@qf13 {{Replace with "isEmpty()"}} + // edit@qf13 [[sc=19;ec=32]]{{isEmpty()}} + + b = s.trim().length() == 1; + b = s.trim().length() > 3; + b = s.trim().length() >= 8; + b = s.trim().length() == sb.length(); + + b = s.isBlank(); + b = !s.isBlank(); + + return b; + } +} \ No newline at end of file diff --git a/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java index 55cecf9441c..2950228efbd 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java @@ -39,17 +39,29 @@ /** * Implement rule that
CharSequence.length() == 0
should be replaced with - *
CharSequence.isEmpty()
. + *
CharSequence.isEmpty()
, and
String.trim().length() == 0
with + *
String.isBlank()
. */ @Rule(key = "S7158") public class StringIsEmptyCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor { + private static final String JAVA_LANG_STRING = "java.lang.String"; + + // `String.isBlank()` is available since Java 11 + private static final int JAVA_11 = 11; + private static final MethodMatchers STRING_LENGTH_METHOD = MethodMatchers.create() - .ofTypes("java.lang.String") + .ofTypes(JAVA_LANG_STRING) .names("length") .addWithoutParametersMatcher() .build(); + private static final MethodMatchers STRING_TRIM_METHOD = MethodMatchers.create() + .ofTypes(JAVA_LANG_STRING) + .names("trim") + .addWithoutParametersMatcher() + .build(); + private static final MethodMatchers CHARSEQUENCE_LENGTH_METHOD = MethodMatchers.create() .ofSubTypes("java.lang.CharSequence") .names("length") @@ -117,21 +129,28 @@ public void visitNode(Tree tree) { } if (lengthCall != null && isEmptinessCheck(comparisonType)) { + // `s.trim().length() == 0` is a blankness check, which `isBlank()` expresses directly. + MethodInvocationTree trimCall = getTrimCall(lengthCall); + String replacedMethod = trimCall != null ? "isBlank()" : "isEmpty()"; + String adjective = trimCall != null ? "blank" : "empty"; + MethodInvocationTree replacementStart = trimCall != null ? trimCall : lengthCall; + QuickFixHelper .newIssue(context) .forRule(this) .onTree(tree) - .withMessage("Use \"isEmpty()\" to check whether a \"" + ownerName(lengthCall) + "\" is empty or not.") - .withQuickFix(() -> getQuickFix(tree, lengthCall, comparisonType)) + .withMessage("Use \"" + replacedMethod + "\" to check whether a \"" + ownerName(lengthCall) + "\" is " + adjective + " or not.") + .withQuickFix(() -> getQuickFix(tree, lengthCall, replacementStart, replacedMethod, comparisonType)) .report(); } } - private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocationTree lengthInvocation, ComparisonType comparisonType) { + private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocationTree lengthInvocation, + MethodInvocationTree replacedInvocation, String replacedMethod, ComparisonType comparisonType) { // There are two cases to deal with: // s.length() OP CONST // CONST OP s.length() - JavaQuickFix.Builder builder = JavaQuickFix.newQuickFix("Replace with \"isEmpty()\""); + JavaQuickFix.Builder builder = JavaQuickFix.newQuickFix("Replace with \"" + replacedMethod + "\""); // Replace "[CONST OP]" with "!"/"". AnalyzerMessage.TextSpan prefixSpan = AnalyzerMessage.textSpanBetween(comparisonExpression, true, lengthInvocation, false); @@ -140,14 +159,14 @@ private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocat builder.addTextEdit(JavaTextEdit.replaceTextSpan(prefixSpan, prefixReplacement)); } - // Replace "length() [OP CONST]" with "isEmpty()". - IdentifierTree lengthIdentifier = ExpressionUtils.methodName(lengthInvocation); + // Replace "length() [OP CONST]" with "isEmpty()", or "trim().length() [OP CONST]" with "isBlank()". + IdentifierTree replacedIdentifier = ExpressionUtils.methodName(replacedInvocation); builder.addTextEdit(JavaTextEdit.replaceBetweenTree( - lengthIdentifier, + replacedIdentifier, true, comparisonExpression, true, - "isEmpty()" + replacedMethod )); return builder.build(); @@ -161,6 +180,21 @@ private static String ownerName(MethodInvocationTree lengthCall) { } } + /** + * @return the
trim()
invocation the given
length()
call is applied to, or null when there is none, + * when it is not directly chained, or when
String.isBlank()
is not available. + */ + @Nullable + private MethodInvocationTree getTrimCall(MethodInvocationTree lengthCall) { + if (context.getJavaVersion().asInt() >= JAVA_11 + && lengthCall.methodSelect() instanceof MemberSelectExpressionTree sel + && sel.expression() instanceof MethodInvocationTree mit + && STRING_TRIM_METHOD.matches(mit)) { + return mit; + } + return null; + } + @Nullable private MethodInvocationTree getLengthCall(ExpressionTree tree) { if (tree instanceof MethodInvocationTree mit) { diff --git a/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java index cb178a81bff..a802b5aac8d 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java @@ -41,6 +41,25 @@ void string_15() { .verifyIssues(); } + @Test + void trim_11() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/StringIsBlankCheckSample.java")) + .withCheck(new StringIsEmptyCheck()) + .withJavaVersion(11) + .verifyIssues(); + } + + @Test + void trim_11_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/StringIsBlankCheckSample.java")) + .withCheck(new StringIsEmptyCheck()) + .withJavaVersion(11) + .withoutSemantic() + .verifyIssues(); + } + @Test void charSequence_15() { CheckVerifier.newVerifier() From 4dae062b6ef52d604b4187cb5dd63a84505b2dfa Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Thu, 15 Oct 2026 11:24:40 +0400 Subject: [PATCH 3/5] SONARJAVA-4606 S7158: Also suggest "isBlank()" for "strip().length()" comparisons "strip()" removes exactly the characters "isBlank()" treats as white space, so the rewrite is equivalent. "trim()" removes every character up to U+0020 instead, so it disagrees on control characters below U+0020 and on Unicode white space above it; those cases keep the suggestion but the issue message and the quick-fix label state what the replacement changes. Co-Authored-By: Claude Opus 5 (1M context) --- .../java/checks/StringIsBlankCheckSample.java | 87 +++++++++++-------- .../sonar/java/checks/StringIsEmptyCheck.java | 53 +++++++---- .../java/checks/StringIsEmptyCheckTest.java | 4 +- 3 files changed, 90 insertions(+), 54 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java index 573611abcdb..683746560b1 100644 --- a/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java @@ -4,84 +4,103 @@ public class StringIsBlankCheckSample { public boolean sample(String s, StringBuilder sb) { boolean b; - // test `trim().length() == 0` and equivalent code + // test `strip().length() == 0` and equivalent code - b = s.trim().length() == 0; // Noncompliant {{Use "isBlank()" to check whether a "String" is blank or not.}} [[quickfixes=qf1]] + b = s.strip().length() == 0; // Noncompliant {{Use "isBlank()" to check whether a "String" is blank or not.}} [[quickfixes=qf1]] // fix@qf1 {{Replace with "isBlank()"}} - // edit@qf1 [[sc=11;ec=31]]{{isBlank()}} + // edit@qf1 [[sc=11;ec=32]]{{isBlank()}} - b = s.trim().length() <= 0; // Noncompliant [[quickfixes=qf2]] + b = s.strip().length() <= 0; // Noncompliant [[quickfixes=qf2]] // fix@qf2 {{Replace with "isBlank()"}} - // edit@qf2 [[sc=11;ec=31]]{{isBlank()}} + // edit@qf2 [[sc=11;ec=32]]{{isBlank()}} - b = s.trim().length() < 1; // Noncompliant [[quickfixes=qf3]] + b = s.strip().length() < 1; // Noncompliant [[quickfixes=qf3]] // fix@qf3 {{Replace with "isBlank()"}} - // edit@qf3 [[sc=11;ec=30]]{{isBlank()}} + // edit@qf3 [[sc=11;ec=31]]{{isBlank()}} - // test `trim().length() != 0` and equivalent code + // test `strip().length() != 0` and equivalent code - b = s.trim().length() != 0; // Noncompliant [[quickfixes=qf4]] + b = s.strip().length() != 0; // Noncompliant [[quickfixes=qf4]] // fix@qf4 {{Replace with "isBlank()"}} // edit@qf4 [[sc=9;ec=9]]{{!}} - // edit@qf4 [[sc=11;ec=31]]{{isBlank()}} + // edit@qf4 [[sc=11;ec=32]]{{isBlank()}} - b = s.trim().length() > 0; // Noncompliant [[quickfixes=qf5]] + b = s.strip().length() > 0; // Noncompliant [[quickfixes=qf5]] // fix@qf5 {{Replace with "isBlank()"}} // edit@qf5 [[sc=9;ec=9]]{{!}} - // edit@qf5 [[sc=11;ec=30]]{{isBlank()}} + // edit@qf5 [[sc=11;ec=31]]{{isBlank()}} - b = s.trim().length() >= 1; // Noncompliant [[quickfixes=qf6]] + b = s.strip().length() >= 1; // Noncompliant [[quickfixes=qf6]] // fix@qf6 {{Replace with "isBlank()"}} // edit@qf6 [[sc=9;ec=9]]{{!}} - // edit@qf6 [[sc=11;ec=31]]{{isBlank()}} + // edit@qf6 [[sc=11;ec=32]]{{isBlank()}} // reversed order - b = 0 == s.trim().length(); // Noncompliant [[quickfixes=qf7]] + b = 0 == s.strip().length(); // Noncompliant [[quickfixes=qf7]] // fix@qf7 {{Replace with "isBlank()"}} // edit@qf7 [[sc=9;ec=14]]{{}} - // edit@qf7 [[sc=16;ec=31]]{{isBlank()}} + // edit@qf7 [[sc=16;ec=32]]{{isBlank()}} - b = 0 != s.trim().length(); // Noncompliant [[quickfixes=qf8]] + b = 0 != s.strip().length(); // Noncompliant [[quickfixes=qf8]] // fix@qf8 {{Replace with "isBlank()"}} // edit@qf8 [[sc=9;ec=14]]{{!}} - // edit@qf8 [[sc=16;ec=31]]{{isBlank()}} + // edit@qf8 [[sc=16;ec=32]]{{isBlank()}} // extra parentheses - b = (s.trim().length()) == 0; // Noncompliant [[quickfixes=qf9]] + b = (s.strip().length()) == 0; // Noncompliant [[quickfixes=qf9]] // fix@qf9 {{Replace with "isBlank()"}} // edit@qf9 [[sc=9;ec=10]]{{}} - // edit@qf9 [[sc=12;ec=33]]{{isBlank()}} + // edit@qf9 [[sc=12;ec=34]]{{isBlank()}} - // the `trim()` call is not directly chained, so only emptiness can be simplified - b = (s.trim()).length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf10]] + // the `strip()` call is not directly chained, so only emptiness can be simplified + b = (s.strip()).length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf10]] // fix@qf10 {{Replace with "isEmpty()"}} - // edit@qf10 [[sc=20;ec=33]]{{isEmpty()}} + // edit@qf10 [[sc=21;ec=34]]{{isEmpty()}} // chained method calls - b = s.toUpperCase().trim().length() == 0; // Noncompliant [[quickfixes=qf11]] + b = s.toUpperCase().strip().length() == 0; // Noncompliant [[quickfixes=qf11]] // fix@qf11 {{Replace with "isBlank()"}} - // edit@qf11 [[sc=25;ec=45]]{{isBlank()}} + // edit@qf11 [[sc=25;ec=46]]{{isBlank()}} - b = sb.toString().trim().length() == 0; // Noncompliant [[quickfixes=qf12]] + b = sb.toString().strip().length() == 0; // Noncompliant [[quickfixes=qf12]] // fix@qf12 {{Replace with "isBlank()"}} - // edit@qf12 [[sc=23;ec=43]]{{isBlank()}} + // edit@qf12 [[sc=23;ec=44]]{{isBlank()}} - // only `trim()` is handled - b = s.strip().length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf13]] - // fix@qf13 {{Replace with "isEmpty()"}} - // edit@qf13 [[sc=19;ec=32]]{{isEmpty()}} + // `trim()` does not remove the same characters as `isBlank()` considers white space, + // so the message and the quick fix spell out what changes + + b = s.trim().length() == 0; // Noncompliant {{Use "isBlank()" to check whether a "String" is blank or not. Use this if Unicode whitespace should be treated as blank and control characters as non-blank.}} [[quickfixes=qf13]] + // fix@qf13 {{Replace with "isBlank()" (Unicode whitespace is treated as blank, control characters as non-blank)}} + // edit@qf13 [[sc=11;ec=31]]{{isBlank()}} + + b = s.trim().length() > 0; // Noncompliant [[quickfixes=qf14]] + // fix@qf14 {{Replace with "isBlank()" (Unicode whitespace is treated as blank, control characters as non-blank)}} + // edit@qf14 [[sc=9;ec=9]]{{!}} + // edit@qf14 [[sc=11;ec=30]]{{isBlank()}} + + b = 0 == s.trim().length(); // Noncompliant [[quickfixes=qf15]] + // fix@qf15 {{Replace with "isBlank()" (Unicode whitespace is treated as blank, control characters as non-blank)}} + // edit@qf15 [[sc=9;ec=14]]{{}} + // edit@qf15 [[sc=16;ec=31]]{{isBlank()}} + + b = s.toLowerCase().trim().length() == 0; // Noncompliant [[quickfixes=qf16]] + // fix@qf16 {{Replace with "isBlank()" (Unicode whitespace is treated as blank, control characters as non-blank)}} + // edit@qf16 [[sc=25;ec=45]]{{isBlank()}} b = s.trim().length() == 1; - b = s.trim().length() > 3; b = s.trim().length() >= 8; - b = s.trim().length() == sb.length(); + + b = s.strip().length() == 1; + b = s.strip().length() > 3; + b = s.strip().length() >= 8; + b = s.strip().length() == sb.length(); b = s.isBlank(); b = !s.isBlank(); return b; } -} \ No newline at end of file +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java index 2950228efbd..28512f5cee0 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java @@ -39,8 +39,8 @@ /** * Implement rule that
CharSequence.length() == 0
should be replaced with - *
CharSequence.isEmpty()
, and
String.trim().length() == 0
with - *
String.isBlank()
. + *
CharSequence.isEmpty()
, and
String.strip().length() == 0
or + *
String.trim().length() == 0
with
String.isBlank()
. */ @Rule(key = "S7158") public class StringIsEmptyCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor { @@ -56,12 +56,24 @@ public class StringIsEmptyCheck extends IssuableSubscriptionVisitor implements J .addWithoutParametersMatcher() .build(); - private static final MethodMatchers STRING_TRIM_METHOD = MethodMatchers.create() + private static final String TRIM_NAME = "trim"; + + private static final MethodMatchers STRING_WHITESPACE_REMOVAL_METHODS = MethodMatchers.create() .ofTypes(JAVA_LANG_STRING) - .names("trim") + .names("strip", TRIM_NAME) .addWithoutParametersMatcher() .build(); + /** + *
trim()
removes every character up to and including
U+0020
, while
isBlank()
relies on + *
Character.isWhitespace
. They disagree on control characters below
U+0020
and on Unicode white + * space above it, so replacing one with the other can change behaviour. + */ + private static final String TRIM_CAVEAT = + "Use this if Unicode whitespace should be treated as blank and control characters as non-blank."; + + private static final String TRIM_FIX_CAVEAT = " (Unicode whitespace is treated as blank, control characters as non-blank)"; + private static final MethodMatchers CHARSEQUENCE_LENGTH_METHOD = MethodMatchers.create() .ofSubTypes("java.lang.CharSequence") .names("length") @@ -129,28 +141,33 @@ public void visitNode(Tree tree) { } if (lengthCall != null && isEmptinessCheck(comparisonType)) { - // `s.trim().length() == 0` is a blankness check, which `isBlank()` expresses directly. - MethodInvocationTree trimCall = getTrimCall(lengthCall); - String replacedMethod = trimCall != null ? "isBlank()" : "isEmpty()"; - String adjective = trimCall != null ? "blank" : "empty"; - MethodInvocationTree replacementStart = trimCall != null ? trimCall : lengthCall; + // `s.strip().length() == 0` and `s.trim().length() == 0` are blankness checks, which `isBlank()` expresses directly. + MethodInvocationTree whitespaceRemovalCall = getWhitespaceRemovalCall(lengthCall); + boolean isBlankCheck = whitespaceRemovalCall != null; + boolean usesTrim = isBlankCheck && TRIM_NAME.equals(ExpressionUtils.methodName(whitespaceRemovalCall).name()); + + String replacedMethod = isBlankCheck ? "isBlank()" : "isEmpty()"; + MethodInvocationTree replacementStart = isBlankCheck ? whitespaceRemovalCall : lengthCall; + String message = "Use \"" + replacedMethod + "\" to check whether a \"" + ownerName(lengthCall) + "\" is " + + (isBlankCheck ? "blank" : "empty") + " or not." + (usesTrim ? (" " + TRIM_CAVEAT) : ""); + String fixDescription = "Replace with \"" + replacedMethod + "\"" + (usesTrim ? TRIM_FIX_CAVEAT : ""); QuickFixHelper .newIssue(context) .forRule(this) .onTree(tree) - .withMessage("Use \"" + replacedMethod + "\" to check whether a \"" + ownerName(lengthCall) + "\" is " + adjective + " or not.") - .withQuickFix(() -> getQuickFix(tree, lengthCall, replacementStart, replacedMethod, comparisonType)) + .withMessage(message) + .withQuickFix(() -> getQuickFix(tree, lengthCall, replacementStart, replacedMethod, fixDescription, comparisonType)) .report(); } } private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocationTree lengthInvocation, - MethodInvocationTree replacedInvocation, String replacedMethod, ComparisonType comparisonType) { + MethodInvocationTree replacedInvocation, String replacedMethod, String fixDescription, ComparisonType comparisonType) { // There are two cases to deal with: // s.length() OP CONST // CONST OP s.length() - JavaQuickFix.Builder builder = JavaQuickFix.newQuickFix("Replace with \"" + replacedMethod + "\""); + JavaQuickFix.Builder builder = JavaQuickFix.newQuickFix(fixDescription); // Replace "[CONST OP]" with "!"/"". AnalyzerMessage.TextSpan prefixSpan = AnalyzerMessage.textSpanBetween(comparisonExpression, true, lengthInvocation, false); @@ -159,7 +176,7 @@ private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocat builder.addTextEdit(JavaTextEdit.replaceTextSpan(prefixSpan, prefixReplacement)); } - // Replace "length() [OP CONST]" with "isEmpty()", or "trim().length() [OP CONST]" with "isBlank()". + // Replace "length() [OP CONST]" with "isEmpty()", or "strip()/trim().length() [OP CONST]" with "isBlank()". IdentifierTree replacedIdentifier = ExpressionUtils.methodName(replacedInvocation); builder.addTextEdit(JavaTextEdit.replaceBetweenTree( replacedIdentifier, @@ -181,15 +198,15 @@ private static String ownerName(MethodInvocationTree lengthCall) { } /** - * @return the
trim()
invocation the given
length()
call is applied to, or null when there is none, - * when it is not directly chained, or when
String.isBlank()
is not available. + * @return the
strip()
or
trim()
invocation the given
length()
call is applied to, or + * null when there is none, when it is not directly chained, or when
String.isBlank()
is not available. */ @Nullable - private MethodInvocationTree getTrimCall(MethodInvocationTree lengthCall) { + private MethodInvocationTree getWhitespaceRemovalCall(MethodInvocationTree lengthCall) { if (context.getJavaVersion().asInt() >= JAVA_11 && lengthCall.methodSelect() instanceof MemberSelectExpressionTree sel && sel.expression() instanceof MethodInvocationTree mit - && STRING_TRIM_METHOD.matches(mit)) { + && STRING_WHITESPACE_REMOVAL_METHODS.matches(mit)) { return mit; } return null; diff --git a/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java index a802b5aac8d..20de16a6d42 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java @@ -42,7 +42,7 @@ void string_15() { } @Test - void trim_11() { + void blank_11() { CheckVerifier.newVerifier() .onFile(mainCodeSourcesPath("checks/StringIsBlankCheckSample.java")) .withCheck(new StringIsEmptyCheck()) @@ -51,7 +51,7 @@ void trim_11() { } @Test - void trim_11_without_semantic() { + void blank_11_without_semantic() { CheckVerifier.newVerifier() .onFile(mainCodeSourcesPath("checks/StringIsBlankCheckSample.java")) .withCheck(new StringIsEmptyCheck()) From e7435b576df7d680114ed7561abc82999515bdff Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Fri, 16 Oct 2026 10:17:09 +0400 Subject: [PATCH 4/5] Fix for QG --- .../sonar/java/checks/StringIsEmptyCheck.java | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java index 28512f5cee0..ce334a7395a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java @@ -141,29 +141,32 @@ public void visitNode(Tree tree) { } if (lengthCall != null && isEmptinessCheck(comparisonType)) { - // `s.strip().length() == 0` and `s.trim().length() == 0` are blankness checks, which `isBlank()` expresses directly. MethodInvocationTree whitespaceRemovalCall = getWhitespaceRemovalCall(lengthCall); - boolean isBlankCheck = whitespaceRemovalCall != null; - boolean usesTrim = isBlankCheck && TRIM_NAME.equals(ExpressionUtils.methodName(whitespaceRemovalCall).name()); - - String replacedMethod = isBlankCheck ? "isBlank()" : "isEmpty()"; - MethodInvocationTree replacementStart = isBlankCheck ? whitespaceRemovalCall : lengthCall; - String message = "Use \"" + replacedMethod + "\" to check whether a \"" + ownerName(lengthCall) + "\" is " - + (isBlankCheck ? "blank" : "empty") + " or not." + (usesTrim ? (" " + TRIM_CAVEAT) : ""); - String fixDescription = "Replace with \"" + replacedMethod + "\"" + (usesTrim ? TRIM_FIX_CAVEAT : ""); QuickFixHelper .newIssue(context) .forRule(this) .onTree(tree) - .withMessage(message) - .withQuickFix(() -> getQuickFix(tree, lengthCall, replacementStart, replacedMethod, fixDescription, comparisonType)) + .withMessage(getMessage(lengthCall, whitespaceRemovalCall)) + .withQuickFix(() -> getQuickFix(tree, lengthCall, whitespaceRemovalCall, comparisonType)) .report(); } } + private static String getMessage(MethodInvocationTree lengthCall, @Nullable MethodInvocationTree whitespaceRemovalCall) { + String adjective = whitespaceRemovalCall != null ? "blank" : "empty"; + String caveat = isTrimBased(whitespaceRemovalCall) ? (" " + TRIM_CAVEAT) : ""; + + return "Use \"" + replacementMethodName(whitespaceRemovalCall) + "\" to check whether a \"" + + ownerName(lengthCall) + "\" is " + adjective + " or not." + caveat; + } + private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocationTree lengthInvocation, - MethodInvocationTree replacedInvocation, String replacedMethod, String fixDescription, ComparisonType comparisonType) { + @Nullable MethodInvocationTree whitespaceRemovalCall, ComparisonType comparisonType) { + String replacedMethod = replacementMethodName(whitespaceRemovalCall); + MethodInvocationTree replacedInvocation = whitespaceRemovalCall != null ? whitespaceRemovalCall : lengthInvocation; + String fixDescription = "Replace with \"" + replacedMethod + "\"" + (isTrimBased(whitespaceRemovalCall) ? TRIM_FIX_CAVEAT : ""); + // There are two cases to deal with: // s.length() OP CONST // CONST OP s.length() @@ -189,6 +192,14 @@ private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocat return builder.build(); } + private static String replacementMethodName(@Nullable MethodInvocationTree whitespaceRemovalCall) { + return whitespaceRemovalCall != null ? "isBlank()" : "isEmpty()"; + } + + private static boolean isTrimBased(@Nullable MethodInvocationTree whitespaceRemovalCall) { + return whitespaceRemovalCall != null && TRIM_NAME.equals(ExpressionUtils.methodName(whitespaceRemovalCall).name()); + } + private static String ownerName(MethodInvocationTree lengthCall) { if (lengthCall.methodSelect() instanceof MemberSelectExpressionTree sel) { return sel.expression().symbolType().name(); From 90954046a12e1eb3d732f64600a6631456151193 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Fri, 16 Oct 2026 14:31:37 +0400 Subject: [PATCH 5/5] SONARJAVA-4606 S7158: Cover the pre-Java-11 behaviour with a test Below Java 11 neither "strip()" nor "trim()" may be rewritten to "isBlank()", since that method does not exist yet. Pin that the comparison is still reported as an emptiness check on the trimmed string, with an "isEmpty()" quick fix. Co-Authored-By: Claude Opus 5 (1M context) --- .../StringIsBlankCheckSample_java10.java | 29 +++++++++++++++++++ .../java/checks/StringIsEmptyCheckTest.java | 9 ++++++ 2 files changed, 38 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample_java10.java diff --git a/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample_java10.java b/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample_java10.java new file mode 100644 index 00000000000..cece611f17c --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample_java10.java @@ -0,0 +1,29 @@ +package checks; + +public class StringIsBlankCheckSample_java10 { + public boolean sample(String s) { + boolean b; + + // `String.isBlank()` does not exist before Java 11, so these stay emptiness checks on the trimmed string + + b = s.strip().length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf1]] + // fix@qf1 {{Replace with "isEmpty()"}} + // edit@qf1 [[sc=19;ec=32]]{{isEmpty()}} + + b = s.strip().length() > 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf2]] + // fix@qf2 {{Replace with "isEmpty()"}} + // edit@qf2 [[sc=9;ec=9]]{{!}} + // edit@qf2 [[sc=19;ec=31]]{{isEmpty()}} + + b = s.trim().length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf3]] + // fix@qf3 {{Replace with "isEmpty()"}} + // edit@qf3 [[sc=18;ec=31]]{{isEmpty()}} + + b = 0 == s.trim().length(); // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf4]] + // fix@qf4 {{Replace with "isEmpty()"}} + // edit@qf4 [[sc=9;ec=14]]{{}} + // edit@qf4 [[sc=23;ec=31]]{{isEmpty()}} + + return b; + } +} \ No newline at end of file diff --git a/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java index 20de16a6d42..caf3f89ed31 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java @@ -60,6 +60,15 @@ void blank_11_without_semantic() { .verifyIssues(); } + @Test + void blank_10() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/StringIsBlankCheckSample_java10.java")) + .withCheck(new StringIsEmptyCheck()) + .withJavaVersion(10) + .verifyIssues(); + } + @Test void charSequence_15() { CheckVerifier.newVerifier()