Restrict where annotations can be applied
Some annotations are only valid in specific contexts.
If an annotation is found in the incorrect context, return
an error when parsing the ADIL language.
Test: runtests.sh
Change-Id: I64bf6e6f5de7c38d6f6928acbe55022ab9de441a
diff --git a/aidl_language.cpp b/aidl_language.cpp
index 5beaea1..7b7d58d 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -314,11 +314,18 @@
writer->Write("%s\n", AidlAnnotatable::ToString().c_str());
}
-bool AidlAnnotatable::CheckValidAnnotations() const {
+bool AidlAnnotatable::CheckValid(const AidlTypenames&) const {
+ std::set<string> supported_annotations = GetSupportedAnnotations();
for (const auto& annotation : GetAnnotations()) {
if (!annotation.CheckValid()) {
return false;
}
+ if (supported_annotations.find(annotation.GetName()) == supported_annotations.end()) {
+ AIDL_ERROR(this) << "'" << annotation.GetName()
+ << "' is not a supported annotation for this node. "
+ << "It must be one of: " << android::base::Join(supported_annotations, ", ");
+ return false;
+ }
}
return true;
@@ -392,8 +399,14 @@
return result.second;
}
+std::set<string> AidlTypeSpecifier::GetSupportedAnnotations() const {
+ // kHide and kUnsupportedAppUsage are both method return annotations
+ // which we don't distinguish from other type specifiers.
+ return {kNullable, kUtf8InCpp, kUnsupportedAppUsage, kHide};
+}
+
bool AidlTypeSpecifier::CheckValid(const AidlTypenames& typenames) const {
- if (!CheckValidAnnotations()) {
+ if (!AidlAnnotatable::CheckValid(typenames)) {
return false;
}
if (IsGeneric()) {
@@ -672,6 +685,14 @@
return Join(package_, '.');
}
+bool AidlDefinedType::CheckValid(const AidlTypenames& typenames) const {
+ if (!AidlAnnotatable::CheckValid(typenames)) {
+ return false;
+ }
+
+ return true;
+}
+
bool AidlDefinedType::IsHidden() const {
return HasHideComment(GetComments());
}
@@ -728,26 +749,17 @@
return true;
}
-bool AidlParcelable::CheckValid(const AidlTypenames&) const {
- static const std::set<string> allowed{kJavaStableParcelable};
- if (!CheckValidAnnotations()) {
+std::set<string> AidlParcelable::GetSupportedAnnotations() const {
+ return {kVintfStability, kUnsupportedAppUsage, kJavaStableParcelable, kHide};
+}
+
+bool AidlParcelable::CheckValid(const AidlTypenames& typenames) const {
+ if (!AidlDefinedType::CheckValid(typenames)) {
return false;
}
if (!AidlParameterizable<std::string>::CheckValid()) {
return false;
}
- for (const auto& v : GetAnnotations()) {
- if (allowed.find(v.GetName()) == allowed.end()) {
- std::ostringstream stream;
- stream << "Unstructured parcelable can contain only";
- for (const string& kv : allowed) {
- stream << " " << kv;
- }
- stream << ".";
- AIDL_ERROR(this) << stream.str();
- return false;
- }
- }
return true;
}
@@ -777,8 +789,16 @@
writer->Write("}\n");
}
+std::set<string> AidlStructuredParcelable::GetSupportedAnnotations() const {
+ return {kVintfStability, kUnsupportedAppUsage, kHide};
+}
+
bool AidlStructuredParcelable::CheckValid(const AidlTypenames& typenames) const {
bool success = true;
+ if (!AidlParcelable::CheckValid(typenames)) {
+ return false;
+ }
+
for (const auto& v : GetFields()) {
success = success && v->CheckValid(typenames);
}
@@ -926,7 +946,14 @@
return true;
}
-bool AidlEnumDeclaration::CheckValid(const AidlTypenames&) const {
+std::set<string> AidlEnumDeclaration::GetSupportedAnnotations() const {
+ return {kVintfStability, kBacking, kHide};
+}
+
+bool AidlEnumDeclaration::CheckValid(const AidlTypenames& typenames) const {
+ if (!AidlDefinedType::CheckValid(typenames)) {
+ return false;
+ }
if (backing_type_ == nullptr) {
AIDL_ERROR(this) << "Enum declaration missing backing type.";
return false;
@@ -1010,8 +1037,12 @@
writer->Write("}\n");
}
+std::set<string> AidlInterface::GetSupportedAnnotations() const {
+ return {kVintfStability, kUnsupportedAppUsage, kHide};
+}
+
bool AidlInterface::CheckValid(const AidlTypenames& typenames) const {
- if (!CheckValidAnnotations()) {
+ if (!AidlDefinedType::CheckValid(typenames)) {
return false;
}
// Has to be a pointer due to deleting copy constructor. No idea why.
diff --git a/aidl_language.h b/aidl_language.h
index 7a40427..55bed95 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -252,7 +252,8 @@
std::string ToString() const;
const vector<AidlAnnotation>& GetAnnotations() const { return annotations_; }
- bool CheckValidAnnotations() const;
+ virtual std::set<string> GetSupportedAnnotations() const = 0;
+ virtual bool CheckValid(const AidlTypenames&) const;
private:
vector<AidlAnnotation> annotations_;
@@ -309,7 +310,8 @@
// resolution fails.
bool Resolve(const AidlTypenames& typenames);
- bool CheckValid(const AidlTypenames& typenames) const;
+ std::set<string> GetSupportedAnnotations() const override;
+ bool CheckValid(const AidlTypenames& typenames) const override;
bool LanguageSpecificCheckValid(Options::Language lang) const;
const AidlNode& AsAidlNode() const override { return *this; }
@@ -659,7 +661,7 @@
virtual const AidlEnumDeclaration* AsEnumDeclaration() const { return nullptr; }
virtual const AidlInterface* AsInterface() const { return nullptr; }
virtual const AidlParameterizable<std::string>* AsParameterizable() const { return nullptr; }
- virtual bool CheckValid(const AidlTypenames&) const { return CheckValidAnnotations(); }
+ bool CheckValid(const AidlTypenames& typenames) const override;
virtual bool LanguageSpecificCheckValid(Options::Language lang) const = 0;
AidlStructuredParcelable* AsStructuredParcelable() {
return const_cast<AidlStructuredParcelable*>(
@@ -713,6 +715,7 @@
std::string GetCppName() const { return name_->GetColonName(); }
std::string GetCppHeader() const { return cpp_header_; }
+ std::set<string> GetSupportedAnnotations() const override;
bool CheckValid(const AidlTypenames& typenames) const override;
bool LanguageSpecificCheckValid(Options::Language lang) const override;
const AidlParcelable* AsParcelable() const override { return this; }
@@ -744,6 +747,7 @@
void Dump(CodeWriter* writer) const override;
+ std::set<string> GetSupportedAnnotations() const override;
bool CheckValid(const AidlTypenames& typenames) const override;
bool LanguageSpecificCheckValid(Options::Language lang) const override;
@@ -790,6 +794,7 @@
return enumerators_;
}
bool Autofill();
+ std::set<string> GetSupportedAnnotations() const override;
bool CheckValid(const AidlTypenames& typenames) const override;
bool LanguageSpecificCheckValid(Options::Language) const override { return true; }
std::string GetPreprocessDeclarationName() const override { return "enum"; }
@@ -824,6 +829,7 @@
void Dump(CodeWriter* writer) const override;
+ std::set<string> GetSupportedAnnotations() const override;
bool CheckValid(const AidlTypenames& typenames) const override;
bool LanguageSpecificCheckValid(Options::Language lang) const override;
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 957c0d0..a2b7c09 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -293,6 +293,74 @@
EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", method, typenames_, Options::Language::JAVA));
}
+TEST_F(AidlTest, RejectUnsupportedInterfaceAnnotations) {
+ AidlError error = AidlError::OK;
+ string method = "package a; @nullable interface IFoo { int f(); }";
+ string expected_stderr =
+ "ERROR: a/IFoo.aidl:1.21-31: 'nullable' is not a supported annotation for this node. "
+ "It must be one of: Hide, UnsupportedAppUsage, VintfStability\n";
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", method, typenames_, Options::Language::CPP, &error));
+ EXPECT_EQ(AidlError::BAD_TYPE, error);
+ EXPECT_EQ(expected_stderr, GetCapturedStderr());
+ CaptureStderr();
+ typenames_.Reset();
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", method, typenames_, Options::Language::JAVA, &error));
+ EXPECT_EQ(AidlError::BAD_TYPE, error);
+ EXPECT_EQ(expected_stderr, GetCapturedStderr());
+ CaptureStderr();
+}
+
+TEST_F(AidlTest, RejectUnsupportedTypeAnnotations) {
+ AidlError error = AidlError::OK;
+ string method = "package a; interface IFoo { @JavaOnlyStableParcelable int f(); }";
+ string expected_stderr =
+ "ERROR: a/IFoo.aidl:1.54-58: 'JavaOnlyStableParcelable' is not a supported annotation "
+ "for this node. It must be one of: Hide, UnsupportedAppUsage, nullable, utf8InCpp\n";
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", method, typenames_, Options::Language::CPP, &error));
+ EXPECT_EQ(AidlError::BAD_TYPE, error);
+ EXPECT_EQ(expected_stderr, GetCapturedStderr());
+ CaptureStderr();
+ typenames_.Reset();
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", method, typenames_, Options::Language::JAVA, &error));
+ EXPECT_EQ(AidlError::BAD_TYPE, error);
+ EXPECT_EQ(expected_stderr, GetCapturedStderr());
+ CaptureStderr();
+}
+
+TEST_F(AidlTest, RejectUnsupportedParcelableAnnotations) {
+ AidlError error = AidlError::OK;
+ string method = "package a; @nullable parcelable IFoo cpp_header \"IFoo.h\";";
+ string expected_stderr =
+ "ERROR: a/Foo.aidl:1.32-37: 'nullable' is not a supported annotation for this node. "
+ "It must be one of: Hide, JavaOnlyStableParcelable, UnsupportedAppUsage, VintfStability\n";
+ EXPECT_EQ(nullptr, Parse("a/Foo.aidl", method, typenames_, Options::Language::CPP, &error));
+ EXPECT_EQ(AidlError::BAD_TYPE, error);
+ EXPECT_EQ(expected_stderr, GetCapturedStderr());
+ CaptureStderr();
+ typenames_.Reset();
+ EXPECT_EQ(nullptr, Parse("a/Foo.aidl", method, typenames_, Options::Language::JAVA, &error));
+ EXPECT_EQ(AidlError::BAD_TYPE, error);
+ EXPECT_EQ(expected_stderr, GetCapturedStderr());
+ CaptureStderr();
+}
+
+TEST_F(AidlTest, RejectUnsupportedParcelableDefineAnnotations) {
+ AidlError error = AidlError::OK;
+ string method = "package a; @nullable parcelable Foo { String a; String b; }";
+ string expected_stderr =
+ "ERROR: a/Foo.aidl:1.32-36: 'nullable' is not a supported annotation for this node. "
+ "It must be one of: Hide, UnsupportedAppUsage, VintfStability\n";
+ EXPECT_EQ(nullptr, Parse("a/Foo.aidl", method, typenames_, Options::Language::CPP, &error));
+ ASSERT_EQ(AidlError::BAD_TYPE, error);
+ EXPECT_EQ(expected_stderr, GetCapturedStderr());
+ CaptureStderr();
+ typenames_.Reset();
+ EXPECT_EQ(nullptr, Parse("a/Foo.aidl", method, typenames_, Options::Language::JAVA, &error));
+ EXPECT_EQ(AidlError::BAD_TYPE, error);
+ EXPECT_EQ(expected_stderr, GetCapturedStderr());
+ CaptureStderr();
+}
+
TEST_F(AidlTest, ParsesNullableAnnotation) {
for (auto is_nullable: {true, false}) {
auto parse_result = Parse("a/IFoo.aidl",