Skip to content

Commit 048473e

Browse files
NoemieBenardNoemieBenard
committed
Implement check
1 parent 134d12b commit 048473e

5 files changed

Lines changed: 336 additions & 1 deletion

File tree

‎java-checks/src/main/java/org/sonar/java/checks/spring/AmbiguousDependencyCheck.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks.spring;
18+
19+
import java.util.ArrayList;
20+
import java.util.Map;
21+
import java.util.List;
22+
import java.util.Set;
23+
import java.util.stream.Collectors;
24+
import org.sonar.check.Rule;
25+
import org.sonar.java.model.springcontext.BeanDefinitionHolder;
26+
import org.sonar.java.model.springcontext.BeanDefinitionRegistry;
27+
import org.sonar.java.model.springcontext.BeanLocation;
28+
import org.sonar.java.model.springcontext.SpringContextModel;
29+
import org.sonar.java.model.springcontext.TypeToBeanNamesIndex;
30+
import org.sonar.plugins.java.api.JavaCheck;
31+
32+
/**
33+
* Not an AST visitor: called directly by {@code SpringContextModelSensor} once the {@link SpringContextModel}
34+
* has been fully populated by the gatherers, since detecting autowiring ambiguity requires reasoning about every
35+
* bean of a given type across the whole analyzed scope, not a single file.
36+
*/
37+
@Rule(key = "S9352")
38+
public class AmbiguousDependencyCheck implements JavaCheck {
39+
40+
/**
41+
* @param location bean whose dependency is ambiguous, used to anchor the reported issue
42+
* @param message issue message describing the ambiguity
43+
*/
44+
public record AmbiguousDependency(BeanLocation location, String message) {
45+
}
46+
47+
private static final String MESSAGE = "Multiple beans of type \"%s\" match this dependency (%s);"
48+
+ " disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\".";
49+
50+
public List<AmbiguousDependency> findAmbiguousDependencies(SpringContextModel model) {
51+
BeanDefinitionRegistry registry = model.getBeanDefinitionRegistry();
52+
TypeToBeanNamesIndex typeToBeanNamesIndex = model.getTypeToBeanNamesIndex();
53+
54+
List<AmbiguousDependency> ambiguousDependencies = new ArrayList<>();
55+
for (BeanDefinitionHolder bean : registry.getAll()) {
56+
for (Map.Entry<String, Set<String>> dependency : bean.getDependingBeans().entrySet()) {
57+
String requiredType = dependency.getKey();
58+
Set<String> candidates = typeToBeanNamesIndex.getNamesForType(requiredType);
59+
if (isAmbiguous(candidates, dependency.getValue(), registry)) {
60+
ambiguousDependencies.add(new AmbiguousDependency(bean.getLocation(), message(requiredType, candidates)));
61+
}
62+
}
63+
}
64+
return ambiguousDependencies;
65+
}
66+
67+
private static boolean isAmbiguous(Set<String> candidates, Set<String> injectionPointNames, BeanDefinitionRegistry registry) {
68+
return candidates.size() > 1
69+
&& candidates.stream().noneMatch(candidate -> isPrimary(registry, candidate))
70+
&& candidates.stream().noneMatch(injectionPointNames::contains);
71+
}
72+
73+
private static boolean isPrimary(BeanDefinitionRegistry registry, String beanName) {
74+
return registry.getByName(beanName).stream().anyMatch(BeanDefinitionHolder::isPrimary);
75+
}
76+
77+
private static String message(String requiredType, Set<String> candidates) {
78+
String sortedCandidates = candidates.stream().sorted().collect(Collectors.joining(", "));
79+
return String.format(MESSAGE, requiredType, sortedCandidates);
80+
}
81+
82+
}

