diff --git a/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java b/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java index 41f81ec4652..88befbffb0c 100644 --- a/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java +++ b/its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java @@ -205,7 +205,8 @@ public void guava() throws Exception { String projectName = "guava"; MavenBuild build = test_project("com.google.guava:guava", projectName); build - .setProperty("java.version", "17") + // Keep compilation and analysis on Java 17 without overriding the Java runtime version seen by the scanner. + .setProperty("sonar.java.source", "17") .setProperty("maven-bundle-plugin.version", "5.1.4") .setProperty("maven.javadoc.skip", "true") .setProperty("animal.sniffer.skip", "true") 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 a4b4e3c3ff1..c4b72ef70fb 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 @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; +import javax.annotation.Nullable; import org.sonar.api.batch.fs.InputFile; import org.sonar.java.reporting.AnalyzerMessage; import org.sonar.java.utils.PackageUtils; @@ -53,6 +54,7 @@ public class BeanDefinitionGatherer extends SpringContextModelGatherer { private static final String PRIMARY_ANNOTATION = "org.springframework.context.annotation.Primary"; + private static final String VALUE_ATTRIBUTE = "value"; private final List collectedBeans = new ArrayList<>(); @@ -63,7 +65,7 @@ private record BeanData( InputFile inputFile, AnalyzerMessage.TextSpan textSpan, boolean isPrimary, - List dependingBeans) { + List dependingBeans) { } @Override @@ -85,7 +87,7 @@ public void visitNode(Tree tree) { if (SpringUtils.STEREOTYPE_ANNOTATIONS.stream().anyMatch(meta::isAnnotatedWith)) { String beanName = extractBeanName(meta) .orElseGet(() -> defaultBeanName(classTree.simpleName().name())); - List deps = collectAutowiredDependencies(classTree); + List deps = collectAutowiredDependencies(classTree); // Class-level bean (stereotype annotations) collectedBeans.add(new BeanData( beanName, fqn, pkg, @@ -128,7 +130,7 @@ private static Optional extractBeanName(SymbolMetadata meta) { List attrs = meta.valuesForAnnotation(annotation); if (attrs != null) { Optional name = attrs.stream() - .filter(v -> "value".equals(v.name()) || "name".equals(v.name())) + .filter(v -> VALUE_ATTRIBUTE.equals(v.name()) || "name".equals(v.name())) .map(v -> (String) v.value()) .filter(s -> !s.isBlank()) .findFirst(); @@ -149,7 +151,7 @@ private void collectBeanMethod(MethodTree method, String pkg) { List attrs = beanMeta.valuesForAnnotation(SpringUtils.BEAN_ANNOTATION); String beanName = Optional.ofNullable(attrs) .flatMap(list -> list.stream() - .filter(v -> "value".equals(v.name()) || "name".equals(v.name())) + .filter(v -> VALUE_ATTRIBUTE.equals(v.name()) || "name".equals(v.name())) .map(v -> { Object val = v.value(); if (val instanceof Object[] arr && arr.length > 0) { @@ -165,8 +167,8 @@ private void collectBeanMethod(MethodTree method, String pkg) { ? method.returnType().symbolType().fullyQualifiedName() : ""; - List paramDeps = method.parameters().stream() - .map(p -> p.symbol().type().fullyQualifiedName()) + List paramDeps = method.parameters().stream() + .map(p -> new BeanDependency(p.symbol().type().fullyQualifiedName(), extractQualifier(p.symbol().metadata()))) .toList(); collectedBeans.add(new BeanData( @@ -177,24 +179,46 @@ private void collectBeanMethod(MethodTree method, String pkg) { paramDeps)); } - private static List collectAutowiredDependencies(ClassTree classTree) { - List deps = new ArrayList<>(); + private static List collectAutowiredDependencies(ClassTree classTree) { + List deps = new ArrayList<>(); + List unannotatedConstructors = new ArrayList<>(); for (Tree member : classTree.members()) { - if (member.is(Tree.Kind.VARIABLE)) { - VariableTree field = (VariableTree) member; + if (member instanceof VariableTree field) { if (field.symbol().metadata().isAnnotatedWith(SpringUtils.AUTOWIRED_ANNOTATION)) { - deps.add(field.symbol().type().fullyQualifiedName()); + deps.add(new BeanDependency(field.symbol().type().fullyQualifiedName(), extractQualifier(field.symbol().metadata()))); } - } else if (member.is(Tree.Kind.CONSTRUCTOR, Tree.Kind.METHOD)) { - MethodTree method = (MethodTree) member; + } else if (member instanceof MethodTree method) { if (method.symbol().metadata().isAnnotatedWith(SpringUtils.AUTOWIRED_ANNOTATION)) { - method.parameters().stream() - .map(p -> p.symbol().type().fullyQualifiedName()) - .forEach(deps::add); + deps.addAll(parameterDependencies(method)); + } else if (method.is(Tree.Kind.CONSTRUCTOR)) { + unannotatedConstructors.add(method); } } } + if (deps.isEmpty() && unannotatedConstructors.size() == 1) { + deps.addAll(parameterDependencies(unannotatedConstructors.get(0))); + } return deps; } + private static List parameterDependencies(MethodTree method) { + return method.parameters().stream() + .map(p -> new BeanDependency(p.symbol().type().fullyQualifiedName(), extractQualifier(p.symbol().metadata()))) + .toList(); + } + + @Nullable + private static String extractQualifier(SymbolMetadata metadata) { + List attrs = metadata.valuesForAnnotation(SpringUtils.QUALIFIER_ANNOTATION); + if (attrs == null) { + return null; + } + return attrs.stream() + .filter(v -> VALUE_ATTRIBUTE.equals(v.name())) + .map(v -> (String) v.value()) + .filter(s -> !s.isBlank()) + .findFirst() + .orElse(null); + } + } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java index 3cca0218e72..1da2d5c3828 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java @@ -51,8 +51,8 @@ public class BeanDefinitionHolder { /** Source location where the bean definition appears. */ private final BeanLocation location; - /** Names of other beans this bean depends on. */ - private List dependingBeans; + /** Dependencies this bean requires, each capturing the required type and an optional {@code @Qualifier} name. */ + private List dependingBeans; /** Comma-separated Spring profile expressions under which this bean is active, or {@code null} if unconditional. */ @Nullable @@ -68,7 +68,7 @@ private BeanDefinitionHolder(String type, String module, String beanPackage, Bea this.location = location; } - private void setDependingBeans(List beansList) { + private void setDependingBeans(List beansList) { this.dependingBeans = beansList; } @@ -96,7 +96,7 @@ public BeanLocation getLocation() { return location; } - public List getDependingBeans() { + public List getDependingBeans() { return dependingBeans; } @@ -114,7 +114,7 @@ public static class Builder { private final String module; private final String beanPackage; private final BeanLocation location; - private List dependingBeans = new ArrayList<>(); + private List dependingBeans = new ArrayList<>(); @Nullable private String profiles; private boolean isPrimary = false; @@ -126,7 +126,7 @@ public Builder(String type, String module, String beanPackage, BeanLocation loca this.location = location; } - public Builder dependingBeans(List beansList) { + public Builder dependingBeans(List beansList) { this.dependingBeans = beansList; return this; } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDependency.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDependency.java new file mode 100644 index 00000000000..2e0aef75b75 --- /dev/null +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDependency.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.model.springcontext; + +import javax.annotation.Nullable; + +/** + * Represents a single Spring autowiring dependency: the required type and an optional {@code @Qualifier} name. + * + * @param typeFqn fully-qualified name of the required type + * @param qualifier value of the {@code @Qualifier} annotation, or {@code null} if absent + */ +public record BeanDependency(String typeFqn, @Nullable String qualifier) { +} diff --git a/java-frontend/src/main/java/org/sonar/java/utils/SpringUtils.java b/java-frontend/src/main/java/org/sonar/java/utils/SpringUtils.java index f1dc9d323be..9673af815dd 100644 --- a/java-frontend/src/main/java/org/sonar/java/utils/SpringUtils.java +++ b/java-frontend/src/main/java/org/sonar/java/utils/SpringUtils.java @@ -33,6 +33,7 @@ public final class SpringUtils { public static final String REPOSITORY_ANNOTATION = "org.springframework.stereotype.Repository"; public static final String SERVICE_ANNOTATION = "org.springframework.stereotype.Service"; public static final String AUTOWIRED_ANNOTATION = "org.springframework.beans.factory.annotation.Autowired"; + public static final String QUALIFIER_ANNOTATION = "org.springframework.beans.factory.annotation.Qualifier"; public static final String VALUE_ANNOTATION = "org.springframework.beans.factory.annotation.Value"; public static final String TRANSACTIONAL_ANNOTATION = "org.springframework.transaction.annotation.Transactional"; public static final String BEAN_ANNOTATION = "org.springframework.context.annotation.Bean"; diff --git a/java-frontend/src/test/files/springcontext/MultipleConstructorsNoDependencies.java b/java-frontend/src/test/files/springcontext/MultipleConstructorsNoDependencies.java new file mode 100644 index 00000000000..9efb0906c9f --- /dev/null +++ b/java-frontend/src/test/files/springcontext/MultipleConstructorsNoDependencies.java @@ -0,0 +1,22 @@ +package checks.spring.context; + +import org.springframework.context.ApplicationContext; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +@Component +class MultipleConstructorsNoDependencies { + + private final ApplicationContext applicationContext; + private final Environment environment; + + MultipleConstructorsNoDependencies(ApplicationContext applicationContext) { + this.applicationContext = applicationContext; + this.environment = null; + } + + MultipleConstructorsNoDependencies(ApplicationContext applicationContext, Environment environment) { + this.applicationContext = applicationContext; + this.environment = environment; + } +} diff --git a/java-frontend/src/test/files/springcontext/QualifiedBeanMethodDependencies.java b/java-frontend/src/test/files/springcontext/QualifiedBeanMethodDependencies.java new file mode 100644 index 00000000000..6e982ea0368 --- /dev/null +++ b/java-frontend/src/test/files/springcontext/QualifiedBeanMethodDependencies.java @@ -0,0 +1,18 @@ +package checks.spring.context; + +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.Environment; + +@Configuration +class QualifiedBeanMethodDependencies { + + @Bean + Object myBean( + @Qualifier("primaryContext") ApplicationContext applicationContext, + Environment environment) { + return new Object(); + } +} diff --git a/java-frontend/src/test/files/springcontext/QualifiedConstructorDependencies.java b/java-frontend/src/test/files/springcontext/QualifiedConstructorDependencies.java new file mode 100644 index 00000000000..b15ab0dd188 --- /dev/null +++ b/java-frontend/src/test/files/springcontext/QualifiedConstructorDependencies.java @@ -0,0 +1,22 @@ +package checks.spring.context; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.ApplicationContext; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +@Component +class QualifiedConstructorDependencies { + + private final ApplicationContext applicationContext; + private final Environment environment; + + @Autowired + QualifiedConstructorDependencies( + @Qualifier("primaryContext") ApplicationContext applicationContext, + Environment environment) { + this.applicationContext = applicationContext; + this.environment = environment; + } +} diff --git a/java-frontend/src/test/files/springcontext/QualifiedFieldDependencies.java b/java-frontend/src/test/files/springcontext/QualifiedFieldDependencies.java new file mode 100644 index 00000000000..4a5973814fe --- /dev/null +++ b/java-frontend/src/test/files/springcontext/QualifiedFieldDependencies.java @@ -0,0 +1,18 @@ +package checks.spring.context; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.ApplicationContext; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +@Component +class QualifiedFieldDependencies { + + @Autowired + @Qualifier("primaryContext") + private ApplicationContext applicationContext; + + @Autowired + private Environment environment; +} diff --git a/java-frontend/src/test/files/springcontext/SingleConstructorDependencies.java b/java-frontend/src/test/files/springcontext/SingleConstructorDependencies.java new file mode 100644 index 00000000000..a48f1cb31c5 --- /dev/null +++ b/java-frontend/src/test/files/springcontext/SingleConstructorDependencies.java @@ -0,0 +1,17 @@ +package checks.spring.context; + +import org.springframework.context.ApplicationContext; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +@Component +class SingleConstructorDependencies { + + private final ApplicationContext applicationContext; + private final Environment environment; + + SingleConstructorDependencies(ApplicationContext applicationContext, Environment environment) { + this.applicationContext = applicationContext; + this.environment = environment; + } +} 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 50ebcf2fb02..f4af2d73c75 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 @@ -172,6 +172,7 @@ void dependencies_collected_as_depending_beans(String filePath, String expectedB var beans = model.getBeanDefinitionRegistry().getByName(expectedBeanName); assertThat(beans).hasSize(1); assertThat(beans.get(0).getDependingBeans()) + .extracting(BeanDependency::typeFqn) .containsExactlyInAnyOrder( "org.springframework.context.ApplicationContext", "org.springframework.core.env.Environment" @@ -182,10 +183,55 @@ static Stream dependencyCollectionArguments() { return Stream.of( Arguments.of("src/test/files/springcontext/AutowiredDependencies.java", "autowiredDependencies"), Arguments.of("src/test/files/springcontext/AutowiredConstructorDependencies.java", "autowiredConstructorDependencies"), - Arguments.of("src/test/files/springcontext/BeanMethodWithDependencies.java", "myBean") + Arguments.of("src/test/files/springcontext/BeanMethodWithDependencies.java", "myBean"), + Arguments.of("src/test/files/springcontext/SingleConstructorDependencies.java", "singleConstructorDependencies") ); } + // ---- Implicit single-constructor injection -------------------------------- + + @Test + void multiple_constructors_without_autowired_yields_no_dependencies() { + scan("src/test/files/springcontext/MultipleConstructorsNoDependencies.java"); + + var beans = model.getBeanDefinitionRegistry().getByName("multipleConstructorsNoDependencies"); + assertThat(beans).hasSize(1); + assertThat(beans.get(0).getDependingBeans()).isEmpty(); + } + + // ---- @Qualifier handling -------------------------------------------------- + + @ParameterizedTest(name = "{0}") + @MethodSource("qualifiedDependencyArguments") + void qualifier_is_captured_on_qualified_dependency(String filePath, String expectedBeanName) { + scan(filePath); + + var beans = model.getBeanDefinitionRegistry().getByName(expectedBeanName); + assertThat(beans).hasSize(1); + assertThat(beans.get(0).getDependingBeans()) + .extracting(BeanDependency::qualifier) + .containsExactlyInAnyOrder("primaryContext", null); + } + + static Stream qualifiedDependencyArguments() { + return Stream.of( + Arguments.of("src/test/files/springcontext/QualifiedFieldDependencies.java", "qualifiedFieldDependencies"), + Arguments.of("src/test/files/springcontext/QualifiedConstructorDependencies.java", "qualifiedConstructorDependencies"), + Arguments.of("src/test/files/springcontext/QualifiedBeanMethodDependencies.java", "myBean") + ); + } + + @Test + void no_qualifier_results_in_null_qualifier() { + scan("src/test/files/springcontext/AutowiredDependencies.java"); + + var beans = model.getBeanDefinitionRegistry().getByName("autowiredDependencies"); + assertThat(beans).hasSize(1); + assertThat(beans.get(0).getDependingBeans()) + .extracting(BeanDependency::qualifier) + .containsOnly((String) null); + } + // ---- Bean location -------------------------------------------------------- @Test @@ -210,4 +256,4 @@ void bean_package_is_captured() { assertThat(beans).hasSize(1); assertThat(beans.get(0).getBeanPackage()).isEqualTo("checks.spring.context"); } -} \ No newline at end of file +}