From e739ac5a057807c05dc6e4e81acc039761cdde25 Mon Sep 17 00:00:00 2001 From: erwan-serandour-sonarsource Date: Wed, 6 Dec 2023 13:45:23 +0100 Subject: [PATCH] SONARJAVA-4695 fix FP in rule S6804 : stop raising issues when we inject a Spring resource (#4603) --- .../resources/autoscan/diffs/diff_S6804.json | 4 ++-- ...ectPropertyOrSpELCheckSampleNonCompiling.java | 8 ++++++++ ...ionShouldInjectPropertyOrSpELCheckSample.java | 16 ++++++++++++++-- ...nnotationShouldInjectPropertyOrSpELCheck.java | 8 +++++--- ...ationShouldInjectPropertyOrSpELCheckTest.java | 9 +++++++++ 5 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSampleNonCompiling.java diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6804.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6804.json index 15ef30254fd..0b2a2f89b4e 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6804.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6804.json @@ -1,6 +1,6 @@ { "ruleKey": "S6804", "hasTruePositives": false, - "falseNegatives": 11, + "falseNegatives": 12, "falsePositives": 0 -} \ No newline at end of file +} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSampleNonCompiling.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSampleNonCompiling.java new file mode 100644 index 00000000000..3c5989f9dad --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSampleNonCompiling.java @@ -0,0 +1,8 @@ +package checks; + +import org.springframework.beans.factory.annotation.Value; + +class ValueAnnotationShouldInjectPropertyOrSpELCheckSampleNonCompiling { + @org.springframework.beans.factory.annotation.Value(null) // compliant + String wrongannotationValue; +} diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSample.java index 4e08d6289f2..87742fb15f9 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSample.java @@ -41,11 +41,11 @@ class ValueAnnotationShouldInjectPropertyOrSpELCheckSample { @Value("") // Noncompliant String empty; - public void setValue(@Value("xxx") String x){ // compliant + public void setValue(@Value("xxx") String x){ // Compliant } @Value("xxx") - public void setValueA(String x){ // compliant + public void setValueA(String x){ // Compliant } @Value("${a") // Noncompliant @@ -56,6 +56,18 @@ public void setValueA(String x){ // compliant @Autowired String c; + + @Value("classpath:some.xml") // Compliant + String classpath; + + @Value("file:aPath") // Compliant + String file; + + @Value("url:anUrl") // Compliant + String url; + + @Value("invlalidPrefix:xxxx") // Noncompliant + String notAResource; } @Value("${myValue.ok}") // Compliant diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheck.java index 6f6aeeb3c90..b4debe99161 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheck.java @@ -22,7 +22,6 @@ import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; -import javax.annotation.CheckForNull; import org.sonar.check.Rule; import org.sonar.java.checks.helpers.ExpressionsHelper; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; @@ -67,12 +66,11 @@ public void visitNode(Tree tree) { private static boolean isSimpleSpringValue(AnnotationTree annotation) { if (annotation.symbolType().is(SPRING_VALUE)) { String value = extractArgumentValue(annotation.arguments().get(0)); - return value != null && !(isPropertyName(value) || isSpEL(value)); + return value != null && !isPropertyName(value) && !isSpEL(value) && !referenceResource(value); } return false; } - @CheckForNull private static String extractArgumentValue(ExpressionTree annotationArgument) { if (annotationArgument.is(Tree.Kind.ASSIGNMENT)) { ExpressionTree expression = ((AssignmentExpressionTree) annotationArgument).expression(); @@ -89,4 +87,8 @@ private static boolean isSpEL(String value) { return value.startsWith("#{") && value.endsWith("}"); } + private static boolean referenceResource(String value) { + return value.startsWith("classpath:") || value.startsWith("file:") || value.startsWith("url:"); + } + } diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckTest.java index 8e400a96707..89f4864e2ec 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckTest.java @@ -23,6 +23,7 @@ import org.sonar.java.checks.verifier.CheckVerifier; import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; +import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath; class ValueAnnotationShouldInjectPropertyOrSpELCheckTest { @@ -33,4 +34,12 @@ void test() { .withCheck(new ValueAnnotationShouldInjectPropertyOrSpELCheck()) .verifyIssues(); } + + @Test + void test_non_compiling() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/spring/ValueAnnotationShouldInjectPropertyOrSpELCheckSampleNonCompiling.java")) + .withCheck(new ValueAnnotationShouldInjectPropertyOrSpELCheck()) + .verifyNoIssues(); + } }