‎java-checks/src/test/java/org/sonar/java/checks/spring/AmbiguousDependencyCheckTest.java‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks.spring;
18+
19+
import java.io.File;
20+
import java.io.IOException;
21+
import java.nio.charset.StandardCharsets;
22+
import java.nio.file.Files;
23+
import java.util.List;
24+
import org.junit.jupiter.api.Test;
25+
import org.sonar.api.batch.fs.InputFile;
26+
import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
27+
import org.sonar.api.batch.sensor.internal.SensorContextTester;
28+
import org.sonar.java.SonarComponents;
29+
import org.sonar.java.checks.verifier.TestUtils;
30+
import org.sonar.java.model.JParser;
31+
import org.sonar.java.model.JParserConfig;
32+
import org.sonar.java.model.VisitorsBridge;
33+
import org.sonar.java.model.springcontext.BeanDefinitionGatherer;
34+
import org.sonar.java.model.springcontext.SpringContextModel;
35+
import org.sonar.java.test.classpath.TestClasspathUtils;
36+
import org.sonar.plugins.java.api.JavaCheck;
37+
import org.sonar.plugins.java.api.JavaVersion;
38+
import org.sonar.plugins.java.api.tree.CompilationUnitTree;
39+
40+
import static org.assertj.core.api.Assertions.assertThat;
41+
42+
class AmbiguousDependencyCheckTest {
43+
44+
private static final String BASE_PATH = "checks/spring/s9352/";
45+
46+
private final AmbiguousDependencyCheck check = new AmbiguousDependencyCheck();
47+
48+
@Test
49+
void ambiguous_dependency_with_no_disambiguation_raises_issue() {
50+
SpringContextModel model = buildModel("ComponentOne.java", "ComponentTwo.java", "UnresolvedConsumer.java");
51+
assertThat(check.findAmbiguousDependencies(model)).hasSize(1);
52+
}
53+
54+
@Test
55+
void primary_candidate_resolves_ambiguity() {
56+
SpringContextModel model = buildModel("BeanNameComponent.java", "PrimaryComponent.java", "PrimaryConsumer.java");
57+
assertThat(check.findAmbiguousDependencies(model)).isEmpty();
58+
}
59+
60+
@Test
61+
void field_name_matching_bean_name_resolves_ambiguity() {
62+
SpringContextModel model = buildModel("BeanFactoryComponentA.java", "BeanFactoryComponentB.java", "NameMatchConsumer.java");
63+
assertThat(check.findAmbiguousDependencies(model)).isEmpty();
64+
}
65+
66+
@Test
67+
void qualifier_resolves_ambiguity() {
68+
SpringContextModel model = buildModel("EnvironmentComponentA.java", "EnvironmentComponentB.java", "QualifierConsumer.java");
69+
assertThat(check.findAmbiguousDependencies(model)).isEmpty();
70+
}
71+
72+
@Test
73+
void single_candidate_does_not_raise_issue() {
74+
SpringContextModel model = buildModel("ResourceLoaderComponent.java", "SingleCandidateConsumer.java");
75+
assertThat(check.findAmbiguousDependencies(model)).isEmpty();
76+
}
77+
78+
/**
79+
* Runs {@link BeanDefinitionGatherer} over the given files (relative to {@link #BASE_PATH}) into a single,
80+
* freshly built {@link SpringContextModel}, mirroring how {@code JavaSensor} drives gatherers during a real
81+
* analysis, without needing java-frontend's test-only scanning helpers.
82+
*/
83+
private static SpringContextModel buildModel(String... relativeFilePaths) {
84+
List<File> classpath = TestClasspathUtils.DEFAULT_MODULE.getClassPath();
85+
SonarComponents sonarComponents = new SonarComponents(null, null, null, null, null, null);
86+
sonarComponents.setSensorContext(SensorContextTester.create(new File("")));
87+
SpringContextModel model = new SpringContextModel();
88+
sonarComponents.setSpringContextModel(model);
89+
90+
BeanDefinitionGatherer gatherer = new BeanDefinitionGatherer();
91+
VisitorsBridge visitorsBridge = new VisitorsBridge(List.of((JavaCheck) gatherer), classpath, sonarComponents);
92+
for (String relativeFilePath : relativeFilePaths) {
93+
File file = new File(TestUtils.mainCodeSourcesPath(BASE_PATH + relativeFilePath));
94+
CompilationUnitTree compilationUnit = parse(file, classpath);
95+
visitorsBridge.setCurrentFile(inputFile(file));
96+
visitorsBridge.visitFile(compilationUnit, false);
97+
}
98+
visitorsBridge.endOfAnalysis();
99+
return model;
100+
}
101+
102+
private static InputFile inputFile(File file) {
103+
try {
104+
return new TestInputFileBuilder("", file.getParentFile(), file)
105+
.setContents(Files.readString(file.toPath(), StandardCharsets.UTF_8))
106+
.setCharset(StandardCharsets.UTF_8)
107+
.setLanguage("java")
108+
.setType(InputFile.Type.MAIN)
109+
.build();
110+
} catch (IOException e) {
111+
throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e);
112+
}
113+
}
114+
115+
private static CompilationUnitTree parse(File file, List<File> classpath) {
116+
String source;
117+
try {
118+
source = Files.readString(file.toPath(), StandardCharsets.UTF_8);
119+
} catch (Exception e) {
120+
throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e);
121+
}
122+
JavaVersion version = JParserConfig.MAXIMUM_SUPPORTED_JAVA_VERSION;
123+
return JParser.parse(JParserConfig.Mode.FILE_BY_FILE.create(version, classpath).astParser(), version.toString(), file.getName(), source);
124+
}
125+
126+
}

