Skip to content

SONARJAVA-6405#5636

Draft
tomasz-tylenda-sonarsource wants to merge 3 commits into
masterfrom
tt/SONARJAVA-6405-multiple-beforeEach
Draft

SONARJAVA-6405#5636
tomasz-tylenda-sonarsource wants to merge 3 commits into
masterfrom
tt/SONARJAVA-6405-multiple-beforeEach

Conversation

@tomasz-tylenda-sonarsource
Copy link
Copy Markdown
Contributor

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource commented May 27, 2026


Summary by Gitar

  • New inspection rule:
    • Added OneBeforeEachAfterEachCheck to identify test classes containing multiple methods annotated with @BeforeEach or @Before.
  • Test coverage:
    • Created OneBeforeEachAfterEachCheckTest and OneBeforeEachAfterEachCheckSample to verify the rule functionality.
  • Rule metadata:
    • Added rule definition files S3360.json and S3360.html (Note: current metadata content currently contains placeholder text unrelated to this rule).

This will update automatically on new commits.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 27, 2026

Agentic Analysis: Early Results

Agentic Analysis and Context Augmentation are available on your project. Here are some issues that could have been prevented. Follow the links to learn how to put them into action.

4 issue(s) found across 2 file(s):

Rule File Line Message
java:S125 java-checks-test-sources/default/src/test/java/checks/tests/OneBeforeEachAfterEachCheckSample.java 8 This block of commented-out lines of code should be removed.
java:S139 java-checks-test-sources/default/src/test/java/checks/tests/OneBeforeEachAfterEachCheckSample.java 8 Move this trailing comment on the previous empty line.
java:S1134 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java 28 Take the required action to fix the issue indicated by this comment.
java:S2325 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java 54 Make "countAnnotations" a "static" method.

Analyzed by SonarQube Agentic Analysis in 5.4 s

@hashicorp-vault-sonar-prod
Copy link
Copy Markdown
Contributor

hashicorp-vault-sonar-prod Bot commented May 27, 2026

SONARJAVA-6405

@Rule(key = "S3360")
public class OneBeforeEachAfterEachCheck extends IssuableSubscriptionVisitor {

private static final String BEFORE_EACH = "org.junit.jupiter.api.BeforeEach";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug: Rule only checks @beforeeach, ignores @AfterEach

The class is named OneBeforeEachAfterEachCheck and the rule intent (per the ticket title) is to check both @BeforeEach and @AfterEach, but the implementation only defines and checks BEFORE_EACH. A corresponding AFTER_EACH = "org.junit.jupiter.api.AfterEach" constant and check is missing.

Add the @AfterEach check alongside the existing @beforeeach check:

private static final String BEFORE_EACH = "org.junit.jupiter.api.BeforeEach";
private static final String AFTER_EACH = "org.junit.jupiter.api.AfterEach";

// ... in visitNode, after the existing beforeEachCount check:

