From b26a6de39880d63bf358fddfd10847008a9bb12a Mon Sep 17 00:00:00 2001 From: gmerr3 <127778000+gmerr3@users.noreply.github.com> Date: Thu, 7 Nov 2024 17:02:24 -0500 Subject: [PATCH] Fix issues #368, #370, #361 (#371) Fix issue #368 , issue #370 , and issue #361 . Tests have been added. One existing test(Issue103) fails, but it was already failing in the main branch. --- .../specimin/JavaParserUtil.java | 13 ++++++++ .../specimin/SpeciminRunner.java | 6 +++- .../specimin/TargetMemberFinderVisitor.java | 30 +++++++++-------- .../specimin/UnsolvedSymbolVisitor.java | 6 +++- .../specimin/BuiltinEnumTest.java | 15 +++++++++ .../specimin/DefaultPackageTest.java | 16 ++++++++++ .../specimin/Issue368Test.java | 32 +++++++++++++++++++ .../resources/defaultpackage/expected/A.java | 6 ++++ .../resources/defaultpackage/input/A.java | 5 +++ .../expected/com/example/Scheduler.java | 13 ++++++++ .../input/com/example/Scheduler.java | 14 ++++++++ src/test/resources/issue368/A_code.java | 21 ++++++++++++ .../issue368/expected/com/example/A.java | 10 ++++++ .../issue368/expected/com/example/B.java | 10 ++++++ .../issue368/input/com/example/A.java | 21 ++++++++++++ .../issue368/input/com/example/B.java | 11 +++++++ 16 files changed, 213 insertions(+), 16 deletions(-) create mode 100644 src/test/java/org/checkerframework/specimin/BuiltinEnumTest.java create mode 100644 src/test/java/org/checkerframework/specimin/DefaultPackageTest.java create mode 100644 src/test/java/org/checkerframework/specimin/Issue368Test.java create mode 100644 src/test/resources/defaultpackage/expected/A.java create mode 100644 src/test/resources/defaultpackage/input/A.java create mode 100644 src/test/resources/enumbuiltin/expected/com/example/Scheduler.java create mode 100644 src/test/resources/enumbuiltin/input/com/example/Scheduler.java create mode 100644 src/test/resources/issue368/A_code.java create mode 100644 src/test/resources/issue368/expected/com/example/A.java create mode 100644 src/test/resources/issue368/expected/com/example/B.java create mode 100644 src/test/resources/issue368/input/com/example/A.java create mode 100644 src/test/resources/issue368/input/com/example/B.java diff --git a/src/main/java/org/checkerframework/specimin/JavaParserUtil.java b/src/main/java/org/checkerframework/specimin/JavaParserUtil.java index 274db62d..df098043 100644 --- a/src/main/java/org/checkerframework/specimin/JavaParserUtil.java +++ b/src/main/java/org/checkerframework/specimin/JavaParserUtil.java @@ -16,6 +16,7 @@ import com.github.javaparser.ast.type.ClassOrInterfaceType; import com.github.javaparser.ast.type.Type; import com.github.javaparser.resolution.UnsolvedSymbolException; +import com.github.javaparser.resolution.declarations.ResolvedMethodLikeDeclaration; import com.github.javaparser.resolution.types.ResolvedReferenceType; import com.github.javaparser.resolution.types.ResolvedType; import com.google.common.base.Splitter; @@ -271,6 +272,18 @@ public static List getReferenceTypesFromCommaSeparatedString(String comm return types; } + /** + * Returns a package prefix that can be prepended to a class name, for a given method or + * constructor declaration. + * + * @param decl declaration to extract a package prefix from(using getPackageName) + * @return empty string if default package, otherwise package name followed by "." + */ + public static String packagePrefix(ResolvedMethodLikeDeclaration decl) { + String packageName = decl.getPackageName(); + return packageName.isEmpty() ? "" : packageName + "."; + } + /** * Returns true iff the innermost enclosing class/interface is an enum. * diff --git a/src/main/java/org/checkerframework/specimin/SpeciminRunner.java b/src/main/java/org/checkerframework/specimin/SpeciminRunner.java index f153ccd9..df3072a9 100644 --- a/src/main/java/org/checkerframework/specimin/SpeciminRunner.java +++ b/src/main/java/org/checkerframework/specimin/SpeciminRunner.java @@ -1,6 +1,7 @@ package org.checkerframework.specimin; import com.github.javaparser.ParseProblemException; +import com.github.javaparser.ParseResult; import com.github.javaparser.ParserConfiguration; import com.github.javaparser.StaticJavaParser; import com.github.javaparser.ast.CompilationUnit; @@ -236,7 +237,10 @@ private static void performMinimizationImpl( Map nonPrimaryClassesToPrimaryClass = new HashMap<>(); SourceRoot sourceRoot = new SourceRoot(Path.of(root)); sourceRoot.tryToParse(); - for (CompilationUnit compilationUnit : sourceRoot.getCompilationUnits()) { + // getCompilationUnits does not seem to include all files, causing some to be deleted + for (ParseResult res : sourceRoot.getCache()) { + CompilationUnit compilationUnit = + res.getResult().orElseThrow(() -> new RuntimeException(res.getProblems().toString())); Path pathOfCurrentJavaFile = compilationUnit.getStorage().get().getPath().toAbsolutePath().normalize(); String primaryTypeQualifiedName = ""; diff --git a/src/main/java/org/checkerframework/specimin/TargetMemberFinderVisitor.java b/src/main/java/org/checkerframework/specimin/TargetMemberFinderVisitor.java index cfe89c99..68ecf21a 100644 --- a/src/main/java/org/checkerframework/specimin/TargetMemberFinderVisitor.java +++ b/src/main/java/org/checkerframework/specimin/TargetMemberFinderVisitor.java @@ -243,7 +243,7 @@ public Visitable visit(ConstructorDeclaration method, Void p) { targetMethods.add(resolvedMethod.getQualifiedSignature()); unfoundMethods.remove(methodName); updateUsedClassWithQualifiedClassName( - resolvedMethod.getPackageName() + "." + resolvedMethod.getClassName(), + JavaParserUtil.packagePrefix(resolvedMethod) + resolvedMethod.getClassName(), usedTypeElements, nonPrimaryClassesToPrimaryClass); if (modularityModel.preserveAllFieldsIfTargetIsConstructor()) { @@ -341,11 +341,11 @@ public Visitable visit(MethodDeclaration method, Void p) { if (parentNode instanceof ObjectCreationExpr) { ObjectCreationExpr parentExpression = (ObjectCreationExpr) parentNode; ResolvedConstructorDeclaration resolved = parentExpression.resolve(); - String methodPackage = resolved.getPackageName(); + String methodPackagePrefix = JavaParserUtil.packagePrefix(resolved); String methodClass = resolved.getClassName(); - usedMembers.add(methodPackage + "." + methodClass + "." + method.getNameAsString() + "()"); + usedMembers.add(methodPackagePrefix + methodClass + "." + method.getNameAsString() + "()"); updateUsedClassWithQualifiedClassName( - methodPackage + "." + methodClass, usedTypeElements, nonPrimaryClassesToPrimaryClass); + methodPackagePrefix + methodClass, usedTypeElements, nonPrimaryClassesToPrimaryClass); } } String methodWithoutAnySpace = methodName.replaceAll("\\s", ""); @@ -353,7 +353,7 @@ public Visitable visit(MethodDeclaration method, Void p) { ResolvedMethodDeclaration resolvedMethod = method.resolve(); updateUsedClassesForInterface(resolvedMethod); updateUsedClassWithQualifiedClassName( - resolvedMethod.getPackageName() + "." + resolvedMethod.getClassName(), + JavaParserUtil.packagePrefix(resolvedMethod) + resolvedMethod.getClassName(), usedTypeElements, nonPrimaryClassesToPrimaryClass); @@ -518,9 +518,11 @@ public Visitable visit(MethodCallExpr call, Void p) { resolvedYetStuckMethodCall.add(scopeAsString + "." + call.getNameAsString()); usedTypeElements.add(scopeAsString); } else { + String packagePrefix = + getCurrentPackage().isEmpty() ? "" : getCurrentPackage() + "."; resolvedYetStuckMethodCall.add( - getCurrentPackage() + "." + scopeAsString + "." + call.getNameAsString()); - usedTypeElements.add(getCurrentPackage() + "." + scopeAsString); + packagePrefix + scopeAsString + "." + call.getNameAsString()); + usedTypeElements.add(packagePrefix + scopeAsString); } } } @@ -564,7 +566,7 @@ public Visitable visit(MethodCallExpr call, Void p) { private void preserveMethodDecl(ResolvedMethodDeclaration decl) { usedMembers.add(decl.getQualifiedSignature()); updateUsedClassWithQualifiedClassName( - decl.getPackageName() + "." + decl.getClassName(), + JavaParserUtil.packagePrefix(decl) + decl.getClassName(), usedTypeElements, nonPrimaryClassesToPrimaryClass); try { @@ -624,7 +626,7 @@ public Visitable visit(ObjectCreationExpr newExpr, Void p) { ResolvedConstructorDeclaration resolved = newExpr.resolve(); usedMembers.add(resolved.getQualifiedSignature()); updateUsedClassWithQualifiedClassName( - resolved.getPackageName() + "." + resolved.getClassName(), + JavaParserUtil.packagePrefix(resolved) + resolved.getClassName(), usedTypeElements, nonPrimaryClassesToPrimaryClass); for (int i = 0; i < resolved.getNumberOfParams(); ++i) { @@ -646,7 +648,7 @@ public Visitable visit(ExplicitConstructorInvocationStmt expr, Void p) { ResolvedConstructorDeclaration resolved = expr.resolve(); usedMembers.add(resolved.getQualifiedSignature()); updateUsedClassWithQualifiedClassName( - resolved.getPackageName() + "." + resolved.getClassName(), + JavaParserUtil.packagePrefix(resolved) + resolved.getClassName(), usedTypeElements, nonPrimaryClassesToPrimaryClass); } @@ -850,10 +852,6 @@ public static void updateUsedClassWithQualifiedClassName( Set usedTypeElement, Map nonPrimaryClassesToPrimaryClass) { - // in case of type variables - if (!qualifiedClassName.contains(".")) { - return; - } // strip type variables, if they're present if (qualifiedClassName.contains("<")) { qualifiedClassName = qualifiedClassName.substring(0, qualifiedClassName.indexOf("<")); @@ -867,6 +865,10 @@ public static void updateUsedClassWithQualifiedClassName( nonPrimaryClassesToPrimaryClass); } + // in case of type variables TODO:investigate side effects of having moved this from earlier + if (!qualifiedClassName.contains(".")) { + return; + } String potentialOuterClass = qualifiedClassName.substring(0, qualifiedClassName.lastIndexOf(".")); if (JavaParserUtil.isAClassPath(potentialOuterClass)) { diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index a16b89bc..6b974f20 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -1588,6 +1588,9 @@ public boolean updatedAddedTargetFilesForPotentialEnum(FieldAccessExpr expr) { return false; } if (resolved.isEnumConstant()) { + if (JavaLangUtils.inJdkPackage(resolved.getType().describe())) { + return false; + } String filePathName = qualifiedNameToFilePath(resolved.getType().describe()); if (!addedTargetFiles.contains(filePathName)) { gotException(); @@ -3257,7 +3260,8 @@ public static UnsolvedClassOrInterface getSimpleSyntheticClassFromFullyQualified + fullyName); } String className = fullyQualifiedToSimple(fullyName); - String packageName = fullyName.replace("." + className, ""); + String packageName = + fullyName.contains("." + className) ? fullyName.replace("." + className, "") : ""; return new UnsolvedClassOrInterface(className, packageName); } diff --git a/src/test/java/org/checkerframework/specimin/BuiltinEnumTest.java b/src/test/java/org/checkerframework/specimin/BuiltinEnumTest.java new file mode 100644 index 00000000..4139787a --- /dev/null +++ b/src/test/java/org/checkerframework/specimin/BuiltinEnumTest.java @@ -0,0 +1,15 @@ +package org.checkerframework.specimin; + +import java.io.IOException; +import org.junit.Test; + +/** This test checks if Specimin is able to process Java builtin enum classes. Issue #361 */ +public class BuiltinEnumTest { + @Test + public void runTest() throws IOException { + SpeciminTestExecutor.runTestWithoutJarPaths( + "enumbuiltin", + new String[] {"com/example/Scheduler.java"}, + new String[] {"com.example.Scheduler#schedule(Runnable, int)"}); + } +} diff --git a/src/test/java/org/checkerframework/specimin/DefaultPackageTest.java b/src/test/java/org/checkerframework/specimin/DefaultPackageTest.java new file mode 100644 index 00000000..ce51414c --- /dev/null +++ b/src/test/java/org/checkerframework/specimin/DefaultPackageTest.java @@ -0,0 +1,16 @@ +package org.checkerframework.specimin; + +import java.io.IOException; +import org.junit.Test; + +/** + * This test checks if Specimin correctly preserves things that are used as arguments to enum + * constants. + */ +public class DefaultPackageTest { + @Test + public void runTest() throws IOException { + SpeciminTestExecutor.runTestWithoutJarPaths( + "defaultpackage", new String[] {"A.java"}, new String[] {"A#schedule(Runnable, int)"}); + } +} diff --git a/src/test/java/org/checkerframework/specimin/Issue368Test.java b/src/test/java/org/checkerframework/specimin/Issue368Test.java new file mode 100644 index 00000000..6139040b --- /dev/null +++ b/src/test/java/org/checkerframework/specimin/Issue368Test.java @@ -0,0 +1,32 @@ +package org.checkerframework.specimin; + +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import org.junit.Test; + +/** + * This test is the test described in .... The issue caused the original + * source file A.java to be deleted, and in addition not appear in the output. + */ +public class Issue368Test { + @Test + public void runTest() throws IOException { + Path fileASource = Path.of("src/test/resources/issue368/A_code.java"); + Path fileAPath = Path.of("src/test/resources/issue368/input/com/example/A.java"); + Files.copy(fileASource, fileAPath, StandardCopyOption.REPLACE_EXISTING); + String fileACode = Files.readString(fileAPath); + SpeciminTestExecutor.runTestWithoutJarPaths( + "issue368", new String[] {"com/example/B.java"}, new String[] {"com.example.B#test()"}); + + try { + assertTrue(Files.exists(fileAPath)); + } finally { + Files.writeString(fileAPath, fileACode); + } + } +} diff --git a/src/test/resources/defaultpackage/expected/A.java b/src/test/resources/defaultpackage/expected/A.java new file mode 100644 index 00000000..3abc8cb8 --- /dev/null +++ b/src/test/resources/defaultpackage/expected/A.java @@ -0,0 +1,6 @@ +public class A { + + public static void schedule(Runnable task, int millis) { + System.out.println(); + } +} \ No newline at end of file diff --git a/src/test/resources/defaultpackage/input/A.java b/src/test/resources/defaultpackage/input/A.java new file mode 100644 index 00000000..c4d794b0 --- /dev/null +++ b/src/test/resources/defaultpackage/input/A.java @@ -0,0 +1,5 @@ +public class A{//no package declaration + public static void schedule(Runnable task, int millis) { + System.out.println(); + } +} \ No newline at end of file diff --git a/src/test/resources/enumbuiltin/expected/com/example/Scheduler.java b/src/test/resources/enumbuiltin/expected/com/example/Scheduler.java new file mode 100644 index 00000000..2970f8fa --- /dev/null +++ b/src/test/resources/enumbuiltin/expected/com/example/Scheduler.java @@ -0,0 +1,13 @@ +package com.example; + +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +public class Scheduler { + + public static final ScheduledExecutorService ses = null; + + public static void schedule(Runnable task, int millis) { + ses.schedule(task, millis, TimeUnit.MILLISECONDS); + } +} diff --git a/src/test/resources/enumbuiltin/input/com/example/Scheduler.java b/src/test/resources/enumbuiltin/input/com/example/Scheduler.java new file mode 100644 index 00000000..a4eca7f0 --- /dev/null +++ b/src/test/resources/enumbuiltin/input/com/example/Scheduler.java @@ -0,0 +1,14 @@ +package com.example; +import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor; + +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +public class Scheduler{ + private Scheduler() {} + public static final ScheduledExecutorService ses=newSingleThreadScheduledExecutor(r->new Thread(r,"Scheduler thread")); + + public static void schedule(Runnable task, int millis) { + ses.schedule(task,millis,TimeUnit.MILLISECONDS); + } +} \ No newline at end of file diff --git a/src/test/resources/issue368/A_code.java b/src/test/resources/issue368/A_code.java new file mode 100644 index 00000000..cc9b1775 --- /dev/null +++ b/src/test/resources/issue368/A_code.java @@ -0,0 +1,21 @@ +package com.example; +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.channels.AsynchronousSocketChannel; + +public class A{ + public AsynchronousSocketChannel client; + public A(AsynchronousSocketChannel client){ + this.client=client; + } + public InetAddress test(){//target method + try { + if(client.getRemoteAddress() instanceof InetSocketAddress isa) + return isa.getAddress(); + } catch (IOException e) { + + } + return null; + } +} \ No newline at end of file diff --git a/src/test/resources/issue368/expected/com/example/A.java b/src/test/resources/issue368/expected/com/example/A.java new file mode 100644 index 00000000..210134df --- /dev/null +++ b/src/test/resources/issue368/expected/com/example/A.java @@ -0,0 +1,10 @@ +package com.example; + +import java.net.InetAddress; + +public class A { + + public InetAddress test() { + throw new Error(); + } +} diff --git a/src/test/resources/issue368/expected/com/example/B.java b/src/test/resources/issue368/expected/com/example/B.java new file mode 100644 index 00000000..b5036ace --- /dev/null +++ b/src/test/resources/issue368/expected/com/example/B.java @@ -0,0 +1,10 @@ +package com.example; + +import java.net.InetAddress; + +public class B extends A { + + public InetAddress test() { + return super.test(); + } +} diff --git a/src/test/resources/issue368/input/com/example/A.java b/src/test/resources/issue368/input/com/example/A.java new file mode 100644 index 00000000..cc9b1775 --- /dev/null +++ b/src/test/resources/issue368/input/com/example/A.java @@ -0,0 +1,21 @@ +package com.example; +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.channels.AsynchronousSocketChannel; + +public class A{ + public AsynchronousSocketChannel client; + public A(AsynchronousSocketChannel client){ + this.client=client; + } + public InetAddress test(){//target method + try { + if(client.getRemoteAddress() instanceof InetSocketAddress isa) + return isa.getAddress(); + } catch (IOException e) { + + } + return null; + } +} \ No newline at end of file diff --git a/src/test/resources/issue368/input/com/example/B.java b/src/test/resources/issue368/input/com/example/B.java new file mode 100644 index 00000000..66667b53 --- /dev/null +++ b/src/test/resources/issue368/input/com/example/B.java @@ -0,0 +1,11 @@ +package com.example; +import java.io.IOException; +import java.net.InetAddress; + +public class B extends A { + + @Override + public InetAddress test(){ + return super.test(); + } +} \ No newline at end of file