From fb195518bde6e3ef29757c390add85dce2486de6 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Fri, 12 Jul 2024 21:40:24 +0530 Subject: [PATCH 01/34] default and nullable params initial changes --- .../DefinitelyDerefedParamsDriver.java | 1 + .../libmodel/LibraryModelIntegrationTest.java | 49 +++++++++++++++++++ .../libmodel/LibraryModelGenerator.java | 47 ++++++++++++++++-- .../uber/nullaway/libmodel/StubxWriter.java | 10 +++- .../com/test/ParameterAnnotationExample.java | 22 +++++++++ .../nullaway/libmodel/AnnotationExample.java | 12 +++++ .../com/test/ParameterAnnotationExample.java | 20 ++++++++ .../nullaway/libmodel/AnnotationExample.java | 12 +++++ .../handlers/LibraryModelsHandler.java | 25 ++++++++-- .../nullaway/handlers/StubxCacheUtil.java | 13 +++++ 10 files changed, 202 insertions(+), 9 deletions(-) create mode 100644 library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java create mode 100644 library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java diff --git a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java index d709e09a50..d955d910ef 100644 --- a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java +++ b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java @@ -456,6 +456,7 @@ private void writeModel(DataOutputStream out) throws IOException { packageAnnotations, typeAnnotations, methodRecords, + Collections.emptySet(), Collections.emptyMap()); } diff --git a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java index 3a230d0200..25e5250418 100644 --- a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java +++ b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java @@ -179,4 +179,53 @@ public void libraryModelNullableUpperBoundsWithoutJarInferTest() { "}") .doTest(); } + + @Test + public void libraryModelDefaultParameterNullabilityTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:JSpecifyMode=true", + "-XepOpt:NullAway:JarInferEnabled=true", + "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.test.ParameterAnnotationExample;", + "class Test {", + " static ParameterAnnotationExample annotationExample = new ParameterAnnotationExample();", + " static void testPositive() {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " annotationExample.add(5,null);", + " }", + "}") + .doTest(); + } + + @Test + public void libraryModelParameterNullabilityTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:JSpecifyMode=true", + "-XepOpt:NullAway:JarInferEnabled=true", + "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.test.ParameterAnnotationExample;", + "class Test {", + " static ParameterAnnotationExample annotationExample = new ParameterAnnotationExample();", + " static void testNegative() {", + " annotationExample.getNewObjectIfNull(null);", + " }", + "}") + .doTest(); + } } diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index bc178ae644..4d9906f856 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -29,6 +29,7 @@ import com.github.javaparser.ast.PackageDeclaration; import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; import com.github.javaparser.ast.body.MethodDeclaration; +import com.github.javaparser.ast.body.Parameter; import com.github.javaparser.ast.expr.AnnotationExpr; import com.github.javaparser.ast.type.ArrayType; import com.github.javaparser.ast.type.ClassOrInterfaceType; @@ -47,6 +48,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -70,10 +72,11 @@ public class LibraryModelGenerator { */ public void generateAstubxForLibraryModels(String inputSourceDirectory, String outputDirectory) { Map methodRecords = new LinkedHashMap<>(); + Set nullMarkedClasses = new HashSet<>(); Map> nullableUpperBounds = new LinkedHashMap<>(); Path root = dirnameToPath(inputSourceDirectory); AnnotationCollectorCallback ac = - new AnnotationCollectorCallback(methodRecords, nullableUpperBounds); + new AnnotationCollectorCallback(methodRecords, nullMarkedClasses, nullableUpperBounds); CollectionStrategy strategy = new ParserCollectionStrategy(); // Required to include directories that contain a module-info.java, which don't parse by // default. @@ -90,7 +93,7 @@ public void generateAstubxForLibraryModels(String inputSourceDirectory, String o throw new RuntimeException(e); } }); - writeToAstubx(outputDirectory, methodRecords, nullableUpperBounds); + writeToAstubx(outputDirectory, methodRecords, nullMarkedClasses, nullableUpperBounds); } /** @@ -102,6 +105,7 @@ public void generateAstubxForLibraryModels(String inputSourceDirectory, String o private void writeToAstubx( String outputPath, Map methodRecords, + Set nullMarkedClasses, Map> nullableUpperBounds) { if (methodRecords.isEmpty() && nullableUpperBounds.isEmpty()) { return; @@ -120,6 +124,7 @@ private void writeToAstubx( Collections.emptyMap(), Collections.emptyMap(), methodRecords, + nullMarkedClasses, nullableUpperBounds); } } catch (IOException e) { @@ -142,9 +147,10 @@ private static class AnnotationCollectorCallback implements SourceRoot.Callback public AnnotationCollectorCallback( Map methodRecords, + Set nullMarkedClasses, Map> nullableUpperBounds) { this.annotationCollectionVisitor = - new AnnotationCollectionVisitor(methodRecords, nullableUpperBounds); + new AnnotationCollectionVisitor(methodRecords, nullMarkedClasses, nullableUpperBounds); } @Override @@ -165,6 +171,7 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter methodRecords; + private final Set nullMarkedClasses; private final Map> nullableUpperBounds; private static final String ARRAY_RETURN_TYPE_STRING = "Array"; private static final String NULL_MARKED = "NullMarked"; @@ -173,8 +180,10 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter methodRecords, + Set nullMarkedClasses, Map> nullableUpperBounds) { this.methodRecords = methodRecords; + this.nullMarkedClasses = nullMarkedClasses; this.nullableUpperBounds = nullableUpperBounds; } @@ -205,6 +214,7 @@ public void visit(ClassOrInterfaceDeclaration cid, Void arg) { a -> { if (a.getNameAsString().equalsIgnoreCase(NULL_MARKED)) { this.isNullMarked = true; + this.nullMarkedClasses.add(parentName); } }); if (this.isNullMarked) { @@ -221,10 +231,16 @@ public void visit(ClassOrInterfaceDeclaration cid, Void arg) { @Override public void visit(MethodDeclaration md, Void arg) { - if (this.isNullMarked && hasNullableReturn(md)) { + ImmutableMap> nullableParameterMap = getNullableParameters(md); + boolean isReturnNullable = hasNullableReturn(md); + ImmutableSet nullableReturn = + isReturnNullable ? ImmutableSet.of(NULLABLE) : ImmutableSet.of(); + // We write the method record into the astubx if it contains a Nullable return or any Nullable + // parameter. + if (this.isNullMarked && (isReturnNullable || !nullableParameterMap.isEmpty())) { methodRecords.put( parentName + ":" + getMethodReturnTypeString(md) + " " + md.getSignature().toString(), - MethodAnnotationsRecord.create(ImmutableSet.of("Nullable"), ImmutableMap.of())); + MethodAnnotationsRecord.create(nullableReturn, nullableParameterMap)); } super.visit(md, null); } @@ -301,5 +317,26 @@ private ImmutableSet getGenericTypeParameterNullableUpperBounds( } return setBuilder.build(); } + + /** + * Takes a MethodDeclaration instance and provides a map containing the Integer indices for the + * parameters with their Nullable annotation to be written in the astubx format. + * + * @param md MethodDeclaration instance. + * @return Map of Nullable parameters for the method. + */ + private ImmutableMap> getNullableParameters( + MethodDeclaration md) { + ImmutableMap.Builder> mapBuilder = ImmutableMap.builder(); + List parameterList = md.getParameters(); + for (int i = 0; i < parameterList.size(); i++) { + Parameter parameter = parameterList.get(i); + Optional nullableAnnotation = parameter.getAnnotationByName(NULLABLE); + if (nullableAnnotation.isPresent() && isAnnotationNullable(nullableAnnotation.get())) { + mapBuilder.put(i, ImmutableSet.of("Nullable")); + } + } + return mapBuilder.build(); + } } } diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java index a3900534c9..ed507c680a 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java @@ -37,6 +37,7 @@ public static void write( Map> packageAnnotations, Map> typeAnnotations, Map methodRecords, + Set nullMarkedClasses, Map> nullableUpperBounds) throws IOException { // File format version/magic number @@ -51,6 +52,7 @@ public static void write( packageAnnotations.keySet(), typeAnnotations.keySet(), methodRecords.keySet(), + nullMarkedClasses, nullableUpperBounds.keySet()); for (Collection keyset : keysets) { for (String key : keyset) { @@ -120,6 +122,12 @@ public static void write( } } } + // Followed by the number of NullMarked Classes + out.writeInt(nullMarkedClasses.size()); + // Followed by the null marked class records from the dictionary + for (String entry : nullMarkedClasses) { + out.writeInt(encodingDictionary.get(entry)); + } // Followed by the number of nullable upper bounds records out.writeInt(nullableUpperBounds.size()); for (Map.Entry> entry : nullableUpperBounds.entrySet()) { @@ -127,7 +135,7 @@ public static void write( Set parameters = entry.getValue(); out.writeInt(parameters.size()); for (Integer parameter : parameters) { - // Followed by the nullable upper bound record as a par of integers + // Followed by the nullable upper bound record as a pair of integers out.writeInt(encodingDictionary.get(entry.getKey())); out.writeInt(parameter); } diff --git a/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java b/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java new file mode 100644 index 0000000000..dbd0ff8538 --- /dev/null +++ b/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java @@ -0,0 +1,22 @@ +package com.test; + +/** + * This class has the same name as the class under + * resources/sample_annotated/src/com/uber/nullaway/libmodel/AnnotationExample.java because we use + * this as the unannotated version for our test cases to see if we are appropriately processing the + * annotations as an external library model. + */ +public class ParameterAnnotationExample { + + public static Integer add(Integer a, Integer b) { + return a + b; + } + + public static Object getNewObjectIfNull(Object object) { + if (object == null) { + return new Object(); + } else { + return object; + } + } +} diff --git a/library-model/test-library-model-generator/src/main/java/com/uber/nullaway/libmodel/AnnotationExample.java b/library-model/test-library-model-generator/src/main/java/com/uber/nullaway/libmodel/AnnotationExample.java index 984b969c35..7b22856025 100644 --- a/library-model/test-library-model-generator/src/main/java/com/uber/nullaway/libmodel/AnnotationExample.java +++ b/library-model/test-library-model-generator/src/main/java/com/uber/nullaway/libmodel/AnnotationExample.java @@ -49,4 +49,16 @@ public T getNullable() { return nullableObject; } } + + public static Integer add(Integer a, Integer b) { + return a + b; + } + + public static Object getNewObjectIfNull(Object object) { + if (object == null) { + return new Object(); + } else { + return object; + } + } } diff --git a/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java new file mode 100644 index 0000000000..672ec6371a --- /dev/null +++ b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java @@ -0,0 +1,20 @@ +package com.test; + +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +@NullMarked +public class ParameterAnnotationExample { + + public static Integer add(Integer a, Integer b) { + return a + b; + } + + public static Object getNewObjectIfNull(@Nullable Object object) { + if (object == null) { + return new Object(); + } else { + return object; + } + } +} diff --git a/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/uber/nullaway/libmodel/AnnotationExample.java b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/uber/nullaway/libmodel/AnnotationExample.java index 5fef2e2118..a46ecca152 100644 --- a/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/uber/nullaway/libmodel/AnnotationExample.java +++ b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/uber/nullaway/libmodel/AnnotationExample.java @@ -54,4 +54,16 @@ public T getNullable() { return nullableObject; } } + + public static Integer add(Integer a, Integer b) { + return a + b; + } + + public static Object getNewObjectIfNull(@Nullable Object object) { + if (object == null) { + return new Object(); + } else { + return object; + } + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index 10445a9aa5..b3ad6fb3f1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -1290,20 +1290,23 @@ private static Symbol.MethodSymbol lookupHandlingOverrides( /** Constructs Library Models from stubx files */ private static class ExternalStubxLibraryModels implements LibraryModels { + @SuppressWarnings("unused") private final Map>>> argAnnotCache; + + private final Set nullMarkedClassesCache; private final Map upperBoundsCache; ExternalStubxLibraryModels() { String libraryModelLogName = "LM"; StubxCacheUtil cacheUtil = new StubxCacheUtil(libraryModelLogName); argAnnotCache = cacheUtil.getArgAnnotCache(); + nullMarkedClassesCache = cacheUtil.getNullMarkedClassesCache(); upperBoundsCache = cacheUtil.getUpperBoundCache(); } @Override public ImmutableSet nullMarkedClasses() { - Set cachedNullMarkedClasses = argAnnotCache.keySet(); - return new ImmutableSet.Builder().addAll(cachedNullMarkedClasses).build(); + return new ImmutableSet.Builder().addAll(nullMarkedClassesCache).build(); } @Override @@ -1323,7 +1326,23 @@ public ImmutableSetMultimap failIfNullParameters() { @Override public ImmutableSetMultimap explicitlyNullableParameters() { - return ImmutableSetMultimap.of(); + ImmutableSetMultimap.Builder mapBuilder = + new ImmutableSetMultimap.Builder<>(); + for (Map.Entry>>> outerEntry : + argAnnotCache.entrySet()) { + String className = outerEntry.getKey(); + for (Map.Entry>> innerEntry : + outerEntry.getValue().entrySet()) { + String methodName = innerEntry.getKey().split(" ")[1]; + for (Map.Entry> entry : innerEntry.getValue().entrySet()) { + Integer index = entry.getKey(); + if (index >= 0) { + mapBuilder.put(methodRef(className, methodName), index); + } + } + } + } + return mapBuilder.build(); } @Override diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java b/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java index e2ac9cb991..6f81318e96 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; @@ -59,6 +60,8 @@ private void LOG(boolean cond, String tag, String msg) { private final Map upperBoundCache; + private final Set nullMarkedClassesCache; + /** * Initializes a new {@code StubxCacheUtil} instance. * @@ -70,6 +73,7 @@ private void LOG(boolean cond, String tag, String msg) { public StubxCacheUtil(String logCaller) { argAnnotCache = new LinkedHashMap<>(); upperBoundCache = new HashMap<>(); + nullMarkedClassesCache = new HashSet<>(); this.logCaller = logCaller; loadStubxFiles(); } @@ -78,6 +82,10 @@ public Map getUpperBoundCache() { return upperBoundCache; } + public Set getNullMarkedClassesCache() { + return nullMarkedClassesCache; + } + public Map>>> getArgAnnotCache() { return argAnnotCache; } @@ -162,6 +170,11 @@ public void parseStubStream(InputStream stubxInputStream, String stubxLocation) "method: " + methodSig + ", argNum: " + argNum + ", arg annotation: " + annotation); cacheAnnotation(methodSig, argNum, annotation); } + // reading the NullMarked classes + int numNullMarkedClasses = in.readInt(); + for (int i = 0; i < numNullMarkedClasses; i++) { + this.nullMarkedClassesCache.add(strings[in.readInt()]); + } // read the number of nullable upper bound entries int numClassesWithNullableUpperBounds = in.readInt(); for (int i = 0; i < numClassesWithNullableUpperBounds; i++) { From d76a3b2ef6c761cf076497a1d66d6b4e37bc74fb Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Mon, 22 Jul 2024 17:47:30 +0530 Subject: [PATCH 02/34] Adding javaparser type solver to use fully qualified names for method parameters --- gradle/dependencies.gradle | 1 + .../library-model-generator/build.gradle | 1 + .../libmodel/LibraryModelGenerator.java | 21 ++++++++++++++++++- .../handlers/InferredJARModelsHandler.java | 2 +- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index e21dbd0003..bb21fe7d01 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -76,6 +76,7 @@ def build = [ checkerDataflow : "org.checkerframework:dataflow-nullaway:${versions.checkerFramework}", guava : "com.google.guava:guava:30.1-jre", javaparser : "com.github.javaparser:javaparser-core:3.26.0", + javaparserSymbolSolver : "com.github.javaparser:javaparser-symbol-solver-core:3.26.1", javaxValidation : "javax.validation:validation-api:2.0.1.Final", jspecify : "org.jspecify:jspecify:0.3.0", jsr305Annotations : "com.google.code.findbugs:jsr305:3.0.2", diff --git a/library-model/library-model-generator/build.gradle b/library-model/library-model-generator/build.gradle index 1d497fccd7..92f68183a1 100644 --- a/library-model/library-model-generator/build.gradle +++ b/library-model/library-model-generator/build.gradle @@ -21,6 +21,7 @@ plugins { dependencies { implementation deps.build.guava implementation deps.build.javaparser + implementation deps.build.javaparserSymbolSolver compileOnly deps.apt.autoValueAnnot annotationProcessor deps.apt.autoValue } diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index 4d9906f856..354693391a 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -35,6 +35,11 @@ import com.github.javaparser.ast.type.ClassOrInterfaceType; import com.github.javaparser.ast.type.TypeParameter; import com.github.javaparser.ast.visitor.VoidVisitorAdapter; +import com.github.javaparser.resolution.TypeSolver; +import com.github.javaparser.symbolsolver.JavaSymbolSolver; +import com.github.javaparser.symbolsolver.resolution.typesolvers.CombinedTypeSolver; +import com.github.javaparser.symbolsolver.resolution.typesolvers.JavaParserTypeSolver; +import com.github.javaparser.symbolsolver.resolution.typesolvers.ReflectionTypeSolver; import com.github.javaparser.utils.CollectionStrategy; import com.github.javaparser.utils.ParserCollectionStrategy; import com.github.javaparser.utils.ProjectRoot; @@ -80,7 +85,12 @@ public void generateAstubxForLibraryModels(String inputSourceDirectory, String o CollectionStrategy strategy = new ParserCollectionStrategy(); // Required to include directories that contain a module-info.java, which don't parse by // default. + TypeSolver typeSolver = + new CombinedTypeSolver( + new ReflectionTypeSolver(), new JavaParserTypeSolver(Paths.get(inputSourceDirectory))); + JavaSymbolSolver symbolSolver = new JavaSymbolSolver(typeSolver); strategy.getParserConfiguration().setLanguageLevel(LanguageLevel.JAVA_17); + strategy.getParserConfiguration().setSymbolResolver(symbolSolver); ProjectRoot projectRoot = strategy.collect(root); projectRoot @@ -237,9 +247,18 @@ public void visit(MethodDeclaration md, Void arg) { isReturnNullable ? ImmutableSet.of(NULLABLE) : ImmutableSet.of(); // We write the method record into the astubx if it contains a Nullable return or any Nullable // parameter. + @SuppressWarnings("unused") + String methodSignatureWithQualifiedParameters = + md.resolve() + .getQualifiedSignature() + .substring(md.resolve().getQualifiedSignature().lastIndexOf(md.getNameAsString())); if (this.isNullMarked && (isReturnNullable || !nullableParameterMap.isEmpty())) { methodRecords.put( - parentName + ":" + getMethodReturnTypeString(md) + " " + md.getSignature().toString(), + parentName + + ":" + + getMethodReturnTypeString(md) + + " " + + methodSignatureWithQualifiedParameters, MethodAnnotationsRecord.create(nullableReturn, nullableParameterMap)); } super.visit(md, null); diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java index f171c502da..508346f71d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -243,7 +243,7 @@ private String getMethodSignature(Symbol.MethodSymbol method) { + "("; if (!method.getParameters().isEmpty()) { for (Symbol.VarSymbol var : method.getParameters()) { - methodSign += getSimpleTypeName(var.type) + ", "; + methodSign += var.type.toString() + ", "; } methodSign = methodSign.substring(0, methodSign.lastIndexOf(',')); } From 9b7d5c16e6c1fdfde072d812eb0f293f0e64ecee Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Mon, 22 Jul 2024 23:47:40 +0530 Subject: [PATCH 03/34] clean up --- .../java/com/uber/nullaway/libmodel/LibraryModelGenerator.java | 1 - .../java/com/uber/nullaway/handlers/LibraryModelsHandler.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index 354693391a..7446679123 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -247,7 +247,6 @@ public void visit(MethodDeclaration md, Void arg) { isReturnNullable ? ImmutableSet.of(NULLABLE) : ImmutableSet.of(); // We write the method record into the astubx if it contains a Nullable return or any Nullable // parameter. - @SuppressWarnings("unused") String methodSignatureWithQualifiedParameters = md.resolve() .getQualifiedSignature() diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index b3ad6fb3f1..65de9258e8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -1333,7 +1333,7 @@ public ImmutableSetMultimap explicitlyNullableParameters() { String className = outerEntry.getKey(); for (Map.Entry>> innerEntry : outerEntry.getValue().entrySet()) { - String methodName = innerEntry.getKey().split(" ")[1]; + String methodName = innerEntry.getKey().substring(innerEntry.getKey().indexOf(" ") + 1); for (Map.Entry> entry : innerEntry.getValue().entrySet()) { Integer index = entry.getKey(); if (index >= 0) { From 96be113626b2d8fe64ddd1ebe4465f296982aaf0 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 22 Jul 2024 12:29:30 -0700 Subject: [PATCH 04/34] specify javaparser version in one place --- gradle/dependencies.gradle | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 6c59cf06d9..fd34620183 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -52,6 +52,7 @@ def versions = [ commonscli : "1.4", autoValue : "1.10.2", autoService : "1.1.1", + javaparser : "3.26.1", ] def apt = [ @@ -75,8 +76,8 @@ def build = [ errorProneTestHelpersOld: "com.google.errorprone:error_prone_test_helpers:${oldestErrorProneVersion}", checkerDataflow : "org.checkerframework:dataflow-nullaway:${versions.checkerFramework}", guava : "com.google.guava:guava:30.1-jre", - javaparser : "com.github.javaparser:javaparser-core:3.26.0", - javaparserSymbolSolver : "com.github.javaparser:javaparser-symbol-solver-core:3.26.1", + javaparser : "com.github.javaparser:javaparser-core:${versions.javaparser}", + javaparserSymbolSolver : "com.github.javaparser:javaparser-symbol-solver-core:${versions.javaparser}", javaxValidation : "javax.validation:validation-api:2.0.1.Final", jspecify : "org.jspecify:jspecify:1.0.0", commonsIO : "commons-io:commons-io:2.11.0", From 972c977e5a4e5775439aaf8c98516a6437bb65fd Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 4 Sep 2024 16:54:10 -0700 Subject: [PATCH 05/34] WIP --- .../DefinitelyDerefedParamsDriver.java | 25 ++----------------- .../handlers/InferredJARModelsHandler.java | 4 +-- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java index d955d910ef..17f1f72a9f 100644 --- a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java +++ b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java @@ -26,6 +26,7 @@ import com.ibm.wala.classLoader.PhantomClass; import com.ibm.wala.classLoader.ShrikeCTMethod; import com.ibm.wala.core.util.config.AnalysisScopeReader; +import com.ibm.wala.core.util.strings.StringStuff; import com.ibm.wala.core.util.warnings.Warnings; import com.ibm.wala.ipa.callgraph.AnalysisCache; import com.ibm.wala.ipa.callgraph.AnalysisCacheImpl; @@ -528,28 +529,6 @@ private static String getAstubxSignature(IMethod mtd) { * @return String Unqualified type name. */ private static String getSimpleTypeName(TypeReference typ) { - ImmutableMap mapFullTypeName = - ImmutableMap.builder() - .put("B", "byte") - .put("C", "char") - .put("D", "double") - .put("F", "float") - .put("I", "int") - .put("J", "long") - .put("S", "short") - .put("Z", "boolean") - .build(); - if (typ.isArrayType()) { - return "Array"; - } - String typName = typ.getName().toString(); - if (typName.startsWith("L")) { - typName = typName.split("<")[0].substring(1); // handle generics - typName = typName.substring(typName.lastIndexOf('/') + 1); // get unqualified name - typName = typName.substring(typName.lastIndexOf('$') + 1); // handle inner classes - } else { - typName = mapFullTypeName.get(typName); - } - return typName; + return StringStuff.jvmToBinaryName(typ.getName().toString()); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java index be1632b08e..c4aba6c0be 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -254,9 +254,9 @@ private String getMethodSignature(Symbol.MethodSymbol method) { private String getSimpleTypeName(Type typ) { if (typ.getKind() == TypeKind.TYPEVAR) { - return typ.getUpperBound().tsym.getSimpleName().toString(); + return typ.getUpperBound().tsym.getQualifiedName().toString(); } else { - return typ.tsym.getSimpleName().toString(); + return typ.tsym.getQualifiedName().toString(); } } } From 38f639f28c9e9a6f9a39499b92155679bf313cbe Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 4 Sep 2024 17:35:36 -0700 Subject: [PATCH 06/34] fix more tests --- .../uber/nullaway/jarinfer/JarInferTest.java | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java index 4188054d4f..02efb7cb85 100644 --- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java +++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java @@ -53,7 +53,6 @@ /** Unit tests for {@link com.uber.nullaway.jarinfer}. */ @RunWith(JUnit4.class) -@SuppressWarnings("CheckTestExtendsBaseClass") public class JarInferTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -206,8 +205,8 @@ public void toyStatic() throws Exception { "toys", "Test", ImmutableMap.of( - "toys.Test:void test(String, Foo, Bar)", Sets.newHashSet(0, 2), - "toys.Foo:boolean run(String)", Sets.newHashSet(1)), + "toys.Test:void test(java.lang.String, toys.Foo, toys.Bar)", Sets.newHashSet(0, 2), + "toys.Foo:boolean run(java.lang.String)", Sets.newHashSet(1)), "class Foo {", " private String foo;", " public Foo(String str) {", @@ -267,7 +266,8 @@ public void toyNonStatic() throws Exception { "toyNonStatic", "toys", "Foo", - ImmutableMap.of("toys.Foo:void test(String, String)", Sets.newHashSet(1)), + ImmutableMap.of( + "toys.Foo:void test(java.lang.String, java.lang.String)", Sets.newHashSet(1)), "class Foo {", " private String foo;", " public Foo(String str) {", @@ -303,7 +303,9 @@ public void toyNullTestAPI() throws Exception { "toyNullTestAPI", "toys", "Foo", - ImmutableMap.of("toys.Foo:void test(String, String, String)", Sets.newHashSet(1, 3)), + ImmutableMap.of( + "toys.Foo:void test(java.lang.String, java.lang.String, java.lang.String)", + Sets.newHashSet(1, 3)), "import com.google.common.base.Preconditions;", "import java.util.Objects;", "import org.junit.Assert;", @@ -332,7 +334,9 @@ public void toyConditionalFlow() throws Exception { "toyNullTestAPI", "toys", "Foo", - ImmutableMap.of("toys.Foo:void test(String, String, String)", Sets.newHashSet(1, 2)), + ImmutableMap.of( + "toys.Foo:void test(java.lang.String, java.lang.String, java.lang.String)", + Sets.newHashSet(1, 2)), "import com.google.common.base.Preconditions;", "import java.util.Objects;", "import org.junit.Assert;", @@ -362,7 +366,8 @@ public void toyConditionalFlow2() throws Exception { "toys", "Foo", ImmutableMap.of( - "toys.Foo:void test(Object, Object, Object, Object)", Sets.newHashSet(1, 4)), + "toys.Foo:void test(java.lang.Object, java.lang.Object, java.lang.Object, java.lang.Object)", + Sets.newHashSet(1, 4)), "import com.google.common.base.Preconditions;", "import java.util.Objects;", "import org.junit.Assert;", @@ -401,7 +406,8 @@ public void toyReassigningTest() throws Exception { "toyNullTestAPI", "toys", "Foo", - ImmutableMap.of("toys.Foo:void test(String, String)", Sets.newHashSet(1)), + ImmutableMap.of( + "toys.Foo:void test(java.lang.String, java.lang.String)", Sets.newHashSet(1)), "import com.google.common.base.Preconditions;", "import java.util.Objects;", "import org.junit.Assert;", From 4d375d12a626660f0eeb91da68e2a351735b0a94 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 4 Sep 2024 18:21:00 -0700 Subject: [PATCH 07/34] more --- .../libmodel/LibraryModelGenerator.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index 7446679123..3962e85c25 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -36,6 +36,8 @@ import com.github.javaparser.ast.type.TypeParameter; import com.github.javaparser.ast.visitor.VoidVisitorAdapter; import com.github.javaparser.resolution.TypeSolver; +import com.github.javaparser.resolution.declarations.ResolvedMethodDeclaration; +import com.github.javaparser.resolution.types.ResolvedType; import com.github.javaparser.symbolsolver.JavaSymbolSolver; import com.github.javaparser.symbolsolver.resolution.typesolvers.CombinedTypeSolver; import com.github.javaparser.symbolsolver.resolution.typesolvers.JavaParserTypeSolver; @@ -183,6 +185,7 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter methodRecords; private final Set nullMarkedClasses; private final Map> nullableUpperBounds; + // TODO this is sketchy! private static final String ARRAY_RETURN_TYPE_STRING = "Array"; private static final String NULL_MARKED = "NullMarked"; private static final String NULLABLE = "Nullable"; @@ -247,15 +250,15 @@ public void visit(MethodDeclaration md, Void arg) { isReturnNullable ? ImmutableSet.of(NULLABLE) : ImmutableSet.of(); // We write the method record into the astubx if it contains a Nullable return or any Nullable // parameter. + ResolvedMethodDeclaration resolved = md.resolve(); + String qualifiedSignature = resolved.getQualifiedSignature(); String methodSignatureWithQualifiedParameters = - md.resolve() - .getQualifiedSignature() - .substring(md.resolve().getQualifiedSignature().lastIndexOf(md.getNameAsString())); + qualifiedSignature.substring(qualifiedSignature.lastIndexOf(md.getNameAsString())); if (this.isNullMarked && (isReturnNullable || !nullableParameterMap.isEmpty())) { methodRecords.put( parentName + ":" - + getMethodReturnTypeString(md) + + getMethodReturnTypeString(resolved) + " " + methodSignatureWithQualifiedParameters, MethodAnnotationsRecord.create(nullableReturn, nullableParameterMap)); @@ -296,13 +299,15 @@ private boolean hasNullableReturn(MethodDeclaration md) { * @param md The MethodDeclaration instance. * @return The return type string value to be written into the astubx file. */ - private String getMethodReturnTypeString(MethodDeclaration md) { - if (md.getType() instanceof ClassOrInterfaceType) { - return md.getType().getChildNodes().get(0).toString(); - } else if (md.getType() instanceof ArrayType) { + private String getMethodReturnTypeString(ResolvedMethodDeclaration md) { + ResolvedType returnType = md.getReturnType(); + if (returnType.isReferenceType()) { + return returnType.asReferenceType().getQualifiedName().toString(); + } else if (returnType.isArray()) { return ARRAY_RETURN_TYPE_STRING; } else { - return md.getType().toString(); + throw new RuntimeException("Unexpected return type: " + returnType); + // return returnType.asPrimitive().toString(); } } From 3f25628d3a8d7f1fa05be08ff1cae6941ebf6611 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Fri, 6 Sep 2024 22:15:26 -0700 Subject: [PATCH 08/34] resolving conflicts for merge --- .../libmodel/LibraryModelGenerator.java | 86 +++++++++++++------ 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index 3962e85c25..583d6be1eb 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -71,19 +71,52 @@ */ public class LibraryModelGenerator { + /** + * Data class for storing the annotation information collected from the source files. This is the + * information that is stored in the astubx file. + */ + public static class LibraryModelData { + public final Map methodRecords; + public final Map> nullableUpperBounds; + public final Set nullMarkedClasses; + + public LibraryModelData( + Map methodRecords, + Map> nullableUpperBounds, + Set nullMarkedClasses) { + this.methodRecords = methodRecords; + this.nullableUpperBounds = nullableUpperBounds; + this.nullMarkedClasses = nullMarkedClasses; + } + + @Override + public String toString() { + return "ModelData{" + + "methodRecords=" + + methodRecords + + ", nullableUpperBounds=" + + nullableUpperBounds + + ", nullMarkedClasses=" + + nullMarkedClasses + + '}'; + } + } + /** * Parses all the source files within the directory using javaparser. * * @param inputSourceDirectory Directory containing annotated java source files. - * @param outputDirectory Directory to write the astubx file into. + * @param outputFile absolute path to the output file. */ - public void generateAstubxForLibraryModels(String inputSourceDirectory, String outputDirectory) { + public static LibraryModelData generateAstubxForLibraryModels( + String inputSourceDirectory, String outputFile) { Map methodRecords = new LinkedHashMap<>(); Set nullMarkedClasses = new HashSet<>(); Map> nullableUpperBounds = new LinkedHashMap<>(); Path root = dirnameToPath(inputSourceDirectory); - AnnotationCollectorCallback ac = - new AnnotationCollectorCallback(methodRecords, nullMarkedClasses, nullableUpperBounds); + LibraryModelData modelData = + new LibraryModelData(methodRecords, nullableUpperBounds, nullMarkedClasses); + AnnotationCollectorCallback ac = new AnnotationCollectorCallback(modelData); CollectionStrategy strategy = new ParserCollectionStrategy(); // Required to include directories that contain a module-info.java, which don't parse by // default. @@ -105,20 +138,20 @@ public void generateAstubxForLibraryModels(String inputSourceDirectory, String o throw new RuntimeException(e); } }); - writeToAstubx(outputDirectory, methodRecords, nullMarkedClasses, nullableUpperBounds); + writeToAstubx(outputFile, modelData); + return modelData; } /** * Writes the Nullability annotation information into the output directory as an astubx file. * - * @param outputPath Output Directory. - * @param methodRecords Map containing the collected Nullability annotation information. + * @param outputPath path to output astubx file. + * @param modelData ModelData instance containing the collected annotation information. */ - private void writeToAstubx( - String outputPath, - Map methodRecords, - Set nullMarkedClasses, - Map> nullableUpperBounds) { + private static void writeToAstubx(String outputPath, LibraryModelData modelData) { + Map methodRecords = modelData.methodRecords; + Map> nullableUpperBounds = modelData.nullableUpperBounds; + Set nullMarkedClasses = modelData.nullMarkedClasses; if (methodRecords.isEmpty() && nullableUpperBounds.isEmpty()) { return; } @@ -144,7 +177,7 @@ private void writeToAstubx( } } - public Path dirnameToPath(String dir) { + public static Path dirnameToPath(String dir) { File f = new File(dir); String absoluteDir = f.getAbsolutePath(); if (absoluteDir.endsWith("/.")) { @@ -157,12 +190,8 @@ private static class AnnotationCollectorCallback implements SourceRoot.Callback private final AnnotationCollectionVisitor annotationCollectionVisitor; - public AnnotationCollectorCallback( - Map methodRecords, - Set nullMarkedClasses, - Map> nullableUpperBounds) { - this.annotationCollectionVisitor = - new AnnotationCollectionVisitor(methodRecords, nullMarkedClasses, nullableUpperBounds); + public AnnotationCollectorCallback(LibraryModelData modelData) { + this.annotationCollectionVisitor = new AnnotationCollectionVisitor(modelData); } @Override @@ -191,13 +220,10 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter methodRecords, - Set nullMarkedClasses, - Map> nullableUpperBounds) { - this.methodRecords = methodRecords; - this.nullMarkedClasses = nullMarkedClasses; - this.nullableUpperBounds = nullableUpperBounds; + public AnnotationCollectionVisitor(LibraryModelData modelData) { + this.methodRecords = modelData.methodRecords; + this.nullableUpperBounds = modelData.nullableUpperBounds; + this.nullMarkedClasses = modelData.nullMarkedClasses; } @Override @@ -221,7 +247,11 @@ public void visit(ClassOrInterfaceDeclaration cid, Void arg) { 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 += "." + cid.getNameAsString(); + String oldParentName = parentName; + if (!parentName.isEmpty()) { + parentName += "."; + } + parentName += cid.getNameAsString(); cid.getAnnotations() .forEach( a -> { @@ -239,7 +269,7 @@ public void visit(ClassOrInterfaceDeclaration cid, Void arg) { } super.visit(cid, null); // We reset the variable that constructs the parent name after visiting all the children. - parentName = parentName.substring(0, parentName.lastIndexOf("." + cid.getNameAsString())); + parentName = oldParentName; } @Override From 515bb29f40b1bae50c216ebb24e563a070ebfd93 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Fri, 6 Sep 2024 23:40:43 -0700 Subject: [PATCH 09/34] updating test with fully qualified name --- .../com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index 25bbbe2091..9f359a4917 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -65,7 +65,7 @@ public void nullableReturn() throws IOException { }; ImmutableMap expectedMethodRecords = ImmutableMap.of( - "AnnotationExample:String makeUpperCase(String)", + "AnnotationExample:java.lang.String makeUpperCase(java.lang.String)", MethodAnnotationsRecord.create(ImmutableSet.of("Nullable"), ImmutableMap.of())); runTest("AnnotationExample.java", lines, expectedMethodRecords, ImmutableMap.of()); } From e1daf7b0ed5f7e9787863974f393313b00ace92e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 7 Sep 2024 09:41:48 -0700 Subject: [PATCH 10/34] fix another --- .../src/main/java/com/uber/nullaway/libmodel/StubxWriter.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java index ed507c680a..dd428f0e6f 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java @@ -56,7 +56,9 @@ public static void write( nullableUpperBounds.keySet()); for (Collection keyset : keysets) { for (String key : keyset) { - assert !encodingDictionary.containsKey(key); + if (encodingDictionary.containsKey(key)) { + continue; + } strings.add(key); encodingDictionary.put(key, numStringEntries); ++numStringEntries; From 0681d1f899cea7d42a29e4d9dd0bc94bbead1336 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 7 Sep 2024 09:46:24 -0700 Subject: [PATCH 11/34] latest javaparser --- gradle/dependencies.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index e71aa8e0fd..8f9e4794a4 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -52,7 +52,7 @@ def versions = [ commonscli : "1.4", autoValue : "1.10.2", autoService : "1.1.1", - javaparser : "3.26.1", + javaparser : "3.26.2", ] def apt = [ From 81ec20cb95c30b70e6ed432f4db4d1cb542f6d21 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Mon, 9 Sep 2024 15:30:38 -0700 Subject: [PATCH 12/34] adding logic and test for checking null marked classes --- .../libmodel/LibraryModelGenerator.java | 2 +- .../libmodel/LibraryModelGeneratorTest.java | 39 +++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index 583d6be1eb..d744aabf89 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -152,7 +152,7 @@ private static void writeToAstubx(String outputPath, LibraryModelData modelData) Map methodRecords = modelData.methodRecords; Map> nullableUpperBounds = modelData.nullableUpperBounds; Set nullMarkedClasses = modelData.nullMarkedClasses; - if (methodRecords.isEmpty() && nullableUpperBounds.isEmpty()) { + if (methodRecords.isEmpty() && nullableUpperBounds.isEmpty() && nullMarkedClasses.isEmpty()) { return; } ImmutableMap importedAnnotations = diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index 9f359a4917..590450cfc9 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -27,7 +27,8 @@ private void runTest( String sourceFileName, String[] lines, ImmutableMap expectedMethodRecords, - ImmutableMap> expectedNullableUpperBounds) + ImmutableMap> expectedNullableUpperBounds, + ImmutableSet expectedNullMarkedClasses) throws IOException { // write it to a source file in inputSourcesFolder with the right file name Files.write( @@ -43,6 +44,7 @@ private void runTest( Assert.assertTrue("astubx file was not created", Files.exists(Paths.get(astubxOutputPath))); assertThat(modelData.methodRecords, equalTo(expectedMethodRecords)); assertThat(modelData.nullableUpperBounds, equalTo(expectedNullableUpperBounds)); + assertThat(modelData.nullMarkedClasses, equalTo(expectedNullMarkedClasses)); } @Test @@ -67,7 +69,12 @@ public void nullableReturn() throws IOException { ImmutableMap.of( "AnnotationExample:java.lang.String makeUpperCase(java.lang.String)", MethodAnnotationsRecord.create(ImmutableSet.of("Nullable"), ImmutableMap.of())); - runTest("AnnotationExample.java", lines, expectedMethodRecords, ImmutableMap.of()); + runTest( + "AnnotationExample.java", + lines, + expectedMethodRecords, + ImmutableMap.of(), + ImmutableSet.of()); } @Test @@ -86,6 +93,32 @@ public void nullableUpperBound() throws IOException { }; ImmutableMap> expectedNullableUpperBounds = ImmutableMap.of("NullableUpperBound", ImmutableSet.of(0)); - runTest("NullableUpperBound.java", lines, ImmutableMap.of(), expectedNullableUpperBounds); + runTest( + "NullableUpperBound.java", + lines, + ImmutableMap.of(), + expectedNullableUpperBounds, + ImmutableSet.of()); + } + + @Test + public void nullMarkedClasses() throws IOException { + String[] lines = + new String[] { + "import org.jspecify.annotations.NullMarked;", + "@NullMarked", + "public class NullMarked{", + "//Only this class should appear under null marked classes", + "}", + "class NotNullMarked{", + "}" + }; + ImmutableSet expectedNullMarkedClasses = ImmutableSet.of("NullMarked"); + runTest( + "NullableUpperBound.java", + lines, + ImmutableMap.of(), + ImmutableMap.of(), + expectedNullMarkedClasses); } } From f61478d767dbd4392baff16a4cbf0b04be75e3a3 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Mon, 9 Sep 2024 17:51:10 -0700 Subject: [PATCH 13/34] updating test cases --- .../com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index 590450cfc9..946f30a170 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -74,7 +74,7 @@ public void nullableReturn() throws IOException { lines, expectedMethodRecords, ImmutableMap.of(), - ImmutableSet.of()); + ImmutableSet.of("AnnotationExample")); } @Test @@ -98,7 +98,7 @@ public void nullableUpperBound() throws IOException { lines, ImmutableMap.of(), expectedNullableUpperBounds, - ImmutableSet.of()); + ImmutableSet.of("NullableUpperBound")); } @Test From aaaa00a77d3d9a7a1832eb5dd9304d54d1aea717 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 10 Sep 2024 00:37:08 -0700 Subject: [PATCH 14/34] adding test case for nullable parameters --- .../libmodel/LibraryModelGeneratorTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index 946f30a170..742a63b14a 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -121,4 +121,34 @@ public void nullMarkedClasses() throws IOException { ImmutableMap.of(), expectedNullMarkedClasses); } + + @Test + public void nullableParameters() throws IOException { + String[] lines = + new String[] { + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.Nullable;", + "@NullMarked", + "public class NullableParameters{", + " public static Object getNewObjectIfNull(@Nullable Object object) {", + " if (object == null) {", + " return new Object();", + " } else {", + " return object;", + " }", + " }", + "}" + }; + ImmutableMap expectedMethodRecords = + ImmutableMap.of( + "NullableParameters:java.lang.Object getNewObjectIfNull(java.lang.Object)", + MethodAnnotationsRecord.create( + ImmutableSet.of(), ImmutableMap.of(0, ImmutableSet.of("Nullable")))); + runTest( + "NullableParameters.java", + lines, + expectedMethodRecords, + ImmutableMap.of(), + ImmutableSet.of("NullableParameters")); + } } From e5dc95f6fd877a2080dbd1b8589689ec7ec5d0a2 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 10 Sep 2024 16:22:24 -0700 Subject: [PATCH 15/34] removing unnecessary suppress warning --- .../java/com/uber/nullaway/handlers/LibraryModelsHandler.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index 005be99d98..1d3d444694 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -1287,9 +1287,7 @@ private NameIndexedMap makeOptimizedLookup( /** Constructs Library Models from stubx files */ private static class ExternalStubxLibraryModels implements LibraryModels { - @SuppressWarnings("unused") private final Map>>> argAnnotCache; - private final Set nullMarkedClassesCache; private final Map upperBoundsCache; From 3a5946b88e8960b1f228d9bda8ddf94080e367cf Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 10 Sep 2024 16:26:10 -0700 Subject: [PATCH 16/34] clean up --- .../uber/nullaway/libmodel/LibraryModelGeneratorTest.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index 742a63b14a..b274510cb5 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -108,18 +108,14 @@ public void nullMarkedClasses() throws IOException { "import org.jspecify.annotations.NullMarked;", "@NullMarked", "public class NullMarked{", - "//Only this class should appear under null marked classes", + " //Only this class should appear under null marked classes", "}", "class NotNullMarked{", "}" }; ImmutableSet expectedNullMarkedClasses = ImmutableSet.of("NullMarked"); runTest( - "NullableUpperBound.java", - lines, - ImmutableMap.of(), - ImmutableMap.of(), - expectedNullMarkedClasses); + "NullMarked.java", lines, ImmutableMap.of(), ImmutableMap.of(), expectedNullMarkedClasses); } @Test From d83e024a1e554f951e3221bed1e7a75b5653a72e Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 10 Sep 2024 21:50:21 -0700 Subject: [PATCH 17/34] adding check for Nullable annotation --- .../java/com/uber/nullaway/handlers/LibraryModelsHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index 1d3d444694..49aee7889b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -1331,7 +1331,7 @@ public ImmutableSetMultimap explicitlyNullableParameters() { String methodName = innerEntry.getKey().substring(innerEntry.getKey().indexOf(" ") + 1); for (Map.Entry> entry : innerEntry.getValue().entrySet()) { Integer index = entry.getKey(); - if (index >= 0) { + if (index >= 0 && entry.getValue().stream().anyMatch(a -> a.contains("Nullable"))) { mapBuilder.put(methodRef(className, methodName), index); } } From 32f6abbe85cea18a87d067c80a414acc232d860a Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Thu, 12 Sep 2024 14:46:49 -0700 Subject: [PATCH 18/34] adding test case for explicitly @NonNull parameter --- .../uber/nullaway/libmodel/LibraryModelIntegrationTest.java | 4 ++++ .../src/main/java/com/test/ParameterAnnotationExample.java | 4 ++++ .../src/com/test/ParameterAnnotationExample.java | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java index 25e5250418..fb34e7e69c 100644 --- a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java +++ b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java @@ -222,6 +222,10 @@ public void libraryModelParameterNullabilityTest() { "import com.test.ParameterAnnotationExample;", "class Test {", " static ParameterAnnotationExample annotationExample = new ParameterAnnotationExample();", + " static void testPositive() {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " annotationExample.printObjectString(null);", + " }", " static void testNegative() {", " annotationExample.getNewObjectIfNull(null);", " }", diff --git a/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java b/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java index dbd0ff8538..0b043216b5 100644 --- a/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java +++ b/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java @@ -19,4 +19,8 @@ public static Object getNewObjectIfNull(Object object) { return object; } } + + public static void printObjectString(Object object) { + System.out.println(object.toString()); + } } diff --git a/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java index 672ec6371a..44f8df322e 100644 --- a/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java +++ b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java @@ -1,5 +1,6 @@ package com.test; +import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -17,4 +18,8 @@ public static Object getNewObjectIfNull(@Nullable Object object) { return object; } } + + public static void printObjectString(@NonNull Object object) { + System.out.println(object.toString()); + } } From b0a25f48cade3efd061dcf0bf6448a39c9339458 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 17 Sep 2024 15:48:36 -0700 Subject: [PATCH 19/34] updating logic for using Array type qualified names --- .../libmodel/LibraryModelGenerator.java | 5 +- .../libmodel/LibraryModelGeneratorTest.java | 60 +++++++++++++++++++ .../handlers/InferredJARModelsHandler.java | 2 +- 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index d744aabf89..3da2d7fc8c 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -214,8 +214,6 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter methodRecords; private final Set nullMarkedClasses; private final Map> nullableUpperBounds; - // TODO this is sketchy! - 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"; @@ -334,7 +332,8 @@ private String getMethodReturnTypeString(ResolvedMethodDeclaration md) { if (returnType.isReferenceType()) { return returnType.asReferenceType().getQualifiedName().toString(); } else if (returnType.isArray()) { - return ARRAY_RETURN_TYPE_STRING; + return returnType.asArrayType().getComponentType().asReferenceType().getQualifiedName() + + "[]"; } else { throw new RuntimeException("Unexpected return type: " + returnType); // return returnType.asPrimitive().toString(); diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index b274510cb5..d3a74e8c00 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -147,4 +147,64 @@ public void nullableParameters() throws IOException { ImmutableMap.of(), ImmutableSet.of("NullableParameters")); } + + @Test + public void nullableParametersInNullUnmarkedClass() throws IOException { + String[] lines = + new String[] { + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.Nullable;", + "@NullMarked", + "public class NullableParameters{", + " public static Object getNewObjectIfNull(@Nullable Object object) {", + " if (object == null) {", + " return new Object();", + " } else {", + " return object;", + " }", + " }", + "}" + }; + ImmutableMap expectedMethodRecords = + ImmutableMap.of( + "NullableParameters:java.lang.Object getNewObjectIfNull(java.lang.Object)", + MethodAnnotationsRecord.create( + ImmutableSet.of(), ImmutableMap.of(0, ImmutableSet.of("Nullable")))); + runTest( + "NullableParameters.java", + lines, + expectedMethodRecords, + ImmutableMap.of(), + ImmutableSet.of("NullableParameters")); + } + + @Test + public void nullableArrayTypeParameter() throws IOException { + String[] lines = + new String[] { + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.Nullable;", + "@NullMarked", + "public class NullableParameters{", + " public static Object[] getNewObjectArrayIfNull(@Nullable Object[] objectArray) {", + " if (objectArray == null) {", + " return new Object[10];", + " } else {", + " return objectArray;", + " }", + " }", + "}" + }; + ImmutableMap expectedMethodRecords = + ImmutableMap.of( + "NullableParameters:java.lang.Object[] getNewObjectArrayIfNull(java.lang.Object[])", + MethodAnnotationsRecord.create( + ImmutableSet.of(), ImmutableMap.of(0, ImmutableSet.of("Nullable")))); + runTest( + "NullableParameters.java", + lines, + expectedMethodRecords, + ImmutableMap.of(), + ImmutableSet.of("NullableParameters")); + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java index c4aba6c0be..6ecb8ffca2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -256,7 +256,7 @@ private String getSimpleTypeName(Type typ) { if (typ.getKind() == TypeKind.TYPEVAR) { return typ.getUpperBound().tsym.getQualifiedName().toString(); } else { - return typ.tsym.getQualifiedName().toString(); + return typ.toString(); } } } From 254ada42c8fc12103ff0f534d5a524db33ad27db Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 17 Sep 2024 16:16:57 -0700 Subject: [PATCH 20/34] updating logic for correctly taking the annotation for @Nullable array elements --- .../uber/nullaway/libmodel/LibraryModelGenerator.java | 9 ++++++++- .../nullaway/libmodel/LibraryModelGeneratorTest.java | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index 3da2d7fc8c..08e91aeda9 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -383,7 +383,14 @@ private ImmutableMap> getNullableParameters( List parameterList = md.getParameters(); for (int i = 0; i < parameterList.size(); i++) { Parameter parameter = parameterList.get(i); - Optional nullableAnnotation = parameter.getAnnotationByName(NULLABLE); + Optional nullableAnnotation; + // For ArrayTypes the annotation is on the type instead of the node when the elements inside + // the Array can be @Nullable for e.g. Object @Nullable [] + if (parameter.getType() instanceof ArrayType) { + nullableAnnotation = ((ArrayType) parameter.getType()).getAnnotationByName(NULLABLE); + } else { + nullableAnnotation = parameter.getAnnotationByName(NULLABLE); + } if (nullableAnnotation.isPresent() && isAnnotationNullable(nullableAnnotation.get())) { mapBuilder.put(i, ImmutableSet.of("Nullable")); } diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index d3a74e8c00..4065a42d8f 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -186,7 +186,7 @@ public void nullableArrayTypeParameter() throws IOException { "import org.jspecify.annotations.Nullable;", "@NullMarked", "public class NullableParameters{", - " public static Object[] getNewObjectArrayIfNull(@Nullable Object[] objectArray) {", + " public static Object[] getNewObjectArrayIfNull(Object @Nullable [] objectArray) {", " if (objectArray == null) {", " return new Object[10];", " } else {", From 10ddcd3d07424f4e67de2095f2339b6d560b8c95 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 17 Sep 2024 16:40:49 -0700 Subject: [PATCH 21/34] updating test case logic --- .../com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index 4065a42d8f..c260a36438 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -187,8 +187,8 @@ public void nullableArrayTypeParameter() throws IOException { "@NullMarked", "public class NullableParameters{", " public static Object[] getNewObjectArrayIfNull(Object @Nullable [] objectArray) {", - " if (objectArray == null) {", - " return new Object[10];", + " if (Arrays.stream(objectArray).allMatch(e -> e == null)) {", + " return new Object[]{new Object(),new Object()};", " } else {", " return objectArray;", " }", From c3f669cd988de46f6b6daef806dde2a28778122f Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 17 Sep 2024 20:13:13 -0700 Subject: [PATCH 22/34] handling primitive return type scenario --- .../libmodel/LibraryModelGenerator.java | 7 +++-- .../libmodel/LibraryModelGeneratorTest.java | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index 08e91aeda9..0819ae6536 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -37,6 +37,7 @@ import com.github.javaparser.ast.visitor.VoidVisitorAdapter; import com.github.javaparser.resolution.TypeSolver; import com.github.javaparser.resolution.declarations.ResolvedMethodDeclaration; +import com.github.javaparser.resolution.types.ResolvedPrimitiveType; import com.github.javaparser.resolution.types.ResolvedType; import com.github.javaparser.symbolsolver.JavaSymbolSolver; import com.github.javaparser.symbolsolver.resolution.typesolvers.CombinedTypeSolver; @@ -58,6 +59,7 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -330,13 +332,14 @@ private boolean hasNullableReturn(MethodDeclaration md) { private String getMethodReturnTypeString(ResolvedMethodDeclaration md) { ResolvedType returnType = md.getReturnType(); if (returnType.isReferenceType()) { - return returnType.asReferenceType().getQualifiedName().toString(); + return returnType.asReferenceType().getQualifiedName(); } else if (returnType.isArray()) { return returnType.asArrayType().getComponentType().asReferenceType().getQualifiedName() + "[]"; + } else if (returnType.isPrimitive()) { + return ((ResolvedPrimitiveType) returnType).name().toLowerCase(Locale.ROOT); } else { throw new RuntimeException("Unexpected return type: " + returnType); - // return returnType.asPrimitive().toString(); } } diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index c260a36438..e47956cd74 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -207,4 +207,33 @@ public void nullableArrayTypeParameter() throws IOException { ImmutableMap.of(), ImmutableSet.of("NullableParameters")); } + + @Test + public void primitiveTypeReturn() throws IOException { + String[] lines = + new String[] { + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.Nullable;", + "@NullMarked", + "public class PrimitiveType {", + " public int multiply(@Nullable Integer num1, @Nullable Integer num2) {", + " if(num1!=null && num2!=null){", + " return num1*num2;", + " }", + " }", + "}" + }; + ImmutableMap expectedMethodRecords = + ImmutableMap.of( + "PrimitiveType:int multiply(java.lang.Integer, java.lang.Integer)", + MethodAnnotationsRecord.create( + ImmutableSet.of(), + ImmutableMap.of(0, ImmutableSet.of("Nullable"), 1, ImmutableSet.of("Nullable")))); + runTest( + "PrimitiveType.java", + lines, + expectedMethodRecords, + ImmutableMap.of(), + ImmutableSet.of("PrimitiveType")); + } } From 8600bf970e155118ae4a4d9aaa8775d648bfb294 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Thu, 19 Sep 2024 14:43:28 -0700 Subject: [PATCH 23/34] adding test cases for JarInfer --- .../uber/nullaway/jarinfer/JarInferTest.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java index 02efb7cb85..e1fe8495d0 100644 --- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java +++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java @@ -427,6 +427,40 @@ public void toyReassigningTest() throws Exception { "}"); } + @Test + public void testObjectArray() throws Exception { + testTemplate( + "testObjectArray", + "arrays", + "TestArray", + ImmutableMap.of( + "arrays.TestArray:java.lang.String foo(java.lang.Object[])", Sets.newHashSet(0)), + "class TestArray {", + " public static String foo(Object[] o) {", + " return o.toString();", + " }", + "}"); + } + + @Test + public void testGenericMethod() throws Exception { + testTemplate( + "testGenericMethod", + "generic", + "TestGeneric", + ImmutableMap.of( + "generic.TestGeneric:java.lang.String foo(java.lang.Object)", Sets.newHashSet(1)), + "public class TestGeneric {", + " public String foo(T t) {", + " return t.toString();", + " }", + " public static void main(String arg[]) {", + " TestGeneric tg = new TestGeneric();", + " System.out.println(tg.foo(\"generic test\"));", + " }", + "}"); + } + @Test public void toyJARAnnotatingClasses() throws Exception { testAnnotationInJarTemplate( From eb832dc1c58904f3ff57ca9a4ddfa2d909c516c3 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 20 Sep 2024 15:59:30 -0700 Subject: [PATCH 24/34] test case --- .../jarinfer/JarInferIntegrationTest.java | 24 +++++++++++++++++++ .../jarinfer/toys/unannotated/Toys.java | 4 ++++ 2 files changed, 28 insertions(+) diff --git a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java index 786e0031d6..be2560233d 100644 --- a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java +++ b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java @@ -43,6 +43,30 @@ public void jarinferLoadStubsTest() { .doTest(); } + @Test + public void arrayTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:JarInferEnabled=true", + "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import com.uber.nullaway.jarinfer.toys.unannotated.Toys;", + "class Test {", + " void test1(Object @Nullable [] o) {", + " // BUG: Diagnostic contains: passing @Nullable parameter 's'", + " Toys.testArray(o);", + " }", + "}") + .doTest(); + } + @Test public void jarinferNullableReturnsTest() { compilationHelper diff --git a/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java b/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java index 62212693cb..7ca85cc3ca 100644 --- a/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java +++ b/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java @@ -32,6 +32,10 @@ public static void test1(@ExpectNonnull String s, String t, String u) { } } + public static int testArray(Object[] o) { + return o.length; + } + public static void main(String arg[]) throws java.io.IOException { String s = "test string..."; Foo f = new Foo("let's"); From 0b898b957bb2f6999744d928bc4544d9fa19db89 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 20 Sep 2024 16:05:13 -0700 Subject: [PATCH 25/34] tweak test --- .../com/uber/nullaway/jarinfer/JarInferIntegrationTest.java | 2 +- .../java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java index be2560233d..2d17d5d643 100644 --- a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java +++ b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java @@ -60,7 +60,7 @@ public void arrayTest() { "import com.uber.nullaway.jarinfer.toys.unannotated.Toys;", "class Test {", " void test1(Object @Nullable [] o) {", - " // BUG: Diagnostic contains: passing @Nullable parameter 's'", + " // BUG: Diagnostic contains: passing @Nullable parameter 'o'", " Toys.testArray(o);", " }", "}") diff --git a/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java b/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java index 7ca85cc3ca..c625b09edd 100644 --- a/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java +++ b/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java @@ -32,8 +32,9 @@ public static void test1(@ExpectNonnull String s, String t, String u) { } } + @SuppressWarnings("ArrayHashCode") public static int testArray(Object[] o) { - return o.length; + return o.hashCode(); } public static void main(String arg[]) throws java.io.IOException { From 6c1cea84ecf77bea31df291e04ab34335815a6b3 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 20 Sep 2024 16:20:50 -0700 Subject: [PATCH 26/34] another test --- .../jarinfer/JarInferIntegrationTest.java | 27 +++++++++++++++++++ .../jarinfer/toys/unannotated/Toys.java | 6 +++++ 2 files changed, 33 insertions(+) diff --git a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java index 2d17d5d643..78ecd7eccd 100644 --- a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java +++ b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java @@ -4,6 +4,7 @@ import com.uber.nullaway.NullAway; import java.util.Arrays; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -67,6 +68,32 @@ public void arrayTest() { .doTest(); } + @Ignore("TODO: support JarInfer and generics") + @Test + public void genericsTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:JarInferEnabled=true", + "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import com.uber.nullaway.jarinfer.toys.unannotated.Toys;", + "class Test {", + " void test1() {", + " Toys.Generic g = new Toys.Generic<>();", + " // BUG: Diagnostic contains: passing @Nullable parameter 'g.getString(null)'", + " g.getString(null);", + " }", + "}") + .doTest(); + } + @Test public void jarinferNullableReturnsTest() { compilationHelper diff --git a/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java b/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java index c625b09edd..1c781faa52 100644 --- a/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java +++ b/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java @@ -37,6 +37,12 @@ public static int testArray(Object[] o) { return o.hashCode(); } + public static class Generic { + public String getString(T t) { + return t.toString(); + } + } + public static void main(String arg[]) throws java.io.IOException { String s = "test string..."; Foo f = new Foo("let's"); From 55dc4e2a86f267d2f527ebacd1d5ba0aafef6c62 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 20 Sep 2024 16:26:19 -0700 Subject: [PATCH 27/34] fix test? --- .../com/uber/nullaway/jarinfer/JarInferIntegrationTest.java | 4 +--- .../com/uber/nullaway/handlers/InferredJARModelsHandler.java | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java index 78ecd7eccd..f77ae54ae3 100644 --- a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java +++ b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java @@ -4,7 +4,6 @@ import com.uber.nullaway.NullAway; import java.util.Arrays; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -68,7 +67,6 @@ public void arrayTest() { .doTest(); } - @Ignore("TODO: support JarInfer and generics") @Test public void genericsTest() { compilationHelper @@ -87,7 +85,7 @@ public void genericsTest() { "class Test {", " void test1() {", " Toys.Generic g = new Toys.Generic<>();", - " // BUG: Diagnostic contains: passing @Nullable parameter 'g.getString(null)'", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null'", " g.getString(null);", " }", "}") diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java index 6ecb8ffca2..88f474205e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -243,7 +243,7 @@ private String getMethodSignature(Symbol.MethodSymbol method) { + "("; if (!method.getParameters().isEmpty()) { for (Symbol.VarSymbol var : method.getParameters()) { - methodSign += var.type.toString() + ", "; + methodSign += getSimpleTypeName(var.type) + ", "; } methodSign = methodSign.substring(0, methodSign.lastIndexOf(',')); } From 6bd3185aaf5b8c07fff27f74fc6c733f4e0284e9 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 20 Sep 2024 17:28:25 -0700 Subject: [PATCH 28/34] more tests and tweaks --- .../libmodel/LibraryModelIntegrationTest.java | 28 +++++++++++++++++++ .../libmodel/LibraryModelGenerator.java | 21 ++++++++------ .../libmodel/LibraryModelGeneratorTest.java | 26 +++++++++++++++++ .../com/test/ParameterAnnotationExample.java | 10 +++++++ .../com/test/ParameterAnnotationExample.java | 11 ++++++++ 5 files changed, 88 insertions(+), 8 deletions(-) diff --git a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java index fb34e7e69c..6d506f99b7 100644 --- a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java +++ b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java @@ -232,4 +232,32 @@ public void libraryModelParameterNullabilityTest() { "}") .doTest(); } + + @Test + public void genericParameterTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:JSpecifyMode=true", + "-XepOpt:NullAway:JarInferEnabled=true", + "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.test.ParameterAnnotationExample;", + "class Test {", + " static ParameterAnnotationExample.Generic ex = new ParameterAnnotationExample.Generic<>();", + " static void testPositive() {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " ex.printObjectString(null);", + " }", + " static void testNegative() {", + " ex.getString(null);", + " }", + "}") + .doTest(); + } } diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index 0819ae6536..82eae109e4 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -252,14 +252,19 @@ public void visit(ClassOrInterfaceDeclaration cid, Void arg) { parentName += "."; } parentName += cid.getNameAsString(); - cid.getAnnotations() - .forEach( - a -> { - if (a.getNameAsString().equalsIgnoreCase(NULL_MARKED)) { - this.isNullMarked = true; - this.nullMarkedClasses.add(parentName); - } - }); + if (this.isNullMarked) { + // nested class, and enclosing class is null marked + this.nullMarkedClasses.add(parentName); + } else { + cid.getAnnotations() + .forEach( + a -> { + if (a.getNameAsString().equalsIgnoreCase(NULL_MARKED)) { + this.isNullMarked = true; + this.nullMarkedClasses.add(parentName); + } + }); + } if (this.isNullMarked) { ImmutableSet nullableUpperBoundParams = getGenericTypeParameterNullableUpperBounds(cid); diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index e47956cd74..e6b42f3ce5 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -208,6 +208,32 @@ public void nullableArrayTypeParameter() throws IOException { ImmutableSet.of("NullableParameters")); } + @Test + public void genericParameter() throws IOException { + String[] lines = + new String[] { + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.Nullable;", + "@NullMarked", + "public class Generic {", + " public String getString(@Nullable T t) {", + " return t.toString();", + " }", + "}" + }; + ImmutableMap expectedMethodRecords = + ImmutableMap.of( + "Generic:java.lang.String getString(T)", + MethodAnnotationsRecord.create( + ImmutableSet.of(), ImmutableMap.of(0, ImmutableSet.of("Nullable")))); + runTest( + "Generic.java", + lines, + expectedMethodRecords, + ImmutableMap.of(), + ImmutableSet.of("Generic")); + } + @Test public void primitiveTypeReturn() throws IOException { String[] lines = diff --git a/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java b/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java index 0b043216b5..2ef532c8a5 100644 --- a/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java +++ b/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java @@ -23,4 +23,14 @@ public static Object getNewObjectIfNull(Object object) { public static void printObjectString(Object object) { System.out.println(object.toString()); } + + public static class Generic { + public String getString(T t) { + return t != null ? t.toString() : ""; + } + + public void printObjectString(T t) { + System.out.println(t.toString()); + } + } } diff --git a/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java index 44f8df322e..ad3f1665d9 100644 --- a/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java +++ b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java @@ -22,4 +22,15 @@ public static Object getNewObjectIfNull(@Nullable Object object) { public static void printObjectString(@NonNull Object object) { System.out.println(object.toString()); } + + public static class Generic { + + public String getString(@Nullable T t) { + return t != null ? t.toString() : ""; + } + + public void printObjectString(T t) { + System.out.println(t.toString()); + } + } } From 1e8ac4497f3dd337dc2601500497c7d20d72cf38 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 20 Sep 2024 17:42:37 -0700 Subject: [PATCH 29/34] fix tests --- .../libmodel/LibraryModelGeneratorTest.java | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index e6b42f3ce5..1c94d58912 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -41,10 +41,15 @@ private void runTest( LibraryModelGenerator.generateAstubxForLibraryModels( inputSourcesFolder.getRoot().getAbsolutePath(), astubxOutputPath); System.err.println("modelData: " + modelData); - Assert.assertTrue("astubx file was not created", Files.exists(Paths.get(astubxOutputPath))); assertThat(modelData.methodRecords, equalTo(expectedMethodRecords)); assertThat(modelData.nullableUpperBounds, equalTo(expectedNullableUpperBounds)); assertThat(modelData.nullMarkedClasses, equalTo(expectedNullMarkedClasses)); + Assert.assertTrue( + "astubx file was not created", + Files.exists(Paths.get(astubxOutputPath)) + || (expectedMethodRecords.isEmpty() + && expectedNullableUpperBounds.isEmpty() + && expectedNullMarkedClasses.isEmpty())); } @Test @@ -107,17 +112,34 @@ public void nullMarkedClasses() throws IOException { new String[] { "import org.jspecify.annotations.NullMarked;", "@NullMarked", - "public class NullMarked{", - " //Only this class should appear under null marked classes", + "public class NullMarked {", + " public static class Nested {}", "}", - "class NotNullMarked{", - "}" }; - ImmutableSet expectedNullMarkedClasses = ImmutableSet.of("NullMarked"); + ImmutableSet expectedNullMarkedClasses = + ImmutableSet.of("NullMarked", "NullMarked.Nested"); runTest( "NullMarked.java", lines, ImmutableMap.of(), ImmutableMap.of(), expectedNullMarkedClasses); } + @Test + public void noNullMarkedClasses() throws IOException { + String[] lines = + new String[] { + "import org.jspecify.annotations.NullMarked;", + "public class NotNullMarked {", + " public static class Nested {}", + "}", + }; + ImmutableSet expectedNullMarkedClasses = ImmutableSet.of(); + runTest( + "NotNullMarked.java", + lines, + ImmutableMap.of(), + ImmutableMap.of(), + expectedNullMarkedClasses); + } + @Test public void nullableParameters() throws IOException { String[] lines = From 16ac993d1443b468bbb17fa857c2551122e0492b Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 20 Sep 2024 18:12:28 -0700 Subject: [PATCH 30/34] bump astubx version number --- .../java/com/uber/nullaway/libmodel/StubxWriter.java | 8 +++++++- .../java/com/uber/nullaway/handlers/StubxCacheUtil.java | 9 +++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java index dd428f0e6f..3ab674652c 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java @@ -19,6 +19,12 @@ public final class StubxWriter { */ private static final int VERSION_0_FILE_MAGIC_NUMBER = 691458791; + /** + * The file magic number for version 1 .astubx files. It should be the first four bytes of any + * compatible .astubx file. + */ + private static final int VERSION_1_FILE_MAGIC_NUMBER = 481874642; + /** * This method writes the provided list of annotations to a DataOutputStream in the astubx format. * @@ -41,7 +47,7 @@ public static void write( Map> nullableUpperBounds) throws IOException { // File format version/magic number - out.writeInt(VERSION_0_FILE_MAGIC_NUMBER); + out.writeInt(VERSION_1_FILE_MAGIC_NUMBER); // Followed by the number of string dictionary entries int numStringEntries = 0; Map encodingDictionary = new LinkedHashMap<>(); diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java b/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java index 6f81318e96..8144f6cb49 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java @@ -44,7 +44,12 @@ */ public class StubxCacheUtil { - private static final int VERSION_0_FILE_MAGIC_NUMBER = 691458791; + /** + * The file magic number for version 1 .astubx files. It should be the first four bytes of any + * compatible .astubx file. + */ + private static final int VERSION_1_FILE_MAGIC_NUMBER = 481874642; + private boolean DEBUG = false; private String logCaller = ""; @@ -117,7 +122,7 @@ public void parseStubStream(InputStream stubxInputStream, String stubxLocation) String[] strings; DataInputStream in = new DataInputStream(stubxInputStream); // Read and check the magic version number - if (in.readInt() != VERSION_0_FILE_MAGIC_NUMBER) { + if (in.readInt() != VERSION_1_FILE_MAGIC_NUMBER) { throw new Error("Invalid file version/magic number for stubx file!" + stubxLocation); } // Read the number of strings in the string dictionary From f8fd56469205522d82ccdcef38f0f5ff0a740eea Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 20 Sep 2024 18:19:57 -0700 Subject: [PATCH 31/34] suppress --- .../src/main/java/com/uber/nullaway/libmodel/StubxWriter.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java index 3ab674652c..bc18dce2b7 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/StubxWriter.java @@ -15,8 +15,9 @@ public final class StubxWriter { /** * The file magic number for version 0 .astubx files. It should be the first four bytes of any - * compatible .astubx file. + * compatible .astubx file. Unused field, keeping for reference. */ + @SuppressWarnings("UnusedVariable") private static final int VERSION_0_FILE_MAGIC_NUMBER = 691458791; /** From d24267c15dc8d445115f3de8596ca80f0135f45b Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 21 Sep 2024 08:49:42 -0700 Subject: [PATCH 32/34] add integration test for nullable array parameter --- .../libmodel/LibraryModelIntegrationTest.java | 27 +++++++++++++++++++ .../libmodel/LibraryModelGenerator.java | 2 ++ .../com/test/ParameterAnnotationExample.java | 10 +++++++ .../com/test/ParameterAnnotationExample.java | 8 ++++++ 4 files changed, 47 insertions(+) diff --git a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java index 6d506f99b7..89dc76b332 100644 --- a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java +++ b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java @@ -233,6 +233,33 @@ public void libraryModelParameterNullabilityTest() { .doTest(); } + @Test + public void nullableArrayTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:JSpecifyMode=true", + "-XepOpt:NullAway:JarInferEnabled=true", + "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import com.test.ParameterAnnotationExample;", + "class Test {", + " static void testPositive() {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " ParameterAnnotationExample.takesNonNullArray(null);", + " }", + " static void testNegative() {", + " ParameterAnnotationExample.takesNullArray(null);", + " }", + "}") + .doTest(); + } + @Test public void genericParameterTest() { compilationHelper diff --git a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java index 82eae109e4..163dc27e25 100644 --- a/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java +++ b/library-model/library-model-generator/src/main/java/com/uber/nullaway/libmodel/LibraryModelGenerator.java @@ -343,6 +343,8 @@ private String getMethodReturnTypeString(ResolvedMethodDeclaration md) { + "[]"; } else if (returnType.isPrimitive()) { return ((ResolvedPrimitiveType) returnType).name().toLowerCase(Locale.ROOT); + } else if (returnType.isVoid()) { + return "void"; } else { throw new RuntimeException("Unexpected return type: " + returnType); } diff --git a/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java b/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java index 2ef532c8a5..8dc1968e50 100644 --- a/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java +++ b/library-model/test-library-model-generator/src/main/java/com/test/ParameterAnnotationExample.java @@ -24,6 +24,16 @@ public static void printObjectString(Object object) { System.out.println(object.toString()); } + @SuppressWarnings("ArrayToString") + public static void takesNullArray(Object[] objects) { + System.out.println(objects); + } + + @SuppressWarnings("ArrayToString") + public static void takesNonNullArray(Object[] objects) { + String unused = objects.toString(); + } + public static class Generic { public String getString(T t) { return t != null ? t.toString() : ""; diff --git a/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java index ad3f1665d9..d7337df9ad 100644 --- a/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java +++ b/library-model/test-library-model-generator/src/main/resources/sample_annotated/src/com/test/ParameterAnnotationExample.java @@ -23,6 +23,14 @@ public static void printObjectString(@NonNull Object object) { System.out.println(object.toString()); } + public static void takesNullArray(Object @Nullable [] objects) { + System.out.println(objects); + } + + public static void takesNonNullArray(Object[] objects) { + String unused = objects.toString(); + } + public static class Generic { public String getString(@Nullable T t) { From 29a902bdb4a8582b2d530b957d0a65ed0db35b0f Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sun, 22 Sep 2024 14:30:33 -0700 Subject: [PATCH 33/34] adding test case for void return type and updating the NullUnmarked test --- .../libmodel/LibraryModelGeneratorTest.java | 44 ++++++++++++++----- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index 1c94d58912..464674e477 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -176,8 +176,7 @@ public void nullableParametersInNullUnmarkedClass() throws IOException { new String[] { "import org.jspecify.annotations.NullMarked;", "import org.jspecify.annotations.Nullable;", - "@NullMarked", - "public class NullableParameters{", + "public class NullUnmarked{", " public static Object getNewObjectIfNull(@Nullable Object object) {", " if (object == null) {", " return new Object();", @@ -187,17 +186,9 @@ public void nullableParametersInNullUnmarkedClass() throws IOException { " }", "}" }; - ImmutableMap expectedMethodRecords = - ImmutableMap.of( - "NullableParameters:java.lang.Object getNewObjectIfNull(java.lang.Object)", - MethodAnnotationsRecord.create( - ImmutableSet.of(), ImmutableMap.of(0, ImmutableSet.of("Nullable")))); + ImmutableMap expectedMethodRecords = ImmutableMap.of(); runTest( - "NullableParameters.java", - lines, - expectedMethodRecords, - ImmutableMap.of(), - ImmutableSet.of("NullableParameters")); + "NullUnmarked.java", lines, expectedMethodRecords, ImmutableMap.of(), ImmutableSet.of()); } @Test @@ -284,4 +275,33 @@ public void primitiveTypeReturn() throws IOException { ImmutableMap.of(), ImmutableSet.of("PrimitiveType")); } + + @Test + public void voidReturn() throws IOException { + String[] lines = + new String[] { + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.Nullable;", + "@NullMarked", + "public class VoidReturn {", + " public void printMultiply(@Nullable Integer num1, @Nullable Integer num2) {", + " if(num1!=null && num2!=null){", + " System.out.println(num1*num2);", + " }", + " }", + "}" + }; + ImmutableMap expectedMethodRecords = + ImmutableMap.of( + "VoidReturn:void printMultiply(java.lang.Integer, java.lang.Integer)", + MethodAnnotationsRecord.create( + ImmutableSet.of(), + ImmutableMap.of(0, ImmutableSet.of("Nullable"), 1, ImmutableSet.of("Nullable")))); + runTest( + "VoidReturn.java", + lines, + expectedMethodRecords, + ImmutableMap.of(), + ImmutableSet.of("VoidReturn")); + } } From 5a4e7cd1a26d5887ff0e34988e1592effd2f6d2e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 22 Sep 2024 15:14:18 -0700 Subject: [PATCH 34/34] remove unnecessary locals --- .../nullaway/libmodel/LibraryModelGeneratorTest.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java index 464674e477..2990a051a8 100644 --- a/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java +++ b/library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/LibraryModelGeneratorTest.java @@ -131,13 +131,7 @@ public void noNullMarkedClasses() throws IOException { " public static class Nested {}", "}", }; - ImmutableSet expectedNullMarkedClasses = ImmutableSet.of(); - runTest( - "NotNullMarked.java", - lines, - ImmutableMap.of(), - ImmutableMap.of(), - expectedNullMarkedClasses); + runTest("NotNullMarked.java", lines, ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()); } @Test @@ -186,9 +180,7 @@ public void nullableParametersInNullUnmarkedClass() throws IOException { " }", "}" }; - ImmutableMap expectedMethodRecords = ImmutableMap.of(); - runTest( - "NullUnmarked.java", lines, expectedMethodRecords, ImmutableMap.of(), ImmutableSet.of()); + runTest("NullUnmarked.java", lines, ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()); } @Test