Skip to content

Commit 654f816

Browse files
GabrielFleischerGabrielFleischer
authored
SONARJAVA-5801 Attempt to properly get rid of the old nullability API (#5322)
1 parent dbe93a3 commit 654f816

File tree

15 files changed

+156
-175
lines changed

15 files changed

+156
-175
lines changed

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S1172.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S1172",
33
"hasTruePositives": true,
4-
"falseNegatives": 25,
4+
"falseNegatives": 24,
55
"falsePositives": 0
66
}

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S2638.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S2638",
33
"hasTruePositives": true,
4-
"falseNegatives": 14,
4+
"falseNegatives": 22,
55
"falsePositives": 0
66
}

‎java-checks-test-sources/default/src/main/java/annotations/nullability/no_default/NullabilityOfParametrizedTypes_Old.java‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

‎java-checks-test-sources/default/src/main/java/checks/S2638_ChangeMethodContractCheck/noPackageInfo/ChangeMethodContractCheck.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package checks.S2638_ChangeMethodContractCheck.noPackageInfo;
22

3+
import java.util.function.Function;
34
import javax.annotation.meta.When;
45
import java.util.List;
56

@@ -45,6 +46,8 @@ void argAnnotatedNonNull(@javax.annotation.Nonnull Object a, @javax.annotation.N
4546
List<@org.jspecify.annotations.Nullable String> typeAnnotatedNullableJSpecify(Object a) { return List.of(); }
4647

4748
List<@org.jspecify.annotations.NonNull String> typeAnnotatedNonNullJSpecify(Object a) { return List.of(); }
49+
50+
void typeAnnotatedNullableFunction(Function<@org.jspecify.annotations.Nullable Object, @org.jspecify.annotations.Nullable Object> func) { }
4851
}
4952

5053
class ChangeMethodContractCheck_B extends ChangeMethodContractCheck {
@@ -95,6 +98,9 @@ void unrelatedMethod(Object a) { }
9598

9699
List<@org.jspecify.annotations.Nullable String> typeAnnotatedNonNullJSpecify(Object a) { return List.of(); } // Noncompliant
97100

101+
@Override
102+
void typeAnnotatedNullableFunction(Function<@org.jspecify.annotations.NonNull Object, @org.jspecify.annotations.NonNull Object> func) { } // Noncompliant 2
103+
98104
public boolean equals(Object o) { return false; } // Compliant: no nullable annotation
99105
}
100106

@@ -119,6 +125,9 @@ void argAnnotatedNonNull(@javax.annotation.Nonnull Object a, @javax.validation.c
119125
String annotatedNonNull(Object a) { return null; } // Noncompliant {{Fix the incompatibility of the annotation @Nullable to honor @Nonnull of the overridden method.}}
120126
//^^^^^^
121127

128+
@Override
129+
void typeAnnotatedNullableFunction(Function func) { } // Compliant (and does not crash)
130+
122131
public boolean equals(@javax.annotation.Nonnull Object o) { return false; } // Compliant, handled by S4454.
123132
}
124133

@@ -253,7 +262,7 @@ class ImplementsFunction implements com.google.common.base.Function<String, Stri
253262
@Override
254263
@javax.annotation.Nonnull
255264
public String apply(@lombok.NonNull String s) { // Noncompliant {{Fix the incompatibility of the annotation @NonNull to honor @Nonnull(when=UNKNOWN) via meta-annotation of the overridden method.}}
256-
// ^
265+
// ^^^^^^
257266
return null;
258267
}
259268

