Skip to content

SONARJAVA-5803 JSpecify @NullUnmarked should be treated as unknown #5327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 27, 2025
Merged

SONARJAVA-5803 JSpecify @NullUnmarked should be treated as unknown #5327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e3a8ee
SONARJAVA-5803 JSpecify @NullUnmarked should be treated as unknown
andreaguarino Oct 23, 2025
c69c346
Update autoscan expectations
andreaguarino Oct 23, 2025
87a44fb
Fixes after review
andreaguarino Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion its/autoscan/src/test/resources/autoscan/diffs/diff_S2638.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S2638",
"hasTruePositives": true,
"falseNegatives": 22,
"falseNegatives": 20,
"falsePositives": 0
}
2 changes: 1 addition & 1 deletion its/autoscan/src/test/resources/autoscan/diffs/diff_S2789.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S2789",
"hasTruePositives": true,
"falseNegatives": 43,
"falseNegatives": 35,
"falsePositives": 0
}
2 changes: 1 addition & 1 deletion its/autoscan/src/test/resources/autoscan/diffs/diff_S4682.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S4682",
"hasTruePositives": true,
"falseNegatives": 10,
"falseNegatives": 2,
"falsePositives": 0
}
2 changes: 1 addition & 1 deletion ...st-sources/default/src/main/java/checks/jspecify/ChangeMethodContractCheckNullMarked.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class ChangeMethodContractCheck_B extends ChangeMethodContractCheck {

@NullMarked
@Override
String annotatedUnmarked(Object a) { return null; } // Noncompliant {{Fix the incompatibility of the annotation @NullMarked to honor @NullUnmarked of the overridden method.}}
String annotatedUnmarked(Object a) { return null; } // Compliant - NullUnmarked doesn't add any information about nullability

}

10 changes: 4 additions & 6 deletions ...t/src/main/java/checks/jspecify/NullShouldNotBeUsedWithOptionalCheckNullMarked_guava.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
@NullMarked
interface NullShouldNotBeUsedWithOptionalCheck_guava {

@NullUnmarked // Noncompliant {{Methods with an "Optional" return type should not be "@NullUnmarked".}}
//^^^^^^^^^^^^^
public Optional<String> getOptionalKo();
@NullUnmarked // Compliant - NullUnmarked doesn't say anything about nullability
public Optional<String> getOptional();

}

