Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
9a3ad61
WIP: S8909 implementation (incomplete after 5 attempts)
romainbrenguier Jun 23, 2026
b60c6f5
SONARJAVA-6489 Fix S8909 implementation and tests
romainbrenguier Jun 24, 2026
f5281da
SONARJAVA-6489 Add S8909 rule metadata and documentation
romainbrenguier Jun 24, 2026
84e8774
Address PR comment from gitar-bot[bot]
romainbrenguier Jun 24, 2026
47f04a7
Fix Windows CI failure
romainbrenguier Jun 24, 2026
35b482e
Update autoscan expected test results for S8909
romainbrenguier Jun 24, 2026
b1c1744
Merge branch 'master' into new-rule/SONARJAVA-6489-S8909
romainbrenguier Jun 25, 2026
841ff2a
Fix CI: Updated hardcoded count in AutoScanTest.java from 11 to 12 to…
romainbrenguier Jun 25, 2026
b83a035
Merge remote-tracking branch 'origin/master' into new-rule/SONARJAVA-…
romainbrenguier Jun 25, 2026
413d87d
Fix CI: Updated diff_S6813.json to reflect 3 additional false negativ…
romainbrenguier Jun 25, 2026
f1356d8
Address review comment from rombirli on java-checks/src/main/java/org…
romainbrenguier Jun 29, 2026
5ed3609
Address review comment from rombirli on java-checks/src/main/java/org…
romainbrenguier Jun 29, 2026
46ffe7f
Address review comment from rombirli on java-checks/src/main/java/org…
romainbrenguier Jun 29, 2026
d1273ff
Address review comment from rombirli on java-checks/src/main/java/org…
romainbrenguier Jun 29, 2026
113116d
Merge remote-tracking branch 'origin/master' into new-rule/SONARJAVA-…
romainbrenguier Jun 29, 2026
72490f1
Fix S8909 autoscan false positives
romainbrenguier Jun 29, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S6813",
"hasTruePositives": true,
"falseNegatives": 66,
"falseNegatives": 69,
"falsePositives": 0
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"ruleKey": "S8909",
"hasTruePositives": true,
"falseNegatives": 0,
"falsePositives": 0
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package checks.quarkus;

import io.quarkus.cache.CacheKeyGenerator;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.context.Dependent;
import jakarta.enterprise.context.RequestScoped;
import jakarta.inject.Inject;
import java.lang.reflect.Method;

class NoncompliantBasic implements CacheKeyGenerator { // Noncompliant {{Make this class a CDI bean by adding a scope annotation, or add a public no-args constructor.}}
// ^^^^^^^^^^^^^^^^^
private final ConfigService configService;

public NoncompliantBasic(ConfigService configService) {
this.configService = configService;
}

@Override
public Object generate(Method method, Object... methodParams) {
return configService.getPrefix() + methodParams[0];
}
}

class NoncompliantMultipleDependencies implements CacheKeyGenerator { // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
private final DatabaseService dbService;
private final CacheConfig config;

public NoncompliantMultipleDependencies(DatabaseService dbService, CacheConfig config) {
this.dbService = dbService;
this.config = config;
}

@Override
public Object generate(Method method, Object... methodParams) {
return dbService.format(methodParams[0]);
}
}

class NoncompliantMultipleConstructors implements CacheKeyGenerator { // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
private final String prefix;

public NoncompliantMultipleConstructors(String prefix) {
this.prefix = prefix;
}

public NoncompliantMultipleConstructors(String prefix, int timeout) {
this.prefix = prefix;
}

@Override
public Object generate(Method method, Object... methodParams) {
return prefix + methodParams[0];
}
}

class NoncompliantPrivateConstructor implements CacheKeyGenerator { // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
private NoncompliantPrivateConstructor() {}

@Override
public Object generate(Method method, Object... methodParams) {
return methodParams[0];
}
}

@ApplicationScoped
class CompliantApplicationScoped implements CacheKeyGenerator {
@Inject
ConfigService configService;

@Override
public Object generate(Method method, Object... methodParams) {
String prefix = configService.getPrefix();
return prefix + methodParams[0];
}
}

