Skip to content

SONARJAVA-6635 Implement S9068: @ApplicationScoped should be preferred over @Singleton in Quarkus applications#5824

Open
NoemieBenard wants to merge 10 commits into
masterfrom
nb/sonarjava-6635-implement-S9068
Open

SONARJAVA-6635 Implement S9068: @ApplicationScoped should be preferred over @Singleton in Quarkus applications#5824
NoemieBenard wants to merge 10 commits into
masterfrom
nb/sonarjava-6635-implement-S9068

Conversation

@NoemieBenard

@NoemieBenard NoemieBenard commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary by Gitar

  • New code rule:
    • Implemented S9068 to encourage the use of @ApplicationScoped over @Singleton in Quarkus applications.
  • Heuristics and configuration:
    • Added logic to trigger the check only in files containing io.quarkus imports.
    • Added support for justifying comments to prevent false positives when @Singleton is used intentionally.
  • Test coverage:
    • Included unit tests for the new rule, covering both compliant and noncompliant scenarios for classes and records.

This will update automatically on new commits.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6635

Comment on lines +51 to +54
private static boolean hasJustifyingComment(AnnotationTree annotation) {
return annotation.firstToken().trivias().stream()
.anyMatch(trivia -> trivia.comment().toLowerCase(java.util.Locale.ROOT).contains("singleton"));
}

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: Justifying-comment heuristic matches any comment containing 'singleton'

The check suppresses an issue whenever a preceding comment merely contains the substring "singleton" (case-insensitive). Unrelated comments such as // TODO: remove this old singleton pattern will silently suppress a legitimate finding (false negative), and the match is not tied to an actual justification. Consider requiring a more explicit marker or documenting the heuristic's looseness; at minimum this behavior should be covered by a test asserting the intent.

Was this helpful? React with 👍 / 👎

Comment on lines +35 to +43
public List<Tree.Kind> nodesToVisit() {
return List.of(Tree.Kind.CLASS, Tree.Kind.METHOD);
}

