Generic validation of AidlDefinedType
This treats all defined types the same when parsing, and since it is
convenient, we also always show generic failures in addition to
language-specific failures.
Before:
ERROR: a/path/Foo.aidl:1.26-43: The NDK and Rust backend does not
support ParcelableHolder yet.
After:
ERROR: a/path/Foo.aidl:1.26-43: Arrays of ParcelableHolder are not
supported.
ERROR: a/path/Foo.aidl:1.26-43: The NDK and Rust backend does not
support ParcelableHolder yet.
Bug: 169958329
Test: aidl_parser_fuzzer tests/corpus/*
Test: atest aidl_unittests
Change-Id: I99804f23eb687664b479a58d56b6c03b638a4ce9
diff --git a/aidl.cpp b/aidl.cpp
index d6d2735..c4a2b16 100644
--- a/aidl.cpp
+++ b/aidl.cpp
@@ -573,25 +573,48 @@
for (const auto& defined_type : types) {
AIDL_FATAL_IF(defined_type == nullptr, main_parser->FileName());
- // Language specific validation
- if (!defined_type->LanguageSpecificCheckValid(*typenames, options.TargetLanguage())) {
- return AidlError::BAD_TYPE;
+ // Ensure type is exactly one of the following:
+ AidlInterface* interface = defined_type->AsInterface();
+ AidlStructuredParcelable* parcelable = defined_type->AsStructuredParcelable();
+ AidlParcelable* unstructured_parcelable = defined_type->AsUnstructuredParcelable();
+ AidlEnumDeclaration* enum_decl = defined_type->AsEnumDeclaration();
+ AIDL_FATAL_IF(!!interface + !!parcelable + !!unstructured_parcelable + !!enum_decl != 1,
+ defined_type);
+
+ // Ensure that foo.bar.IFoo is defined in <some_path>/foo/bar/IFoo.aidl
+ if (num_defined_types == 1 && !check_filename(input_file_name, *defined_type)) {
+ return AidlError::BAD_PACKAGE;
}
- AidlParcelable* unstructuredParcelable = defined_type->AsUnstructuredParcelable();
- if (unstructuredParcelable != nullptr) {
- if (!unstructuredParcelable->CheckValid(*typenames)) {
+ {
+ bool valid_type = true;
+
+ if (!is_check_api) {
+ // Ideally, we could do this for check api, but we can't resolve imports
+ if (!defined_type->CheckValid(*typenames)) {
+ valid_type = false;
+ }
+ }
+
+ if (!defined_type->LanguageSpecificCheckValid(*typenames, options.TargetLanguage())) {
+ valid_type = false;
+ }
+
+ if (!valid_type) {
return AidlError::BAD_TYPE;
}
- bool isStable = unstructuredParcelable->IsStableApiParcelable(options.TargetLanguage());
+ }
+
+ if (unstructured_parcelable != nullptr) {
+ bool isStable = unstructured_parcelable->IsStableApiParcelable(options.TargetLanguage());
if (options.IsStructured() && !isStable) {
- AIDL_ERROR(unstructuredParcelable)
+ AIDL_ERROR(unstructured_parcelable)
<< "Cannot declared parcelable in a --structured interface. Parcelable must be defined "
"in AIDL directly.";
return AidlError::NOT_STRUCTURED;
}
if (options.FailOnParcelable()) {
- AIDL_ERROR(unstructuredParcelable)
+ AIDL_ERROR(unstructured_parcelable)
<< "Refusing to generate code with unstructured parcelables. Declared parcelables "
"should be in their own file and/or cannot be used with --structured interfaces.";
// Continue parsing for more errors
@@ -616,27 +639,6 @@
if (!success) return AidlError::NOT_STRUCTURED;
}
- // Ensure that a type is either an interface, structured parcelable, or
- // enum.
- AidlInterface* interface = defined_type->AsInterface();
- AidlStructuredParcelable* parcelable = defined_type->AsStructuredParcelable();
- AidlEnumDeclaration* enum_decl = defined_type->AsEnumDeclaration();
- AIDL_FATAL_IF(!!interface + !!parcelable + !!enum_decl != 1, defined_type);
-
- // Ensure that foo.bar.IFoo is defined in <some_path>/foo/bar/IFoo.aidl
- if (num_defined_types == 1 && !check_filename(input_file_name, *defined_type)) {
- return AidlError::BAD_PACKAGE;
- }
-
- // Check the referenced types in parsed_doc to make sure we've imported them
- if (!is_check_api) {
- // No need to do this for check api because all typespecs are already
- // using fully qualified name and we don't import in AIDL files.
- if (!defined_type->CheckValid(*typenames)) {
- return AidlError::BAD_TYPE;
- }
- }
-
if (interface != nullptr) {
// add the meta-method 'int getInterfaceVersion()' if version is specified.
if (options.Version() > 0) {
diff --git a/aidl_language.cpp b/aidl_language.cpp
index 15ecd08..87159e6 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -490,8 +490,8 @@
const size_t num_params = GetTypeParameters().size();
if (type_name == "List") {
if (num_params > 1) {
- AIDL_ERROR(this) << " List cannot have type parameters more than one, but got "
- << "'" << ToString() << "'";
+ AIDL_ERROR(this) << "List can only have one type parameter, but got: '" << ToString()
+ << "'";
return false;
}
} else if (type_name == "Map") {
@@ -953,10 +953,6 @@
}
if (this->IsGeneric()) {
if (this->GetName() == "List") {
- if (this->GetTypeParameters().size() != 1) {
- AIDL_ERROR(this) << "List must have only one type parameter.";
- return false;
- }
if (lang == Options::Language::CPP) {
const string& contained_type = this->GetTypeParameters()[0]->GetName();
if (!(contained_type == "String" || contained_type == "IBinder")) {
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 0f2f7e6..5b94a85 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -246,6 +246,42 @@
EXPECT_NE(nullptr, Parse("IFoo.aidl", "interface IFoo { } // foo", typenames_, GetLanguage()));
}
+TEST_P(AidlTest, InterfaceRequiresCorrectPath) {
+ const string expected_stderr =
+ "ERROR: a/Foo.aidl:1.11-21: IBar should be declared in a file called a/IBar.aidl\n";
+ const std::string file_contents = "package a; interface IBar {}";
+ CaptureStderr();
+ EXPECT_EQ(nullptr, Parse("a/Foo.aidl", file_contents, typenames_, GetLanguage()));
+ EXPECT_EQ(expected_stderr, GetCapturedStderr()) << file_contents;
+}
+
+TEST_P(AidlTest, ParcelableRequiresCorrectPath) {
+ const string expected_stderr =
+ "ERROR: a/Foo.aidl:1.11-21: Bar should be declared in a file called a/Bar.aidl\n";
+ const std::string file_contents = "package a; interface Bar {}";
+ CaptureStderr();
+ EXPECT_EQ(nullptr, Parse("a/Foo.aidl", file_contents, typenames_, GetLanguage()));
+ EXPECT_EQ(expected_stderr, GetCapturedStderr()) << file_contents;
+}
+
+TEST_P(AidlTest, UnstructuredParcelableRequiresCorrectPath) {
+ const string expected_stderr =
+ "ERROR: a/Foo.aidl:1.22-26: Bar should be declared in a file called a/Bar.aidl\n";
+ const std::string file_contents = "package a; parcelable Bar cpp_header \"anything.h\";";
+ CaptureStderr();
+ EXPECT_EQ(nullptr, Parse("a/Foo.aidl", file_contents, typenames_, GetLanguage()));
+ EXPECT_EQ(expected_stderr, GetCapturedStderr()) << file_contents;
+}
+
+TEST_P(AidlTest, EnumRequiresCorrectPath) {
+ const string expected_stderr =
+ "ERROR: a/Foo.aidl:1.16-20: Bar should be declared in a file called a/Bar.aidl\n";
+ const std::string file_contents = "package a; enum Bar { A, }";
+ CaptureStderr();
+ EXPECT_EQ(nullptr, Parse("a/Foo.aidl", file_contents, typenames_, GetLanguage()));
+ EXPECT_EQ(expected_stderr, GetCapturedStderr()) << file_contents;
+}
+
TEST_P(AidlTest, RejectsArraysOfBinders) {
import_paths_.emplace("");
io_delegate_.SetFileContents("bar/IBar.aidl",
@@ -373,24 +409,24 @@
AidlError error;
const string method = "package a; @nullable parcelable IFoo cpp_header \"IFoo.h\";";
const string expected_stderr =
- "ERROR: a/Foo.aidl:1.32-37: 'nullable' is not a supported annotation for this node. "
+ "ERROR: a/IFoo.aidl:1.32-37: 'nullable' is not a supported annotation for this node. "
"It must be one of: Hide, JavaOnlyStableParcelable, UnsupportedAppUsage, VintfStability, "
"JavaPassthrough, JavaOnlyImmutable\n";
CaptureStderr();
- EXPECT_EQ(nullptr, Parse("a/Foo.aidl", method, typenames_, GetLanguage(), &error));
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", method, typenames_, GetLanguage(), &error));
EXPECT_EQ(expected_stderr, GetCapturedStderr());
EXPECT_EQ(AidlError::BAD_TYPE, error);
}
TEST_P(AidlTest, RejectUnsupportedParcelableDefineAnnotations) {
AidlError error;
- const string method = "package a; @nullable parcelable Foo { String a; String b; }";
+ const string method = "package a; @nullable parcelable IFoo { String a; String b; }";
const string expected_stderr =
- "ERROR: a/Foo.aidl:1.32-36: 'nullable' is not a supported annotation for this node. "
+ "ERROR: a/IFoo.aidl:1.32-37: 'nullable' is not a supported annotation for this node. "
"It must be one of: Hide, UnsupportedAppUsage, VintfStability, JavaPassthrough, JavaDebug, "
"JavaOnlyImmutable, FixedSize, RustDerive\n";
CaptureStderr();
- EXPECT_EQ(nullptr, Parse("a/Foo.aidl", method, typenames_, GetLanguage(), &error));
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", method, typenames_, GetLanguage(), &error));
EXPECT_EQ(expected_stderr, GetCapturedStderr());
EXPECT_EQ(AidlError::BAD_TYPE, error);
}
@@ -1082,18 +1118,17 @@
switch (GetLanguage()) {
case Options::Language::CPP:
expected_stderr =
+ "ERROR: a/IFoo.aidl:2.1-7: A generic type cannot have any primitive type parameters.\n"
"ERROR: a/IFoo.aidl:2.1-7: List<int> is not supported. List in cpp supports only "
"String and IBinder.\n";
break;
case Options::Language::JAVA:
expected_stderr =
+ "ERROR: a/IFoo.aidl:2.1-7: A generic type cannot have any primitive type parameters.\n"
"ERROR: a/IFoo.aidl:2.1-7: List<int> is not supported. List in Java supports only "
"String, IBinder, and ParcelFileDescriptor.\n";
break;
case Options::Language::NDK:
- expected_stderr =
- "ERROR: a/IFoo.aidl:2.1-7: A generic type cannot have any primitive type parameters.\n";
- break;
case Options::Language::RUST:
expected_stderr =
"ERROR: a/IFoo.aidl:2.1-7: A generic type cannot have any primitive type parameters.\n";
@@ -1187,6 +1222,7 @@
if (GetLanguage() == Options::Language::NDK || GetLanguage() == Options::Language::RUST) {
EXPECT_EQ(
+ "ERROR: a/IFoo.aidl:2.19-23: ParcelableHolder cannot be a return type\n"
"ERROR: a/IFoo.aidl:2.1-19: The NDK and Rust backend does not support ParcelableHolder "
"yet.\n",
GetCapturedStderr());
@@ -1207,6 +1243,7 @@
if (GetLanguage() == Options::Language::NDK || GetLanguage() == Options::Language::RUST) {
EXPECT_EQ(
+ "ERROR: a/IFoo.aidl:2.31-34: ParcelableHolder cannot be an argument type\n"
"ERROR: a/IFoo.aidl:2.14-31: The NDK and Rust backend does not support ParcelableHolder "
"yet.\n",
GetCapturedStderr());
@@ -1334,7 +1371,8 @@
TEST_F(AidlTest, CheckNumGenericTypeSecifier) {
const string expected_list_stderr =
- "ERROR: p/IFoo.aidl:1.37-41: List must have only one type parameter.\n";
+ "ERROR: p/IFoo.aidl:1.37-41: List can only have one type parameter, but got: "
+ "'List<String,String>'\n";
const string expected_map_stderr =
"ERROR: p/IFoo.aidl:1.37-40: Map must have 0 or 2 type parameters, but got 'Map<String>'\n";
Options options = Options::From("aidl p/IFoo.aidl IFoo.java");