From 3351066fdfaff9bee7b510f1a9cf3e1ad08cf0fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Mon, 24 Aug 2026 16:52:45 +0200 Subject: [PATCH 01/22] Generate rule metadata --- .../org/sonar/l10n/java/rules/java/S9352.html | 188 ++++++++++++++++++ .../org/sonar/l10n/java/rules/java/S9352.json | 25 +++ .../java/rules/java/Sonar_way_profile.json | 3 +- 3 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9352.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9352.json diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9352.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9352.html new file mode 100644 index 00000000000..2e8599ff7b4 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9352.html @@ -0,0 +1,188 @@ +

This is an issue when a dependency injection framework cannot determine which component to inject because multiple components of the same type +exist in the application’s dependency injection container without explicit disambiguation.

+

In Spring Framework specifically, this occurs when the application context contains multiple beans of the same type.

+

Why is this an issue?

+

Dependency injection frameworks rely on matching component types to injection points. When you register multiple components of the same type in the +application context, the framework cannot automatically decide which component to inject.

+

For example, if you define two database connection pool components and try to inject a connection pool dependency without specifying which one to +use, the framework has no way to choose between them. This ambiguity violates the framework’s requirement for unambiguous component resolution.

+

At application startup, the framework will throw an exception indicating multiple matching candidates exist and fail to initialize the context. +This means your application will not start at all - it crashes before it can handle any requests or perform any work.

+

This happens because autowiring by type requires exactly one matching component. When multiple candidates exist, explicit disambiguation is +mandatory.

+

Dependency injection frameworks provide two mechanisms to resolve this ambiguity:

+ +

Without using one of these mechanisms, the application context cannot be created, and the application will fail to start.

+

In Spring specifically, this exception is called NoUniqueBeanDefinitionException. The disambiguation mechanisms are implemented using +the @Qualifier annotation (to specify which bean to inject), the @Primary annotation (to designate the default +bean), and the @Fallback annotation (available since Spring 6.2, to mark a bean as a last-resort candidate).

+

What is the potential impact?

+

The application will fail to start with an error indicating ambiguous dependency resolution during the dependency injection framework’s +initialization phase. This prevents the application from running and requires immediate attention to resolve the configuration error.

+

How to fix it in Spring

+

Use the @Qualifier annotation at the injection point to explicitly specify which bean to inject. The qualifier value must match the +bean name (typically the method name for @Bean methods).

+

This approach is useful when different parts of your application need different beans of the same type. Each injection point can specify exactly +which bean it requires.

+

Code examples

+

Noncompliant code example

+
+@Configuration
+public class DataSourceConfig {
+    @Bean
+    public DataSource primaryDataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    public DataSource secondaryDataSource() {
+        return new HikariDataSource();
+    }
+}
+
+@Service
+public class ReportService {
+    @Autowired
+    private DataSource dataSource; // Noncompliant; Spring cannot determine which DataSource bean to inject
+}
+
+

Compliant solution

+
+@Configuration
+public class DataSourceConfig {
+    @Bean
+    public DataSource primaryDataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    public DataSource secondaryDataSource() {
+        return new HikariDataSource();
+    }
+}
+
+@Service
+public class ReportService {
+    @Autowired
+    @Qualifier("primaryDataSource")
+    private DataSource dataSource; // Compliant; @Qualifier explicitly specifies that primaryDataSource needs to be used
+}
+
+

Use the @Primary annotation on one of the bean definitions to mark it as the default choice. When Spring encounters multiple beans of +the same type, it will automatically select the primary bean if no explicit qualifier is specified.

+

This approach is useful when you have a clear default bean that should be used in most cases, with alternative beans available for specific +scenarios.

+

Noncompliant code example

+
+@Configuration
+public class DataSourceConfig {
+    @Bean
+    public DataSource primaryDataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    public DataSource secondaryDataSource() {
+        return new HikariDataSource();
+    }
+}
+
+@Service
+public class ReportService {
+    @Autowired
+    private DataSource dataSource; // Noncompliant
+}
+
+

Compliant solution

+
+@Configuration
+public class DataSourceConfig {
+    @Bean
+    @Primary
+    public DataSource primaryDataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    public DataSource secondaryDataSource() {
+        return new HikariDataSource();
+    }
+}
+
+@Service
+public class ReportService {
+    @Autowired
+    private DataSource dataSource; // Compliant; will use default primaryDataSource
+}
+
+

Use the @Fallback annotation on a bean definition to mark it as a last-resort candidate (available since Spring 6.2). A fallback bean +is only selected when no non-fallback bean of the same type is present in the application context.

+

This approach is the semantic inverse of @Primary: instead of promoting one bean, it demotes another. It is particularly useful for +library or auto-configuration beans that should yield to any user-defined bean of the same type.

+

Noncompliant code example

+
+@Configuration
+public class DataSourceConfig {
+    @Bean
+    public DataSource primaryDataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    public DataSource secondaryDataSource() {
+        return new HikariDataSource();
+    }
+}
+
+@Service
+public class ReportService {
+    @Autowired
+    private DataSource dataSource; // Noncompliant
+}
+
+

Compliant solution

+
+@Configuration
+public class DataSourceConfig {
+    @Bean
+    public DataSource primaryDataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    @Fallback
+    public DataSource secondaryDataSource() { // secondaryDataSource is only used if no other DataSource bean exists
+        return new HikariDataSource();
+    }
+}
+
+@Service
+public class ReportService {
+    @Autowired
+    private DataSource dataSource; // Compliant; will use primaryDataSource
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9352.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9352.json new file mode 100644 index 00000000000..f6ac3ec331c --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9352.json @@ -0,0 +1,25 @@ +{ + "title": "Bean autowiring ambiguity should be resolved using \"@Qualifier\" or \"@Primary\"", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5 min" + }, + "tags": [ + "spring", + "injection", + "configuration" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-9352", + "sqKey": "S9352", + "scope": "All", + "quickfix": "unknown", + "code": { + "impacts": { + "RELIABILITY": "HIGH" + }, + "attribute": "COMPLETE" + } +} 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 31960db62d8..8409023c55d 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 @@ -538,6 +538,7 @@ "S8700", "S8714", "S8715", - "S8745" + "S8745", + "S9352" ] } From 134d12bbb8079e6f37951268106fd566be70fb80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 26 Aug 2026 10:03:36 +0200 Subject: [PATCH 02/22] Create test samples --- .../spring/s9352/FallbackComponent.java | 17 +++++++++++++++++ .../checks/spring/s9352/FallbackConsumer.java | 13 +++++++++++++ .../s9352/FallbackRegularComponent.java | 18 ++++++++++++++++++ .../spring/s9352/BeanFactoryComponentA.java | 19 +++++++++++++++++++ .../spring/s9352/BeanFactoryComponentB.java | 16 ++++++++++++++++ .../spring/s9352/BeanNameComponent.java | 17 +++++++++++++++++ .../checks/spring/s9352/ComponentOne.java | 17 +++++++++++++++++ .../checks/spring/s9352/ComponentTwo.java | 15 +++++++++++++++ .../spring/s9352/EnvironmentComponentA.java | 18 ++++++++++++++++++ .../spring/s9352/EnvironmentComponentB.java | 15 +++++++++++++++ .../spring/s9352/NameMatchConsumer.java | 14 ++++++++++++++ .../checks/spring/s9352/PrimaryComponent.java | 16 ++++++++++++++++ .../checks/spring/s9352/PrimaryConsumer.java | 14 ++++++++++++++ .../spring/s9352/QualifierConsumer.java | 16 ++++++++++++++++ .../spring/s9352/ResourceLoaderComponent.java | 18 ++++++++++++++++++ .../spring/s9352/SingleCandidateConsumer.java | 13 +++++++++++++ .../spring/s9352/UnresolvedConsumer.java | 14 ++++++++++++++ .../java/SpringContextModelSensor.java | 1 + 18 files changed, 271 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackConsumer.java create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanFactoryComponentA.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanFactoryComponentB.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanNameComponent.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/ComponentOne.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/ComponentTwo.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/EnvironmentComponentA.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/EnvironmentComponentB.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/NameMatchConsumer.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/PrimaryComponent.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/PrimaryConsumer.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierConsumer.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/ResourceLoaderComponent.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/SingleCandidateConsumer.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/UnresolvedConsumer.java diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java new file mode 100644 index 00000000000..4987a88839c --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java @@ -0,0 +1,17 @@ +package checks.spring.s9352; + +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; +import org.springframework.context.annotation.Fallback; +import org.springframework.stereotype.Component; + +// See FallbackRegularComponent for context. +@Fallback +@Component +class FallbackComponent implements ApplicationContextAware { + + @Override + public void setApplicationContext(ApplicationContext ctx) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackConsumer.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackConsumer.java new file mode 100644 index 00000000000..e42a2e86717 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackConsumer.java @@ -0,0 +1,13 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationContextAware; +import org.springframework.stereotype.Service; + +// See FallbackRegularComponent for context. +@Service +class FallbackConsumer { + + @Autowired + private ApplicationContextAware contextAware; +} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java new file mode 100644 index 00000000000..735c6cf212a --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java @@ -0,0 +1,18 @@ +package checks.spring.s9352; + +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; +import org.springframework.stereotype.Component; + +// @Fallback (available since Spring 6.2, not present in the spring-context version on this module's classpath, +// hence non-compiling) is not yet handled by AmbiguousDependencyCheck: Spring would resolve this dependency +// unambiguously by ignoring the fallback candidate, but the check does not know that yet. Kept here for when +// @Fallback support is added. See FallbackComponent and FallbackConsumer. +@Component +class FallbackRegularComponent implements ApplicationContextAware { + + @Override + public void setApplicationContext(ApplicationContext ctx) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanFactoryComponentA.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanFactoryComponentA.java new file mode 100644 index 00000000000..7b7c58df1c2 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanFactoryComponentA.java @@ -0,0 +1,19 @@ +package checks.spring.s9352; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.stereotype.Component; + +// Scenario: field name matches a bean name, no issue expected. +// Two candidates of type BeanFactoryAware (this class and BeanFactoryComponentB), used only by +// NameMatchConsumer in this scenario. A distinct interface from the other scenarios in this package, so that a +// whole-module scan does not merge candidate pools across scenarios. +@Component +public class BeanFactoryComponentA implements BeanFactoryAware { + + @Override + public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanFactoryComponentB.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanFactoryComponentB.java new file mode 100644 index 00000000000..2ccbc8aef31 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanFactoryComponentB.java @@ -0,0 +1,16 @@ +package checks.spring.s9352; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.stereotype.Component; + +// See BeanFactoryComponentA for context. +@Component +public class BeanFactoryComponentB implements BeanFactoryAware { + + @Override + public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanNameComponent.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanNameComponent.java new file mode 100644 index 00000000000..218b200f4a6 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/BeanNameComponent.java @@ -0,0 +1,17 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.BeanNameAware; +import org.springframework.stereotype.Component; + +// Scenario: @Primary disambiguates, no issue expected. +// Two candidates of type BeanNameAware (this class and PrimaryComponent), the latter @Primary, used only by +// PrimaryConsumer in this scenario. A distinct interface from the other scenarios in this package, so that a +// whole-module scan does not merge candidate pools across scenarios. +@Component +public class BeanNameComponent implements BeanNameAware { + + @Override + public void setBeanName(String name) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/ComponentOne.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/ComponentOne.java new file mode 100644 index 00000000000..e58d92f15ee --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/ComponentOne.java @@ -0,0 +1,17 @@ +package checks.spring.s9352; + +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; +import org.springframework.stereotype.Component; + +// Scenario: ambiguous dependency, issue expected. +// Two candidates of type ApplicationContextAware (this class and ComponentTwo), neither @Primary, +// used only by UnresolvedConsumer in this scenario. +@Component +public class ComponentOne implements ApplicationContextAware { + + @Override + public void setApplicationContext(ApplicationContext ctx) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/ComponentTwo.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/ComponentTwo.java new file mode 100644 index 00000000000..5c1bbcac47a --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/ComponentTwo.java @@ -0,0 +1,15 @@ +package checks.spring.s9352; + +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; +import org.springframework.stereotype.Component; + +// See ComponentOne for context. +@Component +public class ComponentTwo implements ApplicationContextAware { + + @Override + public void setApplicationContext(ApplicationContext ctx) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/EnvironmentComponentA.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/EnvironmentComponentA.java new file mode 100644 index 00000000000..f50b650a7ff --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/EnvironmentComponentA.java @@ -0,0 +1,18 @@ +package checks.spring.s9352; + +import org.springframework.context.EnvironmentAware; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +// Scenario: @Qualifier disambiguates, no issue expected. +// Two candidates of type EnvironmentAware (this class and EnvironmentComponentB), used only by +// QualifierConsumer in this scenario. A distinct interface from the other scenarios in this package, so that a +// whole-module scan does not merge candidate pools across scenarios. +@Component +public class EnvironmentComponentA implements EnvironmentAware { + + @Override + public void setEnvironment(Environment environment) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/EnvironmentComponentB.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/EnvironmentComponentB.java new file mode 100644 index 00000000000..3ef5bfc63f0 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/EnvironmentComponentB.java @@ -0,0 +1,15 @@ +package checks.spring.s9352; + +import org.springframework.context.EnvironmentAware; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +// See EnvironmentComponentA for context. +@Component +public class EnvironmentComponentB implements EnvironmentAware { + + @Override + public void setEnvironment(Environment environment) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/NameMatchConsumer.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/NameMatchConsumer.java new file mode 100644 index 00000000000..adf245936cb --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/NameMatchConsumer.java @@ -0,0 +1,14 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +// Two beans of type BeanFactoryAware exist, and this field's name matches the bean name "beanFactoryComponentA" +// exactly: Spring resolves by name, no issue expected. +@Service +public class NameMatchConsumer { + + @Autowired + private BeanFactoryAware beanFactoryComponentA; +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/PrimaryComponent.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/PrimaryComponent.java new file mode 100644 index 00000000000..11c1e800041 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/PrimaryComponent.java @@ -0,0 +1,16 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.BeanNameAware; +import org.springframework.context.annotation.Primary; +import org.springframework.stereotype.Component; + +// See BeanNameComponent for context. +@Primary +@Component +public class PrimaryComponent implements BeanNameAware { + + @Override + public void setBeanName(String name) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/PrimaryConsumer.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/PrimaryConsumer.java new file mode 100644 index 00000000000..c90ad321685 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/PrimaryConsumer.java @@ -0,0 +1,14 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.BeanNameAware; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +// Two beans of type BeanNameAware exist, one of them (PrimaryComponent) is @Primary: disambiguated, no issue +// expected. +@Service +public class PrimaryConsumer { + + @Autowired + private BeanNameAware contextAware; +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierConsumer.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierConsumer.java new file mode 100644 index 00000000000..70c80146e62 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierConsumer.java @@ -0,0 +1,16 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.EnvironmentAware; +import org.springframework.stereotype.Service; + +// Two beans of type EnvironmentAware exist, and @Qualifier matches the bean name "environmentComponentB" exactly: +// disambiguated, no issue expected. +@Service +public class QualifierConsumer { + + @Autowired + @Qualifier("environmentComponentB") + private EnvironmentAware contextAware; +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/ResourceLoaderComponent.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/ResourceLoaderComponent.java new file mode 100644 index 00000000000..3b27a5363dc --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/ResourceLoaderComponent.java @@ -0,0 +1,18 @@ +package checks.spring.s9352; + +import org.springframework.context.ResourceLoaderAware; +import org.springframework.core.io.ResourceLoader; +import org.springframework.stereotype.Component; + +// Scenario: only one candidate exists, no issue expected. +// The only bean of type ResourceLoaderAware in this package, used only by SingleCandidateConsumer. A distinct +// interface from the other scenarios in this package, so that a whole-module scan does not merge candidate pools +// across scenarios. +@Component +public class ResourceLoaderComponent implements ResourceLoaderAware { + + @Override + public void setResourceLoader(ResourceLoader resourceLoader) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/SingleCandidateConsumer.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/SingleCandidateConsumer.java new file mode 100644 index 00000000000..a4f4ae1ffbd --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/SingleCandidateConsumer.java @@ -0,0 +1,13 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ResourceLoaderAware; +import org.springframework.stereotype.Service; + +// Only one bean of type ResourceLoaderAware is registered in this scenario: no ambiguity, no issue expected. +@Service +public class SingleCandidateConsumer { + + @Autowired + private ResourceLoaderAware contextAware; +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/UnresolvedConsumer.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/UnresolvedConsumer.java new file mode 100644 index 00000000000..1ed96a7f27b --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/UnresolvedConsumer.java @@ -0,0 +1,14 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationContextAware; +import org.springframework.stereotype.Service; + +// Two beans of type ApplicationContextAware exist (ComponentOne, ComponentTwo), neither is @Primary, and this +// field's name matches neither bean name: ambiguous, issue expected. +@Service +public class UnresolvedConsumer { + + @Autowired + private ApplicationContextAware contextAware; +} diff --git a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java b/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java index 52a2f5dcc06..65e8645b193 100644 --- a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java +++ b/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java @@ -52,3 +52,4 @@ public void execute(SensorContext context) { // Nothing to do for now } } + From 048473ea595d35e59bfc57abac5e88a40a315182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 26 Aug 2026 14:21:20 +0200 Subject: [PATCH 03/22] Implement check --- .../spring/AmbiguousDependencyCheck.java | 82 ++++++++++++ .../spring/AmbiguousDependencyCheckTest.java | 126 ++++++++++++++++++ .../springcontext/BeanDefinitionRegistry.java | 9 ++ .../java/SpringContextModelSensor.java | 22 ++- .../java/SpringContextModelSensorTest.java | 98 ++++++++++++++ 5 files changed, 336 insertions(+), 1 deletion(-) create mode 100644 java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java new file mode 100644 index 00000000000..b659a648c7b --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -0,0 +1,82 @@ +/* + * 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.spring; + +import java.util.ArrayList; +import java.util.Map; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import org.sonar.check.Rule; +import org.sonar.java.model.springcontext.BeanDefinitionHolder; +import org.sonar.java.model.springcontext.BeanDefinitionRegistry; +import org.sonar.java.model.springcontext.BeanLocation; +import org.sonar.java.model.springcontext.SpringContextModel; +import org.sonar.java.model.springcontext.TypeToBeanNamesIndex; +import org.sonar.plugins.java.api.JavaCheck; + +/** + * Not an AST visitor: called directly by {@code SpringContextModelSensor} once the {@link SpringContextModel} + * has been fully populated by the gatherers, since detecting autowiring ambiguity requires reasoning about every + * bean of a given type across the whole analyzed scope, not a single file. + */ +@Rule(key = "S9352") +public class AmbiguousDependencyCheck implements JavaCheck { + + /** + * @param location bean whose dependency is ambiguous, used to anchor the reported issue + * @param message issue message describing the ambiguity + */ + public record AmbiguousDependency(BeanLocation location, String message) { + } + + private static final String MESSAGE = "Multiple beans of type \"%s\" match this dependency (%s);" + + " disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\"."; + + public List findAmbiguousDependencies(SpringContextModel model) { + BeanDefinitionRegistry registry = model.getBeanDefinitionRegistry(); + TypeToBeanNamesIndex typeToBeanNamesIndex = model.getTypeToBeanNamesIndex(); + + List ambiguousDependencies = new ArrayList<>(); + for (BeanDefinitionHolder bean : registry.getAll()) { + for (Map.Entry> dependency : bean.getDependingBeans().entrySet()) { + String requiredType = dependency.getKey(); + Set candidates = typeToBeanNamesIndex.getNamesForType(requiredType); + if (isAmbiguous(candidates, dependency.getValue(), registry)) { + ambiguousDependencies.add(new AmbiguousDependency(bean.getLocation(), message(requiredType, candidates))); + } + } + } + return ambiguousDependencies; + } + + private static boolean isAmbiguous(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { + return candidates.size() > 1 + && candidates.stream().noneMatch(candidate -> isPrimary(registry, candidate)) + && candidates.stream().noneMatch(injectionPointNames::contains); + } + + private static boolean isPrimary(BeanDefinitionRegistry registry, String beanName) { + return registry.getByName(beanName).stream().anyMatch(BeanDefinitionHolder::isPrimary); + } + + private static String message(String requiredType, Set candidates) { + String sortedCandidates = candidates.stream().sorted().collect(Collectors.joining(", ")); + return String.format(MESSAGE, requiredType, sortedCandidates); + } + +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java new file mode 100644 index 00000000000..807ec3c4ad0 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java @@ -0,0 +1,126 @@ +/* + * 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.spring; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.sonar.api.batch.fs.InputFile; +import org.sonar.api.batch.fs.internal.TestInputFileBuilder; +import org.sonar.api.batch.sensor.internal.SensorContextTester; +import org.sonar.java.SonarComponents; +import org.sonar.java.checks.verifier.TestUtils; +import org.sonar.java.model.JParser; +import org.sonar.java.model.JParserConfig; +import org.sonar.java.model.VisitorsBridge; +import org.sonar.java.model.springcontext.BeanDefinitionGatherer; +import org.sonar.java.model.springcontext.SpringContextModel; +import org.sonar.java.test.classpath.TestClasspathUtils; +import org.sonar.plugins.java.api.JavaCheck; +import org.sonar.plugins.java.api.JavaVersion; +import org.sonar.plugins.java.api.tree.CompilationUnitTree; + +import static org.assertj.core.api.Assertions.assertThat; + +class AmbiguousDependencyCheckTest { + + private static final String BASE_PATH = "checks/spring/s9352/"; + + private final AmbiguousDependencyCheck check = new AmbiguousDependencyCheck(); + + @Test + void ambiguous_dependency_with_no_disambiguation_raises_issue() { + SpringContextModel model = buildModel("ComponentOne.java", "ComponentTwo.java", "UnresolvedConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).hasSize(1); + } + + @Test + void primary_candidate_resolves_ambiguity() { + SpringContextModel model = buildModel("BeanNameComponent.java", "PrimaryComponent.java", "PrimaryConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + } + + @Test + void field_name_matching_bean_name_resolves_ambiguity() { + SpringContextModel model = buildModel("BeanFactoryComponentA.java", "BeanFactoryComponentB.java", "NameMatchConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + } + + @Test + void qualifier_resolves_ambiguity() { + SpringContextModel model = buildModel("EnvironmentComponentA.java", "EnvironmentComponentB.java", "QualifierConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + } + + @Test + void single_candidate_does_not_raise_issue() { + SpringContextModel model = buildModel("ResourceLoaderComponent.java", "SingleCandidateConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + } + + /** + * Runs {@link BeanDefinitionGatherer} over the given files (relative to {@link #BASE_PATH}) into a single, + * freshly built {@link SpringContextModel}, mirroring how {@code JavaSensor} drives gatherers during a real + * analysis, without needing java-frontend's test-only scanning helpers. + */ + private static SpringContextModel buildModel(String... relativeFilePaths) { + List classpath = TestClasspathUtils.DEFAULT_MODULE.getClassPath(); + SonarComponents sonarComponents = new SonarComponents(null, null, null, null, null, null); + sonarComponents.setSensorContext(SensorContextTester.create(new File(""))); + SpringContextModel model = new SpringContextModel(); + sonarComponents.setSpringContextModel(model); + + BeanDefinitionGatherer gatherer = new BeanDefinitionGatherer(); + VisitorsBridge visitorsBridge = new VisitorsBridge(List.of((JavaCheck) gatherer), classpath, sonarComponents); + for (String relativeFilePath : relativeFilePaths) { + File file = new File(TestUtils.mainCodeSourcesPath(BASE_PATH + relativeFilePath)); + CompilationUnitTree compilationUnit = parse(file, classpath); + visitorsBridge.setCurrentFile(inputFile(file)); + visitorsBridge.visitFile(compilationUnit, false); + } + visitorsBridge.endOfAnalysis(); + return model; + } + + private static InputFile inputFile(File file) { + try { + return new TestInputFileBuilder("", file.getParentFile(), file) + .setContents(Files.readString(file.toPath(), StandardCharsets.UTF_8)) + .setCharset(StandardCharsets.UTF_8) + .setLanguage("java") + .setType(InputFile.Type.MAIN) + .build(); + } catch (IOException e) { + throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e); + } + } + + private static CompilationUnitTree parse(File file, List classpath) { + String source; + try { + source = Files.readString(file.toPath(), StandardCharsets.UTF_8); + } catch (Exception e) { + throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e); + } + JavaVersion version = JParserConfig.MAXIMUM_SUPPORTED_JAVA_VERSION; + return JParser.parse(JParserConfig.Mode.FILE_BY_FILE.create(version, classpath).astParser(), version.toString(), file.getName(), source); + } + +} diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionRegistry.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionRegistry.java index 8de01d51202..bde14b4df36 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionRegistry.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionRegistry.java @@ -42,6 +42,15 @@ public List getByName(String beanName) { return beanDefinitions.getOrDefault(beanName, List.of()); } + /** + * Returns every bean definition registered so far, regardless of the name it is registered under. + */ + public List getAll() { + return beanDefinitions.values().stream() + .flatMap(List::stream) + .toList(); + } + public void addBeanDefinition(String beanName, BeanDefinitionHolder beanDefinition) { beanDefinitions.computeIfAbsent(beanName, k -> new ArrayList<>()).add(beanDefinition); } diff --git a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java b/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java index 65e8645b193..1e9e6dddb6d 100644 --- a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java +++ b/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java @@ -19,9 +19,15 @@ import org.sonar.api.batch.Phase; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.batch.sensor.SensorDescriptor; +import org.sonar.api.batch.sensor.issue.NewIssue; +import org.sonar.api.rule.RuleKey; import org.sonar.api.scanner.sensor.ProjectSensor; +import org.sonar.java.GeneratedCheckList; +import org.sonar.java.checks.spring.AmbiguousDependencyCheck; import org.sonar.java.jsp.Jasper; +import org.sonar.java.model.springcontext.BeanLocation; import org.sonar.java.model.springcontext.SpringContextModel; +import org.sonar.java.reporting.AnalyzerMessage; /** * A post-phase {@link ProjectSensor} that holds the shared {@link SpringContextModel} built during analysis. @@ -49,7 +55,21 @@ public void describe(SensorDescriptor descriptor) { @Override public void execute(SensorContext context) { - // Nothing to do for now + reportAmbiguousDependencies(context); + } + + private void reportAmbiguousDependencies(SensorContext context) { + RuleKey ruleKey = RuleKey.of(GeneratedCheckList.REPOSITORY_KEY, "S9352"); + for (var ambiguousDependency : new AmbiguousDependencyCheck().findAmbiguousDependencies(springContextModel)) { + BeanLocation location = ambiguousDependency.location(); + AnalyzerMessage.TextSpan span = location.mainLocation(); + NewIssue newIssue = context.newIssue().forRule(ruleKey); + newIssue.at(newIssue.newLocation() + .on(location.inputFile()) + .at(location.inputFile().newRange(span.startLine, span.startCharacter, span.endLine, span.endCharacter)) + .message(ambiguousDependency.message())); + newIssue.save(); + } } } diff --git a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java index d0796f6e08c..e23f634f960 100644 --- a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java +++ b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java @@ -16,14 +16,36 @@ */ package org.sonar.plugins.java; +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.List; import org.junit.jupiter.api.Test; +import org.sonar.api.batch.fs.InputFile; +import org.sonar.api.batch.fs.internal.TestInputFileBuilder; import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor; +import org.sonar.api.batch.sensor.internal.SensorContextTester; +import org.sonar.api.batch.sensor.issue.Issue; +import org.sonar.api.rule.RuleKey; +import org.sonar.java.SonarComponents; +import org.sonar.java.checks.verifier.TestUtils; +import org.sonar.java.model.JParser; +import org.sonar.java.model.JParserConfig; +import org.sonar.java.model.VisitorsBridge; +import org.sonar.java.model.springcontext.BeanDefinitionGatherer; import org.sonar.java.model.springcontext.SpringContextModel; +import org.sonar.java.test.classpath.TestClasspathUtils; +import org.sonar.plugins.java.api.JavaCheck; +import org.sonar.plugins.java.api.JavaVersion; +import org.sonar.plugins.java.api.tree.CompilationUnitTree; import static org.assertj.core.api.Assertions.assertThat; class SpringContextModelSensorTest { + private static final String BASE_PATH = "checks/spring/s9352/"; + @Test void test_toString() { DefaultSensorDescriptor descriptor = new DefaultSensorDescriptor(); @@ -33,4 +55,80 @@ void test_toString() { assertThat(descriptor.languages()).containsExactly("java", "jsp"); } + @Test + void reports_an_issue_for_an_ambiguous_dependency() { + SensorContextTester context = SensorContextTester.create(new File("")); + SpringContextModel model = buildModel(context, "ComponentOne.java", "ComponentTwo.java", "UnresolvedConsumer.java"); + + new SpringContextModelSensor(model).execute(context); + + assertThat(context.allIssues()).hasSize(1); + Issue issue = context.allIssues().iterator().next(); + assertThat(issue.ruleKey()).isEqualTo(RuleKey.of("java", "S9352")); + assertThat(issue.primaryLocation().message()) + .isEqualTo("Multiple beans of type \"org.springframework.context.ApplicationContextAware\" match this dependency" + + " (componentOne, componentTwo); disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\"."); + assertThat(issue.primaryLocation().textRange().start().line()).isEqualTo(10); + } + + @Test + void reports_no_issue_when_only_one_candidate_exists() { + SensorContextTester context = SensorContextTester.create(new File("")); + SpringContextModel model = buildModel(context, "ResourceLoaderComponent.java", "SingleCandidateConsumer.java"); + + new SpringContextModelSensor(model).execute(context); + + assertThat(context.allIssues()).isEmpty(); + } + + /** + * Runs {@link BeanDefinitionGatherer} over the given files (relative to {@link #BASE_PATH}) into a single, + * freshly built {@link SpringContextModel}, registering each file's {@link InputFile} on the given + * {@link SensorContextTester} so that issues reported against it can be resolved. + */ + private static SpringContextModel buildModel(SensorContextTester context, String... relativeFilePaths) { + List classpath = TestClasspathUtils.DEFAULT_MODULE.getClassPath(); + SonarComponents sonarComponents = new SonarComponents(null, null, null, null, null, null); + sonarComponents.setSensorContext(context); + SpringContextModel model = new SpringContextModel(); + sonarComponents.setSpringContextModel(model); + + BeanDefinitionGatherer gatherer = new BeanDefinitionGatherer(); + VisitorsBridge visitorsBridge = new VisitorsBridge(List.of((JavaCheck) gatherer), classpath, sonarComponents); + for (String relativeFilePath : relativeFilePaths) { + File file = new File(TestUtils.mainCodeSourcesPath(BASE_PATH + relativeFilePath)); + CompilationUnitTree compilationUnit = parse(file, classpath); + InputFile inputFile = inputFile(file); + context.fileSystem().add(inputFile); + visitorsBridge.setCurrentFile(inputFile); + visitorsBridge.visitFile(compilationUnit, false); + } + visitorsBridge.endOfAnalysis(); + return model; + } + + private static InputFile inputFile(File file) { + try { + return new TestInputFileBuilder("", file.getParentFile(), file) + .setContents(Files.readString(file.toPath(), StandardCharsets.UTF_8)) + .setCharset(StandardCharsets.UTF_8) + .setLanguage("java") + .setType(InputFile.Type.MAIN) + .build(); + } catch (IOException e) { + throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e); + } + } + + private static CompilationUnitTree parse(File file, List classpath) { + String source; + try { + source = Files.readString(file.toPath(), StandardCharsets.UTF_8); + } catch (Exception e) { + throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e); + } + JavaVersion version = JParserConfig.MAXIMUM_SUPPORTED_JAVA_VERSION; + return JParser.parse(JParserConfig.Mode.FILE_BY_FILE.create(version, classpath).astParser(), version.toString(), file.getName(), source); + } + } From 60170612ce2cd2353d2cbe693643b2899b40bf31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 26 Aug 2026 15:58:05 +0200 Subject: [PATCH 04/22] Add support for `@Fallback` --- .../spring/s9352/FallbackComponent.java | 3 +- .../FallbackTwoCandidatesComponentA.java | 22 ++++++++++ .../FallbackTwoCandidatesComponentB.java | 15 +++++++ .../s9352/FallbackTwoCandidatesConsumer.java | 14 ++++++ ...allbackTwoCandidatesFallbackComponent.java | 16 +++++++ .../spring/AmbiguousDependencyCheck.java | 29 ++++++++++--- .../spring/AmbiguousDependencyCheckTest.java | 43 ++++++++++++++++--- .../springcontext/BeanDefinitionGatherer.java | 22 +++++++--- .../springcontext/BeanDefinitionHolder.java | 20 +++++++++ .../BeanDefinitionGathererTest.java | 6 +-- 10 files changed, 168 insertions(+), 22 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentB.java create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesFallbackComponent.java diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java index 4987a88839c..ccf445acb86 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java @@ -2,11 +2,10 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.context.annotation.Fallback; import org.springframework.stereotype.Component; // See FallbackRegularComponent for context. -@Fallback +@org.springframework.context.annotation.Fallback @Component class FallbackComponent implements ApplicationContextAware { diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java new file mode 100644 index 00000000000..e3d87f30ce6 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java @@ -0,0 +1,22 @@ +package checks.spring.s9352; + +import org.springframework.context.MessageSource; +import org.springframework.context.MessageSourceAware; +import org.springframework.stereotype.Component; + +// Scenario: @Fallback does not resolve ambiguity when at least two other (non-fallback) candidates remain. +// Three candidates of type MessageSourceAware exist: this class, FallbackTwoCandidatesComponentB (both regular), +// and FallbackTwoCandidatesFallbackComponent (@Fallback). Spring ignores the fallback candidate only when a +// single non-fallback candidate remains; here two non-fallback candidates still compete, so the dependency +// remains ambiguous. Not yet handled by AmbiguousDependencyCheck (see FallbackRegularComponent for context on +// why @Fallback support is pending), so this currently (incorrectly) raises no issue; it should once handled. +// A distinct interface from the other scenarios in this package, so that a whole-module scan does not merge +// candidate pools across scenarios. +@Component +public class FallbackTwoCandidatesComponentA implements MessageSourceAware { + + @Override + public void setMessageSource(MessageSource messageSource) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentB.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentB.java new file mode 100644 index 00000000000..0523499ca04 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentB.java @@ -0,0 +1,15 @@ +package checks.spring.s9352; + +import org.springframework.context.MessageSource; +import org.springframework.context.MessageSourceAware; +import org.springframework.stereotype.Component; + +// See FallbackTwoCandidatesComponentA for context. +@Component +public class FallbackTwoCandidatesComponentB implements MessageSourceAware { + + @Override + public void setMessageSource(MessageSource messageSource) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java new file mode 100644 index 00000000000..9dea9a1125e --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java @@ -0,0 +1,14 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.MessageSourceAware; +import org.springframework.stereotype.Service; + +// See FallbackTwoCandidatesComponentA for context: still ambiguous between the two non-fallback candidates, +// issue expected once @Fallback support is added. +@Service +public class FallbackTwoCandidatesConsumer { + + @Autowired + private MessageSourceAware contextAware; +} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesFallbackComponent.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesFallbackComponent.java new file mode 100644 index 00000000000..9a0c5f965a4 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesFallbackComponent.java @@ -0,0 +1,16 @@ +package checks.spring.s9352; + +import org.springframework.context.MessageSource; +import org.springframework.context.MessageSourceAware; +import org.springframework.stereotype.Component; + +// See FallbackTwoCandidatesComponentA for context. +@org.springframework.context.annotation.Fallback +@Component +public class FallbackTwoCandidatesFallbackComponent implements MessageSourceAware { + + @Override + public void setMessageSource(MessageSource messageSource) { + // not needed for test + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index b659a648c7b..b377fdda636 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -56,24 +56,41 @@ public List findAmbiguousDependencies(SpringContextModel mo for (Map.Entry> dependency : bean.getDependingBeans().entrySet()) { String requiredType = dependency.getKey(); Set candidates = typeToBeanNamesIndex.getNamesForType(requiredType); - if (isAmbiguous(candidates, dependency.getValue(), registry)) { - ambiguousDependencies.add(new AmbiguousDependency(bean.getLocation(), message(requiredType, candidates))); + if (isResolved(candidates, dependency.getValue(), registry)) { + continue; + } + // A @Fallback candidate is only a real contender when it is the sole remaining one; otherwise it is + // ignored by Spring, so the effective candidates are whichever bean(s) are not marked @Fallback. + Set effectiveCandidates = excludeFallbackCandidates(candidates, registry); + if (effectiveCandidates.size() > 1) { + ambiguousDependencies.add(new AmbiguousDependency(bean.getLocation(), message(requiredType, effectiveCandidates))); } } } return ambiguousDependencies; } - private static boolean isAmbiguous(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { - return candidates.size() > 1 - && candidates.stream().noneMatch(candidate -> isPrimary(registry, candidate)) - && candidates.stream().noneMatch(injectionPointNames::contains); + private static boolean isResolved(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { + return candidates.size() <= 1 + || candidates.stream().anyMatch(injectionPointNames::contains) + || candidates.stream().anyMatch(candidate -> isPrimary(registry, candidate)); + } + + private static Set excludeFallbackCandidates(Set candidates, BeanDefinitionRegistry registry) { + Set nonFallbackCandidates = candidates.stream() + .filter(candidate -> !isFallback(registry, candidate)) + .collect(Collectors.toUnmodifiableSet()); + return nonFallbackCandidates.isEmpty() ? candidates : nonFallbackCandidates; } private static boolean isPrimary(BeanDefinitionRegistry registry, String beanName) { return registry.getByName(beanName).stream().anyMatch(BeanDefinitionHolder::isPrimary); } + private static boolean isFallback(BeanDefinitionRegistry registry, String beanName) { + return registry.getByName(beanName).stream().anyMatch(BeanDefinitionHolder::isFallback); + } + private static String message(String requiredType, Set candidates) { String sortedCandidates = candidates.stream().sorted().collect(Collectors.joining(", ")); return String.format(MESSAGE, requiredType, sortedCandidates); diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java index 807ec3c4ad0..e90f66f5234 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.util.Arrays; import java.util.List; import org.junit.jupiter.api.Test; import org.sonar.api.batch.fs.InputFile; @@ -75,12 +76,44 @@ void single_candidate_does_not_raise_issue() { assertThat(check.findAmbiguousDependencies(model)).isEmpty(); } + @Test + void fallback_candidate_resolves_ambiguity_when_it_is_the_sole_remaining_candidate() { + SpringContextModel model = buildModelFromNonCompilingSources( + "FallbackRegularComponent.java", "FallbackComponent.java", "FallbackConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + } + + @Test + void fallback_candidate_does_not_resolve_ambiguity_with_two_other_candidates() { + SpringContextModel model = buildModelFromNonCompilingSources( + "FallbackTwoCandidatesComponentA.java", "FallbackTwoCandidatesComponentB.java", + "FallbackTwoCandidatesFallbackComponent.java", "FallbackTwoCandidatesConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).hasSize(1); + } + /** - * Runs {@link BeanDefinitionGatherer} over the given files (relative to {@link #BASE_PATH}) into a single, - * freshly built {@link SpringContextModel}, mirroring how {@code JavaSensor} drives gatherers during a real - * analysis, without needing java-frontend's test-only scanning helpers. + * Runs {@link BeanDefinitionGatherer} over the given files (relative to {@link #BASE_PATH} under + * {@code src/main/java}) into a single, freshly built {@link SpringContextModel}, mirroring how + * {@code JavaSensor} drives gatherers during a real analysis, without needing java-frontend's test-only + * scanning helpers. */ private static SpringContextModel buildModel(String... relativeFilePaths) { + return buildModel(Arrays.stream(relativeFilePaths) + .map(relativeFilePath -> TestUtils.mainCodeSourcesPath(BASE_PATH + relativeFilePath)) + .toList()); + } + + /** + * Same as {@link #buildModel(String...)}, but resolving files under {@code src/main/files/non-compiling} + * instead, for fixtures relying on annotations not present on this module's classpath. + */ + private static SpringContextModel buildModelFromNonCompilingSources(String... relativeFilePaths) { + return buildModel(Arrays.stream(relativeFilePaths) + .map(relativeFilePath -> TestUtils.nonCompilingTestSourcesPath(BASE_PATH + relativeFilePath)) + .toList()); + } + + private static SpringContextModel buildModel(List filePaths) { List classpath = TestClasspathUtils.DEFAULT_MODULE.getClassPath(); SonarComponents sonarComponents = new SonarComponents(null, null, null, null, null, null); sonarComponents.setSensorContext(SensorContextTester.create(new File(""))); @@ -89,8 +122,8 @@ private static SpringContextModel buildModel(String... relativeFilePaths) { BeanDefinitionGatherer gatherer = new BeanDefinitionGatherer(); VisitorsBridge visitorsBridge = new VisitorsBridge(List.of((JavaCheck) gatherer), classpath, sonarComponents); - for (String relativeFilePath : relativeFilePaths) { - File file = new File(TestUtils.mainCodeSourcesPath(BASE_PATH + relativeFilePath)); + for (String filePath : filePaths) { + File file = new File(filePath); CompilationUnitTree compilationUnit = parse(file, classpath); visitorsBridge.setCurrentFile(inputFile(file)); visitorsBridge.visitFile(compilationUnit, false); diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java index c0f7f4a3768..0c3a47e3a8a 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java @@ -61,6 +61,7 @@ *

