diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json index 54f9c6aecab..dd83ce440c0 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json @@ -1,6 +1,6 @@ { "ruleKey": "S1874", "hasTruePositives": true, - "falseNegatives": 245, + "falseNegatives": 246, "falsePositives": 0 -} \ No newline at end of file +} diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8714.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8714.json new file mode 100644 index 00000000000..458ea610c32 --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8714.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S8714", + "hasTruePositives": false, + "falseNegatives": 33, + "falsePositives": 0 +} diff --git a/its/ruling/src/test/resources/sonar-server/java-S8714.json b/its/ruling/src/test/resources/sonar-server/java-S8714.json new file mode 100644 index 00000000000..00ae7949108 --- /dev/null +++ b/its/ruling/src/test/resources/sonar-server/java-S8714.json @@ -0,0 +1,332 @@ +{ +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/app/EmbeddedTomcatTest.java": [ +73 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/app/StartupLogsTest.java": [ +72 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/app/TomcatConnectorsTest.java": [ +82 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/batch/ProjectDataLoaderMediumTest.java": [ +487 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/batch/ProjectDataLoaderTest.java": [ +121 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/component/ComponentImplTest.java": [ +97, +112 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/component/ComponentRootBuilderTest.java": [ +88 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/event/EventRepositoryImplTest.java": [ +75 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/MutableMovedFilesRepositoryImplTest.java": [ +70 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/issue/ScmAccountToUserLoaderTest.java": [ +89 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/measure/MapBasedRawMeasureRepositoryTest.java": [ +187, +236, +246 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/measure/MeasureRepositoryImplTest.java": [ +114, +124, +244, +293, +303 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/qualitygate/ConditionEvaluatorTest.java": [ +68, +76, +84 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/step/ComputeQProfileMeasureStepTest.java": [ +140 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/step/PersistProjectLinksStepTest.java": [ +207 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java": [ +134 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/debt/DebtModelPluginRepositoryTest.java": [ +93 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/debt/DebtRulesXMLImporterTest.java": [ +228 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/email/ws/SendActionTest.java": [ +109 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/IndexDefinitionContextTest.java": [ +43 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/NewIndexTest.java": [ +53 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/SearchOptionsTest.java": [ +76 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/SortingTest.java": [ +128 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyClearCacheRequestBuilderTest.java": [ +77, +87, +97 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyClusterHealthRequestBuilderTest.java": [ +70, +80, +90 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyClusterStateRequestBuilderTest.java": [ +66, +76, +86 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyClusterStatsRequestBuilderTest.java": [ +65, +75, +85 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyCreateIndexRequestBuilderTest.java": [ +66, +76, +86 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyDeleteRequestBuilderTest.java": [ +61, +71, +81 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyFlushRequestBuilderTest.java": [ +62, +73, +83, +93 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyGetRequestBuilderTest.java": [ +69, +80, +90, +100 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyIndexRequestBuilderTest.java": [ +65, +77, +88, +98, +108 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyIndicesExistsRequestBuilderTest.java": [ +58, +76, +86, +96 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyIndicesStatsRequestBuilderTest.java": [ +63, +74, +84, +94 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyMultiGetRequestBuilderTest.java": [ +76, +86, +96 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyPutMappingRequestBuilderTest.java": [ +75, +86, +96, +106 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxyRefreshRequestBuilderTest.java": [ +65, +76, +86, +96 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxySearchRequestBuilderTest.java": [ +65, +76, +86, +96 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/request/ProxySearchScrollRequestBuilderTest.java": [ +71, +82, +92, +102 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/CommentActionTest.java": [ +70 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/IssueQueryFactoryTest.java": [ +295 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/index/IssueIndexTest.java": [ +1299 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/index/IssueIndexerTest.java": [ +116 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/workflow/IssueWorkflowTest.java": [ +123, +214 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/workflow/TransitionTest.java": [ +71, +81, +91, +101 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/notification/email/EmailNotificationChannelTest.java": [ +84, +179 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/organization/DefaultOrganizationProviderImplTest.java": [ +121 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/organization/OrganizationCreationImplTest.java": [ +158 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/organization/OrganizationValidationImplTest.java": [ +79, +94, +142, +176, +205, +234 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/permission/GroupPermissionChangerTest.java": [ +90, +320, +337, +354 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/permission/ws/AddGroupActionTest.java": [ +372 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/permission/ws/template/DeleteTemplateActionTest.java": [ +132, +153 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/platform/db/EmbeddedDatabaseTest.java": [ +165 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/platform/web/requestid/RequestIdFilterTest.java": [ +82 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/plugins/PluginDownloaderTest.java": [ +172, +212, +230 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/plugins/ServerPluginRepositoryTest.java": [ +131, +219, +295, +302 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/project/ws/UpdateVisibilityActionTest.java": [ +155, +193 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java": [ +187, +200, +212 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/qualityprofile/QProfileCopierTest.java": [ +92 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/qualityprofile/QProfileExportersTest.java": [ +131, +141 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java": [ +1150 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java": [ +236 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/qualityprofile/ws/RestoreBuiltInActionTest.java": [ +54 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/DeprecatedRulesDefinitionLoaderTest.java": [ +203 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/RuleCreatorTest.java": [ +292 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/RuleTagHelperTest.java": [ +64 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/RuleUpdaterTest.java": [ +97, +515, +540, +556, +574, +592 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/index/RuleIndexTest.java": [ +931 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/search/BaseDocTest.java": [ +83 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/source/ws/HashActionTest.java": [ +89 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/user/DoPrivilegedTest.java": [ +67 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/user/SecurityRealmFactoryTest.java": [ +67, +101, +124 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/user/UserSessionFilterTest.java": [ +114, +140, +157, +170, +196, +213, +226 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/user/UserUpdaterTest.java": [ +392 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/util/ObjectInputStreamIteratorTest.java": [ +49, +68 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/util/RubyUtilsTest.java": [ +54, +60 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/util/TypeValidationsTest.java": [ +61 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/util/cache/DiskCacheTest.java": [ +57, +73 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/util/cache/MemoryCacheTest.java": [ +63 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/ws/RemovedWebServiceHandlerTest.java": [ +43 +] +} diff --git a/java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java new file mode 100644 index 00000000000..3fe775336f7 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java @@ -0,0 +1,84 @@ +package checks; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class AssertThrowsInsteadOfTryCatchFailCheckSample { + @Test + void tests() { + try { // Noncompliant {{Use assertThrows() instead of try/catch and fail() in the try block.}} +// ^[el=+4;ec=5] + raise(); + fail(); + } catch (Exception _) { + // test passed + } + + try { + dontRaise(); + } catch (Exception _) { // Noncompliant {{Use assertDoesNotThrow() instead of try/catch and fail() in the catch block.}} +// ^[el=+3;ec=5] + fail(); + } + + try { // Noncompliant {{Use assertThrows() instead of try/catch and fail() in the try block.}} +// ^[el=+4;ec=5] + raise(); + org.junit.Assert.fail("expected exception"); + } catch (Exception _) { + // test passed + } + + try { // Noncompliant {{Use assertThrows() instead of try/catch and fail() in the try block.}} +// ^[el=+4;ec=5] + raise(); + junit.framework.Assert.fail("expected exception"); + } catch (Exception _) { + // test passed + } + + try { // Noncompliant {{Use assertThrows() instead of try/catch and fail() in the try block.}} +// ^[el=+4;ec=5] + raise(); + org.fest.assertions.Fail.fail("expected exception"); + } catch (Exception _) { + // test passed + } + + try { // Noncompliant {{Use assertThrows() instead of try/catch and fail() in the try block.}} +// ^[el=+4;ec=5] + raise(); + org.assertj.core.api.Fail.fail("expected exception"); + } catch (Exception _) { + // test passed + } + + try { // Noncompliant {{Use assertThrows() instead of try/catch and fail() in the try block.}} +// ^[el=+4;ec=5] + raise(); + org.assertj.core.api.Assertions.fail("expected exception"); + } catch (Exception _) { + // test passed + } + + try { // Noncompliant {{Use assertThrows() instead of try/catch and fail() in the try block.}} +// ^[el=+4;ec=5] + raise(); + org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown(IllegalStateException.class); + } catch (Exception _) { + // test passed + } + + assertThrows(IllegalStateException.class, AssertThrowsInsteadOfTryCatchFailCheckSample::raise); // compliant + assertDoesNotThrow(AssertThrowsInsteadOfTryCatchFailCheckSample::dontRaise); // non-compliant + } + + private static void raise() { + throw new IllegalStateException(); + } + + private static void dontRaise() { + // do nothing + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java b/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java new file mode 100644 index 00000000000..e996ccfa654 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java @@ -0,0 +1,48 @@ +/* + * 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.sonar.check.Rule; +import org.sonar.java.checks.helpers.UnitTestUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.BlockTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.TryStatementTree; + +import java.util.List; + +@Rule(key = "S8714") +public class AssertThrowsInsteadOfTryCatchFailCheck extends IssuableSubscriptionVisitor { + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.TRY_STATEMENT); + } + + @Override + public void visitNode(Tree tree) { + TryStatementTree tryStatementTree = (TryStatementTree) tree; + checkBlock(tryStatementTree.block(), "Use assertThrows() instead of try/catch and fail() in the try block."); + tryStatementTree.catches().forEach(c -> checkBlock(c.block(), "Use assertDoesNotThrow() instead of try/catch and fail() in the catch block.")); + } + + private void checkBlock(BlockTree block, String message) { + if (UnitTestUtils.doesBlockFail(block)) { + reportIssue(block, message); + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/helpers/UnitTestUtils.java b/java-checks/src/main/java/org/sonar/java/checks/helpers/UnitTestUtils.java index ead40f817eb..8377a1a1ee7 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/helpers/UnitTestUtils.java +++ b/java-checks/src/main/java/org/sonar/java/checks/helpers/UnitTestUtils.java @@ -254,7 +254,7 @@ public static boolean methodNameMatchesAssertionMethodPattern(String methodName, return ASSERTION_METHODS_PATTERN.matcher(methodName).matches(); } - public static boolean isTryCatchFail(BlockTree block) { + public static boolean doesBlockFail(BlockTree block) { List statements = block.body(); if (statements.isEmpty()) { return false; diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/AbstractOneExpectedExceptionRule.java b/java-checks/src/main/java/org/sonar/java/checks/tests/AbstractOneExpectedExceptionRule.java index 5411f979918..d363db0067e 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/AbstractOneExpectedExceptionRule.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/AbstractOneExpectedExceptionRule.java @@ -35,7 +35,7 @@ import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.TryStatementTree; -import static org.sonar.java.checks.helpers.UnitTestUtils.isTryCatchFail; +import static org.sonar.java.checks.helpers.UnitTestUtils.doesBlockFail; public abstract class AbstractOneExpectedExceptionRule extends IssuableSubscriptionVisitor { @@ -120,7 +120,7 @@ private void visitMethodInvocation(MethodInvocationTree mit) { } private void visitTryStatement(TryStatementTree tryStatementTree) { - if (isTryCatchFail(tryStatementTree.block())) { + if (doesBlockFail(tryStatementTree.block())) { List expectedTypes = tryStatementTree.catches().stream().map(c -> c.parameter().type().symbolType()).toList(); reportMultipleCallInTree(expectedTypes, tryStatementTree.block(), tryStatementTree.tryKeyword(), "body of this try/catch"); } diff --git a/java-checks/src/test/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheckTest.java new file mode 100644 index 00000000000..8d9f8d5c019 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheckTest.java @@ -0,0 +1,34 @@ +/* + * 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 AssertThrowsInsteadOfTryCatchFailCheckTest { + + @Test + void detected() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/AssertThrowsInsteadOfTryCatchFailCheckSample.java")) + .withCheck(new AssertThrowsInsteadOfTryCatchFailCheck()) + .verifyIssues(); + } + +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8714.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8714.html new file mode 100644 index 00000000000..0b39e6e4160 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8714.html @@ -0,0 +1,106 @@ +

Using try-catch blocks combined with fail() to test for the presence or absence of exceptions is an anti-pattern.

+

Why is this an issue?

+

Modern assertion libraries have made the clunky try-fail-catch pattern obsolete by introducing cleaner alternatives. For example, +starting with JUnit 5, JUnit Jupiter provides assertThrows and assertDoesNotThrow. AssertJ offers similar methods.

+

Using try-catch with fail() for the same purpose adds boilerplate, makes the test intent less explicit, and is harder to +maintain. Dedicated exception assertions also make it straightforward to keep asserting on the exception when one is expected.

+

How to fix it

+

Replace try-catch blocks that rely on fail() to verify exception behavior with dedicated exception assertions from your +testing library:

+
    +
  • Starting with JUnit 5, JUnit Jupiter provides assertDoesNotThrow and assertThrows.
    • +
  • AssertJ provides assertThatCode, assertThatThrownBy, and assertThatExceptionOfType.
    • +
+

Code examples

+

Noncompliant code example

+
+@Test
+void testNoExceptionThrown() {
+  try {
+    userService.registerUser(validUser);
+  } catch (ValidationException e) {
+    fail("Should not have thrown any exception");
+  }
+}
+
+

Compliant solution

+
+// JUnit 5
+@Test
+void testNoExceptionWithAssertion() {
+  assertDoesNotThrow(() -> userService.registerUser(validUser));
+}
+
+

Noncompliant code example

+
+@Test
+void testExceptionIsThrown() {
+  try {
+    userService.registerUser(invalidUser);
+    fail("Expected ValidationException to be thrown");
+  } catch (ValidationException e) {
+    // Test passes, but code is verbose
+    assertEquals("Invalid email", e.getMessage());
+  }
+}
+
+

Compliant solution

+
+// JUnit 5
+@Test
+void testExceptionWithAssertion() {
+  ValidationException exception = assertThrows(ValidationException.class, () -> userService.registerUser(invalidUser));
+
+  assertEquals("Invalid email", exception.getMessage());
+}
+
+

Noncompliant code example

+
+@Test
+void testNoExceptionThrown() {
+  try {
+    userService.registerUser(validUser);
+  } catch (ValidationException e) {
+    fail("Should not have thrown any exception");
+  }
+}
+
+

Compliant solution

+
+// AssertJ
+@Test
+void testNoExceptionWithAssertion() {
+  assertThatCode(() -> userService.registerUser(validUser)).doesNotThrowAnyException();
+}
+
+

Noncompliant code example

+
+@Test
+void testExceptionIsThrown() {
+  try {
+    userService.registerUser(invalidUser);
+    fail("Expected ValidationException to be thrown");
+  } catch (ValidationException e) {
+    assertThat(e).hasMessage("Invalid email");
+  }
+}
+
+

Compliant solution

+
+// AssertJ
+@Test
+void testExceptionWithAssertion() {
+  assertThatThrownBy(() -> userService.registerUser(invalidUser))
+    .isInstanceOf(ValidationException.class)
+    .hasMessage("Invalid email");
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8714.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8714.json new file mode 100644 index 00000000000..44d6ad1abed --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8714.json @@ -0,0 +1,23 @@ +{ + "title": "Dedicated exception assertions should be used instead of \"try-catch\" with \"fail()\"", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "tests" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-8714", + "sqKey": "S8714", + "scope": "Tests", + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "CONVENTIONAL" + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_agentic_AI_profile.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_agentic_AI_profile.json index 49cea8eb4c2..f753df99887 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_agentic_AI_profile.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_agentic_AI_profile.json @@ -468,6 +468,7 @@ "S8450", "S8465", "S8469", - "S8696" + "S8696", + "S8714" ] } diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index c4ea4993f94..17a7f7bc53c 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -536,6 +536,7 @@ "S8694", "S8695", "S8696", - "S8700" + "S8700", + "S8714" ] } diff --git a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/JavaAgenticWayProfileTest.java b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/JavaAgenticWayProfileTest.java index 07b8e02b4e2..d890a914649 100644 --- a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/JavaAgenticWayProfileTest.java +++ b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/JavaAgenticWayProfileTest.java @@ -53,7 +53,7 @@ void profile_is_registered_as_expected() { BuiltInQualityProfilesDefinition.BuiltInQualityProfile actualProfile = profilesPerLanguages.get("java").get("Sonar agentic AI"); assertThat(actualProfile.isDefault()).isFalse(); assertThat(actualProfile.rules()) - .hasSize(468) + .hasSize(469) .extracting(BuiltInQualityProfilesDefinition.BuiltInActiveRule::ruleKey) .doesNotContainAnyElementsOf(List.of( "S101",