Make GenericsChecks methods static #805
Merged
Conversation
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
…with generic @nullable return type
lazaroclapp
reviewed
Aug 14, 2023
| public void checkTypeParameterNullnessForFunctionReturnType( | ||
| ExpressionTree retExpr, Symbol.MethodSymbol methodSymbol) { | ||
| if (!config.isJSpecifyMode()) { | ||
| public static void checkTypeParameterNullnessForFunctionReturnType( |
There was a problem hiding this comment.
This is a public method, so... javadoc?
| @@ -237,7 +239,7 @@ private void reportInvalidOverridingMethodParamTypeError( | |||
| * @return Type of the tree with preserved annotations. | |||
| */ | |||
| @Nullable | |||
| private Type getTreeType(Tree tree) { | |||
| private static Type getTreeType(Tree tree, VisitorState state) { | |||
There was a problem hiding this comment.
Javadoc needs to be updated with the new @params.
| @@ -267,8 +269,9 @@ private Type getTreeType(Tree tree) { | |||
| * @param tree the tree to check, which must be either an {@link AssignmentTree} or a {@link | |||
| * VariableTree} | |||
| */ | |||
| public void checkTypeParameterNullnessForAssignability(Tree tree) { | |||
| if (!config.isJSpecifyMode()) { | |||
| public static void checkTypeParameterNullnessForAssignability( | |||
There was a problem hiding this comment.
Javadoc needs to be updated with the new @params.
| @@ -334,7 +340,8 @@ public void checkTypeParameterNullnessForFunctionReturnType( | |||
| * @param lhsType type for the lhs of the assignment | |||
| * @param rhsType type for the rhs of the assignment | |||
| */ | |||
| private boolean compareNullabilityAnnotations(Type.ClassType lhsType, Type.ClassType rhsType) { | |||
| private static boolean compareNullabilityAnnotations( | |||
There was a problem hiding this comment.
Javadoc needs to be updated with the new @params.
| @@ -395,7 +402,8 @@ private boolean compareNullabilityAnnotations(Type.ClassType lhsType, Type.Class | |||
| * @param tree A parameterized typed tree for which we need class type with preserved annotations. | |||
| * @return A Type with preserved annotations. | |||
| */ | |||
| private Type.ClassType typeWithPreservedAnnotations(ParameterizedTypeTree tree) { | |||
| private static Type.ClassType typeWithPreservedAnnotations( | |||
There was a problem hiding this comment.
Javadoc needs to be updated with the new @params.
| @@ -686,15 +704,20 @@ public static Nullness getGenericReturnNullnessAtInvocation( | |||
| * @return Nullness of parameter at {@code paramIndex}, or {@code NONNULL} if the call does not | |||
| * invoke an instance method | |||
| */ | |||
| public Nullness getGenericParameterNullnessAtInvocation( | |||
| int paramIndex, Symbol.MethodSymbol invokedMethodSymbol, MethodInvocationTree tree) { | |||
| public static Nullness getGenericParameterNullnessAtInvocation( | |||
There was a problem hiding this comment.
Javadoc needs to be updated with the new @params.
| @@ -726,8 +749,12 @@ public Nullness getGenericParameterNullnessAtInvocation( | |||
| * @return nullability of the relevant parameter type of {@code method} in the context of {@code | |||
| * enclosingType} | |||
| */ | |||
| public Nullness getGenericMethodParameterNullness( | |||
| int parameterIndex, Symbol.MethodSymbol method, Type enclosingType) { | |||
| public static Nullness getGenericMethodParameterNullness( | |||
There was a problem hiding this comment.
Javadoc needs to be updated with the new @params.
| @@ -741,8 +768,8 @@ public Nullness getGenericMethodParameterNullness( | |||
| * @param tree tree for overriding method | |||
| * @param overriddenMethodType type of the overridden method | |||
| */ | |||
| private void checkTypeParameterNullnessForOverridingMethodParameterType( | |||
| MethodTree tree, Type overriddenMethodType) { | |||
| private static void checkTypeParameterNullnessForOverridingMethodParameterType( | |||
There was a problem hiding this comment.
Javadoc needs to be updated with the new @params.
| @@ -771,18 +801,20 @@ private void checkTypeParameterNullnessForOverridingMethodParameterType( | |||
| * @param tree tree for overriding method | |||
| * @param overriddenMethodType type of the overridden method | |||
| */ | |||
| private void checkTypeParameterNullnessForOverridingMethodReturnType( | |||
| MethodTree tree, Type overriddenMethodType) { | |||
| private static void checkTypeParameterNullnessForOverridingMethodReturnType( | |||
There was a problem hiding this comment.
Javadoc needs to be updated with the new @params.
| @@ -2414,6 +2413,10 @@ public ErrorBuilder getErrorBuilder() { | |||
| return errorBuilder; | |||
| } | |||
|
|
|||
| public Config getConfig() { | |||
There was a problem hiding this comment.
Sorry if this is unbelievably pedantic but: public method -> javadoc 😅 (Also, as a getter, maybe worth moving further up within this file? Closer to the field unless that breaks GJF rules?)
|
Thanks for helping keep our docs in a decent shape 🙂 @lazaroclapp this is ready for another look when you can |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously, some methods in
GenericsCheckswere static methods and some were instance methods. We did not get much of any benefit from the instance methods, as it would be painful to create a singleGenericsChecksobject and thread it everywhere it is needed. So, this PR just converts allGenericsChecksmethods to be static and passes parameters as needed (which isn't too painful).