More simplifications based on recent review comments:#8388
Merged
copybara-service[bot] merged 1 commit intomasterfrom Apr 28, 2026
Merged
More simplifications based on recent review comments:#8388copybara-service[bot] merged 1 commit intomasterfrom
copybara-service[bot] merged 1 commit intomasterfrom
Conversation
- `FunctionalEquivalence`: Migrate from `c.g.common.base.Objects.hashCode` to `j.u.Objects.hash`. - (We can't static import it because `Equivalance` declares its own `hash` method.) - `Predicate`: Migrate Javadoc from `c.g.common.base.Object.equal` to `j.u.Objects.equals`. - `Predicates`: Tweak generics, at which point we can eliminate our local `asList` method and some explicit type arguments - various files: Rename+move `Iterators.checkNonnegative` to `CollectPreconditions.checkNonnegativeIndex` to clarify how it differs from `CollectPreconditions.checkNonnegative`—namely, by throwing `IndexOutOfBoundsException` instead of `IllegalArgumentException`. - Thanks to Gemini for catching my mistake of trying to use the existing `CollectPreconditions.checkNonnegative`! - This change also changes the error message slightly. I could have left it alone, but I figured it might as well match the format we're using for the similar methods in `CollectPreconditions`. - I had thought that perhaps we could use `Preconditions.checkPositionIndex`, but (a) that requires a size (which appears in the exception message, so we shouldn't just pass `MAX_VALUE`) and (b) that [throws `IllegalArgumentException`](https://github.com/google/guava/blob/b9d1fd6875d9267c14dc4063e13af85894ea35d1/guava/src/com/google/common/base/Preconditions.java#L1505), too, wait, seriously? What were we thinking in cl/9392102 back in 2008? - I'll bet that many (most?) of our usages of `checkPositionIndex` are wrong. Certainly that's true of [the first one that I looked at](https://github.com/google/guava/blob/b9d1fd6875d9267c14dc4063e13af85894ea35d1/guava/src/com/google/common/collect/Lists.java#L355). - various files: Remove suppressions of StaticImportPreferred for `min`/`max` methods that are in scope. The suppressions are no longer necessary as of cl/897308244, which avoids trying to static import when a method of the same name is already in scope. - Or actually, they're not necessary as of the earlier cl/811482021, which removed those suggestions from StaticImportPreferred entirely. - I kept the suppression in `Booleans`. That one exists because `Comparator.min` does not exist under all versions, so the fix of cl/897308244 is presumably not always sufficient. - Again, that's technically not necessary because StaticImportPreferred doesn't make the suggestion at all anymore (for exactly this reason). I suppose that I could tweak it to suggest `min`/`max` again as long as we're not inside a `Comparator`, but StaticImportPreferred is controversial in general, so I don't think that's necessarily worthwhile. - various files: Static import `min`/`max` where we can (by locally undoing the exclusion for those methods :)). - various files in the backport: Static import `CollectPreconditions` methods in a few more cases. RELNOTES=n/a PiperOrigin-RevId: 907127863
963a832 to
7ae84e3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
More simplifications based on recent review comments:
FunctionalEquivalence: Migrate fromc.g.common.base.Objects.hashCodetoj.u.Objects.hash.Equivalancedeclares its ownhashmethod.)Predicate: Migrate Javadoc fromc.g.common.base.Object.equaltoj.u.Objects.equals.Predicates: Tweak generics, at which point we can eliminate our localasListmethod and some explicit type argumentsIterators.checkNonnegativetoCollectPreconditions.checkNonnegativeIndexto clarify how it differs fromCollectPreconditions.checkNonnegative—namely, by throwingIndexOutOfBoundsExceptioninstead ofIllegalArgumentException.CollectPreconditions.checkNonnegative!CollectPreconditions.Preconditions.checkPositionIndex, but (a) that requires a size (which appears in the exception message, so we shouldn't just passMAX_VALUE) and (b) that throwsIllegalArgumentException, too, wait, seriously? What were we thinking in cl/9392102 back in 2008?checkPositionIndexare wrong. Certainly that's true of the first one that I looked at.min/maxmethods that are in scope. The suppressions are no longer necessary as of cl/897308244, which avoids trying to static import when a method of the same name is already in scope.Booleans. That one exists becauseComparator.mindoes not exist under all versions, so the fix of cl/897308244 is presumably not always sufficient.min/maxagain as long as we're not inside aComparator, but StaticImportPreferred is controversial in general, so I don't think that's necessarily worthwhile.min/maxwhere we can (by locally undoing the exclusion for those methods :)).CollectPreconditionsmethods in a few more cases.RELNOTES=n/a