From 628683bd6b8a470ad4fd0d4365e9cb961e22a6e7 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sat, 13 Apr 2024 20:10:29 +0530 Subject: [PATCH 01/15] adding generic type parameter nullability upper bound support for external library models --- .../build.gradle | 1 + .../libmodel/LibraryModelIntegrationTest.java | 28 ++++ .../libmodel/LibraryModelGenerator.java | 39 +++++ .../nullaway/libmodel/AnnotationExample.java | 7 + .../nullaway/libmodel/AnnotationExample.java | 7 + .../handlers/InferredJARModelsHandler.java | 106 +------------ .../handlers/LibraryModelsHandler.java | 84 ++++++++++ .../nullaway/handlers/StubxCacheUtil.java | 149 ++++++++++++++++++ 8 files changed, 319 insertions(+), 102 deletions(-) create mode 100644 nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java diff --git a/library-model/library-model-generator-integration-test/build.gradle b/library-model/library-model-generator-integration-test/build.gradle index 664e2bc4f0..25fbcef159 100644 --- a/library-model/library-model-generator-integration-test/build.gradle +++ b/library-model/library-model-generator-integration-test/build.gradle @@ -22,6 +22,7 @@ dependencies { testImplementation project(":nullaway") testImplementation project(":library-model:test-library-model-generator") testImplementation deps.test.junit4 + testImplementation deps.build.jspecify testImplementation(deps.build.errorProneTestHelpers) { exclude group: "junit", module: "junit" } 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 c0149fd78f..97434bf953 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 @@ -123,4 +123,32 @@ public void libraryModelInnerClassNullableReturnsTest() { "}") .doTest(); } + + @Test + public void libraryModelInnerClassNullableUpperBoundsTest() { + 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 org.jspecify.annotations.Nullable;", + "import com.uber.nullaway.libmodel.AnnotationExample;", + "class Test {", + " static AnnotationExample.GenericExample<@Nullable Object> genericExample = new AnnotationExample.GenericExample<@Nullable Object>();", + " static void test(Object value){", + " }", + " static void testPositive() {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'genericExample.getNull()'", + " test(genericExample.getNull());", + " }", + "}") + .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 306a36611f..acab7e3bfb 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 @@ -32,6 +32,7 @@ import com.github.javaparser.ast.expr.AnnotationExpr; import com.github.javaparser.ast.type.ArrayType; 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.utils.CollectionStrategy; import com.github.javaparser.utils.ParserCollectionStrategy; @@ -47,6 +48,7 @@ import java.nio.file.Paths; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Optional; @@ -163,6 +165,8 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter methodRecords) { this.methodRecords = methodRecords; @@ -197,6 +201,17 @@ public void visit(ClassOrInterfaceDeclaration cid, Void arg) { this.isNullMarked = true; } }); + ImmutableMap> genericParamAnnotationsMap = + getGenericTypeParameterNullableUpperBounds(cid); + /* + We insert a specialized MethodAnnotationsRecord object to store the generic type parameter nullability + information for the class by using an identifier that can never be an actual method name. + */ + if (this.isNullMarked && !genericParamAnnotationsMap.isEmpty()) { + methodRecords.put( + parentName + ":" + GENERIC_TYPE_IDENTIFIER, + MethodAnnotationsRecord.create(ImmutableSet.of(), genericParamAnnotationsMap)); + } 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())); @@ -261,5 +276,29 @@ private boolean isAnnotationNullable(AnnotationExpr annotation) { && this.isJspecifyNullableImportPresent) || annotation.getNameAsString().equalsIgnoreCase(JSPECIFY_NULLABLE_IMPORT); } + + /** + * Takes a ClassOrInterfaceDeclaration instance and returns a Map of the indexes and a set of + * annotations for generic type parameters with Nullable upper bounds. + * + * @param cid ClassOrInterfaceDeclaration instance. + * @return Map of annotations for generic type parameters with Nullable upper bounds. + */ + private ImmutableMap> getGenericTypeParameterNullableUpperBounds( + ClassOrInterfaceDeclaration cid) { + ImmutableMap> genericParamAnnotationsMap; + ImmutableMap.Builder> mapBuilder = ImmutableMap.builder(); + List typeParamList = cid.getTypeParameters(); + for (int i = 0; i < typeParamList.size(); i++) { + TypeParameter param = typeParamList.get(i); + for (ClassOrInterfaceType type : param.getTypeBound()) { + if (type.isAnnotationPresent(NULLABLE)) { + mapBuilder.put(i, ImmutableSet.of(NULLABLE)); + } + } + } + genericParamAnnotationsMap = mapBuilder.build(); + return genericParamAnnotationsMap; + } } } 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 ce8405fe44..24b4ce228a 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 @@ -39,4 +39,11 @@ public String returnNull() { return null; } } + + public static class GenericExample { + + public T getNull() { + return null; + } + } } 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 f95c54090a..5c93e5f3cd 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 @@ -45,4 +45,11 @@ public String returnNull() { return null; } } + + public static class GenericExample { + + public T getNull() { + return 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 495ac46334..9c75ff8f59 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -34,14 +34,10 @@ import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; -import com.uber.nullaway.jarinfer.JarInferStubxProvider; -import java.io.DataInputStream; -import java.io.IOException; import java.io.InputStream; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; -import java.util.ServiceLoader; import java.util.Set; import javax.annotation.Nullable; import javax.lang.model.element.Modifier; @@ -59,7 +55,6 @@ private static void LOG(boolean cond, String tag, String msg) { } } - private static final int VERSION_0_FILE_MAGIC_NUMBER = 691458791; private static final String ANDROID_ASTUBX_LOCATION = "jarinfer.astubx"; private static final String ANDROID_MODEL_CLASS = "com.uber.nullaway.jarinfer.AndroidJarInferModels"; @@ -69,12 +64,14 @@ private static void LOG(boolean cond, String tag, String msg) { private final Map>>> argAnnotCache; private final Config config; + private final StubxCacheUtil cacheUtil; public InferredJARModelsHandler(Config config) { super(); this.config = config; + this.cacheUtil = new StubxCacheUtil("JI"); argAnnotCache = new LinkedHashMap<>(); - loadStubxFiles(); + cacheUtil.loadStubxFiles(); // Load Android SDK JarInfer models try { InputStream androidStubxIS = @@ -82,7 +79,7 @@ public InferredJARModelsHandler(Config config) { .getClassLoader() .getResourceAsStream(ANDROID_ASTUBX_LOCATION); if (androidStubxIS != null) { - parseStubStream(androidStubxIS, "android.jar: " + ANDROID_ASTUBX_LOCATION); + cacheUtil.parseStubStream(androidStubxIS, "android.jar: " + ANDROID_ASTUBX_LOCATION); LOG(DEBUG, "DEBUG", "Loaded Android RT models."); } } catch (ClassNotFoundException e) { @@ -97,29 +94,6 @@ public InferredJARModelsHandler(Config config) { } } - /** - * Loads all stubx files discovered in the classpath. Stubx files are discovered via - * implementations of {@link JarInferStubxProvider} loaded using a {@link ServiceLoader} - */ - private void loadStubxFiles() { - Iterable astubxProviders = - ServiceLoader.load( - JarInferStubxProvider.class, InferredJARModelsHandler.class.getClassLoader()); - for (JarInferStubxProvider provider : astubxProviders) { - for (String astubxPath : provider.pathsToStubxFiles()) { - Class providerClass = provider.getClass(); - InputStream stubxInputStream = providerClass.getResourceAsStream(astubxPath); - String stubxLocation = providerClass + ":" + astubxPath; - try { - parseStubStream(stubxInputStream, stubxLocation); - LOG(DEBUG, "DEBUG", "loaded stubx file " + stubxLocation); - } catch (IOException e) { - throw new RuntimeException("could not parse stubx file " + stubxLocation, e); - } - } - } - } - @Override public Nullness[] onOverrideMethodInvocationParametersNullability( Context context, @@ -286,76 +260,4 @@ private String getSimpleTypeName(Type typ) { return typ.tsym.getSimpleName().toString(); } } - - private void parseStubStream(InputStream stubxInputStream, String stubxLocation) - throws IOException { - String[] strings; - DataInputStream in = new DataInputStream(stubxInputStream); - // Read and check the magic version number - if (in.readInt() != VERSION_0_FILE_MAGIC_NUMBER) { - throw new Error("Invalid file version/magic number for stubx file!" + stubxLocation); - } - // Read the number of strings in the string dictionary - int numStrings = in.readInt(); - // Populate the string dictionary {idx => value}, where idx is encoded by the string position - // inside this section. - strings = new String[numStrings]; - for (int i = 0; i < numStrings; ++i) { - strings[i] = in.readUTF(); - } - // Read the number of (package, annotation) entries - int numPackages = in.readInt(); - // Read each (package, annotation) entry, where the int values point into the string - // dictionary loaded before. - for (int i = 0; i < numPackages; ++i) { - in.readInt(); // String packageName = strings[in.readInt()]; - in.readInt(); // String annotation = strings[in.readInt()]; - } - // Read the number of (type, annotation) entries - int numTypes = in.readInt(); - // Read each (type, annotation) entry, where the int values point into the string - // dictionary loaded before. - for (int i = 0; i < numTypes; ++i) { - in.readInt(); // String typeName = strings[in.readInt()]; - in.readInt(); // String annotation = strings[in.readInt()]; - } - // Read the number of (method, annotation) entries - int numMethods = in.readInt(); - // Read each (method, annotation) record - for (int i = 0; i < numMethods; ++i) { - String methodSig = strings[in.readInt()]; - String annotation = strings[in.readInt()]; - LOG(DEBUG, "DEBUG", "method: " + methodSig + ", return annotation: " + annotation); - cacheAnnotation(methodSig, RETURN, annotation); - } - // Read the number of (method, argument, annotation) entries - int numArgumentRecords = in.readInt(); - // Read each (method, argument, annotation) record - for (int i = 0; i < numArgumentRecords; ++i) { - String methodSig = strings[in.readInt()]; - if (methodSig.lastIndexOf(':') == -1 || methodSig.split(":")[0].lastIndexOf('.') == -1) { - throw new Error( - "Invalid method signature " + methodSig + " in stubx file " + stubxLocation); - } - int argNum = in.readInt(); - String annotation = strings[in.readInt()]; - LOG( - DEBUG, - "DEBUG", - "method: " + methodSig + ", argNum: " + argNum + ", arg annotation: " + annotation); - cacheAnnotation(methodSig, argNum, annotation); - } - } - - private void cacheAnnotation(String methodSig, Integer argNum, String annotation) { - // TODO: handle inner classes properly - String className = methodSig.split(":")[0].replace('$', '.'); - Map>> cacheForClass = - argAnnotCache.computeIfAbsent(className, s -> new LinkedHashMap<>()); - Map> cacheForMethod = - cacheForClass.computeIfAbsent(methodSig, s -> new LinkedHashMap<>()); - Set cacheForArgument = - cacheForMethod.computeIfAbsent(argNum, s -> new LinkedHashSet<>()); - cacheForArgument.add(annotation); - } } 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 7b5676984c..796fec162a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -378,6 +378,9 @@ private static LibraryModels loadLibraryModels(Config config) { ServiceLoader.load(LibraryModels.class, LibraryModels.class.getClassLoader()); ImmutableSet.Builder libModelsBuilder = new ImmutableSet.Builder<>(); libModelsBuilder.add(new DefaultLibraryModels()).addAll(externalLibraryModels); + if (config.isJarInferEnabled()) { + libModelsBuilder.add(new ExternalStubxLibraryModels()); + } return new CombinedLibraryModels(libModelsBuilder.build(), config); } @@ -1281,4 +1284,85 @@ private static Symbol.MethodSymbol lookupHandlingOverrides( return null; } } + + /** Constructs Library Models from stubx files */ + private static class ExternalStubxLibraryModels implements LibraryModels { + + private static final Map>>> argAnnotCache; + + static { + argAnnotCache = new StubxCacheUtil("LM").loadStubxFiles(); + } + + @Override + public ImmutableSet nullMarkedClasses() { + Set cachedNullMarkedClasses = argAnnotCache.keySet(); + return new ImmutableSet.Builder().addAll(cachedNullMarkedClasses).build(); + } + + @Override + public ImmutableSetMultimap typeVariablesWithNullableUpperBounds() { + ImmutableSetMultimap.Builder mapBuilder = + new ImmutableSetMultimap.Builder<>(); + for (Map.Entry>>> entry : + argAnnotCache.entrySet()) { + String className = entry.getKey(); + Map>> innerMap = entry.getValue(); + String generic_key = className + ":-1_GENERIC_TYPE"; + if (innerMap.containsKey(generic_key)) { + Map> innerInnerMap = innerMap.get(generic_key); + for (Map.Entry> innerEntry : innerInnerMap.entrySet()) { + Integer innerInnerKey = innerEntry.getKey(); + mapBuilder.put(className, innerInnerKey); + } + } + } + return mapBuilder.build(); + } + + @Override + public ImmutableSetMultimap failIfNullParameters() { + return ImmutableSetMultimap.of(); + } + + @Override + public ImmutableSetMultimap explicitlyNullableParameters() { + return ImmutableSetMultimap.of(); + } + + @Override + public ImmutableSetMultimap nonNullParameters() { + return ImmutableSetMultimap.of(); + } + + @Override + public ImmutableSetMultimap nullImpliesTrueParameters() { + return ImmutableSetMultimap.of(); + } + + @Override + public ImmutableSetMultimap nullImpliesFalseParameters() { + return ImmutableSetMultimap.of(); + } + + @Override + public ImmutableSetMultimap nullImpliesNullParameters() { + return ImmutableSetMultimap.of(); + } + + @Override + public ImmutableSet nullableReturns() { + return ImmutableSet.of(); + } + + @Override + public ImmutableSet nonNullReturns() { + return ImmutableSet.of(); + } + + @Override + public ImmutableSetMultimap castToNonNullMethods() { + return ImmutableSetMultimap.of(); + } + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java b/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java new file mode 100644 index 0000000000..c63d774dd3 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java @@ -0,0 +1,149 @@ +package com.uber.nullaway.handlers; + +/* + * Copyright (c) 2024 Uber Technologies, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +import com.uber.nullaway.jarinfer.JarInferStubxProvider; +import java.io.DataInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.ServiceLoader; +import java.util.Set; + +public class StubxCacheUtil { + + private static final int VERSION_0_FILE_MAGIC_NUMBER = 691458791; + private boolean DEBUG = false; + private String logCaller = ""; + + private void LOG(boolean cond, String tag, String msg) { + if (cond) { + System.out.println("[" + logCaller + " " + tag + "] " + msg); + } + } + + private static final int RETURN = -1; + private final Map>>> argAnnotCache; + + public StubxCacheUtil(String logCaller) { + argAnnotCache = new LinkedHashMap<>(); + this.logCaller = logCaller; + } + + /** + * Loads all stubx files discovered in the classpath. Stubx files are discovered via + * implementations of {@link JarInferStubxProvider} loaded using a {@link ServiceLoader} + */ + public Map>>> loadStubxFiles() { + Iterable astubxProviders = + ServiceLoader.load(JarInferStubxProvider.class, StubxCacheUtil.class.getClassLoader()); + for (JarInferStubxProvider provider : astubxProviders) { + for (String astubxPath : provider.pathsToStubxFiles()) { + Class providerClass = provider.getClass(); + InputStream stubxInputStream = providerClass.getResourceAsStream(astubxPath); + String stubxLocation = providerClass + ":" + astubxPath; + try { + parseStubStream(stubxInputStream, stubxLocation); + LOG(DEBUG, "DEBUG", "loaded stubx file " + stubxLocation); + } catch (IOException e) { + throw new RuntimeException("could not parse stubx file " + stubxLocation, e); + } + } + } + return argAnnotCache; + } + + public void parseStubStream(InputStream stubxInputStream, String stubxLocation) + throws IOException { + String[] strings; + DataInputStream in = new DataInputStream(stubxInputStream); + // Read and check the magic version number + if (in.readInt() != VERSION_0_FILE_MAGIC_NUMBER) { + throw new Error("Invalid file version/magic number for stubx file!" + stubxLocation); + } + // Read the number of strings in the string dictionary + int numStrings = in.readInt(); + // Populate the string dictionary {idx => value}, where idx is encoded by the string position + // inside this section. + strings = new String[numStrings]; + for (int i = 0; i < numStrings; ++i) { + strings[i] = in.readUTF(); + } + // Read the number of (package, annotation) entries + int numPackages = in.readInt(); + // Read each (package, annotation) entry, where the int values point into the string + // dictionary loaded before. + for (int i = 0; i < numPackages; ++i) { + in.readInt(); // String packageName = strings[in.readInt()]; + in.readInt(); // String annotation = strings[in.readInt()]; + } + // Read the number of (type, annotation) entries + int numTypes = in.readInt(); + // Read each (type, annotation) entry, where the int values point into the string + // dictionary loaded before. + for (int i = 0; i < numTypes; ++i) { + in.readInt(); // String typeName = strings[in.readInt()]; + in.readInt(); // String annotation = strings[in.readInt()]; + } + // Read the number of (method, annotation) entries + int numMethods = in.readInt(); + // Read each (method, annotation) record + for (int i = 0; i < numMethods; ++i) { + String methodSig = strings[in.readInt()]; + String annotation = strings[in.readInt()]; + LOG(DEBUG, "DEBUG", "method: " + methodSig + ", return annotation: " + annotation); + cacheAnnotation(methodSig, RETURN, annotation); + } + // Read the number of (method, argument, annotation) entries + int numArgumentRecords = in.readInt(); + // Read each (method, argument, annotation) record + for (int i = 0; i < numArgumentRecords; ++i) { + String methodSig = strings[in.readInt()]; + if (methodSig.lastIndexOf(':') == -1 || methodSig.split(":")[0].lastIndexOf('.') == -1) { + throw new Error( + "Invalid method signature " + methodSig + " in stubx file " + stubxLocation); + } + int argNum = in.readInt(); + String annotation = strings[in.readInt()]; + LOG( + DEBUG, + "DEBUG", + "method: " + methodSig + ", argNum: " + argNum + ", arg annotation: " + annotation); + cacheAnnotation(methodSig, argNum, annotation); + } + } + + private void cacheAnnotation(String methodSig, Integer argNum, String annotation) { + // TODO: handle inner classes properly + String className = methodSig.split(":")[0].replace('$', '.'); + Map>> cacheForClass = + argAnnotCache.computeIfAbsent(className, s -> new LinkedHashMap<>()); + Map> cacheForMethod = + cacheForClass.computeIfAbsent(methodSig, s -> new LinkedHashMap<>()); + Set cacheForArgument = + cacheForMethod.computeIfAbsent(argNum, s -> new LinkedHashSet<>()); + cacheForArgument.add(annotation); + } +} From 0c4d69429ade2574d08a9f90243f99b79ea87702 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sat, 13 Apr 2024 20:35:56 +0530 Subject: [PATCH 02/15] fix --- .../com/uber/nullaway/handlers/InferredJARModelsHandler.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 9c75ff8f59..a1f0085407 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -35,7 +35,6 @@ import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; import java.io.InputStream; -import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; @@ -70,8 +69,7 @@ public InferredJARModelsHandler(Config config) { super(); this.config = config; this.cacheUtil = new StubxCacheUtil("JI"); - argAnnotCache = new LinkedHashMap<>(); - cacheUtil.loadStubxFiles(); + argAnnotCache = cacheUtil.loadStubxFiles(); // Load Android SDK JarInfer models try { InputStream androidStubxIS = From ddaa68a89f5533f20e2672f5d90bfb5396c9ef3f Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sun, 14 Apr 2024 12:43:27 +0530 Subject: [PATCH 03/15] comment --- .../java/com/uber/nullaway/libmodel/LibraryModelGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 acab7e3bfb..279b7c838b 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 @@ -166,7 +166,7 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter methodRecords) { this.methodRecords = methodRecords; From d351cc95522f520d27e37f8e27c3934a6e0c89f0 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Mon, 15 Apr 2024 08:17:39 +0530 Subject: [PATCH 04/15] comment --- .../java/com/uber/nullaway/libmodel/LibraryModelGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 279b7c838b..93ca58a09b 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 @@ -166,7 +166,7 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter methodRecords) { this.methodRecords = methodRecords; From c3c37b0791d30460c95f24c78fce16bcc7228d15 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Mon, 3 Jun 2024 23:30:09 +0530 Subject: [PATCH 05/15] writing and reading nullable upper bounds from stubx file --- .../DefinitelyDerefedParamsDriver.java | 9 ++- .../libmodel/LibraryModelGenerator.java | 73 +++++++++---------- .../uber/nullaway/libmodel/StubxWriter.java | 18 ++++- .../handlers/InferredJARModelsHandler.java | 2 +- .../handlers/LibraryModelsHandler.java | 19 ++--- .../nullaway/handlers/StubxCacheUtil.java | 29 +++++++- 6 files changed, 94 insertions(+), 56 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 6616785dbc..2d5d195765 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 @@ -58,6 +58,7 @@ import java.nio.file.Paths; import java.nio.file.attribute.FileTime; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -449,7 +450,13 @@ private void writeModel(DataOutputStream out) throws IOException { nullableReturnMethodSign, MethodAnnotationsRecord.create(ImmutableSet.of("Nullable"), ImmutableMap.of())); } - StubxWriter.write(out, importedAnnotations, packageAnnotations, typeAnnotations, methodRecords); + StubxWriter.write( + out, + importedAnnotations, + packageAnnotations, + typeAnnotations, + methodRecords, + Collections.emptyMap()); } private void writeAnnotations(String inPath, String outFile) throws IOException { 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 93ca58a09b..a13d720152 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 @@ -51,6 +51,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; /** * Utilized for generating an astubx file from a directory containing annotated Java source code. @@ -61,21 +62,18 @@ */ public class LibraryModelGenerator { - public void generateAstubxForLibraryModels(String inputSourceDirectory, String outputDirectory) { - Map methodRecords = processDirectory(inputSourceDirectory); - writeToAstubx(outputDirectory, methodRecords); - } - /** * Parses all the source files within the directory using javaparser. * - * @param sourceDirectoryRoot Directory containing annotated java source files. - * @return a Map containing the Nullability annotation information from the source files. + * @param inputSourceDirectory Directory containing annotated java source files. + * @param outputDirectory Directory to write the astubx file into. */ - private Map processDirectory(String sourceDirectoryRoot) { + public void generateAstubxForLibraryModels(String inputSourceDirectory, String outputDirectory) { Map methodRecords = new LinkedHashMap<>(); - Path root = dirnameToPath(sourceDirectoryRoot); - AnnotationCollectorCallback ac = new AnnotationCollectorCallback(methodRecords); + Map> nullableUpperBounds = new LinkedHashMap<>(); + Path root = dirnameToPath(inputSourceDirectory); + AnnotationCollectorCallback ac = + new AnnotationCollectorCallback(methodRecords, nullableUpperBounds); CollectionStrategy strategy = new ParserCollectionStrategy(); // Required to include directories that contain a module-info.java, which don't parse by // default. @@ -92,7 +90,7 @@ private Map processDirectory(String sourceDirec throw new RuntimeException(e); } }); - return methodRecords; + writeToAstubx(outputDirectory, methodRecords, nullableUpperBounds); } /** @@ -102,8 +100,10 @@ private Map processDirectory(String sourceDirec * @param methodRecords Map containing the collected Nullability annotation information. */ private void writeToAstubx( - String outputPath, Map methodRecords) { - if (methodRecords.isEmpty()) { + String outputPath, + Map methodRecords, + Map> nullableUpperBounds) { + if (methodRecords.isEmpty() && nullableUpperBounds.isEmpty()) { return; } Map importedAnnotations = @@ -119,7 +119,8 @@ private void writeToAstubx( importedAnnotations, Collections.emptyMap(), Collections.emptyMap(), - methodRecords); + methodRecords, + nullableUpperBounds); } } catch (IOException e) { throw new RuntimeException(e); @@ -139,8 +140,11 @@ private static class AnnotationCollectorCallback implements SourceRoot.Callback private final AnnotationCollectionVisitor annotationCollectionVisitor; - public AnnotationCollectorCallback(Map methodRecords) { - this.annotationCollectionVisitor = new AnnotationCollectionVisitor(methodRecords); + public AnnotationCollectorCallback( + Map methodRecords, + Map> nullableUpperBounds) { + this.annotationCollectionVisitor = + new AnnotationCollectionVisitor(methodRecords, nullableUpperBounds); } @Override @@ -160,16 +164,18 @@ private static class AnnotationCollectionVisitor extends VoidVisitorAdapter methodRecords; + private final Map methodRecords; + private final Map> nullableUpperBounds; 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"; - private static final String GENERIC_TYPE_IDENTIFIER = - "-1_GENERIC_TYPE"; // A unique identifier to store generic type parameter nullability - public AnnotationCollectionVisitor(Map methodRecords) { + public AnnotationCollectionVisitor( + Map methodRecords, + Map> nullableUpperBounds) { this.methodRecords = methodRecords; + this.nullableUpperBounds = nullableUpperBounds; } @Override @@ -201,16 +207,11 @@ public void visit(ClassOrInterfaceDeclaration cid, Void arg) { this.isNullMarked = true; } }); - ImmutableMap> genericParamAnnotationsMap = - getGenericTypeParameterNullableUpperBounds(cid); - /* - We insert a specialized MethodAnnotationsRecord object to store the generic type parameter nullability - information for the class by using an identifier that can never be an actual method name. - */ - if (this.isNullMarked && !genericParamAnnotationsMap.isEmpty()) { - methodRecords.put( - parentName + ":" + GENERIC_TYPE_IDENTIFIER, - MethodAnnotationsRecord.create(ImmutableSet.of(), genericParamAnnotationsMap)); + if (this.isNullMarked) { + Set nullableUpperBoundParams = getGenericTypeParameterNullableUpperBounds(cid); + if (!nullableUpperBoundParams.isEmpty()) { + nullableUpperBounds.put(parentName, nullableUpperBoundParams); + } } super.visit(cid, null); // We reset the variable that constructs the parent name after visiting all the children. @@ -282,23 +283,21 @@ private boolean isAnnotationNullable(AnnotationExpr annotation) { * annotations for generic type parameters with Nullable upper bounds. * * @param cid ClassOrInterfaceDeclaration instance. - * @return Map of annotations for generic type parameters with Nullable upper bounds. + * @return Set of indices for generic type parameters with Nullable upper bounds. */ - private ImmutableMap> getGenericTypeParameterNullableUpperBounds( + private ImmutableSet getGenericTypeParameterNullableUpperBounds( ClassOrInterfaceDeclaration cid) { - ImmutableMap> genericParamAnnotationsMap; - ImmutableMap.Builder> mapBuilder = ImmutableMap.builder(); + ImmutableSet.Builder setBuilder = ImmutableSet.builder(); List typeParamList = cid.getTypeParameters(); for (int i = 0; i < typeParamList.size(); i++) { TypeParameter param = typeParamList.get(i); for (ClassOrInterfaceType type : param.getTypeBound()) { if (type.isAnnotationPresent(NULLABLE)) { - mapBuilder.put(i, ImmutableSet.of(NULLABLE)); + setBuilder.add(i); } } } - genericParamAnnotationsMap = mapBuilder.build(); - return genericParamAnnotationsMap; + return setBuilder.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 9dc6052341..8bcb99323f 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 @@ -36,7 +36,8 @@ public static void write( Map importedAnnotations, Map> packageAnnotations, Map> typeAnnotations, - Map methodRecords) + Map methodRecords, + Map> nullableUpperBounds) throws IOException { // File format version/magic number out.writeInt(VERSION_0_FILE_MAGIC_NUMBER); @@ -49,7 +50,8 @@ public static void write( importedAnnotations.values(), packageAnnotations.keySet(), typeAnnotations.keySet(), - methodRecords.keySet()); + methodRecords.keySet(), + nullableUpperBounds.keySet()); for (Collection keyset : keysets) { for (String key : keyset) { assert !encodingDictionary.containsKey(key); @@ -118,5 +120,17 @@ public static void write( } } } + // Followed by the number of nullable upper bounds records + out.writeInt(nullableUpperBounds.size()); + for (Map.Entry> entry : nullableUpperBounds.entrySet()) { + // Followed by the number of parameters with nullable upper bound + Set parameters = entry.getValue(); + out.writeInt(parameters.size()); + for (Integer parameter : parameters) { + // Followed by the nullable upper bound record as a par of integers + out.writeInt(encodingDictionary.get(entry.getKey())); + out.writeInt(parameter); + } + } } } 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 a1f0085407..01d047e48e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -69,7 +69,7 @@ public InferredJARModelsHandler(Config config) { super(); this.config = config; this.cacheUtil = new StubxCacheUtil("JI"); - argAnnotCache = cacheUtil.loadStubxFiles(); + argAnnotCache = cacheUtil.getArgAnnotCache(); // Load Android SDK JarInfer models try { InputStream androidStubxIS = 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 796fec162a..fe1eee118a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -1289,9 +1289,12 @@ private static Symbol.MethodSymbol lookupHandlingOverrides( private static class ExternalStubxLibraryModels implements LibraryModels { private static final Map>>> argAnnotCache; + private static final Map upperBoundsCache; static { - argAnnotCache = new StubxCacheUtil("LM").loadStubxFiles(); + StubxCacheUtil cacheUtil = new StubxCacheUtil("LM"); + argAnnotCache = cacheUtil.getArgAnnotCache(); + upperBoundsCache = cacheUtil.getUpperBoundCache(); } @Override @@ -1304,18 +1307,8 @@ public ImmutableSet nullMarkedClasses() { public ImmutableSetMultimap typeVariablesWithNullableUpperBounds() { ImmutableSetMultimap.Builder mapBuilder = new ImmutableSetMultimap.Builder<>(); - for (Map.Entry>>> entry : - argAnnotCache.entrySet()) { - String className = entry.getKey(); - Map>> innerMap = entry.getValue(); - String generic_key = className + ":-1_GENERIC_TYPE"; - if (innerMap.containsKey(generic_key)) { - Map> innerInnerMap = innerMap.get(generic_key); - for (Map.Entry> innerEntry : innerInnerMap.entrySet()) { - Integer innerInnerKey = innerEntry.getKey(); - mapBuilder.put(className, innerInnerKey); - } - } + for (Map.Entry entry : upperBoundsCache.entrySet()) { + mapBuilder.put(entry.getKey(), entry.getValue()); } return mapBuilder.build(); } 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 c63d774dd3..e37fc467f7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java @@ -26,6 +26,7 @@ import java.io.DataInputStream; import java.io.IOException; import java.io.InputStream; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; @@ -45,18 +46,31 @@ private void LOG(boolean cond, String tag, String msg) { } private static final int RETURN = -1; + private final Map>>> argAnnotCache; + private final Map upperBoundCache; + public StubxCacheUtil(String logCaller) { argAnnotCache = new LinkedHashMap<>(); + upperBoundCache = new HashMap<>(); this.logCaller = logCaller; + loadStubxFiles(); + } + + public Map getUpperBoundCache() { + return upperBoundCache; + } + + public Map>>> getArgAnnotCache() { + return argAnnotCache; } /** * Loads all stubx files discovered in the classpath. Stubx files are discovered via * implementations of {@link JarInferStubxProvider} loaded using a {@link ServiceLoader} */ - public Map>>> loadStubxFiles() { + private void loadStubxFiles() { Iterable astubxProviders = ServiceLoader.load(JarInferStubxProvider.class, StubxCacheUtil.class.getClassLoader()); for (JarInferStubxProvider provider : astubxProviders) { @@ -72,7 +86,6 @@ public Map>>> loadStubxFiles() { } } } - return argAnnotCache; } public void parseStubStream(InputStream stubxInputStream, String stubxLocation) @@ -133,6 +146,14 @@ public void parseStubStream(InputStream stubxInputStream, String stubxLocation) "method: " + methodSig + ", argNum: " + argNum + ", arg annotation: " + annotation); cacheAnnotation(methodSig, argNum, annotation); } + // read the number of nullable upper bound entries + int numClassesWithNullableUpperBounds = in.readInt(); + for (int i = 0; i < numClassesWithNullableUpperBounds; i++) { + int numParams = in.readInt(); + for (int j = 0; j < numParams; j++) { + cacheUpperBounds(strings[in.readInt()], in.readInt()); + } + } } private void cacheAnnotation(String methodSig, Integer argNum, String annotation) { @@ -146,4 +167,8 @@ private void cacheAnnotation(String methodSig, Integer argNum, String annotation cacheForMethod.computeIfAbsent(argNum, s -> new LinkedHashSet<>()); cacheForArgument.add(annotation); } + + private void cacheUpperBounds(String className, Integer paramIndex) { + upperBoundCache.put(className, paramIndex); + } } From a28fa1c512140e6d328e6965ca5f41eb11f34ad2 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Mon, 3 Jun 2024 23:44:50 +0530 Subject: [PATCH 06/15] refactoring: changing class name --- .../uber/nullaway/libmodel/LibraryModelIntegrationTest.java | 4 ++-- .../java/com/uber/nullaway/libmodel/AnnotationExample.java | 2 +- .../src/com/uber/nullaway/libmodel/AnnotationExample.java | 2 +- 3 files changed, 4 insertions(+), 4 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 97434bf953..1420821178 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 @@ -141,12 +141,12 @@ public void libraryModelInnerClassNullableUpperBoundsTest() { "import org.jspecify.annotations.Nullable;", "import com.uber.nullaway.libmodel.AnnotationExample;", "class Test {", - " static AnnotationExample.GenericExample<@Nullable Object> genericExample = new AnnotationExample.GenericExample<@Nullable Object>();", + " static AnnotationExample.UpperBoundExample<@Nullable Object> upperBoundExample = new AnnotationExample.UpperBoundExample<@Nullable Object>();", " static void test(Object value){", " }", " static void testPositive() {", " // BUG: Diagnostic contains: passing @Nullable parameter 'genericExample.getNull()'", - " test(genericExample.getNull());", + " test(upperBoundExample.getNull());", " }", "}") .doTest(); 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 24b4ce228a..dd2ffc5e93 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 @@ -40,7 +40,7 @@ public String returnNull() { } } - public static class GenericExample { + public static class UpperBoundExample { public T getNull() { return null; 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 5c93e5f3cd..207b4f6447 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 @@ -46,7 +46,7 @@ public String returnNull() { } } - public static class GenericExample { + public static class UpperBoundExample { public T getNull() { return null; From 6cab3030d1d7a5be19fd4ef3edee466f1ae3662a Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Mon, 3 Jun 2024 23:47:51 +0530 Subject: [PATCH 07/15] fix --- .../com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1420821178..90e4026596 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 @@ -145,7 +145,7 @@ public void libraryModelInnerClassNullableUpperBoundsTest() { " static void test(Object value){", " }", " static void testPositive() {", - " // BUG: Diagnostic contains: passing @Nullable parameter 'genericExample.getNull()'", + " // BUG: Diagnostic contains: passing @Nullable parameter 'upperBoundExample.getNull()'", " test(upperBoundExample.getNull());", " }", "}") From 7d833db147019525777ed62db3f6cbccf8b56574 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 4 Jun 2024 00:38:43 +0530 Subject: [PATCH 08/15] docs --- .../uber/nullaway/handlers/StubxCacheUtil.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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 e37fc467f7..e2ac9cb991 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java @@ -33,6 +33,14 @@ import java.util.ServiceLoader; import java.util.Set; +/** + * A class responsible for caching annotation information extracted from stubx files. + * + *

This class provides mechanisms to cache annotations and retrieve them efficiently when needed. + * It uses a nested map structure to store annotations, which are indexed by class name, method + * signature, and argument index. It also stores a Map containing the indices for Nullable upper + * bounds for generic type parameters. + */ public class StubxCacheUtil { private static final int VERSION_0_FILE_MAGIC_NUMBER = 691458791; @@ -51,6 +59,14 @@ private void LOG(boolean cond, String tag, String msg) { private final Map upperBoundCache; + /** + * Initializes a new {@code StubxCacheUtil} instance. + * + *

This sets up the caches for argument annotations and upper bounds, sets the log caller, and + * loads the stubx files. + * + * @param logCaller Identifier for logging purposes. + */ public StubxCacheUtil(String logCaller) { argAnnotCache = new LinkedHashMap<>(); upperBoundCache = new HashMap<>(); From 9b63aa76bd6183fae5b830a50858c32d20bb1a7f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 16 Apr 2024 16:31:48 -0700 Subject: [PATCH 09/15] add a comment --- .../main/java/com/uber/nullaway/libmodel/AnnotationExample.java | 1 + 1 file changed, 1 insertion(+) 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 dd2ffc5e93..5763684bf1 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 @@ -40,6 +40,7 @@ public String returnNull() { } } + /** In the library model we add a {@code @Nullable} upper bound for T */ public static class UpperBoundExample { public T getNull() { From 4186ca7066bd61535fbdbeb990041d6e7e250503 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 4 Jun 2024 11:43:14 -0700 Subject: [PATCH 10/15] comment --- .../uber/nullaway/libmodel/LibraryModelIntegrationTest.java | 5 +++++ 1 file changed, 5 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 90e4026596..105a1decfc 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 @@ -8,6 +8,11 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; +/** + * Integration test for library model support. The library models are contained in the jar for the + * test-library-model-generator project, as a stubx file. These tests ensure that NullAway correctly + * loads the stubx file and reports the right errors based on those models. + */ public class LibraryModelIntegrationTest { @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); From f3698174195ab19e06dc4dae4b33aef7e111ba97 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 4 Jun 2024 12:04:11 -0700 Subject: [PATCH 11/15] comment --- .../com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java | 2 ++ 1 file changed, 2 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 105a1decfc..f51781e070 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 @@ -47,6 +47,8 @@ public void libraryModelNullableReturnsTest() { " test(annotationExample.makeUpperCase(\"nullaway\"));", " }", " static void testNegative() {", + " // no error since nullReturn is annotated with javax.annotation.Nullable,", + " // which is not considered when generating stubx files", " test(annotationExample.nullReturn());", " }", "}") From a190b9f0e2bdd19ddf2526955b05a4c3957280b6 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Fri, 7 Jun 2024 02:50:37 +0530 Subject: [PATCH 12/15] rewriting getNull method from UpperBoundExample and refactoring --- .../libmodel/LibraryModelIntegrationTest.java | 24 +++++++++++++++++-- .../nullaway/libmodel/AnnotationExample.java | 6 +++-- .../nullaway/libmodel/AnnotationExample.java | 6 +++-- .../handlers/InferredJARModelsHandler.java | 3 ++- .../handlers/LibraryModelsHandler.java | 3 ++- 5 files changed, 34 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 90e4026596..9d4351cfd3 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 @@ -145,10 +145,30 @@ public void libraryModelInnerClassNullableUpperBoundsTest() { " static void test(Object value){", " }", " static void testPositive() {", - " // BUG: Diagnostic contains: passing @Nullable parameter 'upperBoundExample.getNull()'", - " test(upperBoundExample.getNull());", + " // BUG: Diagnostic contains: passing @Nullable parameter 'upperBoundExample.getNullable()'", + " test(upperBoundExample.getNullable());", " }", "}") .doTest(); } + + @Test + public void libraryModelNullableUpperBoundsWithoutJarInferTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import com.uber.nullaway.libmodel.AnnotationExample;", + "class Test {", + " //TODO: We should get an error here since jar infer is not enabled", + " static AnnotationExample.UpperBoundExample<@Nullable Object> upperBoundExample = new AnnotationExample.UpperBoundExample<@Nullable Object>();", + "}") + .doTest(); + } } 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 dd2ffc5e93..ef53805bc2 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 @@ -42,8 +42,10 @@ public String returnNull() { public static class UpperBoundExample { - public T getNull() { - return null; + T nullableObject; + + public T getNullable() { + return nullableObject; } } } 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 207b4f6447..5fef2e2118 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 @@ -48,8 +48,10 @@ public String returnNull() { public static class UpperBoundExample { - public T getNull() { - return null; + T nullableObject; + + public T getNullable() { + return nullableObject; } } } 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 01d047e48e..f171c502da 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -68,7 +68,8 @@ private static void LOG(boolean cond, String tag, String msg) { public InferredJARModelsHandler(Config config) { super(); this.config = config; - this.cacheUtil = new StubxCacheUtil("JI"); + String jarInferLogName = "JI"; + this.cacheUtil = new StubxCacheUtil(jarInferLogName); argAnnotCache = cacheUtil.getArgAnnotCache(); // Load Android SDK JarInfer models try { 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 fe1eee118a..32c318b276 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -1292,7 +1292,8 @@ private static class ExternalStubxLibraryModels implements LibraryModels { private static final Map upperBoundsCache; static { - StubxCacheUtil cacheUtil = new StubxCacheUtil("LM"); + String libraryModelLogName = "LM"; + StubxCacheUtil cacheUtil = new StubxCacheUtil(libraryModelLogName); argAnnotCache = cacheUtil.getArgAnnotCache(); upperBoundsCache = cacheUtil.getUpperBoundCache(); } From 4f81dc0561a17a7013f14b9db643bb312131b8bf Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Fri, 7 Jun 2024 03:15:47 +0530 Subject: [PATCH 13/15] updating Nullability logic for upper bounds to consider only org.jspecify.annotations.Nullable --- .../java/com/uber/nullaway/libmodel/LibraryModelGenerator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 a13d720152..97ef44c7d3 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 @@ -292,7 +292,8 @@ private ImmutableSet getGenericTypeParameterNullableUpperBounds( for (int i = 0; i < typeParamList.size(); i++) { TypeParameter param = typeParamList.get(i); for (ClassOrInterfaceType type : param.getTypeBound()) { - if (type.isAnnotationPresent(NULLABLE)) { + Optional nullableAnnotation = type.getAnnotationByName(NULLABLE); + if (nullableAnnotation.isPresent() && isAnnotationNullable(nullableAnnotation.get())) { setBuilder.add(i); } } From 71ce2e2c2c404397ccba8739ba7ab84b41a5746f Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Fri, 7 Jun 2024 20:48:57 +0530 Subject: [PATCH 14/15] making cache fields instance --- .../com/uber/nullaway/handlers/LibraryModelsHandler.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 32c318b276..b85b8a560a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -1288,10 +1288,10 @@ private static Symbol.MethodSymbol lookupHandlingOverrides( /** Constructs Library Models from stubx files */ private static class ExternalStubxLibraryModels implements LibraryModels { - private static final Map>>> argAnnotCache; - private static final Map upperBoundsCache; + private final Map>>> argAnnotCache; + private final Map upperBoundsCache; - static { + ExternalStubxLibraryModels() { String libraryModelLogName = "LM"; StubxCacheUtil cacheUtil = new StubxCacheUtil(libraryModelLogName); argAnnotCache = cacheUtil.getArgAnnotCache(); From 3f4c3e42fd4ff69daed447921d9eb763a737a731 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Fri, 7 Jun 2024 20:59:10 +0530 Subject: [PATCH 15/15] updating libraryModelNullableUpperBoundsWithoutJarInferTest --- .../uber/nullaway/libmodel/LibraryModelIntegrationTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 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 269068a2cb..3a230d0200 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 @@ -166,14 +166,15 @@ public void libraryModelNullableUpperBoundsWithoutJarInferTest() { Arrays.asList( "-d", temporaryFolder.getRoot().getAbsolutePath(), - "-XepOpt:NullAway:AnnotatedPackages=com.uber")) + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:JSpecifyMode=true")) .addSourceLines( "Test.java", "package com.uber;", "import org.jspecify.annotations.Nullable;", "import com.uber.nullaway.libmodel.AnnotationExample;", "class Test {", - " //TODO: We should get an error here since jar infer is not enabled", + " // BUG: Diagnostic contains: Generic type parameter cannot be @Nullable", " static AnnotationExample.UpperBoundExample<@Nullable Object> upperBoundExample = new AnnotationExample.UpperBoundExample<@Nullable Object>();", "}") .doTest();