Skip to content
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.

Sign up for GitHub

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

Make FindMissingTypes stricter for method invocations #4688

Merged
merged 7 commits into from
Nov 20, 2024
Merged

Make FindMissingTypes stricter for method invocations #4688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
920c74d
Make `FindMissingTypes` stricter for method invocations
knutwannheden Nov 20, 2024
7cfe437
Try to fix linking in `ReloadableJava17JavadocVisitor`
knutwannheden Nov 20, 2024
3ad943b
Try to fix `TypeUtils#isAssignableTo()`
knutwannheden Nov 20, 2024
df18f76
Also update Java 11 and Java 21 Javadoc visitors
knutwannheden Nov 20, 2024
80b5d41
Also update Java 11 and Java 21 Javadoc visitors
knutwannheden Nov 20, 2024
a1834a8
More updates and simplifications
knutwannheden Nov 20, 2024
428ee7c
Corner case in `TypeUtils`
knutwannheden Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 17 additions & 31 deletions ...e-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11JavadocVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ private void init() {
} else {
// Handle consecutive new lines.
if ((prev == '\n' ||
prev == '\r' && source.charAt(i - 2) == '\n')) {
prev == '\r' && source.charAt(i - 2) == '\n')) {
String prevLineLine = prev == '\n' ? "\n" : "\r\n";
lineBreaks.put(javadocContent.length(), new Javadoc.LineBreak(randomId(), prevLineLine, Markers.EMPTY));
} else if (marginBuilder != null) { // A new line with no '*' that only contains whitespace.
Expand Down Expand Up @@ -581,14 +581,14 @@ public Tree visitProvides(ProvidesTree node, List<Javadoc> body) {
if (ref.qualifierExpression != null) {
try {
attr.attribType(ref.qualifierExpression, symbol);
} catch(NullPointerException ignored) {
} catch (NullPointerException ignored) {
// best effort, can result in:
// java.lang.NullPointerException: Cannot read field "info" because "env" is null
// at com.sun.tools.javac.comp.Attr.attribType(Attr.java:404)
}
}

if(ref.qualifierExpression != null) {
if (ref.qualifierExpression != null) {
qualifier = (TypedTree) javaVisitor.scan(ref.qualifierExpression, Space.EMPTY);
qualifierType = qualifier.getType();
if (ref.memberName != null) {
Expand Down Expand Up @@ -678,32 +678,23 @@ public Tree visitProvides(ProvidesTree node, List<Javadoc> body) {
}

private JavaType.@Nullable Method methodReferenceType(DCTree.DCReference ref, @Nullable JavaType type) {
if (type instanceof JavaType.Class) {
if (type instanceof JavaType.Class) {
JavaType.Class classType = (JavaType.Class) type;

nextMethod:
for (JavaType.Method method : classType.getMethods()) {
if (method.getName().equals(ref.memberName.toString())) {
if (ref.paramTypes != null) {
for (JCTree param : ref.paramTypes) {
for (JavaType testParamType : method.getParameterTypes()) {
Type paramType = attr.attribType(param, symbol);
if (testParamType instanceof JavaType.GenericTypeVariable) {
List<JavaType> bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds();
if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) {
return method;
}
for (JavaType bound : bounds) {
if (paramTypeMatches(bound, paramType)) {
return method;
}
}
continue nextMethod;
}

if (paramTypeMatches(testParamType, paramType)) {
continue nextMethod;
}
List<JavaType> parameterTypes = method.getParameterTypes();
if (ref.paramTypes.size() != parameterTypes.size()) {
continue;
}
for (int i = 0; i < ref.paramTypes.size(); i++) {
JCTree param = ref.paramTypes.get(i);
JavaType testParamType = parameterTypes.get(i);
Type paramType = attr.attribType(param, symbol);
if (!paramTypeMatches(testParamType, paramType)) {
continue nextMethod;
}
}
}
Expand All @@ -725,12 +716,7 @@ public Tree visitProvides(ProvidesTree node, List<Javadoc> body) {
}

private boolean paramTypeMatches(JavaType testParamType, Type paramType) {
if (paramType instanceof Type.ClassType) {
JavaType.FullyQualified fqTestParamType = TypeUtils.asFullyQualified(testParamType);
return fqTestParamType == null || !fqTestParamType.getFullyQualifiedName().equals(((Symbol.ClassSymbol) paramType.tsym)
.fullname.toString());
}
return false;
return TypeUtils.isAssignableTo(testParamType, typeMapping.type(paramType));
}

private JavaType.@Nullable Variable fieldReferenceType(DCTree.DCReference ref, @Nullable JavaType type) {
Expand Down Expand Up @@ -865,14 +851,14 @@ public Tree visitVersion(VersionTree node, List<Javadoc> body) {
@Override
public Tree visitText(TextTree node, List<Javadoc> body) {
throw new UnsupportedOperationException("Anywhere text can occur, we need to call the visitText override that " +
"returns a list of Javadoc elements.");
"returns a list of Javadoc elements.");
}

public List<Javadoc> visitText(String node) {
List<Javadoc> texts = new ArrayList<>();

if (!node.isEmpty() && Character.isWhitespace(node.charAt(0)) &&
!Character.isWhitespace(source.charAt(cursor))) {
!Character.isWhitespace(source.charAt(cursor))) {
node = node.stripLeading();
}

Expand Down
36 changes: 11 additions & 25 deletions ...e-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17JavadocVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,25 +688,16 @@ public Tree visitProvides(ProvidesTree node, List<Javadoc> body) {
for (JavaType.Method method : classType.getMethods()) {
if (method.getName().equals(ref.memberName.toString())) {
if (ref.paramTypes != null) {
for (JCTree param : ref.paramTypes) {
for (JavaType testParamType : method.getParameterTypes()) {
Type paramType = attr.attribType(param, symbol);
if (testParamType instanceof JavaType.GenericTypeVariable) {
List<JavaType> bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds();
if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) {
return method;
}
for (JavaType bound : bounds) {
if (paramTypeMatches(bound, paramType)) {
return method;
}
}
continue nextMethod;
}

if (paramTypeMatches(testParamType, paramType)) {
continue nextMethod;
}
List<JavaType> parameterTypes = method.getParameterTypes();
if (ref.paramTypes.size() != parameterTypes.size()) {
continue;
}
for (int i = 0; i < ref.paramTypes.size(); i++) {
JCTree param = ref.paramTypes.get(i);
JavaType testParamType = parameterTypes.get(i);
Type paramType = attr.attribType(param, symbol);
if (!paramTypeMatches(testParamType, paramType)) {
continue nextMethod;
}
}
}
Expand All @@ -728,12 +719,7 @@ public Tree visitProvides(ProvidesTree node, List<Javadoc> body) {
}

private boolean paramTypeMatches(JavaType testParamType, Type paramType) {
if (paramType instanceof Type.ClassType) {
JavaType.FullyQualified fqTestParamType = TypeUtils.asFullyQualified(testParamType);
return fqTestParamType == null || !fqTestParamType.getFullyQualifiedName().equals(((Symbol.ClassSymbol) paramType.tsym)
.fullname.toString());
}
return false;
return TypeUtils.isAssignableTo(testParamType, typeMapping.type(paramType));
}

private JavaType.@Nullable Variable fieldReferenceType(DCTree.DCReference ref, @Nullable JavaType type) {
Expand Down
36 changes: 11 additions & 25 deletions ...e-java-21/src/main/java/org/openrewrite/java/isolated/ReloadableJava21JavadocVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,25 +688,16 @@ public Tree visitProvides(ProvidesTree node, List<Javadoc> body) {
for (JavaType.Method method : classType.getMethods()) {
if (method.getName().equals(ref.memberName.toString())) {
if (ref.paramTypes != null) {
for (JCTree param : ref.paramTypes) {
for (JavaType testParamType : method.getParameterTypes()) {
Type paramType = attr.attribType(param, symbol);
if (testParamType instanceof JavaType.GenericTypeVariable) {
List<JavaType> bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds();
if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) {
return method;
}
for (JavaType bound : bounds) {
if (paramTypeMatches(bound, paramType)) {
return method;
}
}
continue nextMethod;
}

if (paramTypeMatches(testParamType, paramType)) {
continue nextMethod;
}
List<JavaType> parameterTypes = method.getParameterTypes();
if (ref.paramTypes.size() != parameterTypes.size()) {
continue;
}
for (int i = 0; i < ref.paramTypes.size(); i++) {
JCTree param = ref.paramTypes.get(i);
JavaType testParamType = parameterTypes.get(i);
Type paramType = attr.attribType(param, symbol);
if (!paramTypeMatches(testParamType, paramType)) {
continue nextMethod;
}
}
}
Expand All @@ -728,12 +719,7 @@ public Tree visitProvides(ProvidesTree node, List<Javadoc> body) {
}

private boolean paramTypeMatches(JavaType testParamType, Type paramType) {
if (paramType instanceof Type.ClassType) {
JavaType.FullyQualified fqTestParamType = TypeUtils.asFullyQualified(testParamType);
return fqTestParamType == null || !fqTestParamType.getFullyQualifiedName().equals(((Symbol.ClassSymbol) paramType.tsym)
.fullname.toString());
}
return false;
return TypeUtils.isAssignableTo(testParamType, typeMapping.type(paramType));
}

private JavaType.@Nullable Variable fieldReferenceType(DCTree.DCReference ref, @Nullable JavaType type) {
Expand Down
36 changes: 11 additions & 25 deletions rewrite-java-8/src/main/java/org/openrewrite/java/ReloadableJava8JavadocVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,25 +647,16 @@ public Tree visitParam(ParamTree node, List<Javadoc> body) {
for (JavaType.Method method : classType.getMethods()) {
if (method.getName().equals(ref.memberName.toString())) {
if (ref.paramTypes != null) {
for (JCTree param : ref.paramTypes) {
for (JavaType testParamType : method.getParameterTypes()) {
Type paramType = attr.attribType(param, symbol);
if (testParamType instanceof JavaType.GenericTypeVariable) {
List<JavaType> bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds();
if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) {
return method;
}
for (JavaType bound : bounds) {
if (paramTypeMatches(bound, paramType)) {
return method;
}
}
continue nextMethod;
}

if (paramTypeMatches(testParamType, paramType)) {
continue nextMethod;
}
List<JavaType> parameterTypes = method.getParameterTypes();
if (ref.paramTypes.size() != parameterTypes.size()) {
continue;
}
for (int i = 0; i < ref.paramTypes.size(); i++) {
JCTree param = ref.paramTypes.get(i);
JavaType testParamType = parameterTypes.get(i);
Type paramType = attr.attribType(param, symbol);
if (!paramTypeMatches(testParamType, paramType)) {
continue nextMethod;
}
}
}
Expand All @@ -679,12 +670,7 @@ public Tree visitParam(ParamTree node, List<Javadoc> body) {
}

private boolean paramTypeMatches(JavaType testParamType, Type paramType) {
if (paramType instanceof Type.ClassType) {
JavaType.FullyQualified fqTestParamType = TypeUtils.asFullyQualified(testParamType);
return fqTestParamType == null || !fqTestParamType.getFullyQualifiedName().equals(((Symbol.ClassSymbol) paramType.tsym)
.fullname.toString());
}
return false;
return TypeUtils.isAssignableTo(testParamType, typeMapping.type(paramType));
}

private JavaType.@Nullable Variable fieldReferenceType(DCTree.DCReference ref, @Nullable JavaType type) {
Expand Down
2 changes: 1 addition & 1 deletion rewrite-java-tck/src/main/java/org/openrewrite/java/tree/JavadocTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1662,7 +1662,7 @@ void arrayTypeLiterals2() {
rewriteRun(
java("" +
"/**\n" +
" * <p>Values are converted to strings using {@link java.util.Arrays#compare(Comparable[], Comparable[])}}.\n" +
" * <p>Values are converted to strings using {@link java.util.Arrays#compare(Comparable[], Comparable[])}.\n" +
" */\n" +
"class A {}"
)
Expand Down
12 changes: 12 additions & 0 deletions rewrite-java/src/main/java/org/openrewrite/java/search/FindMissingTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,18 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
// A different object in one implies a type has changed, either in the method signature or deeper in the type tree.
mi = SearchResult.found(mi, "MethodInvocation#name#type is not the same instance as the MethodType of MethodInvocation.");
}
if (type != null) {
int argCount = 0;
for (Expression argument : mi.getArguments()) {
if (!(argument instanceof J.Empty)) {
argCount++;
}
}
int minCount = type.hasFlags(Flag.Varargs) ? type.getParameterTypes().size() - 1 : type.getParameterTypes().size();
if (argCount < minCount) {
mi = SearchResult.found(mi, "argument count mismatch: " + argCount + " != " + type.getParameterTypes().size());
}
}
}
return mi;
}
Expand Down
6 changes: 2 additions & 4 deletions rewrite-java/src/main/java/org/openrewrite/java/tree/TypeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,8 @@ public static boolean isAssignableTo(@Nullable JavaType to, @Nullable JavaType f
}
JavaType.GenericTypeVariable toGeneric = (JavaType.GenericTypeVariable) to;
List<JavaType> toBounds = toGeneric.getBounds();
if (!toGeneric.getName().equals("?")) {
return false;
if (toBounds.isEmpty()) {
return from instanceof JavaType.FullyQualified;
} else if (toGeneric.getVariance() == JavaType.GenericTypeVariable.Variance.COVARIANT) {
for (JavaType toBound : toBounds) {
if (!isAssignableTo(toBound, from)) {
Expand All @@ -288,8 +288,6 @@ public static boolean isAssignableTo(@Nullable JavaType to, @Nullable JavaType f
}
}
return true;
} else if (toBounds.isEmpty()) {
return from instanceof JavaType.FullyQualified;
}
return false;
} else if (to instanceof JavaType.Variable) {
Expand Down