Skip to content

Commit

Permalink
SONARJAVA-4695 fix FP in rule S6804 : stop raising issues when we inj…
Browse files Browse the repository at this point in the history
…ect a Spring resource (#4603)
  • Loading branch information
erwan-serandour authored Dec 6, 2023
1 parent 7db58b1 commit e739ac5
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S6804",
"hasTruePositives": false,
"falseNegatives": 11,
"falseNegatives": 12,
"falsePositives": 0
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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:");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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();
}
}

0 comments on commit e739ac5

Please sign in to comment.