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

Preparing for upcoming release of NullAway (0.9.9) #15

Merged
merged 4 commits into from
Jul 14, 2022
Merged

Preparing for upcoming release of NullAway (0.9.9) #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7d806fc
init
nimakarimipour Jul 13, 2022
09cda4c
added preconditions
nimakarimipour Jul 13, 2022
cdaf034
Commented precondition
nimakarimipour Jul 13, 2022
fd81f74
Reformat
nimakarimipour Jul 13, 2022
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
8 changes: 5 additions & 3 deletions core/src/main/java/edu/ucr/cs/riple/core/Annotator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private void preprocess() {
System.out.println("Making the first build...");
Utility.buildProject(config, true);
Set<Fix> uninitializedFields =
Utility.readFixesFromOutputDirectory(config, Fix.factory(null)).stream()
Utility.readFixesFromOutputDirectory(config, Fix.factory(config, null)).stream()
.filter(fix -> fix.reasons.contains("FIELD_NO_INIT") && fix.isOnField())
.collect(Collectors.toSet());
FieldInitializationAnalysis analysis =
Expand All @@ -94,7 +94,8 @@ private void explore() {
while (true) {
Utility.buildProject(config);
Set<Fix> remainingFixes =
Utility.readFixesFromOutputDirectory(config, Fix.factory(fieldDeclarationAnalysis));
Utility.readFixesFromOutputDirectory(
config, Fix.factory(config, fieldDeclarationAnalysis));
if (config.useCache) {
remainingFixes =
remainingFixes.stream()
Expand All @@ -104,7 +105,8 @@ private void explore() {
ImmutableSet<Fix> fixes = ImmutableSet.copyOf(remainingFixes);
Bank<Error> errorBank = new Bank<>(config.dir.resolve("errors.tsv"), Error::new);
Bank<Fix> fixBank =
new Bank<>(config.dir.resolve("fixes.tsv"), Fix.factory(fieldDeclarationAnalysis));
new Bank<>(
config.dir.resolve("fixes.tsv"), Fix.factory(config, fieldDeclarationAnalysis));
MethodInheritanceTree tree =
new MethodInheritanceTree(config.dir.resolve(Serializer.METHOD_INFO_FILE_NAME));
RegionTracker tracker = new CompoundTracker(config.dir, tree);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Factory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@

public interface Factory<T extends Hashable> {

T build(String[] infos);
T build(String[] info);
}
12 changes: 8 additions & 4 deletions core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Fix.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
package edu.ucr.cs.riple.core.metadata.index;

import com.google.common.collect.Sets;
import edu.ucr.cs.riple.core.Config;
import edu.ucr.cs.riple.core.metadata.field.FieldDeclarationAnalysis;
import edu.ucr.cs.riple.injector.Change;
import edu.ucr.cs.riple.injector.location.Location;
Expand All @@ -50,9 +51,9 @@ public Fix(Change change, String reason, String encClass, String endMethod) {
this.reasons = reason != null ? Sets.newHashSet(reason) : new HashSet<>();
}

public static Factory<Fix> factory(FieldDeclarationAnalysis analysis) {
return infos -> {
Location location = Location.createLocationFromArrayInfo(infos);
public static Factory<Fix> factory(Config config, FieldDeclarationAnalysis analysis) {
return info -> {
Location location = Location.createLocationFromArrayInfo(info);
if (analysis != null) {
location.ifField(
field -> {
Expand All @@ -61,7 +62,10 @@ public static Factory<Fix> factory(FieldDeclarationAnalysis analysis) {
field.variables.addAll(variables);
});
}
return new Fix(new Change(location, infos[7], true), infos[6], infos[8], infos[9]);
// TODO: Uncomment preconditions below once NullAway 0.9.9 is released.
// Preconditions.checkArgument(info[7].equals("nullable"), "unsupported annotation: " +
// info[7]);
return new Fix(new Change(location, config.nullableAnnot, true), info[6], info[8], info[9]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure, all fixes will have "nullable" on their 7th TSV position, right? There aren't currently any "nonnull" fixes or any other kind of fix entries? Maybe worth a Preconditions.checkArgument(info[7].equals("nullable")) before this line, just to guard against that ever changing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly, sure sounds good. This will remind us to change this part if we ever work on new annotations.

09cda4c

Copy link
Member Author

@nimakarimipour nimakarimipour Jul 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, the tests on CI are going to fail since it is using the maven central version of NullAway, and the precondition above is causing the termination with error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lazaroclapp For unknown reason, github does nothing when I re-request a review from you, please find the changes applied to your comment.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, the tests on CI are going to fail since it is using the maven central version of NullAway, and the precondition above is causing the termination with error.

Um, I imagine you want to make other changes to the auto-annotator, so probably we don't want to delay landing this. If we had more than one user for this project, the right call would probably to version our TSV formats and keep support for both versions for a while, but I guess that's overkill here. At the same time, I don't think the preconditions check is worth dealing with a broken CI...

I think my suggestion here would be to comment out the preconditions check with a TODO+Issue to re-enable it once NullAway 0.9.9 has been released. It's a bit hacky, but preferable to all the alternatives above. Apologies for the bikeshedding :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure commented the precondition (cdaf034 ) and added issue :D

};
}

Expand Down
84 changes: 0 additions & 84 deletions core/src/main/java/edu/ucr/cs/riple/core/util/AnnotationConfig.java
View file
Open in desktop

This file was deleted.

42 changes: 1 addition & 41 deletions core/src/main/java/edu/ucr/cs/riple/core/util/FixSerializationConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,41 +38,26 @@ public class FixSerializationConfig {

public final boolean fieldInitInfoEnabled;

public final boolean methodParamProtectionTestEnabled;

public final int paramTestIndex;

/** The directory where all files generated/read by Fix Serialization package resides. */
@Nullable public final String outputDirectory;

public final AnnotationConfig annotationConfig;

/** Default Constructor, all features are disabled with this config. */
public FixSerializationConfig() {
suggestEnabled = false;
suggestEnclosing = false;
fieldInitInfoEnabled = false;
methodParamProtectionTestEnabled = false;
paramTestIndex = Integer.MAX_VALUE;
annotationConfig = new AnnotationConfig();
outputDirectory = null;
}

public FixSerializationConfig(
boolean suggestEnabled,
boolean suggestEnclosing,
boolean fieldInitInfoEnabled,
boolean methodParamProtectionTestEnabled,
int paramTestIndex,
AnnotationConfig annotationConfig,
String outputDirectory) {
this.suggestEnabled = suggestEnabled;
this.suggestEnclosing = suggestEnclosing;
this.fieldInitInfoEnabled = fieldInitInfoEnabled;
this.methodParamProtectionTestEnabled = methodParamProtectionTestEnabled;
this.paramTestIndex = paramTestIndex;
this.outputDirectory = outputDirectory;
this.annotationConfig = annotationConfig;
}

/** Builder class for Serialization Config */
Expand All @@ -81,18 +66,12 @@ public static class Builder {
private boolean suggestEnabled;
private boolean suggestEnclosing;
private boolean fieldInitInfo;
private boolean methodParamProtectionTestEnabled;
private int paramIndex;
private String nullable;
private String nonnull;
@Nullable private String outputDir;

public Builder() {
suggestEnabled = false;
suggestEnclosing = false;
fieldInitInfo = false;
nullable = "javax.annotation.Nullable";
nonnull = "javax.annotation.Nonnull";
}

public Builder setSuggest(boolean value, boolean withEnclosing) {
Expand All @@ -101,12 +80,6 @@ public Builder setSuggest(boolean value, boolean withEnclosing) {
return this;
}

public Builder setAnnotations(String nullable, String nonnull) {
this.nullable = nullable;
this.nonnull = nonnull;
return this;
}

public Builder setFieldInitInfo(boolean enabled) {
this.fieldInitInfo = enabled;
return this;
Expand All @@ -117,12 +90,6 @@ public Builder setOutputDirectory(String outputDir) {
return this;
}

public Builder setParamProtectionTest(boolean value, int index) {
this.methodParamProtectionTestEnabled = value;
this.paramIndex = index;
return this;
}

/**
* Builds and writes the config with the state in builder at the given path as XML.
*
Expand All @@ -137,14 +104,7 @@ public FixSerializationConfig build() {
if (outputDir == null) {
throw new IllegalStateException("did not set mandatory output directory");
}
return new FixSerializationConfig(
suggestEnabled,
suggestEnclosing,
fieldInitInfo,
methodParamProtectionTestEnabled,
paramIndex,
new AnnotationConfig(nullable, nonnull),
outputDir);
return new FixSerializationConfig(suggestEnabled, suggestEnclosing, fieldInitInfo, outputDir);
}
}
}
19 changes: 0 additions & 19 deletions core/src/main/java/edu/ucr/cs/riple/core/util/Utility.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
/** Utility class. */
public class Utility {

@SuppressWarnings("StatementWithEmptyBody")
public static void executeCommand(String command, Config config) {
try {
Process p = Runtime.getRuntime().exec(new String[] {"/bin/sh", "-c", command});
Expand Down Expand Up @@ -198,23 +197,6 @@ public static void writeNullAwayConfigInXMLFormat(FixSerializationConfig config,
fieldInitInfoEnabled.setAttribute("active", String.valueOf(config.fieldInitInfoEnabled));
rootElement.appendChild(fieldInitInfoEnabled);

// Method Parameter Protection Test
Element paramTestElement = doc.createElement("paramTest");
paramTestElement.setAttribute(
"active", String.valueOf(config.methodParamProtectionTestEnabled));
paramTestElement.setAttribute("index", String.valueOf(config.paramTestIndex));
rootElement.appendChild(paramTestElement);

// Annotations
Element annots = doc.createElement("annotation");
Element nonnull = doc.createElement("nonnull");
nonnull.setTextContent(config.annotationConfig.getNonNull().getFullName());
Element nullable = doc.createElement("nullable");
nullable.setTextContent(config.annotationConfig.getNullable().getFullName());
annots.appendChild(nullable);
annots.appendChild(nonnull);
rootElement.appendChild(annots);

// Output dir
Element outputDir = doc.createElement("path");
outputDir.setTextContent(config.outputDirectory);
Expand Down Expand Up @@ -288,7 +270,6 @@ public static void buildProject(Config config, boolean initSerializationEnabled)
FixSerializationConfig.Builder nullAwayConfig =
new FixSerializationConfig.Builder()
.setSuggest(true, true)
.setAnnotations(config.nullableAnnot, "UNKNOWN")
.setOutputDirectory(config.dir.toString())
.setFieldInitInfo(initSerializationEnabled);
nullAwayConfig.writeAsXML(config.nullAwayConfigPath.toString());
Expand Down