Suppress ReturnValueIgnored errorprone issues

To be able to run this check on protobuf generated sources. Also changed string_field.cc from using a variable to using the SuppressWarnings annotation, because that takes less bytecode. PiperOrigin-RevId: 676534547
protocolbuffers · Sep 19, 2024 · bbbc7b9 · bbbc7b9
1 parent c47de0f
commit bbbc7b9
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 24 deletions.
diff --git a/src/google/protobuf/compiler/java/lite/enum_field.cc b/src/google/protobuf/compiler/java/lite/enum_field.cc
@@ -109,9 +109,6 @@ void SetEnumVariables(
     variables->insert({"unknown", (*variables)["default"]});
   }
 
-  // We use `x.getClass()` as a null check because it generates less bytecode
-  // than an `if (x == null) { throw ... }` statement.
-  (*variables)["null_check"] = "value.getClass();\n";
   // Calls to Annotate() use variable ranges to know which text to annotate.
   (*variables)["{"] = "";
   (*variables)["}"] = "";
@@ -693,17 +690,19 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateMembers(
   WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER,
                                context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void set$capitalized_name$(\n"
                  "    int index, $type$ value) {\n"
-                 "  $null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  ensure$capitalized_name$IsMutable();\n"
                  "  $name$_.setInt(index, value.getNumber());\n"
                  "}\n");
   WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER,
                                context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void add$capitalized_name$($type$ value) {\n"
-                 "  $null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  ensure$capitalized_name$IsMutable();\n"
                  "  $name$_.addInt(value.getNumber());\n"
                  "}\n");

diff --git a/src/google/protobuf/compiler/java/lite/message_field.cc b/src/google/protobuf/compiler/java/lite/message_field.cc
@@ -86,9 +86,6 @@ void SetMessageVariables(
   (*variables)["set_has_field_bit_to_local"] =
       GenerateSetBitToLocal(messageBitIndex);
 
-  // We use `x.getClass()` as a null check because it generates less bytecode
-  // than an `if (x == null) { throw ... }` statement.
-  (*variables)["null_check"] = "value.getClass();\n";
   // Annotations often use { and } to determine ranges.
   (*variables)["{"] = "";
   (*variables)["}"] = "";
@@ -178,8 +175,9 @@ void ImmutableMessageFieldLiteGenerator::GenerateMembers(
   // Field.Builder setField(Field value)
   WriteFieldDocComment(printer, descriptor_, context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void set$capitalized_name$($type$ value) {\n"
-                 "  $null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  $name$_ = value;\n"
                  "  $set_has_field_bit_message$\n"
                  "  }\n");
@@ -188,9 +186,10 @@ void ImmutableMessageFieldLiteGenerator::GenerateMembers(
   WriteFieldDocComment(printer, descriptor_, context_->options());
   printer->Print(
       variables_,
-      "@java.lang.SuppressWarnings({\"ReferenceEquality\"})\n"
+      "@java.lang.SuppressWarnings({\"ReferenceEquality\", "
+      "\"ReturnValueIgnored\"})\n"
       "private void merge$capitalized_name$($type$ value) {\n"
-      "  $null_check$"
+      "  value.getClass();  // minimal bytecode null check\n"
       "  if ($name$_ != null &&\n"
       "      $name$_ != $type$.getDefaultInstance()) {\n"
       "    $name$_ =\n"
@@ -384,8 +383,9 @@ void ImmutableMessageOneofFieldLiteGenerator::GenerateMembers(
   // Field.Builder setField(Field value)
   WriteFieldDocComment(printer, descriptor_, context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void set$capitalized_name$($type$ value) {\n"
-                 "  $null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  $oneof_name$_ = value;\n"
                  "  $set_oneof_case_message$;\n"
                  "}\n");
@@ -394,8 +394,9 @@ void ImmutableMessageOneofFieldLiteGenerator::GenerateMembers(
   WriteFieldDocComment(printer, descriptor_, context_->options());
   printer->Print(
       variables_,
+      "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
       "private void merge$capitalized_name$($type$ value) {\n"
-      "  $null_check$"
+      "  value.getClass();  // minimal bytecode null check\n"
       "  if ($has_oneof_case_message$ &&\n"
       "      $oneof_name$_ != $type$.getDefaultInstance()) {\n"
       "    $oneof_name$_ = $type$.newBuilder(($type$) $oneof_name$_)\n"
@@ -599,28 +600,31 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateMembers(
   // Builder setRepeatedField(int index, Field value)
   WriteFieldDocComment(printer, descriptor_, context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void set$capitalized_name$(\n"
                  "    int index, $type$ value) {\n"
-                 "  $null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  ensure$capitalized_name$IsMutable();\n"
                  "  $name$_.set(index, value);\n"
                  "}\n");
 
   // Builder addRepeatedField(Field value)
   WriteFieldDocComment(printer, descriptor_, context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void add$capitalized_name$($type$ value) {\n"
-                 "  $null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  ensure$capitalized_name$IsMutable();\n"
                  "  $name$_.add(value);\n"
                  "}\n");
 
   // Builder addRepeatedField(int index, Field value)
   WriteFieldDocComment(printer, descriptor_, context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void add$capitalized_name$(\n"
                  "    int index, $type$ value) {\n"
-                 "  $null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  ensure$capitalized_name$IsMutable();\n"
                  "  $name$_.add(index, value);\n"
                  "}\n");

diff --git a/src/google/protobuf/compiler/java/lite/string_field.cc b/src/google/protobuf/compiler/java/lite/string_field.cc
@@ -57,10 +57,6 @@ void SetPrimitiveVariables(
       absl::StrCat(static_cast<int32_t>(WireFormat::MakeTag(descriptor)));
   (*variables)["tag_size"] = absl::StrCat(
       WireFormat::TagSize(descriptor->number(), GetType(descriptor)));
-  // We use `x.getClass()` as a null check because it generates less bytecode
-  // than an `if (x == null) { throw ... }` statement.
-  (*variables)["null_check"] =
-      "  java.lang.Class<?> valueClass = value.getClass();\n";
 
   // TODO: Add @deprecated javadoc when generating javadoc is supported
   // by the proto compiler
@@ -228,9 +224,10 @@ void ImmutableStringFieldLiteGenerator::GenerateMembers(
   WriteFieldAccessorDocComment(printer, descriptor_, SETTER,
                                context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void set$capitalized_name$(\n"
                  "    java.lang.String value) {\n"
-                 "$null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  $set_has_field_bit_message$\n"
                  "  $name$_ = value;\n"
                  "}\n");
@@ -447,9 +444,10 @@ void ImmutableStringOneofFieldLiteGenerator::GenerateMembers(
   WriteFieldAccessorDocComment(printer, descriptor_, SETTER,
                                context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void ${$set$capitalized_name$$}$(\n"
                  "    java.lang.String value) {\n"
-                 "$null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  $set_oneof_case_message$;\n"
                  "  $oneof_name$_ = value;\n"
                  "}\n");
@@ -665,18 +663,20 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateMembers(
   WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER,
                                context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void set$capitalized_name$(\n"
                  "    int index, java.lang.String value) {\n"
-                 "$null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  ensure$capitalized_name$IsMutable();\n"
                  "  $name$_.set(index, value);\n"
                  "}\n");
   WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER,
                                context_->options());
   printer->Print(variables_,
+                 "@java.lang.SuppressWarnings(\"ReturnValueIgnored\")\n"
                  "private void add$capitalized_name$(\n"
                  "    java.lang.String value) {\n"
-                 "$null_check$"
+                 "  value.getClass();  // minimal bytecode null check\n"
                  "  ensure$capitalized_name$IsMutable();\n"
                  "  $name$_.add(value);\n"
                  "}\n");