@Override
public void visitNode(Tree tree) {
ModifiersTree modifiers = tree instanceof ClassTree classTree
? classTree.modifiers()
: ((MethodTree) tree).modifiers();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Edge Case: Rule ignores @singleton on enums, records and interfaces

nodesToVisit registers only Tree.Kind.CLASS and Tree.Kind.METHOD. A CDI bean declared as a record (or enum) is a distinct tree kind (RECORD/ENUM), so @Singleton on such declarations is never flagged. If those are intended targets, add the relevant kinds; otherwise consider documenting the intentional scope limitation with a test.

Was this helpful? React with 👍 / 👎

@NoemieBenard
NoemieBenard marked this pull request as ready for review July 21, 2026 13:21
@NoemieBenard
NoemieBenard requested a review from rombirli July 21, 2026 13:22
@gitar-bot

gitar-bot Bot commented Jul 21, 2026

Copy link
Copy Markdown

Analyzing CI failures

CI failed: The build failed due to duplicate class definitions in the new test source file and a Java version incompatibility in the integration test environment.

Overview

The CI build failed due to a combination of a compilation error caused by duplicate class names in the new test source file and a configuration issue in the integration test environment where the Java version was insufficient for the sonar-maven-plugin.

Failures

Duplicate Class Compilation Error (confidence: high)

  • Type: build
  • Affected jobs: 88358708610
  • Related to change: yes
  • Root cause: The new test file SingletonInsteadOfApplicationScopedCheckSample.java introduces classes CompliantApplicationScoped and CompliantDependent that already exist in the test source path.
  • Suggested fix: Rename the conflicting classes in java-checks-test-sources/default/src/main/java/checks/quarkus/SingletonInsteadOfApplicationScopedCheckSample.java to ensure uniqueness.

Java Version Incompatibility (confidence: high)

  • Type: build
  • Affected jobs: 88359320731
  • Related to change: no
  • Root cause: The integration test environment is running on Java 17, while the sonar-maven-plugin requires Java 21 or newer.
  • Suggested fix: Update the CI workflow configuration for the integration tests to use a JDK 21+ environment.

Summary

  • Change-related failures: 1 (Compilation failure due to name collision in new test code).
  • Infrastructure/flaky failures: 1 (Java version mismatch in the CI runner environment).
  • Recommended action: First, rename the conflicting classes in your PR to resolve the compilation error. Separately, the project's CI infrastructure team should update the integration test runners to use Java 21.
Code Review 👍 Approved with suggestions 2 resolved / 4 findings

Implements S9068 to prefer @ApplicationScoped over @Singleton in Quarkus, addressing detection of trailing comments and missing bean imports. Refine the comment-based suppression heuristic to be more precise and expand node support to include enums, records, and interfaces.

💡 Quality: Justifying-comment heuristic matches any comment containing 'singleton'

📄 java-checks/src/main/java/org/sonar/java/checks/quarkus/SingletonInsteadOfApplicationScopedCheck.java:51-54

The check suppresses an issue whenever a preceding comment merely contains the substring "singleton" (case-insensitive). Unrelated comments such as // TODO: remove this old singleton pattern will silently suppress a legitimate finding (false negative), and the match is not tied to an actual justification. Consider requiring a more explicit marker or documenting the heuristic's looseness; at minimum this behavior should be covered by a test asserting the intent.

💡 Edge Case: Rule ignores @Singleton on enums, records and interfaces

📄 java-checks/src/main/java/org/sonar/java/checks/quarkus/SingletonInsteadOfApplicationScopedCheck.java:35-43

nodesToVisit registers only Tree.Kind.CLASS and Tree.Kind.METHOD. A CDI bean declared as a record (or enum) is a distinct tree kind (RECORD/ENUM), so @Singleton on such declarations is never flagged. If those are intended targets, add the relevant kinds; otherwise consider documenting the intentional scope limitation with a test.

✅ 2 resolved
Edge Case: Justifying comment not detected when placed after @singleton

📄 java-checks/src/main/java/org/sonar/java/checks/quarkus/SingletonInsteadOfApplicationScopedCheck.java:51-54 📄 java-checks-test-sources/default/src/main/java/checks/quarkus/SingletonInsteadOfApplicationScopedCheckSample.java:22-23 📄 java-checks-test-sources/default/src/main/java/checks/quarkus/SingletonInsteadOfApplicationScopedCheckSample.java:32-34
hasJustifyingComment only inspects annotation.firstToken().trivias(), which contains comments on the lines ABOVE the annotation (leading trivia of the '@' token). A trailing comment on the same line, e.g. @Singleton // singleton needed for eager init, is attached to the following token (class/public), not to the annotation — as confirmed by TrailingCommentCheck (trivia appears in the next token's list on the previous token's line). The rule message explicitly tells users to "add a comment indicating why @singleton is necessary", so developers who add the natural inline comment will still get a false positive. Consider also scanning the trailing same-line comment (the trivia of the token immediately following the annotation) in addition to the leading trivia.

Edge Case: Quarkus heuristic misses beans that only import jakarta.*

📄 java-checks/src/main/java/org/sonar/java/checks/quarkus/SingletonInsteadOfApplicationScopedCheck.java:58-67
The rule only fires when a file contains at least one io.quarkus.* import. Many legitimate Quarkus beans annotate a class with @Singleton while importing only jakarta.inject.Singleton/jakarta.enterprise.context.* and nothing from io.quarkus, so those true positives are silently skipped (false negatives). If this trade-off is intentional to reduce noise it is fine, but consider also treating the presence of Quarkus CDI annotations (or a broader marker such as a Quarkus dependency) as a trigger so common cases are still covered.

🤖 Prompt for agents
Code Review: Implements S9068 to prefer `@ApplicationScoped` over `@Singleton` in Quarkus, addressing detection of trailing comments and missing bean imports. Refine the comment-based suppression heuristic to be more precise and expand node support to include enums, records, and interfaces.

1. 💡 Quality: Justifying-comment heuristic matches any comment containing 'singleton'
   Files: java-checks/src/main/java/org/sonar/java/checks/quarkus/SingletonInsteadOfApplicationScopedCheck.java:51-54

   The check suppresses an issue whenever a preceding comment merely contains the substring "singleton" (case-insensitive). Unrelated comments such as `// TODO: remove this old singleton pattern` will silently suppress a legitimate finding (false negative), and the match is not tied to an actual justification. Consider requiring a more explicit marker or documenting the heuristic's looseness; at minimum this behavior should be covered by a test asserting the intent.

2. 💡 Edge Case: Rule ignores @Singleton on enums, records and interfaces
   Files: java-checks/src/main/java/org/sonar/java/checks/quarkus/SingletonInsteadOfApplicationScopedCheck.java:35-43

   nodesToVisit registers only Tree.Kind.CLASS and Tree.Kind.METHOD. A CDI bean declared as a record (or enum) is a distinct tree kind (RECORD/ENUM), so `@Singleton` on such declarations is never flagged. If those are intended targets, add the relevant kinds; otherwise consider documenting the intentional scope limitation with a test.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

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

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

Copy link
Copy Markdown

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