  long afterEachCount = classTree.members().stream()
    .filter(member -> member.is(Tree.Kind.METHOD))
    .map(MethodTree.class::cast)
    .filter(method -> method.symbol().metadata().isAnnotatedWith(AFTER_EACH))
    .count();
  if (afterEachCount > 1) {
    reportIssue(className, "Only one method should be annotated @AfterEach.");
  }
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

Comment on lines +1 to +2
{
"title": "Test class names should end with \"Test\" or \"TestCase\"",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug: S3360 rule metadata describes a completely different rule

The S3360.json title is "Test class names should end with 'Test' or 'TestCase'" and S3360.html documents a naming convention rule. These are clearly copied from another rule and do not match the @BeforeEach/@AfterEach check being implemented. This will surface incorrect documentation to users. The FIXME at line 28 of the check also indicates the rule key is provisional.

Was this helpful? React with 👍 / 👎

Comment on lines +1 to +15
package checks.tests;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.fail;

class OneBeforeEachAfterEachCheckSample { // Noncompliant {{Only one method should be annotated @Before(Each).}}

@BeforeEach
void setUp1() {
// pass
}

@BeforeEach
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Test sample only covers @beforeeach, not @before or @after*

The test sample OneBeforeEachAfterEachCheckSample.java only tests the case of multiple @BeforeEach methods. It doesn't test @Before, @AfterEach, @After, compliant cases (single annotation), or mixed annotation scenarios. This leaves most of the rule's intended behavior unverified.

Was this helpful? React with 👍 / 👎

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 28, 2026

Code Review ⚠️ Changes requested 2 resolved / 5 findings

Adds OneBeforeEachAfterEachCheck to detect redundant JUnit annotations, but the implementation fails to verify @AfterEach and @After, contains mismatched metadata, and exhibits incomplete test coverage.

⚠️ Bug: Rule only checks @BeforeEach, ignores @AfterEach

📄 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:32 📄 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:46-53 📄 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:32-33 📄 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:47-51 📄 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:47-50

The class is named OneBeforeEachAfterEachCheck and the rule intent (per the ticket title) is to check both @BeforeEach and @AfterEach, but the implementation only defines and checks BEFORE_EACH. A corresponding AFTER_EACH = "org.junit.jupiter.api.AfterEach" constant and check is missing.

Add the @AfterEach check alongside the existing @beforeeach check
private static final String BEFORE_EACH = "org.junit.jupiter.api.BeforeEach";
private static final String AFTER_EACH = "org.junit.jupiter.api.AfterEach";

// ... in visitNode, after the existing beforeEachCount check:

  long afterEachCount = classTree.members().stream()
    .filter(member -> member.is(Tree.Kind.METHOD))
    .map(MethodTree.class::cast)
    .filter(method -> method.symbol().metadata().isAnnotatedWith(AFTER_EACH))
    .count();
  if (afterEachCount > 1) {
    reportIssue(className, "Only one method should be annotated @AfterEach.");
  }
⚠️ Bug: S3360 rule metadata describes a completely different rule

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S3360.json:1-2 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S3360.html:1-4 📄 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:28-29

The S3360.json title is "Test class names should end with 'Test' or 'TestCase'" and S3360.html documents a naming convention rule. These are clearly copied from another rule and do not match the @BeforeEach/``@AfterEach check being implemented. This will surface incorrect documentation to users. The FIXME at line 28 of the check also indicates the rule key is provisional.

💡 Quality: Test sample only covers @BeforeEach, not @Before or @After*

📄 java-checks-test-sources/default/src/test/java/checks/tests/OneBeforeEachAfterEachCheckSample.java:1-15

The test sample OneBeforeEachAfterEachCheckSample.java only tests the case of multiple @BeforeEach methods. It doesn't test @Before, @AfterEach, @After, compliant cases (single annotation), or mixed annotation scenarios. This leaves most of the rule's intended behavior unverified.

✅ 2 resolved
Bug: Grammar error in issue message: "one methods"

📄 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:52
The reported message says "Only one methods should be annotated @BeforeEach." — "methods" should be singular "method".

Edge Case: Mixed @before and @beforeeach usage not flagged

📄 java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:49
If a class has one @Before method and one @BeforeEach method (which can happen during a JUnit 4→5 migration), the rule won't report an issue since each count is exactly 1. Consider whether beforeCount + beforeEachCount > 1 is the intended logic, or if the current per-annotation check is deliberate.

🤖 Prompt for agents
Code Review: Adds OneBeforeEachAfterEachCheck to detect redundant JUnit annotations, but the implementation fails to verify @AfterEach and @After, contains mismatched metadata, and exhibits incomplete test coverage.

1. ⚠️ Bug: Rule only checks @BeforeEach, ignores @AfterEach
   Files: java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:32, java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:46-53, java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:32-33, java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:47-51, java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:47-50

   The class is named `OneBeforeEachAfterEachCheck` and the rule intent (per the ticket title) is to check both `@BeforeEach` and `@AfterEach`, but the implementation only defines and checks `BEFORE_EACH`. A corresponding `AFTER_EACH = "org.junit.jupiter.api.AfterEach"` constant and check is missing.

   Fix (Add the @AfterEach check alongside the existing @BeforeEach check):
   private static final String BEFORE_EACH = "org.junit.jupiter.api.BeforeEach";
   private static final String AFTER_EACH = "org.junit.jupiter.api.AfterEach";
   
   // ... in visitNode, after the existing beforeEachCount check:
   
     long afterEachCount = classTree.members().stream()
       .filter(member -> member.is(Tree.Kind.METHOD))
       .map(MethodTree.class::cast)
       .filter(method -> method.symbol().metadata().isAnnotatedWith(AFTER_EACH))
       .count();
     if (afterEachCount > 1) {
       reportIssue(className, "Only one method should be annotated @AfterEach.");
     }

2. ⚠️ Bug: S3360 rule metadata describes a completely different rule
   Files: sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S3360.json:1-2, sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S3360.html:1-4, java-checks/src/main/java/org/sonar/java/checks/tests/OneBeforeEachAfterEachCheck.java:28-29

   The `S3360.json` title is *"Test class names should end with 'Test' or 'TestCase'"* and `S3360.html` documents a naming convention rule. These are clearly copied from another rule and do not match the @BeforeEach/@AfterEach check being implemented. This will surface incorrect documentation to users. The FIXME at line 28 of the check also indicates the rule key is provisional.

3. 💡 Quality: Test sample only covers @BeforeEach, not @Before or @After*
   Files: java-checks-test-sources/default/src/test/java/checks/tests/OneBeforeEachAfterEachCheckSample.java:1-15

   The test sample `OneBeforeEachAfterEachCheckSample.java` only tests the case of multiple `@BeforeEach` methods. It doesn't test `@Before`, `@AfterEach`, `@After`, compliant cases (single annotation), or mixed annotation scenarios. This leaves most of the rule's intended behavior unverified.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.
Unblock → Override a blocking verdict and allow merging.

Comment with these commands to change:

Auto-apply Compact Unblock
gitar auto-apply:on         
gitar display:verbose         
gitar unblock         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant