Refactor CanBeOutParameter() and error with reason
Previously, the test fails to check what we want to check.
It's fixed with an additional hint why it fails.
Bug: n/a
Test: aidl_unittests
Change-Id: Iaf09dc0a0018bc9fef63f8f484d5d8a9c687a7e9
diff --git a/aidl_language.cpp b/aidl_language.cpp
index 82adaf8..5162960 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -1324,7 +1324,8 @@
AIDL_ERROR(m) << "oneway method '" << m->GetName() << "' cannot have out parameters";
return false;
}
- const bool can_be_out = typenames.CanBeOutParameter(arg->GetType());
+
+ const auto [can_be_out, type_aspect] = typenames.CanBeOutParameter(arg->GetType());
if (!arg->DirectionWasSpecified() && can_be_out) {
AIDL_ERROR(arg) << "'" << arg->GetType().ToString()
<< "' can be an out type, so you must declare it as in, out, or inout.";
@@ -1332,7 +1333,8 @@
}
if (arg->GetDirection() != AidlArgument::IN_DIR && !can_be_out) {
- AIDL_ERROR(arg) << "'" << arg->ToString() << "' can only be an in parameter.";
+ AIDL_ERROR(arg) << "'" << arg->GetName() << "' can't be an " << arg->GetDirectionSpecifier()
+ << " parameter because " << type_aspect << " can only be an in parameter.";
return false;
}
diff --git a/aidl_typenames.cpp b/aidl_typenames.cpp
index acad42f..d431056 100644
--- a/aidl_typenames.cpp
+++ b/aidl_typenames.cpp
@@ -279,16 +279,30 @@
}
// Only T[], List, Map, ParcelFileDescriptor and mutable Parcelable can be an out parameter.
-bool AidlTypenames::CanBeOutParameter(const AidlTypeSpecifier& type) const {
+// Returns pair of
+// - bool: tells if the type can be an out/inout parameter
+// - string: the aspect of the type which decides whether the type can be "out" or not.
+pair<bool, string> AidlTypenames::CanBeOutParameter(const AidlTypeSpecifier& type) const {
const string& name = type.GetName();
- if (IsBuiltinTypename(name) || GetEnumDeclaration(type)) {
- return type.IsArray() || type.GetName() == "List" || type.GetName() == "Map" ||
- type.GetName() == "ParcelFileDescriptor";
+ if (type.IsArray()) return {true, "array"};
+
+ if (IsBuiltinTypename(name)) {
+ if (name == "List" || name == "Map" || name == "ParcelFileDescriptor") {
+ return {true, name};
+ }
+ return {false, name};
}
- const AidlDefinedType* t = TryGetDefinedType(type.GetName());
- AIDL_FATAL_IF(t == nullptr, type) << "Unrecognized type: '" << type.GetName() << "'";
+
+ const AidlDefinedType* t = TryGetDefinedType(name);
+ AIDL_FATAL_IF(t == nullptr, type) << "Unrecognized type: '" << name << "'";
+
// An 'out' field is passed as an argument, so it doesn't make sense if it is immutable.
- return t->AsParcelable() != nullptr && !t->IsJavaOnlyImmutable();
+ if (t->AsParcelable() != nullptr) {
+ if (t->IsJavaOnlyImmutable()) return {false, "@JavaOnlyImmutable"};
+ return {true, "parcelable/union"};
+ }
+
+ return {false, t->GetPreprocessDeclarationName()};
}
const AidlEnumDeclaration* AidlTypenames::GetEnumDeclaration(const AidlTypeSpecifier& type) const {
diff --git a/aidl_typenames.h b/aidl_typenames.h
index ca41009..43ac82f 100644
--- a/aidl_typenames.h
+++ b/aidl_typenames.h
@@ -18,12 +18,14 @@
#include <functional>
#include <map>
#include <memory>
+#include <optional>
#include <set>
#include <string>
#include <utility>
#include <vector>
using std::map;
+using std::optional;
using std::pair;
using std::set;
using std::string;
@@ -69,7 +71,7 @@
bool is_resolved;
};
ResolvedTypename ResolveTypename(const string& type_name) const;
- bool CanBeOutParameter(const AidlTypeSpecifier& type) const;
+ pair<bool, string> CanBeOutParameter(const AidlTypeSpecifier& type) const;
bool CanBeJavaOnlyImmutable(const AidlTypeSpecifier& type) const;
bool CanBeFixedSize(const AidlTypeSpecifier& type) const;
static bool IsList(const AidlTypeSpecifier& type);
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 325ea7c..bc29cd5 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -330,7 +330,8 @@
TEST_P(AidlTest, RejectOutParametersForIBinder) {
const string interface_ibinder = "package a; interface IBaz { void f(out IBinder bar); }";
const string expected_ibinder_stderr =
- "ERROR: a/IBaz.aidl:1.47-51: 'out IBinder bar' can only be an in parameter.\n";
+ "ERROR: a/IBaz.aidl:1.47-51: 'bar' can't be an out parameter because IBinder can only be an "
+ "in parameter.\n";
CaptureStderr();
EXPECT_EQ(nullptr, Parse("a/IBaz.aidl", interface_ibinder, typenames_, GetLanguage()));
EXPECT_EQ(expected_ibinder_stderr, GetCapturedStderr());
@@ -2816,16 +2817,26 @@
TEST_F(AidlTest, ImmutableParcelableCannotBeInOut) {
io_delegate_.SetFileContents("Foo.aidl", "@JavaOnlyImmutable parcelable Foo { int a; }");
- io_delegate_.SetFileContents("IBar.aidl", "interface IBar { void my(inout Foo); }");
+ io_delegate_.SetFileContents("IBar.aidl", "interface IBar { void my(inout Foo foo); }");
+ string expected_error =
+ "ERROR: IBar.aidl:1.35-39: 'foo' can't be an inout parameter because @JavaOnlyImmutable can "
+ "only be an in parameter.\n";
+ CaptureStderr();
Options options = Options::From("aidl --lang=java IBar.aidl -I .");
EXPECT_NE(0, ::android::aidl::compile_aidl(options, io_delegate_));
+ EXPECT_EQ(expected_error, GetCapturedStderr());
}
TEST_F(AidlTest, ImmutableParcelableCannotBeOut) {
io_delegate_.SetFileContents("Foo.aidl", "@JavaOnlyImmutable parcelable Foo { int a; }");
- io_delegate_.SetFileContents("IBar.aidl", "interface IBar { void my(out Foo); }");
+ io_delegate_.SetFileContents("IBar.aidl", "interface IBar { void my(out Foo foo); }");
+ string expected_error =
+ "ERROR: IBar.aidl:1.33-37: 'foo' can't be an out parameter because @JavaOnlyImmutable can "
+ "only be an in parameter.\n";
+ CaptureStderr();
Options options = Options::From("aidl --lang=java IBar.aidl -I .");
EXPECT_NE(0, ::android::aidl::compile_aidl(options, io_delegate_));
+ EXPECT_EQ(expected_error, GetCapturedStderr());
}
TEST_F(AidlTest, ImmutableParcelableFieldNameRestriction) {