diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 16e09cd4..24d229f3 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -24,4 +24,4 @@ jobs: - name: Setup Gradle uses: gradle/gradle-build-action@v2 - name: Run build with Gradle Wrapper - run: ./gradlew build + run: ./gradlew build expectedTestOutputsMustCompile diff --git a/build.gradle b/build.gradle index 3a8693b2..78541e18 100644 --- a/build.gradle +++ b/build.gradle @@ -47,6 +47,11 @@ task requireJavadoc(type: JavaExec) { args "src/main/java" } +task expectedTestOutputsMustCompile(type: Exec) { + // TODO: should this task run in CI, or as part of the regular tests? + commandLine "sh", "typecheck_test_outputs.sh" +} + checkerFramework { checkers = [ 'org.checkerframework.checker.nullness.NullnessChecker', diff --git a/src/main/java/org/checkerframework/specimin/GetTypesFullNameVisitor.java b/src/main/java/org/checkerframework/specimin/GetTypesFullNameVisitor.java new file mode 100644 index 00000000..19e083e4 --- /dev/null +++ b/src/main/java/org/checkerframework/specimin/GetTypesFullNameVisitor.java @@ -0,0 +1,64 @@ +package org.checkerframework.specimin; + +import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; +import com.github.javaparser.ast.type.ClassOrInterfaceType; +import com.github.javaparser.ast.visitor.ModifierVisitor; +import com.github.javaparser.ast.visitor.Visitable; +import com.github.javaparser.resolution.UnsolvedSymbolException; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +/** + * A visitor that traverses a Java file's AST and creates a map, associating the file name with the + * set of types used inside it. + */ +public class GetTypesFullNameVisitor extends ModifierVisitor { + + /** The directory path of the Java file. */ + private String fileDirectory = ""; + + /** + * A map that associates the file directory with the set of fully qualified names of types used + * within that file. + */ + private Map> fileAndAssociatedTypes = new HashMap<>(); + + /** + * Get the map of files' directories and types used within those files. + * + * @return the value of fileAndAssociatedTypes + */ + public Map> getFileAndAssociatedTypes() { + return Collections.unmodifiableMap(fileAndAssociatedTypes); + } + + @Override + public Visitable visit(ClassOrInterfaceDeclaration decl, Void p) { + // Nested type classes don't have a separate class file. + if (!decl.isNestedType()) { + fileDirectory = decl.getFullyQualifiedName().get().replace(".", "/") + ".java"; + fileAndAssociatedTypes.put(fileDirectory, new HashSet<>()); + } + return super.visit(decl, p); + } + + @Override + public Visitable visit(ClassOrInterfaceType type, Void p) { + String typeFullName; + try { + typeFullName = type.resolve().getQualifiedName(); + } catch (UnsolvedSymbolException e) { + return super.visit(type, p); + } + if (fileAndAssociatedTypes.containsKey(fileDirectory)) { + fileAndAssociatedTypes.get(fileDirectory).add(typeFullName); + return super.visit(type, p); + } else { + throw new RuntimeException( + "Unexpected files and types: " + fileDirectory + ", " + typeFullName); + } + } +} diff --git a/src/main/java/org/checkerframework/specimin/JavaTypeCorrect.java b/src/main/java/org/checkerframework/specimin/JavaTypeCorrect.java index 5d7f84f0..86845b21 100644 --- a/src/main/java/org/checkerframework/specimin/JavaTypeCorrect.java +++ b/src/main/java/org/checkerframework/specimin/JavaTypeCorrect.java @@ -9,8 +9,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.checkerframework.checker.signature.qual.ClassGetSimpleName; -import org.checkerframework.checker.signature.qual.DotSeparatedIdentifiers; /** * This class uses javac to analyze files. If there are any incompatible type errors in those files, @@ -34,7 +32,13 @@ class JavaTypeCorrect { * This map is for type correcting. The key is the name of the current incorrect type, and the * value is the name of the desired correct type. */ - private Map<@ClassGetSimpleName String, @ClassGetSimpleName String> typeToChange; + private Map typeToChange; + + /** + * A map that associates the file directory with the set of fully qualified names of types used + * within that file. + */ + private Map> fileAndAssociatedTypes = new HashMap<>(); /** * Create a new JavaTypeCorrect instance. The directories of files in fileNameList are relative to @@ -43,10 +47,14 @@ class JavaTypeCorrect { * @param rootDirectory the root directory of the files to correct types * @param fileNameList the list of the relative directory of the files to correct types */ - public JavaTypeCorrect(String rootDirectory, Set fileNameList) { + public JavaTypeCorrect( + String rootDirectory, + Set fileNameList, + Map> fileAndAssociatedTypes) { this.fileNameList = fileNameList; this.sourcePath = new File(rootDirectory).getAbsolutePath(); this.typeToChange = new HashMap<>(); + this.fileAndAssociatedTypes = fileAndAssociatedTypes; } /** @@ -54,7 +62,7 @@ public JavaTypeCorrect(String rootDirectory, Set fileNameList) { * * @return the value of typeToChange */ - public Map<@ClassGetSimpleName String, @ClassGetSimpleName String> getTypeToChange() { + public Map getTypeToChange() { return typeToChange; } @@ -87,7 +95,7 @@ public void runJavacAndUpdateTypes(String filePath) { String line; while ((line = reader.readLine()) != null) { if (line.contains("error: incompatible types")) { - updateTypeToChange(line); + updateTypeToChange(line, filePath); } } } catch (Exception e) { @@ -99,8 +107,9 @@ public void runJavacAndUpdateTypes(String filePath) { * This method updates typeToChange by relying on the error messages from javac * * @param errorMessage the error message to be analyzed + * @param filePath the path of the file where this error happens */ - private void updateTypeToChange(String errorMessage) { + private void updateTypeToChange(String errorMessage, String filePath) { List splitErrorMessage = Splitter.onPattern("\\s+").splitToList(errorMessage); if (splitErrorMessage.size() < 7) { throw new RuntimeException("Unexpected type error messages: " + errorMessage); @@ -110,38 +119,38 @@ private void updateTypeToChange(String errorMessage) { * 2. error: incompatible types: found required */ if (errorMessage.contains("cannot be converted to")) { - // since this is from javac, we know that these will be dot-separated identifiers. - @SuppressWarnings("signature") - @DotSeparatedIdentifiers String incorrectType = splitErrorMessage.get(4); - @SuppressWarnings("signature") - @DotSeparatedIdentifiers String correctType = splitErrorMessage.get(splitErrorMessage.size() - 1); - typeToChange.put(toSimpleName(incorrectType), toSimpleName(correctType)); + String incorrectType = splitErrorMessage.get(4); + String correctType = splitErrorMessage.get(splitErrorMessage.size() - 1); + typeToChange.put(incorrectType, tryResolveFullyQualifiedType(correctType, filePath)); } else { - @SuppressWarnings("signature") - @DotSeparatedIdentifiers String incorrectType = splitErrorMessage.get(5); - @SuppressWarnings("signature") - @DotSeparatedIdentifiers String correctType = splitErrorMessage.get(splitErrorMessage.size() - 1); - typeToChange.put(toSimpleName(incorrectType), toSimpleName(correctType)); + String incorrectType = splitErrorMessage.get(5); + String correctType = splitErrorMessage.get(splitErrorMessage.size() - 1); + typeToChange.put(incorrectType, tryResolveFullyQualifiedType(correctType, filePath)); } } /** - * This method takes the name of a class and converts it to the @ClassGetSimpleName type according - * to Checker Framework. If the name is already in the @ClassGetSimpleName form, this method will - * not make any changes + * This method tries to get the fully-qualified name of a type based on the simple name of that + * type and the class file where that type is used. * - * @param className the name of the class to be converted - * @return the simple name of the class + * @param type the type to be taken as input + * @param filePath the path of the file where type is used + * @return the fully-qualified name of that type if any. Otherwise, return the original expression + * of type. */ - // the code is self-explanatory, essentially the last element of a class name is the simple name - // of that class. This method takes the input from the error message of javac, so we know that - // className will be a dot-separated identifier. - @SuppressWarnings("signature") - public static @ClassGetSimpleName String toSimpleName(@DotSeparatedIdentifiers String className) { - List classNameParts = Splitter.onPattern("[.]").splitToList(className); - if (classNameParts.size() < 2) { - return className; + public String tryResolveFullyQualifiedType(String type, String filePath) { + // type is already in the fully qualifed format + if (Splitter.onPattern("\\.").splitToList(type).size() > 1) { + return type; + } + if (fileAndAssociatedTypes.containsKey(filePath)) { + Set fullyQualifiedType = fileAndAssociatedTypes.get(filePath); + for (String typeFullName : fullyQualifiedType) { + if (typeFullName.substring(typeFullName.lastIndexOf(".") + 1).equals(type)) { + return typeFullName; + } + } } - return classNameParts.get(classNameParts.size() - 1); + return type; } } diff --git a/src/main/java/org/checkerframework/specimin/MethodPrunerVisitor.java b/src/main/java/org/checkerframework/specimin/MethodPrunerVisitor.java index 1345c020..ba847660 100644 --- a/src/main/java/org/checkerframework/specimin/MethodPrunerVisitor.java +++ b/src/main/java/org/checkerframework/specimin/MethodPrunerVisitor.java @@ -1,6 +1,8 @@ package org.checkerframework.specimin; import com.github.javaparser.StaticJavaParser; +import com.github.javaparser.ast.ImportDeclaration; +import com.github.javaparser.ast.Node; import com.github.javaparser.ast.body.ConstructorDeclaration; import com.github.javaparser.ast.body.MethodDeclaration; import com.github.javaparser.ast.visitor.ModifierVisitor; @@ -28,6 +30,13 @@ public class MethodPrunerVisitor extends ModifierVisitor { */ private Set methodsToEmpty; + /** + * This is the set of classes used by the target methods. We use this set to determine if we + * should keep or delete an import statement. The strings representing the classes are in + * the @FullyQualifiedName form. + */ + private Set classesUsedByTargetMethods; + /** * This boolean tracks whether the element currently being visited is inside a target method. It * is set by {@link #visit(MethodDeclaration, Void)}. @@ -42,10 +51,25 @@ public class MethodPrunerVisitor extends ModifierVisitor { * @param methodsToKeep the set of methods whose bodies should be kept intact (usually the target * methods for specimin) * @param methodsToEmpty the set of methods whose bodies should be removed + * @param classesUsedByTargetMethods the classes used by target methods */ - public MethodPrunerVisitor(Set methodsToKeep, Set methodsToEmpty) { + public MethodPrunerVisitor( + Set methodsToKeep, + Set methodsToEmpty, + Set classesUsedByTargetMethods) { this.methodsToLeaveUnchanged = methodsToKeep; this.methodsToEmpty = methodsToEmpty; + this.classesUsedByTargetMethods = classesUsedByTargetMethods; + } + + @Override + public Node visit(ImportDeclaration decl, Void p) { + String classFullName = decl.getNameAsString(); + if (classesUsedByTargetMethods.contains(classFullName)) { + return super.visit(decl, p); + } + decl.remove(); + return decl; } @Override diff --git a/src/main/java/org/checkerframework/specimin/SpeciminRunner.java b/src/main/java/org/checkerframework/specimin/SpeciminRunner.java index ebb1f9d2..49941560 100644 --- a/src/main/java/org/checkerframework/specimin/SpeciminRunner.java +++ b/src/main/java/org/checkerframework/specimin/SpeciminRunner.java @@ -164,8 +164,15 @@ public static void performMinimization( relatedClass.add(directoryOfFile); } } + GetTypesFullNameVisitor getTypesFullNameVisitor = new GetTypesFullNameVisitor(); + for (CompilationUnit cu : parsedTargetFiles.values()) { + cu.accept(getTypesFullNameVisitor, null); + } + Map> filesAndAssociatedTypes = + getTypesFullNameVisitor.getFileAndAssociatedTypes(); // correct the types of all related files before adding them to parsedTargetFiles - JavaTypeCorrect typeCorrecter = new JavaTypeCorrect(root, relatedClass); + JavaTypeCorrect typeCorrecter = + new JavaTypeCorrect(root, relatedClass, filesAndAssociatedTypes); typeCorrecter.correctTypesForAllFiles(); Map<@ClassGetSimpleName String, @ClassGetSimpleName String> typesToChange = typeCorrecter.getTypeToChange(); @@ -180,7 +187,8 @@ public static void performMinimization( } MethodPrunerVisitor methodPruner = - new MethodPrunerVisitor(finder.getTargetMethods(), finder.getUsedMembers()); + new MethodPrunerVisitor( + finder.getTargetMethods(), finder.getUsedMembers(), finder.getUsedClass()); for (CompilationUnit cu : parsedTargetFiles.values()) { cu.accept(methodPruner, null); diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedClass.java b/src/main/java/org/checkerframework/specimin/UnsolvedClass.java index a08f1ad2..e29b63a1 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedClass.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedClass.java @@ -3,6 +3,7 @@ import com.google.common.base.Splitter; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import org.checkerframework.checker.signature.qual.ClassGetSimpleName; @@ -12,14 +13,20 @@ * The reason is that the class file is not in the root directory. */ public class UnsolvedClass { - /** Set of methods belongs to the class */ - private final Set methods; + /** + * Set of methods belongs to the class. Must be a linked set to ensure deterministic iteration + * order when writing files synthetic classes. + */ + private final LinkedHashSet methods; /** The name of the class */ private final @ClassGetSimpleName String className; - /** The fields of this class */ - private final Set classFields; + /** + * The fields of this class. Must be a linked set to ensure deterministic iteration order when + * writing files for synthetic classes. + */ + private final LinkedHashSet classFields; /** * The name of the package of the class. We rely on the import statements from the source codes to @@ -38,9 +45,9 @@ public class UnsolvedClass { */ public UnsolvedClass(@ClassGetSimpleName String className, String packageName) { this.className = className; - this.methods = new HashSet<>(); + this.methods = new LinkedHashSet<>(); this.packageName = packageName; - this.classFields = new HashSet<>(); + this.classFields = new LinkedHashSet<>(); } /** @@ -119,8 +126,7 @@ public int getNumberOfTypeVariables() { * @param currentReturnType the current return type of this method * @param desiredReturnType the new return type */ - public void updateMethodByReturnType( - @ClassGetSimpleName String currentReturnType, @ClassGetSimpleName String desiredReturnType) { + public void updateMethodByReturnType(String currentReturnType, String desiredReturnType) { for (UnsolvedMethod method : methods) { if (method.getReturnType().equals(currentReturnType)) { method.setReturnType(desiredReturnType); diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedMethod.java b/src/main/java/org/checkerframework/specimin/UnsolvedMethod.java index 33b6211f..f8944e1e 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedMethod.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedMethod.java @@ -1,7 +1,6 @@ package org.checkerframework.specimin; import java.util.List; -import org.checkerframework.checker.signature.qual.ClassGetSimpleName; /** * An UnsolvedMethod instance is a representation of a method that can not be solved by @@ -15,7 +14,7 @@ public class UnsolvedMethod { * The return type of the method. At the moment, we set the return type the same as the class * where the method belongs to. */ - private @ClassGetSimpleName String returnType; + private String returnType; /** * The list of parameters of the method. (Right now we won't touch it until the new variant of @@ -33,8 +32,7 @@ public class UnsolvedMethod { * @param returnType the return type of the method * @param parameterList the list of parameters for this method */ - public UnsolvedMethod( - String name, @ClassGetSimpleName String returnType, List parameterList) { + public UnsolvedMethod(String name, String returnType, List parameterList) { this.name = name; this.returnType = returnType; this.parameterList = parameterList; @@ -46,7 +44,7 @@ public UnsolvedMethod( * * @param returnType the return type to bet set for this method */ - public void setReturnType(@ClassGetSimpleName String returnType) { + public void setReturnType(String returnType) { this.returnType = returnType; } @@ -55,7 +53,7 @@ public void setReturnType(@ClassGetSimpleName String returnType) { * * @return the value of returnType */ - public @ClassGetSimpleName String getReturnType() { + public String getReturnType() { return returnType; } diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index 8d8becca..a843d0fd 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -1525,8 +1525,7 @@ public void updateClassSetWithQualifiedStaticMethodCall( * * @param typeToCorrect the Map to be analyzed */ - public void updateTypes( - Map<@ClassGetSimpleName String, @ClassGetSimpleName String> typeToCorrect) { + public void updateTypes(Map typeToCorrect) { for (String incorrectType : typeToCorrect.keySet()) { // convert MethodNameReturnType to methodName String involvedMethod = diff --git a/src/test/resources/HiddenSuperFieldAndUnsolvedLocal/expected/com/example/Parent.java b/src/test/resources/HiddenSuperFieldAndUnsolvedLocal/expected/com/example/Parent.java index 1a388019..7b2b95a3 100644 --- a/src/test/resources/HiddenSuperFieldAndUnsolvedLocal/expected/com/example/Parent.java +++ b/src/test/resources/HiddenSuperFieldAndUnsolvedLocal/expected/com/example/Parent.java @@ -2,5 +2,5 @@ public class Parent { - public MyType theName = null; + public sample.pack.MyType theName = null; } diff --git a/src/test/resources/innerclass/expected/com/example/Maternal.java b/src/test/resources/innerclass/expected/com/example/Maternal.java index 62d581f4..e4c90760 100644 --- a/src/test/resources/innerclass/expected/com/example/Maternal.java +++ b/src/test/resources/innerclass/expected/com/example/Maternal.java @@ -2,5 +2,5 @@ public class Maternal { - public String maternalLastName = null; + public java.lang.String maternalLastName = null; } diff --git a/src/test/resources/innerclass/expected/com/example/Paternal.java b/src/test/resources/innerclass/expected/com/example/Paternal.java index b1ac0153..068888a8 100644 --- a/src/test/resources/innerclass/expected/com/example/Paternal.java +++ b/src/test/resources/innerclass/expected/com/example/Paternal.java @@ -2,5 +2,5 @@ public class Paternal { - public String paternalLastName = null; + public java.lang.String paternalLastName = null; } diff --git a/src/test/resources/sameclassname/expected/com/example/Simple.java b/src/test/resources/sameclassname/expected/com/example/Simple.java index b3f45034..0cfb43b6 100644 --- a/src/test/resources/sameclassname/expected/com/example/Simple.java +++ b/src/test/resources/sameclassname/expected/com/example/Simple.java @@ -1,7 +1,5 @@ package com.example; -import org.first.Calculator; - class Simple { public void secondCalculator () { org.second.Calculator thisCal = new org.second.Calculator(); diff --git a/src/test/resources/staticnestedclass/expected/com/example/Maternal.java b/src/test/resources/staticnestedclass/expected/com/example/Maternal.java index 62d581f4..e4c90760 100644 --- a/src/test/resources/staticnestedclass/expected/com/example/Maternal.java +++ b/src/test/resources/staticnestedclass/expected/com/example/Maternal.java @@ -2,5 +2,5 @@ public class Maternal { - public String maternalLastName = null; + public java.lang.String maternalLastName = null; } diff --git a/src/test/resources/staticnestedclass/expected/com/example/Paternal.java b/src/test/resources/staticnestedclass/expected/com/example/Paternal.java index b1ac0153..068888a8 100644 --- a/src/test/resources/staticnestedclass/expected/com/example/Paternal.java +++ b/src/test/resources/staticnestedclass/expected/com/example/Paternal.java @@ -2,5 +2,5 @@ public class Paternal { - public String paternalLastName = null; + public java.lang.String paternalLastName = null; } diff --git a/src/test/resources/syntheticsupervariables/expected/org/wild/Mammal.java b/src/test/resources/syntheticsupervariables/expected/org/wild/Mammal.java index fb724e63..9661e3de 100644 --- a/src/test/resources/syntheticsupervariables/expected/org/wild/Mammal.java +++ b/src/test/resources/syntheticsupervariables/expected/org/wild/Mammal.java @@ -1,5 +1,7 @@ package org.wild; public class Mammal { - public boolean canBreathUnderWater = false; + public String habitat = null; + + public boolean canBreathUnderWater = false; } diff --git a/typecheck_test_outputs.sh b/typecheck_test_outputs.sh new file mode 100755 index 00000000..ebb0ee8a --- /dev/null +++ b/typecheck_test_outputs.sh @@ -0,0 +1,28 @@ +#!/bin/sh + +# This script runs javac on all of the expected test outputs under src/test/resources. +# It returns 2 if any of them fail to compile, 1 if there are any malformed test directories, +# and 0 if all of them do compile. +# +# It is desirable that all of the expected test outputs compile, because Specimin +# should produce independently-compilable programs. + +returnval=0 + +cd src/test/resources || exit 1 +for testcase in * ; do + cd "${testcase}/expected/" || exit 1 + # javac relies on word splitting + # shellcheck disable=SC2046 + javac -proc:only -nowarn $(find . -name "*.java") \ + || { echo "Running javac on ${testcase}/expected issues one or more errors, which are printed above."; returnval=2; } + cd ../.. || exit 1 +done + +if [ "${returnval}" = 0 ]; then + echo "All expected test outputs compiled successfully." +elif [ "${returnval}" = 2 ]; then + echo "Some expected test outputs do not compile successfully. See the above error output for details." +fi + +exit ${returnval}