@Dependent
class CompliantDependent implements CacheKeyGenerator {
@Inject
ConfigService configService;

@Override
public Object generate(Method method, Object... methodParams) {
return configService.getPrefix() + methodParams[0];
}
}

@RequestScoped
class CompliantRequestScoped implements CacheKeyGenerator {
@Inject
ConfigService configService;

@Override
public Object generate(Method method, Object... methodParams) {
return configService.getPrefix() + methodParams[0];
}
}

class NoncompliantPackagePrivateImplicitConstructor implements CacheKeyGenerator { // Noncompliant
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@Override
public Object generate(Method method, Object... methodParams) {
return methodParams[0];
}
}

class CompliantExplicitNoArgsConstructor implements CacheKeyGenerator {
public CompliantExplicitNoArgsConstructor() {}

@Override
public Object generate(Method method, Object... methodParams) {
return methodParams[0];
}
}

class CompliantMultipleConstructorsWithNoArgs implements CacheKeyGenerator {
private final String prefix;

public CompliantMultipleConstructorsWithNoArgs() {
this.prefix = "default";
}

public CompliantMultipleConstructorsWithNoArgs(String prefix) {
this.prefix = prefix;
}

@Override
public Object generate(Method method, Object... methodParams) {
return prefix + methodParams[0];
}
}

abstract class CompliantAbstractClass implements CacheKeyGenerator {
}

interface CompliantInterface extends CacheKeyGenerator {
}

class NotACacheKeyGenerator {
public NotACacheKeyGenerator(String param) {}
}

class ConfigService {
public String getPrefix() {
return "prefix";
}
}

class DatabaseService {
public String format(Object obj) {
return obj.toString();
}
}

