Bump Error Prone to 2.49.0 and fix new warnings#3030
Conversation
Previously `NATURAL_ORDER` was never properly used, it was only checked using reference equality but its `compare` method was not called at all. Future Error Prone versions also warned about this reference equality check. Therefore simplify the implementation by using `null` for natural order.
| /** See {@link AccessibleObject#canAccess(Object)} (Java >= 9) */ | ||
| @SuppressWarnings("InvalidLink") // suppress Error Prone warning about link to Java 9 method |
There was a problem hiding this comment.
Refers to the {@link AccessibleObject#canAccess(Object)} in the line above
| assertThrows(IllegalArgumentException.class, () -> writer.value(Float.valueOf(Float.NaN))); | ||
| assertThrows(IllegalArgumentException.class, () -> writer.value((Number) Float.NaN)); |
There was a problem hiding this comment.
Used (Number) ... casts here to point out that the main purpose is to call the value(Number) overload instead of value(double) or value(float).
The previous code using Double.valueOf and Float.valueOf was achieving the same; but Error Prone reported AssertThrowsMinimizer warnings there.
(It still reports those warnings for (Number) ... though.)
| @Test | ||
| public void testStrictLazyNansAndInfinities() throws IOException { |
There was a problem hiding this comment.
Noticed that unlike JsonWriterTest the test here did not check LazilyParsedNumber, so I added this new method.
| // Should contain entries in same order | ||
| assertThat(new ArrayList<>(entrySet)).isEqualTo(expectedEntrySet); | ||
|
|
||
| Map.Entry<String, JsonElement> dummyEntry = new SimpleEntry<>("c", new JsonPrimitive(3)); |
There was a problem hiding this comment.
Used the name dummyEntry instead of entry because a few lines below there is already a local variable named entry.
| () -> { | ||
| runTest( | ||
| className, | ||
| c -> { | ||
| fail("Class should have been removed during shrinking: " + c); | ||
| }); | ||
| }); | ||
| () -> | ||
| runTest( | ||
| className, c -> fail("Class should have been removed during shrinking: " + c))); |
There was a problem hiding this comment.
Converted block -> { ... } to -> ....
Was reported by Error Prone AssertThrowsBlockToExpression.
eamonnmcmanus
left a comment
eamonnmcmanus
left a comment
There was a problem hiding this comment.
This is fantastic, thanks for doing it! I wasn't looking forward to fixing all those problems.
2 participants
Bumps Error Prone to 2.49.0 and fixes the new warnings it reports.
This should fix the build error of #3022.
The largest changes are:
LinkedTreeMapto removeNATURAL_ORDERand related reference equality checks (first commit)assertThrows(...)callsPlease let me know if you would prefer other local variable names (currently I mostly used "
value" for the object to be serialized) or if you have any suggestions for changes.