diff --git a/MODULE.bazel b/MODULE.bazel index 9a529357..5ccde365 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -16,6 +16,6 @@ bazel_dep(name = "rules_java", version = "7.12.1") bazel_dep(name = "google_bazel_common") git_override( module_name = "google_bazel_common", - commit = "e2204d625d97dda88112af0b0a9e56ed7712d15c", + commit = "a7761f9d45b65283274d8d86abb3b87e9fc87258", remote = "https://github.com/google/bazel-common", ) diff --git a/api/BUILD b/api/BUILD index ac985b0d..78b6fc09 100644 --- a/api/BUILD +++ b/api/BUILD @@ -89,6 +89,7 @@ genrule( srcs = STACK_GETTER_STACK_WALKER_IMPL_SRCS + [ ":stack_getter_common", ":checks", + "@google_bazel_common//third_party/java/jspecify_annotations", ], outs = ["stack_getter_stack_walker_impl.jar"], cmd = """ @@ -97,9 +98,11 @@ TMPDIR=$$(mktemp -d) OUTPUT=$$PWD/$@ STACK_GETTER_COMMON=$(location :stack_getter_common) CHECKS=$(location :checks) +JSPECIFY_ANNOTATIONS=$(location @google_bazel_common//third_party/java/jspecify_annotations) JAVABASE=$$PWD/$(JAVABASE) JAR=$$PWD/$(JAVABASE)/bin/jar -$$JAVABASE/bin/javac -d $$TMPDIR -source 8 -target 8 -cp $${STACK_GETTER_COMMON}:$${CHECKS} -implicit:none \ +$$JAVABASE/bin/javac -d $$TMPDIR -source 8 -target 8 \ + -cp $${STACK_GETTER_COMMON}:$${CHECKS}:$${JSPECIFY_ANNOTATIONS} -implicit:none \ $(location src/main/java/com/google/common/flogger/util/StackWalkerStackGetter.java) cd $$TMPDIR && $$JAR cf $$OUTPUT com/google/common/flogger/util/*StackGetter*.class """, diff --git a/api/platformprovider/main/java/com/google/common/flogger/backend/PlatformProviderGenerator.java b/api/platformprovider/main/java/com/google/common/flogger/backend/PlatformProviderGenerator.java index d8f58fc5..9512b179 100644 --- a/api/platformprovider/main/java/com/google/common/flogger/backend/PlatformProviderGenerator.java +++ b/api/platformprovider/main/java/com/google/common/flogger/backend/PlatformProviderGenerator.java @@ -16,6 +16,7 @@ package com.google.common.flogger.backend; +import static java.util.Objects.requireNonNull; import static org.objectweb.asm.Opcodes.ACC_FINAL; import static org.objectweb.asm.Opcodes.ACC_PRIVATE; import static org.objectweb.asm.Opcodes.ACC_PUBLIC; @@ -108,7 +109,7 @@ public static void main(String[] args) throws IOException { // Write the class to the output file. Path path = Paths.get(args[0]); - Files.createDirectories(path.getParent()); + Files.createDirectories(requireNonNull(path.getParent(), "path must have a parent: " + path)); try (JarOutputStream jar = new JarOutputStream(Files.newOutputStream(path, StandardOpenOption.CREATE_NEW))) { ZipEntry entry = new ZipEntry("com/google/common/flogger/backend/PlatformProvider.class"); diff --git a/api/src/main/java/com/google/common/flogger/LogContext.java b/api/src/main/java/com/google/common/flogger/LogContext.java index c48f723f..4ef485d0 100644 --- a/api/src/main/java/com/google/common/flogger/LogContext.java +++ b/api/src/main/java/com/google/common/flogger/LogContext.java @@ -18,6 +18,7 @@ import static com.google.common.flogger.util.CallerFinder.getStackForCallerOf; import static com.google.common.flogger.util.Checks.checkNotNull; +import static com.google.common.flogger.util.Checks.checkState; import static java.util.concurrent.TimeUnit.NANOSECONDS; import com.google.common.flogger.DurationRateLimiter.RateLimitPeriod; @@ -435,16 +436,18 @@ public final LogSite getLogSite() { @Override public final Object[] getArguments() { - if (templateContext == null) { - throw new IllegalStateException("cannot get arguments unless a template context exists"); + checkState(templateContext != null, "cannot get arguments unless a template context exists"); + if (args == null) { + throw new IllegalStateException("cannot get arguments before calling log()"); } return args; } @Override public final Object getLiteralArgument() { - if (templateContext != null) { - throw new IllegalStateException("cannot get literal argument if a template context exists"); + checkState(templateContext == null, "cannot get literal argument if a template context exists"); + if (args == null) { + throw new IllegalStateException("cannot get literal argument before calling log()"); } return args[0]; } @@ -683,7 +686,7 @@ private boolean shouldLog() { if (rateLimitStatus != null) { // We check rate limit status even if it is "DISALLOW" to update the skipped logs count. int skippedLogs = RateLimitStatus.checkStatus(rateLimitStatus, logSiteKey, metadata); - if (shouldLog && skippedLogs > 0) { + if (shouldLog && skippedLogs > 0 && metadata != null) { metadata.addValue(Key.SKIPPED_LOG_COUNT, skippedLogs); } // checkStatus() returns -1 in two cases: diff --git a/api/src/main/java/com/google/common/flogger/SamplingRateLimiter.java b/api/src/main/java/com/google/common/flogger/SamplingRateLimiter.java index 43fd4995..5f64aaf8 100644 --- a/api/src/main/java/com/google/common/flogger/SamplingRateLimiter.java +++ b/api/src/main/java/com/google/common/flogger/SamplingRateLimiter.java @@ -60,6 +60,12 @@ protected Random initialValue() { } }; + // Android annotates ThreadLocal.get() with @RecentlyNullable, but random.get() is never null. + @SuppressWarnings("nullness") + private static Random getRandom() { + return random.get(); + } + // Visible for testing. final AtomicInteger pendingCount = new AtomicInteger(); @@ -72,7 +78,7 @@ RateLimitStatus sampleOneIn(int rateLimitCount) { // zero and the number of logs will end up statistically close to 1-in-N (even if at // times they can be "bursty" due to the action of other rate limiting mechanisms). int pending; - if (random.get().nextInt(rateLimitCount) == 0) { + if (getRandom().nextInt(rateLimitCount) == 0) { pending = pendingCount.incrementAndGet(); } else { pending = pendingCount.get(); diff --git a/api/src/main/java/com/google/common/flogger/backend/Platform.java b/api/src/main/java/com/google/common/flogger/backend/Platform.java index 20e6d6b2..88fc1c0c 100644 --- a/api/src/main/java/com/google/common/flogger/backend/Platform.java +++ b/api/src/main/java/com/google/common/flogger/backend/Platform.java @@ -87,11 +87,12 @@ private static Platform loadFirstAvailablePlatform(String[] platformClass) { return (Platform) Class.forName(clazz).getConstructor().newInstance(); } catch (Throwable e) { // Catch errors so if we can't find _any_ implementations, we can report something useful. - // Unwrap any generic wrapper exceptions for readability here (extend this as needed). - if (e instanceof InvocationTargetException) { - e = e.getCause(); - } - errorMessage.append('\n').append(clazz).append(": ").append(e); + errorMessage + .append('\n') + .append(clazz) + .append(": ") + // Unwrap any wrapper exceptions for readability here (extend as needed). + .append(e instanceof InvocationTargetException ? e.getCause() : e); } } throw new IllegalStateException( diff --git a/api/src/main/java/com/google/common/flogger/backend/system/AbstractBackend.java b/api/src/main/java/com/google/common/flogger/backend/system/AbstractBackend.java index 022effe9..6d459ca3 100644 --- a/api/src/main/java/com/google/common/flogger/backend/system/AbstractBackend.java +++ b/api/src/main/java/com/google/common/flogger/backend/system/AbstractBackend.java @@ -22,7 +22,6 @@ import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.logging.Logger; -import org.jspecify.annotations.Nullable; /** * Common backend to handle everything except formatting of log message and metadata. This is an @@ -146,7 +145,7 @@ public final void log(LogRecord record, boolean wasForced) { // would get a lot of extra stack frames to search through). However for Flogger, the LogSite // was already determined in the "shouldLog()" method because it's needed for things like // rate limiting. Thus we don't have to care about using iterative methods vs recursion here. - private static void publish(@Nullable Logger logger, LogRecord record) { + private static void publish(Logger logger, LogRecord record) { // Annoyingly this method appears to copy the array every time it is called, but there's // nothing much we can do about this (and there could be synchronization issues even if we // could access things directly because handlers can be changed at any time). Most of the @@ -155,9 +154,9 @@ private static void publish(@Nullable Logger logger, LogRecord record) { handler.publish(record); } if (logger.getUseParentHandlers()) { - logger = logger.getParent(); - if (logger != null) { - publish(logger, record); + Logger parent = logger.getParent(); + if (parent != null) { + publish(parent, record); } } } diff --git a/api/src/main/java/com/google/common/flogger/backend/system/AbstractLogRecord.java b/api/src/main/java/com/google/common/flogger/backend/system/AbstractLogRecord.java index cad52abf..baca8b66 100644 --- a/api/src/main/java/com/google/common/flogger/backend/system/AbstractLogRecord.java +++ b/api/src/main/java/com/google/common/flogger/backend/system/AbstractLogRecord.java @@ -170,6 +170,12 @@ public final void setParameters(Object @Nullable [] parameters) { super.setParameters(parameters); } + @Override + @SuppressWarnings("nullness") // setParameters() above guarantees this is never null. + public final Object[] getParameters() { + return super.getParameters(); + } + @Override public final void setMessage(@Nullable String message) { if (message == null) { diff --git a/api/src/main/java/com/google/common/flogger/context/Tags.java b/api/src/main/java/com/google/common/flogger/context/Tags.java index 7d2030db..b7d8d555 100644 --- a/api/src/main/java/com/google/common/flogger/context/Tags.java +++ b/api/src/main/java/com/google/common/flogger/context/Tags.java @@ -513,8 +513,12 @@ private int mergeTagMaps( int newEntryIndex = 0; while (lhsEntry != null || rhsEntry != null) { // Nulls count as being *bigger* than anything (since they indicate the end of the array). - int signum = (lhsEntry == null) ? 1 : (rhsEntry == null) ? -1 : 0; - if (signum == 0) { + int signum; + if (lhsEntry == null) { + signum = 1; + } else if (rhsEntry == null) { + signum = -1; + } else { // Both entries exist and must be compared. signum = lhsEntry.getKey().compareTo(rhsEntry.getKey()); if (signum == 0) { @@ -528,6 +532,7 @@ private int mergeTagMaps( continue; } } + // Signum is non-zero and indicates which entry to process next (without merging). if (signum < 0) { valueStart = copyEntryAndValues(lhsEntry, newEntryIndex++, valueStart, array, offsets); diff --git a/api/src/main/java/com/google/common/flogger/parameter/SimpleParameter.java b/api/src/main/java/com/google/common/flogger/parameter/SimpleParameter.java index 9901a41f..e00dbcd9 100644 --- a/api/src/main/java/com/google/common/flogger/parameter/SimpleParameter.java +++ b/api/src/main/java/com/google/common/flogger/parameter/SimpleParameter.java @@ -68,7 +68,7 @@ public static SimpleParameter of(int index, FormatChar formatChar, FormatOptions // We can safely test FormatSpec with '==' because the factory methods always return the default // instance if applicable (and the class has no visible constructors). if (index < MAX_CACHED_PARAMETERS && options.isDefault()) { - return DEFAULT_PARAMETERS.get(formatChar)[index]; + return checkNotNull(DEFAULT_PARAMETERS.get(formatChar), "default parameter")[index]; } return new SimpleParameter(index, formatChar, options); } diff --git a/api/src/main/java/com/google/common/flogger/util/RecursionDepth.java b/api/src/main/java/com/google/common/flogger/util/RecursionDepth.java index b0bcf728..107d8aed 100644 --- a/api/src/main/java/com/google/common/flogger/util/RecursionDepth.java +++ b/api/src/main/java/com/google/common/flogger/util/RecursionDepth.java @@ -36,9 +36,15 @@ protected RecursionDepth initialValue() { } }; + // Android annotates ThreadLocal.get() with @RecentlyNullable, but holder.get() is never null. + @SuppressWarnings("nullness") + private static RecursionDepth getRecursionDepth() { + return holder.get(); + } + /** Do not call this method directly, use {@code Platform.getCurrentRecursionDepth()}. */ public static int getCurrentDepth() { - return holder.get().value; + return getRecursionDepth().value; } /** Do not call this method directly, use {@code Platform.getCurrentRecursionDepth()}. */ @@ -48,7 +54,7 @@ public int getValue() { /** Internal API for use by core Flogger library. */ public static RecursionDepth enterLogStatement() { - RecursionDepth depth = holder.get(); + RecursionDepth depth = getRecursionDepth(); // Can only reach 0 if it wrapped around completely or someone is manipulating the value badly. // We really don't expect 2^32 levels of recursion however, so assume it's a bug. if (++depth.value == 0) { diff --git a/api/src/main/java/com/google/common/flogger/util/StackWalkerStackGetter.java b/api/src/main/java/com/google/common/flogger/util/StackWalkerStackGetter.java index 3cc6c5f7..7f34202f 100644 --- a/api/src/main/java/com/google/common/flogger/util/StackWalkerStackGetter.java +++ b/api/src/main/java/com/google/common/flogger/util/StackWalkerStackGetter.java @@ -21,6 +21,7 @@ import java.lang.StackWalker.StackFrame; import java.util.function.Predicate; import java.util.stream.Stream; +import org.jspecify.annotations.Nullable; /** * StackWalker based implementation of the {@link StackGetter} interface. @@ -39,9 +40,9 @@ public StackWalkerStackGetter() { } @Override - public StackTraceElement callerOf(Class target, int skipFrames) { + public @Nullable StackTraceElement callerOf(Class target, int skipFrames) { checkArgument(skipFrames >= 0, "skipFrames must be >= 0"); - return STACK_WALKER.walk( + return STACK_WALKER.<@Nullable StackTraceElement>walk( s -> filterStackTraceAfterTarget(isTargetClass(target), skipFrames, s) .findFirst()