External Library Models Integration

build.gradle

@@ -95,7 +95,7 @@ subprojects { project ->
         google()
     }
 
-    // For some reason, spotless complains when applied to the folders containing projects
+    // Spotless complains when applied to the folders containing projects
     // when they do not have a build.gradle file
     if (project.name != "jar-infer" && project.name != "library-model") {
         project.apply plugin: "com.diffplug.spotless"

code-coverage-report/build.gradle

@@ -80,4 +80,6 @@ dependencies {
     implementation project(':jar-infer:nullaway-integration-test')
     implementation project(':guava-recent-unit-tests')
     implementation project(':jdk-recent-unit-tests')
+    implementation project(':library-model:library-model-generator')
+    implementation project(':library-model:library-model-generator-integration-test')
 }

library-model/library-model-generator-cli/src/main/java/com/uber/nullaway/libmodel/LibraryModelGeneratorCLI.java

@@ -36,6 +36,11 @@ public class LibraryModelGeneratorCLI {
    *     directory.
    */
   public static void main(String[] args) {
+    if (args.length != 2) {
+      System.out.println(
+          "Incorrect number of command line arguments. Required arguments:  <inputSourceDirectory> <outputDirectory>");
+      return;
+    }
     LibraryModelGenerator libraryModelGenerator = new LibraryModelGenerator();
     libraryModelGenerator.generateAstubxForLibraryModels(args[0], args[1]);
   }

library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java

@@ -73,4 +73,54 @@ public void libraryModelNullableReturnsArrayTest() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void libraryModelWithoutJarInferEnabledTest() {
+    compilationHelper
+        .setArgs(
+            Arrays.asList(
+                "-d",
+                temporaryFolder.getRoot().getAbsolutePath(),
+                "-XepOpt:NullAway:AnnotatedPackages=com.uber"))
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import com.uber.nullaway.libmodel.AnnotationExample;",
+            "class Test {",
+            "  static AnnotationExample annotationExample = new AnnotationExample();",
+            "  static void test(String value){",
+            "  }",
+            "  static void testNegative() {",
+            "    // Since the JarInferEnabled and JarInferUseReturnAnnotations flags are not set, we don't get an error here",
+            "    test(annotationExample.makeUpperCase(\"nullaway\"));",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void libraryModelInnerClassNullableReturnsTest() {
+    compilationHelper
+        .setArgs(
+            Arrays.asList(
+                "-d",
+                temporaryFolder.getRoot().getAbsolutePath(),
+                "-XepOpt:NullAway:AnnotatedPackages=com.uber",
+                "-XepOpt:NullAway:JarInferEnabled=true",
+                "-XepOpt:NullAway:JarInferUseReturnAnnotations=true"))
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import com.uber.nullaway.libmodel.AnnotationExample;",
+            "class Test {",
+            "  static AnnotationExample.InnerExample innerExample = new AnnotationExample.InnerExample();",
+            "  static void test(String value){",
+            "  }",
+            "  static void testPositive() {",
+            "    // BUG: Diagnostic contains: passing @Nullable parameter 'innerExample.returnNull()'",
+            "    test(innerExample.returnNull());",
+            "  }",
+            "}")
+        .doTest();
+  }
 }

library-model/library-model-generator/build.gradle

@@ -14,7 +14,8 @@
  * limitations under the License.
  */
 plugins {
-    id "java-library"
+    id 'java-library'
+    id 'nullaway.java-test-conventions'
 }
 
 dependencies {

library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java

@@ -26,10 +26,8 @@
 import com.github.javaparser.ParserConfiguration.LanguageLevel;
 import com.github.javaparser.ast.CompilationUnit;
 import com.github.javaparser.ast.ImportDeclaration;
-import com.github.javaparser.ast.Node;
 import com.github.javaparser.ast.PackageDeclaration;
 import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
-import com.github.javaparser.ast.body.EnumDeclaration;
 import com.github.javaparser.ast.body.MethodDeclaration;
 import com.github.javaparser.ast.expr.AnnotationExpr;
 import com.github.javaparser.ast.type.ArrayType;
@@ -59,12 +57,12 @@
  * directory. It processes the annotated Java source code to generate an astubx file that contains
  * the required annotation information to be able to generate library models.
  */
 public class LibraryModelGenerator {
 
   public void generateAstubxForLibraryModels(String inputSourceDirectory, String outputDirectory) {
     Map<String, MethodAnnotationsRecord> methodRecords = processDirectory(inputSourceDirectory);
     writeToAstubx(outputDirectory, methodRecords);
   }
 
   /**
    * Parses all the source files within the directory using javaparser.
@@ -73,26 +71,26 @@
    * @return a Map containing the Nullability annotation information from the source files.
    */
   private Map<String, MethodAnnotationsRecord> processDirectory(String sourceDirectoryRoot) {
     Map<String, MethodAnnotationsRecord> methodRecords = new LinkedHashMap<>();
     Path root = dirnameToPath(sourceDirectoryRoot);
     AnnotationCollectorCallback ac = new AnnotationCollectorCallback(methodRecords);
     CollectionStrategy strategy = new ParserCollectionStrategy();
     // Required to include directories that contain a module-info.java, which don't parse by
     // default.
     strategy.getParserConfiguration().setLanguageLevel(LanguageLevel.JAVA_17);
     ProjectRoot projectRoot = strategy.collect(root);
 
     projectRoot
         .getSourceRoots()
         .forEach(
             sourceRoot -> {
               try {
                 sourceRoot.parse("", ac);
               } catch (IOException e) {
                 throw new RuntimeException(e);
               }
             });
     return methodRecords;
   }
 
   /**
@@ -104,124 +102,115 @@
   private void writeToAstubx(
       String outputPath, Map<String, MethodAnnotationsRecord> methodRecords) {
     if (methodRecords.isEmpty()) {
       return;
     }
     Map<String, String> importedAnnotations =
         ImmutableMap.of(
-            "Nonnull", "javax.annotation.Nonnull",
-            "Nullable", "javax.annotation.Nullable");
+            "NonNull", "org.jspecify.annotations.NonNull",
+            "Nullable", "org.jspecify.annotations.Nullable");
     Path outputPathInstance = Paths.get(outputPath);
     try {
       Files.createDirectories(outputPathInstance.getParent());
       try (DataOutputStream dos = new DataOutputStream(Files.newOutputStream(outputPathInstance))) {
         StubxWriter.write(
             dos,
             importedAnnotations,
             Collections.emptyMap(),
             Collections.emptyMap(),
             methodRecords);
       }
     } catch (IOException e) {
       throw new RuntimeException(e);
     }
   }
 
   public Path dirnameToPath(String dir) {
     File f = new File(dir);
     String absoluteDir = f.getAbsolutePath();
     if (absoluteDir.endsWith("/.")) {
       absoluteDir = absoluteDir.substring(0, absoluteDir.length() - 2);
     }
     return Paths.get(absoluteDir);
   }
 
   private static class AnnotationCollectorCallback implements SourceRoot.Callback {
 
     private final AnnotationCollectionVisitor annotationCollectionVisitor;
 
     public AnnotationCollectorCallback(Map<String, MethodAnnotationsRecord> methodRecords) {
       this.annotationCollectionVisitor = new AnnotationCollectionVisitor(methodRecords);
     }
 
     @Override
     public Result process(Path localPath, Path absolutePath, ParseResult<CompilationUnit> result) {
       Result res = Result.SAVE;
       Optional<CompilationUnit> opt = result.getResult();
       if (opt.isPresent()) {
         CompilationUnit cu = opt.get();
         cu.accept(annotationCollectionVisitor, null);
       }
       return res;
     }
   }
 
   private static class AnnotationCollectionVisitor extends VoidVisitorAdapter<Void> {
 
-    private String packageName = "";
+    private StringBuilder parentName;
     private boolean isJspecifyNullableImportPresent = false;
     private boolean isNullMarked = false;
     private Map<String, MethodAnnotationsRecord> methodRecords;
     private static final String ARRAY_RETURN_TYPE_STRING = "Array";
     private static final String NULL_MARKED = "NullMarked";
     private static final String NULLABLE = "Nullable";
     private static final String JSPECIFY_NULLABLE_IMPORT = "org.jspecify.annotations.Nullable";
 
     public AnnotationCollectionVisitor(Map<String, MethodAnnotationsRecord> methodRecords) {
       this.methodRecords = methodRecords;
     }
 
     @Override
     public void visit(PackageDeclaration pd, Void arg) {
-      this.packageName = pd.getNameAsString();
+      this.parentName = new StringBuilder(pd.getNameAsString());
       super.visit(pd, null);
     }
 
     @Override
     public void visit(ImportDeclaration id, Void arg) {
       if (id.getName().toString().contains(JSPECIFY_NULLABLE_IMPORT)) {
         this.isJspecifyNullableImportPresent = true;
       }
       super.visit(id, null);
     }
 
     @Override
     public void visit(ClassOrInterfaceDeclaration cid, Void arg) {
-      /*Currently, this logic only supports @NullMarked annotations on top-level classes and
-      does not handle cases where @NullMarked appears on some nested classes but not others*/
+      /*This logic assumes an explicit @NullMarked annotation on the top-level class within a
+      source file, and it's expected that each source file contains only one top-level class. The
+      logic does not currently handle cases where @NullMarked annotations appear on some nested
+      classes but not others. It also does not consider annotations within package-info.java or
+      module-info.java files.*/
+      parentName.append(".").append(cid.getNameAsString());
       cid.getAnnotations()
           .forEach(
               a -> {
                 if (a.getNameAsString().equalsIgnoreCase(NULL_MARKED)) {
                   this.isNullMarked = true;
                 }
               });
       super.visit(cid, null);
+      // We reset the variable that constructs the parent name after visiting all the children.
+      parentName.delete(parentName.lastIndexOf("." + cid.getNameAsString()), parentName.length());
     }
 
     @Override
     public void visit(MethodDeclaration md, Void arg) {
-      Optional<Node> parentClassNode = md.getParentNode();
-      String parentClassName = "";
-      if (parentClassNode.isPresent()) {
-        if (parentClassNode.get() instanceof ClassOrInterfaceDeclaration) {
-          parentClassName = ((ClassOrInterfaceDeclaration) parentClassNode.get()).getNameAsString();
-        } else if (parentClassNode.get() instanceof EnumDeclaration) {
-          parentClassName = ((EnumDeclaration) parentClassNode.get()).getNameAsString();
-        }
-      }
       if (this.isNullMarked && hasNullableReturn(md)) {
         methodRecords.put(
-            packageName
-                + "."
-                + parentClassName
-                + ":"
-                + getMethodReturnTypeString(md)
-                + " "
-                + md.getSignature().toString(),
+            parentName + ":" + getMethodReturnTypeString(md) + " " + md.getSignature().toString(),
             MethodAnnotationsRecord.create(ImmutableSet.of("Nullable"), ImmutableMap.of()));
       }
       super.visit(md, null);
     }
 
     /**
      * Determines if a MethodDeclaration can return null.
@@ -236,17 +225,17 @@
         Nullable(@Nullable Object []) */
         for (AnnotationExpr annotation : md.getType().getAnnotations()) {
           if (isAnnotationNullable(annotation)) {
             return true;
           }
         }
       } else {
         for (AnnotationExpr annotation : md.getAnnotations()) {
           if (isAnnotationNullable(annotation)) {
             return true;
           }
         }
       }
       return false;
     }
 
     /**
@@ -258,16 +247,16 @@
      */
     private String getMethodReturnTypeString(MethodDeclaration md) {
       if (md.getType() instanceof ClassOrInterfaceType) {
         return md.getType().getChildNodes().get(0).toString();
       } else if (md.getType() instanceof ArrayType) {
         return ARRAY_RETURN_TYPE_STRING;
       } else {
         return md.getType().toString();
       }
     }
 
     private boolean isAnnotationNullable(AnnotationExpr annotation) {
-      // Since we only want to consider jspecify Nullable annotations.
+      // We only consider jspecify Nullable annotations(star imports are not supported).
       return (annotation.getNameAsString().equalsIgnoreCase(NULLABLE)
               && this.isJspecifyNullableImportPresent)
           || annotation.getNameAsString().equalsIgnoreCase(JSPECIFY_NULLABLE_IMPORT);

library-model/test-library-model-generator/build.gradle

@@ -34,7 +34,6 @@ jar {
 jar.doLast {
     javaexec {
         classpath = files("${rootProject.projectDir}/library-model/library-model-generator-cli/build/libs/library-model-generator-cli.jar")
-        main = "com.uber.nullaway.libmodel.LibraryModelGeneratorCLI"
         args = [
             testInputsPath,
             "${jar.destinationDirectory.get()}/${astubxPath}"

library-model/test-library-model-generator/src/main/java/com/uber/nullaway/libmodel/AnnotationExample.java

@@ -32,4 +32,11 @@ public Integer[] generateIntArray(int size) {
   public String nullReturn() {
     return null;
   }
+
+  public static class InnerExample {
+
+    public String returnNull() {
+      return null;
+    }
+  }
 }

library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/uber/nullaway/libmodel/AnnotationExample.java

@@ -30,9 +30,19 @@ public String makeUpperCase(String inputString) {
     /**
      * This method exists to test that
      * we do not process this annotation.
+     * Since for the purposes of this tool,
+     * we are only considering the jspecify annotation.
      */
     @javax.annotation.Nullable
     public String nullReturn() {
         return null;
     }
+
+    public static class InnerExample {
+
+        @Nullable
+        public String returnNull() {
+            return null;
+        }
+    }
 }

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

@@ -218,7 +218,8 @@ private boolean isReturnAnnotatedNullable(Symbol.MethodSymbol methodSymbol) {
         if (methodArgAnnotations != null) {
           Set<String> methodAnnotations = methodArgAnnotations.get(RETURN);
           if (methodAnnotations != null) {
-            if (methodAnnotations.contains("javax.annotation.Nullable")) {
+            if (methodAnnotations.contains("javax.annotation.Nullable")
+                || methodAnnotations.contains("org.jspecify.annotations.Nullable")) {
               LOG(DEBUG, "DEBUG", "Nullable return for method: " + methodSign);
               return true;
             }