‎java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionRegistry.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,15 @@ public List<BeanDefinitionHolder> getByName(String beanName) {
4242
return beanDefinitions.getOrDefault(beanName, List.of());
4343
}
4444

45+
/**
46+
* Returns every bean definition registered so far, regardless of the name it is registered under.
47+
*/
48+
public List<BeanDefinitionHolder> getAll() {
49+
return beanDefinitions.values().stream()
50+
.flatMap(List::stream)
51+
.toList();
52+
}
53+
4554
public void addBeanDefinition(String beanName, BeanDefinitionHolder beanDefinition) {
4655
beanDefinitions.computeIfAbsent(beanName, k -> new ArrayList<>()).add(beanDefinition);
4756
}

‎sonar-java-plugin/src/main/java/org/sonar/plugins/java/SpringContextModelSensor.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,15 @@
1919
import org.sonar.api.batch.Phase;
2020
import org.sonar.api.batch.sensor.SensorContext;
2121
import org.sonar.api.batch.sensor.SensorDescriptor;
22+
import org.sonar.api.batch.sensor.issue.NewIssue;
23+
import org.sonar.api.rule.RuleKey;
2224
import org.sonar.api.scanner.sensor.ProjectSensor;
25+
import org.sonar.java.GeneratedCheckList;
26+
import org.sonar.java.checks.spring.AmbiguousDependencyCheck;
2327
import org.sonar.java.jsp.Jasper;
28+
import org.sonar.java.model.springcontext.BeanLocation;
2429
import org.sonar.java.model.springcontext.SpringContextModel;
30+
import org.sonar.java.reporting.AnalyzerMessage;
2531

2632
/**
2733
* A post-phase {@link ProjectSensor} that holds the shared {@link SpringContextModel} built during analysis.
@@ -49,7 +55,21 @@ public void describe(SensorDescriptor descriptor) {
4955

5056
@Override
5157
public void execute(SensorContext context) {
52-
// Nothing to do for now
58+
reportAmbiguousDependencies(context);
59+
}
60+
61+
private void reportAmbiguousDependencies(SensorContext context) {
62+
RuleKey ruleKey = RuleKey.of(GeneratedCheckList.REPOSITORY_KEY, "S9352");
63+
for (var ambiguousDependency : new AmbiguousDependencyCheck().findAmbiguousDependencies(springContextModel)) {
64+
BeanLocation location = ambiguousDependency.location();
65+
AnalyzerMessage.TextSpan span = location.mainLocation();
66+
NewIssue newIssue = context.newIssue().forRule(ruleKey);
67+
newIssue.at(newIssue.newLocation()
68+
.on(location.inputFile())
69+
.at(location.inputFile().newRange(span.startLine, span.startCharacter, span.endLine, span.endCharacter))
70+
.message(ambiguousDependency.message()));
71+
newIssue.save();
72+
}
5373
}
5474
}
5575

‎sonar-java-plugin/src/test/java/org/sonar/plugins/java/SpringContextModelSensorTest.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,36 @@
1616
*/
1717
package org.sonar.plugins.java;
1818

19+
import java.io.File;
20+
import java.io.IOException;
21+
import java.nio.charset.StandardCharsets;
22+
import java.nio.file.Files;
23+
import java.util.List;
1924
import org.junit.jupiter.api.Test;
25+
import org.sonar.api.batch.fs.InputFile;
26+
import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
2027
import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor;
28+
import org.sonar.api.batch.sensor.internal.SensorContextTester;
29+
import org.sonar.api.batch.sensor.issue.Issue;
30+
import org.sonar.api.rule.RuleKey;
31+
import org.sonar.java.SonarComponents;
32+
import org.sonar.java.checks.verifier.TestUtils;
33+
import org.sonar.java.model.JParser;
34+
import org.sonar.java.model.JParserConfig;
35+
import org.sonar.java.model.VisitorsBridge;
36+
import org.sonar.java.model.springcontext.BeanDefinitionGatherer;
2137
import org.sonar.java.model.springcontext.SpringContextModel;
38+
import org.sonar.java.test.classpath.TestClasspathUtils;
39+
import org.sonar.plugins.java.api.JavaCheck;
40+
import org.sonar.plugins.java.api.JavaVersion;
41+
import org.sonar.plugins.java.api.tree.CompilationUnitTree;
2242

2343
import static org.assertj.core.api.Assertions.assertThat;
2444

