checkapi: enum type for new field should have 0
checkapi checks if new fields has a default value or it is nullable. But
enum type can't have a default value(b/142893595) and can't be nullable.
So checkapi should allow enum types as a new field if enum types
have 0 as a valid value (enum type's implicit default value).
Bug: 171003934
Test: aidl_unittests / aidl_integeration_test
Change-Id: Ie4f262121e623cfe983111beb672a311b2a23128
diff --git a/aidl_checkapi.cpp b/aidl_checkapi.cpp
index f20b5ca..a539589 100644
--- a/aidl_checkapi.cpp
+++ b/aidl_checkapi.cpp
@@ -182,8 +182,18 @@
return specifier.IsNullable();
}
+static bool HasZeroEnumerator(const AidlEnumDeclaration& enum_decl) {
+ return std::any_of(enum_decl.GetEnumerators().begin(), enum_decl.GetEnumerators().end(),
+ [&](const unique_ptr<AidlEnumerator>& enumerator) {
+ return enumerator->GetValue()->ValueString(
+ enum_decl.GetBackingType(), AidlConstantValueDecorator) == "0";
+ });
+}
+
template <typename ParcelableType>
-static bool are_compatible_parcelables(const ParcelableType& older, const ParcelableType& newer) {
+static bool are_compatible_parcelables(const ParcelableType& older, const AidlTypenames&,
+ const ParcelableType& newer,
+ const AidlTypenames& new_types) {
const auto& old_fields = older.GetFields();
const auto& new_fields = newer.GetFields();
if (old_fields.size() > new_fields.size()) {
@@ -231,37 +241,60 @@
for (size_t i = old_fields.size(); i < new_fields.size(); i++) {
const auto& new_field = new_fields.at(i);
- if (!new_field->GetDefaultValue() && !has_usable_nil_type(new_field->GetType())) {
- // 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 (new_field->GetDefaultValue()) {
+ continue;
}
+
+ // null is accepted as a valid default value
+ if (has_usable_nil_type(new_field->GetType())) {
+ 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;
+ }
+ }
+ 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;
}
return compatible;
}
@@ -368,8 +401,8 @@
compatible = false;
continue;
}
- compatible &= are_compatible_parcelables(*(old_type->AsStructuredParcelable()),
- *(new_type->AsStructuredParcelable()));
+ compatible &= are_compatible_parcelables(*(old_type->AsStructuredParcelable()), *old_tns,
+ *(new_type->AsStructuredParcelable()), *new_tns);
} else if (old_type->AsUnionDeclaration() != nullptr) {
if (new_type->AsUnionDeclaration() == nullptr) {
AIDL_ERROR(new_type) << "Type mismatch: " << old_type->GetCanonicalName()
@@ -378,8 +411,8 @@
compatible = false;
continue;
}
- compatible &= are_compatible_parcelables(*(old_type->AsUnionDeclaration()),
- *(new_type->AsUnionDeclaration()));
+ compatible &= are_compatible_parcelables(*(old_type->AsUnionDeclaration()), *old_tns,
+ *(new_type->AsUnionDeclaration()), *new_tns);
} else if (old_type->AsEnumDeclaration() != nullptr) {
if (new_type->AsEnumDeclaration() == nullptr) {
AIDL_ERROR(new_type) << "Type mismatch: " << old_type->GetCanonicalName()