github.com-google-guava

Address some more counterfactual [CheckedExceptionNotThrown](https://errorprone.info/bugpattern/CheckedExceptionNotThrown) findings. I suspect that IntelliJ flags most of these, especially since its analyses seem happy to treat Guava as a "closed world" with no callers/overrides outside the project, if I'm remembering right. Anyway, that makes them nice to fix so that F2 doesn't jump to them. We don't normally worry much about a stray `throws Exception` on a `test*` method in particular, but it was easier to have the tooling remove them (and then glance over the results) than to make it smarter, and it's a nice bonus to remove such `throws` clauses, anyway. It's possible that I could rerun this cleanup until I reach a fixed point, since each removal may unlock removals downstream. I think that I made one follow-up change somewhere in `util.concurrent`. Maybe I'll do another "full" round, maybe using IntelliJ in case it catches anything that Error Prone did not. I made no effort to look at `abstract` methods that might have no implementations that actually throw a declared exception. I made no effort to remove calls to `super.setUp`/`tearDown` when those calls are no-ops (because they call the methods on plain `TestCase`). If I were to do so, that might unlock more `throws` removals on the `setUp` and `tearDown` methods that are making those calls. There were some cases that I had to revert: - `TypeTokenTest.CannotConstruct` constructor - `AbstractIdleService`: `DefaultService` and `TestService`, both `startUp` and `shutDown` (though I may or may not have needed to revert all four) - `AbstractExecutionThreadServiceTest.FakeServices.startUp` Finally, I made `ListeningExecutorServiceTest.FakeExecutorService` `final` while I was in the area. RELNOTES=n/a PiperOrigin-RevId: 938063962

Remove unnecessary type arguments. This CL comes courtesy of IntelliJ. (I did revert a few files that ended up with errors, probably often from non-javac tools like our transpilers, and I reverted one tiny part of `OrderingTest`. I may have been able to get away with reverting less.) (previously: https://github.com/google/guava/commit/cb0e7e0cb1c62012934d998ca4d0222a758a9831, https://github.com/google/guava/commit/c1ffb313fd084c865e539c9b6dc795edade21d6a, https://github.com/google/guava/commit/35a4ccbcbb86952f1c021424acd70c7526e8964d, https://github.com/google/guava/commit/08f213923dab324b53a5ee93151a2a2c04be0784, https://github.com/google/guava/commit/274062cdc2a3819ff4b0286e57973b1bbe8c8519, which together explain why I'm not seeing hits in the prod code for packages like `collect`) RELNOTES=n/a PiperOrigin-RevId: 906444162

Mark test-suite-builder suites and related code as `@AndroidIncompatible`. (Along the way, break out `SynchronizedSetTest.TestSet` into a top-level type `LockHeldAssertingSet`, which should also make it easier to run each test in an individual build target someday.) (I did also sneak in one change to the backport copy of `IteratorsTest`. It resolve a diff that we accidentally introduced in cl/613629330.) Such code is already not run when we run our tests under Android emulators. (The Android JUnit 3 runner just ignores `suite()` methods, I want to say?) However, it's still _built_ for them. Besides being a waste, this requires pulling in the whole suite-builder ecosystem, which is again causing problems when I try to bump various things related to Android version—quite possibly a sign that the Android runner starts actually using `suite()` methods, whose large numbers of tests (at least in `common.collect`) are just too much for that environment? By adding `@AndroidIncompatible`, I configure our Android test build to strip the code altogether. RELNOTES=n/a PiperOrigin-RevId: 719651707

Use most of the main `AbstractFuture` implementation from J2KT and from GWT/J2CL. This CL introduces a superclass, `AbstractFutureState`, following the pattern of [`AggregateFutureState`](https://github.com/google/guava/blob/master/guava/src/com/google/common/util/concurrent/AggregateFutureState.java). That superclass contains platform-specific operations. Fixes https://github.com/google/guava/issues/2934 RELNOTES=n/a PiperOrigin-RevId: 729328833

Use `assertThrows` more. I addressed the resulting https://errorprone.info/bugpattern/AssertThrowsMinimizer warnings where I saw them in an early snapshot. But I seem to be seeing them at different places at different times, so I'm leaving some that I'll get in a future round. Also, add two missing(?) tests of `clear()` in backport copy of `MapsTest.ensureNotDirectlyModifiable`. RELNOTES=n/a PiperOrigin-RevId: 895922607

Address various findings, mostly from IntelliJ. Most of the findings are around Javadoc links to non-visible APIs. The mildly annoying case is when we write something like `{@link #keySet}`: If we were building Javadoc with the `-private` flag, then these references would be to the private field of that name. And that's... probably fine, since the rendered Javadoc is in fact going to include private members? But I guess it's questionable to refer to private members of another class at all, even from within Javadoc? Anyway, when we run plain old Javadoc, the links get resolved to `keySet()`, as desired. So the changes of that nature are about making IntelliJ happy (so that I can jump from "real error" to "real error" more easily in cl/934353699) and maybe being slightly more principled about things, rather than fixing errors in our rendered Javadoc. (Also notice that plenty of the findings were in `javatests`, which we don't normally generate Javadoc for.) A couple other notes on Javadoc: - I stopped trying to link to `EmptyImmutableTable`, which was deleted in cl/43207266. There may well have been some other deletions (or at least moves, like `hashFloodingDetected`, mentioned in `ImmutableSetHashFloodingDetectionBenchmark`) that I didn't dig into or at least didn't save notes on. - I made the Android flavor of `TopKSelector` refer to `Comparators.least`, eliminating a diff from the mainline, since we added `least` to the Android flavor a while back. The other notable class of findings is unnecessary casts. RELNOTES=n/a PiperOrigin-RevId: 934979370

Refactor the main `AbstractFuture` implementation to prepare for using it for more environments. That includes J2KT and [GWT/J2CL](https://github.com/google/guava/issues/2934). Changes include: - Suppress nullness at smaller scopes, fixing errors that were hidden by the old, broad suppression. - Remove `synchronized` from an override of `fillInStackTrace`. J2KT doesn't like `synchronized` except on very specific types, and we don't need it. - Introduce `rethrowIfErrorOtherThanStackOverflow` and `interruptCurrentThread`, `Platform` methods that will require different implementations for J2KT and J2CL/GWT. - Introduce helper methods like `casListeners(expect, update)` for `AtomicHelper` operations. These reduce verbosity relative to `ATOMIC_HELPER.casListeners(this, expect, update)`. They also prepare for `AbstractFuture` implementations that don't use `AtomicHelper`. - Introduce `notInstanceOfSetFuture`. This is arguably nicer than `!(localValue instanceof SetFuture)`, but the real purpose is to prepare for when code in a different file needs to check `instance SetFuture`. (I could be convinced that I should just make `SetFuture` package-private instead, even though no one outside the file should use it for anything but an `instanceof` check.) - Mysteriously move things around, increase visibility of members, and introduce and sometimes use accessors. This is to prepare for when some of the code will be moving to a separate file so that the remainder of `AbstractFuture` can be wholly shared across different platforms. - Fix a few typos in comments. Also, rename `SetFuture`. This isn't directly related, but now is as good a time as any to do it. Additional bonus: This CL probably makes [the logging at the "end" of `AbstractFuture` static initialization](https://github.com/google/guava/blob/7ec362ec68b630363231d5292cd6b2577c710be6/guava/src/com/google/common/util/concurrent/AbstractFuture.java#L210) have a better chance of actually working in the hypothetical situation that a logger uses `AbstractFuture`: Currently, `AbstractFuture` performs some further initialization _after_ that logging (such as the initialization of `NULL`). Now, it performs all that initialization before the `static` block that might log. RELNOTES=n/a PiperOrigin-RevId: 729313044

Migrate parts of `javatests/com/google/common/testing/...` from JUnit3 to JUnit4, so that we can use `TestParameterInjector` in an upcoming CL. RELNOTES=n/a PiperOrigin-RevId: 938246066

Use _more_ lambdas. RELNOTES=n/a PiperOrigin-RevId: 725740741

Use Truth in place of `assertEquals` assertions for `Enum` types. RELNOTES=n/a PiperOrigin-RevId: 899198656

Use `assertThat(...).isEqualTo(...)` instead of `assertEquals(..., ...)` for `String` assertions. ...in `base`. RELNOTES=n/a PiperOrigin-RevId: 882875041

Enable translation of actual visibility modifiers in `com.google.common` (excluding Truth). For `protected` methods which are not accessible elsewhere in the package/module under Kotlin, declare a bridge `internal` method, and updates call sites to use it instead. (It's possible that I've made updates in some places that weren't strictly necessary.) RELNOTES=n/a PiperOrigin-RevId: 937492418

Enable translation of actual visibility modifiers in `com.google.common` (excluding Truth). For `protected` methods which are not accessible elsewhere in the package/module under Kotlin, declare a bridge `internal` method, and updates call sites to use it instead. (It's possible that I've made updates in some places that weren't strictly necessary.) RELNOTES=n/a PiperOrigin-RevId: 936234321

Statically import members of `Funnels` and `Hashing`. ...except for `Funnels.asOutputStream`. RELNOTES=n/a PiperOrigin-RevId: 886295873

Address nullness-checking errors in tests. RELNOTES=n/a PiperOrigin-RevId: 884464459

Avoid setting `final` fields in `ConcurrentHashMultiset` deserialization. This covers the first class from https://github.com/google/guava/issues/8099. As part of doing that, I needed a way to create a new `ConcurrentHashMap` instances without failing the existing `countMap.isEmpty()` check in the constructor. To do that, I moved the check out of the constructor and into the `create(ConcurrentMap)` static factory method. Then I updated all existing users of the constructor to instead call `create(ConcurrentMap)`. Along the way, I noticed that we'd never removed a TODO about exposing `create(ConcurrentMap)` in the first place, so I removed that. Plus: - I updated the comment about why we override `toArray`. (Spoiler: We probably shouldn't override one of the two overloads anymore.) I also tweaked the name of a helper method. - I added `@Serial` annotations to our serialization-related methods, and I removed one such method's `@J2ktIncompatible` annotation, which was unnecessary because the entire class is `@J2ktIncompatible`. - other tiny tweaks, including in tests (Fun fact: Back when I was discussing with Gemini the alternative of switching to using default serialization, it noticed the existing incorrect type for `deserializedCountMap`. back then, the type was worth fixing just to be pedantic, but now it actually matters.) RELNOTES=`collect`: Changed `ConcurrentHashMultiset` deserialization to [avoid mutating `final` fields](https://openjdk.org/jeps/500). In extremely unlikely scenarios in which an instance of that type contains an object that refers back to that instance, this could lead to [a broken instance](https://docs.oracle.com/en/java/javase/25/docs/specs/serialization/input.html#the-readresolve-method:~:text=in%20cases%20where%20an%20object%20being%20serialized%20nominates%20a%20replacement%20object%20whose%20object%20graph%20has%20a%20reference%20to%20the%20original%20object) that throws `NullPointerException` when used. PiperOrigin-RevId: 841920488

Make `UnsafeComparator` class initialization succeed even in cases in which we can't use that comparator. This accommodates some apps that eagerly initialize internal Guava classes that they've seen used in previous runs. Currently, such apps crash when `UnsafeComparator` fails to initialize, either because of `--sun-misc-unsafe-memory-access=deny` or because of `-XX:+UseCompactObjectHeaders`. This CL makes the class initialization succeed, setting a flag to indicate whether we should actually use the resulting comparator. Really, failing class initialization has always been a gross way to operate. Concretely, we've seen similar tricks cause log spam on Android (https://github.com/google/guava/commit/fb109b051c9ec48d5e77bf73b33a228ab5d57462). (Granted, we'd expect `Unsafe` to remain available for a long time under Android, so we shouldn't see that kind of issue here—at least not unless optimizers like Proguard are removing `UnsafeComparator`, in which case we'd rather make them stop doing that, rather than silence the warning as we do here....) I also found it convenient to eliminate an `unchecked` warning as part of introducing `interface LexicographicalComparator`, and I suppressed some `deprecation` warnings for `Unsafe`. Finally, I belatedly updated some documentation that was still suggesting that `Unsafe` was our preferred implementation in all environments. RELNOTES=n/a PiperOrigin-RevId: 843239216

Refactor the non-Android implementation of `HashBiMap`: - Rename `BiEntry` to `Node`, and make it not an `Entry`, since we already expose wrapper `Entry` objects to users in all cases. (Then use `equals` on it instead of `==` when we want identity comparison.) - Rename `Itr` to `BiIterator`, and make it `static` (with some `final` methods along the way). - Add a TODO about memory retention. - Add a couple "missing" `@CanIgnoreReturnValue` annotations to overrides in private, nested classes. - Make `Inverse` `static`. Also, make it use the name "obverse" instead of "forward," and rename parameters to make them match the superclass's parameters. The parameter names currently upset [OverridingMethodInconsistentArgumentNamesChecker](https://errorprone.info/bugpattern/OverridingMethodInconsistentArgumentNamesChecker). _Any_ name change from the base method upsets Kotlin builds, as we are likely already performing through J2KT. It's probably just a warning, but we may try to make it stricter someday. (Then update the Android implementation for the "observe" naming, patching up some gratuitous textual diffs between the flavors and making some members `private`, `private`, or `@LazyInit` along the way.) RELNOTES=n/a PiperOrigin-RevId: 845246988

Use Truth in `IntMathTest` and `LongMathTest`. Fix an expected-actual mixup in `LongMath.testPow`. (Thanks, Gemini!) Suppress some false-positive warnings about seeming expected-actual mixups. PiperOrigin-RevId: 901341514

Make `ClosingFuture` use `PhantomReference` instead of `finalize()`. Compare what https://github.com/google/guava/commit/7c6b17c0199c875ffa426c0fb0f51ba7d66f98ec did for `FileBackedOutputStream`. As in that CL, this CL adds `reachabilityFence` calls. I think that the calls are technically more necessary here than they were in `FileBackedOutputStream`, where we could probably [rely on `synchronized` to keep the object alive](https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/lang/ref/Reference.html#reachabilityFence(java.lang.Object)). Incidentally, they are equally necessary under the current `ClosingFuture` approach that uses `finalize()`. (So that gives some indication of how much they matter _in practice_: probably not much.) I had a thought at one point that maybe `reachabilityFence` was necessary only in the _terminal_ operations: Since every `ClosingFuture` pipeline should end with a terminal operation, we should be able to rely on a `reachabilityFence` in that terminal operation to keep the chain alive. However, I now think it's best to put a fence in all operations: - It's simpler. - It behaves better for broken code that lacks a terminal operation: If not for fences on intermediate operations, then such code might see its `ClosingFuture` closed by GC while it's still operating on it. And while such code is buggy, I don't think we want to cause its intermediate operations to _throw_. By putting `reachabilityFence` calls in such operations, we keep the `ClosingFuture` alive until those operations complete, at which point it can leak "properly," getting closed with no harm to anyone except the resulting log message. Finally: As a minor follow-up to https://github.com/google/guava/commit/9744861cb9b985bb832b81da91233b423b812276, I arranged for the `Finalizer` thread to always have an unobfuscated _thread name_, even when the class name of `Finalizer` gets obfuscated. RELNOTES=Migrated some prod classes from `finalize()` to `PhantomReference` in preparation for [the removal of finalization](https://openjdk.org/jeps/421). (7c6b17c) PiperOrigin-RevId: 881753103