fix: use %s instead of %d format specifier in checkArgument #163
Conversation
googlebot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #163 +/- ##
============================================
+ Coverage 63.40% 63.46% +0.06%
Complexity 537 537
============================================
Files 30 30
Lines 4752 4752
Branches 427 427
============================================
+ Hits 3013 3016 +3
+ Misses 1579 1576 -3
Partials 160 160
Continue to review full report at Codecov.
|
| @@ -689,7 +689,7 @@ public long read( | |||
| Span span = startSpan(HttpStorageRpcSpans.SPAN_NAME_READ); | |||
| Scope scope = tracer.withSpan(span); | |||
| try { | |||
| checkArgument(position >= 0, "Position should be non-negative, is %d", position); | |||
| checkArgument(position >= 0, "Position should be non-negative, is %s", position); | |||
There was a problem hiding this comment.
I can't tell the difference between these two. %d looks correct to me. How about we use string concatenation with a plus sign here instead to avoid the issue.
I do notice this should probably not be inside the try block.
There was a problem hiding this comment.
%d had been correct with standard formater, but is incorrect when using guava's Precondition library formatting. (You can check documentation and code linked in the associated issue)
There was a problem hiding this comment.
The mere fact that we have to check documentation and that this is not obvious strongly suggests that we shouldn't use a format string at all. Just use string concatenation and call it a day.
There was a problem hiding this comment.
Done. I have used string concatenation + instead of %s. @elharo PTAL
Fixes #159