[AutoValue] NarrowingCompoundAssignment warning for primitives
ben-manes opened this issue · 3 comments
The set$0
field is generated if a primitive setting is used, e.g. boolean or float. It results in an warning like below,
AutoValue_PolicyStats_Metric.java:137: warning:
[NarrowingCompoundAssignment] Compound assignments from int to byte hide lossy casts
set$0 |= 1;
^
(see https://errorprone.info/bugpattern/NarrowingCompoundAssignment)
Did you mean 'set$0 = (byte) (set$0 | 1);'?
static final class Builder extends PolicyStats.Metric.Builder {
private boolean required;
private byte set$0;
public PolicyStats.Metric.Builder required(boolean required) {
this.required = required;
set$0 |= 1;
return this;
}
This can be suppressed using.
@AutoValue.Builder @CopyAnnotations
@SuppressWarnings("NarrowingCompoundAssignment")
This is NarrowingCompoundAssignment from Error Prone, right? I wonder if its check should be refined. This case is clearly safe, unlike set$0 += 1
for example. On the other hand, it's also not very difficult to change AutoValue so it emits the "Did you mean" version instead.
yep, this was from errorprone. I think updating to the suggestion is best. An even more explicit way to write it might be as 0b1
?
I've updated Error Prone so it should no longer warn about these lines (after the next release). However I think I will also update AutoValue so it doesn't trigger the warning even from the current Error Prone. The "Did you mean" suggestion is this:
set$0 = (byte) (set$0 | 1);
But I think this would be simpler and should also work:
set$0 |= (byte) 1;
Regarding 0b1
, I think the existing scheme is probably better. It uses 1
instead of 0x1
, but will start using explicit hex constants as soon as hex diverges from decimal: 0x10
, 0x20
, ..., 0x8000_0000
. I think bit 30 is a lot clearer as 0x4000_0000
than as 0b100_0000_0000_0000_0000_0000_0000_0000
. We could also use 1 << 30
, but in practice I don't think that's really clearer.