Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package annotations.nullability.no_default;

import java.util.List;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;

class ExternalGenericParent<T> {

T inherited(T value) {
return value;
}
}

public class ExternalGenericNullability<T> extends ExternalGenericParent<T> {

public ExternalGenericNullability(T value) {
}

@NonNull
public T declaredNonNull(@NonNull T value) {
return value;
}

@Nullable
public T declaredNullable(@Nullable T value) {
return value;
}

public List<@Nullable T> nestedNullable(List<@Nullable T> values) {
return values;
}

public <U> U genericMethod(U value) {
return value;
}

public <U> U genericVarargs(U... values) {
return values[0];
}

@org.eclipse.jdt.annotation.NonNullByDefault
public T defaulted(T value) {
return value;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package annotations.nullability.no_default;

import java.util.List;
import org.eclipse.jdt.annotation.NonNull;

public class ExternalGenericNullabilityUsage {

public void calls(ExternalGenericNullability<@NonNull String> dependency) {
dependency.declaredNonNull("value");
dependency.declaredNullable(null);
dependency.nestedNullable(List.of("value"));
dependency.inherited(null);
new ExternalGenericNullability<@NonNull String>(null);
dependency.<@NonNull String>genericMethod("value");
dependency.<@NonNull String>genericVarargs("value");
dependency.defaulted("value");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package annotations.nullability.no_default;

import java.util.Optional;
import org.eclipse.jdt.annotation.NonNull;

public class NullabilityWithInferredTypeArgument {

private final String value = "value";

@NonNull
public String getValue() {
return value;
}

public String callOrElse(NullabilityWithInferredTypeArgument a) {
// "map" infers "Optional<@NonNull String>", the @NonNull is part of the type argument and says nothing about the parameter of "orElse"
return Optional.ofNullable(a)
.map(NullabilityWithInferredTypeArgument::getValue)
.orElse(null);
}

public boolean nullCheckAfterOrElse(NullabilityWithInferredTypeArgument a) {
String result = Optional.ofNullable(a)
.map(NullabilityWithInferredTypeArgument::getValue)
.orElse(null);
// the inferred @NonNull must not make "orElse" look like it never returns null, the check below is not always false
return result == null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,18 @@ public List<Symbol> declarationParameters() {
} else {
parameters = new ArrayList<>();
IMethodBinding methodBinding = methodBinding();
IMethodBinding methodDeclaration = methodBinding.getMethodDeclaration();
ITypeBinding[] parameterTypeBindings = methodBinding.getParameterTypes();
ITypeBinding[] declaredParameterTypeBindings = methodDeclaration.getParameterTypes();
for (int i = 0; i < parameterTypeBindings.length; i++) {
parameters.add(new JVariableSymbol.ParameterPlaceholderSymbol(i, sema, methodBinding.getMethodDeclaration(), parameterTypeBindings[i]));
// Annotations must come from the declared type: type annotations of the substituted type can be inferred from the call site
// (e.g. "Optional.of(x).map(A::nonNullMethod).orElse(null)" infers "Optional<@NonNull String>", making "orElse" look like it takes
// a @NonNull argument), which does not tell anything about the parameter of the declared method.
// When semantic recovery does not provide the corresponding declared parameter, keep the previous behavior instead of
// treating its nullability as unknown. This preserves existing detections and avoids introducing false negatives while
// refining the common case to remove false positives.
ITypeBinding declaredParameterType = i < declaredParameterTypeBindings.length ? declaredParameterTypeBindings[i] : parameterTypeBindings[i];
parameters.add(new JVariableSymbol.ParameterPlaceholderSymbol(i, sema, methodDeclaration, parameterTypeBindings[i], declaredParameterType));
}
}
}
Expand Down
13 changes: 11 additions & 2 deletions java-frontend/src/main/java/org/sonar/java/model/JSymbol.java
Original file line number Diff line number Diff line change
Expand Up @@ -386,12 +386,21 @@ private SymbolMetadata convertMetadata() {
}
return convertMetadata(type);
case IBinding.METHOD:
ITypeBinding returnType = ((IMethodBinding) binding).getReturnType();
IMethodBinding methodBinding = (IMethodBinding) binding;
ITypeBinding returnType = methodBinding.getReturnType();
// In rare circumstances, when the semantic information is incomplete, returnType can be null.
if (returnType == null) {
return Symbols.EMPTY_METADATA;
}
return convertMetadata(returnType);
// Annotations are read from the declared return type, so that annotations inferred for a type variable at the call site
// (e.g. "Optional.of(x).map(A::nonNullMethod)" infers "Optional<@NonNull String>", making "orElse" look like it never returns null)
// are not mistaken for annotations of the method itself.
IMethodBinding methodDeclaration = methodBinding.getMethodDeclaration();
ITypeBinding declaredReturnType = methodDeclaration == null ? null : methodDeclaration.getReturnType();
// When semantic recovery does not provide a declared return type, keep the previous behavior instead of treating its
// nullability as unknown. This preserves existing detections and avoids introducing false negatives while refining the
// common case to remove false positives.
return convertMetadata(declaredReturnType == null ? returnType : declaredReturnType);
default:
return new JSymbolMetadata(sema, this, binding.getAnnotations());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,16 @@ static class ParameterPlaceholderSymbol extends Symbols.DefaultSymbol implements
private final Type type;
private final SymbolMetadata metadata;

ParameterPlaceholderSymbol(int index, JSema sema, IMethodBinding owner, ITypeBinding typeBinding) {
/**
* @param typeBinding the type of the parameter at the call site, after type substitution
* @param declaredTypeBinding the type of the parameter as declared, before type substitution. Annotations are read from it, so that
* annotations inferred for a type variable are not mistaken for annotations of the parameter itself.
*/
ParameterPlaceholderSymbol(int index, JSema sema, IMethodBinding owner, ITypeBinding typeBinding, ITypeBinding declaredTypeBinding) {
this.name = "arg" + index;
this.owner = sema.methodSymbol(owner);
this.type = sema.type(typeBinding);
this.metadata = JSymbolMetadata.of(sema, this, typeBinding, owner.getParameterAnnotations(index));
this.metadata = JSymbolMetadata.of(sema, this, declaredTypeBinding, owner.getParameterAnnotations(index));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import org.sonar.plugins.java.api.tree.IdentifierTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.MethodTree;
import org.sonar.plugins.java.api.tree.NewClassTree;
import org.sonar.plugins.java.api.tree.ReturnStatementTree;
import org.sonar.plugins.java.api.tree.Tree;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -237,6 +239,112 @@ void generics_nullability() throws IOException {
.isSameAs(invocation2ParamData);
}

@Test
void nullability_is_not_inferred_from_type_arguments() throws IOException {
Path sourceFile = NULLABILITY_SOURCE_DIR.resolve(Paths.get("no_default", "NullabilityWithInferredTypeArgument.java"));
CompilationUnitTree cut = JParserTestUtils.parse(sourceFile.toRealPath().toFile(), JParserTestUtils.checksTestClassPath());
ClassTree classTree = (ClassTree) cut.types().get(0);
MethodTree callOrElse = (MethodTree) classTree.members().get(2);

MethodInvocationTree orElseInvocation = (MethodInvocationTree) ((ReturnStatementTree) callOrElse.block().body().get(0)).expression();
Symbol orElseParameter = orElseInvocation.methodSymbol().declarationParameters().get(0);

assertThat(orElseParameter.metadata().annotations()).isEmpty();
assertThat(orElseParameter.metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);
}

@Test
void method_nullability_is_not_inferred_from_type_arguments() throws IOException {
Path sourceFile = NULLABILITY_SOURCE_DIR.resolve(Paths.get("no_default", "NullabilityWithInferredTypeArgument.java"));
CompilationUnitTree cut = JParserTestUtils.parse(sourceFile.toRealPath().toFile(), JParserTestUtils.checksTestClassPath());
ClassTree classTree = (ClassTree) cut.types().get(0);
MethodTree callOrElse = (MethodTree) classTree.members().get(2);

MethodInvocationTree orElseInvocation = (MethodInvocationTree) ((ReturnStatementTree) callOrElse.block().body().get(0)).expression();
SymbolMetadata orElseMetadata = orElseInvocation.methodSymbol().metadata();

assertThat(orElseMetadata.symbolAnnotations()).isEmpty();
assertThat(orElseMetadata.nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);
}

@Test
void generic_external_method_metadata_comes_from_declaration() throws IOException {
MethodTree calls = externalGenericCalls();
MethodInvocationTree declaredNonNull = invocationAt(calls, 0);
MethodInvocationTree declaredNullable = invocationAt(calls, 1);
MethodInvocationTree inherited = invocationAt(calls, 3);

assertThat(declaredNonNull.methodSymbol().parameterTypes().get(0).name()).isEqualTo("String");
assertThat(declaredNonNull.methodSymbol().returnType().name()).isEqualTo("String");
assertThat(declaredNonNull.methodSymbol().declarationParameters().get(0).metadata().nullabilityData().type()).isEqualTo(NON_NULL);
assertThat(declaredNonNull.methodSymbol().metadata().nullabilityData().type()).isEqualTo(NON_NULL);

assertThat(declaredNullable.methodSymbol().parameterTypes().get(0).name()).isEqualTo("String");
assertThat(declaredNullable.methodSymbol().returnType().name()).isEqualTo("String");
assertThat(declaredNullable.methodSymbol().declarationParameters().get(0).metadata().nullabilityData().type()).isEqualTo(STRONG_NULLABLE);
assertThat(declaredNullable.methodSymbol().metadata().nullabilityData().type()).isEqualTo(STRONG_NULLABLE);

assertThat(inherited.methodSymbol().parameterTypes().get(0).name()).isEqualTo("String");
assertThat(inherited.methodSymbol().returnType().name()).isEqualTo("String");
assertThat(inherited.methodSymbol().declarationParameters().get(0).metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);
assertThat(inherited.methodSymbol().metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);
}

@Test
void generic_external_constructor_and_nested_type_metadata_come_from_declaration() throws IOException {
MethodTree calls = externalGenericCalls();
MethodInvocationTree nestedNullable = invocationAt(calls, 2);
NewClassTree constructor = (NewClassTree) ((ExpressionStatementTree) calls.block().body().get(4)).expression();

assertThat(nestedNullable.methodSymbol().parameterTypes().get(0).name()).isEqualTo("List");
assertThat(nestedNullable.methodSymbol().returnType().name()).isEqualTo("List");
assertThat(nestedNullable.methodSymbol().metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);
assertThat(nestedNullable.methodSymbol().metadata().parametersMetadata()).singleElement()
.extracting(metadata -> metadata.nullabilityData().type())
.isEqualTo(STRONG_NULLABLE);

assertThat(constructor.methodSymbol().parameterTypes().get(0).name()).isEqualTo("String");
assertThat(constructor.methodSymbol().declarationParameters().get(0).metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);
}

@Test
void generic_external_method_type_parameter_metadata_comes_from_declaration() throws IOException {
MethodTree calls = externalGenericCalls();
MethodInvocationTree genericMethod = invocationAt(calls, 5);
MethodInvocationTree genericVarargs = invocationAt(calls, 6);

assertThat(genericMethod.methodSymbol().parameterTypes().get(0).name()).isEqualTo("String");
assertThat(genericMethod.methodSymbol().returnType().name()).isEqualTo("String");
assertThat(genericMethod.methodSymbol().declarationParameters().get(0).metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);
assertThat(genericMethod.methodSymbol().metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);

assertThat(genericVarargs.methodSymbol().parameterTypes().get(0).name()).isEqualTo("String[]");
assertThat(genericVarargs.methodSymbol().returnType().name()).isEqualTo("String");
assertThat(genericVarargs.methodSymbol().declarationParameters().get(0).metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);
assertThat(genericVarargs.methodSymbol().metadata().nullabilityData().type()).isEqualTo(NullabilityType.NO_ANNOTATION);
}

@Test
void generic_external_method_default_nullability_is_preserved() throws IOException {
MethodInvocationTree defaulted = invocationAt(externalGenericCalls(), 7);

assertThat(defaulted.methodSymbol().parameterTypes().get(0).name()).isEqualTo("String");
assertThat(defaulted.methodSymbol().returnType().name()).isEqualTo("String");
assertThat(defaulted.methodSymbol().declarationParameters().get(0).metadata().nullabilityData().type()).isEqualTo(NON_NULL);
assertThat(defaulted.methodSymbol().metadata().nullabilityData().type()).isEqualTo(NON_NULL);
}

private static MethodTree externalGenericCalls() throws IOException {
Path sourceFile = NULLABILITY_SOURCE_DIR.resolve(Paths.get("no_default", "ExternalGenericNullabilityUsage.java"));
CompilationUnitTree cut = JParserTestUtils.parse(sourceFile.toRealPath().toFile(), JParserTestUtils.checksTestClassPath());
ClassTree classTree = (ClassTree) cut.types().get(0);
return (MethodTree) classTree.members().get(0);
}

private static MethodInvocationTree invocationAt(MethodTree method, int index) {
return (MethodInvocationTree) ((ExpressionStatementTree) method.block().body().get(index)).expression();
}

@Nested
class NullabilityDataTest {

Expand Down
Loading