Skip to content

Commit

Permalink
Flag setProtoField(Enum.UNRECOGNIZED), as this will fail at runtime.
Browse files Browse the repository at this point in the history
Flume: unknown commit
PiperOrigin-RevId: 650713065
  • Loading branch information
graememorgan authored and Error Prone Team committed Jul 10, 2024
1 parent 1b0a0d2 commit 3c8d497
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Copyright 2024 The Error Prone Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.isSubtype;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.suppliers.Supplier;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Type;
import java.util.regex.Pattern;

/** A BugPattern; see the summary. */
@BugPattern(
summary =
"Setting a proto field to an UNRECOGNIZED value will result in an exception at runtime when"
+ " building.",
severity = ERROR)
public final class SetUnrecognized extends BugChecker implements MethodInvocationTreeMatcher {
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!IS_PROTO_SETTER.matches(tree, state)) {
return NO_MATCH;
}
if (tree.getArguments().size() != 1) {
return NO_MATCH;
}
ExpressionTree arg = tree.getArguments().get(0);
var argSymbol = getSymbol(arg);
if (!argSymbol.getSimpleName().contentEquals("UNRECOGNIZED")) {
return NO_MATCH;
}
if (!isSubtype(argSymbol.owner.type, ENUM_LITE.get(state), state)) {
return NO_MATCH;
}
return describeMatch(tree);
}

private static final Matcher<ExpressionTree> IS_PROTO_SETTER =
instanceMethod()
.onDescendantOfAny("com.google.protobuf.MessageLite.Builder")
.withNameMatching(Pattern.compile("(add|set).*"));

private static final Supplier<Type> ENUM_LITE =
VisitorState.memoize(
state -> state.getTypeFromString("com.google.protobuf.Internal.EnumLite"));
}
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@
import com.google.errorprone.bugpatterns.SelfAssignment;
import com.google.errorprone.bugpatterns.SelfComparison;
import com.google.errorprone.bugpatterns.SelfEquals;
import com.google.errorprone.bugpatterns.SetUnrecognized;
import com.google.errorprone.bugpatterns.ShortCircuitBoolean;
import com.google.errorprone.bugpatterns.ShouldHaveEvenArgs;
import com.google.errorprone.bugpatterns.SizeGreaterThanOrEqualsZero;
Expand Down Expand Up @@ -808,6 +809,7 @@ public static ScannerSupplier warningChecks() {
SelfAssignment.class,
SelfComparison.class,
SelfEquals.class,
SetUnrecognized.class,
ShouldHaveEvenArgs.class,
SizeGreaterThanOrEqualsZero.class,
StreamToString.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright 2024 The Error Prone Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.bugpatterns;

import com.google.errorprone.CompilationTestHelper;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public final class SetUnrecognizedTest {
private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(SetUnrecognized.class, getClass());

@Test
public void positive() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.bugpatterns.proto.Proto3Test.TestProto3Enum;",
"import com.google.errorprone.bugpatterns.proto.Proto3Test.TestProto3Message;",
"class Test {",
" void test() {",
" // BUG: Diagnostic contains:",
" TestProto3Message.newBuilder().setMyEnum(TestProto3Enum.UNRECOGNIZED);",
" }",
"}")
.doTest();
}

@Test
public void negative() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.bugpatterns.proto.Proto3Test.TestProto3Enum;",
"import com.google.errorprone.bugpatterns.proto.Proto3Test.TestProto3Message;",
"class Test {",
" void test() {",
" TestProto3Message.newBuilder().setMyEnum(TestProto3Enum.VALUE_1);",
" }",
"}")
.doTest();
}
}
11 changes: 9 additions & 2 deletions core/src/test/proto/proto3_test.proto
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ package com.google.errorprone.bugpatterns;
option java_package = "com.google.errorprone.bugpatterns.proto";

message TestProto3Message {
string myString = 1;
bytes myBytes = 2;
string my_string = 1;
bytes my_bytes = 2;
TestProto3Enum my_enum = 3;
}

enum TestProto3Enum {
UNKNOWN = 0;
VALUE_1 = 1;
VALUE_2 = 2;
}

0 comments on commit 3c8d497

Please sign in to comment.