Skip to content

Commit e691be5

Browse files
graememorganError Prone Team
authored andcommitted
Fix (behind a flag) a silly bug in RedundantSetterCall.
I clearly wanted "if this isn't a proto, return empty" here, and what I wrote was "if this is a lite proto, return empty". The bug would have been more obvious if GeneratedMessage extended GeneratedMessageLite (this is true if you drop the "Generated"!), as the code would have been hopelessly broken, but this isn't the case. To add further to the pain, this is a no-op given lite proto oneof enums don't implement the marker interface. I'm seeing if I can fix that in unknown commit, but I'll need _a_ flag in place so that all the ensuing detected errors don't break. Also this is borderline impossible to test given you can't compile lite protos into the same target as full-fat protos. PiperOrigin-RevId: 806285118
1 parent ce1a422 commit e691be5

File tree

1 file changed

+6
-6
lines changed

1 file changed

+6
-6
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/RedundantSetterCall.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,12 @@ && isWithinAutoValueBuilder(symbol, state)
110110
state)));
111111

112112
private final boolean improvements;
113+
private final boolean matchLiteProtos;
113114

114115
@Inject
115116
RedundantSetterCall(ErrorProneFlags flags) {
116117
this.improvements = flags.getBoolean("RedundantSetterCall:Improvements").orElse(true);
118+
this.matchLiteProtos = flags.getBoolean("OneOfChecks:MatchLiteProtos").orElse(true);
117119
}
118120

119121
@Override
@@ -127,7 +129,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
127129

128130
ListMultimap<Field, FieldWithValue> setters = ArrayListMultimap.create();
129131
ImmutableMap<String, OneOfField> oneOfSetters =
130-
isProto ? scanForOneOfSetters(getType(tree), state) : ImmutableMap.of();
132+
isProto ? scanForOneOfSetters(owner, state) : ImmutableMap.of();
131133
ImmutableSet<String> fieldNames = isProto ? getFields(owner) : ImmutableSet.of();
132134

133135
Type type = ASTHelpers.getReturnType(tree);
@@ -184,14 +186,12 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
184186
return Description.NO_MATCH;
185187
}
186188

187-
private static ImmutableMap<String, OneOfField> scanForOneOfSetters(
188-
Type type, VisitorState state) {
189-
var owner = getUpperBound(type, state.getTypes()).tsym.owner;
190-
if (owner == null || isSubtype(owner.type, GENERATED_MESSAGE_LITE.get(state), state)) {
189+
private ImmutableMap<String, OneOfField> scanForOneOfSetters(Symbol proto, VisitorState state) {
190+
if (!matchLiteProtos && isSubtype(proto.type, GENERATED_MESSAGE_LITE.get(state), state)) {
191191
return ImmutableMap.of();
192192
}
193193
var builder = ImmutableMap.<String, OneOfField>builder();
194-
for (Symbol element : getEnclosedElements(owner)) {
194+
for (Symbol element : getEnclosedElements(proto)) {
195195
if (!ONE_OF_ENUM.apply(element.type, state)) {
196196
continue;
197197
}

0 commit comments

Comments
 (0)