‎java-checks-test-sources/default/src/main/java/checks/S2638_ChangeMethodContractCheck/nullableApi/ChangeMethodContractCheck.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ void argAnnotatedNonNull(@Nonnull Object a) { }
1212
class ChangeMethodContractCheck_Child extends ChangeMethodContractCheck {
1313
@Override
1414
void nullableArguments(@javax.annotation.Nonnull Object a) { } // Noncompliant {{Fix the incompatibility of the annotation @Nonnull to honor @ParametersAreNullableByDefault at package level of the overridden method.}}
15-
// ^^^^^^^^^^^^^^^^^^^^^^^^^> ^ 1
15+
// ^^^^^^^^^^^^^^^^^^^^^^^^^>^^^^^^ 1
1616
@Override
1717
void argAnnotatedNonNull(Object a) { } // Nonnull to Nullable is compliant
1818
}
@@ -22,7 +22,7 @@ void argAnnotatedNonNull(Object a) { } // Nonnull to Nullable is compliant
2222
class ChangeMethodContractCheck_Child_Annotated extends ChangeMethodContractCheck {
2323
@Override
2424
void nullableArguments(Object a) { } // Noncompliant {{Fix the incompatibility of the annotation @ParametersAreNonnullByDefault at class level to honor @ParametersAreNullableByDefault at package level of the overridden method.}}
25-
// ^
25+
// ^^^^^^
2626

2727
@Override
2828
void argAnnotatedNonNull(Object a) { } // Compliant: Nonnull to Nonnull

‎java-checks/src/main/java/org/sonar/java/checks/ChangeMethodContractCheck.java‎

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
import org.sonar.plugins.java.api.semantic.SymbolMetadata;
3030
import org.sonar.plugins.java.api.semantic.SymbolMetadata.NullabilityData;
3131
import org.sonar.plugins.java.api.tree.MethodTree;
32+
import org.sonar.plugins.java.api.tree.ParameterizedTypeTree;
3233
import org.sonar.plugins.java.api.tree.Tree;
34+
import org.sonar.plugins.java.api.tree.TypeTree;
3335
import org.sonar.plugins.java.api.tree.VariableTree;
3436

3537
import static org.sonar.java.checks.helpers.NullabilityDataUtils.nullabilityAsString;
@@ -62,35 +64,60 @@ private void checkContractChange(MethodTree methodTree, Symbol.MethodSymbol over
6264
// Handled by S4454.
6365
return;
6466
}
67+
6568
for (int i = 0; i < methodTree.parameters().size(); i++) {
6669
VariableTree parameter = methodTree.parameters().get(i);
6770
checkParameter(parameter, JUtils.parameterAnnotations(overridee, i));
6871
}
6972

7073
// If the method from the parent claims to never return null, the method from the child
7174
// that can actually be executed at runtime should not return null.
72-
NullabilityData overrideeNullability = overridee.metadata().oldNullabilityData();
73-
if (overrideeNullability.isNonNull(PACKAGE, false, false)) {
74-
NullabilityData methodNullability = methodTree.symbol().metadata().oldNullabilityData();
75-
if (methodNullability.isNullable(PACKAGE, false, false)) {
76-
// returnType() returns null in case of constructor: the rule does not support them.
77-
reportIssue(methodTree.returnType(), overrideeNullability, methodNullability);
78-
}
75+
var returnType = methodTree.returnType();
76+
// returnType() returns null in case of constructor: the rule does not support them.
77+
if (returnType == null) {
78+
return;
7979
}
80+
81+
compareNullability(returnType, methodTree.symbol().metadata(), overridee.metadata(), true);
8082
}
8183

82-
private void checkParameter(VariableTree parameter, SymbolMetadata overrideeParamMetadata) {
83-
// Annotations on parameters is the opposite of return value: if arguments of the parent can be null, the child method has to accept null value.
84-
NullabilityData overrideeParamNullability = overrideeParamMetadata.oldNullabilityData();
85-
if (overrideeParamNullability.isNullable(PACKAGE, false, false)) {
86-
NullabilityData paramNullability = parameter.symbol().metadata().oldNullabilityData();
87-
if (paramNullability.isNonNull(PACKAGE, false, false)) {
88-
reportIssue(parameter.simpleName(), overrideeParamNullability, paramNullability);
89-
}
84+
private void compareNullability(TypeTree tree, SymbolMetadata upperBound, SymbolMetadata lowerBound, boolean overriddenIsLowerBound) {
85+
// Check current level
86+
if (upperBound.nullabilityData().isNullable(PACKAGE, false, false)
87+
&& lowerBound.nullabilityData().isNonNull(PACKAGE, false, false)) {
88+
reportIssue(tree, lowerBound.nullabilityData(), upperBound.nullabilityData(), overriddenIsLowerBound);
89+
}
90+
91+
// Check type parameters
92+
var upperParams = upperBound.parametersMetadata();
93+
var lowerParams = lowerBound.parametersMetadata();
94+
if (upperParams.length != lowerParams.length) {
95+
return;
96+
}
97+
98+
// Compare parameters
99+
for (var i = 0; i < upperParams.length; i++) {
100+
compareNullability(getTypeArg(tree, i), upperParams[i], lowerParams[i], overriddenIsLowerBound);
90101
}
91102
}
92103

93-
private void reportIssue(Tree reportLocation, NullabilityData overrideeNullability, NullabilityData otherNullability) {
104+
private static TypeTree getTypeArg(TypeTree tree, int index) {
105+
if (tree.is(Tree.Kind.PARAMETERIZED_TYPE)) {
106+
return ((ParameterizedTypeTree) tree).typeArguments().get(index);
107+
}
108+
109+
// That's weird, but ok
110+
return tree;
111+
}
112+
113+
private void checkParameter(VariableTree parameter, SymbolMetadata overrideeParam) {
114+
compareNullability(parameter.type(), overrideeParam, parameter.symbol().metadata(), false);
115+
}
116+
117+
private void reportIssue(Tree reportLocation, NullabilityData upperBound, NullabilityData lowerBound, boolean overriddenIsLowerBound) {
118+
NullabilityData otherNullability = overriddenIsLowerBound ? lowerBound : upperBound;
119+
NullabilityData overrideeNullability = overriddenIsLowerBound ? upperBound : lowerBound;
120+
94121
Optional<String> overrideeAsString = nullabilityAsString(otherNullability);
95122
Optional<String> otherAsString = nullabilityAsString(overrideeNullability);
96123
if (overrideeAsString.isPresent() && otherAsString.isPresent()) {

‎java-checks/src/main/java/org/sonar/java/checks/RedundantNullabilityAnnotationsCheck.java‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import java.util.Optional;
2323
import org.sonar.check.Rule;
2424
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
25-
import org.sonar.plugins.java.api.semantic.Symbol;
2625
import org.sonar.plugins.java.api.semantic.SymbolMetadata;
2726
import org.sonar.plugins.java.api.tree.ClassTree;
2827
import org.sonar.plugins.java.api.tree.MethodTree;
@@ -72,7 +71,7 @@ private void checkMembers(SymbolMetadata.NullabilityData classNullabilityData,
7271
if (member.is(Tree.Kind.VARIABLE)) {
7372
// check field
7473
VariableTree variableTree = (VariableTree) member;
75-
checkSymbol(classNullabilityData, variableTree, VARIABLE, variableTree.symbol(), scope);
74+
checkSymbol(classNullabilityData, variableTree, VARIABLE, variableTree.symbol().metadata(), scope);
7675
} else if (member.is(Tree.Kind.METHOD)) {
7776
// check method
7877
checkMethod(classNullabilityData, (MethodTree) member, scope);
@@ -106,16 +105,17 @@ private void checkInnerClass(SymbolMetadata.NullabilityData classNullabilityData
106105
private void checkMethod(SymbolMetadata.NullabilityData classNullabilityData,
107106
MethodTree method, NULLABILITY_SCOPE scope) {
108107
// check return type at method level - do not look up hierarchy
109-
checkSymbol(classNullabilityData, method, METHOD, method.symbol(), scope);
108+
checkSymbol(classNullabilityData, method, METHOD, method.symbol().metadata(), scope);
110109
// check parameters at variable level - do not look up hierarchy
111110
method.parameters().forEach(parameter ->
112-
checkSymbol(classNullabilityData, parameter, VARIABLE, parameter.symbol(), scope)
111+
checkSymbol(classNullabilityData, parameter, VARIABLE, parameter.symbol().metadata(), scope)
113112
);
114113
}
115114

116115
private void checkSymbol(SymbolMetadata.NullabilityData classNullabilityData, Tree tree,
117-
SymbolMetadata.NullabilityLevel treeLevel, Symbol symbol, NULLABILITY_SCOPE scope) {
118-
SymbolMetadata.NullabilityData symbolNullabilityData = symbol.metadata().oldNullabilityData();
116+
SymbolMetadata.NullabilityLevel treeLevel, SymbolMetadata metadata, NULLABILITY_SCOPE scope) {
117+
SymbolMetadata.NullabilityData symbolNullabilityData = metadata.nullabilityData();
118+
119119
if (symbolNullabilityData.isNonNull(treeLevel, false, false) &&
120120
scope.equals(NULLABILITY_SCOPE.NON_NULLABLE)) {
121121
reportIssue(tree, symbolNullabilityData, classNullabilityData);
@@ -124,6 +124,11 @@ private void checkSymbol(SymbolMetadata.NullabilityData classNullabilityData, Tr
124124
scope.equals(NULLABILITY_SCOPE.NULLABLE)) {
125125
reportIssue(tree, symbolNullabilityData, classNullabilityData);
126126
}
127+
128+
// Check parameter types nullability
129+
for (var paramMetadata : metadata.parametersMetadata()) {
130+
checkSymbol(classNullabilityData, tree, treeLevel, paramMetadata, scope);
131+
}
127132
}
128133

129134
// helpful method that handles string conversions of NullabilityData annotations prior to issue reporting

‎java-frontend/src/main/java/org/sonar/java/model/JSymbol.java‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,11 @@
1616
*/
1717
package org.sonar.java.model;
1818

19-
import java.util.ArrayList;
2019
import java.util.Arrays;
2120
import java.util.Collections;
2221
import java.util.List;
2322
import java.util.Objects;
2423
import javax.annotation.Nullable;
25-
import org.eclipse.jdt.core.dom.IAnnotationBinding;
2624
import org.eclipse.jdt.core.dom.IBinding;
2725
import org.eclipse.jdt.core.dom.IMethodBinding;
2826
import org.eclipse.jdt.core.dom.ITypeBinding;
@@ -400,26 +398,7 @@ private SymbolMetadata convertMetadata() {
400398
}
401399

402400
private SymbolMetadata convertMetadata(ITypeBinding type) {
403-
var symbolAnnotations = new IAnnotationBinding[binding.getAnnotations().length + type.getTypeAnnotations().length];
404-
System.arraycopy(binding.getAnnotations(), 0, symbolAnnotations, 0, binding.getAnnotations().length);
405-
System.arraycopy(type.getTypeAnnotations(), 0, symbolAnnotations, binding.getAnnotations().length, type.getTypeAnnotations().length);
406-
407-
var parameterAnnotations = getParamAnnotations(type);
408-
409-
return new JSymbolMetadata(
410-
sema,
411-
this,
412-
symbolAnnotations,
413-
parameterAnnotations
414-
);
415-
}
416-
417-
private static IAnnotationBinding[] getParamAnnotations(ITypeBinding type) {
418-
List<IAnnotationBinding> iAnnotationBindings = new ArrayList<>();
419-
for (ITypeBinding typeArgument : type.getTypeArguments()) {
420-
Collections.addAll(iAnnotationBindings, typeArgument.getTypeAnnotations());
421-
}
422-
return iAnnotationBindings.toArray(new IAnnotationBinding[0]);
401+
return JSymbolMetadata.of(sema, this, type, binding.getAnnotations());
423402
}
424403

425404
/**

0 commit comments

Comments
 (0)