Skip to content

Commit

Permalink
SONARJAVA-4682 S6816: Nullable injected fields should provide a defau…
Browse files Browse the repository at this point in the history
…lt value
  • Loading branch information
dorian-burihabwa-sonarsource committed Nov 6, 2023
1 parent e17b9ad commit f25d9b7
Show file tree
Hide file tree
Showing 10 changed files with 451 additions and 3 deletions.
4 changes: 2 additions & 2 deletions its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,15 @@ public void javaCheckTestSources() throws Exception {
softly.assertThat(newTotal).isEqualTo(knownTotal);
softly.assertThat(rulesCausingFPs).hasSize(6);
softly.assertThat(rulesNotReporting).hasSize(7);
softly.assertThat(rulesSilenced).hasSize(81);
softly.assertThat(rulesSilenced).hasSize(82);

/**
* 4. Check total number of differences (FPs + FNs)
*
* No differences would mean that we find the same issues with and without the bytecode and libraries
*/
String differences = Files.readString(pathFor(TARGET_ACTUAL + PROJECT_KEY + "-no-binaries_differences"));
softly.assertThat(differences).isEqualTo("Issues differences: 3579");
softly.assertThat(differences).isEqualTo("Issues differences: 3594");

softly.assertAll();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2876,7 +2876,7 @@
{
"ruleKey": "S6804",
"hasTruePositives": false,
"falseNegatives": 9,
"falseNegatives": 11,
"falsePositives": 0
},
{
Expand Down Expand Up @@ -2909,6 +2909,12 @@
"falseNegatives": 5,
"falsePositives": 0
},
{
"ruleKey": "S6816",
"hasTruePositives": false,
"falseNegatives": 11,
"falsePositives": 0
},
{
"ruleKey": "S6817",
"hasTruePositives": false,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package checks.spring;

class Constants {
public static final String NON_COMPLIANT_PROPERTY_EXPRESSION_WITHOUT_NULLABLE_DEFAULT = "${myPropertyWithoutDefault}" ;
private Constants() {
/* No instance needed here */
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package checks.spring;

import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import org.springframework.beans.factory.annotation.Value;

public class NullableInjectedFieldsHaveDefaultValueSample {
@Nullable
@Value("${my.property}") // Noncompliant [[sc=3;ec=27;secondary=-1]] {{Provide a default null value for this field.}}
private String myProperty;

@Value("${my.property}") // Noncompliant [[sc=3;ec=27;secondary=+1]] {{Provide a default null value for this field.}}
@Nullable
private String myPropertyWithReversedAnnotations;

@Nullable
@Value(value = "${my.property}") // Noncompliant [[sc=3;ec=35]]
private String myOtherProperty;

@Nullable
@Value("${my.property}") // Noncompliant [[sc=3;ec=27;quickfixes=literalfix]]
private String myPropertyToFix;
// fix@literalfix {{Set null as default value}}
// edit@literalfix [[sc=10;ec=26]] {{"${my.property:#{null}}"}}

@Nullable
@Value(value = "${my.property}") // Noncompliant [[sc=3;ec=35;quickfixes=argumentfix]]
private String myPropertyToFixOnNamedArgument;
// fix@argumentfix {{Set null as default value}}
// edit@argumentfix [[sc=18;ec=34]] {{"${my.property:#{null}}"}}

public void foo(@Nullable @Value("${my.property}") String argument) { // Noncompliant [[sc=29;ec=53;secondary=+0]] {{Provide a default null value for this parameter.}}
/* Do something */
}

public void bar(@Value(value = "${my.property}") @Nullable String argument) { // Noncompliant [[sc=19;ec=51;secondary=+0]] {{Provide a default null value for this parameter.}}
/* Do something */
}

private static final String NON_COMPLIANT_IN_A_CONSTANT = "${non.compliant.constant}";
//fix@fixStaticConstant {{Set null as default value}}
//edit@fixStaticConstant [[sl=40;el=40;sc=61;ec=88]] {{"${non.compliant.constant:#{null}}"}}
@Nullable
@Value(value = NON_COMPLIANT_IN_A_CONSTANT) // Noncompliant [[sc=3;ec=46;secondary=-1;quickfixes=fixStaticConstant,localFixOnStaticConstant]]
//fix@localFixOnStaticConstant {{Set null as default value locally}}
//edit@localFixOnStaticConstant [[sc=18;ec=45]] {{"${non.compliant.constant:#{null}}"}}
private String myNoncompliantIndirectProperty;

private final String finalButNotStatic = "${non.compliant.constant}";
//fix@fixConstant {{Set null as default value}}
//edit@fixConstant [[sl=49;el=49;sc=44;ec=71]] {{"${non.compliant.constant:#{null}}"}}
@Nullable
@Value(finalButNotStatic) // Noncompliant [[sc=3;ec=28;secondary=-1;quickfixes=fixConstant,localFixOnConstant]]
// fix@localFixOnConstant {{Set null as default value locally}}
//edit@localFixOnConstant [[sc=10;ec=27]] {{"${non.compliant.constant:#{null}}"}}
private String myOtherNoncompliantIndirectProperty;

@Nullable
@Value(Constants.NON_COMPLIANT_PROPERTY_EXPRESSION_WITHOUT_NULLABLE_DEFAULT) // Noncompliant [[sc=3;ec=79;quickfixes=fixEternal]]
// fix@fixEternal {{Set null as default value locally}}
// edit@fixEternal [[sc=10;ec=78]] {{"${myPropertyWithoutDefault:#{null}}"}}
private String resolvableButForeignConstant;

private static final String DEFINED_IN_A_CONSTANT = "${my.constant:#{null}}";
@Nullable
@Value(value = DEFINED_IN_A_CONSTANT) // Compliant
private String myCompliantIndirectProperty;

private String myNonCompliantAttribute;

@Value("${my.property}") // Noncompliant [[sc=3;ec=27;secondary=+1]]
public void setMyNonCompliantAttribute(@Nullable String myNonCompliantAttribute) {
this.myNonCompliantAttribute = myNonCompliantAttribute;
}

@Nullable
@Value(value = DEFINED_IN_A_CONSTANT) // Compliant
private String myOtherCompliantIndirectProperty;

@Nullable
@Value("myProperty") // Compliant for S6816, even if wrong by other rules
private String myPropertyNotUsingSpEL;

@Nullable
@Value("${myProperty") // Compliant for S6816, even if wrong by other rules
private String myPropertyWithBrokenSpEL;

@Nullable
@Value("${my.property:#{null}}") // Compliant, a default value is provided
private String myCompliantProperty;

@Value(value = "${my.property}") // Compliant, the field is not nullable
private String myNonNullProperty;

@Value("#{null}")
@Nullable
private String nullAnyway;


private String myCompliantAttribute;

@Value("${my.property:#{null}}") // Compliant
public void setMyCompliantAttribute(@Nullable String myCompliantAttribute) {
this.myCompliantAttribute = myCompliantAttribute;
}

@CheckForNull
private String setNothing(@Nullable String useless) {
return useless;
}

@Value("${my.property}") // Compliant not a setter
private String setNothingWithTwoParameters(@Nullable String a, String b) {
a = b;
return a;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
import org.sonar.java.checks.spring.FieldDependencyInjectionCheck;
import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck;
import org.sonar.java.checks.spring.NonSingletonAutowiredInSingletonCheck;
import org.sonar.java.checks.spring.NullableInjectedFieldsHaveDefaultValueCheck;
import org.sonar.java.checks.spring.OptionalRestParametersShouldBeObjectsCheck;
import org.sonar.java.checks.spring.PersistentEntityUsedAsRequestParameterCheck;
import org.sonar.java.checks.spring.RequestMappingMethodPublicCheck;
Expand Down Expand Up @@ -524,6 +525,7 @@ public final class CheckList {
NonShortCircuitLogicCheck.class,
NonStaticClassInitializerCheck.class,
NotifyCheck.class,
NullableInjectedFieldsHaveDefaultValueCheck.class,
NullCheckWithInstanceofCheck.class,
NullReturnedOnComputeIfPresentOrAbsentCheck.class,
OSCommandsPathCheck.class,
Expand Down
Loading

0 comments on commit f25d9b7

Please sign in to comment.