checkapi -= new field defaults for unions

I don't think this requirement is intentional,
it's for parcelables.

A new server sending a new union field to an old server
is rejected. An old server couldn't send the new type,
so I don't think there is any need for the default
field for unions.

Bug: N/A (email thread)
Test: N/A
Change-Id: I4227795f14f8d6ce569fa3f5432ddaad5494a44b

1 files changed, 48 insertions, 45 deletions

diff --git a/aidl_checkapi.cpp b/aidl_checkapi.cpp
index 039c01b3..a18a8f81 100644
--- a/aidl_checkapi.cpp
+++ b/aidl_checkapi.cpp
@@ -302,57 +302,60 @@ static bool are_compatible_parcelables(const AidlDefinedType& older, const AidlT
     }
   }
 
-  for (size_t i = old_fields.size(); i < new_fields.size(); i++) {
-    const auto& new_field = new_fields.at(i);
-    if (new_field->HasUsefulDefaultValue()) {
-      continue;
-    }
-
-    // enum can't be nullable, but it's okay if it has 0 as a valid enumerator.
-    if (const auto& enum_decl = new_types.GetEnumDeclaration(new_field->GetType());
-        enum_decl != nullptr) {
-      if (HasZeroEnumerator(*enum_decl)) {
+  // New fields must have default values.
+  if (older.AsUnionDeclaration() == nullptr) {
+    for (size_t i = old_fields.size(); i < new_fields.size(); i++) {
+      const auto& new_field = new_fields.at(i);
+      if (new_field->HasUsefulDefaultValue()) {
         continue;
       }
 
-      // TODO(b/142893595): Rephrase the message: "provide a default value or make sure ..."
-      AIDL_ERROR(new_field) << "Field '" << new_field->GetName() << "' of enum '"
-                            << enum_decl->GetName()
-                            << "' can't be initialized as '0'. Please make sure '"
-                            << enum_decl->GetName() << "' has '0' as a valid value.";
-      compatible = false;
-      continue;
-    }
+      // enum can't be nullable, but it's okay if it has 0 as a valid enumerator.
+      if (const auto& enum_decl = new_types.GetEnumDeclaration(new_field->GetType());
+          enum_decl != nullptr) {
+        if (HasZeroEnumerator(*enum_decl)) {
+          continue;
+        }
+
+        // TODO(b/142893595): Rephrase the message: "provide a default value or make sure ..."
+        AIDL_ERROR(new_field) << "Field '" << new_field->GetName() << "' of enum '"
+                              << enum_decl->GetName()
+                              << "' can't be initialized as '0'. Please make sure '"
+                              << enum_decl->GetName() << "' has '0' as a valid value.";
+        compatible = false;
+        continue;
+      }
 
-    // Old API versions may suffer from the issue presented here. There is
-    // only a finite number in Android, which we must allow indefinitely.
-    struct HistoricalException {
-      std::string canonical;
-      std::string field;
-    };
-    static std::vector<HistoricalException> exceptions = {
-        {"android.net.DhcpResultsParcelable", "serverHostName"},
-        {"android.net.ResolverParamsParcel", "resolverOptions"},
-    };
-    bool excepted = false;
-    for (const HistoricalException& exception : exceptions) {
-      if (older.GetCanonicalName() == exception.canonical &&
-          new_field->GetName() == exception.field) {
-        excepted = true;
-        break;
+      // Old API versions may suffer from the issue presented here. There is
+      // only a finite number in Android, which we must allow indefinitely.
+      struct HistoricalException {
+        std::string canonical;
+        std::string field;
+      };
+      static std::vector<HistoricalException> exceptions = {
+          {"android.net.DhcpResultsParcelable", "serverHostName"},
+          {"android.net.ResolverParamsParcel", "resolverOptions"},
+      };
+      bool excepted = false;
+      for (const HistoricalException& exception : exceptions) {
+        if (older.GetCanonicalName() == exception.canonical &&
+            new_field->GetName() == exception.field) {
+          excepted = true;
+          break;
+        }
       }
+      if (excepted) continue;
+
+      AIDL_ERROR(new_field)
+          << "Field '" << new_field->GetName()
+          << "' does not have a useful default in some backends. Please either provide a default "
+             "value for this field or mark the field as @nullable. This value or a null value will "
+             "be used automatically when an old version of this parcelable is sent to a process "
+             "which understands a new version of this parcelable. In order to make sure your code "
+             "continues to be backwards compatible, make sure the default or null value does not "
+             "cause a semantic change to this parcelable.";
+      compatible = false;
     }
-    if (excepted) continue;
-
-    AIDL_ERROR(new_field)
-        << "Field '" << new_field->GetName()
-        << "' does not have a useful default in some backends. Please either provide a default "
-           "value for this field or mark the field as @nullable. This value or a null value will "
-           "be used automatically when an old version of this parcelable is sent to a process "
-           "which understands a new version of this parcelable. In order to make sure your code "
-           "continues to be backwards compatible, make sure the default or null value does not "
-           "cause a semantic change to this parcelable.";
-    compatible = false;
   }
 
   compatible = are_compatible_constants(older, newer) && compatible;