Expand All @@ -22,9 +21,8 @@ class NullShouldNotBeUsedWithOptionalCheck_guavaClassA {
public NullShouldNotBeUsedWithOptionalCheck_guavaClassA() {
}

@NullUnmarked // Noncompliant {{Methods with an "Optional" return type should not be "@NullUnmarked".}}
//^^^^^^^^^^^^^
public Optional<String> getOptionalKo() {
@NullUnmarked // Compliant - NullUnmarked doesn't say anything about nullability
public Optional<String> getOptional() {
return null; // Noncompliant {{Methods with an "Optional" return type should never return null.}}
// ^^^^
}
Expand Down
10 changes: 4 additions & 6 deletions ...ult/src/main/java/checks/jspecify/NullShouldNotBeUsedWithOptionalCheckNullMarked_jdk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@
@NullMarked
interface NullShouldNotBeUsedWithOptionalCheck_jdk {

@NullUnmarked // Noncompliant {{Methods with an "Optional" return type should not be "@NullUnmarked".}}
//^^^^^^^^^^^^^
public Optional<String> getOptionalKo();
@NullUnmarked // Compliant - NullUnmarked doesn't say anything about nullability
public Optional<String> getOptional();

}

Expand All @@ -23,9 +22,8 @@ class NullShouldNotBeUsedWithOptionalCheck_jdkClassA {
public NullShouldNotBeUsedWithOptionalCheck_jdkClassA() {
}

@NullUnmarked // Noncompliant {{Methods with an "Optional" return type should not be "@NullUnmarked".}}
//^^^^^^^^^^^^^
public Optional<String> getOptionalKo() {
@NullUnmarked // Compliant - NullUnmarked doesn't say anything about nullability
public Optional<String> getOptional() {
return null; // Noncompliant {{Methods with an "Optional" return type should never return null.}}
// ^^^^
}
Expand Down
12 changes: 3 additions & 9 deletions ...ources/default/src/main/java/checks/jspecify/PrimitivesMarkedNullableCheckNullMarked.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,10 @@ abstract class PrimitivesMarkedNullableCheckSample {
abstract int getInt2();

@NullUnmarked
protected abstract boolean isBool(); // Noncompliant {{"@NullUnmarked" annotation should not be used on primitive types}} [[quickfixes=qf1]]
// ^^^^^^^
// fix@qf1 {{Remove "@NullUnmarked"}}
// edit@qf1 [[sl=-1;el=+0;sc=3;ec=3]] {{}}
protected abstract boolean isBool(); // Compliant

@NullUnmarked
public double getDouble1() { return 0.0; } // Noncompliant {{"@NullUnmarked" annotation should not be used on primitive types}}
public double getDouble1() { return 0.0; } // Compliant

public double getDouble2() { return 0.0; }

Expand All @@ -49,10 +46,7 @@ abstract class PrimitivesMarkedNullableCheckSample {

public Object getObj2() { return null; }

protected abstract @NullUnmarked boolean isBool2(); // Noncompliant {{"@NullUnmarked" annotation should not be used on primitive types}} [[quickfixes=qf3]]
// ^^^^^^^
// fix@qf3 {{Remove "@NullUnmarked"}}
// edit@qf3 [[sc=22;ec=36]] {{}}
protected abstract @NullUnmarked boolean isBool2(); // Compliant

@NullUnmarked
Object containsAnonymousClass() {
Expand Down
6 changes: 3 additions & 3 deletions ...ces/default/src/main/java/checks/jspecify/RedundantNullabilityAnnotationsCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ class InnerNullUnmarked {
@NullUnmarked
class Inner {
@NonNull Object o; // Compliant
public void methodNonNullParamTyped(List<@Nullable Object> o) { // Noncompliant {{Remove redundant annotation @Nullable as inside scope annotation @NullUnmarked at class level.}}
public void methodNonNullParamTyped(List<@Nullable Object> o) { // Compliant
// ..
}
}
Expand All @@ -291,15 +291,15 @@ class InnerInner {}
class InnerRedundantNullUnmarked {
@NullUnmarked // Compliant
class Inner {
@NullUnmarked // Noncompliant {{Remove redundant annotation @NullUnmarked at class level as inside scope annotation @NullUnmarked at class level.}}
@NullUnmarked // FN
class InnerInner {}
}
}
}

@NullUnmarked
class InnerClassTestsTwo {
@NullUnmarked // Noncompliant {{Remove redundant annotation @NullUnmarked at class level as inside scope annotation @NullUnmarked at class level.}}
@NullUnmarked // FN
class InnerNullmarked {
@NullMarked // Compliant
class Inner {
Expand Down
2 changes: 1 addition & 1 deletion ...t-sources/default/src/main/java/checks/jspecify/nullmarked/ChangeMethodContractCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class ChangeMethodContractCheck_B extends ChangeMethodContractCheck {

@NullMarked
@Override
String annotatedUnmarked(Object a) { return null; } // Noncompliant {{Fix the incompatibility of the annotation @NullMarked to honor @NullUnmarked of the overridden method.}}
String annotatedUnmarked(Object a) { return null; } // Compliant - NullUnmarked doesn't add any information about nullability

}

10 changes: 4 additions & 6 deletions .../src/main/java/checks/jspecify/nullmarked/NullShouldNotBeUsedWithOptionalCheck_guava.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@
// NullMarked at the package level
interface NullShouldNotBeUsedWithOptionalCheck_guava {

@NullUnmarked // Noncompliant {{Methods with an "Optional" return type should not be "@NullUnmarked".}}
//^^^^^^^^^^^^^
public Optional<String> getOptionalKo();
@NullUnmarked // Compliant - NullUnmarked doesn't say anything about nullability
public Optional<String> getOptional();

}

Expand All @@ -21,9 +20,8 @@ class NullShouldNotBeUsedWithOptionalCheck_guavaClassA {
public NullShouldNotBeUsedWithOptionalCheck_guavaClassA() {
}

@NullUnmarked // Noncompliant {{Methods with an "Optional" return type should not be "@NullUnmarked".}}
//^^^^^^^^^^^^^
public Optional<String> getOptionalKo() {
@NullUnmarked // Compliant - NullUnmarked doesn't say anything about nullability
public Optional<String> getOptional() {
return null; // Noncompliant {{Methods with an "Optional" return type should never return null.}}
// ^^^^
}
Expand Down
10 changes: 4 additions & 6 deletions ...lt/src/main/java/checks/jspecify/nullmarked/NullShouldNotBeUsedWithOptionalCheck_jdk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
// NullMarked at the package level
interface NullShouldNotBeUsedWithOptionalCheck_jdk {

@NullUnmarked // Noncompliant {{Methods with an "Optional" return type should not be "@NullUnmarked".}}
//^^^^^^^^^^^^^
public Optional<String> getOptionalKo();
@NullUnmarked // Compliant - NullUnmarked doesn't say anything about nullability
public Optional<String> getOptional();

}

Expand All @@ -22,9 +21,8 @@ class NullShouldNotBeUsedWithOptionalCheck_jdkClassA {
public NullShouldNotBeUsedWithOptionalCheck_jdkClassA() {
}

@NullUnmarked // Noncompliant {{Methods with an "Optional" return type should not be "@NullUnmarked".}}
//^^^^^^^^^^^^^
public Optional<String> getOptionalKo() {
@NullUnmarked // Compliant - NullUnmarked doesn't say anything about nullability
public Optional<String> getOptional() {
return null; // Noncompliant {{Methods with an "Optional" return type should never return null.}}
// ^^^^
}
Expand Down
12 changes: 3 additions & 9 deletions ...urces/default/src/main/java/checks/jspecify/nullmarked/PrimitivesMarkedNullableCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,10 @@ abstract class PrimitivesMarkedNullableCheckSample {
abstract int getInt2();

@NullUnmarked
protected abstract boolean isBool(); // Noncompliant {{"@NullUnmarked" annotation should not be used on primitive types}} [[quickfixes=qf1]]
// ^^^^^^^
// fix@qf1 {{Remove "@NullUnmarked"}}
// edit@qf1 [[sl=-1;el=+0;sc=3;ec=3]] {{}}
protected abstract boolean isBool(); // Compliant - NullUnmarked doesn't add any information about nullability
Comment on lines 21 to +22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using @NullUnmarked in these tests was a mistake. We should migrate to @Nullable as the test is doesn't prove anything otherwise...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the test was there to assert how the check "PrimitivesMarkedNullableCheck" behave in presence of "@NullMarked" / "@NullUnmarked" annotations, so I'm not sure we should replace it with "@nullable" because it's already asserted inside "checks/PrimitivesMarkedNullableCheckSample". So maybe we should just remove this test file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I eventually decided to let the existing testcase and just replace the Noncompliant -> Compliant, to still verify we don't raise FPs


@NullUnmarked
public double getDouble1() { return 0.0; } // Noncompliant {{"@NullUnmarked" annotation should not be used on primitive types}}
public double getDouble1() { return 0.0; } // Compliant - NullUnmarked doesn't add any information about nullability

public double getDouble2() { return 0.0; }

Expand All @@ -49,10 +46,7 @@ abstract class PrimitivesMarkedNullableCheckSample {

public Object getObj2() { return null; }

protected abstract @NullUnmarked boolean isBool2(); // Noncompliant {{"@NullUnmarked" annotation should not be used on primitive types}} [[quickfixes=qf3]]
// ^^^^^^^
// fix@qf3 {{Remove "@NullUnmarked"}}
// edit@qf3 [[sc=22;ec=36]] {{}}
protected abstract @NullUnmarked boolean isBool2(); // Compliant - NullUnmarked doesn't add any information about nullability

@NullUnmarked
Object containsAnonymousClass() {
Expand Down
13 changes: 12 additions & 1 deletion java-checks/src/main/java/org/sonar/java/checks/RedundantNullabilityAnnotationsCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ public void visitNode(Tree tree) {
// if nullable, either directly or inherited from higher scope
// then check my members are not directly annotated with non-null
checkMembers(classNullabilityData, classTree, NULLABILITY_SCOPE.NULLABLE);
} else if (classNullabilityData.type() == SymbolMetadata.NullabilityType.UNKNOWN) {
// Handle classes with UNKNOWN nullability (e.g., @NullUnmarked)
// Still need to check their members in case they have nested @NullMarked/@NullUnmarked
checkMembers(classNullabilityData, classTree, NULLABILITY_SCOPE.UNSPECIFIED);
}
}
}
Expand Down Expand Up @@ -99,6 +103,12 @@ private void checkInnerClass(SymbolMetadata.NullabilityData classNullabilityData
}
// now recurse to check class members
checkMembers(innerNullabilityData, tree, NULLABILITY_SCOPE.NULLABLE);
} else {
// Handle classes with UNKNOWN nullability (e.g., @NullUnmarked)
// Still need to check their members in case they have nested @NullMarked/@NullUnmarked
if (innerNullabilityData.type() == SymbolMetadata.NullabilityType.UNKNOWN) {
checkMembers(innerNullabilityData, tree, NULLABILITY_SCOPE.UNSPECIFIED);
}
}
}

Expand Down Expand Up @@ -148,7 +158,8 @@ private void reportIssue(Tree reportLocation,
// track class scope nullability state during recursion
private enum NULLABILITY_SCOPE {
NULLABLE,
NON_NULLABLE
NON_NULLABLE,
UNSPECIFIED
}

}
4 changes: 2 additions & 2 deletions java-checks/src/test/java/org/sonar/java/checks/ChangeMethodContractCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ void test_jspecify_null_marked() {
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/jspecify/ChangeMethodContractCheckNullMarked.java"))
.withCheck(new ChangeMethodContractCheck())
.verifyIssues();
.verifyNoIssues();
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/jspecify/nullmarked/ChangeMethodContractCheck.java"))
.withCheck(new ChangeMethodContractCheck())
.verifyIssues();
.verifyNoIssues();
}

@Test
Expand Down
5 changes: 2 additions & 3 deletions java-checks/src/test/java/org/sonar/java/checks/PrimitivesMarkedNullableCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ void test_jspecify_null_marked() {
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/jspecify/PrimitivesMarkedNullableCheckNullMarked.java"))
.withCheck(new PrimitivesMarkedNullableCheck())
.verifyIssues();
.verifyNoIssues();
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/jspecify/nullmarked/PrimitivesMarkedNullableCheck.java"))
.withCheck(new PrimitivesMarkedNullableCheck())
.verifyIssues();
.verifyNoIssues();
}

}
2 changes: 1 addition & 1 deletion java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadataNullabilityHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ private JSymbolMetadataNullabilityHelper() {
configureAnnotation(ORG_JSPECIFY_ANNOTATIONS_NULL_MARKED, NON_NULL,
Arrays.asList(NullabilityTarget.CLASS, FIELD, METHOD, PARAMETER),
Arrays.asList(NullabilityLevel.METHOD, CLASS, PACKAGE));
configureAnnotation(ORG_JSPECIFY_ANNOTATIONS_NULL_UNMARKED, WEAK_NULLABLE,
configureAnnotation(ORG_JSPECIFY_ANNOTATIONS_NULL_UNMARKED, UNKNOWN,
Arrays.asList(NullabilityTarget.CLASS, FIELD, METHOD, PARAMETER),
Arrays.asList(NullabilityLevel.METHOD, CLASS, PACKAGE));

Expand Down