Measurement of Method protection against nullability of arguments. (#575

)

This PR is a followup to enable nullability inference implemented in [AutoFixer](https://github.com/nimakarimipour/NullAwayAutoFixer). This PR provides the facility to measure protection of methods against nullability of arguments. When activated, it transforms all method arguments's state at certain index to `@Nullable`.

nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java

@@ -51,6 +51,19 @@ public class FixSerializationConfig {
    * computed at request.
    */
   public final boolean suggestEnclosing;
+
+  /**
+   * 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. */
   public final String outputDirectory;
 
@@ -62,6 +75,8 @@ public class FixSerializationConfig {
   public FixSerializationConfig() {
     suggestEnabled = false;
     suggestEnclosing = false;
+    methodParamProtectionTestEnabled = false;
+    paramTestIndex = Integer.MAX_VALUE;
     annotationConfig = new AnnotationConfig();
     outputDirectory = null;
     serializer = null;
@@ -70,10 +85,14 @@ public FixSerializationConfig() {
   public FixSerializationConfig(
       boolean suggestEnabled,
       boolean suggestEnclosing,
+      boolean methodParamProtectionTestEnabled,
+      int paramTestIndex,
       AnnotationConfig annotationConfig,
       String outputDirectory) {
     this.suggestEnabled = suggestEnabled;
     this.suggestEnclosing = suggestEnclosing;
+    this.methodParamProtectionTestEnabled = methodParamProtectionTestEnabled;
+    this.paramTestIndex = paramTestIndex;
     this.outputDirectory = outputDirectory;
     this.annotationConfig = annotationConfig;
     serializer = new Serializer(this);
@@ -110,6 +129,12 @@ public FixSerializationConfig(String configFilePath) {
       throw new IllegalStateException(
           "Error in the fix serialization configuration, suggest flag must be enabled to activate enclosing method and class serialization.");
     }
+    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");
@@ -129,6 +154,8 @@ public static class Builder {
 
     private boolean suggestEnabled;
     private boolean suggestEnclosing;
+    private boolean methodParamProtectionTestEnabled;
+    private int paramIndex;
     private String nullable;
     private String nonnull;
     private String outputDir;
@@ -157,6 +184,12 @@ 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.
      *
@@ -169,7 +202,12 @@ public void writeAsXML(String path) {
 
     public FixSerializationConfig build() {
       return new FixSerializationConfig(
-          suggestEnabled, suggestEnclosing, new AnnotationConfig(nullable, nonnull), outputDir);
+          suggestEnabled,
+          suggestEnclosing,
+          methodParamProtectionTestEnabled,
+          paramIndex,
+          new AnnotationConfig(nullable, nonnull),
+          outputDir);
     }
   }
 }

nullaway/src/main/java/com/uber/nullaway/fixserialization/XMLUtil.java

@@ -57,7 +57,7 @@ public static <T> DefaultXMLValueProvider<T> getValueFromAttribute(
     try {
       XPath xPath = XPathFactory.newInstance().newXPath();
       Node node = (Node) xPath.compile(key).evaluate(doc, XPathConstants.NODE);
-      if (node.getNodeType() == Node.ELEMENT_NODE) {
+      if (node != null && node.getNodeType() == Node.ELEMENT_NODE) {
         Element eElement = (Element) node;
         return new DefaultXMLValueProvider<>(eElement.getAttribute(attr), klass);
       }
@@ -82,7 +82,7 @@ public static <T> DefaultXMLValueProvider<T> getValueFromTag(
     try {
       XPath xPath = XPathFactory.newInstance().newXPath();
       Node node = (Node) xPath.compile(key).evaluate(doc, XPathConstants.NODE);
-      if (node.getNodeType() == Node.ELEMENT_NODE) {
+      if (node != null && node.getNodeType() == Node.ELEMENT_NODE) {
         Element eElement = (Element) node;
         return new DefaultXMLValueProvider<>(eElement.getTextContent(), klass);
       }
@@ -114,6 +114,13 @@ public static void writeInXMLFormat(FixSerializationConfig config, String path)
       suggestElement.setAttribute("enclosing", String.valueOf(config.suggestEnclosing));
       rootElement.appendChild(suggestElement);
 
+      // 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");
@@ -152,8 +159,8 @@ static class DefaultXMLValueProvider<T> {
       } else {
         String content = value.toString();
         switch (klass.getSimpleName()) {
-          case "Double":
-            this.value = Double.valueOf(content);
+          case "Integer":
+            this.value = Integer.valueOf(content);
             break;
           case "Boolean":
             this.value = Boolean.valueOf(content);

nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java

@@ -63,6 +63,10 @@ public static Handler buildDefault(Config config) {
     handlerListBuilder.add(new GrpcHandler());
     handlerListBuilder.add(new RequiresNonNullHandler());
     handlerListBuilder.add(new EnsuresNonNullHandler());
+    if (config.serializationIsActive()
+        && config.getSerializationConfig().methodParamProtectionTestEnabled) {
+      handlerListBuilder.add(new MethodParamNullableInjectorHandler(config));
+    }
     if (config.checkOptionalEmptiness()) {
       handlerListBuilder.add(new OptionalEmptinessHandler(config, methodNameUtil));
     }

nullaway/src/main/java/com/uber/nullaway/handlers/MethodParamNullableInjectorHandler.java

@@ -0,0 +1,60 @@
+/*
+ * 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;
+  }
+}

nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java

@@ -576,9 +576,9 @@ public void test_custom_annot() {
                 "-XepOpt:NullAway:SerializeFixMetadata=true",
                 "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath))
         .addSourceLines(
-            "com/uber/SubClass.java",
+            "com/uber/Test.java",
             "package com.uber;",
-            "public class SubClass {",
+            "public class Test {",
             "   Object test(boolean flag) {",
             "       if(flag) {",
             "           return new Object();",
@@ -593,8 +593,71 @@ public void test_custom_annot() {
                 "test(boolean)",
                 "null",
                 "METHOD",
-                "com.uber.SubClass",
-                "com/uber/SubClass.java"))
+                "com.uber.Test",
+                "com/uber/Test.java"))
+        .doTest();
+  }
+
+  @Test
+  public void test_method_param_protection_test() {
+    Path tempRoot = Paths.get(temporaryFolder.getRoot().getAbsolutePath(), "custom_annot");
+    String output = tempRoot.toString();
+    try {
+      Files.createDirectories(tempRoot);
+      serializationTestHelper = new SerializationTestHelper(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);
+    }
+    serializationTestHelper
+        .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());",
+            "   }",
+            "}")
+        .setExpectedFixes(
+            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"))
         .doTest();
   }
 }