Adding support for nullable type upper bounds considering Library mod… #893
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,6 +112,26 @@ | |
| */ | ||
| ImmutableSet<MethodRef> nonNullReturns(); | ||
|
|
||
| /** | ||
| * Get the (className, type argument index) pairs for library classes where the generic type | ||
| * argument has a {@code @Nullable} upper bound. Only used in JSpecify mode. | ||
| * | ||
| * @return map from the className to the positions of the generic type arguments that have a | ||
| * {@code Nullable} upper bound. | ||
| */ | ||
| default ImmutableSetMultimap<String, Integer> typeVariablesWithNullableUpperBounds() { | ||
| return ImmutableSetMultimap.of(); | ||
| } | ||
|
|
||
| /** | ||
| * Get the set of library classes that are NullMarked. Only used in JSpecify mode. | ||
| * | ||
| * @return set of library classes that are NullMarked. | ||
| */ | ||
| default ImmutableSet<String> nullMarkedClasses() { | ||
| return ImmutableSet.of(); | ||
| } | ||
|
|
||
| /** | ||
| * Get (method, parameter) pairs that act as castToNonNull(...) methods. | ||
| * | ||
|
|
@@ -137,7 +157,9 @@ | |
| * | ||
| * @return set of library fields that may be {@code null}. | ||
| */ | ||
| default ImmutableSet<FieldRef> nullableFields() { | ||
| return ImmutableSet.of(); | ||
| } | ||
|
|
||
| /** | ||
| * Get a list of custom stream library specifications. | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,7 @@ | |
| import com.google.auto.service.AutoService; | ||
| import com.google.auto.value.AutoValue; | ||
| import com.google.common.base.Preconditions; | ||
| import com.google.common.base.Verify; | ||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.common.collect.ImmutableMultimap; | ||
| import com.google.common.collect.ImmutableSet; | ||
|
|
@@ -306,7 +307,7 @@ public NullAway(ErrorProneFlags flags) { | |
| private boolean isMethodUnannotated(MethodInvocationNode invocationNode) { | ||
| return invocationNode == null | ||
| || codeAnnotationInfo.isSymbolUnannotated( | ||
| ASTHelpers.getSymbol(invocationNode.getTree()), config, handler); | ||
| } | ||
|
|
||
| private boolean withinAnnotatedCode(VisitorState state) { | ||
|
|
@@ -345,7 +346,7 @@ private boolean checkMarkingForPath(VisitorState state) { | |
| if (enclosingMarkableSymbol == null) { | ||
| return false; | ||
| } | ||
| return !codeAnnotationInfo.isSymbolUnannotated(enclosingMarkableSymbol, config, handler); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -399,16 +400,31 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState | |
| if (!withinAnnotatedCode(state)) { | ||
| return Description.NO_MATCH; | ||
| } | ||
| Symbol.MethodSymbol methodSymbol = getSymbolForMethodInvocation(tree, state); | ||
| handler.onMatchMethodInvocation(this, tree, state, methodSymbol); | ||
| // assuming this list does not include the receiver | ||
| List<? extends ExpressionTree> actualParams = tree.getArguments(); | ||
| return handleInvocation(tree, state, methodSymbol, actualParams); | ||
| } | ||
|
|
||
| private static Symbol.MethodSymbol getSymbolForMethodInvocation( | ||
| MethodInvocationTree tree, VisitorState state) { | ||
| Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(tree); | ||
| Verify.verify(methodSymbol != null, "not expecting unresolved method here"); | ||
| // For interface methods, if the method is an implicit method corresponding to a method from | ||
| // java.lang.Object, use the symbol for the java.lang.Object method instead. We do this to | ||
| // properly treat the method as unannotated, which is particularly important for equals() | ||
| // methods. This is an adaptation to a change in JDK 18; see | ||
| // https://bugs.openjdk.org/browse/JDK-8272564 | ||
| if (methodSymbol.owner.isInterface()) { | ||
| Symbol.MethodSymbol baseSymbol = (Symbol.MethodSymbol) methodSymbol.baseSymbol(); | ||
| if (baseSymbol != methodSymbol && baseSymbol.owner == state.getSymtab().objectType.tsym) { | ||
| methodSymbol = baseSymbol; | ||
| } | ||
| } | ||
| return methodSymbol; | ||
| } | ||
|
|
||
| @Override | ||
| public Description matchNewClass(NewClassTree tree, VisitorState state) { | ||
| if (!withinAnnotatedCode(state)) { | ||
|
|
@@ -596,7 +612,7 @@ private void checkForMethodNullMarkedness(MethodTree tree, VisitorState state) { | |
| methodSymbol, NullabilityUtil.NULLMARKED_SIMPLE_NAME)) { | ||
| // We still care here if this is a transition between @NullUnmarked and @NullMarked code, | ||
| // within partially marked code, see checks below for markedMethodInUnmarkedContext. | ||
| if (!codeAnnotationInfo.isClassNullAnnotated(methodSymbol.enclClass(), config, handler)) { | ||
| markedMethodInUnmarkedContext = true; | ||
| } | ||
| } | ||
|
|
@@ -697,7 +713,8 @@ public Description matchParameterizedType(ParameterizedTypeTree tree, VisitorSta | |
| return Description.NO_MATCH; | ||
| } | ||
| if (config.isJSpecifyMode()) { | ||
| GenericsChecks.checkInstantiationForParameterizedTypedTree( | ||
| tree, state, this, config, handler); | ||
| } | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
@@ -725,7 +742,7 @@ private Description checkParamOverriding( | |
| (memberReferenceTree != null) | ||
| && ((JCTree.JCMemberReference) memberReferenceTree).kind.isUnbound(); | ||
| final boolean isOverriddenMethodAnnotated = | ||
| !codeAnnotationInfo.isSymbolUnannotated(overriddenMethod, config, handler); | ||
|
|
||
| // Get argument nullability for the overridden method. If overriddenMethodArgNullnessMap[i] is | ||
| // null, parameter i is treated as unannotated. | ||
|
|
@@ -845,7 +862,8 @@ static Trees getTreesInstance(VisitorState state) { | |
|
|
||
| private Nullness getMethodReturnNullness( | ||
| Symbol.MethodSymbol methodSymbol, VisitorState state, Nullness defaultForUnannotated) { | ||
| final boolean isMethodAnnotated = | ||
| !codeAnnotationInfo.isSymbolUnannotated(methodSymbol, config, handler); | ||
| Nullness methodReturnNullness = | ||
| defaultForUnannotated; // Permissive default for unannotated code. | ||
| if (isMethodAnnotated) { | ||
|
|
@@ -900,13 +918,15 @@ private Description checkReturnExpression( | |
| // type is @Nullable, and if so, bail out. | ||
| if (getMethodReturnNullness(methodSymbol, state, Nullness.NULLABLE).equals(Nullness.NULLABLE)) { | ||
| return Description.NO_MATCH; | ||
| } else if (config.isJSpecifyMode() && lambdaTree != null) { | ||
| if (GenericsChecks.getGenericMethodReturnTypeNullness( | ||
| methodSymbol, ASTHelpers.getType(lambdaTree), state, config) | ||
| .equals(Nullness.NULLABLE) | ||
| || GenericsChecks.passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode( | ||
| methodSymbol, lambdaTree, state, config, codeAnnotationInfo, handler)) { | ||
| // In JSpecify mode, the return type of a lambda may be @Nullable via a type argument | ||
| return Description.NO_MATCH; | ||
| } | ||
| } | ||
|
|
||
| // Return type is @NonNull. Check if the expression is @Nullable | ||
|
|
@@ -934,7 +954,7 @@ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState | |
| // (like Rx nullability) run dataflow analysis | ||
| updateEnvironmentMapping(state.getPath(), state); | ||
| handler.onMatchLambdaExpression(this, tree, state, funcInterfaceMethod); | ||
| if (codeAnnotationInfo.isSymbolUnannotated(funcInterfaceMethod, config, handler)) { | ||
| return Description.NO_MATCH; | ||
| } | ||
| Description description = | ||
|
|
@@ -1049,8 +1069,10 @@ private boolean overriddenMethodReturnsNonNull( | |
| // For a method reference, we get generic type arguments from javac's inferred type for the | ||
| // tree, which properly preserves type-use annotations | ||
| return GenericsChecks.getGenericMethodReturnTypeNullness( | ||
| overriddenMethod, ASTHelpers.getType(memberReferenceTree), state, config) | ||
| .equals(Nullness.NONNULL) | ||
| && !GenericsChecks.passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode( | ||
| overriddenMethod, memberReferenceTree, state, config, codeAnnotationInfo, handler); | ||
| } else { | ||
| // Use the enclosing class of the overriding method to find generic type arguments | ||
| return GenericsChecks.getGenericMethodReturnTypeNullness( | ||
|
|
@@ -1160,7 +1182,7 @@ private boolean relevantInitializerMethodOrBlock( | |
| // in those cases, we won't even have a populated class2Entities map). We skip this check if | ||
| // we are not inside a @NullMarked/annotated *class*: | ||
| if (nullMarkingForTopLevelClass == NullMarking.PARTIALLY_MARKED | ||
| && !codeAnnotationInfo.isClassNullAnnotated(enclClassSymbol, config, handler)) { | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -1681,7 +1703,8 @@ private Description handleInvocation( | |
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| final boolean isMethodAnnotated = | ||
| !codeAnnotationInfo.isSymbolUnannotated(methodSymbol, config, handler); | ||
| // If argumentPositionNullness[i] == null, parameter i is unannotated | ||
| Nullness[] argumentPositionNullness = new Nullness[formalParams.size()]; | ||
|
|
||
|
|
@@ -2271,7 +2294,7 @@ private boolean isExcludedClass(Symbol.ClassSymbol classSymbol) { | |
| if (config.isExcludedClass(className)) { | ||
| return true; | ||
| } | ||
| if (!codeAnnotationInfo.isClassNullAnnotated(classSymbol, config, handler)) { | ||
| return true; | ||
| } | ||
| // check annotations | ||
|
|
@@ -2404,7 +2427,7 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) { | |
|
|
||
| private boolean mayBeNullMethodCall( | ||
| Symbol.MethodSymbol exprSymbol, MethodInvocationTree invocationTree, VisitorState state) { | ||
| if (codeAnnotationInfo.isSymbolUnannotated(exprSymbol, config, handler)) { | ||
| return false; | ||
| } | ||
| if (Nullness.hasNullableAnnotation(exprSymbol, config)) { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -361,7 +361,7 @@ public static boolean mayBeNullFieldFromType( | |
| Symbol symbol, Config config, CodeAnnotationInfo codeAnnotationInfo) { | ||
| return !(symbol.getSimpleName().toString().equals("class") | ||
| || symbol.isEnum() | ||
| || codeAnnotationInfo.isSymbolUnannotated(symbol, config, null)) | ||
| && Nullness.hasNullableAnnotation(symbol, config); | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,7 +100,8 @@ private static NullnessStore lambdaInitialStore( | |
| types.memberType(ASTHelpers.getType(code), fiMethodSymbol).getParameterTypes(); | ||
| // If fiArgumentPositionNullness[i] == null, parameter position i is unannotated | ||
| Nullness[] fiArgumentPositionNullness = new Nullness[fiMethodParameters.size()]; | ||
| final boolean isFIAnnotated = | ||
| !codeAnnotationInfo.isSymbolUnannotated(fiMethodSymbol, config, handler); | ||
| if (isFIAnnotated) { | ||
| for (int i = 0; i < fiMethodParameters.size(); i++) { | ||
| if (Nullness.hasNullableAnnotation(fiMethodParameters.get(i), config)) { | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.