error if annotation is repeated
unless it declares "repeatable" in schema.
Bug: 171536650
Test: aidl_unittests
Change-Id: I90fb16f5df43794319b14706f4e1e66752abd7b3
diff --git a/aidl_language.cpp b/aidl_language.cpp
index fc4929c..92f4961 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -117,24 +117,25 @@
const std::vector<AidlAnnotation::Schema>& AidlAnnotation::AllSchemas() {
static const std::vector<Schema> kSchemas{
- {AidlAnnotation::Type::NULLABLE, "nullable", {}},
- {AidlAnnotation::Type::UTF8_IN_CPP, "utf8InCpp", {}},
- {AidlAnnotation::Type::VINTF_STABILITY, "VintfStability", {}},
+ {AidlAnnotation::Type::NULLABLE, "nullable", {}, false},
+ {AidlAnnotation::Type::UTF8_IN_CPP, "utf8InCpp", {}, false},
+ {AidlAnnotation::Type::VINTF_STABILITY, "VintfStability", {}, false},
{AidlAnnotation::Type::UNSUPPORTED_APP_USAGE,
"UnsupportedAppUsage",
{{"expectedSignature", "String"},
{"implicitMember", "String"},
{"maxTargetSdk", "int"},
{"publicAlternatives", "String"},
- {"trackingBug", "long"}}},
- {AidlAnnotation::Type::JAVA_STABLE_PARCELABLE, "JavaOnlyStableParcelable", {}},
- {AidlAnnotation::Type::HIDE, "Hide", {}},
- {AidlAnnotation::Type::BACKING, "Backing", {{"type", "String"}}},
- {AidlAnnotation::Type::JAVA_PASSTHROUGH, "JavaPassthrough", {{"annotation", "String"}}},
- {AidlAnnotation::Type::JAVA_DEBUG, "JavaDebug", {}},
- {AidlAnnotation::Type::JAVA_ONLY_IMMUTABLE, "JavaOnlyImmutable", {}},
- {AidlAnnotation::Type::FIXED_SIZE, "FixedSize", {}},
- {AidlAnnotation::Type::DESCRIPTOR, "Descriptor", {{"value", "String"}}},
+ {"trackingBug", "long"}},
+ false},
+ {AidlAnnotation::Type::JAVA_STABLE_PARCELABLE, "JavaOnlyStableParcelable", {}, false},
+ {AidlAnnotation::Type::HIDE, "Hide", {}, false},
+ {AidlAnnotation::Type::BACKING, "Backing", {{"type", "String"}}, false},
+ {AidlAnnotation::Type::JAVA_PASSTHROUGH, "JavaPassthrough", {{"annotation", "String"}}, true},
+ {AidlAnnotation::Type::JAVA_DEBUG, "JavaDebug", {}, false},
+ {AidlAnnotation::Type::JAVA_ONLY_IMMUTABLE, "JavaOnlyImmutable", {}, false},
+ {AidlAnnotation::Type::FIXED_SIZE, "FixedSize", {}, false},
+ {AidlAnnotation::Type::DESCRIPTOR, "Descriptor", {{"value", "String"}}, false},
{AidlAnnotation::Type::RUST_DERIVE,
"RustDerive",
{{"Copy", "boolean"},
@@ -143,7 +144,8 @@
{"Ord", "boolean"},
{"PartialEq", "boolean"},
{"Eq", "boolean"},
- {"Hash", "boolean"}}},
+ {"Hash", "boolean"}},
+ false},
};
return kSchemas;
}
@@ -268,6 +270,8 @@
AidlAnnotation::Type type) {
for (const auto& a : annotations) {
if (a.GetType() == type) {
+ AIDL_FATAL_IF(a.Repeatable(), a)
+ << "Trying to get a single annotation when it is repeatable.";
return &a;
}
}
@@ -364,22 +368,32 @@
bool AidlAnnotatable::CheckValid(const AidlTypenames&) const {
std::set<AidlAnnotation::Type> supported_annotations = GetSupportedAnnotations();
for (const auto& annotation : GetAnnotations()) {
- if (!annotation.CheckValid()) {
- return false;
- }
-
- std::vector<std::string> supported_annot_strings;
- for (AidlAnnotation::Type type : supported_annotations) {
- supported_annot_strings.push_back(AidlAnnotation::TypeToString(type));
- }
-
+ // check if it is allowed for this node
if (supported_annotations.find(annotation.GetType()) == supported_annotations.end()) {
+ std::vector<std::string> supported_annot_strings;
+ for (AidlAnnotation::Type type : supported_annotations) {
+ supported_annot_strings.push_back(AidlAnnotation::TypeToString(type));
+ }
AIDL_ERROR(this) << "'" << annotation.GetName()
<< "' is not a supported annotation for this node. "
<< "It must be one of: "
<< android::base::Join(supported_annot_strings, ", ");
return false;
}
+ // CheckValid() only if it is okay to be here
+ if (!annotation.CheckValid()) {
+ return false;
+ }
+ }
+
+ std::map<AidlAnnotation::Type, AidlLocation> declared;
+ for (const auto& annotation : GetAnnotations()) {
+ const auto& [iter, inserted] = declared.emplace(annotation.GetType(), annotation.GetLocation());
+ if (!inserted && !annotation.Repeatable()) {
+ AIDL_ERROR(this) << "'" << annotation.GetName()
+ << "' is repeated, but not allowed. Previous location: " << iter->second;
+ return false;
+ }
}
return true;
diff --git a/aidl_language.h b/aidl_language.h
index 4b56a5f..1e8b600 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -182,8 +182,9 @@
virtual ~AidlAnnotation() = default;
bool CheckValid() const;
- const string& GetName() const { return schema_.name; };
+ const string& GetName() const { return schema_.name; }
const Type& GetType() const { return schema_.type; }
+ bool Repeatable() const { return schema_.repeatable; }
string ToString(const ConstantValueDecorator& decorator) const;
std::map<std::string, std::string> AnnotationParams(
const ConstantValueDecorator& decorator) const;
@@ -199,6 +200,8 @@
// map from param name -> value type
std::map<std::string, std::string> supported_parameters;
+
+ bool repeatable;
};
static const std::vector<Schema>& AllSchemas();
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index da40a38..be01fa4 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -398,6 +398,16 @@
EXPECT_EQ(expected_stderr, GetCapturedStderr());
}
+TEST_P(AidlTest, RejectsRepeatedAnnotations) {
+ const string method = R"(@Hide @Hide parcelable Foo {})";
+ const string expected_stderr =
+ "ERROR: Foo.aidl:1.23-27: 'Hide' is repeated, but not allowed. Previous location: "
+ "Foo.aidl:1.1-6\n";
+ CaptureStderr();
+ EXPECT_EQ(nullptr, Parse("Foo.aidl", method, typenames_, GetLanguage()));
+ EXPECT_EQ(expected_stderr, GetCapturedStderr());
+}
+
TEST_P(AidlTest, RejectsDuplicatedAnnotationParams) {
const string method = "package a; interface IFoo { @UnsupportedAppUsage(foo=1, foo=2)void f(); }";
const string expected_stderr = "ERROR: a/IFoo.aidl:1.56-62: Trying to redefine parameter foo.\n";