class CacheConfig {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package io.quarkus.cache;

import java.lang.reflect.Method;

/**
* Mock-up interface for Quarkus CacheKeyGenerator for testing purposes
*/
public interface CacheKeyGenerator {
Object generate(Method method, Object... methodParams);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* 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.quarkus;

import java.util.Collection;
import java.util.List;
import java.util.Objects;
import org.sonar.check.Rule;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.semantic.Symbol;
import org.sonar.plugins.java.api.tree.AnnotationTree;
import org.sonar.plugins.java.api.tree.ClassTree;
import org.sonar.plugins.java.api.tree.IdentifierTree;
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
import org.sonar.plugins.java.api.tree.Tree;

@Rule(key = "S8909")
Comment thread
gitar-bot[bot] marked this conversation as resolved.
public class CacheKeyGeneratorInstantiableCheck extends IssuableSubscriptionVisitor {

private static final String CACHE_KEY_GENERATOR = "io.quarkus.cache.CacheKeyGenerator";

private static final List<String> CDI_SCOPE_ANNOTATIONS = List.of(
"jakarta.enterprise.context.ApplicationScoped",
"jakarta.enterprise.context.Dependent",
"jakarta.enterprise.context.RequestScoped",
"jakarta.enterprise.context.SessionScoped",
"jakarta.enterprise.context.ConversationScoped",
"javax.enterprise.context.ApplicationScoped",
"javax.enterprise.context.Dependent",
"javax.enterprise.context.RequestScoped",
"javax.enterprise.context.SessionScoped",
"javax.enterprise.context.ConversationScoped"
);

@Override
public List<Tree.Kind> nodesToVisit() {
return List.of(Tree.Kind.CLASS);
}

@Override
public void visitNode(Tree tree) {
ClassTree classTree = (ClassTree) tree;
if (!isApplicableClass(classTree) || hasCdiScopeAnnotation(classTree) || hasPublicNoArgsConstructor(classTree)) {
return;
}
reportIssue(Objects.requireNonNull(classTree.simpleName()),
"Make this class a CDI bean by adding a scope annotation, or add a public no-args constructor.");
}

private static boolean isApplicableClass(ClassTree classTree) {
return !isAnonymous(classTree)
&& !classTree.symbol().isAbstract()
&& !classTree.symbol().isInterface()
&& implementsCacheKeyGenerator(classTree);
}

private static boolean isAnonymous(ClassTree classTree) {
return classTree.simpleName() == null;
}

private static boolean implementsCacheKeyGenerator(ClassTree classTree) {
return classTree.symbol().type().isSubtypeOf(CACHE_KEY_GENERATOR);
}

private static boolean hasCdiScopeAnnotation(ClassTree classTree) {
return CDI_SCOPE_ANNOTATIONS.stream().anyMatch(annotation ->
classTree.symbol().metadata().isAnnotatedWith(annotation)
|| classTree.modifiers().annotations().stream().anyMatch(tree -> matchesAnnotation(tree, annotation)));
}

private static boolean matchesAnnotation(AnnotationTree annotationTree, String fullyQualifiedName) {
String simpleName = fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf('.') + 1);
Tree annotationType = annotationTree.annotationType();
return switch (annotationType.kind()) {
case IDENTIFIER -> ((IdentifierTree) annotationType).name().equals(simpleName);
case MEMBER_SELECT -> {
MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) annotationType;
yield memberSelect.identifier().name().equals(simpleName) || memberSelect.expression().symbolType().is(fullyQualifiedName);
}
default -> false;
};
}
Comment on lines +85 to +96

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: Simple-name annotation match may suppress issues for unrelated annotations

matchesAnnotation falls back to matching only the annotation's simple name for the IDENTIFIER case (and as one OR branch for MEMBER_SELECT). Because all entries in CDI_SCOPE_ANNOTATIONS share common simple names (e.g. ApplicationScoped, Dependent, RequestScoped), any annotation with one of those simple names — regardless of its actual package — will be treated as a CDI scope annotation and suppress the issue. This is a reasonable tradeoff to avoid autoscan false positives when semantics are unresolved, but in fully-resolved compilations it could produce a false negative if a class uses an unrelated @Dependent/@RequestScoped from a different library and still lacks a real CDI scope. Consider preferring the resolved symbolType().is(fqn) check and only relying on the simple-name match when the type cannot be resolved (unknown type), to keep the fully-resolved path precise.

Was this helpful? React with 👍 / 👎


private static boolean hasPublicNoArgsConstructor(ClassTree classTree) {
Collection<Symbol> constructors = classTree.symbol().lookupSymbols("<init>");
return constructors.stream()
.map(Symbol.MethodSymbol.class::cast)
.filter(CacheKeyGeneratorInstantiableCheck::isNoArgConstructor)
.findFirst()
.map(Symbol::isPublic)
.orElse(classTree.symbol().isPublic());
}
Comment thread
gitar-bot[bot] marked this conversation as resolved.

private static boolean isNoArgConstructor(Symbol.MethodSymbol constructor) {
return constructor.parameterTypes().isEmpty();
}
Comment on lines +98 to +110

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A bit verbose, I propose

Suggested change
private static boolean hasPublicNoArgsConstructor(ClassTree classTree) {
Collection<Symbol> constructors = classTree.symbol().lookupSymbols("<init>");
var noArgConstructor = constructors.stream()
.filter(CacheKeyGeneratorInstantiableCheck::isNoArgConstructor)
.findFirst();
if (noArgConstructor.isEmpty()) {
return false;
}
Symbol constructor = noArgConstructor.get();
if (constructor.declaration() == null) {
return classTree.symbol().isPublic();
}
return constructor.isPublic();
}
private static boolean isNoArgConstructor(Symbol constructor) {
return ((Symbol.MethodSymbol) constructor).parameterTypes().isEmpty();
}
private static boolean hasPublicNoArgsConstructor(ClassTree classTree) {
Collection<Symbol> constructors = classTree.symbol().lookupSymbols("<init>");
return constructors.stream()
.map(Symbol.MethodSymbol.class::cast)
.filter(CacheKeyGeneratorInstantiableCheck::isNoArgConstructor)
.findFirst()
.map(Symbol::isPublic)
.orElse(classTree.symbol().isPublic());
}
private static boolean isNoArgConstructor(Symbol.MethodSymbol constructor) {
return constructor.parameterTypes().isEmpty();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* 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.quarkus;

import org.junit.jupiter.api.Test;
import org.sonar.java.checks.verifier.CheckVerifier;

import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;

class CacheKeyGeneratorInstantiableCheckTest {

@Test
void test() {
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/quarkus/CacheKeyGeneratorInstantiableCheckSample.java"))
.withCheck(new CacheKeyGeneratorInstantiableCheck())
.verifyIssues();
}
}
Loading
Loading