Update errorprone and lombok and fix warnings and errors #270
Conversation
vprithvi
commented
Oct 12, 2017
Signed-off-by: Prithvi Raj <p.r@uber.com>
- Update errorprone to 2.1.1 - Update lombok to 1.16.18 - Disable the InstanceOfAndCastMatchWrongType and NestedInstanceOfConditions checks to ensure compatibility with lombok. See google/error-prone#750 Signed-off-by: Prithvi Raj <p.r@uber.com>
Signed-off-by: Prithvi Raj <p.r@uber.com>
| } | ||
| tasks.withType(JavaCompile) { | ||
| // These are disabled because of a bug with errorprone. See https://github.com/google/error-prone/issues/750 |
There was a problem hiding this comment.
not following how that issue is related to disabling checks
There was a problem hiding this comment.
Enabling those checks causes a runtime exception in the error prone compiler
There was a problem hiding this comment.
so using @Data is a convenience, but turning off InstanceOfAndCastMatchWrongType sounds like it could mask real errors.
There was a problem hiding this comment.
InstanceOfAndCastMatchWrongType doesn't exist in the current version of errorprone that the project uses.
Could you please justify your statement of using @Data is a convenience. Have there been no bugs made in manually writing out the convenience functions that @Data/@Value provides? If there have, is the cost of fixing the bugs introduced by incorrect hashcode/tostring/getters less than the potential time saved by having these new error checks enabled?
Has the time taken to write boilerplate code been factored into your argument?
There was a problem hiding this comment.
the methods that @Data/@Value generate can be just as easily created by the IDE.
There was a problem hiding this comment.
I really feel that you are arguing with me for the sake for arguing.
How would you maintain consistency between the IDE generated code?
Should bumping a couple of versions have so much friction?
| @@ -268,7 +268,7 @@ private SpanContext createNewContext(String debugId) { | |||
|
|
|||
| byte flags = 0; | |||
| if (debugId != null) { | |||
| flags |= SpanContext.flagSampled | SpanContext.flagDebug; | |||
| flags = (byte) (flags | SpanContext.flagSampled | SpanContext.flagDebug); | |||
There was a problem hiding this comment.
all operands are already of type byte, why do we need to re-cast?
There was a problem hiding this comment.
Because of assignment conversion. See http://docs.oracle.com/javase/specs/jls/se7/html/jls-5.html#jls-5.2 and http://errorprone.info/bugpattern/NarrowingCompoundAssignment
The result of the bitwise OR is being promoted to a int which was previously implicitly cast to a byte.
Codecov Report
@@ Coverage Diff @@
## master #270 +/- ##
=========================================
Coverage ? 82.63%
Complexity ? 535
=========================================
Files ? 87
Lines ? 2021
Branches ? 237
=========================================
Hits ? 1670
Misses ? 253
Partials ? 98
Continue to review full report at Codecov.
|
