Remove method parameter protection analysis (#622)
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>
```
diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java
index bbdc0cc..047ac22 100644
--- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java
+++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java
@@ -59,18 +59,6 @@
*/
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;
@@ -83,8 +71,6 @@
suggestEnabled = false;
suggestEnclosing = false;
fieldInitInfoEnabled = false;
- methodParamProtectionTestEnabled = false;
- paramTestIndex = Integer.MAX_VALUE;
annotationConfig = new AnnotationConfig();
outputDirectory = null;
serializer = null;
@@ -94,15 +80,11 @@
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);
@@ -142,12 +124,6 @@
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");
@@ -169,8 +145,6 @@
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;
@@ -205,12 +179,6 @@
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.
*
@@ -229,8 +197,6 @@
suggestEnabled,
suggestEnclosing,
fieldInitInfo,
- methodParamProtectionTestEnabled,
- paramIndex,
new AnnotationConfig(nullable, nonnull),
outputDir);
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/XMLUtil.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/XMLUtil.java
index 2a66b17..fca8b23 100644
--- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/XMLUtil.java
+++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/XMLUtil.java
@@ -121,13 +121,6 @@
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");
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java
index 90c04c7..7c8c627 100644
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java
+++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java
@@ -68,10 +68,6 @@
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));
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/MethodParamNullableInjectorHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/MethodParamNullableInjectorHandler.java
deleted file mode 100644
index f259de5..0000000
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/MethodParamNullableInjectorHandler.java
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright (c) 2022 Uber Technologies, Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-package com.uber.nullaway.handlers;
-
-import com.uber.nullaway.Config;
-import com.uber.nullaway.Nullness;
-import com.uber.nullaway.dataflow.AccessPath;
-import com.uber.nullaway.dataflow.NullnessStore;
-import com.uber.nullaway.fixserialization.FixSerializationConfig;
-import java.util.List;
-import org.checkerframework.nullaway.dataflow.cfg.UnderlyingAST;
-import org.checkerframework.nullaway.dataflow.cfg.node.LocalVariableNode;
-
-/**
- * This handler transforms method parameter's state at index {@link
- * FixSerializationConfig#paramTestIndex} for all methods to {@code @Nullable}. It provides the
- * facility to measure protection of methods against nullability of each argument. This handler is
- * activated only if {@link FixSerializationConfig#methodParamProtectionTestEnabled} is enabled.
- */
-public class MethodParamNullableInjectorHandler extends BaseNoOpHandler {
-
- private final FixSerializationConfig config;
-
- public MethodParamNullableInjectorHandler(Config config) {
- this.config = config.getSerializationConfig();
- }
-
- @Override
- public NullnessStore.Builder onDataflowInitialStore(
- UnderlyingAST underlyingAST,
- List<LocalVariableNode> parameters,
- NullnessStore.Builder result) {
- int index = config.paramTestIndex;
- if (index >= parameters.size() || !(underlyingAST instanceof UnderlyingAST.CFGMethod)) {
- return super.onDataflowInitialStore(underlyingAST, parameters, result);
- }
- result.setInformation(AccessPath.fromLocal(parameters.get(index)), Nullness.NULLABLE);
- return result;
- }
-}
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java
index d9f1d03..351a683 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java
@@ -682,71 +682,6 @@
}
@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);
tester
