fix const-ref with --dumpapi & --checkapi
--dumpapi dumps enumerator references as fully-qualified.
--checkapi compares const-expr values with their literals.
: in checkapi mode, we don't resolve type/field references. So we
can't evaluate const-ref values.
Bug: 142893595
Test: aidl_unittests
Change-Id: Ia564c227c0683c646b6f1654a0b9349125d4992b
diff --git a/aidl_checkapi.cpp b/aidl_checkapi.cpp
index d513f5c..c3eff08 100644
--- a/aidl_checkapi.cpp
+++ b/aidl_checkapi.cpp
@@ -121,8 +121,8 @@
const auto new_c = found->second;
compatible &= are_compatible_types(old_c->GetType(), new_c->GetType());
- const string old_value = old_c->ValueString(AidlConstantValueDecorator);
- const string new_value = new_c->ValueString(AidlConstantValueDecorator);
+ const string old_value = old_c->GetValue().Literal();
+ const string new_value = new_c->GetValue().Literal();
if (old_value != new_value) {
AIDL_ERROR(newer) << "Changed constant value: " << older.GetCanonicalName() << "."
<< old_c->GetName() << " from " << old_value << " to " << new_value << ".";
@@ -207,8 +207,7 @@
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";
+ return enumerator->GetValue()->Literal() == "0";
});
}
@@ -236,8 +235,8 @@
const auto& new_field = new_fields.at(i);
compatible &= are_compatible_types(old_field->GetType(), new_field->GetType());
- const string old_value = old_field->ValueString(AidlConstantValueDecorator);
- const string new_value = new_field->ValueString(AidlConstantValueDecorator);
+ string old_value = old_field->GetDefaultValue() ? old_field->GetDefaultValue()->Literal() : "";
+ string new_value = new_field->GetDefaultValue() ? new_field->GetDefaultValue()->Literal() : "";
if (old_value != new_value) {
AIDL_ERROR(new_field) << "Changed default value: " << old_value << " to " << new_value << ".";
compatible = false;
@@ -346,10 +345,8 @@
compatible = false;
continue;
}
- const string old_value =
- old_enum_map[name]->ValueString(older.GetBackingType(), AidlConstantValueDecorator);
- const string new_value =
- new_enum_map[name]->ValueString(newer.GetBackingType(), AidlConstantValueDecorator);
+ const string old_value = old_enum_map[name]->Literal();
+ const string new_value = new_enum_map[name]->Literal();
if (old_value != new_value) {
AIDL_ERROR(newer) << "Changed enumerator value: " << older.GetCanonicalName() << "::" << name
<< " from " << old_value << " to " << new_value << ".";
diff --git a/aidl_language.cpp b/aidl_language.cpp
index ad6aa8b..ed01066 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -614,8 +614,17 @@
return true;
}
-std::string AidlConstantValueDecorator(const AidlTypeSpecifier& /*type*/,
+std::string AidlConstantValueDecorator(const AidlTypeSpecifier& type,
const std::string& raw_value) {
+ if (type.IsArray()) {
+ return raw_value;
+ }
+
+ if (auto defined_type = type.GetDefinedType(); defined_type) {
+ auto enum_type = defined_type->AsEnumDeclaration();
+ AIDL_FATAL_IF(!enum_type, type) << "Invalid type for \"" << raw_value << "\"";
+ return type.GetName() + "." + raw_value.substr(raw_value.find_last_of('.') + 1);
+ }
return raw_value;
}
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 4d6ee2e..d30a63e 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -1599,6 +1599,56 @@
actual);
}
+TEST_F(AidlTest, ApiDumpWithEnums) {
+ io_delegate_.SetFileContents("foo/bar/Enum.aidl",
+ "package foo.bar;\n"
+ "enum Enum {\n"
+ " FOO,\n"
+ " BAR = FOO + 1,\n"
+ "}\n");
+
+ vector<string> args = {"aidl", "--dumpapi", "-I . ", "-o dump", "foo/bar/Enum.aidl"};
+ Options options = Options::From(args);
+ CaptureStderr();
+ EXPECT_TRUE(dump_api(options, io_delegate_));
+ EXPECT_EQ("", GetCapturedStderr());
+ string actual;
+ EXPECT_TRUE(io_delegate_.GetWrittenContents("dump/foo/bar/Enum.aidl", &actual));
+ EXPECT_EQ(string(kPreamble).append("package foo.bar;\n"
+ "enum Enum {\n"
+ " FOO = 0,\n"
+ " BAR = 1,\n"
+ "}\n"),
+ actual);
+}
+
+TEST_F(AidlTest, ApiDumpWithEnumDefaultValues) {
+ io_delegate_.SetFileContents("foo/bar/Enum.aidl",
+ "package foo.bar;\n"
+ "enum Enum {\n"
+ " FOO,\n"
+ "}\n");
+ io_delegate_.SetFileContents("foo/bar/Foo.aidl",
+ "package foo.bar;\n"
+ "import foo.bar.Enum;\n"
+ "parcelable Foo {\n"
+ " Enum e = Enum.FOO;\n"
+ "}\n");
+
+ vector<string> args = {"aidl", "--dumpapi", "-I . ", "-o dump", "foo/bar/Foo.aidl"};
+ Options options = Options::From(args);
+ CaptureStderr();
+ EXPECT_TRUE(dump_api(options, io_delegate_));
+ EXPECT_EQ("", GetCapturedStderr());
+ string actual;
+ EXPECT_TRUE(io_delegate_.GetWrittenContents("dump/foo/bar/Foo.aidl", &actual));
+ EXPECT_EQ(string(kPreamble).append("package foo.bar;\n"
+ "parcelable Foo {\n"
+ " foo.bar.Enum e = foo.bar.Enum.FOO;\n"
+ "}\n"),
+ actual);
+}
+
TEST_F(AidlTest, CheckNumGenericTypeSecifier) {
const string expected_list_stderr =
"ERROR: p/IFoo.aidl:1.37-41: List can only have one type parameter, but got: "
@@ -1886,6 +1936,18 @@
EXPECT_TRUE(::android::aidl::check_api(options, io_delegate_));
}
+TEST_F(AidlTest, CheckApi_EnumFieldsWithDefaultValues) {
+ Options options = Options::From("aidl --checkapi old new");
+ const string foo_definition = "package p; parcelable Foo{ p.Enum e = p.Enum.FOO; }";
+ const string enum_definition = "package p; enum Enum { FOO }";
+ io_delegate_.SetFileContents("old/p/Foo.aidl", foo_definition);
+ io_delegate_.SetFileContents("old/p/Enum.aidl", enum_definition);
+ io_delegate_.SetFileContents("new/p/Foo.aidl", foo_definition);
+ io_delegate_.SetFileContents("new/p/Enum.aidl", enum_definition);
+
+ EXPECT_TRUE(::android::aidl::check_api(options, io_delegate_));
+}
+
class AidlTestCompatibleChanges : public AidlTest {
protected:
Options options_ = Options::From("aidl --checkapi old new");