Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -20,11 +20,14 @@
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Base64;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.annotation.Nullable;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.java.reporting.AnalyzerMessage;
import org.sonar.java.utils.PackageUtils;
Expand Down Expand Up @@ -64,8 +67,10 @@ public class BeanDefinitionGatherer extends SpringContextModelGatherer {
private static final String BEAN_SEPARATOR = "\n";
private static final String FIELD_SEPARATOR = "|";
private static final String DEP_SEPARATOR = ",";
private static final String DEP_KEY_VALUE_SEPARATOR = ":";

private static final String PRIMARY_ANNOTATION = "org.springframework.context.annotation.Primary";
private static final String VALUE_ATTRIBUTE = "value";

private final List<BeanData> collectedBeans = new ArrayList<>();

Expand All @@ -79,7 +84,7 @@ private record BeanData(
InputFile inputFile,
AnalyzerMessage.TextSpan textSpan,
boolean isPrimary,
List<String> dependingBeans) {
Map<String, String> dependingBeans) {
}

@Override
Expand Down Expand Up @@ -107,20 +112,16 @@ public void visitNode(Tree tree) {
if (SpringUtils.STEREOTYPE_ANNOTATIONS.stream().anyMatch(meta::isAnnotatedWith)) {
String beanName = extractBeanName(meta)
.orElseGet(() -> defaultBeanName(classTree.simpleName().name()));
List<String> deps = collectAutowiredDependencies(classTree);
Map<String, String> deps = collectAutowiredDependencies(classTree);
// Class-level bean (stereotype annotations)
collectedBeans.add(new BeanData(
var beanData = new BeanData(
beanName, fqn, pkg,
context.getInputFile(),
AnalyzerMessage.textSpanFor(classTree.simpleName()),
meta.isAnnotatedWith(PRIMARY_ANNOTATION),
deps));
beansCollectedAtFileLevel.add(new BeanData(
beanName, fqn, pkg,
context.getInputFile(),
AnalyzerMessage.textSpanFor(classTree.simpleName()),
meta.isAnnotatedWith(PRIMARY_ANNOTATION),
deps));
deps);
collectedBeans.add(beanData);
beansCollectedAtFileLevel.add(beanData);

// @Bean methods — only if class is a configuration/component class
for (MethodTree method : SpringUtils.getBeanMethods(classTree)) {
Expand Down Expand Up @@ -155,7 +156,10 @@ private static void writeToCache(JavaFileScannerContext context, List<BeanData>
}

private static String serializeBean(BeanData bean) {
Comment thread
gitar-bot[bot] marked this conversation as resolved.
var deps = String.join(DEP_SEPARATOR, bean.dependingBeans());
var deps = bean.dependingBeans().entrySet().stream()
.map(e -> Base64.getEncoder().encodeToString(e.getKey().getBytes(StandardCharsets.UTF_8))
+ DEP_KEY_VALUE_SEPARATOR + e.getValue())
.collect(Collectors.joining(DEP_SEPARATOR));
var span = bean.textSpan();
var encodedName = Base64.getEncoder().encodeToString(bean.beanName().getBytes(StandardCharsets.UTF_8));
return String.join(FIELD_SEPARATOR,
Expand Down Expand Up @@ -225,7 +229,14 @@ private static BeanData deserializeBean(String line, InputFile inputFile) {
Integer.parseInt(spanParts[2]),
Integer.parseInt(spanParts[3]));
boolean isPrimary = Boolean.parseBoolean(fields[4]);
List<String> deps = fields[5].isEmpty() ? List.of() : List.of(fields[5].split(DEP_SEPARATOR));
Map<String, String> deps = new LinkedHashMap<>();
if (!fields[5].isEmpty()) {
for (String entry : fields[5].split(DEP_SEPARATOR)) {
int idx = entry.indexOf(DEP_KEY_VALUE_SEPARATOR);
String key = new String(Base64.getDecoder().decode(entry.substring(0, idx)), StandardCharsets.UTF_8);
deps.put(key, entry.substring(idx + 1));
}
}
return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, deps);
}

Expand All @@ -234,7 +245,7 @@ private static Optional<String> extractBeanName(SymbolMetadata meta) {
List<SymbolMetadata.AnnotationValue> attrs = meta.valuesForAnnotation(annotation);
if (attrs != null) {
Optional<String> name = attrs.stream()
.filter(v -> "value".equals(v.name()) || "name".equals(v.name()))
.filter(v -> VALUE_ATTRIBUTE.equals(v.name()) || "name".equals(v.name()))
.map(v -> (String) v.value())
.filter(s -> !s.isBlank())
.findFirst();
Expand All @@ -255,7 +266,7 @@ private void collectBeanMethod(MethodTree method, String pkg) {
List<SymbolMetadata.AnnotationValue> attrs = beanMeta.valuesForAnnotation(SpringUtils.BEAN_ANNOTATION);
String beanName = Optional.ofNullable(attrs)
.flatMap(list -> list.stream()
.filter(v -> "value".equals(v.name()) || "name".equals(v.name()))
.filter(v -> VALUE_ATTRIBUTE.equals(v.name()) || "name".equals(v.name()))
.map(v -> {
Object val = v.value();
if (val instanceof Object[] arr && arr.length > 0) {
Expand All @@ -271,42 +282,61 @@ private void collectBeanMethod(MethodTree method, String pkg) {
? method.returnType().symbolType().fullyQualifiedName()
: "";

List<String> paramDeps = method.parameters().stream()
.map(p -> p.symbol().type().fullyQualifiedName())
.toList();
Map<String, String> paramDeps = parameterDependencies(method);

collectedBeans.add(new BeanData(
beanName, returnTypeFqn, pkg,
context.getInputFile(),
AnalyzerMessage.textSpanFor(method.simpleName()),
beanMeta.isAnnotatedWith(PRIMARY_ANNOTATION),
paramDeps));
beansCollectedAtFileLevel.add(new BeanData(
var beanData = new BeanData(
beanName, returnTypeFqn, pkg,
context.getInputFile(),
AnalyzerMessage.textSpanFor(method.simpleName()),
beanMeta.isAnnotatedWith(PRIMARY_ANNOTATION),
paramDeps));
paramDeps);
collectedBeans.add(beanData);
beansCollectedAtFileLevel.add(beanData);
}

private static List<String> collectAutowiredDependencies(ClassTree classTree) {
List<String> deps = new ArrayList<>();
private static Map<String, String> collectAutowiredDependencies(ClassTree classTree) {
Map<String, String> deps = new LinkedHashMap<>();
for (Tree member : classTree.members()) {
if (member.is(Tree.Kind.VARIABLE)) {
VariableTree field = (VariableTree) member;
if (member instanceof VariableTree field) {
if (field.symbol().metadata().isAnnotatedWith(SpringUtils.AUTOWIRED_ANNOTATION)) {
deps.add(field.symbol().type().fullyQualifiedName());
String typeFqn = field.symbol().type().fullyQualifiedName();
deps.put(dependencyKey(field.simpleName().name(), extractQualifier(field.symbol().metadata())), typeFqn);
}
} else if (member.is(Tree.Kind.CONSTRUCTOR, Tree.Kind.METHOD)) {
MethodTree method = (MethodTree) member;
if (method.symbol().metadata().isAnnotatedWith(SpringUtils.AUTOWIRED_ANNOTATION)) {
method.parameters().stream()
.map(p -> p.symbol().type().fullyQualifiedName())
.forEach(deps::add);
deps.putAll(parameterDependencies(method));
}
}
}
Comment on lines +297 to 311

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: Map keyed by field/param name silently drops colliding dependencies

Switching dependingBeans from a List to a Map keyed by qualifier-or-field/param-name means two distinct dependencies that share a key collapse into one, silently losing a dependency the old list preserved. Collisions are reachable: e.g. an @Autowired field named service plus an @Autowired setter/constructor param also named service, or a @Qualifier("x") dependency alongside another dependency whose field name is x (putAll/put overwrite). Consider keying by a guaranteed-unique value or keeping a collection of dependencies per key if multiple injection points must be retained.

Allow multiple dependency types per key to avoid silently dropping colliding entries.:

// If multiple deps can legitimately share a name, keep them all, e.g.:
Map<String, List<String>> deps = new LinkedHashMap<>();
...
deps.computeIfAbsent(dependencyKey(name, qualifier), k -> new ArrayList<>()).add(typeFqn);
  • Apply fix

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

return deps;
}

private static Map<String, String> parameterDependencies(MethodTree method) {
Map<String, String> deps = new LinkedHashMap<>();
for (var p : method.parameters()) {
String typeFqn = p.symbol().type().fullyQualifiedName();
deps.put(dependencyKey(p.simpleName().name(), extractQualifier(p.symbol().metadata())), typeFqn);
}
return deps;
}

private static String dependencyKey(String fieldOrParamName, @Nullable String qualifier) {
return qualifier != null ? qualifier : fieldOrParamName;
}

@Nullable
private static String extractQualifier(SymbolMetadata metadata) {
List<SymbolMetadata.AnnotationValue> attrs = metadata.valuesForAnnotation(SpringUtils.QUALIFIER_ANNOTATION);
if (attrs == null) {
return null;
}
return attrs.stream()
.filter(v -> VALUE_ATTRIBUTE.equals(v.name()))
.map(v -> (String) v.value())
.filter(s -> !s.isBlank())
.findFirst()
.orElse(null);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
*/
package org.sonar.java.model.springcontext;

import java.util.ArrayList;
import java.util.List;
import java.util.LinkedHashMap;
import java.util.Map;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -51,8 +51,12 @@ public class BeanDefinitionHolder {
/** Source location where the bean definition appears. */
private final BeanLocation location;

/** Names of other beans this bean depends on. */
private List<String> dependingBeans;
/**
* Dependencies this bean requires.
* Key: the {@code @Qualifier} value if present, otherwise the field or parameter name at the injection point.
* Value: the fully-qualified type name.
*/
Comment thread
gitar-bot[bot] marked this conversation as resolved.
private Map<String, String> dependingBeans;

/** Comma-separated Spring profile expressions under which this bean is active, or {@code null} if unconditional. */
@Nullable
Expand All @@ -68,8 +72,8 @@ private BeanDefinitionHolder(String type, String module, String beanPackage, Bea
this.location = location;
}

private void setDependingBeans(List<String> beansList) {
this.dependingBeans = beansList;
private void setDependingBeans(Map<String, String> beans) {
this.dependingBeans = beans;
}

private void setProfiles(@Nullable String profiles) {
Expand All @@ -96,7 +100,7 @@ public BeanLocation getLocation() {
return location;
}

public List<String> getDependingBeans() {
public Map<String, String> getDependingBeans() {
return dependingBeans;
}

Expand All @@ -114,7 +118,7 @@ public static class Builder {
private final String module;
private final String beanPackage;
private final BeanLocation location;
private List<String> dependingBeans = new ArrayList<>();
private Map<String, String> dependingBeans = new LinkedHashMap<>();
@Nullable
private String profiles;
private boolean isPrimary = false;
Expand All @@ -126,8 +130,8 @@ public Builder(String type, String module, String beanPackage, BeanLocation loca
this.location = location;
}

public Builder dependingBeans(List<String> beansList) {
this.dependingBeans = beansList;
public Builder dependingBeans(Map<String, String> beans) {
this.dependingBeans = beans;
return this;
}

Expand All @@ -143,7 +147,7 @@ public Builder primary() {

public BeanDefinitionHolder build() {
BeanDefinitionHolder holder = new BeanDefinitionHolder(type, module, beanPackage, location);
holder.setDependingBeans(List.copyOf(dependingBeans));
holder.setDependingBeans(Map.copyOf(dependingBeans));
holder.setProfiles(profiles);
if (isPrimary) {
holder.setPrimary();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public final class SpringUtils {
public static final String REPOSITORY_ANNOTATION = "org.springframework.stereotype.Repository";
public static final String SERVICE_ANNOTATION = "org.springframework.stereotype.Service";
public static final String AUTOWIRED_ANNOTATION = "org.springframework.beans.factory.annotation.Autowired";
public static final String QUALIFIER_ANNOTATION = "org.springframework.beans.factory.annotation.Qualifier";
public static final String VALUE_ANNOTATION = "org.springframework.beans.factory.annotation.Value";
public static final String TRANSACTIONAL_ANNOTATION = "org.springframework.transaction.annotation.Transactional";
public static final String BEAN_ANNOTATION = "org.springframework.context.annotation.Bean";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package checks.spring.context;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.ApplicationContext;
import org.springframework.stereotype.Component;

@Component
class BlankQualifierDependency {

@Autowired
@Qualifier("")
private ApplicationContext applicationContext;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package checks.spring.context;

import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.env.Environment;

@Configuration
class QualifiedBeanMethodDependencies {

@Bean
Object myBean(
@Qualifier("primaryContext") ApplicationContext applicationContext,
Environment environment) {
return new Object();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package checks.spring.context;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.ApplicationContext;
import org.springframework.core.env.Environment;
import org.springframework.stereotype.Component;

@Component
class QualifiedConstructorDependencies {

private final ApplicationContext applicationContext;
private final Environment environment;

@Autowired
QualifiedConstructorDependencies(
@Qualifier("primaryContext") ApplicationContext applicationContext,
Environment environment) {
this.applicationContext = applicationContext;
this.environment = environment;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package checks.spring.context;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.ApplicationContext;
import org.springframework.core.env.Environment;
import org.springframework.stereotype.Component;

@Component
class QualifiedFieldDependencies {

@Autowired
@Qualifier("primaryContext")
private ApplicationContext applicationContext;

@Autowired
private Environment environment;
}
Loading
Loading