Also captures: *

    *
  • {@code @Primary} designation
    • + *
  • {@code @Fallback} designation
    • *
  • Dependencies via {@code @Autowired} fields, constructors, and setters for class-level beans
    • *
  • Dependencies via method parameters for {@code @Bean} method beans
    • *
  • Implicit single-constructor injection (no {@code @Autowired} required)
    • @@ -82,6 +83,7 @@ public class BeanDefinitionGatherer extends SpringContextModelGatherer { private static final String TYPE_HIERARCHY_SEPARATOR = ";"; private static final String PRIMARY_ANNOTATION = "org.springframework.context.annotation.Primary"; + private static final String FALLBACK_ANNOTATION = "org.springframework.context.annotation.Fallback"; private static final String VALUE_ATTRIBUTE = "value"; private final List collectedBeans = new ArrayList<>(); @@ -96,6 +98,7 @@ private record BeanData( InputFile inputFile, AnalyzerMessage.TextSpan textSpan, boolean isPrimary, + boolean isFallback, Map> dependingBeans, Set typeHierarchy) { } @@ -132,6 +135,7 @@ public void visitNode(Tree tree) { context.getInputFile(), AnalyzerMessage.textSpanFor(classTree.simpleName()), meta.isAnnotatedWith(PRIMARY_ANNOTATION), + meta.isAnnotatedWith(FALLBACK_ANNOTATION), deps, typeHierarchy); collectedBeans.add(beanData); @@ -186,6 +190,7 @@ private static String serializeBean(BeanData bean) { bean.beanPackage(), span.startLine + ":" + span.startCharacter + ":" + span.endLine + ":" + span.endCharacter, Boolean.toString(bean.isPrimary()), + Boolean.toString(bean.isFallback()), deps, typeHierarchy); } @@ -200,6 +205,9 @@ public void gatherSpringContextData(ModuleScannerContext context, SpringContextM if (data.isPrimary()) { holderBuilder.primary(); } + if (data.isFallback()) { + holderBuilder.fallback(); + } springContextModel.getBeanDefinitionRegistry() .addBeanDefinition(data.beanName(), holderBuilder.build()); for (String typeFqn : data.typeHierarchy()) { @@ -251,9 +259,10 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { Integer.parseInt(spanParts[2]), Integer.parseInt(spanParts[3])); boolean isPrimary = Boolean.parseBoolean(fields[4]); + boolean isFallback = Boolean.parseBoolean(fields[5]); Map> deps = new LinkedHashMap<>(); - if (!fields[5].isEmpty()) { - for (String entry : fields[5].split(DEP_SEPARATOR)) { + if (!fields[6].isEmpty()) { + for (String entry : fields[6].split(DEP_SEPARATOR)) { int idx = entry.indexOf(DEP_KEY_VALUE_SEPARATOR); String typeFqn = new String(Base64.getDecoder().decode(entry.substring(0, idx)), StandardCharsets.UTF_8); Set names = Arrays.stream(entry.substring(idx + 1).split(DEP_NAMES_SEPARATOR)) @@ -262,10 +271,10 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { deps.put(typeFqn, names); } } - Set typeHierarchy = !fields[6].isEmpty() - ? new LinkedHashSet<>(List.of(fields[6].split(TYPE_HIERARCHY_SEPARATOR))) + Set typeHierarchy = !fields[7].isEmpty() + ? new LinkedHashSet<>(List.of(fields[7].split(TYPE_HIERARCHY_SEPARATOR))) : new LinkedHashSet<>(); - return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, deps, typeHierarchy); + return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, isFallback, deps, typeHierarchy); } private static Optional extractBeanName(SymbolMetadata meta) { @@ -316,11 +325,12 @@ private void collectBeanMethod(MethodTree method, String pkg) { Map> paramDeps = parameterDependencies(method); boolean isPrimary = beanMeta.isAnnotatedWith(PRIMARY_ANNOTATION); + boolean isFallback = beanMeta.isAnnotatedWith(FALLBACK_ANNOTATION); var textSpan = AnalyzerMessage.textSpanFor(method.simpleName()); var inputFile = context.getInputFile(); for (String beanName : beanNames) { - var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, paramDeps, typeHierarchy); + var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, isFallback, paramDeps, typeHierarchy); collectedBeans.add(beanData); beansCollectedAtFileLevel.add(beanData); } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java index 212aab4f0dd..bc3fd66f0eb 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java @@ -66,6 +66,9 @@ public class BeanDefinitionHolder { /** Whether the bean is marked as {@code @Primary}, making it the preferred candidate for autowiring. */ private boolean isPrimary = false; + /** Whether the bean is marked as {@code @Fallback}, making it a last-resort candidate for autowiring. */ + private boolean isFallback = false; + private BeanDefinitionHolder(String type, String module, String beanPackage, BeanLocation location) { this.type = type; this.module = module; @@ -85,6 +88,10 @@ private void setPrimary() { this.isPrimary = true; } + private void setFallback() { + this.isFallback = true; + } + public String getType() { return type; } @@ -114,6 +121,10 @@ public boolean isPrimary() { return isPrimary; } + public boolean isFallback() { + return isFallback; + } + public static class Builder { private final String type; private final String module; @@ -123,6 +134,7 @@ public static class Builder { @Nullable private String profiles; private boolean isPrimary = false; + private boolean isFallback = false; public Builder(String type, String module, String beanPackage, BeanLocation location) { this.type = type; @@ -146,6 +158,11 @@ public Builder primary() { return this; } + public Builder fallback() { + this.isFallback = true; + return this; + } + public BeanDefinitionHolder build() { BeanDefinitionHolder holder = new BeanDefinitionHolder(type, module, beanPackage, location); holder.setDependingBeans(dependingBeans.entrySet().stream() @@ -154,6 +171,9 @@ public BeanDefinitionHolder build() { if (isPrimary) { holder.setPrimary(); } + if (isFallback) { + holder.setFallback(); + } return holder; } } diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index 4e02bbcf04c..b3e91be5be4 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -342,7 +342,7 @@ void scanWithoutParsing_returns_true_and_restores_beans_on_cache_hit() { InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/SimpleComponent.java")); String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); String encodedName = Base64.getEncoder().encodeToString("simpleComponent".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.SimpleComponent|checks.spring.context|6:6:6:21|false||checks.spring.context.SimpleComponent"; + String serialized = encodedName + "|checks.spring.context.SimpleComponent|checks.spring.context|6:6:6:21|false|false||checks.spring.context.SimpleComponent"; JavaReadCache readCache = mock(JavaReadCache.class); when(readCache.readBytes(cacheKey)).thenReturn(serialized.getBytes(StandardCharsets.UTF_8)); @@ -462,7 +462,7 @@ void scanWithoutParsing_restores_dependencies_with_and_without_qualifier_from_ca String encodedEnvType = Base64.getEncoder().encodeToString("org.springframework.core.env.Environment".getBytes(StandardCharsets.UTF_8)); String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|" + String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|false|" + encodedAppContext + ":" + encodedPrimaryContext + "," + encodedEnvType + ":" + encodedEnvironment + "|checks.spring.context.QualifiedFieldDependencies"; @@ -536,7 +536,7 @@ void scanWithoutParsing_restores_full_type_hierarchy_from_cache() { InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/ComponentImplementingInterface.java")); String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); String encodedName = Base64.getEncoder().encodeToString("componentImplementingInterface".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.ComponentImplementingInterface|checks.spring.context|8:6:8:36|false|" + String serialized = encodedName + "|checks.spring.context.ComponentImplementingInterface|checks.spring.context|8:6:8:36|false|false|" + "|checks.spring.context.ComponentImplementingInterface" + ";org.springframework.context.ApplicationContextAware" + ";org.springframework.beans.factory.Aware"; From a9b173b38c541496848e85e5768a7c9d0015ec9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 26 Aug 2026 16:44:31 +0200 Subject: [PATCH 05/22] Fix bug when one resolved injection point hides another ambiguous one --- .../s9352/MixedInjectionComponentA.java | 19 +++++++++++++++++++ .../s9352/MixedInjectionComponentB.java | 15 +++++++++++++++ .../spring/s9352/MixedInjectionConsumer.java | 19 +++++++++++++++++++ .../spring/AmbiguousDependencyCheck.java | 4 +++- .../spring/AmbiguousDependencyCheckTest.java | 6 ++++++ 5 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionComponentA.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionComponentB.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionConsumer.java diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionComponentA.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionComponentA.java new file mode 100644 index 00000000000..8274bddcf5c --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionComponentA.java @@ -0,0 +1,19 @@ +package checks.spring.s9352; + +import org.springframework.context.EmbeddedValueResolverAware; +import org.springframework.stereotype.Component; +import org.springframework.util.StringValueResolver; + +// Scenario: one injection point resolves this type by qualifier, but a second, unrelated injection point of the +// same type on the same bean does not: the dependency remains ambiguous for that second point, issue expected. +// Two candidates of type EmbeddedValueResolverAware (this class and MixedInjectionComponentB), used only by +// MixedInjectionConsumer in this scenario. A distinct interface from the other scenarios in this package, so +// that a whole-module scan does not merge candidate pools across scenarios. +@Component +public class MixedInjectionComponentA implements EmbeddedValueResolverAware { + + @Override + public void setEmbeddedValueResolver(StringValueResolver resolver) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionComponentB.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionComponentB.java new file mode 100644 index 00000000000..b85215d1858 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionComponentB.java @@ -0,0 +1,15 @@ +package checks.spring.s9352; + +import org.springframework.context.EmbeddedValueResolverAware; +import org.springframework.stereotype.Component; +import org.springframework.util.StringValueResolver; + +// See MixedInjectionComponentA for context. +@Component +public class MixedInjectionComponentB implements EmbeddedValueResolverAware { + + @Override + public void setEmbeddedValueResolver(StringValueResolver resolver) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionConsumer.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionConsumer.java new file mode 100644 index 00000000000..d2c2e07aaad --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/MixedInjectionConsumer.java @@ -0,0 +1,19 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.EmbeddedValueResolverAware; +import org.springframework.stereotype.Service; + +// See MixedInjectionComponentA for context: "resolved" is fine, but "unresolved" is genuinely ambiguous and +// must still be reported, even though both injection points share the same required type. +@Service +public class MixedInjectionConsumer { + + @Autowired + @Qualifier("mixedInjectionComponentA") + private EmbeddedValueResolverAware resolved; + + @Autowired + private EmbeddedValueResolverAware unresolved; +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index b377fdda636..3aff7140729 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -71,8 +71,10 @@ public List findAmbiguousDependencies(SpringContextModel mo } private static boolean isResolved(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { + // injectionPointNames merges every injection point of this type declared on the bean: it is only resolved + // if EVERY one of them names a candidate, otherwise at least one injection point remains ambiguous. return candidates.size() <= 1 - || candidates.stream().anyMatch(injectionPointNames::contains) + || injectionPointNames.stream().allMatch(candidates::contains) || candidates.stream().anyMatch(candidate -> isPrimary(registry, candidate)); } diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java index e90f66f5234..da26a0bde49 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java @@ -76,6 +76,12 @@ void single_candidate_does_not_raise_issue() { assertThat(check.findAmbiguousDependencies(model)).isEmpty(); } + @Test + void one_resolved_injection_point_does_not_hide_another_ambiguous_one_of_the_same_type() { + SpringContextModel model = buildModel("MixedInjectionComponentA.java", "MixedInjectionComponentB.java", "MixedInjectionConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).hasSize(1); + } + @Test void fallback_candidate_resolves_ambiguity_when_it_is_the_sole_remaining_candidate() { SpringContextModel model = buildModelFromNonCompilingSources( From 56b5849894320ad9c7ae9f3f481dba990faf5ca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Thu, 27 Aug 2026 11:37:46 +0200 Subject: [PATCH 06/22] Fix FP when qualifier is defined on the bean itself --- .../s9352/QualifierOnBeanComponentA.java | 19 +++++++++++++++ .../s9352/QualifierOnBeanComponentB.java | 14 +++++++++++ .../spring/s9352/QualifierOnBeanConsumer.java | 16 +++++++++++++ .../spring/AmbiguousDependencyCheck.java | 21 ++++++++++++++-- .../spring/AmbiguousDependencyCheckTest.java | 6 +++++ .../springcontext/BeanDefinitionGatherer.java | 24 ++++++++++++++----- .../springcontext/BeanDefinitionHolder.java | 21 ++++++++++++++++ .../BeanDefinitionGathererTest.java | 6 ++--- 8 files changed, 116 insertions(+), 11 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentA.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentB.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanConsumer.java diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentA.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentA.java new file mode 100644 index 00000000000..13bf9ecbd0c --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentA.java @@ -0,0 +1,19 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.BeanClassLoaderAware; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.stereotype.Component; + +// Scenario: @Qualifier declared on the bean itself (not on an injection point) resolves ambiguity, no issue +// expected. Two candidates of type BeanClassLoaderAware (this class and QualifierOnBeanComponentB), used only +// by QualifierOnBeanConsumer in this scenario. A distinct interface from the other scenarios in this package, +// so that a whole-module scan does not merge candidate pools across scenarios. +@Qualifier("main") +@Component +public class QualifierOnBeanComponentA implements BeanClassLoaderAware { + + @Override + public void setBeanClassLoader(ClassLoader classLoader) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentB.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentB.java new file mode 100644 index 00000000000..72762b310be --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentB.java @@ -0,0 +1,14 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.BeanClassLoaderAware; +import org.springframework.stereotype.Component; + +// See QualifierOnBeanComponentA for context. +@Component +public class QualifierOnBeanComponentB implements BeanClassLoaderAware { + + @Override + public void setBeanClassLoader(ClassLoader classLoader) { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanConsumer.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanConsumer.java new file mode 100644 index 00000000000..538efea5926 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanConsumer.java @@ -0,0 +1,16 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.BeanClassLoaderAware; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.stereotype.Service; + +// "main" matches QualifierOnBeanComponentA's own declared @Qualifier value, not its bean name: still resolved, +// no issue expected. See QualifierOnBeanComponentA for context. +@Service +public class QualifierOnBeanConsumer { + + @Autowired + @Qualifier("main") + private BeanClassLoaderAware contextAware; +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index 3aff7140729..fad0cd9b4b3 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -19,8 +19,10 @@ import java.util.ArrayList; import java.util.Map; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; +import javax.annotation.Nullable; import org.sonar.check.Rule; import org.sonar.java.model.springcontext.BeanDefinitionHolder; import org.sonar.java.model.springcontext.BeanDefinitionRegistry; @@ -72,12 +74,27 @@ public List findAmbiguousDependencies(SpringContextModel mo private static boolean isResolved(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { // injectionPointNames merges every injection point of this type declared on the bean: it is only resolved - // if EVERY one of them names a candidate, otherwise at least one injection point remains ambiguous. + // if EVERY one of them names a candidate (by bean name or by a @Qualifier declared on that candidate bean + // itself), otherwise at least one injection point remains ambiguous. return candidates.size() <= 1 - || injectionPointNames.stream().allMatch(candidates::contains) + || injectionPointNames.stream().allMatch(name -> matchesCandidate(name, candidates, registry)) || candidates.stream().anyMatch(candidate -> isPrimary(registry, candidate)); } + private static boolean matchesCandidate(String injectionPointName, Set candidates, BeanDefinitionRegistry registry) { + return candidates.contains(injectionPointName) + || candidates.stream().anyMatch(candidate -> injectionPointName.equals(qualifierOf(registry, candidate))); + } + + @Nullable + private static String qualifierOf(BeanDefinitionRegistry registry, String beanName) { + return registry.getByName(beanName).stream() + .map(BeanDefinitionHolder::getQualifier) + .filter(Objects::nonNull) + .findFirst() + .orElse(null); + } + private static Set excludeFallbackCandidates(Set candidates, BeanDefinitionRegistry registry) { Set nonFallbackCandidates = candidates.stream() .filter(candidate -> !isFallback(registry, candidate)) diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java index da26a0bde49..9ea22f24ee6 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java @@ -82,6 +82,12 @@ void one_resolved_injection_point_does_not_hide_another_ambiguous_one_of_the_sam assertThat(check.findAmbiguousDependencies(model)).hasSize(1); } + @Test + void qualifier_declared_on_the_bean_itself_resolves_ambiguity() { + SpringContextModel model = buildModel("QualifierOnBeanComponentA.java", "QualifierOnBeanComponentB.java", "QualifierOnBeanConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + } + @Test void fallback_candidate_resolves_ambiguity_when_it_is_the_sole_remaining_candidate() { SpringContextModel model = buildModelFromNonCompilingSources( diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java index 0c3a47e3a8a..95839aa9530 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java @@ -62,6 +62,7 @@ *
      *
    • {@code @Primary} designation
      • *
    • {@code @Fallback} designation
      • + *
    • {@code @Qualifier} value declared on the bean itself (as opposed to on an injection point)
      • *
    • Dependencies via {@code @Autowired} fields, constructors, and setters for class-level beans
      • *
    • Dependencies via method parameters for {@code @Bean} method beans
      • *
    • Implicit single-constructor injection (no {@code @Autowired} required)
      • @@ -99,6 +100,7 @@ private record BeanData( AnalyzerMessage.TextSpan textSpan, boolean isPrimary, boolean isFallback, + @Nullable String qualifier, Map> dependingBeans, Set typeHierarchy) { } @@ -136,6 +138,7 @@ public void visitNode(Tree tree) { AnalyzerMessage.textSpanFor(classTree.simpleName()), meta.isAnnotatedWith(PRIMARY_ANNOTATION), meta.isAnnotatedWith(FALLBACK_ANNOTATION), + extractQualifier(meta), deps, typeHierarchy); collectedBeans.add(beanData); @@ -184,6 +187,9 @@ private static String serializeBean(BeanData bean) { var typeHierarchy = String.join(TYPE_HIERARCHY_SEPARATOR, bean.typeHierarchy()); var span = bean.textSpan(); var encodedName = Base64.getEncoder().encodeToString(bean.beanName().getBytes(StandardCharsets.UTF_8)); + var encodedQualifier = bean.qualifier() != null + ? Base64.getEncoder().encodeToString(bean.qualifier().getBytes(StandardCharsets.UTF_8)) + : ""; return String.join(FIELD_SEPARATOR, encodedName, bean.type(), @@ -191,6 +197,7 @@ private static String serializeBean(BeanData bean) { span.startLine + ":" + span.startCharacter + ":" + span.endLine + ":" + span.endCharacter, Boolean.toString(bean.isPrimary()), Boolean.toString(bean.isFallback()), + encodedQualifier, deps, typeHierarchy); } @@ -208,6 +215,7 @@ public void gatherSpringContextData(ModuleScannerContext context, SpringContextM if (data.isFallback()) { holderBuilder.fallback(); } + holderBuilder.qualifier(data.qualifier()); springContextModel.getBeanDefinitionRegistry() .addBeanDefinition(data.beanName(), holderBuilder.build()); for (String typeFqn : data.typeHierarchy()) { @@ -260,9 +268,12 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { Integer.parseInt(spanParts[3])); boolean isPrimary = Boolean.parseBoolean(fields[4]); boolean isFallback = Boolean.parseBoolean(fields[5]); + String qualifier = !fields[6].isEmpty() + ? new String(Base64.getDecoder().decode(fields[6]), StandardCharsets.UTF_8) + : null; Map> deps = new LinkedHashMap<>(); - if (!fields[6].isEmpty()) { - for (String entry : fields[6].split(DEP_SEPARATOR)) { + if (!fields[7].isEmpty()) { + for (String entry : fields[7].split(DEP_SEPARATOR)) { int idx = entry.indexOf(DEP_KEY_VALUE_SEPARATOR); String typeFqn = new String(Base64.getDecoder().decode(entry.substring(0, idx)), StandardCharsets.UTF_8); Set names = Arrays.stream(entry.substring(idx + 1).split(DEP_NAMES_SEPARATOR)) @@ -271,10 +282,10 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { deps.put(typeFqn, names); } } - Set typeHierarchy = !fields[7].isEmpty() - ? new LinkedHashSet<>(List.of(fields[7].split(TYPE_HIERARCHY_SEPARATOR))) + Set typeHierarchy = !fields[8].isEmpty() + ? new LinkedHashSet<>(List.of(fields[8].split(TYPE_HIERARCHY_SEPARATOR))) : new LinkedHashSet<>(); - return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, isFallback, deps, typeHierarchy); + return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, isFallback, qualifier, deps, typeHierarchy); } private static Optional extractBeanName(SymbolMetadata meta) { @@ -326,11 +337,12 @@ private void collectBeanMethod(MethodTree method, String pkg) { Map> paramDeps = parameterDependencies(method); boolean isPrimary = beanMeta.isAnnotatedWith(PRIMARY_ANNOTATION); boolean isFallback = beanMeta.isAnnotatedWith(FALLBACK_ANNOTATION); + String qualifier = extractQualifier(beanMeta); var textSpan = AnalyzerMessage.textSpanFor(method.simpleName()); var inputFile = context.getInputFile(); for (String beanName : beanNames) { - var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, isFallback, paramDeps, typeHierarchy); + var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, isFallback, qualifier, paramDeps, typeHierarchy); collectedBeans.add(beanData); beansCollectedAtFileLevel.add(beanData); } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java index bc3fd66f0eb..62b467f6f01 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java @@ -69,6 +69,10 @@ public class BeanDefinitionHolder { /** Whether the bean is marked as {@code @Fallback}, making it a last-resort candidate for autowiring. */ private boolean isFallback = false; + /** Value of the {@code @Qualifier} annotation declared on the bean itself, or {@code null} if absent. */ + @Nullable + private String qualifier; + private BeanDefinitionHolder(String type, String module, String beanPackage, BeanLocation location) { this.type = type; this.module = module; @@ -92,6 +96,10 @@ private void setFallback() { this.isFallback = true; } + private void setQualifier(@Nullable String qualifier) { + this.qualifier = qualifier; + } + public String getType() { return type; } @@ -125,6 +133,11 @@ public boolean isFallback() { return isFallback; } + @Nullable + public String getQualifier() { + return qualifier; + } + public static class Builder { private final String type; private final String module; @@ -135,6 +148,8 @@ public static class Builder { private String profiles; private boolean isPrimary = false; private boolean isFallback = false; + @Nullable + private String qualifier; public Builder(String type, String module, String beanPackage, BeanLocation location) { this.type = type; @@ -163,11 +178,17 @@ public Builder fallback() { return this; } + public Builder qualifier(@Nullable String qualifier) { + this.qualifier = qualifier; + return this; + } + public BeanDefinitionHolder build() { BeanDefinitionHolder holder = new BeanDefinitionHolder(type, module, beanPackage, location); holder.setDependingBeans(dependingBeans.entrySet().stream() .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, e -> Set.copyOf(e.getValue())))); holder.setProfiles(profiles); + holder.setQualifier(qualifier); if (isPrimary) { holder.setPrimary(); } diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index b3e91be5be4..0df08f9e93b 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -342,7 +342,7 @@ void scanWithoutParsing_returns_true_and_restores_beans_on_cache_hit() { InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/SimpleComponent.java")); String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); String encodedName = Base64.getEncoder().encodeToString("simpleComponent".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.SimpleComponent|checks.spring.context|6:6:6:21|false|false||checks.spring.context.SimpleComponent"; + String serialized = encodedName + "|checks.spring.context.SimpleComponent|checks.spring.context|6:6:6:21|false|false|||checks.spring.context.SimpleComponent"; JavaReadCache readCache = mock(JavaReadCache.class); when(readCache.readBytes(cacheKey)).thenReturn(serialized.getBytes(StandardCharsets.UTF_8)); @@ -462,7 +462,7 @@ void scanWithoutParsing_restores_dependencies_with_and_without_qualifier_from_ca String encodedEnvType = Base64.getEncoder().encodeToString("org.springframework.core.env.Environment".getBytes(StandardCharsets.UTF_8)); String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|false|" + String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|false||" + encodedAppContext + ":" + encodedPrimaryContext + "," + encodedEnvType + ":" + encodedEnvironment + "|checks.spring.context.QualifiedFieldDependencies"; @@ -536,7 +536,7 @@ void scanWithoutParsing_restores_full_type_hierarchy_from_cache() { InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/ComponentImplementingInterface.java")); String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); String encodedName = Base64.getEncoder().encodeToString("componentImplementingInterface".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.ComponentImplementingInterface|checks.spring.context|8:6:8:36|false|false|" + String serialized = encodedName + "|checks.spring.context.ComponentImplementingInterface|checks.spring.context|8:6:8:36|false|false||" + "|checks.spring.context.ComponentImplementingInterface" + ";org.springframework.context.ApplicationContextAware" + ";org.springframework.beans.factory.Aware"; From 44d349aec8eb53b0bdc4f3cb1eb5c1d5dd0cf7c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Thu, 27 Aug 2026 15:05:39 +0200 Subject: [PATCH 07/22] Fix duplicate issue bug --- .../checks/spring/AmbiguousDependencyCheck.java | 15 +++++++-------- .../main/java/org/sonar/java/JavaFrontend.java | 10 +++++++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index fad0cd9b4b3..17613803d2d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -58,14 +58,13 @@ public List findAmbiguousDependencies(SpringContextModel mo for (Map.Entry> dependency : bean.getDependingBeans().entrySet()) { String requiredType = dependency.getKey(); Set candidates = typeToBeanNamesIndex.getNamesForType(requiredType); - if (isResolved(candidates, dependency.getValue(), registry)) { - continue; - } - // A @Fallback candidate is only a real contender when it is the sole remaining one; otherwise it is - // ignored by Spring, so the effective candidates are whichever bean(s) are not marked @Fallback. - Set effectiveCandidates = excludeFallbackCandidates(candidates, registry); - if (effectiveCandidates.size() > 1) { - ambiguousDependencies.add(new AmbiguousDependency(bean.getLocation(), message(requiredType, effectiveCandidates))); + if (!isResolved(candidates, dependency.getValue(), registry)) { + // A @Fallback candidate is only a real contender when it is the sole remaining one; otherwise it is + // ignored by Spring, so the effective candidates are whichever bean(s) are not marked @Fallback. + Set effectiveCandidates = excludeFallbackCandidates(candidates, registry); + if (effectiveCandidates.size() > 1) { + ambiguousDependencies.add(new AmbiguousDependency(bean.getLocation(), message(requiredType, effectiveCandidates))); + } } } } diff --git a/java-frontend/src/main/java/org/sonar/java/JavaFrontend.java b/java-frontend/src/main/java/org/sonar/java/JavaFrontend.java index b315122b452..64766352989 100644 --- a/java-frontend/src/main/java/org/sonar/java/JavaFrontend.java +++ b/java-frontend/src/main/java/org/sonar/java/JavaFrontend.java @@ -83,9 +83,6 @@ public JavaFrontend(JavaVersion javaVersion, SonarComponents sonarComponents, Me List commonVisitors = new ArrayList<>(); commonVisitors.add(javaResourceLocator); commonVisitors.add(new Java25FeaturesTelemetryVisitor(telemetry)); - if (sonarComponents.getSpringContextModel() != null) { - commonVisitors.addAll(SpringContextModelGatherers.getAllGatherers()); - } if (postAnalysisIssueFilter != null) { commonVisitors.add(postAnalysisIssueFilter); } @@ -98,6 +95,13 @@ public JavaFrontend(JavaVersion javaVersion, SonarComponents sonarComponents, Me List testCodeVisitors = new ArrayList<>(commonVisitors); testCodeVisitors.add(measurer.new TestFileMeasurer()); + if (sonarComponents.getSpringContextModel() != null) { + // Call SpringContextModelGatherers.getAllGatherers twice to have separate gatherer + // instances between the main and test scanners to avoid duplicating the issues + codeVisitors.addAll(SpringContextModelGatherers.getAllGatherers()); + testCodeVisitors.addAll(SpringContextModelGatherers.getAllGatherers()); + } + if (!sonarComponents.isSonarLintContext()) { codeVisitors.add(new FileLinesVisitor(sonarComponents)); codeVisitors.add(new SyntaxHighlighterVisitor(sonarComponents)); From 9558a3b2d9c2b4cf2029aae2607d652ec775b786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Thu, 27 Aug 2026 15:34:44 +0200 Subject: [PATCH 08/22] Fix FN when there are two `@Primary` candidates --- .../spring/s9352/TwoPrimaryComponentA.java | 19 +++++++++++++++++++ .../spring/s9352/TwoPrimaryComponentB.java | 16 ++++++++++++++++ .../spring/s9352/TwoPrimaryConsumer.java | 13 +++++++++++++ .../spring/AmbiguousDependencyCheck.java | 7 ++++++- .../spring/AmbiguousDependencyCheckTest.java | 6 ++++++ 5 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryComponentA.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryComponentB.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryConsumer.java diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryComponentA.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryComponentA.java new file mode 100644 index 00000000000..c6cbdf05840 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryComponentA.java @@ -0,0 +1,19 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.DisposableBean; +import org.springframework.context.annotation.Primary; +import org.springframework.stereotype.Component; + +// Scenario: two candidates both marked @Primary, still ambiguous, issue expected. A single @Primary candidate +// disambiguates, but two conflicting primaries do not — Spring itself requires exactly one. A distinct interface +// from the other scenarios in this package, so that a whole-module scan does not merge candidate pools across +// scenarios. +@Primary +@Component +public class TwoPrimaryComponentA implements DisposableBean { + + @Override + public void destroy() { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryComponentB.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryComponentB.java new file mode 100644 index 00000000000..a7140548182 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryComponentB.java @@ -0,0 +1,16 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.DisposableBean; +import org.springframework.context.annotation.Primary; +import org.springframework.stereotype.Component; + +// See TwoPrimaryComponentA for context. +@Primary +@Component +public class TwoPrimaryComponentB implements DisposableBean { + + @Override + public void destroy() { + // not needed for test + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryConsumer.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryConsumer.java new file mode 100644 index 00000000000..96a36cdfd19 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/TwoPrimaryConsumer.java @@ -0,0 +1,13 @@ +package checks.spring.s9352; + +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +// See TwoPrimaryComponentA for context. +@Service +public class TwoPrimaryConsumer { + + @Autowired + private DisposableBean contextAware; +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index 17613803d2d..0d7f1f94a64 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -77,7 +77,12 @@ private static boolean isResolved(Set candidates, Set injectionP // itself), otherwise at least one injection point remains ambiguous. return candidates.size() <= 1 || injectionPointNames.stream().allMatch(name -> matchesCandidate(name, candidates, registry)) - || candidates.stream().anyMatch(candidate -> isPrimary(registry, candidate)); + || hasExactlyOnePrimaryCandidate(candidates, registry); + } + + private static boolean hasExactlyOnePrimaryCandidate(Set candidates, BeanDefinitionRegistry registry) { + // Two or more @Primary candidates still leave the dependency ambiguous. + return candidates.stream().filter(candidate -> isPrimary(registry, candidate)).count() == 1; } private static boolean matchesCandidate(String injectionPointName, Set candidates, BeanDefinitionRegistry registry) { diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java index 9ea22f24ee6..1a3e97c8251 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java @@ -58,6 +58,12 @@ void primary_candidate_resolves_ambiguity() { assertThat(check.findAmbiguousDependencies(model)).isEmpty(); } + @Test + void two_primary_candidates_still_raise_issue() { + SpringContextModel model = buildModel("TwoPrimaryComponentA.java", "TwoPrimaryComponentB.java", "TwoPrimaryConsumer.java"); + assertThat(check.findAmbiguousDependencies(model)).hasSize(1); + } + @Test void field_name_matching_bean_name_resolves_ambiguity() { SpringContextModel model = buildModel("BeanFactoryComponentA.java", "BeanFactoryComponentB.java", "NameMatchConsumer.java"); From 95075b9af2d03ce21b2b82741a3b85b7ff878796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Thu, 27 Aug 2026 17:01:33 +0200 Subject: [PATCH 09/22] Fix test gap and incorrect comments --- .../checks/spring/s9352/FallbackRegularComponent.java | 5 +---- .../spring/s9352/FallbackTwoCandidatesComponentA.java | 5 +---- .../java/checks/spring/AmbiguousDependencyCheckTest.java | 6 +++++- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java index 735c6cf212a..9471d540aa5 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java @@ -4,10 +4,7 @@ import org.springframework.context.ApplicationContextAware; import org.springframework.stereotype.Component; -// @Fallback (available since Spring 6.2, not present in the spring-context version on this module's classpath, -// hence non-compiling) is not yet handled by AmbiguousDependencyCheck: Spring would resolve this dependency -// unambiguously by ignoring the fallback candidate, but the check does not know that yet. Kept here for when -// @Fallback support is added. See FallbackComponent and FallbackConsumer. +// Spring resolves this dependency unambiguously by ignoring the fallback candidate. @Component class FallbackRegularComponent implements ApplicationContextAware { diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java index e3d87f30ce6..c9fef436109 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java @@ -8,10 +8,7 @@ // Three candidates of type MessageSourceAware exist: this class, FallbackTwoCandidatesComponentB (both regular), // and FallbackTwoCandidatesFallbackComponent (@Fallback). Spring ignores the fallback candidate only when a // single non-fallback candidate remains; here two non-fallback candidates still compete, so the dependency -// remains ambiguous. Not yet handled by AmbiguousDependencyCheck (see FallbackRegularComponent for context on -// why @Fallback support is pending), so this currently (incorrectly) raises no issue; it should once handled. -// A distinct interface from the other scenarios in this package, so that a whole-module scan does not merge -// candidate pools across scenarios. +// remains ambiguous. @Component public class FallbackTwoCandidatesComponentA implements MessageSourceAware { diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java index 1a3e97c8251..448567d806e 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java @@ -106,7 +106,11 @@ void fallback_candidate_does_not_resolve_ambiguity_with_two_other_candidates() { SpringContextModel model = buildModelFromNonCompilingSources( "FallbackTwoCandidatesComponentA.java", "FallbackTwoCandidatesComponentB.java", "FallbackTwoCandidatesFallbackComponent.java", "FallbackTwoCandidatesConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).hasSize(1); + List found = check.findAmbiguousDependencies(model); + assertThat(found).hasSize(1); + assertThat(found.get(0).message()) + .contains("fallbackTwoCandidatesComponentA", "fallbackTwoCandidatesComponentB") + .doesNotContain("fallbackTwoCandidatesFallbackComponent"); } /** From 7224507483d9f854b7de66be08a23af8a6a7b7fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Thu, 27 Aug 2026 17:24:15 +0200 Subject: [PATCH 10/22] Update comments --- .../checks/spring/s9352/FallbackTwoCandidatesComponentA.java | 4 ---- .../checks/spring/s9352/FallbackTwoCandidatesConsumer.java | 3 +-- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java index c9fef436109..36e047fe7fd 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java @@ -5,10 +5,6 @@ import org.springframework.stereotype.Component; // Scenario: @Fallback does not resolve ambiguity when at least two other (non-fallback) candidates remain. -// Three candidates of type MessageSourceAware exist: this class, FallbackTwoCandidatesComponentB (both regular), -// and FallbackTwoCandidatesFallbackComponent (@Fallback). Spring ignores the fallback candidate only when a -// single non-fallback candidate remains; here two non-fallback candidates still compete, so the dependency -// remains ambiguous. @Component public class FallbackTwoCandidatesComponentA implements MessageSourceAware { diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java index 9dea9a1125e..ee150967678 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java @@ -4,8 +4,7 @@ import org.springframework.context.MessageSourceAware; import org.springframework.stereotype.Service; -// See FallbackTwoCandidatesComponentA for context: still ambiguous between the two non-fallback candidates, -// issue expected once @Fallback support is added. +// See FallbackTwoCandidatesComponentA for context. @Service public class FallbackTwoCandidatesConsumer { From a0a90d3738876da1e493007fefb010a2da730209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Fri, 28 Aug 2026 17:38:12 +0200 Subject: [PATCH 11/22] Refactor project sensor to use Strategy design pattern --- .../spring/s9352/FallbackComponent.java | 16 ------- .../checks/spring/s9352/FallbackConsumer.java | 13 ------ .../s9352/FallbackRegularComponent.java | 15 ------- .../FallbackTwoCandidatesComponentA.java | 15 ------- .../FallbackTwoCandidatesComponentB.java | 15 ------- .../s9352/FallbackTwoCandidatesConsumer.java | 13 ------ ...allbackTwoCandidatesFallbackComponent.java | 16 ------- .../spring/AmbiguousDependencyCheck.java | 39 +++++----------- .../checks/spring/SpringContextCheck.java | 32 +++++++++++++ .../checks/spring/SpringContextChecks.java | 42 +++++++++++++++++ .../checks/spring/SpringContextIssue.java | 28 ++++++++++++ .../spring/AmbiguousDependencyCheckTest.java | 45 ++++--------------- .../springcontext/BeanDefinitionGatherer.java | 26 ++++------- .../springcontext/BeanDefinitionHolder.java | 20 --------- .../BeanDefinitionGathererTest.java | 6 +-- .../java/SpringContextModelSensor.java | 19 +++++--- 16 files changed, 145 insertions(+), 215 deletions(-) delete mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java delete mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackConsumer.java delete mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java delete mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java delete mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentB.java delete mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java delete mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesFallbackComponent.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextCheck.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextChecks.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextIssue.java diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java deleted file mode 100644 index ccf445acb86..00000000000 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackComponent.java +++ /dev/null @@ -1,16 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.context.ApplicationContext; -import org.springframework.context.ApplicationContextAware; -import org.springframework.stereotype.Component; - -// See FallbackRegularComponent for context. -@org.springframework.context.annotation.Fallback -@Component -class FallbackComponent implements ApplicationContextAware { - - @Override - public void setApplicationContext(ApplicationContext ctx) { - // not needed for test - } -} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackConsumer.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackConsumer.java deleted file mode 100644 index e42a2e86717..00000000000 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackConsumer.java +++ /dev/null @@ -1,13 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.ApplicationContextAware; -import org.springframework.stereotype.Service; - -// See FallbackRegularComponent for context. -@Service -class FallbackConsumer { - - @Autowired - private ApplicationContextAware contextAware; -} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java deleted file mode 100644 index 9471d540aa5..00000000000 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackRegularComponent.java +++ /dev/null @@ -1,15 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.context.ApplicationContext; -import org.springframework.context.ApplicationContextAware; -import org.springframework.stereotype.Component; - -// Spring resolves this dependency unambiguously by ignoring the fallback candidate. -@Component -class FallbackRegularComponent implements ApplicationContextAware { - - @Override - public void setApplicationContext(ApplicationContext ctx) { - // not needed for test - } -} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java deleted file mode 100644 index 36e047fe7fd..00000000000 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentA.java +++ /dev/null @@ -1,15 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.context.MessageSource; -import org.springframework.context.MessageSourceAware; -import org.springframework.stereotype.Component; - -// Scenario: @Fallback does not resolve ambiguity when at least two other (non-fallback) candidates remain. -@Component -public class FallbackTwoCandidatesComponentA implements MessageSourceAware { - - @Override - public void setMessageSource(MessageSource messageSource) { - // not needed for test - } -} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentB.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentB.java deleted file mode 100644 index 0523499ca04..00000000000 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesComponentB.java +++ /dev/null @@ -1,15 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.context.MessageSource; -import org.springframework.context.MessageSourceAware; -import org.springframework.stereotype.Component; - -// See FallbackTwoCandidatesComponentA for context. -@Component -public class FallbackTwoCandidatesComponentB implements MessageSourceAware { - - @Override - public void setMessageSource(MessageSource messageSource) { - // not needed for test - } -} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java deleted file mode 100644 index ee150967678..00000000000 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesConsumer.java +++ /dev/null @@ -1,13 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.MessageSourceAware; -import org.springframework.stereotype.Service; - -// See FallbackTwoCandidatesComponentA for context. -@Service -public class FallbackTwoCandidatesConsumer { - - @Autowired - private MessageSourceAware contextAware; -} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesFallbackComponent.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesFallbackComponent.java deleted file mode 100644 index 9a0c5f965a4..00000000000 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/s9352/FallbackTwoCandidatesFallbackComponent.java +++ /dev/null @@ -1,16 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.context.MessageSource; -import org.springframework.context.MessageSourceAware; -import org.springframework.stereotype.Component; - -// See FallbackTwoCandidatesComponentA for context. -@org.springframework.context.annotation.Fallback -@Component -public class FallbackTwoCandidatesFallbackComponent implements MessageSourceAware { - - @Override - public void setMessageSource(MessageSource messageSource) { - // not needed for test - } -} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index 0d7f1f94a64..7d977a163a2 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -26,7 +26,6 @@ import org.sonar.check.Rule; import org.sonar.java.model.springcontext.BeanDefinitionHolder; import org.sonar.java.model.springcontext.BeanDefinitionRegistry; -import org.sonar.java.model.springcontext.BeanLocation; import org.sonar.java.model.springcontext.SpringContextModel; import org.sonar.java.model.springcontext.TypeToBeanNamesIndex; import org.sonar.plugins.java.api.JavaCheck; @@ -37,51 +36,40 @@ * bean of a given type across the whole analyzed scope, not a single file. */ @Rule(key = "S9352") -public class AmbiguousDependencyCheck implements JavaCheck { - - /** - * @param location bean whose dependency is ambiguous, used to anchor the reported issue - * @param message issue message describing the ambiguity - */ - public record AmbiguousDependency(BeanLocation location, String message) { - } +public class AmbiguousDependencyCheck implements JavaCheck, SpringContextCheck { private static final String MESSAGE = "Multiple beans of type \"%s\" match this dependency (%s);" + " disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\"."; - public List findAmbiguousDependencies(SpringContextModel model) { + @Override + public List execute(SpringContextModel model) { BeanDefinitionRegistry registry = model.getBeanDefinitionRegistry(); TypeToBeanNamesIndex typeToBeanNamesIndex = model.getTypeToBeanNamesIndex(); - List ambiguousDependencies = new ArrayList<>(); + List issues = new ArrayList<>(); for (BeanDefinitionHolder bean : registry.getAll()) { for (Map.Entry> dependency : bean.getDependingBeans().entrySet()) { String requiredType = dependency.getKey(); Set candidates = typeToBeanNamesIndex.getNamesForType(requiredType); if (!isResolved(candidates, dependency.getValue(), registry)) { - // A @Fallback candidate is only a real contender when it is the sole remaining one; otherwise it is - // ignored by Spring, so the effective candidates are whichever bean(s) are not marked @Fallback. - Set effectiveCandidates = excludeFallbackCandidates(candidates, registry); + // excluding all beans with a configured profile, no matter what the profile is, to avoid FPs + Set effectiveCandidates = excludeCandidatesWithProfile(candidates, registry); if (effectiveCandidates.size() > 1) { - ambiguousDependencies.add(new AmbiguousDependency(bean.getLocation(), message(requiredType, effectiveCandidates))); + issues.add(new SpringContextIssue(bean.getLocation(), message(requiredType, effectiveCandidates))); } } } } - return ambiguousDependencies; + return issues; } private static boolean isResolved(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { - // injectionPointNames merges every injection point of this type declared on the bean: it is only resolved - // if EVERY one of them names a candidate (by bean name or by a @Qualifier declared on that candidate bean - // itself), otherwise at least one injection point remains ambiguous. return candidates.size() <= 1 || injectionPointNames.stream().allMatch(name -> matchesCandidate(name, candidates, registry)) || hasExactlyOnePrimaryCandidate(candidates, registry); } private static boolean hasExactlyOnePrimaryCandidate(Set candidates, BeanDefinitionRegistry registry) { - // Two or more @Primary candidates still leave the dependency ambiguous. return candidates.stream().filter(candidate -> isPrimary(registry, candidate)).count() == 1; } @@ -99,19 +87,16 @@ private static String qualifierOf(BeanDefinitionRegistry registry, String beanNa .orElse(null); } - private static Set excludeFallbackCandidates(Set candidates, BeanDefinitionRegistry registry) { - Set nonFallbackCandidates = candidates.stream() - .filter(candidate -> !isFallback(registry, candidate)) - .collect(Collectors.toUnmodifiableSet()); - return nonFallbackCandidates.isEmpty() ? candidates : nonFallbackCandidates; + private static Set excludeCandidatesWithProfile(Set candidates, BeanDefinitionRegistry registry) { + return candidates.stream().filter(candidate -> !hasProfile(registry, candidate)).collect(Collectors.toUnmodifiableSet()); } private static boolean isPrimary(BeanDefinitionRegistry registry, String beanName) { return registry.getByName(beanName).stream().anyMatch(BeanDefinitionHolder::isPrimary); } - private static boolean isFallback(BeanDefinitionRegistry registry, String beanName) { - return registry.getByName(beanName).stream().anyMatch(BeanDefinitionHolder::isFallback); + private static boolean hasProfile(BeanDefinitionRegistry registry, String beanName) { + return registry.getByName(beanName).stream().anyMatch(bean -> bean.getProfiles() != null); } private static String message(String requiredType, Set candidates) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextCheck.java new file mode 100644 index 00000000000..ec8a6344b0e --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextCheck.java @@ -0,0 +1,32 @@ +/* + * 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.spring; + +import java.util.List; +import org.sonar.java.model.springcontext.SpringContextModel; + +/** + * A check that reasons over the whole, already-populated {@link SpringContextModel} rather than visiting a + * single file's AST, since some Spring configuration issues (e.g. ambiguous autowiring) can only be detected + * once every bean across the analyzed scope is known. Implementations are run once, at the end of the + * analysis, by {@code SpringContextModelSensor}. + */ +public interface SpringContextCheck { + + List execute(SpringContextModel model); + +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextChecks.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextChecks.java new file mode 100644 index 00000000000..5f62e244aec --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextChecks.java @@ -0,0 +1,42 @@ +/* + * 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.spring; + +import java.util.List; + +/** + * Registry of all {@link SpringContextCheck}s to be run against the {@code SpringContextModel}. + * + *

      Use {@link #getAllChecks()} to obtain the full list of checks to be run by the scanner. + * New checks should be added here as the set of Spring context issues we detect grows. + */ +public final class SpringContextChecks { + + private SpringContextChecks() { + // utility class, should not be instantiated + } + + /** + * Returns all checks that reason over the {@code SpringContextModel}. + */ + public static List getAllChecks() { + return List.of( + new AmbiguousDependencyCheck() + ); + } + +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextIssue.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextIssue.java new file mode 100644 index 00000000000..8be6482dc17 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextIssue.java @@ -0,0 +1,28 @@ +/* + * 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.spring; + +import org.sonar.java.model.springcontext.BeanLocation; + +/** + * An issue found by a {@link SpringContextCheck}, ready to be reported by {@code SpringContextModelSensor}. + * + * @param location bean location the issue is anchored to + * @param message issue message + */ +public record SpringContextIssue(BeanLocation location, String message) { +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java index 448567d806e..74d072325be 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java @@ -49,68 +49,49 @@ class AmbiguousDependencyCheckTest { @Test void ambiguous_dependency_with_no_disambiguation_raises_issue() { SpringContextModel model = buildModel("ComponentOne.java", "ComponentTwo.java", "UnresolvedConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).hasSize(1); + assertThat(check.execute(model)).hasSize(1); } @Test void primary_candidate_resolves_ambiguity() { SpringContextModel model = buildModel("BeanNameComponent.java", "PrimaryComponent.java", "PrimaryConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + assertThat(check.execute(model)).isEmpty(); } @Test void two_primary_candidates_still_raise_issue() { SpringContextModel model = buildModel("TwoPrimaryComponentA.java", "TwoPrimaryComponentB.java", "TwoPrimaryConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).hasSize(1); + assertThat(check.execute(model)).hasSize(1); } @Test void field_name_matching_bean_name_resolves_ambiguity() { SpringContextModel model = buildModel("BeanFactoryComponentA.java", "BeanFactoryComponentB.java", "NameMatchConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + assertThat(check.execute(model)).isEmpty(); } @Test void qualifier_resolves_ambiguity() { SpringContextModel model = buildModel("EnvironmentComponentA.java", "EnvironmentComponentB.java", "QualifierConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + assertThat(check.execute(model)).isEmpty(); } @Test void single_candidate_does_not_raise_issue() { SpringContextModel model = buildModel("ResourceLoaderComponent.java", "SingleCandidateConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).isEmpty(); + assertThat(check.execute(model)).isEmpty(); } @Test void one_resolved_injection_point_does_not_hide_another_ambiguous_one_of_the_same_type() { SpringContextModel model = buildModel("MixedInjectionComponentA.java", "MixedInjectionComponentB.java", "MixedInjectionConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).hasSize(1); + assertThat(check.execute(model)).hasSize(1); } @Test void qualifier_declared_on_the_bean_itself_resolves_ambiguity() { SpringContextModel model = buildModel("QualifierOnBeanComponentA.java", "QualifierOnBeanComponentB.java", "QualifierOnBeanConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).isEmpty(); - } - - @Test - void fallback_candidate_resolves_ambiguity_when_it_is_the_sole_remaining_candidate() { - SpringContextModel model = buildModelFromNonCompilingSources( - "FallbackRegularComponent.java", "FallbackComponent.java", "FallbackConsumer.java"); - assertThat(check.findAmbiguousDependencies(model)).isEmpty(); - } - - @Test - void fallback_candidate_does_not_resolve_ambiguity_with_two_other_candidates() { - SpringContextModel model = buildModelFromNonCompilingSources( - "FallbackTwoCandidatesComponentA.java", "FallbackTwoCandidatesComponentB.java", - "FallbackTwoCandidatesFallbackComponent.java", "FallbackTwoCandidatesConsumer.java"); - List found = check.findAmbiguousDependencies(model); - assertThat(found).hasSize(1); - assertThat(found.get(0).message()) - .contains("fallbackTwoCandidatesComponentA", "fallbackTwoCandidatesComponentB") - .doesNotContain("fallbackTwoCandidatesFallbackComponent"); + assertThat(check.execute(model)).isEmpty(); } /** @@ -125,16 +106,6 @@ private static SpringContextModel buildModel(String... relativeFilePaths) { .toList()); } - /** - * Same as {@link #buildModel(String...)}, but resolving files under {@code src/main/files/non-compiling} - * instead, for fixtures relying on annotations not present on this module's classpath. - */ - private static SpringContextModel buildModelFromNonCompilingSources(String... relativeFilePaths) { - return buildModel(Arrays.stream(relativeFilePaths) - .map(relativeFilePath -> TestUtils.nonCompilingTestSourcesPath(BASE_PATH + relativeFilePath)) - .toList()); - } - private static SpringContextModel buildModel(List filePaths) { List classpath = TestClasspathUtils.DEFAULT_MODULE.getClassPath(); SonarComponents sonarComponents = new SonarComponents(null, null, null, null, null, null); diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java index 95839aa9530..d367e2c94ec 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java @@ -61,7 +61,6 @@ *

      Also captures: *

        *
      • {@code @Primary} designation
        • - *
      • {@code @Fallback} designation
        • *
      • {@code @Qualifier} value declared on the bean itself (as opposed to on an injection point)
        • *
      • Dependencies via {@code @Autowired} fields, constructors, and setters for class-level beans
        • *
      • Dependencies via method parameters for {@code @Bean} method beans
        • @@ -84,7 +83,6 @@ public class BeanDefinitionGatherer extends SpringContextModelGatherer { private static final String TYPE_HIERARCHY_SEPARATOR = ";"; private static final String PRIMARY_ANNOTATION = "org.springframework.context.annotation.Primary"; - private static final String FALLBACK_ANNOTATION = "org.springframework.context.annotation.Fallback"; private static final String VALUE_ATTRIBUTE = "value"; private final List collectedBeans = new ArrayList<>(); @@ -99,7 +97,6 @@ private record BeanData( InputFile inputFile, AnalyzerMessage.TextSpan textSpan, boolean isPrimary, - boolean isFallback, @Nullable String qualifier, Map> dependingBeans, Set typeHierarchy) { @@ -137,7 +134,6 @@ public void visitNode(Tree tree) { context.getInputFile(), AnalyzerMessage.textSpanFor(classTree.simpleName()), meta.isAnnotatedWith(PRIMARY_ANNOTATION), - meta.isAnnotatedWith(FALLBACK_ANNOTATION), extractQualifier(meta), deps, typeHierarchy); @@ -196,7 +192,6 @@ private static String serializeBean(BeanData bean) { bean.beanPackage(), span.startLine + ":" + span.startCharacter + ":" + span.endLine + ":" + span.endCharacter, Boolean.toString(bean.isPrimary()), - Boolean.toString(bean.isFallback()), encodedQualifier, deps, typeHierarchy); @@ -212,9 +207,6 @@ public void gatherSpringContextData(ModuleScannerContext context, SpringContextM if (data.isPrimary()) { holderBuilder.primary(); } - if (data.isFallback()) { - holderBuilder.fallback(); - } holderBuilder.qualifier(data.qualifier()); springContextModel.getBeanDefinitionRegistry() .addBeanDefinition(data.beanName(), holderBuilder.build()); @@ -267,13 +259,12 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { Integer.parseInt(spanParts[2]), Integer.parseInt(spanParts[3])); boolean isPrimary = Boolean.parseBoolean(fields[4]); - boolean isFallback = Boolean.parseBoolean(fields[5]); - String qualifier = !fields[6].isEmpty() - ? new String(Base64.getDecoder().decode(fields[6]), StandardCharsets.UTF_8) + String qualifier = !fields[5].isEmpty() + ? new String(Base64.getDecoder().decode(fields[5]), StandardCharsets.UTF_8) : null; Map> deps = new LinkedHashMap<>(); - if (!fields[7].isEmpty()) { - for (String entry : fields[7].split(DEP_SEPARATOR)) { + if (!fields[6].isEmpty()) { + for (String entry : fields[6].split(DEP_SEPARATOR)) { int idx = entry.indexOf(DEP_KEY_VALUE_SEPARATOR); String typeFqn = new String(Base64.getDecoder().decode(entry.substring(0, idx)), StandardCharsets.UTF_8); Set names = Arrays.stream(entry.substring(idx + 1).split(DEP_NAMES_SEPARATOR)) @@ -282,10 +273,10 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { deps.put(typeFqn, names); } } - Set typeHierarchy = !fields[8].isEmpty() - ? new LinkedHashSet<>(List.of(fields[8].split(TYPE_HIERARCHY_SEPARATOR))) + Set typeHierarchy = !fields[7].isEmpty() + ? new LinkedHashSet<>(List.of(fields[7].split(TYPE_HIERARCHY_SEPARATOR))) : new LinkedHashSet<>(); - return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, isFallback, qualifier, deps, typeHierarchy); + return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, qualifier, deps, typeHierarchy); } private static Optional extractBeanName(SymbolMetadata meta) { @@ -336,13 +327,12 @@ private void collectBeanMethod(MethodTree method, String pkg) { Map> paramDeps = parameterDependencies(method); boolean isPrimary = beanMeta.isAnnotatedWith(PRIMARY_ANNOTATION); - boolean isFallback = beanMeta.isAnnotatedWith(FALLBACK_ANNOTATION); String qualifier = extractQualifier(beanMeta); var textSpan = AnalyzerMessage.textSpanFor(method.simpleName()); var inputFile = context.getInputFile(); for (String beanName : beanNames) { - var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, isFallback, qualifier, paramDeps, typeHierarchy); + var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, qualifier, paramDeps, typeHierarchy); collectedBeans.add(beanData); beansCollectedAtFileLevel.add(beanData); } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java index 62b467f6f01..9095a290aae 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java @@ -66,9 +66,6 @@ public class BeanDefinitionHolder { /** Whether the bean is marked as {@code @Primary}, making it the preferred candidate for autowiring. */ private boolean isPrimary = false; - /** Whether the bean is marked as {@code @Fallback}, making it a last-resort candidate for autowiring. */ - private boolean isFallback = false; - /** Value of the {@code @Qualifier} annotation declared on the bean itself, or {@code null} if absent. */ @Nullable private String qualifier; @@ -92,10 +89,6 @@ private void setPrimary() { this.isPrimary = true; } - private void setFallback() { - this.isFallback = true; - } - private void setQualifier(@Nullable String qualifier) { this.qualifier = qualifier; } @@ -129,10 +122,6 @@ public boolean isPrimary() { return isPrimary; } - public boolean isFallback() { - return isFallback; - } - @Nullable public String getQualifier() { return qualifier; @@ -147,7 +136,6 @@ public static class Builder { @Nullable private String profiles; private boolean isPrimary = false; - private boolean isFallback = false; @Nullable private String qualifier; @@ -173,11 +161,6 @@ public Builder primary() { return this; } - public Builder fallback() { - this.isFallback = true; - return this; - } - public Builder qualifier(@Nullable String qualifier) { this.qualifier = qualifier; return this; @@ -192,9 +175,6 @@ public BeanDefinitionHolder build() { if (isPrimary) { holder.setPrimary(); } - if (isFallback) { - holder.setFallback(); - } return holder; } } diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index 0df08f9e93b..a30ddd6f5d7 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -342,7 +342,7 @@ void scanWithoutParsing_returns_true_and_restores_beans_on_cache_hit() { InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/SimpleComponent.java")); String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); String encodedName = Base64.getEncoder().encodeToString("simpleComponent".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.SimpleComponent|checks.spring.context|6:6:6:21|false|false|||checks.spring.context.SimpleComponent"; + String serialized = encodedName + "|checks.spring.context.SimpleComponent|checks.spring.context|6:6:6:21|false|||checks.spring.context.SimpleComponent"; JavaReadCache readCache = mock(JavaReadCache.class); when(readCache.readBytes(cacheKey)).thenReturn(serialized.getBytes(StandardCharsets.UTF_8)); @@ -462,7 +462,7 @@ void scanWithoutParsing_restores_dependencies_with_and_without_qualifier_from_ca String encodedEnvType = Base64.getEncoder().encodeToString("org.springframework.core.env.Environment".getBytes(StandardCharsets.UTF_8)); String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|false||" + String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false||" + encodedAppContext + ":" + encodedPrimaryContext + "," + encodedEnvType + ":" + encodedEnvironment + "|checks.spring.context.QualifiedFieldDependencies"; @@ -536,7 +536,7 @@ void scanWithoutParsing_restores_full_type_hierarchy_from_cache() { InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/ComponentImplementingInterface.java")); String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); String encodedName = Base64.getEncoder().encodeToString("componentImplementingInterface".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.ComponentImplementingInterface|checks.spring.context|8:6:8:36|false|false||" + String serialized = encodedName + "|checks.spring.context.ComponentImplementingInterface|checks.spring.context|8:6:8:36|false||" + "|checks.spring.context.ComponentImplementingInterface" + ";org.springframework.context.ApplicationContextAware" + ";org.springframework.beans.factory.Aware"; diff --git a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java b/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java index 1e9e6dddb6d..35692c7cf60 100644 --- a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java +++ b/sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java @@ -22,8 +22,11 @@ import org.sonar.api.batch.sensor.issue.NewIssue; import org.sonar.api.rule.RuleKey; import org.sonar.api.scanner.sensor.ProjectSensor; +import org.sonar.check.Rule; import org.sonar.java.GeneratedCheckList; -import org.sonar.java.checks.spring.AmbiguousDependencyCheck; +import org.sonar.java.checks.spring.SpringContextCheck; +import org.sonar.java.checks.spring.SpringContextChecks; +import org.sonar.java.checks.spring.SpringContextIssue; import org.sonar.java.jsp.Jasper; import org.sonar.java.model.springcontext.BeanLocation; import org.sonar.java.model.springcontext.SpringContextModel; @@ -55,19 +58,21 @@ public void describe(SensorDescriptor descriptor) { @Override public void execute(SensorContext context) { - reportAmbiguousDependencies(context); + for (SpringContextCheck check : SpringContextChecks.getAllChecks()) { + reportIssues(context, check); + } } - private void reportAmbiguousDependencies(SensorContext context) { - RuleKey ruleKey = RuleKey.of(GeneratedCheckList.REPOSITORY_KEY, "S9352"); - for (var ambiguousDependency : new AmbiguousDependencyCheck().findAmbiguousDependencies(springContextModel)) { - BeanLocation location = ambiguousDependency.location(); + private void reportIssues(SensorContext context, SpringContextCheck check) { + RuleKey ruleKey = RuleKey.of(GeneratedCheckList.REPOSITORY_KEY, check.getClass().getAnnotation(Rule.class).key()); + for (SpringContextIssue issue : check.execute(springContextModel)) { + BeanLocation location = issue.location(); AnalyzerMessage.TextSpan span = location.mainLocation(); NewIssue newIssue = context.newIssue().forRule(ruleKey); newIssue.at(newIssue.newLocation() .on(location.inputFile()) .at(location.inputFile().newRange(span.startLine, span.startCharacter, span.endLine, span.endCharacter)) - .message(ambiguousDependency.message())); + .message(issue.message())); newIssue.save(); } } From da8696a011462f59eec5c19d35bd8b24d16d26f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Mon, 31 Aug 2026 14:12:26 +0200 Subject: [PATCH 12/22] Create new structure for storing all collected dependencies --- .../springcontext/BeanDefinitionGatherer.java | 8 ++- .../springcontext/SpringContextModel.java | 7 ++ .../TypeToDependenciesIndex.java | 58 +++++++++++++++++ .../BeanDefinitionGathererTest.java | 65 +++++++++++++++++++ 4 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java index d367e2c94ec..fc8c443b4fb 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java @@ -67,8 +67,10 @@ *
      • Implicit single-constructor injection (no {@code @Autowired} required)
        • *
      * - *

      Also populates {@link TypeToBeanNamesIndex} with the full type hierarchy of each bean, - * so that rules can look up all beans assignable to a given type. + *

      Also populates: + *

        + *
      • {@link TypeToBeanNamesIndex} with the full type hierarchy of each bean
        • + *
      • {@link TypeToDependenciesIndex} with the full type hierarchy of each bean
        • */ public class BeanDefinitionGatherer extends SpringContextModelGatherer { @@ -213,6 +215,8 @@ public void gatherSpringContextData(ModuleScannerContext context, SpringContextM for (String typeFqn : data.typeHierarchy()) { springContextModel.getTypeToBeanNamesIndex().addBeanForType(typeFqn, data.beanName()); } + data.dependingBeans().forEach((typeFqn, names) -> + names.forEach(name -> springContextModel.getTypeToDependenciesIndex().addBeanForType(typeFqn, name))); } } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/SpringContextModel.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/SpringContextModel.java index 675b8b6c670..3272679fbd7 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/SpringContextModel.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/SpringContextModel.java @@ -43,6 +43,9 @@ public class SpringContextModel { /** Index for resolving bean names by their fully-qualified type. */ private final TypeToBeanNamesIndex typeToBeanNamesIndex = new TypeToBeanNamesIndex(); + /** Index for storing injected dependencies by their fully-qualified type. */ + private final TypeToDependenciesIndex typeToDependenciesIndex = new TypeToDependenciesIndex(); + /** Index of properties associated with Spring Data / Hibernate {@code @Entity} classes. */ private final EntityClassToPropertiesIndex entityClassToPropertiesIndex = new EntityClassToPropertiesIndex(); @@ -58,6 +61,10 @@ public TypeToBeanNamesIndex getTypeToBeanNamesIndex() { return typeToBeanNamesIndex; } + public TypeToDependenciesIndex getTypeToDependenciesIndex() { + return typeToDependenciesIndex; + } + public EntityClassToPropertiesIndex getEntityClassToPropertiesIndex() { return entityClassToPropertiesIndex; } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java new file mode 100644 index 00000000000..f7efc1150a7 --- /dev/null +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java @@ -0,0 +1,58 @@ +/* + * 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.model.springcontext; + +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +/** + * Index mapping fully-qualified bean type names to the names of all dependencies of that type + * discovered during Spring context scanning. + * + *

        A single type may have multiple dependencies registered. Lookup returns an empty set for types with no + * registered dependencies. + * + *

        Dependencies are represented by a name (either the field/parameter name at the injection point or the value of + * the `@Qualifier` annotation if present) + */ +public class TypeToDependenciesIndex { + /** Dependencies (name, location) indexed by fully-qualified type. */ + private final Map> beanDependenciesByType = new HashMap<>(); + + /** + * Registers a dependency under the given type. + * + * @param dependencyType fully-qualified name of the dependency's type + * @param dependencyName the dependency name to associate with that type + */ + public void addBeanForType(String dependencyType, String dependencyName) { + beanDependenciesByType.computeIfAbsent(dependencyType, k -> new HashSet<>()).add(dependencyName); + } + + /** + * Returns an immutable set of all bean names registered for the given type. + * + * @param dependencyType fully-qualified class name of the dependency's type + * @return an unmodifiable set of bean names, or an empty set if none were registered + */ + public Set getNamesForType(String dependencyType) { + return Collections.unmodifiableSet(beanDependenciesByType.getOrDefault(dependencyType, Set.of())); + } +} diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index a30ddd6f5d7..bb27ebe50ea 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -37,6 +37,7 @@ import org.sonar.plugins.java.api.caching.CacheContext; import org.sonar.plugins.java.api.caching.JavaReadCache; import org.sonar.plugins.java.api.caching.JavaWriteCache; + import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.ArgumentMatchers.any; @@ -633,4 +634,68 @@ private static CacheContext mockCacheContext(JavaReadCache readCache, JavaWriteC when(cacheContext.getWriteCache()).thenReturn(writeCache); return cacheContext; } + + // ---- TypeToDependenciesIndex ------------------------------------------------- + @ParameterizedTest(name = "{0}") + @ValueSource(strings = {"src/test/files/springcontext/AutowiredDependencies.java", + "src/test/files/springcontext/AutowiredConstructorDependencies.java" + }) + void regular_dependencies_registered_in_index(String filePath) { + scan(filePath); + assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.context.ApplicationContext")).containsOnly("applicationContext"); + assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.core.env.Environment")).containsOnly("environment"); + } + + @Test + void dependency_with_qualifier_registered_with_qualifier_value() { + scan("src/test/files/springcontext/QualifiedBeanMethodDependencies.java"); + assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.context.ApplicationContext")).containsOnly("primaryContext"); + } + + @Test + void dependency_with_qualifier_on_constructor_registered_with_qualifier_value() { + scan("src/test/files/springcontext/OrderService.java"); + assertThat(model.getTypeToDependenciesIndex().getNamesForType("PaymentProcessor")).containsOnly("paypal"); + } + + @Test + void dependencies_in_multiple_files_all_registered() { + scan("src/test/files/springcontext/OrderService.java", "src/test/files/springcontext/BlankQualifierDependency.java"); + assertThat(model.getTypeToDependenciesIndex().getNamesForType("PaymentProcessor")).containsOnly("paypal"); + assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.context.ApplicationContext")).containsOnly("applicationContext"); + } + + @Test + void scanWithoutParsing_restores_dependencies_index_from_cache() { + InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/QualifiedFieldDependencies.java")); + String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); + String encodedName = Base64.getEncoder().encodeToString("qualifiedFieldDependencies".getBytes(StandardCharsets.UTF_8)); + String encodedAppContext = Base64.getEncoder().encodeToString("org.springframework.context.ApplicationContext".getBytes(StandardCharsets.UTF_8)); + String encodedEnvType = Base64.getEncoder().encodeToString("org.springframework.core.env.Environment".getBytes(StandardCharsets.UTF_8)); + String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); + String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); + String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false||" + + encodedAppContext + ":" + encodedPrimaryContext + + "," + encodedEnvType + ":" + encodedEnvironment + + "|checks.spring.context.QualifiedFieldDependencies"; + + JavaReadCache readCache = mock(JavaReadCache.class); + when(readCache.readBytes(cacheKey)).thenReturn(serialized.getBytes(StandardCharsets.UTF_8)); + CacheContext cacheContext = mockCacheContext(readCache, mock(JavaWriteCache.class)); + + InputFileScannerContext context = mock(InputFileScannerContext.class); + when(context.getInputFile()).thenReturn(inputFile); + when(context.getCacheContext()).thenReturn(cacheContext); + + assertThat(gatherer.scanWithoutParsing(context)).isTrue(); + + ModuleScannerContext moduleScannerContext = mock(ModuleScannerContext.class); + when(moduleScannerContext.getModuleKey()).thenReturn(""); + gatherer.gatherSpringContextData(moduleScannerContext, model); + + assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.context.ApplicationContext")) + .containsOnly("primaryContext"); + assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.core.env.Environment")) + .containsOnly("environment"); + } } From 14b05cc00d7e1f8b9a70ced2c8284e0148c01793 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Mon, 31 Aug 2026 16:04:22 +0200 Subject: [PATCH 13/22] Refactor check to use TypeToDependenciesIndex --- .../spring/AmbiguousDependencyCheck.java | 35 ++++--- .../springcontext/BeanDefinitionGatherer.java | 83 +++++++++++----- .../springcontext/TypeToBeanNamesIndex.java | 4 + .../TypeToDependenciesIndex.java | 15 +-- .../BeanDefinitionGathererTest.java | 95 +++++++++++++++---- 5 files changed, 170 insertions(+), 62 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index 7d977a163a2..66ddb073609 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -17,7 +17,6 @@ package org.sonar.java.checks.spring; import java.util.ArrayList; -import java.util.Map; import java.util.List; import java.util.Objects; import java.util.Set; @@ -28,6 +27,8 @@ import org.sonar.java.model.springcontext.BeanDefinitionRegistry; import org.sonar.java.model.springcontext.SpringContextModel; import org.sonar.java.model.springcontext.TypeToBeanNamesIndex; +import org.sonar.java.model.springcontext.TypeToDependenciesIndex; +import org.sonar.java.model.springcontext.TypeToDependenciesIndex.InjectionPoint; import org.sonar.plugins.java.api.JavaCheck; /** @@ -38,24 +39,25 @@ @Rule(key = "S9352") public class AmbiguousDependencyCheck implements JavaCheck, SpringContextCheck { - private static final String MESSAGE = "Multiple beans of type \"%s\" match this dependency (%s);" + private static final String MESSAGE = "Multiple beans match this dependency (%s);" + " disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\"."; @Override public List execute(SpringContextModel model) { BeanDefinitionRegistry registry = model.getBeanDefinitionRegistry(); TypeToBeanNamesIndex typeToBeanNamesIndex = model.getTypeToBeanNamesIndex(); + TypeToDependenciesIndex typeToDependenciesIndex = model.getTypeToDependenciesIndex(); List issues = new ArrayList<>(); - for (BeanDefinitionHolder bean : registry.getAll()) { - for (Map.Entry> dependency : bean.getDependingBeans().entrySet()) { - String requiredType = dependency.getKey(); - Set candidates = typeToBeanNamesIndex.getNamesForType(requiredType); - if (!isResolved(candidates, dependency.getValue(), registry)) { - // excluding all beans with a configured profile, no matter what the profile is, to avoid FPs - Set effectiveCandidates = excludeCandidatesWithProfile(candidates, registry); - if (effectiveCandidates.size() > 1) { - issues.add(new SpringContextIssue(bean.getLocation(), message(requiredType, effectiveCandidates))); + for (String type : typeToBeanNamesIndex.getKeys()) { + Set candidates = typeToBeanNamesIndex.getNamesForType(type); + Set injectionPoints = typeToDependenciesIndex.getDependenciesForType(type); + if (!isResolved(candidates, registry)) { + // excluding all beans with a configured profile, no matter what the profile is, to avoid FPs + Set effectiveCandidates = excludeCandidatesWithProfile(candidates, registry); + if (effectiveCandidates.size() > 1) { + for (InjectionPoint unresolvedInjectionPoint : computeUnmatchingNames(effectiveCandidates, injectionPoints, registry)) { + issues.add(new SpringContextIssue(unresolvedInjectionPoint.location(), message(effectiveCandidates))); } } } @@ -63,12 +65,15 @@ public List execute(SpringContextModel model) { return issues; } - private static boolean isResolved(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { + private static boolean isResolved(Set candidates, BeanDefinitionRegistry registry) { return candidates.size() <= 1 - || injectionPointNames.stream().allMatch(name -> matchesCandidate(name, candidates, registry)) || hasExactlyOnePrimaryCandidate(candidates, registry); } + private static Set computeUnmatchingNames(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { + return injectionPointNames.stream().filter(injectionPoint -> matchesCandidate(injectionPoint.name(), candidates, registry)).collect(Collectors.toSet()); + } + private static boolean hasExactlyOnePrimaryCandidate(Set candidates, BeanDefinitionRegistry registry) { return candidates.stream().filter(candidate -> isPrimary(registry, candidate)).count() == 1; } @@ -99,9 +104,9 @@ private static boolean hasProfile(BeanDefinitionRegistry registry, String beanNa return registry.getByName(beanName).stream().anyMatch(bean -> bean.getProfiles() != null); } - private static String message(String requiredType, Set candidates) { + private static String message(Set candidates) { String sortedCandidates = candidates.stream().sorted().collect(Collectors.joining(", ")); - return String.format(MESSAGE, requiredType, sortedCandidates); + return String.format(MESSAGE, sortedCandidates); } } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java index fc8c443b4fb..600cae446c6 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java @@ -82,6 +82,7 @@ public class BeanDefinitionGatherer extends SpringContextModelGatherer { private static final String DEP_SEPARATOR = ","; private static final String DEP_KEY_VALUE_SEPARATOR = ":"; private static final String DEP_NAMES_SEPARATOR = ";"; + private static final String DEP_LOCATION_SEPARATOR = "#"; private static final String TYPE_HIERARCHY_SEPARATOR = ";"; private static final String PRIMARY_ANNOTATION = "org.springframework.context.annotation.Primary"; @@ -101,6 +102,7 @@ private record BeanData( boolean isPrimary, @Nullable String qualifier, Map> dependingBeans, + Map> dependencyInjectionPoints, Set typeHierarchy) { } @@ -129,7 +131,8 @@ public void visitNode(Tree tree) { if (SpringUtils.STEREOTYPE_ANNOTATIONS.stream().anyMatch(meta::isAnnotatedWith)) { String beanName = extractBeanName(meta) .orElseGet(() -> defaultBeanName(classTree.simpleName().name())); - Map> deps = collectAutowiredDependencies(classTree); + Map> injectionPoints = collectAutowiredDependencies(classTree, context.getInputFile()); + Map> deps = toNameMap(injectionPoints); Set typeHierarchy = collectTypeHierarchy(classTree.symbol()); var beanData = new BeanData( beanName, fqn, pkg, @@ -138,6 +141,7 @@ public void visitNode(Tree tree) { meta.isAnnotatedWith(PRIMARY_ANNOTATION), extractQualifier(meta), deps, + injectionPoints, typeHierarchy); collectedBeans.add(beanData); beansCollectedAtFileLevel.add(beanData); @@ -175,11 +179,11 @@ private static void writeToCache(JavaFileScannerContext context, List } private static String serializeBean(BeanData bean) { - var deps = bean.dependingBeans().entrySet().stream() + var deps = bean.dependencyInjectionPoints().entrySet().stream() .map(e -> Base64.getEncoder().encodeToString(e.getKey().getBytes(StandardCharsets.UTF_8)) + DEP_KEY_VALUE_SEPARATOR + e.getValue().stream() - .map(n -> Base64.getEncoder().encodeToString(n.getBytes(StandardCharsets.UTF_8))) + .map(BeanDefinitionGatherer::encodeInjectionPoint) .collect(Collectors.joining(DEP_NAMES_SEPARATOR))) .collect(Collectors.joining(DEP_SEPARATOR)); var typeHierarchy = String.join(TYPE_HIERARCHY_SEPARATOR, bean.typeHierarchy()); @@ -199,6 +203,13 @@ private static String serializeBean(BeanData bean) { typeHierarchy); } + private static String encodeInjectionPoint(TypeToDependenciesIndex.InjectionPoint point) { + var span = point.location().mainLocation(); + return Base64.getEncoder().encodeToString(point.name().getBytes(StandardCharsets.UTF_8)) + + DEP_LOCATION_SEPARATOR + + span.startLine + ":" + span.startCharacter + ":" + span.endLine + ":" + span.endCharacter; + } + @Override public void gatherSpringContextData(ModuleScannerContext context, SpringContextModel springContextModel) { for (BeanData data : collectedBeans) { @@ -215,8 +226,9 @@ public void gatherSpringContextData(ModuleScannerContext context, SpringContextM for (String typeFqn : data.typeHierarchy()) { springContextModel.getTypeToBeanNamesIndex().addBeanForType(typeFqn, data.beanName()); } - data.dependingBeans().forEach((typeFqn, names) -> - names.forEach(name -> springContextModel.getTypeToDependenciesIndex().addBeanForType(typeFqn, name))); + data.dependencyInjectionPoints().forEach((typeFqn, points) -> + points.forEach(point -> springContextModel.getTypeToDependenciesIndex() + .addDependencyForType(typeFqn, point.name(), point.location()))); } } @@ -266,21 +278,34 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { String qualifier = !fields[5].isEmpty() ? new String(Base64.getDecoder().decode(fields[5]), StandardCharsets.UTF_8) : null; - Map> deps = new LinkedHashMap<>(); + Map> injectionPoints = new LinkedHashMap<>(); if (!fields[6].isEmpty()) { for (String entry : fields[6].split(DEP_SEPARATOR)) { int idx = entry.indexOf(DEP_KEY_VALUE_SEPARATOR); String typeFqn = new String(Base64.getDecoder().decode(entry.substring(0, idx)), StandardCharsets.UTF_8); - Set names = Arrays.stream(entry.substring(idx + 1).split(DEP_NAMES_SEPARATOR)) - .map(n -> new String(Base64.getDecoder().decode(n), StandardCharsets.UTF_8)) + Set points = Arrays.stream(entry.substring(idx + 1).split(DEP_NAMES_SEPARATOR)) + .map(token -> decodeInjectionPoint(token, inputFile)) .collect(Collectors.toCollection(LinkedHashSet::new)); - deps.put(typeFqn, names); + injectionPoints.put(typeFqn, points); } } + Map> deps = toNameMap(injectionPoints); Set typeHierarchy = !fields[7].isEmpty() ? new LinkedHashSet<>(List.of(fields[7].split(TYPE_HIERARCHY_SEPARATOR))) : new LinkedHashSet<>(); - return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, qualifier, deps, typeHierarchy); + return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, qualifier, deps, injectionPoints, typeHierarchy); + } + + private static TypeToDependenciesIndex.InjectionPoint decodeInjectionPoint(String token, InputFile inputFile) { + int idx = token.indexOf(DEP_LOCATION_SEPARATOR); + String name = new String(Base64.getDecoder().decode(token.substring(0, idx)), StandardCharsets.UTF_8); + String[] spanParts = token.substring(idx + 1).split(":"); + var span = new AnalyzerMessage.TextSpan( + Integer.parseInt(spanParts[0]), + Integer.parseInt(spanParts[1]), + Integer.parseInt(spanParts[2]), + Integer.parseInt(spanParts[3])); + return new TypeToDependenciesIndex.InjectionPoint(name, new BeanLocation(inputFile, span)); } private static Optional extractBeanName(SymbolMetadata meta) { @@ -329,55 +354,67 @@ private void collectBeanMethod(MethodTree method, String pkg) { ? collectTypeHierarchy(method.returnType().symbolType().symbol()) : Set.of(); - Map> paramDeps = parameterDependencies(method); + var inputFile = context.getInputFile(); + Map> injectionPoints = parameterDependencies(method, inputFile); + Map> paramDeps = toNameMap(injectionPoints); boolean isPrimary = beanMeta.isAnnotatedWith(PRIMARY_ANNOTATION); String qualifier = extractQualifier(beanMeta); var textSpan = AnalyzerMessage.textSpanFor(method.simpleName()); - var inputFile = context.getInputFile(); for (String beanName : beanNames) { - var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, qualifier, paramDeps, typeHierarchy); + var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, qualifier, paramDeps, injectionPoints, typeHierarchy); collectedBeans.add(beanData); beansCollectedAtFileLevel.add(beanData); } } - private static Map> collectAutowiredDependencies(ClassTree classTree) { - Map> deps = new LinkedHashMap<>(); + private static Map> collectAutowiredDependencies(ClassTree classTree, InputFile inputFile) { + Map> deps = new LinkedHashMap<>(); List unannotatedConstructors = new ArrayList<>(); boolean hasAutowiredConstructor = false; for (Tree member : classTree.members()) { if (member instanceof VariableTree field && field.symbol().metadata().isAnnotatedWith(SpringUtils.AUTOWIRED_ANNOTATION)) { String typeFqn = field.symbol().type().fullyQualifiedName(); String name = dependencyKey(field.simpleName().name(), extractQualifier(field.symbol().metadata())); - deps.computeIfAbsent(typeFqn, k -> new LinkedHashSet<>()).add(name); + var location = new BeanLocation(inputFile, AnalyzerMessage.textSpanFor(field.simpleName())); + deps.computeIfAbsent(typeFqn, k -> new LinkedHashSet<>()).add(new TypeToDependenciesIndex.InjectionPoint(name, location)); } else if (member instanceof MethodTree method) { if (method.symbol().metadata().isAnnotatedWith(SpringUtils.AUTOWIRED_ANNOTATION)) { hasAutowiredConstructor |= method.is(Tree.Kind.CONSTRUCTOR); - parameterDependencies(method).forEach((type, names) -> - deps.computeIfAbsent(type, k -> new LinkedHashSet<>()).addAll(names)); + parameterDependencies(method, inputFile).forEach((type, points) -> + deps.computeIfAbsent(type, k -> new LinkedHashSet<>()).addAll(points)); } else if (method.is(Tree.Kind.CONSTRUCTOR)) { unannotatedConstructors.add(method); } } } if (!hasAutowiredConstructor && unannotatedConstructors.size() == 1) { - parameterDependencies(unannotatedConstructors.get(0)).forEach((type, names) -> - deps.computeIfAbsent(type, k -> new LinkedHashSet<>()).addAll(names)); + parameterDependencies(unannotatedConstructors.get(0), inputFile).forEach((type, points) -> + deps.computeIfAbsent(type, k -> new LinkedHashSet<>()).addAll(points)); } return deps; } - private static Map> parameterDependencies(MethodTree method) { - Map> deps = new LinkedHashMap<>(); + private static Map> parameterDependencies(MethodTree method, InputFile inputFile) { + Map> deps = new LinkedHashMap<>(); for (var p : method.parameters()) { String typeFqn = p.symbol().type().fullyQualifiedName(); String name = dependencyKey(p.simpleName().name(), extractQualifier(p.symbol().metadata())); - deps.computeIfAbsent(typeFqn, k -> new LinkedHashSet<>()).add(name); + var location = new BeanLocation(inputFile, AnalyzerMessage.textSpanFor(p.simpleName())); + deps.computeIfAbsent(typeFqn, k -> new LinkedHashSet<>()).add(new TypeToDependenciesIndex.InjectionPoint(name, location)); } return deps; } + private static Map> toNameMap(Map> injectionPointsByType) { + Map> names = new LinkedHashMap<>(); + injectionPointsByType.forEach((typeFqn, points) -> + names.put(typeFqn, points.stream() + .map(TypeToDependenciesIndex.InjectionPoint::name) + .collect(Collectors.toCollection(LinkedHashSet::new)))); + return names; + } + private static String dependencyKey(String fieldOrParamName, @Nullable String qualifier) { return qualifier != null ? qualifier : fieldOrParamName; } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToBeanNamesIndex.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToBeanNamesIndex.java index ac054f4bfe6..9076b1c6662 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToBeanNamesIndex.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToBeanNamesIndex.java @@ -53,4 +53,8 @@ public void addBeanForType(String beanType, String beanName) { public Set getNamesForType(String beanType) { return Collections.unmodifiableSet(beanNamesByType.getOrDefault(beanType, Set.of())); } + + public Set getKeys() { + return beanNamesByType.keySet(); + } } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java index f7efc1150a7..e411beaefa6 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java @@ -16,7 +16,6 @@ */ package org.sonar.java.model.springcontext; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -33,8 +32,11 @@ * the `@Qualifier` annotation if present) */ public class TypeToDependenciesIndex { + + public record InjectionPoint(String name, BeanLocation location) {} + /** Dependencies (name, location) indexed by fully-qualified type. */ - private final Map> beanDependenciesByType = new HashMap<>(); + private final Map> injectionPointsByType = new HashMap<>(); /** * Registers a dependency under the given type. @@ -42,8 +44,9 @@ public class TypeToDependenciesIndex { * @param dependencyType fully-qualified name of the dependency's type * @param dependencyName the dependency name to associate with that type */ - public void addBeanForType(String dependencyType, String dependencyName) { - beanDependenciesByType.computeIfAbsent(dependencyType, k -> new HashSet<>()).add(dependencyName); + public void addDependencyForType(String dependencyType, String dependencyName, BeanLocation location) { + injectionPointsByType.computeIfAbsent(dependencyType, k -> new HashSet<>()) + .add(new InjectionPoint(dependencyName, location)); } /** @@ -52,7 +55,7 @@ public void addBeanForType(String dependencyType, String dependencyName) { * @param dependencyType fully-qualified class name of the dependency's type * @return an unmodifiable set of bean names, or an empty set if none were registered */ - public Set getNamesForType(String dependencyType) { - return Collections.unmodifiableSet(beanDependenciesByType.getOrDefault(dependencyType, Set.of())); + public Set getDependenciesForType(String dependencyType) { + return injectionPointsByType.getOrDefault(dependencyType, Set.of()); } } diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index bb27ebe50ea..d11ed2f2803 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -20,6 +20,7 @@ import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.List; +import java.util.Set; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -40,6 +41,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.tuple; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doThrow; @@ -463,9 +465,10 @@ void scanWithoutParsing_restores_dependencies_with_and_without_qualifier_from_ca String encodedEnvType = Base64.getEncoder().encodeToString("org.springframework.core.env.Environment".getBytes(StandardCharsets.UTF_8)); String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); + // Field declaration lines in QualifiedFieldDependencies.java: applicationContext=14, environment=17. String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false||" - + encodedAppContext + ":" + encodedPrimaryContext - + "," + encodedEnvType + ":" + encodedEnvironment + + encodedAppContext + ":" + encodedPrimaryContext + "#14:2:14:41" + + "," + encodedEnvType + ":" + encodedEnvironment + "#17:2:17:31" + "|checks.spring.context.QualifiedFieldDependencies"; JavaReadCache readCache = mock(JavaReadCache.class); @@ -636,33 +639,86 @@ private static CacheContext mockCacheContext(JavaReadCache readCache, JavaWriteC } // ---- TypeToDependenciesIndex ------------------------------------------------- + @ParameterizedTest(name = "{0}") - @ValueSource(strings = {"src/test/files/springcontext/AutowiredDependencies.java", - "src/test/files/springcontext/AutowiredConstructorDependencies.java" - }) - void regular_dependencies_registered_in_index(String filePath) { + @MethodSource("regularDependencyArguments") + void regular_dependencies_registered_in_index(String filePath, int applicationContextLine, int environmentLine) { scan(filePath); - assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.context.ApplicationContext")).containsOnly("applicationContext"); - assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.core.env.Environment")).containsOnly("environment"); + InputFile inputFile = TestUtils.inputFile(new File(filePath)); + + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.context.ApplicationContext"), + "applicationContext", inputFile, applicationContextLine); + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.core.env.Environment"), + "environment", inputFile, environmentLine); + } + + static Stream regularDependencyArguments() { + return Stream.of( + Arguments.of("src/test/files/springcontext/AutowiredDependencies.java", 12, 15), + Arguments.of("src/test/files/springcontext/AutowiredConstructorDependencies.java", 15, 15) + ); } @Test void dependency_with_qualifier_registered_with_qualifier_value() { scan("src/test/files/springcontext/QualifiedBeanMethodDependencies.java"); - assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.context.ApplicationContext")).containsOnly("primaryContext"); + InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/QualifiedBeanMethodDependencies.java")); + + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.context.ApplicationContext"), + "primaryContext", inputFile, 14); } @Test void dependency_with_qualifier_on_constructor_registered_with_qualifier_value() { scan("src/test/files/springcontext/OrderService.java"); - assertThat(model.getTypeToDependenciesIndex().getNamesForType("PaymentProcessor")).containsOnly("paypal"); + InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/OrderService.java")); + + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("PaymentProcessor"), + "paypal", inputFile, 13); } @Test void dependencies_in_multiple_files_all_registered() { scan("src/test/files/springcontext/OrderService.java", "src/test/files/springcontext/BlankQualifierDependency.java"); - assertThat(model.getTypeToDependenciesIndex().getNamesForType("PaymentProcessor")).containsOnly("paypal"); - assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.context.ApplicationContext")).containsOnly("applicationContext"); + InputFile orderServiceFile = TestUtils.inputFile(new File("src/test/files/springcontext/OrderService.java")); + InputFile blankQualifierFile = TestUtils.inputFile(new File("src/test/files/springcontext/BlankQualifierDependency.java")); + + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("PaymentProcessor"), + "paypal", orderServiceFile, 13); + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.context.ApplicationContext"), + "applicationContext", blankQualifierFile, 13); + } + + @Test + void two_beans_depending_on_same_type_and_name_both_tracked_with_distinct_locations() { + scan("src/test/files/springcontext/AutowiredDependencies.java", "src/test/files/springcontext/AutowiredConstructorDependencies.java"); + InputFile autowiredDependenciesFile = TestUtils.inputFile(new File("src/test/files/springcontext/AutowiredDependencies.java")); + InputFile autowiredConstructorFile = TestUtils.inputFile(new File("src/test/files/springcontext/AutowiredConstructorDependencies.java")); + + var injectionPoints = model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.context.ApplicationContext"); + assertThat(injectionPoints).hasSize(2); + assertThat(injectionPoints).extracting(TypeToDependenciesIndex.InjectionPoint::name).containsOnly("applicationContext"); + // Same type and same name, but two distinct occurrences — each must keep its own location, not collapse into one. + assertThat(injectionPoints) + .extracting(p -> p.location().inputFile(), p -> p.location().mainLocation().startLine) + .containsExactlyInAnyOrder( + tuple(autowiredDependenciesFile, 12), + tuple(autowiredConstructorFile, 15)); + } + + private static void assertInjectionPoint(Set injectionPoints, String expectedName, + InputFile expectedInputFile, int expectedLine) { + assertThat(injectionPoints).hasSize(1); + var point = injectionPoints.iterator().next(); + assertThat(point.name()).isEqualTo(expectedName); + assertThat(point.location().inputFile()).isEqualTo(expectedInputFile); + assertThat(point.location().mainLocation().startLine).isEqualTo(expectedLine); } @Test @@ -674,9 +730,10 @@ void scanWithoutParsing_restores_dependencies_index_from_cache() { String encodedEnvType = Base64.getEncoder().encodeToString("org.springframework.core.env.Environment".getBytes(StandardCharsets.UTF_8)); String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); + // Field declaration lines in QualifiedFieldDependencies.java: applicationContext=14, environment=17. String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false||" - + encodedAppContext + ":" + encodedPrimaryContext - + "," + encodedEnvType + ":" + encodedEnvironment + + encodedAppContext + ":" + encodedPrimaryContext + "#14:2:14:41" + + "," + encodedEnvType + ":" + encodedEnvironment + "#17:2:17:31" + "|checks.spring.context.QualifiedFieldDependencies"; JavaReadCache readCache = mock(JavaReadCache.class); @@ -693,9 +750,11 @@ void scanWithoutParsing_restores_dependencies_index_from_cache() { when(moduleScannerContext.getModuleKey()).thenReturn(""); gatherer.gatherSpringContextData(moduleScannerContext, model); - assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.context.ApplicationContext")) - .containsOnly("primaryContext"); - assertThat(model.getTypeToDependenciesIndex().getNamesForType("org.springframework.core.env.Environment")) - .containsOnly("environment"); + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.context.ApplicationContext"), + "primaryContext", inputFile, 14); + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.core.env.Environment"), + "environment", inputFile, 17); } } From d4085cba4fd89704f66ee6cc2e1e8080f0a7c57f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Mon, 31 Aug 2026 16:19:04 +0200 Subject: [PATCH 14/22] Fix incorrect condition --- .../org/sonar/java/checks/spring/AmbiguousDependencyCheck.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index 66ddb073609..827ec97141a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -71,7 +71,7 @@ private static boolean isResolved(Set candidates, BeanDefinitionRegistry } private static Set computeUnmatchingNames(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { - return injectionPointNames.stream().filter(injectionPoint -> matchesCandidate(injectionPoint.name(), candidates, registry)).collect(Collectors.toSet()); + return injectionPointNames.stream().filter(injectionPoint -> !matchesCandidate(injectionPoint.name(), candidates, registry)).collect(Collectors.toSet()); } private static boolean hasExactlyOnePrimaryCandidate(Set candidates, BeanDefinitionRegistry registry) { From 2787db5ab868519d411c9c73c2e069c6f0280b74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Mon, 31 Aug 2026 17:04:06 +0200 Subject: [PATCH 15/22] Fix test expectations --- .../java/org/sonar/plugins/java/GeneratedCheckListTest.java | 4 +++- .../org/sonar/plugins/java/SpringContextModelSensorTest.java | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/GeneratedCheckListTest.java b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/GeneratedCheckListTest.java index 039b44a26b5..8e2fb4a01cf 100644 --- a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/GeneratedCheckListTest.java +++ b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/GeneratedCheckListTest.java @@ -49,7 +49,8 @@ class GeneratedCheckListTest { private static final Set BLACK_LIST = SetUtils.immutableSetOf( "AbstractXPathBasedCheck.java", "AbstractWebXmlXPathBasedCheck.java", - "AbstractRegexCheck.java"); + "AbstractRegexCheck.java", + "SpringContextCheck.java"); /** * Enforces that each check declared in list. @@ -156,6 +157,7 @@ void enforce_CheckList_registration() { files.stream() .filter(file -> file.getName().endsWith("Check.java")) .filter(file -> !file.getName().startsWith("Abstract")) + .filter(file -> !BLACK_LIST.contains(file.getName())) .map(File::getAbsolutePath) .map(f -> f.replace(File.separatorChar, '.')) .map(f -> f.substring(f.indexOf("org.sonar.java.checks"), f.length() - 5)) diff --git a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java index e23f634f960..db1fb328c29 100644 --- a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java +++ b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java @@ -66,9 +66,9 @@ void reports_an_issue_for_an_ambiguous_dependency() { Issue issue = context.allIssues().iterator().next(); assertThat(issue.ruleKey()).isEqualTo(RuleKey.of("java", "S9352")); assertThat(issue.primaryLocation().message()) - .isEqualTo("Multiple beans of type \"org.springframework.context.ApplicationContextAware\" match this dependency" + .isEqualTo("Multiple beans match this dependency" + " (componentOne, componentTwo); disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\"."); - assertThat(issue.primaryLocation().textRange().start().line()).isEqualTo(10); + assertThat(issue.primaryLocation().textRange().start().line()).isEqualTo(13); } @Test From e428ced1bf69172eee4b9ea077833c830f58ca0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Tue, 1 Sep 2026 08:29:46 +0200 Subject: [PATCH 16/22] Fix FN when there are multiple matching candidates --- .../checks/spring/AmbiguousDependencyCheck.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index 827ec97141a..0628df64ad7 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -56,7 +56,7 @@ public List execute(SpringContextModel model) { // excluding all beans with a configured profile, no matter what the profile is, to avoid FPs Set effectiveCandidates = excludeCandidatesWithProfile(candidates, registry); if (effectiveCandidates.size() > 1) { - for (InjectionPoint unresolvedInjectionPoint : computeUnmatchingNames(effectiveCandidates, injectionPoints, registry)) { + for (InjectionPoint unresolvedInjectionPoint : computeUnresolvedInjectionPoints(effectiveCandidates, injectionPoints, registry)) { issues.add(new SpringContextIssue(unresolvedInjectionPoint.location(), message(effectiveCandidates))); } } @@ -70,17 +70,19 @@ private static boolean isResolved(Set candidates, BeanDefinitionRegistry || hasExactlyOnePrimaryCandidate(candidates, registry); } - private static Set computeUnmatchingNames(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { - return injectionPointNames.stream().filter(injectionPoint -> !matchesCandidate(injectionPoint.name(), candidates, registry)).collect(Collectors.toSet()); + private static Set computeUnresolvedInjectionPoints(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { + return injectionPointNames.stream().filter(injectionPoint -> !hasExactlyOneMatchingCandidate(injectionPoint.name(), candidates, registry)).collect(Collectors.toSet()); } private static boolean hasExactlyOnePrimaryCandidate(Set candidates, BeanDefinitionRegistry registry) { return candidates.stream().filter(candidate -> isPrimary(registry, candidate)).count() == 1; } - private static boolean matchesCandidate(String injectionPointName, Set candidates, BeanDefinitionRegistry registry) { - return candidates.contains(injectionPointName) - || candidates.stream().anyMatch(candidate -> injectionPointName.equals(qualifierOf(registry, candidate))); + private static boolean hasExactlyOneMatchingCandidate(String injectionPointName, Set candidates, BeanDefinitionRegistry registry) { + long matchCount = candidates.stream() + .filter(candidate -> candidate.equals(injectionPointName) || injectionPointName.equals(qualifierOf(registry, candidate))) + .count(); + return matchCount == 1; } @Nullable From 3352b3afc46277a7f11cac0c5ed3d431f32c03f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Tue, 1 Sep 2026 08:50:30 +0200 Subject: [PATCH 17/22] Remove `@Qualifier` annotation on the bean definition --- .../s9352/QualifierOnBeanComponentA.java | 19 --------------- .../s9352/QualifierOnBeanComponentB.java | 14 ----------- .../spring/s9352/QualifierOnBeanConsumer.java | 16 ------------- .../spring/AmbiguousDependencyCheck.java | 24 +++---------------- .../spring/AmbiguousDependencyCheckTest.java | 6 ----- .../springcontext/BeanDefinitionGatherer.java | 24 +++++-------------- .../springcontext/BeanDefinitionHolder.java | 21 ---------------- .../BeanDefinitionGathererTest.java | 8 +++---- 8 files changed, 13 insertions(+), 119 deletions(-) delete mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentA.java delete mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentB.java delete mode 100644 java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanConsumer.java diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentA.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentA.java deleted file mode 100644 index 13bf9ecbd0c..00000000000 --- a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentA.java +++ /dev/null @@ -1,19 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.beans.factory.BeanClassLoaderAware; -import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.stereotype.Component; - -// Scenario: @Qualifier declared on the bean itself (not on an injection point) resolves ambiguity, no issue -// expected. Two candidates of type BeanClassLoaderAware (this class and QualifierOnBeanComponentB), used only -// by QualifierOnBeanConsumer in this scenario. A distinct interface from the other scenarios in this package, -// so that a whole-module scan does not merge candidate pools across scenarios. -@Qualifier("main") -@Component -public class QualifierOnBeanComponentA implements BeanClassLoaderAware { - - @Override - public void setBeanClassLoader(ClassLoader classLoader) { - // not needed for test - } -} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentB.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentB.java deleted file mode 100644 index 72762b310be..00000000000 --- a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanComponentB.java +++ /dev/null @@ -1,14 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.beans.factory.BeanClassLoaderAware; -import org.springframework.stereotype.Component; - -// See QualifierOnBeanComponentA for context. -@Component -public class QualifierOnBeanComponentB implements BeanClassLoaderAware { - - @Override - public void setBeanClassLoader(ClassLoader classLoader) { - // not needed for test - } -} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanConsumer.java b/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanConsumer.java deleted file mode 100644 index 538efea5926..00000000000 --- a/java-checks-test-sources/default/src/main/java/checks/spring/s9352/QualifierOnBeanConsumer.java +++ /dev/null @@ -1,16 +0,0 @@ -package checks.spring.s9352; - -import org.springframework.beans.factory.BeanClassLoaderAware; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.stereotype.Service; - -// "main" matches QualifierOnBeanComponentA's own declared @Qualifier value, not its bean name: still resolved, -// no issue expected. See QualifierOnBeanComponentA for context. -@Service -public class QualifierOnBeanConsumer { - - @Autowired - @Qualifier("main") - private BeanClassLoaderAware contextAware; -} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index 0628df64ad7..74a38247cb6 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -18,10 +18,8 @@ import java.util.ArrayList; import java.util.List; -import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; -import javax.annotation.Nullable; import org.sonar.check.Rule; import org.sonar.java.model.springcontext.BeanDefinitionHolder; import org.sonar.java.model.springcontext.BeanDefinitionRegistry; @@ -56,7 +54,7 @@ public List execute(SpringContextModel model) { // excluding all beans with a configured profile, no matter what the profile is, to avoid FPs Set effectiveCandidates = excludeCandidatesWithProfile(candidates, registry); if (effectiveCandidates.size() > 1) { - for (InjectionPoint unresolvedInjectionPoint : computeUnresolvedInjectionPoints(effectiveCandidates, injectionPoints, registry)) { + for (InjectionPoint unresolvedInjectionPoint : computeUnresolvedInjectionPoints(effectiveCandidates, injectionPoints)) { issues.add(new SpringContextIssue(unresolvedInjectionPoint.location(), message(effectiveCandidates))); } } @@ -70,30 +68,14 @@ private static boolean isResolved(Set candidates, BeanDefinitionRegistry || hasExactlyOnePrimaryCandidate(candidates, registry); } - private static Set computeUnresolvedInjectionPoints(Set candidates, Set injectionPointNames, BeanDefinitionRegistry registry) { - return injectionPointNames.stream().filter(injectionPoint -> !hasExactlyOneMatchingCandidate(injectionPoint.name(), candidates, registry)).collect(Collectors.toSet()); + private static Set computeUnresolvedInjectionPoints(Set candidates, Set injectionPointNames) { + return injectionPointNames.stream().filter(injectionPoint -> !candidates.contains(injectionPoint.name())).collect(Collectors.toSet()); } private static boolean hasExactlyOnePrimaryCandidate(Set candidates, BeanDefinitionRegistry registry) { return candidates.stream().filter(candidate -> isPrimary(registry, candidate)).count() == 1; } - private static boolean hasExactlyOneMatchingCandidate(String injectionPointName, Set candidates, BeanDefinitionRegistry registry) { - long matchCount = candidates.stream() - .filter(candidate -> candidate.equals(injectionPointName) || injectionPointName.equals(qualifierOf(registry, candidate))) - .count(); - return matchCount == 1; - } - - @Nullable - private static String qualifierOf(BeanDefinitionRegistry registry, String beanName) { - return registry.getByName(beanName).stream() - .map(BeanDefinitionHolder::getQualifier) - .filter(Objects::nonNull) - .findFirst() - .orElse(null); - } - private static Set excludeCandidatesWithProfile(Set candidates, BeanDefinitionRegistry registry) { return candidates.stream().filter(candidate -> !hasProfile(registry, candidate)).collect(Collectors.toUnmodifiableSet()); } diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java index 74d072325be..80ed5a43634 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java @@ -88,12 +88,6 @@ void one_resolved_injection_point_does_not_hide_another_ambiguous_one_of_the_sam assertThat(check.execute(model)).hasSize(1); } - @Test - void qualifier_declared_on_the_bean_itself_resolves_ambiguity() { - SpringContextModel model = buildModel("QualifierOnBeanComponentA.java", "QualifierOnBeanComponentB.java", "QualifierOnBeanConsumer.java"); - assertThat(check.execute(model)).isEmpty(); - } - /** * Runs {@link BeanDefinitionGatherer} over the given files (relative to {@link #BASE_PATH} under * {@code src/main/java}) into a single, freshly built {@link SpringContextModel}, mirroring how diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java index 600cae446c6..b064f028537 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java @@ -61,7 +61,6 @@ *

        Also captures: *

          *
        • {@code @Primary} designation
          • - *
        • {@code @Qualifier} value declared on the bean itself (as opposed to on an injection point)
          • *
        • Dependencies via {@code @Autowired} fields, constructors, and setters for class-level beans
          • *
        • Dependencies via method parameters for {@code @Bean} method beans
          • *
        • Implicit single-constructor injection (no {@code @Autowired} required)
          • @@ -100,7 +99,6 @@ private record BeanData( InputFile inputFile, AnalyzerMessage.TextSpan textSpan, boolean isPrimary, - @Nullable String qualifier, Map> dependingBeans, Map> dependencyInjectionPoints, Set typeHierarchy) { @@ -139,7 +137,6 @@ public void visitNode(Tree tree) { context.getInputFile(), AnalyzerMessage.textSpanFor(classTree.simpleName()), meta.isAnnotatedWith(PRIMARY_ANNOTATION), - extractQualifier(meta), deps, injectionPoints, typeHierarchy); @@ -189,16 +186,12 @@ private static String serializeBean(BeanData bean) { var typeHierarchy = String.join(TYPE_HIERARCHY_SEPARATOR, bean.typeHierarchy()); var span = bean.textSpan(); var encodedName = Base64.getEncoder().encodeToString(bean.beanName().getBytes(StandardCharsets.UTF_8)); - var encodedQualifier = bean.qualifier() != null - ? Base64.getEncoder().encodeToString(bean.qualifier().getBytes(StandardCharsets.UTF_8)) - : ""; return String.join(FIELD_SEPARATOR, encodedName, bean.type(), bean.beanPackage(), span.startLine + ":" + span.startCharacter + ":" + span.endLine + ":" + span.endCharacter, Boolean.toString(bean.isPrimary()), - encodedQualifier, deps, typeHierarchy); } @@ -220,7 +213,6 @@ public void gatherSpringContextData(ModuleScannerContext context, SpringContextM if (data.isPrimary()) { holderBuilder.primary(); } - holderBuilder.qualifier(data.qualifier()); springContextModel.getBeanDefinitionRegistry() .addBeanDefinition(data.beanName(), holderBuilder.build()); for (String typeFqn : data.typeHierarchy()) { @@ -275,12 +267,9 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { Integer.parseInt(spanParts[2]), Integer.parseInt(spanParts[3])); boolean isPrimary = Boolean.parseBoolean(fields[4]); - String qualifier = !fields[5].isEmpty() - ? new String(Base64.getDecoder().decode(fields[5]), StandardCharsets.UTF_8) - : null; Map> injectionPoints = new LinkedHashMap<>(); - if (!fields[6].isEmpty()) { - for (String entry : fields[6].split(DEP_SEPARATOR)) { + if (!fields[5].isEmpty()) { + for (String entry : fields[5].split(DEP_SEPARATOR)) { int idx = entry.indexOf(DEP_KEY_VALUE_SEPARATOR); String typeFqn = new String(Base64.getDecoder().decode(entry.substring(0, idx)), StandardCharsets.UTF_8); Set points = Arrays.stream(entry.substring(idx + 1).split(DEP_NAMES_SEPARATOR)) @@ -290,10 +279,10 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { } } Map> deps = toNameMap(injectionPoints); - Set typeHierarchy = !fields[7].isEmpty() - ? new LinkedHashSet<>(List.of(fields[7].split(TYPE_HIERARCHY_SEPARATOR))) + Set typeHierarchy = !fields[6].isEmpty() + ? new LinkedHashSet<>(List.of(fields[6].split(TYPE_HIERARCHY_SEPARATOR))) : new LinkedHashSet<>(); - return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, qualifier, deps, injectionPoints, typeHierarchy); + return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, deps, injectionPoints, typeHierarchy); } private static TypeToDependenciesIndex.InjectionPoint decodeInjectionPoint(String token, InputFile inputFile) { @@ -358,11 +347,10 @@ private void collectBeanMethod(MethodTree method, String pkg) { Map> injectionPoints = parameterDependencies(method, inputFile); Map> paramDeps = toNameMap(injectionPoints); boolean isPrimary = beanMeta.isAnnotatedWith(PRIMARY_ANNOTATION); - String qualifier = extractQualifier(beanMeta); var textSpan = AnalyzerMessage.textSpanFor(method.simpleName()); for (String beanName : beanNames) { - var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, qualifier, paramDeps, injectionPoints, typeHierarchy); + var beanData = new BeanData(beanName, returnTypeFqn, pkg, inputFile, textSpan, isPrimary, paramDeps, injectionPoints, typeHierarchy); collectedBeans.add(beanData); beansCollectedAtFileLevel.add(beanData); } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java index 9095a290aae..212aab4f0dd 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java @@ -66,10 +66,6 @@ public class BeanDefinitionHolder { /** Whether the bean is marked as {@code @Primary}, making it the preferred candidate for autowiring. */ private boolean isPrimary = false; - /** Value of the {@code @Qualifier} annotation declared on the bean itself, or {@code null} if absent. */ - @Nullable - private String qualifier; - private BeanDefinitionHolder(String type, String module, String beanPackage, BeanLocation location) { this.type = type; this.module = module; @@ -89,10 +85,6 @@ private void setPrimary() { this.isPrimary = true; } - private void setQualifier(@Nullable String qualifier) { - this.qualifier = qualifier; - } - public String getType() { return type; } @@ -122,11 +114,6 @@ public boolean isPrimary() { return isPrimary; } - @Nullable - public String getQualifier() { - return qualifier; - } - public static class Builder { private final String type; private final String module; @@ -136,8 +123,6 @@ public static class Builder { @Nullable private String profiles; private boolean isPrimary = false; - @Nullable - private String qualifier; public Builder(String type, String module, String beanPackage, BeanLocation location) { this.type = type; @@ -161,17 +146,11 @@ public Builder primary() { return this; } - public Builder qualifier(@Nullable String qualifier) { - this.qualifier = qualifier; - return this; - } - public BeanDefinitionHolder build() { BeanDefinitionHolder holder = new BeanDefinitionHolder(type, module, beanPackage, location); holder.setDependingBeans(dependingBeans.entrySet().stream() .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, e -> Set.copyOf(e.getValue())))); holder.setProfiles(profiles); - holder.setQualifier(qualifier); if (isPrimary) { holder.setPrimary(); } diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index d11ed2f2803..eb592aa1bcf 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -345,7 +345,7 @@ void scanWithoutParsing_returns_true_and_restores_beans_on_cache_hit() { InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/SimpleComponent.java")); String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); String encodedName = Base64.getEncoder().encodeToString("simpleComponent".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.SimpleComponent|checks.spring.context|6:6:6:21|false|||checks.spring.context.SimpleComponent"; + String serialized = encodedName + "|checks.spring.context.SimpleComponent|checks.spring.context|6:6:6:21|false||checks.spring.context.SimpleComponent"; JavaReadCache readCache = mock(JavaReadCache.class); when(readCache.readBytes(cacheKey)).thenReturn(serialized.getBytes(StandardCharsets.UTF_8)); @@ -466,7 +466,7 @@ void scanWithoutParsing_restores_dependencies_with_and_without_qualifier_from_ca String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); // Field declaration lines in QualifiedFieldDependencies.java: applicationContext=14, environment=17. - String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false||" + String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|" + encodedAppContext + ":" + encodedPrimaryContext + "#14:2:14:41" + "," + encodedEnvType + ":" + encodedEnvironment + "#17:2:17:31" + "|checks.spring.context.QualifiedFieldDependencies"; @@ -540,7 +540,7 @@ void scanWithoutParsing_restores_full_type_hierarchy_from_cache() { InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/ComponentImplementingInterface.java")); String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); String encodedName = Base64.getEncoder().encodeToString("componentImplementingInterface".getBytes(StandardCharsets.UTF_8)); - String serialized = encodedName + "|checks.spring.context.ComponentImplementingInterface|checks.spring.context|8:6:8:36|false||" + String serialized = encodedName + "|checks.spring.context.ComponentImplementingInterface|checks.spring.context|8:6:8:36|false|" + "|checks.spring.context.ComponentImplementingInterface" + ";org.springframework.context.ApplicationContextAware" + ";org.springframework.beans.factory.Aware"; @@ -731,7 +731,7 @@ void scanWithoutParsing_restores_dependencies_index_from_cache() { String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); // Field declaration lines in QualifiedFieldDependencies.java: applicationContext=14, environment=17. - String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false||" + String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|" + encodedAppContext + ":" + encodedPrimaryContext + "#14:2:14:41" + "," + encodedEnvType + ":" + encodedEnvironment + "#17:2:17:31" + "|checks.spring.context.QualifiedFieldDependencies"; From dfcb7336c10d22749a4685ccee5bcde1f18de41c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Tue, 1 Sep 2026 09:59:40 +0200 Subject: [PATCH 18/22] Add cross-module tests --- .../AmbiguousDependencyCrossModuleTest.java | 61 +++++++++++++++++++ .../app/pom.xml | 46 ++++++++++++++ .../java/com/example/app/CacheConsumer.java | 13 ++++ .../java/com/example/app/CarrierConsumer.java | 16 +++++ .../com/example/app/DiscountConsumer.java | 19 ++++++ .../com/example/app/InventoryConsumer.java | 13 ++++ .../com/example/app/NotificationConsumer.java | 17 ++++++ .../app/NotificationQualifiedConsumer.java | 15 +++++ .../java/com/example/app/PaymentConsumer.java | 16 +++++ .../java/com/example/app/PricingConsumer.java | 13 ++++ .../com/example/app/ReportingConsumer.java | 17 ++++++ .../example/app/SpringExampleApplication.java | 11 ++++ .../module-a/pom.xml | 27 ++++++++ .../java/com/example/modulea/CacheConfig.java | 24 ++++++++ .../modulea/CreditCardPaymentGateway.java | 12 ++++ .../modulea/EmailNotificationService.java | 12 ++++ .../modulea/ExcelReportingService.java | 14 +++++ .../example/modulea/PdfReportingService.java | 14 +++++ .../com/example/modulea/PrimaryCarrier.java | 14 +++++ .../modulea/PrimaryPricingService.java | 14 +++++ .../modulea/SmsNotificationService.java | 12 ++++ .../modulea/StandardDiscountService.java | 12 ++++ .../modulea/StandardPricingService.java | 12 ++++ .../modulea/WarehouseInventoryConfig.java | 14 +++++ .../module-b/pom.xml | 27 ++++++++ .../moduleb/DigitalWalletPaymentGateway.java | 12 ++++ .../moduleb/PremiumDiscountService.java | 12 ++++ .../com/example/moduleb/SecondaryCarrier.java | 12 ++++ .../example/moduleb/StoreInventoryConfig.java | 14 +++++ .../module-common/pom.xml | 15 +++++ .../com/example/common/CacheProvider.java | 5 ++ .../com/example/common/DiscountService.java | 5 ++ .../com/example/common/InventoryService.java | 5 ++ .../example/common/NotificationService.java | 5 ++ .../com/example/common/PaymentGateway.java | 5 ++ .../com/example/common/PricingService.java | 5 ++ .../com/example/common/ReportingService.java | 5 ++ .../com/example/common/ShippingCarrier.java | 5 ++ .../pom.xml | 29 +++++++++ 39 files changed, 599 insertions(+) create mode 100644 its/scanner-integration-tests/src/test/java/org/sonar/java/it/spring/AmbiguousDependencyCrossModuleTest.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/CacheConsumer.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/CarrierConsumer.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/DiscountConsumer.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/InventoryConsumer.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/NotificationConsumer.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/NotificationQualifiedConsumer.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/PaymentConsumer.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/PricingConsumer.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/ReportingConsumer.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/SpringExampleApplication.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/pom.xml create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/CacheConfig.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/CreditCardPaymentGateway.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/EmailNotificationService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/ExcelReportingService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PdfReportingService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PrimaryCarrier.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PrimaryPricingService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/SmsNotificationService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/StandardDiscountService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/StandardPricingService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/WarehouseInventoryConfig.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/pom.xml create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/DigitalWalletPaymentGateway.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/PremiumDiscountService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/SecondaryCarrier.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/StoreInventoryConfig.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/pom.xml create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/CacheProvider.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/DiscountService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/InventoryService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/NotificationService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/PaymentGateway.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/PricingService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/ReportingService.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/ShippingCarrier.java create mode 100644 its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/pom.xml diff --git a/its/scanner-integration-tests/src/test/java/org/sonar/java/it/spring/AmbiguousDependencyCrossModuleTest.java b/its/scanner-integration-tests/src/test/java/org/sonar/java/it/spring/AmbiguousDependencyCrossModuleTest.java new file mode 100644 index 00000000000..417a914bedb --- /dev/null +++ b/its/scanner-integration-tests/src/test/java/org/sonar/java/it/spring/AmbiguousDependencyCrossModuleTest.java @@ -0,0 +1,61 @@ +/* + * 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.it.spring; + +import com.sonarsource.scanner.integrationtester.dsl.issue.TextRange; +import com.sonarsource.scanner.integrationtester.dsl.issue.TextRangeIssue; +import org.junit.jupiter.api.Test; +import org.sonar.java.it.ScannerIntegrationAbstractTest; + +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; + +class AmbiguousDependencyCrossModuleTest extends ScannerIntegrationAbstractTest { + + @Test + void test() { + var issues = analyze(Path.of("ambiguous-dependencies-should-be-resolved"), "S9352"); + assertThat(issues) + .hasSize(5) + .contains(new TextRangeIssue( + "app/src/main/java/com/example/app/ReportingConsumer.java", + "java:S9352", + "Multiple beans match this dependency (excelReportingService, pdfReportingService); disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\".", + new TextRange(16, 16, 29, 45)), + new TextRangeIssue( + "app/src/main/java/com/example/app/CacheConsumer.java", + "java:S9352", + "Multiple beans match this dependency (diskCacheProvider, inMemoryCacheProvider, redisCacheProvider); disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\".", + new TextRange(12, 12, 26, 39)), + new TextRangeIssue( + "app/src/main/java/com/example/app/PaymentConsumer.java", + "java:S9352", + "Multiple beans match this dependency (creditCardPaymentGateway, digitalWalletPaymentGateway); disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\".", + new TextRange(15, 15, 27, 41)), + new TextRangeIssue( + "app/src/main/java/com/example/app/NotificationConsumer.java", + "java:S9352", + "Multiple beans match this dependency (emailNotificationService, smsNotificationService); disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\".", + new TextRange(16, 16, 32, 51)), + new TextRangeIssue( + "app/src/main/java/com/example/app/InventoryConsumer.java", + "java:S9352", + "Multiple beans match this dependency (storeInventoryService, warehouseInventoryService); disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\".", + new TextRange(12, 12, 29, 45))); + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml new file mode 100644 index 00000000000..37a0d89115d --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml @@ -0,0 +1,46 @@ + + + 4.0.0 + + + com.example + ambiguous-dependencies-should-be-resolved + 0.0.1-SNAPSHOT + + + app + jar + + + + com.example + module-common + ${project.version} + + + com.example + module-a + ${project.version} + + + com.example + module-b + ${project.version} + + + org.springframework.boot + spring-boot-starter + + + + + + + org.springframework.boot + spring-boot-maven-plugin + + + + diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/CacheConsumer.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/CacheConsumer.java new file mode 100644 index 00000000000..89531bd6194 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/CacheConsumer.java @@ -0,0 +1,13 @@ +package com.example.app; + +import com.example.common.CacheProvider; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +// CASE: ambiguity between three @Bean-method-declared beans (not @Component), same module. +@Component +public class CacheConsumer { + + @Autowired + private CacheProvider cacheProvider; +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/CarrierConsumer.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/CarrierConsumer.java new file mode 100644 index 00000000000..47655444a99 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/CarrierConsumer.java @@ -0,0 +1,16 @@ +package com.example.app; + +import com.example.common.ShippingCarrier; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +/** + * CASE: ambiguity resolved via @Primary, cross-module. + * PrimaryCarrier (@Primary) lives in module-a, SecondaryCarrier in module-b. + */ +@Component +public class CarrierConsumer { + + @Autowired + private ShippingCarrier shippingCarrier; +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/DiscountConsumer.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/DiscountConsumer.java new file mode 100644 index 00000000000..9989448f663 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/DiscountConsumer.java @@ -0,0 +1,19 @@ +package com.example.app; + +import com.example.common.DiscountService; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.stereotype.Component; + +/** + * CASE: ambiguity resolved via @Qualifier on a constructor parameter, cross-module. + * StandardDiscountService lives in module-a, PremiumDiscountService in module-b. + */ +@Component +public class DiscountConsumer { + + private final DiscountService discountService; + + public DiscountConsumer(@Qualifier("premiumDiscountService") DiscountService discountService) { + this.discountService = discountService; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/InventoryConsumer.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/InventoryConsumer.java new file mode 100644 index 00000000000..f384473a19a --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/InventoryConsumer.java @@ -0,0 +1,13 @@ +package com.example.app; + +import com.example.common.InventoryService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +// CASE: ambiguity between two @Bean-method-declared beans, each in its own @Configuration class, cross-module. +@Component +public class InventoryConsumer { + + @Autowired + private InventoryService inventoryService; +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/NotificationConsumer.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/NotificationConsumer.java new file mode 100644 index 00000000000..d8f59aff1c9 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/NotificationConsumer.java @@ -0,0 +1,17 @@ +package com.example.app; + +import com.example.common.NotificationService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +/** + * CASE: unresolved ambiguity, same module. + * EmailNotificationService and SmsNotificationService (module-a) are both + * plain beans with no @Primary/@Qualifier - ambiguous. + */ +@Component +public class NotificationConsumer { + + @Autowired + private NotificationService notificationService; +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/NotificationQualifiedConsumer.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/NotificationQualifiedConsumer.java new file mode 100644 index 00000000000..9aff5bf9983 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/NotificationQualifiedConsumer.java @@ -0,0 +1,15 @@ +package com.example.app; + +import com.example.common.NotificationService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.stereotype.Component; + +// CASE: ambiguity resolved via @Qualifier, same module. +@Component +public class NotificationQualifiedConsumer { + + @Autowired + @Qualifier("smsNotificationService") + private NotificationService notificationService; +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/PaymentConsumer.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/PaymentConsumer.java new file mode 100644 index 00000000000..0d4f7b40638 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/PaymentConsumer.java @@ -0,0 +1,16 @@ +package com.example.app; + +import com.example.common.PaymentGateway; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +/** + * CASE: unresolved ambiguity, cross-module. + * CreditCardPaymentGateway lives in module-a, DigitalWalletPaymentGateway in module-b. + */ +@Component +public class PaymentConsumer { + + @Autowired + private PaymentGateway paymentGateway; +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/PricingConsumer.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/PricingConsumer.java new file mode 100644 index 00000000000..8890d4567db --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/PricingConsumer.java @@ -0,0 +1,13 @@ +package com.example.app; + +import com.example.common.PricingService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +// CASE: ambiguity resolved via @Primary, same module. +@Component +public class PricingConsumer { + + @Autowired + private PricingService pricingService; +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/ReportingConsumer.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/ReportingConsumer.java new file mode 100644 index 00000000000..63b13c6cd0d --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/ReportingConsumer.java @@ -0,0 +1,17 @@ +package com.example.app; + +import com.example.common.ReportingService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +/** + * CASE: two beans both marked @Primary, same module - still ambiguous. + * Having more than one "primary" candidate is itself unresolved: Spring + * throws NoUniqueBeanDefinitionException rather than picking either one. + */ +@Component +public class ReportingConsumer { + + @Autowired + private ReportingService reportingService; +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/SpringExampleApplication.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/SpringExampleApplication.java new file mode 100644 index 00000000000..95b3e1b0e67 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/src/main/java/com/example/app/SpringExampleApplication.java @@ -0,0 +1,11 @@ +package com.example.app; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; + +@SpringBootApplication(scanBasePackages = "com.example") +public class SpringExampleApplication { + public static void main(String[] args) { + SpringApplication.run(SpringExampleApplication.class, args); + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/pom.xml b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/pom.xml new file mode 100644 index 00000000000..d26b573e7cf --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/pom.xml @@ -0,0 +1,27 @@ + + + 4.0.0 + + + com.example + ambiguous-dependencies-should-be-resolved + 0.0.1-SNAPSHOT + + + module-a + jar + + + + com.example + module-common + ${project.version} + + + org.springframework + spring-context + + + diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/CacheConfig.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/CacheConfig.java new file mode 100644 index 00000000000..69951dbf93a --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/CacheConfig.java @@ -0,0 +1,24 @@ +package com.example.modulea; + +import com.example.common.CacheProvider; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class CacheConfig { + + @Bean + public CacheProvider redisCacheProvider() { + return key -> "redis:" + key; + } + + @Bean + public CacheProvider inMemoryCacheProvider() { + return key -> "memory:" + key; + } + + @Bean + public CacheProvider diskCacheProvider() { + return key -> "disk:" + key; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/CreditCardPaymentGateway.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/CreditCardPaymentGateway.java new file mode 100644 index 00000000000..4eda3bd158b --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/CreditCardPaymentGateway.java @@ -0,0 +1,12 @@ +package com.example.modulea; + +import com.example.common.PaymentGateway; +import org.springframework.stereotype.Component; + +@Component +public class CreditCardPaymentGateway implements PaymentGateway { + @Override + public void charge(double amount) { + System.out.println("Charging credit card: " + amount); + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/EmailNotificationService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/EmailNotificationService.java new file mode 100644 index 00000000000..42cebeaf28f --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/EmailNotificationService.java @@ -0,0 +1,12 @@ +package com.example.modulea; + +import com.example.common.NotificationService; +import org.springframework.stereotype.Component; + +@Component +public class EmailNotificationService implements NotificationService { + @Override + public void send(String message) { + System.out.println("Email: " + message); + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/ExcelReportingService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/ExcelReportingService.java new file mode 100644 index 00000000000..20096c9cff2 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/ExcelReportingService.java @@ -0,0 +1,14 @@ +package com.example.modulea; + +import com.example.common.ReportingService; +import org.springframework.context.annotation.Primary; +import org.springframework.stereotype.Component; + +@Primary +@Component +public class ExcelReportingService implements ReportingService { + @Override + public String generate() { + return "excel-report"; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PdfReportingService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PdfReportingService.java new file mode 100644 index 00000000000..53da1fd09eb --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PdfReportingService.java @@ -0,0 +1,14 @@ +package com.example.modulea; + +import com.example.common.ReportingService; +import org.springframework.context.annotation.Primary; +import org.springframework.stereotype.Component; + +@Primary +@Component +public class PdfReportingService implements ReportingService { + @Override + public String generate() { + return "pdf-report"; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PrimaryCarrier.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PrimaryCarrier.java new file mode 100644 index 00000000000..ab4fa84e01b --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PrimaryCarrier.java @@ -0,0 +1,14 @@ +package com.example.modulea; + +import com.example.common.ShippingCarrier; +import org.springframework.context.annotation.Primary; +import org.springframework.stereotype.Component; + +@Primary +@Component +public class PrimaryCarrier implements ShippingCarrier { + @Override + public String track(String trackingId) { + return "primary:" + trackingId; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PrimaryPricingService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PrimaryPricingService.java new file mode 100644 index 00000000000..b8b6faf9811 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/PrimaryPricingService.java @@ -0,0 +1,14 @@ +package com.example.modulea; + +import com.example.common.PricingService; +import org.springframework.context.annotation.Primary; +import org.springframework.stereotype.Component; + +@Primary +@Component +public class PrimaryPricingService implements PricingService { + @Override + public double getPrice() { + return 9.99; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/SmsNotificationService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/SmsNotificationService.java new file mode 100644 index 00000000000..385c4b69aa0 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/SmsNotificationService.java @@ -0,0 +1,12 @@ +package com.example.modulea; + +import com.example.common.NotificationService; +import org.springframework.stereotype.Component; + +@Component +public class SmsNotificationService implements NotificationService { + @Override + public void send(String message) { + System.out.println("SMS: " + message); + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/StandardDiscountService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/StandardDiscountService.java new file mode 100644 index 00000000000..cd421125985 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/StandardDiscountService.java @@ -0,0 +1,12 @@ +package com.example.modulea; + +import com.example.common.DiscountService; +import org.springframework.stereotype.Component; + +@Component +public class StandardDiscountService implements DiscountService { + @Override + public double applyDiscount(double amount) { + return amount * 0.95; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/StandardPricingService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/StandardPricingService.java new file mode 100644 index 00000000000..0908e0d3b76 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/StandardPricingService.java @@ -0,0 +1,12 @@ +package com.example.modulea; + +import com.example.common.PricingService; +import org.springframework.stereotype.Component; + +@Component +public class StandardPricingService implements PricingService { + @Override + public double getPrice() { + return 14.99; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/WarehouseInventoryConfig.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/WarehouseInventoryConfig.java new file mode 100644 index 00000000000..587d4012173 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-a/src/main/java/com/example/modulea/WarehouseInventoryConfig.java @@ -0,0 +1,14 @@ +package com.example.modulea; + +import com.example.common.InventoryService; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class WarehouseInventoryConfig { + + @Bean + public InventoryService warehouseInventoryService() { + return sku -> 100; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/pom.xml b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/pom.xml new file mode 100644 index 00000000000..176521d612d --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/pom.xml @@ -0,0 +1,27 @@ + + + 4.0.0 + + + com.example + ambiguous-dependencies-should-be-resolved + 0.0.1-SNAPSHOT + + + module-b + jar + + + + com.example + module-common + ${project.version} + + + org.springframework + spring-context + + + diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/DigitalWalletPaymentGateway.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/DigitalWalletPaymentGateway.java new file mode 100644 index 00000000000..6ad8b1d35b2 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/DigitalWalletPaymentGateway.java @@ -0,0 +1,12 @@ +package com.example.moduleb; + +import com.example.common.PaymentGateway; +import org.springframework.stereotype.Component; + +@Component +public class DigitalWalletPaymentGateway implements PaymentGateway { + @Override + public void charge(double amount) { + System.out.println("Charging digital wallet: " + amount); + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/PremiumDiscountService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/PremiumDiscountService.java new file mode 100644 index 00000000000..4abecf08ce4 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/PremiumDiscountService.java @@ -0,0 +1,12 @@ +package com.example.moduleb; + +import com.example.common.DiscountService; +import org.springframework.stereotype.Component; + +@Component +public class PremiumDiscountService implements DiscountService { + @Override + public double applyDiscount(double amount) { + return amount * 0.8; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/SecondaryCarrier.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/SecondaryCarrier.java new file mode 100644 index 00000000000..83791a48da4 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/SecondaryCarrier.java @@ -0,0 +1,12 @@ +package com.example.moduleb; + +import com.example.common.ShippingCarrier; +import org.springframework.stereotype.Component; + +@Component +public class SecondaryCarrier implements ShippingCarrier { + @Override + public String track(String trackingId) { + return "secondary:" + trackingId; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/StoreInventoryConfig.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/StoreInventoryConfig.java new file mode 100644 index 00000000000..1a88fe2f26a --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-b/src/main/java/com/example/moduleb/StoreInventoryConfig.java @@ -0,0 +1,14 @@ +package com.example.moduleb; + +import com.example.common.InventoryService; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class StoreInventoryConfig { + + @Bean + public InventoryService storeInventoryService() { + return sku -> 10; + } +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/pom.xml b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/pom.xml new file mode 100644 index 00000000000..878571d5661 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/pom.xml @@ -0,0 +1,15 @@ + + + 4.0.0 + + + com.example + ambiguous-dependencies-should-be-resolved + 0.0.1-SNAPSHOT + + + module-common + jar + diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/CacheProvider.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/CacheProvider.java new file mode 100644 index 00000000000..44638cfd18b --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/CacheProvider.java @@ -0,0 +1,5 @@ +package com.example.common; + +public interface CacheProvider { + String get(String key); +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/DiscountService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/DiscountService.java new file mode 100644 index 00000000000..a9542208f22 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/DiscountService.java @@ -0,0 +1,5 @@ +package com.example.common; + +public interface DiscountService { + double applyDiscount(double amount); +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/InventoryService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/InventoryService.java new file mode 100644 index 00000000000..b30260cfb58 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/InventoryService.java @@ -0,0 +1,5 @@ +package com.example.common; + +public interface InventoryService { + int checkStock(String sku); +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/NotificationService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/NotificationService.java new file mode 100644 index 00000000000..ac19e1a9b75 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/NotificationService.java @@ -0,0 +1,5 @@ +package com.example.common; + +public interface NotificationService { + void send(String message); +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/PaymentGateway.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/PaymentGateway.java new file mode 100644 index 00000000000..ea34ead8dbc --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/PaymentGateway.java @@ -0,0 +1,5 @@ +package com.example.common; + +public interface PaymentGateway { + void charge(double amount); +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/PricingService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/PricingService.java new file mode 100644 index 00000000000..99fe9799471 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/PricingService.java @@ -0,0 +1,5 @@ +package com.example.common; + +public interface PricingService { + double getPrice(); +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/ReportingService.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/ReportingService.java new file mode 100644 index 00000000000..c3974f22734 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/ReportingService.java @@ -0,0 +1,5 @@ +package com.example.common; + +public interface ReportingService { + String generate(); +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/ShippingCarrier.java b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/ShippingCarrier.java new file mode 100644 index 00000000000..14f89715a0c --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/module-common/src/main/java/com/example/common/ShippingCarrier.java @@ -0,0 +1,5 @@ +package com.example.common; + +public interface ShippingCarrier { + String track(String trackingId); +} diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/pom.xml b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/pom.xml new file mode 100644 index 00000000000..05e48b27ced --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/pom.xml @@ -0,0 +1,29 @@ + + + 4.0.0 + + + org.springframework.boot + spring-boot-starter-parent + 3.4.1 + + + + com.example + ambiguous-dependencies-should-be-resolved + 0.0.1-SNAPSHOT + pom + + + 17 + + + + module-common + module-a + module-b + app + + From 8fb01c5f71d400a15c4c504469924afa66176519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Tue, 1 Sep 2026 11:08:48 +0200 Subject: [PATCH 19/22] Fix Spring dependency --- .../app/pom.xml | 11 +------- .../pom.xml | 28 +++++++++++-------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml index 37a0d89115d..18a40f2557f 100644 --- a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml @@ -31,16 +31,7 @@ org.springframework.boot - spring-boot-starter + spring-boot-autoconfigure - - - - - org.springframework.boot - spring-boot-maven-plugin - - - diff --git a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/pom.xml b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/pom.xml index 05e48b27ced..4bc43cb0336 100644 --- a/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/pom.xml +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/pom.xml @@ -4,26 +4,32 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 - - org.springframework.boot - spring-boot-starter-parent - 3.4.1 - - - com.example ambiguous-dependencies-should-be-resolved 0.0.1-SNAPSHOT pom - - 17 - - module-common module-a module-b app + + + 17 + 3.2.0 + + + + + + org.springframework.boot + spring-boot-dependencies + ${spring-boot.version} + pom + import + + + From c946cf000e95b0ab162f146e2820036a6d5f5eb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Tue, 1 Sep 2026 11:36:04 +0200 Subject: [PATCH 20/22] Update comments and clean up --- .../checks/spring/SpringContextChecks.java | 3 +- .../springcontext/BeanDefinitionGatherer.java | 3 +- .../springcontext/BeanDefinitionRegistry.java | 9 -- .../TypeToDependenciesIndex.java | 4 +- .../BeanDefinitionGathererTest.java | 3 - .../java/SpringContextModelSensorTest.java | 102 ++++++------------ 6 files changed, 37 insertions(+), 87 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextChecks.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextChecks.java index 5f62e244aec..6d191dfdc2b 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextChecks.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextChecks.java @@ -22,7 +22,6 @@ * Registry of all {@link SpringContextCheck}s to be run against the {@code SpringContextModel}. * *

          Use {@link #getAllChecks()} to obtain the full list of checks to be run by the scanner. - * New checks should be added here as the set of Spring context issues we detect grows. */ public final class SpringContextChecks { @@ -31,7 +30,7 @@ private SpringContextChecks() { } /** - * Returns all checks that reason over the {@code SpringContextModel}. + * Return all checks that reason over the {@code SpringContextModel}. */ public static List getAllChecks() { return List.of( diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java index b064f028537..e5a737efcde 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java @@ -69,7 +69,8 @@ *

          Also populates: *

            *
          • {@link TypeToBeanNamesIndex} with the full type hierarchy of each bean
            • - *
          • {@link TypeToDependenciesIndex} with the full type hierarchy of each bean
            • + *
          • {@link TypeToDependenciesIndex} with all the dependencies collected by type
            • + *
          */ public class BeanDefinitionGatherer extends SpringContextModelGatherer { diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionRegistry.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionRegistry.java index bde14b4df36..8de01d51202 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionRegistry.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionRegistry.java @@ -42,15 +42,6 @@ public List getByName(String beanName) { return beanDefinitions.getOrDefault(beanName, List.of()); } - /** - * Returns every bean definition registered so far, regardless of the name it is registered under. - */ - public List getAll() { - return beanDefinitions.values().stream() - .flatMap(List::stream) - .toList(); - } - public void addBeanDefinition(String beanName, BeanDefinitionHolder beanDefinition) { beanDefinitions.computeIfAbsent(beanName, k -> new ArrayList<>()).add(beanDefinition); } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java index e411beaefa6..9adfaf4b81f 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.java @@ -32,10 +32,10 @@ * the `@Qualifier` annotation if present) */ public class TypeToDependenciesIndex { - + /** Injection points stored by dependency name (either field/parameter name or qualifier annotation value) and location*/ public record InjectionPoint(String name, BeanLocation location) {} - /** Dependencies (name, location) indexed by fully-qualified type. */ + /** Dependencies indexed by fully-qualified required type. */ private final Map> injectionPointsByType = new HashMap<>(); /** diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index eb592aa1bcf..8de035f8bce 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -465,7 +465,6 @@ void scanWithoutParsing_restores_dependencies_with_and_without_qualifier_from_ca String encodedEnvType = Base64.getEncoder().encodeToString("org.springframework.core.env.Environment".getBytes(StandardCharsets.UTF_8)); String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); - // Field declaration lines in QualifiedFieldDependencies.java: applicationContext=14, environment=17. String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|" + encodedAppContext + ":" + encodedPrimaryContext + "#14:2:14:41" + "," + encodedEnvType + ":" + encodedEnvironment + "#17:2:17:31" @@ -704,7 +703,6 @@ void two_beans_depending_on_same_type_and_name_both_tracked_with_distinct_locati var injectionPoints = model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.context.ApplicationContext"); assertThat(injectionPoints).hasSize(2); assertThat(injectionPoints).extracting(TypeToDependenciesIndex.InjectionPoint::name).containsOnly("applicationContext"); - // Same type and same name, but two distinct occurrences — each must keep its own location, not collapse into one. assertThat(injectionPoints) .extracting(p -> p.location().inputFile(), p -> p.location().mainLocation().startLine) .containsExactlyInAnyOrder( @@ -730,7 +728,6 @@ void scanWithoutParsing_restores_dependencies_index_from_cache() { String encodedEnvType = Base64.getEncoder().encodeToString("org.springframework.core.env.Environment".getBytes(StandardCharsets.UTF_8)); String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); - // Field declaration lines in QualifiedFieldDependencies.java: applicationContext=14, environment=17. String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|" + encodedAppContext + ":" + encodedPrimaryContext + "#14:2:14:41" + "," + encodedEnvType + ":" + encodedEnvironment + "#17:2:17:31" diff --git a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java index db1fb328c29..598ac0495e8 100644 --- a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java +++ b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java @@ -17,10 +17,6 @@ package org.sonar.plugins.java; import java.io.File; -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.util.List; import org.junit.jupiter.api.Test; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.internal.TestInputFileBuilder; @@ -28,23 +24,17 @@ import org.sonar.api.batch.sensor.internal.SensorContextTester; import org.sonar.api.batch.sensor.issue.Issue; import org.sonar.api.rule.RuleKey; -import org.sonar.java.SonarComponents; -import org.sonar.java.checks.verifier.TestUtils; -import org.sonar.java.model.JParser; -import org.sonar.java.model.JParserConfig; -import org.sonar.java.model.VisitorsBridge; -import org.sonar.java.model.springcontext.BeanDefinitionGatherer; +import org.sonar.java.model.springcontext.BeanDefinitionHolder; +import org.sonar.java.model.springcontext.BeanLocation; import org.sonar.java.model.springcontext.SpringContextModel; -import org.sonar.java.test.classpath.TestClasspathUtils; -import org.sonar.plugins.java.api.JavaCheck; -import org.sonar.plugins.java.api.JavaVersion; -import org.sonar.plugins.java.api.tree.CompilationUnitTree; +import org.sonar.java.reporting.AnalyzerMessage.TextSpan; import static org.assertj.core.api.Assertions.assertThat; class SpringContextModelSensorTest { - private static final String BASE_PATH = "checks/spring/s9352/"; + private static final String MODULE_KEY = "module"; + private static final String PACKAGE = "checks.spring.s9352"; @Test void test_toString() { @@ -58,7 +48,13 @@ void test_toString() { @Test void reports_an_issue_for_an_ambiguous_dependency() { SensorContextTester context = SensorContextTester.create(new File("")); - SpringContextModel model = buildModel(context, "ComponentOne.java", "ComponentTwo.java", "UnresolvedConsumer.java"); + SpringContextModel model = new SpringContextModel(); + InputFile inputFile = fakeInputFile(context, "UnresolvedConsumer.java"); + String type = "org.springframework.context.ApplicationContextAware"; + + registerBean(model, type, "componentOne", inputFile, 5, 0, 5, 12); + registerBean(model, type, "componentTwo", inputFile, 6, 0, 6, 12); + registerDependency(model, type, "contextAware", inputFile, 13, 13, 13, 25); new SpringContextModelSensor(model).execute(context); @@ -71,64 +67,30 @@ void reports_an_issue_for_an_ambiguous_dependency() { assertThat(issue.primaryLocation().textRange().start().line()).isEqualTo(13); } - @Test - void reports_no_issue_when_only_one_candidate_exists() { - SensorContextTester context = SensorContextTester.create(new File("")); - SpringContextModel model = buildModel(context, "ResourceLoaderComponent.java", "SingleCandidateConsumer.java"); - - new SpringContextModelSensor(model).execute(context); - - assertThat(context.allIssues()).isEmpty(); - } - - /** - * Runs {@link BeanDefinitionGatherer} over the given files (relative to {@link #BASE_PATH}) into a single, - * freshly built {@link SpringContextModel}, registering each file's {@link InputFile} on the given - * {@link SensorContextTester} so that issues reported against it can be resolved. - */ - private static SpringContextModel buildModel(SensorContextTester context, String... relativeFilePaths) { - List classpath = TestClasspathUtils.DEFAULT_MODULE.getClassPath(); - SonarComponents sonarComponents = new SonarComponents(null, null, null, null, null, null); - sonarComponents.setSensorContext(context); - SpringContextModel model = new SpringContextModel(); - sonarComponents.setSpringContextModel(model); - - BeanDefinitionGatherer gatherer = new BeanDefinitionGatherer(); - VisitorsBridge visitorsBridge = new VisitorsBridge(List.of((JavaCheck) gatherer), classpath, sonarComponents); - for (String relativeFilePath : relativeFilePaths) { - File file = new File(TestUtils.mainCodeSourcesPath(BASE_PATH + relativeFilePath)); - CompilationUnitTree compilationUnit = parse(file, classpath); - InputFile inputFile = inputFile(file); - context.fileSystem().add(inputFile); - visitorsBridge.setCurrentFile(inputFile); - visitorsBridge.visitFile(compilationUnit, false); - } - visitorsBridge.endOfAnalysis(); - return model; + private static void registerBean(SpringContextModel model, String type, String beanName, InputFile inputFile, + int startLine, int startCharacter, int endLine, int endCharacter) { + BeanLocation location = new BeanLocation(inputFile, new TextSpan(startLine, startCharacter, endLine, endCharacter)); + model.getBeanDefinitionRegistry().addBeanDefinition(beanName, + new BeanDefinitionHolder.Builder(type, MODULE_KEY, PACKAGE, location).build()); + model.getTypeToBeanNamesIndex().addBeanForType(type, beanName); } - private static InputFile inputFile(File file) { - try { - return new TestInputFileBuilder("", file.getParentFile(), file) - .setContents(Files.readString(file.toPath(), StandardCharsets.UTF_8)) - .setCharset(StandardCharsets.UTF_8) - .setLanguage("java") - .setType(InputFile.Type.MAIN) - .build(); - } catch (IOException e) { - throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e); - } + private static void registerDependency(SpringContextModel model, String type, String dependencyName, InputFile inputFile, + int startLine, int startCharacter, int endLine, int endCharacter) { + BeanLocation location = new BeanLocation(inputFile, new TextSpan(startLine, startCharacter, endLine, endCharacter)); + model.getTypeToDependenciesIndex().addDependencyForType(type, dependencyName, location); } - private static CompilationUnitTree parse(File file, List classpath) { - String source; - try { - source = Files.readString(file.toPath(), StandardCharsets.UTF_8); - } catch (Exception e) { - throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e); - } - JavaVersion version = JParserConfig.MAXIMUM_SUPPORTED_JAVA_VERSION; - return JParser.parse(JParserConfig.Mode.FILE_BY_FILE.create(version, classpath).astParser(), version.toString(), file.getName(), source); + private static InputFile fakeInputFile(SensorContextTester context, String fileName) { + String line = "// dummy source line //////\n"; + String contents = line.repeat(20); + InputFile inputFile = new TestInputFileBuilder("", fileName) + .setContents(contents) + .setLanguage("java") + .setType(InputFile.Type.MAIN) + .build(); + context.fileSystem().add(inputFile); + return inputFile; } } From 43ad38dc1d91b961dfd8a5eed0e9e0796a774393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 2 Sep 2026 09:40:33 +0200 Subject: [PATCH 21/22] Add javadoc for execute method --- .../spring/AmbiguousDependencyCheck.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java index 74a38247cb6..ebd93c46b0c 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -40,6 +40,16 @@ public class AmbiguousDependencyCheck implements JavaCheck, SpringContextCheck { private static final String MESSAGE = "Multiple beans match this dependency (%s);" + " disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\"."; + /** Creates the list of issues using the spring context model. + * For each bean type in the project, gets the names of beans of this type (candidates) and the dependencies + * (injection points) that require this type, then check if there are ambiguous dependencies of this type: + * a single candidate, or a single one marked {@code @Primary} is unambiguous; otherwise + * candidates with a profile are excluded as potentially mutually exclusive, and if multiple + * candidates remain, an issue is created for each injection point that does not match a candidate + * by name. + * + * @param model the Spring context model of the project + */ @Override public List execute(SpringContextModel model) { BeanDefinitionRegistry registry = model.getBeanDefinitionRegistry(); @@ -50,11 +60,10 @@ public List execute(SpringContextModel model) { for (String type : typeToBeanNamesIndex.getKeys()) { Set candidates = typeToBeanNamesIndex.getNamesForType(type); Set injectionPoints = typeToDependenciesIndex.getDependenciesForType(type); - if (!isResolved(candidates, registry)) { - // excluding all beans with a configured profile, no matter what the profile is, to avoid FPs + if (!hasUniqueOrPrimaryCandidate(candidates, registry)) { Set effectiveCandidates = excludeCandidatesWithProfile(candidates, registry); if (effectiveCandidates.size() > 1) { - for (InjectionPoint unresolvedInjectionPoint : computeUnresolvedInjectionPoints(effectiveCandidates, injectionPoints)) { + for (InjectionPoint unresolvedInjectionPoint : findInjectionPointsNotMatchingCandidateByName(effectiveCandidates, injectionPoints)) { issues.add(new SpringContextIssue(unresolvedInjectionPoint.location(), message(effectiveCandidates))); } } @@ -63,12 +72,12 @@ public List execute(SpringContextModel model) { return issues; } - private static boolean isResolved(Set candidates, BeanDefinitionRegistry registry) { + private static boolean hasUniqueOrPrimaryCandidate(Set candidates, BeanDefinitionRegistry registry) { return candidates.size() <= 1 || hasExactlyOnePrimaryCandidate(candidates, registry); } - private static Set computeUnresolvedInjectionPoints(Set candidates, Set injectionPointNames) { + private static Set findInjectionPointsNotMatchingCandidateByName(Set candidates, Set injectionPointNames) { return injectionPointNames.stream().filter(injectionPoint -> !candidates.contains(injectionPoint.name())).collect(Collectors.toSet()); } From 15e3ee1216e2630d1e84bf4fa305274feb7d6343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 2 Sep 2026 09:46:18 +0200 Subject: [PATCH 22/22] Remove concurrent execution on ScannerIntegrationAbstractTest --- .../java/org/sonar/java/it/ScannerIntegrationAbstractTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/its/scanner-integration-tests/src/test/java/org/sonar/java/it/ScannerIntegrationAbstractTest.java b/its/scanner-integration-tests/src/test/java/org/sonar/java/it/ScannerIntegrationAbstractTest.java index 4558e6a485b..2ec3cc00959 100644 --- a/its/scanner-integration-tests/src/test/java/org/sonar/java/it/ScannerIntegrationAbstractTest.java +++ b/its/scanner-integration-tests/src/test/java/org/sonar/java/it/ScannerIntegrationAbstractTest.java @@ -53,7 +53,7 @@ import org.junit.jupiter.api.parallel.ExecutionMode; import org.sonar.java.test.classpath.TestClasspathUtils; -@Execution(ExecutionMode.CONCURRENT) +@Execution(ExecutionMode.SAME_THREAD) public abstract class ScannerIntegrationAbstractTest { private static FileLocation javaPluginLocation;