Skip to content

Commit

Permalink
Remove method parameter protection analysis (#622)
Browse files Browse the repository at this point in the history
This PR removes the method parameter protection feature from NullAway. This feature enables us to make `NullAway` to assume all method parameters at given index are `@Nullable` while processing the method bodies.  However, this feature is not used by the other tool ([Annotator](https://github.com/nimakarimipour/NullAwayAnnotator)) which had the first motivation toward implementing this feature.

In previous versions of [Annotator](https://github.com/nimakarimipour/NullAwayAnnotator), processing of method parameters was performed in a different manner compared to all other code locations and was using this specific feature. However, in every version upper that `1.3.0-LOCAL` this feature is not used anymore.

After this PR the following entity will be removed from the config `.xml` file:
```xml
<paramTest "active": boolean, "index": i> 
```
  • Loading branch information
nimakarimipour authored Jul 11, 2022
1 parent 2c98f2b commit 58029f8
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 170 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,6 @@ public class FixSerializationConfig {
*/
public final boolean fieldInitInfoEnabled;

/**
* If enabled, the formal parameter at index {@link FixSerializationConfig#paramTestIndex} in all
* methods will be treated as {@code @Nullable}
*/
public final boolean methodParamProtectionTestEnabled;

/**
* Index of the formal parameter of all methods which will be considered {@code @Nullable}, if
* {@link FixSerializationConfig#methodParamProtectionTestEnabled} is enabled.
*/
public final int paramTestIndex;

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

Expand All @@ -83,8 +71,6 @@ public FixSerializationConfig() {
suggestEnabled = false;
suggestEnclosing = false;
fieldInitInfoEnabled = false;
methodParamProtectionTestEnabled = false;
paramTestIndex = Integer.MAX_VALUE;
annotationConfig = new AnnotationConfig();
outputDirectory = null;
serializer = null;
Expand All @@ -94,15 +80,11 @@ 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;
serializer = new Serializer(this);
Expand Down Expand Up @@ -142,12 +124,6 @@ public FixSerializationConfig(String configFilePath) {
XMLUtil.getValueFromAttribute(
document, "/serialization/fieldInitInfo", "active", Boolean.class)
.orElse(false);
methodParamProtectionTestEnabled =
XMLUtil.getValueFromAttribute(document, "/serialization/paramTest", "active", Boolean.class)
.orElse(false);
paramTestIndex =
XMLUtil.getValueFromAttribute(document, "/serialization/paramTest", "index", Integer.class)
.orElse(Integer.MAX_VALUE);
String nullableAnnot =
XMLUtil.getValueFromTag(document, "/serialization/annotation/nullable", String.class)
.orElse("javax.annotation.Nullable");
Expand All @@ -169,8 +145,6 @@ 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;
Expand Down Expand Up @@ -205,12 +179,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 @@ -229,8 +197,6 @@ public FixSerializationConfig build() {
suggestEnabled,
suggestEnclosing,
fieldInitInfo,
methodParamProtectionTestEnabled,
paramIndex,
new AnnotationConfig(nullable, nonnull),
outputDir);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,6 @@ public static void writeInXMLFormat(FixSerializationConfig config, String path)
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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,6 @@ public static Handler buildDefault(Config config) {
handlerListBuilder.add(
new FieldInitializationSerializationHandler(config.getSerializationConfig()));
}
if (config.serializationIsActive()
&& config.getSerializationConfig().methodParamProtectionTestEnabled) {
handlerListBuilder.add(new MethodParamNullableInjectorHandler(config));
}
if (config.checkOptionalEmptiness()) {
handlerListBuilder.add(new OptionalEmptinessHandler(config, methodNameUtil));
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -681,71 +681,6 @@ public void suggestCustomAnnotTest() {
.doTest();
}

@Test
public void examineMethodParamProtectionTest() {
Path tempRoot = Paths.get(temporaryFolder.getRoot().getAbsolutePath(), "custom_annot");
String output = tempRoot.toString();
SerializationTestHelper<FixDisplay> tester = new SerializationTestHelper<>(tempRoot);
try {
Files.createDirectories(tempRoot);
FixSerializationConfig.Builder builder =
new FixSerializationConfig.Builder()
.setSuggest(true, false)
.setParamProtectionTest(true, 0)
.setOutputDirectory(output);
Path config = tempRoot.resolve("serializer.xml");
Files.createFile(config);
configPath = config.toString();
builder.writeAsXML(configPath);
} catch (IOException ex) {
throw new UncheckedIOException(ex);
}
tester
.setArgs(
Arrays.asList(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
"-XepOpt:NullAway:SerializeFixMetadata=true",
"-XepOpt:NullAway:FixSerializationConfigPath=" + configPath))
.addSourceLines(
"com/uber/Test.java",
"package com.uber;",
"public class Test {",
" Object test(Object foo) {",
" // BUG: Diagnostic contains: returning @Nullable",
" return foo;",
" }",
" Object test1(Object foo, Object bar) {",
" // BUG: Diagnostic contains: dereferenced expression foo is @Nullable",
" Integer hash = foo.hashCode();",
" return bar;",
" }",
" void test2(Object f) {",
" // BUG: Diagnostic contains: passing @Nullable",
" test1(f, new Object());",
" }",
"}")
.setExpectedOutputs(
new FixDisplay(
"javax.annotation.Nullable",
"test(java.lang.Object)",
"null",
"METHOD",
"com.uber.Test",
"com/uber/Test.java"),
new FixDisplay(
"javax.annotation.Nullable",
"test1(java.lang.Object,java.lang.Object)",
"foo",
"PARAMETER",
"com.uber.Test",
"com/uber/Test.java"))
.setFactory(fixDisplayFactory)
.setOutputFileNameAndHeader(SUGGEST_FIX_FILE_NAME, SUGGEST_FIX_FILE_HEADER)
.doTest();
}

@Test
public void errorSerializationTest() {
SerializationTestHelper<ErrorDisplay> tester = new SerializationTestHelper<>(root);
Expand Down

0 comments on commit 58029f8

Please sign in to comment.