Open
Conversation
All known users of this library also require Java 17 anyway.
Commit generated automatically with Google Error Prone.
Commit generated automatically with Eclipse.
Manually refactored.
Commit generated automatically with Google Error Prone.
Commit generated automatically with Google Error Prone.
There were only two cases where google/error-prone#4866 could be a problem, these were checked and handled manually. Any other instances of that bug would be detected by Error Prone, which checks the number of format arguments.
These classes are not public anyway.
The disadvantage is that now the constructor is public, but for this class it does not hurt. The change should be binary and source compatible because we keep all public methods, and nobody could inherit from the old class.
The disadvantage is that now the constructor is public, but for this class it does not hurt. The change should be binary and source compatible because we keep all public methods, and nobody could inherit from the old class.
These are only cases that are not public or where inheritance was already prevented with a private constructor.
Member
Author
|
@kfriedberger Are you interested in looking at this? |
kfriedberger
approved these changes
Apr 1, 2026
Member
kfriedberger
left a comment
kfriedberger
left a comment
There was a problem hiding this comment.
I made a quick review. Changes look good to me.
| String.format( | ||
| "The option %s specifies the path '%s' that is forbidden in safe mode " + reason + ".", | ||
| FluentIterable.<Object>of(optionName, path).append(args).toArray(Object.class))); | ||
| ("The option %s specifies the path '%s' that is forbidden in safe mode " + reason + ".") |
Member
There was a problem hiding this comment.
Why do you mix formatter and concatenation here?
Member
Author
There was a problem hiding this comment.
Ah, I also stumbled over this. The reason is that reason is a (part of the) format string. I added a comment explaining this.
Thanks for having a look!
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
2 participants
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.
JavaSMT is also doing this, so all projects depending on this library are now at Java 17 or newer.
The PR updates the build config and then applies several commits that make use of Java 17 features (pattern-matching instanceof, new switch syntax,
String.formatted(), andsealedclasses). Most of the commits are auto-generated.For reviewing, the interesting questions are:
Fixes #46.