Handle interaction between AcknowledgeRestrictiveAnnotations and TreatGeneratedAsUnannotated (#254)
If both the `TreatGeneratedAsUnannotated` and `AcknowledgeRestrictiveAnnotations` are set, previously we would still use any `@Nullable` annotations in generated code when checking clients. This PR implements the desired behavior, which is to ignore those `@Nullable` annotations.
This points to a deeper issue in our terminology (kind of weird to use annotations from "unannotated" code) and config options. But fully addressing that is outside the scope of this PR.
diff --git a/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java b/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java
index 0614e9a..2cc6ab9 100644
--- a/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java
+++ b/nullaway/src/main/java/com/uber/nullaway/AbstractConfig.java
@@ -248,4 +248,9 @@
public String getErrorURL() {
return errorURL;
}
+
+ @Override
+ public boolean treatGeneratedAsUnannotated() {
+ return treatGeneratedAsUnannotated;
+ }
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/Config.java b/nullaway/src/main/java/com/uber/nullaway/Config.java
index ced23a0..3e7210b 100644
--- a/nullaway/src/main/java/com/uber/nullaway/Config.java
+++ b/nullaway/src/main/java/com/uber/nullaway/Config.java
@@ -135,4 +135,7 @@
/** @return the URL to show with NullAway error messages */
String getErrorURL();
+
+ /** @return true if generated code should be treated as unannotated */
+ boolean treatGeneratedAsUnannotated();
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java b/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java
index ea62380..bd5f75c 100644
--- a/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java
+++ b/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java
@@ -143,4 +143,9 @@
public String getErrorURL() {
throw new IllegalStateException(error_msg);
}
+
+ @Override
+ public boolean treatGeneratedAsUnannotated() {
+ throw new IllegalStateException(error_msg);
+ }
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
index 1e6a4bb..f22eedc 100644
--- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
+++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
@@ -199,4 +199,14 @@
return !config.fromAnnotatedPackage(outermostClassSymbol)
|| config.isUnannotatedClass(outermostClassSymbol);
}
+
+ /**
+ * @param symbol symbol for entity
+ * @return true if symbol represents an entity from a class annotated with {@code @Generated};
+ * false otherwise
+ */
+ public static boolean isGenerated(Symbol symbol) {
+ Symbol.ClassSymbol outermostClassSymbol = getOutermostClassSymbol(symbol);
+ return ASTHelpers.hasDirectAnnotationWithSimpleName(outermostClassSymbol, "Generated");
+ }
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java
index f148895..d7ac355 100644
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java
+++ b/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java
@@ -70,7 +70,13 @@
if (expr.getKind().equals(Tree.Kind.METHOD_INVOCATION)) {
Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) expr);
if (NullabilityUtil.isUnannotated(methodSymbol, config)) {
- return Nullness.hasNullableAnnotation(methodSymbol) || exprMayBeNull;
+ // with the generated-as-unannotated option enabled, we want to ignore
+ // annotations in generated code
+ if (config.treatGeneratedAsUnannotated() && NullabilityUtil.isGenerated(methodSymbol)) {
+ return exprMayBeNull;
+ } else {
+ return Nullness.hasNullableAnnotation(methodSymbol) || exprMayBeNull;
+ }
} else {
return exprMayBeNull;
}
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java
index c825419..dcdab86 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java
@@ -316,6 +316,35 @@
}
@Test
+ public void generatedAsUnannotatedPlusRestrictive() {
+ compilationHelper
+ .setArgs(
+ Arrays.asList(
+ "-d",
+ temporaryFolder.getRoot().getAbsolutePath(),
+ "-XepOpt:NullAway:AnnotatedPackages=com.uber",
+ "-XepOpt:NullAway:TreatGeneratedAsUnannotated=true",
+ "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+ .addSourceLines(
+ "Generated.java",
+ "package com.uber;",
+ "@javax.annotation.Generated(\"foo\")",
+ "public class Generated {",
+ " @javax.annotation.Nullable",
+ " public Object retNull() {",
+ " return null;",
+ " }",
+ "}")
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "class Test {",
+ " void foo() { (new Generated()).retNull().toString(); }",
+ "}")
+ .doTest();
+ }
+
+ @Test
public void basicContractAnnotation() {
compilationHelper
.setArgs(
