From 4a2e8bc4cbec503368aca437873fa28c307af718 Mon Sep 17 00:00:00 2001 From: Theron Wang Date: Mon, 29 Jul 2024 14:07:37 -0400 Subject: [PATCH 1/7] fully qualify types in generic synthetic method return types --- .../specimin/JavaParserUtil.java | 10 ++++ .../specimin/MustImplementMethodsVisitor.java | 14 +----- .../specimin/UnsolvedSymbolVisitor.java | 47 +++++++++++++++++-- .../specimin/MethodReturnGenericTest.java | 24 ++++++++++ .../expected/com/bar/Bar.java | 16 +++++++ .../expected/com/example/InSamePackage.java | 4 ++ .../expected/com/example/Simple.java | 19 ++++++++ .../expected/com/foo/InOtherPackage.java | 4 ++ .../input/com/example/Simple.java | 18 +++++++ 9 files changed, 140 insertions(+), 16 deletions(-) create mode 100644 src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java create mode 100644 src/test/resources/methodreturngeneric/expected/com/bar/Bar.java create mode 100644 src/test/resources/methodreturngeneric/expected/com/example/InSamePackage.java create mode 100644 src/test/resources/methodreturngeneric/expected/com/example/Simple.java create mode 100644 src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage.java create mode 100644 src/test/resources/methodreturngeneric/input/com/example/Simple.java diff --git a/src/main/java/org/checkerframework/specimin/JavaParserUtil.java b/src/main/java/org/checkerframework/specimin/JavaParserUtil.java index b1aa5445..0962d656 100644 --- a/src/main/java/org/checkerframework/specimin/JavaParserUtil.java +++ b/src/main/java/org/checkerframework/specimin/JavaParserUtil.java @@ -85,6 +85,16 @@ public static ResolvedReferenceType classOrInterfaceTypeToResolvedReferenceType( return type.resolve().asReferenceType(); } + /** + * Erases type arguments from a method signature string. + * + * @param signature the signature + * @return the same signature without type arguments + */ + public static String erase(String signature) { + return signature.replaceAll("<.*>", ""); + } + /** * Returns the corresponding type name for an Expression within an annotation. * diff --git a/src/main/java/org/checkerframework/specimin/MustImplementMethodsVisitor.java b/src/main/java/org/checkerframework/specimin/MustImplementMethodsVisitor.java index cfc0a59e..98ffad51 100644 --- a/src/main/java/org/checkerframework/specimin/MustImplementMethodsVisitor.java +++ b/src/main/java/org/checkerframework/specimin/MustImplementMethodsVisitor.java @@ -277,12 +277,12 @@ private boolean overridesAnInterfaceMethodImpl( // (i.e., a raw type). Note though that this doesn't mean there are no type variables in the // signature: // add(E) is still add(E). - targetSignature = erase(targetSignature); + targetSignature = JavaParserUtil.erase(targetSignature); for (ResolvedMethodDeclaration methodInInterface : resolvedInterface.getAllMethodsVisibleToInheritors()) { try { - if (erase(methodInInterface.getSignature()).equals(targetSignature)) { + if (JavaParserUtil.erase(methodInInterface.getSignature()).equals(targetSignature)) { if (methodInInterface.isAbstract()) { // once we've found the correct method, we return to whether we // control it or not. If we don't, it must be preserved. If we do, then we only @@ -366,16 +366,6 @@ private static Collection getAllImplementations( return qualifiedNameToType.values(); } - /** - * Erases type arguments from a method signature string. - * - * @param signature the signature - * @return the same signature without type arguments - */ - private static String erase(String signature) { - return signature.replaceAll("<.*>", ""); - } - /** * Given a MethodDeclaration, this method returns the method that it overrides, if one exists in * one of its super classes. If one does not exist, it returns null. diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index bdd3cb2a..2f4a9772 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -3565,12 +3565,51 @@ private String lookupFQNs(String javacType) { new ModifierVisitor() { @Override public Visitable visit(ClassOrInterfaceType type, Void p) { - if (classAndPackageMap.containsKey(type.asString())) { - return new ClassOrInterfaceType( - classAndPackageMap.get(type.asString()) + "." + type.asString()); - } else { + StringBuilder fullyQualifiedName = new StringBuilder(); + if (classAndPackageMap.containsKey(JavaParserUtil.erase(type.asString()))) { + fullyQualifiedName.append(classAndPackageMap.get(type.asString())); + fullyQualifiedName.append("."); + } else if (!type.getTypeArguments().isPresent()) { + // This type is in the same package and doesn't contain any type parameters return super.visit(type, p); } + + NodeList typeArguments = type.getTypeArguments().orElse(null); + + if (typeArguments != null) { + fullyQualifiedName.append( + type.asString().substring(0, type.asString().indexOf('<') + 1)); + + for (int i = 0; i < typeArguments.size(); i++) { + Type typeArgument = typeArguments.get(i); + // Erase type parameters from class name to use in classAndPackageMap + String erased = JavaParserUtil.erase(typeArgument.asString()); + if (classAndPackageMap.containsKey(erased)) { + fullyQualifiedName + .append(classAndPackageMap.get(erased)) + .append(".") + .append(typeArgument.asString()); + } else if (JavaLangUtils.isJavaLangName(erased)) { + // Keep java.lang type arguments as is (Integer, String, etc.) + fullyQualifiedName.append(typeArgument.asString()); + } else { + // If it's not imported, it's probably in the same package + fullyQualifiedName + .append(currentPackage) + .append(".") + .append(typeArgument.toString()); + } + + if (i < typeArguments.size() - 1) { + fullyQualifiedName.append(", "); + } + } + fullyQualifiedName.append(">"); + } else { + fullyQualifiedName.append(type.asString()); + } + + return StaticJavaParser.parseClassOrInterfaceType(fullyQualifiedName.toString()); } }, null); diff --git a/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java b/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java new file mode 100644 index 00000000..d51825a7 --- /dev/null +++ b/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java @@ -0,0 +1,24 @@ +package org.checkerframework.specimin; + +import java.io.IOException; +import org.junit.Test; + +/** + * This test checks that Specimin includes the fully qualified class name in generics when + * generating a synthetic method. + * For example, if a return type is Foo, Specimin should always generate the synthetic + * method return type as com.example.Foo. + */ +public class MethodReturnGenericTest { + @Test + public void runTest() throws IOException { + SpeciminTestExecutor.runTestWithoutJarPaths( + "methodreturngeneric", + new String[] {"com/example/Simple.java"}, + new String[] { + "com.example.Simple#foo()", + "com.example.Simple#bar()", + "com.example.Simple#baz()" + }); + } +} diff --git a/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java b/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java new file mode 100644 index 00000000..5762efcd --- /dev/null +++ b/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java @@ -0,0 +1,16 @@ +package com.bar; + +public class Bar { + + public static com.bar.Bar getOtherPackage() { + throw new Error(); + } + + public static com.bar.Bar getSamePackage() { + throw new Error(); + } + + public static com.bar.Bar getJavaLang() { + throw new Error(); + } +} diff --git a/src/test/resources/methodreturngeneric/expected/com/example/InSamePackage.java b/src/test/resources/methodreturngeneric/expected/com/example/InSamePackage.java new file mode 100644 index 00000000..54524550 --- /dev/null +++ b/src/test/resources/methodreturngeneric/expected/com/example/InSamePackage.java @@ -0,0 +1,4 @@ +package com.example; + +public class InSamePackage { +} diff --git a/src/test/resources/methodreturngeneric/expected/com/example/Simple.java b/src/test/resources/methodreturngeneric/expected/com/example/Simple.java new file mode 100644 index 00000000..9dc11bd3 --- /dev/null +++ b/src/test/resources/methodreturngeneric/expected/com/example/Simple.java @@ -0,0 +1,19 @@ +package com.example; + +import com.bar.Bar; +import com.foo.InOtherPackage; + +public class Simple { + + public Bar foo() { + return Bar.getOtherPackage(); + } + + public Bar bar() { + return Bar.getSamePackage(); + } + + public Bar baz() { + return Bar.getJavaLang(); + } +} diff --git a/src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage.java b/src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage.java new file mode 100644 index 00000000..28c4334b --- /dev/null +++ b/src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage.java @@ -0,0 +1,4 @@ +package com.foo; + +public class InOtherPackage { +} diff --git a/src/test/resources/methodreturngeneric/input/com/example/Simple.java b/src/test/resources/methodreturngeneric/input/com/example/Simple.java new file mode 100644 index 00000000..5913800f --- /dev/null +++ b/src/test/resources/methodreturngeneric/input/com/example/Simple.java @@ -0,0 +1,18 @@ +package com.example; + +import com.bar.Bar; +import com.foo.InOtherPackage; + +public class Simple { + public Bar foo() { + return Bar.getOtherPackage(); + } + + public Bar bar() { + return Bar.getSamePackage(); + } + + public Bar baz() { + return Bar.getJavaLang(); + } +} From 7d41afbb89cb045bf989ef348aafc8c224265611 Mon Sep 17 00:00:00 2001 From: Theron Wang Date: Mon, 29 Jul 2024 14:49:45 -0400 Subject: [PATCH 2/7] Handle wildcards --- .../specimin/UnsolvedSymbolVisitor.java | 51 ++++++++++++------- .../specimin/MethodReturnGenericTest.java | 13 ++--- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index 2f4a9772..3265abc3 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -3582,23 +3582,7 @@ public Visitable visit(ClassOrInterfaceType type, Void p) { for (int i = 0; i < typeArguments.size(); i++) { Type typeArgument = typeArguments.get(i); - // Erase type parameters from class name to use in classAndPackageMap - String erased = JavaParserUtil.erase(typeArgument.asString()); - if (classAndPackageMap.containsKey(erased)) { - fullyQualifiedName - .append(classAndPackageMap.get(erased)) - .append(".") - .append(typeArgument.asString()); - } else if (JavaLangUtils.isJavaLangName(erased)) { - // Keep java.lang type arguments as is (Integer, String, etc.) - fullyQualifiedName.append(typeArgument.asString()); - } else { - // If it's not imported, it's probably in the same package - fullyQualifiedName - .append(currentPackage) - .append(".") - .append(typeArgument.toString()); - } + lookupTypeArgumentFQN(fullyQualifiedName, typeArgument); if (i < typeArguments.size() - 1) { fullyQualifiedName.append(", "); @@ -3616,6 +3600,39 @@ public Visitable visit(ClassOrInterfaceType type, Void p) { return typeVarDecl + parsedJavac.toString(); } + /** + * Helper method for lookupFQNs which adds the fully qualified type argument to + * fullyQualifiedName. + * + * @param fullyQualifiedName the fully qualified name to build + * @param typeArgument the type argument to lookup + */ + private void lookupTypeArgumentFQN(StringBuilder fullyQualifiedName, Type typeArgument) { + String erased = JavaParserUtil.erase(typeArgument.asString()); + if (classAndPackageMap.containsKey(erased)) { + fullyQualifiedName + .append(classAndPackageMap.get(erased)) + .append(".") + .append(typeArgument.asString()); + } else if (JavaLangUtils.isJavaLangName(erased)) { + // Keep java.lang type arguments as is (Integer, String, etc.) + fullyQualifiedName.append(typeArgument.asString()); + } else if (typeArgument.isWildcardType()) { + WildcardType asWildcardType = typeArgument.asWildcardType(); + + if (asWildcardType.getSuperType().isPresent()) { + fullyQualifiedName.append("? super "); + lookupTypeArgumentFQN(fullyQualifiedName, asWildcardType.getSuperType().get()); + } else if (asWildcardType.getExtendedType().isPresent()) { + fullyQualifiedName.append("? extends "); + lookupTypeArgumentFQN(fullyQualifiedName, asWildcardType.getExtendedType().get()); + } + } else { + // If it's not imported, it's probably in the same package + fullyQualifiedName.append(currentPackage).append(".").append(typeArgument.toString()); + } + } + /** * If and only if synthetic classes exist for both input type names, this method migrates all the * content of the sythetic class for the incorrect type name to the synthetic class for the diff --git a/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java b/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java index d51825a7..7fc2072f 100644 --- a/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java +++ b/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java @@ -4,10 +4,9 @@ import org.junit.Test; /** - * This test checks that Specimin includes the fully qualified class name in generics when - * generating a synthetic method. - * For example, if a return type is Foo, Specimin should always generate the synthetic - * method return type as com.example.Foo. + * This test checks that Specimin includes the fully qualified class name in generics when + * generating a synthetic method. For example, if a return type is Foo, Specimin should always + * generate the synthetic method return type as com.example.Foo. */ public class MethodReturnGenericTest { @Test @@ -15,10 +14,8 @@ public void runTest() throws IOException { SpeciminTestExecutor.runTestWithoutJarPaths( "methodreturngeneric", new String[] {"com/example/Simple.java"}, - new String[] { - "com.example.Simple#foo()", - "com.example.Simple#bar()", - "com.example.Simple#baz()" + new String[] { + "com.example.Simple#foo()", "com.example.Simple#bar()", "com.example.Simple#baz()" }); } } From d046c73b5274825e72b42909004a5f5d0d51164d Mon Sep 17 00:00:00 2001 From: Theron Wang Date: Mon, 29 Jul 2024 16:47:07 -0400 Subject: [PATCH 3/7] check if type argument is already fully qualified --- .../org/checkerframework/specimin/UnsolvedSymbolVisitor.java | 3 +++ .../checkerframework/specimin/MethodReturnGenericTest.java | 5 ++++- .../resources/methodreturngeneric/expected/com/bar/Bar.java | 4 ++++ .../methodreturngeneric/expected/com/example/Simple.java | 4 ++++ .../expected/com/foo/InOtherPackage2.java | 4 ++++ .../methodreturngeneric/input/com/example/Simple.java | 4 ++++ 6 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage2.java diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index 3265abc3..0b15683d 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -3627,6 +3627,9 @@ private void lookupTypeArgumentFQN(StringBuilder fullyQualifiedName, Type typeAr fullyQualifiedName.append("? extends "); lookupTypeArgumentFQN(fullyQualifiedName, asWildcardType.getExtendedType().get()); } + } else if (isAClassPath(erased)) { + // If it's already a fully qualified name, don't do anything + fullyQualifiedName.append(typeArgument.asString()); } else { // If it's not imported, it's probably in the same package fullyQualifiedName.append(currentPackage).append(".").append(typeArgument.toString()); diff --git a/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java b/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java index 7fc2072f..9bf66e1a 100644 --- a/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java +++ b/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java @@ -15,7 +15,10 @@ public void runTest() throws IOException { "methodreturngeneric", new String[] {"com/example/Simple.java"}, new String[] { - "com.example.Simple#foo()", "com.example.Simple#bar()", "com.example.Simple#baz()" + "com.example.Simple#foo()", + "com.example.Simple#bar()", + "com.example.Simple#baz()", + "com.example.Simple#alreadyQualified()" }); } } diff --git a/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java b/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java index 5762efcd..6b413a91 100644 --- a/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java +++ b/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java @@ -13,4 +13,8 @@ public static com.bar.Bar getSamePackage() { public static com.bar.Bar getJavaLang() { throw new Error(); } + + public static com.bar.Bar getOtherPackage2() { + throw new Error(); + } } diff --git a/src/test/resources/methodreturngeneric/expected/com/example/Simple.java b/src/test/resources/methodreturngeneric/expected/com/example/Simple.java index 9dc11bd3..c9b3af5f 100644 --- a/src/test/resources/methodreturngeneric/expected/com/example/Simple.java +++ b/src/test/resources/methodreturngeneric/expected/com/example/Simple.java @@ -16,4 +16,8 @@ public Bar bar() { public Bar baz() { return Bar.getJavaLang(); } + + public Bar alreadyQualified() { + return Bar.getOtherPackage2(); + } } diff --git a/src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage2.java b/src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage2.java new file mode 100644 index 00000000..dc8c7b9e --- /dev/null +++ b/src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage2.java @@ -0,0 +1,4 @@ +package com.foo; + +public class InOtherPackage2 { +} diff --git a/src/test/resources/methodreturngeneric/input/com/example/Simple.java b/src/test/resources/methodreturngeneric/input/com/example/Simple.java index 5913800f..495b9962 100644 --- a/src/test/resources/methodreturngeneric/input/com/example/Simple.java +++ b/src/test/resources/methodreturngeneric/input/com/example/Simple.java @@ -15,4 +15,8 @@ public Bar bar() { public Bar baz() { return Bar.getJavaLang(); } + + public Bar alreadyQualified() { + return Bar.getOtherPackage2(); + } } From a3037f42cc2036adfbd4a3a3c0eefd67b85a1c53 Mon Sep 17 00:00:00 2001 From: Theron Wang Date: Tue, 30 Jul 2024 11:51:11 -0400 Subject: [PATCH 4/7] test case + todo --- .../specimin/UnsolvedSymbolVisitor.java | 4 ++++ .../resources/min_program_compile_status.json | 2 +- ...MethodReturnFullyQualifiedGenericTest.java | 20 +++++++++++++++++++ .../expected/com/bar/Bar.java | 8 ++++++++ .../expected/com/example/Simple.java | 10 ++++++++++ .../expected/com/foo/InOtherPackage2.java | 4 ++++ .../input/com/example/Simple.java | 9 +++++++++ typecheck_one_test.sh | 2 ++ typecheck_test_outputs.bat | 2 ++ 9 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/checkerframework/specimin/MethodReturnFullyQualifiedGenericTest.java create mode 100644 src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/bar/Bar.java create mode 100644 src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/example/Simple.java create mode 100644 src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/foo/InOtherPackage2.java create mode 100644 src/test/resources/methodreturnfullyqualifiedgeneric/input/com/example/Simple.java diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index 1185f68f..0f03b260 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -3665,6 +3665,10 @@ private void lookupTypeArgumentFQN(StringBuilder fullyQualifiedName, Type typeAr fullyQualifiedName.append(typeArgument.asString()); } else { // If it's not imported, it's probably in the same package + // TODO: handle already fully qualified generic type arguments. Right now JavaTypeCorrect + // only outputs the simple class name, so there is no way to get its fully qualified name + // here without ambiguity (i.e. org.example.Foo and com.example.Foo are different signatures) + // with the same simple class name fullyQualifiedName.append(currentPackage).append(".").append(typeArgument.toString()); } } diff --git a/src/main/resources/min_program_compile_status.json b/src/main/resources/min_program_compile_status.json index b0a27e89..94bd06d6 100644 --- a/src/main/resources/min_program_compile_status.json +++ b/src/main/resources/min_program_compile_status.json @@ -28,5 +28,5 @@ "na-389": "PASS", "na-705": "PASS", "na-791a": "PASS", - "na-791b": "FAIL" + "na-791b": "PASS" } diff --git a/src/test/java/org/checkerframework/specimin/MethodReturnFullyQualifiedGenericTest.java b/src/test/java/org/checkerframework/specimin/MethodReturnFullyQualifiedGenericTest.java new file mode 100644 index 00000000..70d1bfcf --- /dev/null +++ b/src/test/java/org/checkerframework/specimin/MethodReturnFullyQualifiedGenericTest.java @@ -0,0 +1,20 @@ +package org.checkerframework.specimin; + +import java.io.IOException; +import org.junit.Test; + +/** + * This test checks that Specimin includes the fully qualified class name in generics when + * generating a synthetic method. For example, if a return type is Bar, the actual + * synthetic return type should be com.qualified.Bar. At this moment, we should expect + * this test case to fail. + */ +public class MethodReturnFullyQualifiedGenericTest { + @Test + public void runTest() throws IOException { + SpeciminTestExecutor.runTestWithoutJarPaths( + "methodreturnfullyqualifiedgeneric", + new String[] {"com/example/Simple.java"}, + new String[] {"com.example.Simple#alreadyQualified()"}); + } +} diff --git a/src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/bar/Bar.java b/src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/bar/Bar.java new file mode 100644 index 00000000..701edad9 --- /dev/null +++ b/src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/bar/Bar.java @@ -0,0 +1,8 @@ +package com.bar; + +public class Bar { + + public static com.bar.Bar getOtherPackage2() { + throw new Error(); + } +} diff --git a/src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/example/Simple.java b/src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/example/Simple.java new file mode 100644 index 00000000..5d09c4d1 --- /dev/null +++ b/src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/example/Simple.java @@ -0,0 +1,10 @@ +package com.example; + +import com.bar.Bar; + +public class Simple { + + public Bar alreadyQualified() { + return Bar.getOtherPackage2(); + } +} diff --git a/src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/foo/InOtherPackage2.java b/src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/foo/InOtherPackage2.java new file mode 100644 index 00000000..dc8c7b9e --- /dev/null +++ b/src/test/resources/methodreturnfullyqualifiedgeneric/expected/com/foo/InOtherPackage2.java @@ -0,0 +1,4 @@ +package com.foo; + +public class InOtherPackage2 { +} diff --git a/src/test/resources/methodreturnfullyqualifiedgeneric/input/com/example/Simple.java b/src/test/resources/methodreturnfullyqualifiedgeneric/input/com/example/Simple.java new file mode 100644 index 00000000..eff46a48 --- /dev/null +++ b/src/test/resources/methodreturnfullyqualifiedgeneric/input/com/example/Simple.java @@ -0,0 +1,9 @@ +package com.example; + +import com.bar.Bar; + +public class Simple { + public Bar alreadyQualified() { + return Bar.getOtherPackage2(); + } +} diff --git a/typecheck_one_test.sh b/typecheck_one_test.sh index 019ea2bd..e8868514 100644 --- a/typecheck_one_test.sh +++ b/typecheck_one_test.sh @@ -14,6 +14,8 @@ if [ "${testcase}" = "superinterfaceextends" ]; then exit 0; fi # incomplete handling of method references: https://github.com/njit-jerse/specimin/issues/291 # this test exists to check that no crash occurs, not that Specimin produces the correct output if [ "${testcase}" = "methodref2" ]; then exit 0; fi +# this test will not compile right now; this is a TODO in UnsolvedSymbolVisitor#lookupTypeArgumentFQN +if [ "${testcase}" = "methodreturnfullyqualifiedgeneric" ]; then exit 0; fi cd "${testcase}/expected/" || exit 2 # javac relies on word splitting # shellcheck disable=SC2046 diff --git a/typecheck_test_outputs.bat b/typecheck_test_outputs.bat index b07d0dea..9efe890a 100644 --- a/typecheck_test_outputs.bat +++ b/typecheck_test_outputs.bat @@ -20,6 +20,8 @@ for /d %%t in (*) do ( rem incomplete handling of method references: https://github.com/njit-jerse/specimin/issues/291 rem this test exists to check that no crash occurs, not that Specimin produces the correct output if "%%t"=="methodref2" set continue=1 + rem this test will not compile right now; this is a TODO in UnsolvedSymbolVisitor#lookupTypeArgumentFQN + if "%%t"=="methodreturnfullyqualifiedgeneric" set continue=1 if !continue!==0 ( cd "%%t/expected/" || exit 1 rem javac relies on word splitting From a43bf5a5693fd6dc5c37dde609b972a347c5e252 Mon Sep 17 00:00:00 2001 From: Theron Wang Date: Tue, 30 Jul 2024 11:51:59 -0400 Subject: [PATCH 5/7] change previous test case --- .../resources/methodreturngeneric/expected/com/bar/Bar.java | 4 ---- .../methodreturngeneric/expected/com/example/Simple.java | 4 ---- .../methodreturngeneric/expected/com/foo/InOtherPackage2.java | 4 ---- .../methodreturngeneric/input/com/example/Simple.java | 4 ---- 4 files changed, 16 deletions(-) delete mode 100644 src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage2.java diff --git a/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java b/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java index 6b413a91..5762efcd 100644 --- a/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java +++ b/src/test/resources/methodreturngeneric/expected/com/bar/Bar.java @@ -13,8 +13,4 @@ public static com.bar.Bar getSamePackage() { public static com.bar.Bar getJavaLang() { throw new Error(); } - - public static com.bar.Bar getOtherPackage2() { - throw new Error(); - } } diff --git a/src/test/resources/methodreturngeneric/expected/com/example/Simple.java b/src/test/resources/methodreturngeneric/expected/com/example/Simple.java index c9b3af5f..9dc11bd3 100644 --- a/src/test/resources/methodreturngeneric/expected/com/example/Simple.java +++ b/src/test/resources/methodreturngeneric/expected/com/example/Simple.java @@ -16,8 +16,4 @@ public Bar bar() { public Bar baz() { return Bar.getJavaLang(); } - - public Bar alreadyQualified() { - return Bar.getOtherPackage2(); - } } diff --git a/src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage2.java b/src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage2.java deleted file mode 100644 index dc8c7b9e..00000000 --- a/src/test/resources/methodreturngeneric/expected/com/foo/InOtherPackage2.java +++ /dev/null @@ -1,4 +0,0 @@ -package com.foo; - -public class InOtherPackage2 { -} diff --git a/src/test/resources/methodreturngeneric/input/com/example/Simple.java b/src/test/resources/methodreturngeneric/input/com/example/Simple.java index 495b9962..5913800f 100644 --- a/src/test/resources/methodreturngeneric/input/com/example/Simple.java +++ b/src/test/resources/methodreturngeneric/input/com/example/Simple.java @@ -15,8 +15,4 @@ public Bar bar() { public Bar baz() { return Bar.getJavaLang(); } - - public Bar alreadyQualified() { - return Bar.getOtherPackage2(); - } } From e9c1b1453837069eadc9bdce3379c61eed41063c Mon Sep 17 00:00:00 2001 From: Theron Wang Date: Tue, 30 Jul 2024 11:55:41 -0400 Subject: [PATCH 6/7] fix test --- .../org/checkerframework/specimin/MethodReturnGenericTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java b/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java index 9bf66e1a..fe1d448f 100644 --- a/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java +++ b/src/test/java/org/checkerframework/specimin/MethodReturnGenericTest.java @@ -17,8 +17,7 @@ public void runTest() throws IOException { new String[] { "com.example.Simple#foo()", "com.example.Simple#bar()", - "com.example.Simple#baz()", - "com.example.Simple#alreadyQualified()" + "com.example.Simple#baz()" }); } } From 62a2ca32569889cf547678a984417e48da7a35fb Mon Sep 17 00:00:00 2001 From: Theron Wang Date: Tue, 30 Jul 2024 12:48:00 -0400 Subject: [PATCH 7/7] change comments --- .../org/checkerframework/specimin/UnsolvedSymbolVisitor.java | 4 +++- .../specimin/MethodReturnFullyQualifiedGenericTest.java | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index 0f03b260..e4482a43 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -3667,8 +3667,10 @@ private void lookupTypeArgumentFQN(StringBuilder fullyQualifiedName, Type typeAr // If it's not imported, it's probably in the same package // TODO: handle already fully qualified generic type arguments. Right now JavaTypeCorrect // only outputs the simple class name, so there is no way to get its fully qualified name - // here without ambiguity (i.e. org.example.Foo and com.example.Foo are different signatures) + // here without ambiguity (i.e. org.example.Foo and com.example.Foo have different signatures) // with the same simple class name + // Check MethodReturnFullyQualifiedGenericTest for more details; the current return type in + // Bar.java is com.example.InOtherPackage2 rather than com.foo.InOtherPackage2 (expected) fullyQualifiedName.append(currentPackage).append(".").append(typeArgument.toString()); } } diff --git a/src/test/java/org/checkerframework/specimin/MethodReturnFullyQualifiedGenericTest.java b/src/test/java/org/checkerframework/specimin/MethodReturnFullyQualifiedGenericTest.java index 70d1bfcf..0bdf967a 100644 --- a/src/test/java/org/checkerframework/specimin/MethodReturnFullyQualifiedGenericTest.java +++ b/src/test/java/org/checkerframework/specimin/MethodReturnFullyQualifiedGenericTest.java @@ -6,8 +6,9 @@ /** * This test checks that Specimin includes the fully qualified class name in generics when * generating a synthetic method. For example, if a return type is Bar, the actual - * synthetic return type should be com.qualified.Bar. At this moment, we should expect - * this test case to fail. + * synthetic return type should be com.qualified.Bar. This test encodes Specimin's + * current behavior, which doesn't produce compilable output. If you break this test, it might + * not be a bad thing. */ public class MethodReturnFullyQualifiedGenericTest { @Test