2545
class SpringContextModelSensorTest {
2646

47+
private static final String BASE_PATH = "checks/spring/s9352/";
48+
2749
@Test
2850
void test_toString() {
2951
DefaultSensorDescriptor descriptor = new DefaultSensorDescriptor();
@@ -33,4 +55,80 @@ void test_toString() {
3355
assertThat(descriptor.languages()).containsExactly("java", "jsp");
3456
}
3557

58+
@Test
59+
void reports_an_issue_for_an_ambiguous_dependency() {
60+
SensorContextTester context = SensorContextTester.create(new File(""));
61+
SpringContextModel model = buildModel(context, "ComponentOne.java", "ComponentTwo.java", "UnresolvedConsumer.java");
62+
63+
new SpringContextModelSensor(model).execute(context);
64+
65+
assertThat(context.allIssues()).hasSize(1);
66+
Issue issue = context.allIssues().iterator().next();
67+
assertThat(issue.ruleKey()).isEqualTo(RuleKey.of("java", "S9352"));
68+
assertThat(issue.primaryLocation().message())
69+
.isEqualTo("Multiple beans of type \"org.springframework.context.ApplicationContextAware\" match this dependency"
70+
+ " (componentOne, componentTwo); disambiguate it with \"@Qualifier\" or mark one bean as \"@Primary\".");
71+
assertThat(issue.primaryLocation().textRange().start().line()).isEqualTo(10);
72+
}
73+
74+
@Test
75+
void reports_no_issue_when_only_one_candidate_exists() {
76+
SensorContextTester context = SensorContextTester.create(new File(""));
77+
SpringContextModel model = buildModel(context, "ResourceLoaderComponent.java", "SingleCandidateConsumer.java");
78+
79+
new SpringContextModelSensor(model).execute(context);
80+
81+
assertThat(context.allIssues()).isEmpty();
82+
}
83+
84+
/**
85+
* Runs {@link BeanDefinitionGatherer} over the given files (relative to {@link #BASE_PATH}) into a single,
86+
* freshly built {@link SpringContextModel}, registering each file's {@link InputFile} on the given
87+
* {@link SensorContextTester} so that issues reported against it can be resolved.
88+
*/
89+
private static SpringContextModel buildModel(SensorContextTester context, String... relativeFilePaths) {
90+
List<File> classpath = TestClasspathUtils.DEFAULT_MODULE.getClassPath();
91+
SonarComponents sonarComponents = new SonarComponents(null, null, null, null, null, null);
92+
sonarComponents.setSensorContext(context);
93+
SpringContextModel model = new SpringContextModel();
94+
sonarComponents.setSpringContextModel(model);
95+
96+
BeanDefinitionGatherer gatherer = new BeanDefinitionGatherer();
97+
VisitorsBridge visitorsBridge = new VisitorsBridge(List.of((JavaCheck) gatherer), classpath, sonarComponents);
98+
for (String relativeFilePath : relativeFilePaths) {
99+
File file = new File(TestUtils.mainCodeSourcesPath(BASE_PATH + relativeFilePath));
100+
CompilationUnitTree compilationUnit = parse(file, classpath);
101+
InputFile inputFile = inputFile(file);
102+
context.fileSystem().add(inputFile);
103+
visitorsBridge.setCurrentFile(inputFile);
104+
visitorsBridge.visitFile(compilationUnit, false);
105+
}
106+
visitorsBridge.endOfAnalysis();
107+
return model;
108+
}
109+
110+
private static InputFile inputFile(File file) {
111+
try {
112+
return new TestInputFileBuilder("", file.getParentFile(), file)
113+
.setContents(Files.readString(file.toPath(), StandardCharsets.UTF_8))
114+
.setCharset(StandardCharsets.UTF_8)
115+
.setLanguage("java")
116+
.setType(InputFile.Type.MAIN)
117+
.build();
118+
} catch (IOException e) {
119+
throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e);
120+
}
121+
}
122+
123+
private static CompilationUnitTree parse(File file, List<File> classpath) {
124+
String source;
125+
try {
126+
source = Files.readString(file.toPath(), StandardCharsets.UTF_8);
127+
} catch (Exception e) {
128+
throw new IllegalStateException("Unable to read file '" + file.getAbsolutePath() + "'", e);
129+
}
130+
JavaVersion version = JParserConfig.MAXIMUM_SUPPORTED_JAVA_VERSION;
131+
return JParser.parse(JParserConfig.Mode.FILE_BY_FILE.create(version, classpath).astParser(), version.toString(), file.getName(), source);
132+
}
133+
36134
}

0 commit comments

Comments
 (0)