Fix method patterns for dead UnsignedInts/UnsignedLongs recipes#1176
Merged
Conversation
Guava's `UnsignedInts`/`UnsignedLongs` have no `remainderUnsigned`, `compareUnsigned` or `divideUnsigned` methods; the real names are `remainder`, `compare` and `divide`. Six of the eight method patterns in this block therefore never matched: - `PreferIntegerRemainderUnsigned` and `PreferLongRemainderUnsigned` targeted a nonexistent `remainderUnsigned`; they now rename `remainder` first, mirroring the existing Compare/Divide chaining. - `PreferLongCompareUnsigned` and `PreferLongDivideUnsigned` declared `(int, int)` where `UnsignedLongs` takes `(long, long)`. - `PreferLongParseUnsignedLong` pointed at `UnsignedInts` rather than `UnsignedLongs`, and its display name and description said `parseUnsignedInt`. Also add `PreferIntegerRemainderUnsigned` to `NoGuava`, as it was declared but never referenced, and add `NoGuavaUnsignedTest` to cover all four conversions per type.
None of `UnsignedInts`/`UnsignedLongs#compareUnsigned`, `#divideUnsigned` or `IntMath#addExact`/`#subtractExact`/`#multiplyExact` exist in Guava, so naming them as the "instead of" source was misleading. The Subtract and Multiply descriptions also referenced `com.google.common.primitives.IntMath`, a package that does not exist; the class is `com.google.common.math.IntMath`. Descriptions only; the underlying method patterns were already correct for the Math recipes.
Pinning the exact parameter types is what allowed the earlier mismatches to go unnoticed, so match on `(..)` instead, as the sibling `PreferMath*Exact` recipes already do. `UnsignedInts` and `UnsignedLongs` each declare a single overload of `compare`, `divide` and `remainder`, and those signatures have been unchanged since the classes were introduced in Guava 10.0/11.0, so this cannot over-match. Also cover the two-argument radix overloads of `parseUnsignedInt` and `parseUnsignedLong`, and add the missing blank line before `PreferLongDivideUnsigned`.
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
1 participant
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.
Guava's
UnsignedInts/UnsignedLongshave noremainderUnsigned,compareUnsignedordivideUnsignedmethods — the real names areremainder,compareanddivide— so six of the eight method patterns in this block never matched anything. I checked every release from 10.0 to 33.5.0 withjavap: the*Unsignednames appear in none of them, and the real signatures have been byte-identical since the classes were introduced, so this was never a case of Guava dropping methods.Both
Prefer*RemainderUnsignedrecipes now renameremainderfirst, mirroring the existing Compare/Divide chaining;PreferLongCompareUnsignedandPreferLongDivideUnsignedhad(int, int)whereUnsignedLongstakes(long, long); andPreferLongParseUnsignedLongpointed atUnsignedIntsinstead ofUnsignedLongs(its display name and description also saidparseUnsignedInt). Separately,PreferIntegerRemainderUnsignedwas declared but never referenced fromNoGuava, so it would not have run even once fixed — it is now wired in.There were no tests for any of these recipes, so this adds
NoGuavaUnsignedTestcovering every conversion for both types plus the radix overloads, run through the aggregateNoGuavarecipe so it guards the wiring too. Confirmed these are genuine regression tests by stashing only the YAML:unsignedLongs()reported "Recipe was expected to make a change but made no changes" andunsignedInts()failed onremainder.Two follow-up commits: one drops descriptions advertising methods Guava does not have (
#compareUnsigned,#divideUnsigned,IntMath#addExact/#subtractExact/#multiplyExact) and correctscom.google.common.primitives.IntMathtocom.google.common.math.IntMath, a package that does not exist; the other matches on(..)rather than pinned parameter types, since pinning them is what let these mismatches hide, and the siblingPreferMath*Exactrecipes already do this.