diff --git a/its/ruling/src/test/resources/expected/eclipse-jetty-similar-to-main/java-S9432.json b/its/ruling/src/test/resources/expected/eclipse-jetty-similar-to-main/java-S9432.json new file mode 100644 index 00000000000..64489596bfd --- /dev/null +++ b/its/ruling/src/test/resources/expected/eclipse-jetty-similar-to-main/java-S9432.json @@ -0,0 +1,5 @@ +{ +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java": [ +143 +] +} diff --git a/its/ruling/src/test/resources/expected/eclipse-jetty/java-S9432.json b/its/ruling/src/test/resources/expected/eclipse-jetty/java-S9432.json new file mode 100644 index 00000000000..e84fc39f2c0 --- /dev/null +++ b/its/ruling/src/test/resources/expected/eclipse-jetty/java-S9432.json @@ -0,0 +1,13 @@ +{ +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java": [ +143 +], +"eclipse-jetty:jetty-util/src/main/java/org/eclipse/jetty/util/HttpCookieStore.java": [ +84 +], +"eclipse-jetty:jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java": [ +1324, +1333, +1485 +] +} diff --git a/java-checks-test-sources/default/src/main/java/checks/S9432CheckSample.java b/java-checks-test-sources/default/src/main/java/checks/S9432CheckSample.java new file mode 100644 index 00000000000..5579fcc8962 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/S9432CheckSample.java @@ -0,0 +1,73 @@ +package checks; + +public class S9432CheckSample { + + boolean hasContent(String input) { + boolean result; + result = input.trim().length() == 0; // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + result = input.trim().length() <= 0; // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + result = input.trim().length() < 1; // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + result = input.strip().length() != 0; // Noncompliant {{Use "isBlank()" on the original string instead.}} + result = input.strip().length() > 0; // Noncompliant {{Use "isBlank()" on the original string instead.}} + result = input.strip().length() >= 1; // Noncompliant {{Use "isBlank()" on the original string instead.}} + result = 0 == input.trim().length(); // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + result = 0 >= input.trim().length(); // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + result = 1 > input.trim().length(); // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + result = 0 != input.strip().length(); // Noncompliant {{Use "isBlank()" on the original string instead.}} + result = 0 < input.strip().length(); // Noncompliant {{Use "isBlank()" on the original string instead.}} + result = 1 <= input.strip().length(); // Noncompliant {{Use "isBlank()" on the original string instead.}} + result = input.trim().isEmpty(); // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + result = !input.strip().isEmpty(); // Noncompliant {{Use "isBlank()" on the original string instead.}} + result = input.trim().isBlank(); // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + result = input.strip().isBlank(); // Noncompliant {{Use "isBlank()" on the original string instead.}} + result = (input.trim().length()) == 0; // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + result = 0 < ((input.strip().length())); // Noncompliant {{Use "isBlank()" on the original string instead.}} + result = (input.trim()).isEmpty(); // Noncompliant {{Use "isBlank()" on the original string instead, if Unicode whitespace should be treated as blank and control characters as non-blank.}} + return result; + } + + void validUses(String input) { + boolean blank = input.isBlank(); + boolean present = !input.isBlank(); + boolean longEnough = input.trim().length() > 1; + boolean exactLength = input.strip().length() == 5; + boolean otherComparisons = input.trim().length() != 2 || input.trim().length() <= 1 || input.trim().length() < 2 || input.trim().length() >= 2 + || 2 != input.trim().length() || 1 >= input.trim().length() || 2 > input.trim().length() || 2 <= input.trim().length() || 5 == input.trim().length() || 0 > input.trim().length(); + int trimmedLength = input.trim().length(); + boolean sumIsZero = input.trim().length() + trimmedLength == 0; + boolean parenthesizedSum = (input.strip().length()) + 1 > 0; + boolean matches = input.trim().equals("value"); + String normalized = input.trim(); + audit(input.strip()); + audit(normalized + blank + present + longEnough + exactLength + matches + sumIsZero + parenthesizedSum + otherComparisons); + } + + void audit(String input) { + } + + static class TextBuffer { + TextBuffer trim() { + return this; + } + + TextBuffer strip() { + return this; + } + + int length() { + return 0; + } + + boolean isEmpty() { + return true; + } + + boolean isBlank() { + return true; + } + } + + boolean customText(TextBuffer input) { + return input.trim().length() == 0 || input.strip().isEmpty() || input.trim().isBlank(); + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/S9432Check.java b/java-checks/src/main/java/org/sonar/java/checks/S9432Check.java new file mode 100644 index 00000000000..528bec58a66 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/S9432Check.java @@ -0,0 +1,129 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.List; +import javax.annotation.Nullable; +import org.sonar.check.Rule; +import org.sonar.java.model.ExpressionUtils; +import org.sonar.java.model.LiteralUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.JavaVersion; +import org.sonar.plugins.java.api.JavaVersionAwareVisitor; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.tree.BinaryExpressionTree; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S9432") +public class S9432Check extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor { + + private static final String MESSAGE = "Use \"isBlank()\" on the original string instead."; + private static final String TRIM_MESSAGE = "Use \"isBlank()\" on the original string instead, " + + "if Unicode whitespace should be treated as blank and control characters as non-blank."; + private static final String STRING_TYPE = "java.lang.String"; + + private static final MethodMatchers WHITESPACE_REMOVAL_METHODS = MethodMatchers.create() + .ofTypes(STRING_TYPE) + .names("trim", "strip") + .addWithoutParametersMatcher() + .build(); + + private static final MethodMatchers BLANKNESS_METHODS = MethodMatchers.create() + .ofTypes(STRING_TYPE) + .names("isEmpty", "isBlank") + .addWithoutParametersMatcher() + .build(); + + private static final MethodMatchers LENGTH_METHOD = MethodMatchers.create() + .ofTypes(STRING_TYPE) + .names("length") + .addWithoutParametersMatcher() + .build(); + + @Override + public boolean isCompatibleWithJavaVersion(JavaVersion version) { + return version.asInt() >= 11; + } + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.METHOD_INVOCATION); + } + + @Override + public void visitNode(Tree tree) { + MethodInvocationTree invocation = (MethodInvocationTree) tree; + MethodInvocationTree removalInvocation = whitespaceRemovalInvocation(invocation); + if (removalInvocation == null) { + return; + } + if (BLANKNESS_METHODS.matches(invocation) || (LENGTH_METHOD.matches(invocation) && isEmptinessCheck(invocation))) { + String message = "trim".equals(removalInvocation.methodSymbol().name()) ? TRIM_MESSAGE : MESSAGE; + reportIssue(removalInvocation, message); + } + } + + @Nullable + private static MethodInvocationTree whitespaceRemovalInvocation(MethodInvocationTree invocation) { + ExpressionTree receiver = receiver(invocation); + if (receiver instanceof MethodInvocationTree receiverInvocation && WHITESPACE_REMOVAL_METHODS.matches(receiverInvocation)) { + return receiverInvocation; + } + return null; + } + + @Nullable + private static ExpressionTree receiver(MethodInvocationTree invocation) { + if (invocation.methodSelect() instanceof MemberSelectExpressionTree memberSelect) { + return ExpressionUtils.skipParentheses(memberSelect.expression()); + } + return null; + } + + private static boolean isEmptinessCheck(MethodInvocationTree lengthInvocation) { + Tree parent = lengthInvocation.parent(); + while (parent.is(Tree.Kind.PARENTHESIZED_EXPRESSION)) { + parent = parent.parent(); + } + if (!(parent instanceof BinaryExpressionTree binaryExpression)) { + return false; + } + ExpressionTree left = ExpressionUtils.skipParentheses(binaryExpression.leftOperand()); + if (left == lengthInvocation) { + ExpressionTree right = ExpressionUtils.skipParentheses(binaryExpression.rightOperand()); + return comparisonWithLiteralOnRight(binaryExpression, right); + } + return comparisonWithLiteralOnLeft(binaryExpression, left); + } + + private static boolean comparisonWithLiteralOnRight(BinaryExpressionTree binaryExpression, ExpressionTree literal) { + return (binaryExpression.is(Tree.Kind.EQUAL_TO, Tree.Kind.LESS_THAN_OR_EQUAL_TO) && LiteralUtils.isZero(literal)) + || (binaryExpression.is(Tree.Kind.NOT_EQUAL_TO, Tree.Kind.GREATER_THAN) && LiteralUtils.isZero(literal)) + || (binaryExpression.is(Tree.Kind.LESS_THAN) && LiteralUtils.isOne(literal)) + || (binaryExpression.is(Tree.Kind.GREATER_THAN_OR_EQUAL_TO) && LiteralUtils.isOne(literal)); + } + + private static boolean comparisonWithLiteralOnLeft(BinaryExpressionTree binaryExpression, ExpressionTree literal) { + return (LiteralUtils.isZero(literal) && binaryExpression.is(Tree.Kind.EQUAL_TO, Tree.Kind.GREATER_THAN_OR_EQUAL_TO)) + || (LiteralUtils.isZero(literal) && binaryExpression.is(Tree.Kind.NOT_EQUAL_TO, Tree.Kind.LESS_THAN)) + || (LiteralUtils.isOne(literal) && binaryExpression.is(Tree.Kind.GREATER_THAN)) + || (LiteralUtils.isOne(literal) && binaryExpression.is(Tree.Kind.LESS_THAN_OR_EQUAL_TO)); + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/S9432CheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/S9432CheckTest.java new file mode 100644 index 00000000000..4fbdb06e86b --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/S9432CheckTest.java @@ -0,0 +1,52 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class S9432CheckTest { + + @Test + void detects_blankness_checks_after_whitespace_removal() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/S9432CheckSample.java")) + .withCheck(new S9432Check()) + .withJavaVersion(11) + .verifyIssues(); + } + + @Test + void ignores_pre_java_11_code() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/S9432CheckSample.java")) + .withCheck(new S9432Check()) + .withJavaVersion(8) + .verifyNoIssues(); + } + + @Test + void ignores_code_without_semantic_information() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/S9432CheckSample.java")) + .withCheck(new S9432Check()) + .withoutSemantic() + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9432.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9432.html new file mode 100644 index 00000000000..7175fdf8a8b --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9432.html @@ -0,0 +1,57 @@ +

This rule raises an issue when code calls a method that removes leading and trailing whitespace from a string and then checks its length or +emptiness to determine if the string contains only whitespace.

+

In Java, this specifically refers to the trim() and strip() methods.

+

Why is this an issue?

+

Invoking operations that create new string objects with whitespace removed incurs unnecessary memory allocation and management overhead. When the +goal is simply to check if a string contains only whitespace, creating this intermediate string object is wasteful.

+

The following patterns are inefficient:

+ +

These checks create a temporary string object just to examine it, which is discarded immediately after the check completes.

+

In performance-sensitive code or when processing many strings, this inefficiency can accumulate, increasing memory pressure and CPU usage for +memory management operations.

+

In Java specifically, this applies to patterns like string.trim().length() > 0, string.trim().isEmpty(), +string.strip().length() > 0, string.strip().isEmpty(), or string.strip().isBlank().

+

What is the potential impact?

+

The performance impact of this issue depends on how frequently the code executes:

+ +

The issue also affects code readability. Using the appropriate whitespace-checking method makes the intent clearer than checking the length of a +trimmed string.

+

In Java, the appropriate method is isBlank().

+

How to fix it

+

For Java 11 and later, replace trim/strip-based checks with the isBlank() method, which efficiently checks if a string contains only +whitespace without creating intermediate objects.

+

Code examples

+

Noncompliant code example

+
+public void validate(String input) {
+    if (input != null && input.trim().length() > 0) { // Noncompliant
+        process(input);
+    }
+}
+
+

Compliant solution

+
+public void validate(String input) {
+    if (input != null && !input.isBlank()) {
+        process(input);
+    }
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9432.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9432.json new file mode 100644 index 00000000000..dc6fe1f0be3 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9432.json @@ -0,0 +1,25 @@ +{ + "title": "Strings should not be trimmed or stripped just to check if they are blank", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "performance", + "optimization", + "pitfall" + ], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-9432", + "sqKey": "S9432", + "scope": "Main", + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "LOW" + }, + "attribute": "EFFICIENT" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9432 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9432 new file mode 100644 index 00000000000..e69de29bb2d