diff --git a/.cirrus.yml b/.cirrus.yml index b45e5ebc4e0..df9d1dce8c3 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -148,7 +148,7 @@ plugin_qa_task: orchestrator_LATEST_RELEASE_cache: <<: *ORCHESTRATOR_CACHE_ELEMENTS_DEFINITION - env: - SQ_VERSION: DEV + SQ_VERSION: DEV[10.8] orchestrator_DEV_cache: <<: *ORCHESTRATOR_CACHE_ELEMENTS_DEFINITION diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S7158.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S7158.json new file mode 100644 index 00000000000..5a1bc4f4019 --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S7158.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S7158", + "hasTruePositives": true, + "falseNegatives": 0, + "falsePositives": 0 +} diff --git a/its/ruling/src/test/resources/commons-beanutils/java-S7158.json b/its/ruling/src/test/resources/commons-beanutils/java-S7158.json new file mode 100644 index 00000000000..64396ac4e01 --- /dev/null +++ b/its/ruling/src/test/resources/commons-beanutils/java-S7158.json @@ -0,0 +1,46 @@ +{ +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanPropertyValueChangeClosure.java": [ +137 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanPropertyValueEqualsPredicate.java": [ +167 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanToPropertyValueTransformer.java": [ +122 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/JDBCDynaClass.java": [ +193 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/MappedPropertyDescriptor.java": [ +90, +157, +203, +326 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java": [ +477, +833, +1578, +1963 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/converters/CharacterConverter.java": [ +72 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/converters/DateTimeConverter.java": [ +329 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/converters/NumberConverter.java": [ +261 +], +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/expression/DefaultResolver.java": [ +80, +93, +118, +144, +167, +182, +205, +228, +266 +] +} diff --git a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S7158.json b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S7158.json new file mode 100644 index 00000000000..1764b0743d3 --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S7158.json @@ -0,0 +1,72 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ +267, +298, +315, +317, +382, +396, +400, +508 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java": [ +202 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java": [ +69 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java": [ +469 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Cookies.java": [ +67 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java": [ +427, +476 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java": [ +166, +174 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/RequestLogWriter.java": [ +80 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java": [ +262, +388 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Response.java": [ +1299 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java": [ +1624, +1822, +2228 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/jmx/AbstractHandlerMBean.java": [ +79, +83 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java": [ +236 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java": [ +425, +468 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java": [ +719, +735, +768, +784 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java": [ +919, +934, +964, +1479, +1652, +1667 +] +} diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S7158.json b/its/ruling/src/test/resources/eclipse-jetty/java-S7158.json new file mode 100644 index 00000000000..b1fd0b5ab71 --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S7158.json @@ -0,0 +1,161 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ +267, +298, +315, +317, +382, +396, +400, +508 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java": [ +202 +], +"org.eclipse.jetty:jetty-project:jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java": [ +69 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java": [ +469 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Cookies.java": [ +67 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java": [ +427, +476 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java": [ +166, +174 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/RequestLogWriter.java": [ +80 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java": [ +262, +388 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Response.java": [ +1299 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java": [ +1624, +1822, +2228 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/jmx/AbstractHandlerMBean.java": [ +79, +83 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java": [ +236 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java": [ +425, +468 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/DumpHandler.java": [ +158 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java": [ +719, +735, +768, +784 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java": [ +919, +934, +964, +1479, +1652, +1667 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java": [ +1149 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/StopTest.java": [ +210 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/handler/InetAccessHandlerTest.java": [ +93, +100, +107, +114, +123 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLCloseTest.java": [ +79 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLSelectChannelConnectorLoadTest.java": [ +290 +], +"org.eclipse.jetty:jetty-project:jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerConfiguration.java": [ +180 +], +"org.eclipse.jetty:jetty-project:jetty-slf4j-impl/src/main/java/org/eclipse/jetty/logging/JettyLoggerFactory.java": [ +85, +120 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/MultiReleaseJarFile.java": [ +243 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/PathWatcher.java": [ +420 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java": [ +278, +342, +595 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/RolloverFileOutputStream.java": [ +174 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java": [ +66, +300, +303, +344, +347, +625, +631, +680, +686, +1029, +1122 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java": [ +131, +208, +242, +282, +365, +443, +588 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java": [ +339 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java": [ +126 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java": [ +341 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceCollection.java": [ +103, +210, +251 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/security/Password.java": [ +219, +223, +234 +], +"org.eclipse.jetty:jetty-project:jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java": [ +1324, +1333, +1485 +] +} diff --git a/its/ruling/src/test/resources/guava/java-S7158.json b/its/ruling/src/test/resources/guava/java-S7158.json new file mode 100644 index 00000000000..72b47f1d0c8 --- /dev/null +++ b/its/ruling/src/test/resources/guava/java-S7158.json @@ -0,0 +1,28 @@ +{ +"com.google.guava:guava:src/com/google/common/base/Splitter.java": [ +176 +], +"com.google.guava:guava:src/com/google/common/base/Strings.java": [ +78 +], +"com.google.guava:guava:src/com/google/common/io/Files.java": [ +730 +], +"com.google.guava:guava:src/com/google/common/net/InetAddresses.java": [ +233, +247, +250 +], +"com.google.guava:guava:src/com/google/common/net/InternetDomainName.java": [ +259 +], +"com.google.guava:guava:src/com/google/common/primitives/ParseRequest.java": [ +36 +], +"com.google.guava:guava:src/com/google/common/primitives/UnsignedLongs.java": [ +304 +], +"com.google.guava:guava:src/com/google/thirdparty/publicsuffix/TrieParser.java": [ +85 +] +} diff --git a/its/ruling/src/test/resources/sonar-server/java-S7158.json b/its/ruling/src/test/resources/sonar-server/java-S7158.json new file mode 100644 index 00000000000..b62b0241e21 --- /dev/null +++ b/its/ruling/src/test/resources/sonar-server/java-S7158.json @@ -0,0 +1,5 @@ +{ +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/user/UserUpdater.java": [ +325 +] +} 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 new file mode 100644 index 00000000000..068a002020b --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/StringIsEmptyCheckSample.java @@ -0,0 +1,54 @@ +package checks; + +public class StringIsEmptyCheckSample { + public boolean sample(String s, String t) { + boolean b; + + // test `length() == 0` and equivalent code + b = s.length() == 0; // Noncompliant + b = s.length() <= 0; // Noncompliant + b = s.length() < 1; // Noncompliant + + // test `length() != 0` and equivalent code + b = s.length() != 0; // Noncompliant + b = s.length() > 0; // Noncompliant + b = s.length() >= 1; // Noncompliant + + // reversed order + b = 0 == s.length(); // Noncompliant + b = 0 >= s.length(); // Noncompliant + b = 1 > s.length(); // Noncompliant + b = 0 != s.length(); // Noncompliant + b = 0 < s.length(); // Noncompliant + b = 1 <= s.length(); // Noncompliant + + // extra parentheses + b = (s.length()) == 0; // Noncompliant + + // chained method calls + b = s.toUpperCase().length() == 0; // Noncompliant + + // problem in a nested expression + b = "abc".equals(s) || s.length() == 0; // Noncompliant + + b = s.length() == 1; + b = s.length() > 3; + b = s.length() <= 10; + b = 2 < s.length(); + + b = s.trim().length() >= 8; + + b = s.length() == t.length(); + + b = s.isEmpty(); + b = !s.isEmpty(); + + b = 1 < 0; + + // StringBuilder does not have `isEmpty()` + StringBuilder stringBuilder = new StringBuilder(); + b = stringBuilder.length() == 0; + + return b; + } +} 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 new file mode 100644 index 00000000000..227439636f8 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java @@ -0,0 +1,106 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2024 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * 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 GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.java.checks; + +import java.util.List; +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.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.Tree.Kind; + +/** + * Implement rule that
String.length() == 0should be replaced with + *
String.isEmpty(). + */ +@Rule(key = "S7158") +public class StringIsEmptyCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor { + + private static final MethodMatchers LENGTH_METHOD = MethodMatchers.create() + .ofTypes("java.lang.String") + .names("length") + .addWithoutParametersMatcher() + .build(); + + private static final Kind[] TARGETED_BINARY_OPERATOR_TREES = { + Kind.EQUAL_TO, + Kind.NOT_EQUAL_TO, + Kind.LESS_THAN, + Kind.LESS_THAN_OR_EQUAL_TO, + Kind.GREATER_THAN, + Kind.GREATER_THAN_OR_EQUAL_TO + }; + + // `String.isEmpty()` is available since Java 6. + @Override + public boolean isCompatibleWithJavaVersion(JavaVersion version) { + return version.isJava6Compatible(); + } + + @Override + public List
length() OP VALUE. + */ + private static boolean isComparisonOnRight(BinaryExpressionTree tree, boolean rightIsZero, boolean rightIsOne) { + return (tree.is(Kind.EQUAL_TO, Kind.LESS_THAN_OR_EQUAL_TO, Kind.NOT_EQUAL_TO, Kind.GREATER_THAN) && rightIsZero) + || (tree.is(Kind.LESS_THAN, Kind.GREATER_THAN_OR_EQUAL_TO) && rightIsOne); + } + + /** + * Check the comparison for
VALUE OP length(). + */ + private static boolean isComparisonOnLeft(boolean leftIsZero, boolean leftIsOne, BinaryExpressionTree tree) { + return (leftIsZero && tree.is(Kind.EQUAL_TO, Kind.GREATER_THAN_OR_EQUAL_TO, Kind.NOT_EQUAL_TO, Kind.LESS_THAN)) + || (leftIsOne && tree.is(Kind.GREATER_THAN, Kind.LESS_THAN_OR_EQUAL_TO)); + } +} 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 new file mode 100644 index 00000000000..5249027d283 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java @@ -0,0 +1,45 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2024 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * 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 GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +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 StringIsEmptyCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/StringIsEmptyCheckSample.java")) + .withCheck(new StringIsEmptyCheck()) + .verifyIssues(); + } + + @Test + void testOlderJavaVersion() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/StringIsEmptyCheckSample.java")) + .withCheck(new StringIsEmptyCheck()) + .withJavaVersion(5) + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S7158.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S7158.html new file mode 100644 index 00000000000..5553eea55d7 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S7158.html @@ -0,0 +1,24 @@ +
Calling String.isEmpty()
clearly communicates the code’s intention, which is to test if the string is empty. Using
+String.length() == 0
is less direct and makes the code less readable.
+if ("string".length() == 0) { /* … */ } // Noncompliant + +if ("string".length() > 0) { /* … */ } // Noncompliant ++
+if ("string".isEmpty()){ /* … */ } + +if (!"string".isEmpty()){ /* … */ } ++