-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce Check{Argument,State}WithMessageAndArguments
Refaster rules
#1135
Introduce Check{Argument,State}WithMessageAndArguments
Refaster rules
#1135
Conversation
* Prefer {@link Preconditions#checkArgument(boolean, String, Object...)} over more verbose | ||
* alternatives. | ||
*/ | ||
// XXX: This check assumes that the format string only uses `%s` placeholders. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this isn't fully correct. One option is to create an BugChecker instead, but that's more work 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I went down the rabbit hole: #1139.
Looks good. No mutations were possible for these changes. |
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing this PR in favour of #1139.
* Prefer {@link Preconditions#checkArgument(boolean, String, Object...)} over more verbose | ||
* alternatives. | ||
*/ | ||
// XXX: This check assumes that the format string only uses `%s` placeholders. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I went down the rabbit hole: #1139.
Suggested commit message:
The tests pass, but when I run this against an internal code base relevant violations aren't replaced. Requires a deeper look.