You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This pull request fixes an inconsistent exception behavior in the Strings.repeat() method. When the calculated size of the resulting
String causes an integer overflow, the method currently throws an ArrayIndexOutOfBoundsException. This PR replaces it with an IllegalArgumentException, accompanied by a descriptive error message.
ArrayIndexOutOfBoundsException is an implementation detail that leaks
internal behavior to the caller. It does not communicate why the
operation failed — it only signals that something went wrong internally.
The Guava API consistently uses IllegalArgumentException to signal
that a caller passed an invalid argument. For example:
Strings.padStart() throws IllegalArgumentException for negative
padding sizes
Throwing ArrayIndexOutOfBoundsException in this case breaks that
contract and forces callers to catch an unintuitive low-level exception
when handling large repeat counts.
This fix was identified through an empirical static analysis of
Google Guava across 20 releases (v10.0 to v29.0) using SpotBugs,
which detected a consistent growth in the number of bugs, reaching
851 total in v29.0. This case illustrates how static analysis can
surface semantic inconsistencies beyond traditional bug patterns.
It's possible that what we should do is copy the JDK precedent of throwing an Error, especially since we encourage users to migrate to the JDK's equivalent method. But I'm not sure that a failed call to repeat really justifies that strong a response (even though of course a call might trigger an Error anyway but requesting a valid but unavailable amount of memory), so it's hard to get too excited about such a change. I also doubt that we'd change the error message except in perhaps the case of matching the JDK (though it probably isn't worth it).
We might have another API or two like this elsewhere in Guava. It probably won't be worth trying to change them, but this general situation is one that we may want to keep in mind for any future similar APIs.
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
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.
This pull request fixes an inconsistent exception behavior in the
Strings.repeat()method. When the calculated size of the resultingString causes an integer overflow, the method currently throws an
ArrayIndexOutOfBoundsException. This PR replaces it with anIllegalArgumentException, accompanied by a descriptive error message.ArrayIndexOutOfBoundsExceptionis an implementation detail that leaksinternal behavior to the caller. It does not communicate why the
operation failed — it only signals that something went wrong internally.
The Guava API consistently uses
IllegalArgumentExceptionto signalthat a caller passed an invalid argument. For example:
Preconditions.checkArgument()throwsIllegalArgumentExceptionStrings.padStart()throwsIllegalArgumentExceptionfor negativepadding sizes
Throwing
ArrayIndexOutOfBoundsExceptionin this case breaks thatcontract and forces callers to catch an unintuitive low-level exception
when handling large repeat counts.
This fix was identified through an empirical static analysis of
Google Guava across 20 releases (v10.0 to v29.0) using SpotBugs,
which detected a consistent growth in the number of bugs, reaching
851 total in v29.0. This case illustrates how static analysis can
surface semantic inconsistencies beyond traditional bug patterns.