Skip to content

Commit

Permalink
Create basic unit tests for library model generation (#1031)
Browse files Browse the repository at this point in the history
This is in preparation for #1006. When working on that PR I realized we
really need some unit tests to more easily debug various scenarios
(right now all we have are integration tests). Plus this will make our
code coverage reports more useful. This adds some infrastructure for
unit tests and a couple of basic tests; we will add more as we go. Also
fix a couple minor issues in `LibraryModelGenerator`.
  • Loading branch information
msridhar authored Sep 6, 2024
1 parent b7756ea commit fe9476a
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 24 deletions.
2 changes: 2 additions & 0 deletions library-model/library-model-generator/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,6 @@ dependencies {
implementation deps.build.javaparser
compileOnly deps.apt.autoValueAnnot
annotationProcessor deps.apt.autoValue

testImplementation deps.test.junit4
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,45 @@
*/
public class LibraryModelGenerator {

/**
* Data class for storing the annotation information collected from the source files. This is the
* information that is stored in the astubx file.
*/
public static class LibraryModelData {
public final Map<String, MethodAnnotationsRecord> methodRecords;
public final Map<String, Set<Integer>> nullableUpperBounds;

public LibraryModelData(
Map<String, MethodAnnotationsRecord> methodRecords,
Map<String, Set<Integer>> nullableUpperBounds) {
this.methodRecords = methodRecords;
this.nullableUpperBounds = nullableUpperBounds;
}

@Override
public String toString() {
return "ModelData{"
+ "methodRecords="
+ methodRecords
+ ", nullableUpperBounds="
+ nullableUpperBounds
+ '}';
}
}

/**
* Parses all the source files within the directory using javaparser.
*
* @param inputSourceDirectory Directory containing annotated java source files.
* @param outputDirectory Directory to write the astubx file into.
* @param outputFile absolute path to the output file.
*/
public void generateAstubxForLibraryModels(String inputSourceDirectory, String outputDirectory) {
public static LibraryModelData generateAstubxForLibraryModels(
String inputSourceDirectory, String outputFile) {
Map<String, MethodAnnotationsRecord> methodRecords = new LinkedHashMap<>();
Map<String, Set<Integer>> nullableUpperBounds = new LinkedHashMap<>();
Path root = dirnameToPath(inputSourceDirectory);
AnnotationCollectorCallback ac =
new AnnotationCollectorCallback(methodRecords, nullableUpperBounds);
LibraryModelData modelData = new LibraryModelData(methodRecords, nullableUpperBounds);
AnnotationCollectorCallback ac = new AnnotationCollectorCallback(modelData);
CollectionStrategy strategy = new ParserCollectionStrategy();
// Required to include directories that contain a module-info.java, which don't parse by
// default.
Expand All @@ -90,19 +117,19 @@ public void generateAstubxForLibraryModels(String inputSourceDirectory, String o
throw new RuntimeException(e);
}
});
writeToAstubx(outputDirectory, methodRecords, nullableUpperBounds);
writeToAstubx(outputFile, modelData);
return modelData;
}

/**
* Writes the Nullability annotation information into the output directory as an astubx file.
*
* @param outputPath Output Directory.
* @param methodRecords Map containing the collected Nullability annotation information.
* @param outputPath path to output astubx file.
* @param modelData ModelData instance containing the collected annotation information.
*/
private void writeToAstubx(
String outputPath,
Map<String, MethodAnnotationsRecord> methodRecords,
Map<String, Set<Integer>> nullableUpperBounds) {
private static void writeToAstubx(String outputPath, LibraryModelData modelData) {
Map<String, MethodAnnotationsRecord> methodRecords = modelData.methodRecords;
Map<String, Set<Integer>> nullableUpperBounds = modelData.nullableUpperBounds;
if (methodRecords.isEmpty() && nullableUpperBounds.isEmpty()) {
return;
}
Expand All @@ -127,7 +154,7 @@ private void writeToAstubx(
}
}

public Path dirnameToPath(String dir) {
public static Path dirnameToPath(String dir) {
File f = new File(dir);
String absoluteDir = f.getAbsolutePath();
if (absoluteDir.endsWith("/.")) {
Expand All @@ -140,11 +167,8 @@ private static class AnnotationCollectorCallback implements SourceRoot.Callback

private final AnnotationCollectionVisitor annotationCollectionVisitor;

public AnnotationCollectorCallback(
Map<String, MethodAnnotationsRecord> methodRecords,
Map<String, Set<Integer>> nullableUpperBounds) {
this.annotationCollectionVisitor =
new AnnotationCollectionVisitor(methodRecords, nullableUpperBounds);
public AnnotationCollectorCallback(LibraryModelData modelData) {
this.annotationCollectionVisitor = new AnnotationCollectionVisitor(modelData);
}

@Override
Expand All @@ -171,11 +195,9 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter<Void
private static final String NULLABLE = "Nullable";
private static final String JSPECIFY_NULLABLE_IMPORT = "org.jspecify.annotations.Nullable";

public AnnotationCollectionVisitor(
Map<String, MethodAnnotationsRecord> methodRecords,
Map<String, Set<Integer>> nullableUpperBounds) {
this.methodRecords = methodRecords;
this.nullableUpperBounds = nullableUpperBounds;
public AnnotationCollectionVisitor(LibraryModelData modelData) {
this.methodRecords = modelData.methodRecords;
this.nullableUpperBounds = modelData.nullableUpperBounds;
}

@Override
Expand All @@ -199,7 +221,11 @@ public void visit(ClassOrInterfaceDeclaration cid, Void arg) {
logic does not currently handle cases where @NullMarked annotations appear on some nested
classes but not others. It also does not consider annotations within package-info.java or
module-info.java files.*/
parentName += "." + cid.getNameAsString();
String oldParentName = parentName;
if (!parentName.isEmpty()) {
parentName += ".";
}
parentName += cid.getNameAsString();
cid.getAnnotations()
.forEach(
a -> {
Expand All @@ -216,7 +242,7 @@ public void visit(ClassOrInterfaceDeclaration cid, Void arg) {
}
super.visit(cid, null);
// We reset the variable that constructs the parent name after visiting all the children.
parentName = parentName.substring(0, parentName.lastIndexOf("." + cid.getNameAsString()));
parentName = oldParentName;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package com.uber.nullaway.libmodel;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Set;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

public class LibraryModelGeneratorTest {

/** For input source code files */
@Rule public TemporaryFolder inputSourcesFolder = new TemporaryFolder();

/** For output astubx files */
@Rule public TemporaryFolder outputFolder = new TemporaryFolder();

private void runTest(
String sourceFileName,
String[] lines,
ImmutableMap<String, MethodAnnotationsRecord> expectedMethodRecords,
ImmutableMap<String, Set<Integer>> expectedNullableUpperBounds)
throws IOException {
// write it to a source file in inputSourcesFolder with the right file name
Files.write(
inputSourcesFolder.newFile(sourceFileName).toPath(),
String.join("\n", lines).getBytes(StandardCharsets.UTF_8));
// run the generator
String astubxOutputPath =
Paths.get(outputFolder.getRoot().getAbsolutePath(), "output.astubx").toString();
LibraryModelGenerator.LibraryModelData modelData =
LibraryModelGenerator.generateAstubxForLibraryModels(
inputSourcesFolder.getRoot().getAbsolutePath(), astubxOutputPath);
System.err.println("modelData: " + modelData);
Assert.assertTrue("astubx file was not created", Files.exists(Paths.get(astubxOutputPath)));
assertThat(modelData.methodRecords, equalTo(expectedMethodRecords));
assertThat(modelData.nullableUpperBounds, equalTo(expectedNullableUpperBounds));
}

@Test
public void nullableReturn() throws IOException {
String[] lines =
new String[] {
"import org.jspecify.annotations.NullMarked;",
"import org.jspecify.annotations.Nullable;",
"@NullMarked",
"public class AnnotationExample {",
" @Nullable",
" public String makeUpperCase(String inputString) {",
" if (inputString == null || inputString.isEmpty()) {",
" return null;",
" } else {",
" return inputString.toUpperCase();",
" }",
" }",
"}"
};
ImmutableMap<String, MethodAnnotationsRecord> expectedMethodRecords =
ImmutableMap.of(
"AnnotationExample:String makeUpperCase(String)",
MethodAnnotationsRecord.create(ImmutableSet.of("Nullable"), ImmutableMap.of()));
runTest("AnnotationExample.java", lines, expectedMethodRecords, ImmutableMap.of());
}

@Test
public void nullableUpperBound() throws IOException {
String[] lines =
new String[] {
"import org.jspecify.annotations.NullMarked;",
"import org.jspecify.annotations.Nullable;",
"@NullMarked",
"public class NullableUpperBound<T extends @Nullable Object> {",
" T nullableObject;",
" public T getNullable() {",
" return nullableObject;",
" }",
"}"
};
ImmutableMap<String, Set<Integer>> expectedNullableUpperBounds =
ImmutableMap.of("NullableUpperBound", ImmutableSet.of(0));
runTest("NullableUpperBound.java", lines, ImmutableMap.of(), expectedNullableUpperBounds);
}
}

0 comments on commit fe9476a

Please sign in to comment.