diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/SetUnrecognized.java b/core/src/main/java/com/google/errorprone/bugpatterns/SetUnrecognized.java new file mode 100644 index 000000000000..afc49c81a3b1 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/SetUnrecognized.java @@ -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 IS_PROTO_SETTER = + instanceMethod() + .onDescendantOfAny("com.google.protobuf.MessageLite.Builder") + .withNameMatching(Pattern.compile("(add|set).*")); + + private static final Supplier ENUM_LITE = + VisitorState.memoize( + state -> state.getTypeFromString("com.google.protobuf.Internal.EnumLite")); +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index 23a64368749a..237fdeac8ac9 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -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; @@ -808,6 +809,7 @@ public static ScannerSupplier warningChecks() { SelfAssignment.class, SelfComparison.class, SelfEquals.class, + SetUnrecognized.class, ShouldHaveEvenArgs.class, SizeGreaterThanOrEqualsZero.class, StreamToString.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/SetUnrecognizedTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/SetUnrecognizedTest.java new file mode 100644 index 000000000000..860a4fdb82df --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/SetUnrecognizedTest.java @@ -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(); + } +} diff --git a/core/src/test/proto/proto3_test.proto b/core/src/test/proto/proto3_test.proto index 3553add1efc1..0b03993d5170 100644 --- a/core/src/test/proto/proto3_test.proto +++ b/core/src/test/proto/proto3_test.proto @@ -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; }