-
Notifications
You must be signed in to change notification settings - Fork 29
Add subtyping check to receiver type argument for method invocation #793
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
ed8e47f
ab59a90
0df34f6
997dc5d
82c9faa
e160b61
a5d1fad
a4387a1
053340d
91dd74f
5768f2c
c6afb70
b7f5767
f66d93d
714b75a
0c4519a
f806c1d
c529f0d
badbfd3
8a67c54
c876775
bf0e385
64711f7
ce6e91d
287347d
e6e92d4
b554427
c63ae6c
f22dccf
b0674e9
fbaf351
c494a8b
9e63567
0fe838e
86fbbde
be8c65e
c2a2ecf
201481b
7519918
1e3f9ae
229bd9b
c6f38f7
1b33ca6
c020864
9a2ef29
85137e6
1d7655a
6127c1d
d05ccf3
bceb0b6
5fbc0d7
c88795d
8145d7d
307ee12
dbcff2d
70493a1
a37741d
6d7cb16
e0e4461
50f67da
5c0ee2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import org.checkerframework.checker.nullness.qual.NonNull; | ||
| import org.checkerframework.checker.nullness.qual.Nullable; | ||
| import org.checkerframework.checker.nullness.qual.PolyNull; | ||
|
|
||
| public class PolyQualifierOnTypeArgument<T> { | ||
| void toArray(PolyQualifierOnTypeArgument<@PolyNull T> this) { | ||
| // This method has @PolyNull on receiver's type argument. | ||
| } | ||
|
|
||
| void test(PolyQualifierOnTypeArgument<?> p) { | ||
| // When calling toArray on a receiver with an unbounded wildcard type argument, the LUB of | ||
| // the annotations on the wildcard's bounds is @Nullable. As a result, the receiver type is | ||
| // substituted as PolyQualifierOnTypeArgument<@Nullable T>, which is not compatible with an | ||
| // unbounded wildcard type argument. | ||
| // If the method receiver type argument subtyping check is enabled, the error is: | ||
| // found : @NonNull PolyQualifierOnTypeArgument<? [ extends @Nullable Object super | ||
| // @NonNull NullType ]> | ||
| // required: @NonNull PolyQualifierOnTypeArgument<capture#01 [ extends @Nullable Object | ||
| // super @Nullable NullType ]> | ||
| p.toArray(); | ||
|
aosen-xiong marked this conversation as resolved.
|
||
| } | ||
|
|
||
| void test2(PolyQualifierOnTypeArgument<T> p) { | ||
| p.toArray(); // same bound mismatch as unbounded wildcard case | ||
| } | ||
|
|
||
| void test3(PolyQualifierOnTypeArgument<@NonNull T> p) { | ||
| p.toArray(); // ok, the LUB is @NonNull, both bounds are @NonNull is compatible with | ||
| // @PolyNull T after substitution | ||
| } | ||
|
|
||
| void test4(PolyQualifierOnTypeArgument<@Nullable Object> p) { | ||
| p.toArray(); // ok, the LUB is @Nullable, both bounds are @Nullable is compatible | ||
| // with @PolyNull T after substitution | ||
| } | ||
|
|
||
| void test5(PolyQualifierOnTypeArgument<Object> p) { | ||
| p.toArray(); // ok, the LUB is @NonNull, both bounds are @NonNull is compatible with | ||
| // @PolyNull T after substitution | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // Test case for issue 104: https://github.com/eisop/checker-framework/issues/104 | ||
|
|
||
| import org.checkerframework.checker.nullness.qual.NonNull; | ||
|
aosen-xiong marked this conversation as resolved.
|
||
| import org.checkerframework.checker.nullness.qual.Nullable; | ||
|
|
||
| public class ReceiverTypeArgs { | ||
| static class Box<T> { | ||
| T item; | ||
|
|
||
| public Box(T item) { | ||
| this.item = item; | ||
| } | ||
|
|
||
| void test(Box<@NonNull T> this) {} | ||
| } | ||
|
|
||
| private static void foo(Box<@Nullable String> box) { | ||
| // :: error: (method.invocation.invalid) | ||
| box.test(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2178,12 +2178,9 @@ public Void visitMethodInvocation(MethodInvocationTree tree, Void p) { | |
| typeCheckVectorCopyIntoArgument(tree, params); | ||
| } | ||
|
|
||
| ExecutableElement invokedMethodElement = invokedMethod.getElement(); | ||
| if (!ElementUtils.isStatic(invokedMethodElement) | ||
| && !TreeUtils.isSuperConstructorCall(tree)) { | ||
| checkMethodInvocability(invokedMethod, tree); | ||
| } | ||
| checkMethodInvocability(invokedMethod, tree); | ||
|
|
||
| ExecutableElement invokedMethodElement = invokedMethod.getElement(); | ||
| // check precondition annotations | ||
| checkPreconditions( | ||
| tree, atypeFactory.getContractsFromMethod().getPreconditions(invokedMethodElement)); | ||
|
|
@@ -3970,11 +3967,12 @@ protected boolean skipReceiverSubtypeCheck( | |
| */ | ||
| protected void checkMethodInvocability( | ||
| AnnotatedExecutableType method, MethodInvocationTree tree) { | ||
| if (method.getReceiverType() == null) { | ||
| ExecutableElement invokedMethodElement = method.getElement(); | ||
| if (ElementUtils.isStatic(invokedMethodElement)) { | ||
| // Static methods don't have a receiver to check. | ||
| return; | ||
| } | ||
| if (method.getElement().getKind() == ElementKind.CONSTRUCTOR) { | ||
| if (invokedMethodElement.getKind() == ElementKind.CONSTRUCTOR) { | ||
| // TODO: Explicit "this()" calls of constructors have an implicit passed | ||
| // from the enclosing constructor. We must not use the self type, but | ||
| // instead should find a way to determine the receiver of the enclosing constructor. | ||
|
|
@@ -3983,20 +3981,35 @@ protected void checkMethodInvocability( | |
| return; | ||
| } | ||
|
|
||
| AnnotatedTypeMirror methodReceiver = method.getReceiverType(); | ||
| AnnotatedTypeMirror erasedMethodReceiver = methodReceiver.getErased(); | ||
| AnnotatedTypeMirror erasedTreeReceiver = erasedMethodReceiver.shallowCopy(false); | ||
| AnnotatedTypeMirror treeReceiver = atypeFactory.getReceiverType(tree); | ||
| ParameterizedExecutableType methodDefPreSubstitution = | ||
| atypeFactory.methodFromUseWithoutTypeArgInference(tree); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why can't this not simply use
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I renamed the variable to
|
||
| List<AnnotatedTypeMirror> declaredTypeArgs = | ||
| methodDefPreSubstitution.executableType.getReceiverType().getTypeArguments(); | ||
|
Comment on lines
+3984
to
+3987
|
||
| // Don't check when method receiver's type argument has a poly annotation. | ||
|
aosen-xiong marked this conversation as resolved.
|
||
| // See checker/tests/nullness-genericwildcard/PolyQualifierOnTypeArgument.java. | ||
| for (AnnotationMirror top : qualHierarchy.getTopAnnotations()) { | ||
| AnnotationMirror poly = qualHierarchy.getPolymorphicAnnotation(top); | ||
| // If this hierarchy does not have a poly annotation, then continue to next hierarchy. | ||
| if (poly == null) { | ||
| continue; | ||
| } | ||
| for (AnnotatedTypeMirror typeArg : declaredTypeArgs) { | ||
| // If one of the type arguments has a poly annotation, then skip the check. | ||
| if (typeArg.hasAnnotation(poly)) { | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| erasedTreeReceiver.addAnnotations(treeReceiver.getEffectiveAnnotations()); | ||
| AnnotatedDeclaredType methodReceiver = method.getReceiverType(); | ||
| AnnotatedTypeMirror treeReceiver = atypeFactory.getReceiverType(tree); | ||
|
|
||
| if (!skipReceiverSubtypeCheck(tree, erasedMethodReceiver, treeReceiver)) { | ||
| if (!skipReceiverSubtypeCheck(tree, methodReceiver, treeReceiver)) { | ||
| // The diagnostic can be a bit misleading because the check is of the receiver but | ||
| // `tree` is the entire method invocation (where the receiver might be implicit). | ||
| commonAssignmentCheckStartDiagnostic(methodReceiver, erasedTreeReceiver, tree); | ||
| boolean success = typeHierarchy.isSubtype(erasedTreeReceiver, erasedMethodReceiver); | ||
| commonAssignmentCheckEndDiagnostic( | ||
| success, null, methodReceiver, erasedTreeReceiver, tree); | ||
| commonAssignmentCheckStartDiagnostic(methodReceiver, treeReceiver, tree); | ||
| boolean success = typeHierarchy.isSubtype(treeReceiver, methodReceiver); | ||
| commonAssignmentCheckEndDiagnostic(success, null, methodReceiver, treeReceiver, tree); | ||
| if (!success) { | ||
| // Don't report the erased types because they show up with '</*RAW*/>' as type args. | ||
| reportMethodInvocabilityError(tree, treeReceiver, methodReceiver); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.