From 06c7a6790133e453df199f9c34e9abf8519ea52e Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Fri, 26 Jan 2024 00:07:01 +0100 Subject: [PATCH] Use of eclipse external annotations (EEA) can cause false positive warning: Null type safety: generic needs unchecked conversion to conform to @NonNull type. (#1913) fixes #1008 Improve propagation of annotations during inference --- .../lookup/ParameterizedTypeBinding.java | 2 + .../compiler/lookup/WildcardBinding.java | 20 ++++- .../model/ExternalAnnotations18Test.java | 80 +++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java index 5587093320c..cdbd3cf22d8 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java @@ -1837,6 +1837,8 @@ public TypeBinding[] getNonWildcardParameters(Scope scope) { types[i] = typeParameters[i].superclass; // assumably j.l.Object? break; } + if (types[i] != null) + types[i] = wildcard.propagateNonConflictingNullAnnotations(types[i]); } else { // If Ai is a type, then Ti = Ai. types[i] = typeArgument; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/WildcardBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/WildcardBinding.java index 859fd866cd0..8d48c76080e 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/WildcardBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/WildcardBinding.java @@ -755,7 +755,8 @@ TypeBinding substituteInferenceVariable(InferenceVariable var, TypeBinding subst } haveSubstitution |= currentOtherBounds != null; if (haveSubstitution) { - return this.environment.createWildcard(this.genericType, this.rank, currentBound, currentOtherBounds, this.boundKind); + WildcardBinding wildcard = this.environment.createWildcard(this.genericType, this.rank, currentBound, currentOtherBounds, this.boundKind); + return propagateNonConflictingNullAnnotations(wildcard); } return this; } @@ -1092,4 +1093,21 @@ public long updateTagBits() { public boolean isNonDenotable() { return true; } + + /** When substituting this wildcard with 'type', perhaps null tagbits on this wildcard should be propagated. */ + TypeBinding propagateNonConflictingNullAnnotations(TypeBinding type) { + if (!this.environment.usesNullTypeAnnotations()) + return type; + if (type instanceof InferenceVariable) { + // just accumulate any hints: + ((InferenceVariable) type).nullHints |= (this.tagBits & TagBits.AnnotationNullMASK); + return type; + } + if ((type.tagBits & TagBits.AnnotationNullMASK) != 0) + return type; // already annotated + AnnotationBinding[] annots = this.environment.nullAnnotationsFromTagBits(this.tagBits & TagBits.AnnotationNullMASK); + if (annots == null) + return type; + return this.environment.createAnnotatedType(type, annots); + } } diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ExternalAnnotations18Test.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ExternalAnnotations18Test.java index 76f0ccff64e..2ff69c3ca72 100644 --- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ExternalAnnotations18Test.java +++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ExternalAnnotations18Test.java @@ -3273,4 +3273,84 @@ public void testAnnotatedSourceSharesOutputFolder() throws CoreException { prj1.getProject().delete(true, true , null); } } + + public void testGH1008() throws Exception { + myCreateJavaProject("ValueOf"); + Map options = this.project.getOptions(true); + options.put(JavaCore.COMPILER_PB_NULL_UNCHECKED_CONVERSION, JavaCore.ERROR); + this.project.setOptions(options); + + addLibraryWithExternalAnnotations(this.project, "jreext.jar", "annots", new String[] { + "/UnannotatedLib/java/lang/Integer.java", + """ + package java.lang; + public class Integer { + public static Integer valueOf(String s) { return null; } + } + """, + "/UnannotatedLib/java/util/function/Function.java", + """ + package java.util.function; + public interface Function { + R apply(T t); + } + """ + }, null); + createFileInProject("annots/java/lang", "Integer.eea", + """ + class java/lang/Integer + valueOf + (Ljava/lang/String;)Ljava/lang/Integer; + (L1java/lang/String;)L1java/lang/Integer; + """); + + IPackageFragment fragment = this.project.getPackageFragmentRoots()[0].createPackageFragment("test", true, null); + ICompilationUnit unit = fragment.createCompilationUnit("UncheckedConversionFalsePositive.java", + """ + package test; + import org.eclipse.jdt.annotation.Checks; + import org.eclipse.jdt.annotation.NonNull; + import org.eclipse.jdt.annotation.Nullable; + + public class UncheckedConversionFalsePositive { + + public static @NonNull Integer doSomething(@NonNull final String someValue) { + return Integer.valueOf(someValue); + } + + Integer test() { + final @Nullable String nullableString = "12"; + @Nullable Integer result; + + // This first example that uses my own annotated method and not eclipse external annotations + // works no problem... + result = Checks.applyIfNonNull(nullableString, UncheckedConversionFalsePositive::doSomething); + + // But now, if I do this, which relies on EEA annotations instead of my in-code annotations.... + result = Checks.applyIfNonNull(nullableString, Integer::valueOf); + // Then Integer::valueOf is flagged with the following message: + /* + * Null type safety: parameter 1 provided via method descriptor + * Function.apply(String) needs unchecked conversion to conform to '@NonNull + * String' + */ + + // Note that the same warning is shown on "someValue" below when using a lambda expression + // instead of a method reference. + result = Checks.applyIfNonNull(nullableString, someValue -> Integer.valueOf(someValue)); + + // The workaround to eliminate this warning without suppressing it is to make the + // generic parameters explicit with @NonNull but this is very verbose and should + // ideally be unnecessary... + result = Checks.<@NonNull String, Integer>applyIfNonNull(nullableString, Integer::valueOf); + + return result; + } + } + """, + true, new NullProgressMonitor()).getWorkingCopy(new NullProgressMonitor()); + CompilationUnit reconciled = unit.reconcile(getJLS8(), true, null, new NullProgressMonitor()); + IProblem[] problems = reconciled.getProblems(); + assertNoProblems(problems); + } }