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; 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..18a40f2557f --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/app/pom.xml @@ -0,0 +1,37 @@ + + + 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-autoconfigure + + + 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..4bc43cb0336 --- /dev/null +++ b/its/scanner-integration-tests/src/test/resources/ambiguous-dependencies-should-be-resolved/pom.xml @@ -0,0 +1,35 @@ + + + 4.0.0 + + com.example + ambiguous-dependencies-should-be-resolved + 0.0.1-SNAPSHOT + pom + + + module-common + module-a + module-b + app + + + + 17 + 3.2.0 + + + + + + org.springframework.boot + spring-boot-dependencies + ${spring-boot.version} + pom + import + + + + 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/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-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/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-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/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..ebd93c46b0c --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java @@ -0,0 +1,105 @@ +/* + * 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.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.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; + +/** + * 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, 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(); + TypeToBeanNamesIndex typeToBeanNamesIndex = model.getTypeToBeanNamesIndex(); + TypeToDependenciesIndex typeToDependenciesIndex = model.getTypeToDependenciesIndex(); + + List issues = new ArrayList<>(); + for (String type : typeToBeanNamesIndex.getKeys()) { + Set candidates = typeToBeanNamesIndex.getNamesForType(type); + Set injectionPoints = typeToDependenciesIndex.getDependenciesForType(type); + if (!hasUniqueOrPrimaryCandidate(candidates, registry)) { + Set effectiveCandidates = excludeCandidatesWithProfile(candidates, registry); + if (effectiveCandidates.size() > 1) { + for (InjectionPoint unresolvedInjectionPoint : findInjectionPointsNotMatchingCandidateByName(effectiveCandidates, injectionPoints)) { + issues.add(new SpringContextIssue(unresolvedInjectionPoint.location(), message(effectiveCandidates))); + } + } + } + } + return issues; + } + + private static boolean hasUniqueOrPrimaryCandidate(Set candidates, BeanDefinitionRegistry registry) { + return candidates.size() <= 1 + || hasExactlyOnePrimaryCandidate(candidates, registry); + } + + private static Set findInjectionPointsNotMatchingCandidateByName(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 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 hasProfile(BeanDefinitionRegistry registry, String beanName) { + return registry.getByName(beanName).stream().anyMatch(bean -> bean.getProfiles() != null); + } + + private static String message(Set candidates) { + String sortedCandidates = candidates.stream().sorted().collect(Collectors.joining(", ")); + return String.format(MESSAGE, sortedCandidates); + } + +} 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..6d191dfdc2b --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringContextChecks.java @@ -0,0 +1,41 @@ +/* + * 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. + */ +public final class SpringContextChecks { + + private SpringContextChecks() { + // utility class, should not be instantiated + } + + /** + * Return 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 new file mode 100644 index 00000000000..80ed5a43634 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java @@ -0,0 +1,146 @@ +/* + * 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.Arrays; +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.execute(model)).hasSize(1); + } + + @Test + void primary_candidate_resolves_ambiguity() { + SpringContextModel model = buildModel("BeanNameComponent.java", "PrimaryComponent.java", "PrimaryConsumer.java"); + assertThat(check.execute(model)).isEmpty(); + } + + @Test + void two_primary_candidates_still_raise_issue() { + SpringContextModel model = buildModel("TwoPrimaryComponentA.java", "TwoPrimaryComponentB.java", "TwoPrimaryConsumer.java"); + 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.execute(model)).isEmpty(); + } + + @Test + void qualifier_resolves_ambiguity() { + SpringContextModel model = buildModel("EnvironmentComponentA.java", "EnvironmentComponentB.java", "QualifierConsumer.java"); + assertThat(check.execute(model)).isEmpty(); + } + + @Test + void single_candidate_does_not_raise_issue() { + SpringContextModel model = buildModel("ResourceLoaderComponent.java", "SingleCandidateConsumer.java"); + 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.execute(model)).hasSize(1); + } + + /** + * 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()); + } + + 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(""))); + SpringContextModel model = new SpringContextModel(); + sonarComponents.setSpringContextModel(model); + + BeanDefinitionGatherer gatherer = new BeanDefinitionGatherer(); + VisitorsBridge visitorsBridge = new VisitorsBridge(List.of((JavaCheck) gatherer), classpath, sonarComponents); + for (String filePath : filePaths) { + File file = new File(filePath); + 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/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)); 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..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 @@ -66,8 +66,11 @@ *

  • 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 all the dependencies collected by type
    • + *
    */ public class BeanDefinitionGatherer extends SpringContextModelGatherer { @@ -79,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"; @@ -97,6 +101,7 @@ private record BeanData( AnalyzerMessage.TextSpan textSpan, boolean isPrimary, Map> dependingBeans, + Map> dependencyInjectionPoints, Set typeHierarchy) { } @@ -125,7 +130,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, @@ -133,6 +139,7 @@ public void visitNode(Tree tree) { AnalyzerMessage.textSpanFor(classTree.simpleName()), meta.isAnnotatedWith(PRIMARY_ANNOTATION), deps, + injectionPoints, typeHierarchy); collectedBeans.add(beanData); beansCollectedAtFileLevel.add(beanData); @@ -170,11 +177,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()); @@ -190,6 +197,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) { @@ -205,6 +219,9 @@ public void gatherSpringContextData(ModuleScannerContext context, SpringContextM for (String typeFqn : data.typeHierarchy()) { springContextModel.getTypeToBeanNamesIndex().addBeanForType(typeFqn, data.beanName()); } + data.dependencyInjectionPoints().forEach((typeFqn, points) -> + points.forEach(point -> springContextModel.getTypeToDependenciesIndex() + .addDependencyForType(typeFqn, point.name(), point.location()))); } } @@ -251,21 +268,34 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { Integer.parseInt(spanParts[2]), Integer.parseInt(spanParts[3])); boolean isPrimary = Boolean.parseBoolean(fields[4]); - Map> deps = new LinkedHashMap<>(); + Map> injectionPoints = new LinkedHashMap<>(); 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 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[6].isEmpty() ? new LinkedHashSet<>(List.of(fields[6].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, 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) { @@ -314,54 +344,66 @@ 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); 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, 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/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/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 new file mode 100644 index 00000000000..9adfaf4b81f --- /dev/null +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/TypeToDependenciesIndex.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.model.springcontext; + +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 { + /** Injection points stored by dependency name (either field/parameter name or qualifier annotation value) and location*/ + public record InjectionPoint(String name, BeanLocation location) {} + + /** Dependencies indexed by fully-qualified required type. */ + private final Map> injectionPointsByType = 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 addDependencyForType(String dependencyType, String dependencyName, BeanLocation location) { + injectionPointsByType.computeIfAbsent(dependencyType, k -> new HashSet<>()) + .add(new InjectionPoint(dependencyName, location)); + } + + /** + * 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 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 4e02bbcf04c..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 @@ -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; @@ -37,8 +38,10 @@ 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.assertj.core.api.Assertions.tuple; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doThrow; @@ -463,8 +466,8 @@ 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)); 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); @@ -633,4 +636,122 @@ private static CacheContext mockCacheContext(JavaReadCache readCache, JavaWriteC when(cacheContext.getWriteCache()).thenReturn(writeCache); return cacheContext; } + + // ---- TypeToDependenciesIndex ------------------------------------------------- + + @ParameterizedTest(name = "{0}") + @MethodSource("regularDependencyArguments") + void regular_dependencies_registered_in_index(String filePath, int applicationContextLine, int environmentLine) { + scan(filePath); + 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"); + 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"); + 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"); + 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"); + 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 + 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 + "#14:2:14:41" + + "," + encodedEnvType + ":" + encodedEnvironment + "#17:2:17:31" + + "|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); + + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.context.ApplicationContext"), + "primaryContext", inputFile, 14); + assertInjectionPoint( + model.getTypeToDependenciesIndex().getDependenciesForType("org.springframework.core.env.Environment"), + "environment", inputFile, 17); + } } 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..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 @@ -19,9 +19,18 @@ 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.check.Rule; +import org.sonar.java.GeneratedCheckList; +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; +import org.sonar.java.reporting.AnalyzerMessage; /** * A post-phase {@link ProjectSensor} that holds the shared {@link SpringContextModel} built during analysis. @@ -49,6 +58,23 @@ public void describe(SensorDescriptor descriptor) { @Override public void execute(SensorContext context) { - // Nothing to do for now + for (SpringContextCheck check : SpringContextChecks.getAllChecks()) { + reportIssues(context, check); + } + } + + 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(issue.message())); + newIssue.save(); + } } } + 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:

    +
      +
    • Qualifier annotations - Use a custom qualifier at the injection point to specify which of the multiple beans of the same type + to inject
    • +
    • Primary designation - Designate one component as the default choice when multiple candidates exist
    • +
    +

    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" ] } 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 d0796f6e08c..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 @@ -16,14 +16,26 @@ */ package org.sonar.plugins.java; +import java.io.File; 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.model.springcontext.BeanDefinitionHolder; +import org.sonar.java.model.springcontext.BeanLocation; import org.sonar.java.model.springcontext.SpringContextModel; +import org.sonar.java.reporting.AnalyzerMessage.TextSpan; import static org.assertj.core.api.Assertions.assertThat; class SpringContextModelSensorTest { + private static final String MODULE_KEY = "module"; + private static final String PACKAGE = "checks.spring.s9352"; + @Test void test_toString() { DefaultSensorDescriptor descriptor = new DefaultSensorDescriptor(); @@ -33,4 +45,52 @@ 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 = 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); + + 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 match this dependency" + + " (componentOne, componentTwo); disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\"."); + assertThat(issue.primaryLocation().textRange().start().line()).isEqualTo(13); + } + + 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 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 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; + } + }