Better type attribution for ReplaceLambdaWithMethodReference
#131
Conversation
Improves the type attribution for method references by "manually" instantiating the `J.MemberReference` rather than via `JavaTemplate`. This may seem strange, but the reason is that the recipe doesn't know where the type is coming from (JDK, 3rd party library, or even source path), so the template's compiler may not have the type on its classpath and cannot really produce a properly type-attributed result.
|
|
||
| import static org.openrewrite.Tree.randomId; | ||
|
|
||
| final class JavaElementFactory { |
There was a problem hiding this comment.
Please note that this class was intentionally not made public. I don't know how we intend to address this kind of problem in the long run (creating LST elements which potentially refer to types from the source set). While I think it would be nice if JavaTemplate could be used for this, I have my doubts if we will be able to provide this any time soon. So in the meantime I suggest that we have a factory like this one.
| J.Binary binary = (J.Binary) body; | ||
| if (isNullCheck(binary.getLeft(), binary.getRight()) || | ||
| isNullCheck(binary.getRight(), binary.getLeft())) { | ||
| maybeAddImport("java.util.Objects"); |
There was a problem hiding this comment.
This is basically how it all got started: I wanted to use the new ShortenFullyQualifiedTypeReferences recipe to allow removing this line (to avoid conflicts with any other imports for types with Objects as the simple name). That is how I discovered the incorrect type attribution with the stub from a few lines above.
| !(j.getType() instanceof JavaType.GenericTypeVariable)) { | ||
| body = tree; | ||
| code = "#{}.class::cast"; | ||
| J tree = j.getTree(); |
There was a problem hiding this comment.
We were checking if j could be null previously, it was not needed?
There was a problem hiding this comment.
TypeCast#getClazz() is not annotated as @Nullable, so I think this should be fine.
| "Objects::nonNull"; | ||
| doAfterVisit(new ShortenFullyQualifiedTypeReferences().getVisitor()); | ||
| code = J.Binary.Type.Equal.equals(binary.getOperator()) ? "java.util.Objects::isNull" : | ||
| "java.util.Objects::nonNull"; |
There was a problem hiding this comment.
OpenRewrite doesn't provide any API to do a lookup of types. That would sometimes be useful to have.
Improves the type attribution for method references by "manually" instantiating the
J.MemberReferencerather than viaJavaTemplate. This may seem strange, but the reason is that the recipe doesn't know where the type is coming from (JDK, 3rd party library, or even source path), so the template's compiler may not have the type on its classpath and cannot really produce a properly type-attributed result.