Make string matching the last resort way to look up types
That is, Find(const AidlType&) will call FindByName(const std::string&)
rather than the other way around. Later we'll use annotations in the
AidlType to pick one of several different type name strings.
Bug: 26729450
Test: unittests continue to pass
Change-Id: I10fcb88eabda30b88d5457ecfc8931755a0aa872
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index e319da8..8528023 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -154,21 +154,19 @@
TEST_F(AidlTest, ParsesPreprocessedFile) {
string simple_content = "parcelable a.Foo;\ninterface b.IBar;";
io_delegate_.SetFileContents("path", simple_content);
- EXPECT_FALSE(java_types_.HasType("a.Foo"));
+ EXPECT_FALSE(java_types_.HasTypeByCanonicalName("a.Foo"));
EXPECT_TRUE(parse_preprocessed_file(io_delegate_, "path", &java_types_));
- EXPECT_TRUE(java_types_.HasType("Foo"));
- EXPECT_TRUE(java_types_.HasType("a.Foo"));
- EXPECT_TRUE(java_types_.HasType("b.IBar"));
+ EXPECT_TRUE(java_types_.HasTypeByCanonicalName("a.Foo"));
+ EXPECT_TRUE(java_types_.HasTypeByCanonicalName("b.IBar"));
}
TEST_F(AidlTest, ParsesPreprocessedFileWithWhitespace) {
string simple_content = "parcelable a.Foo;\n interface b.IBar ;\t";
io_delegate_.SetFileContents("path", simple_content);
- EXPECT_FALSE(java_types_.HasType("a.Foo"));
+ EXPECT_FALSE(java_types_.HasTypeByCanonicalName("a.Foo"));
EXPECT_TRUE(parse_preprocessed_file(io_delegate_, "path", &java_types_));
- EXPECT_TRUE(java_types_.HasType("Foo"));
- EXPECT_TRUE(java_types_.HasType("a.Foo"));
- EXPECT_TRUE(java_types_.HasType("b.IBar"));
+ EXPECT_TRUE(java_types_.HasTypeByCanonicalName("a.Foo"));
+ EXPECT_TRUE(java_types_.HasTypeByCanonicalName("b.IBar"));
}
TEST_F(AidlTest, PreferImportToPreprocessed) {
@@ -182,10 +180,11 @@
&java_types_);
EXPECT_NE(nullptr, parse_result);
// We expect to know about both kinds of IBar
- EXPECT_TRUE(java_types_.HasType("one.IBar"));
- EXPECT_TRUE(java_types_.HasType("another.IBar"));
+ EXPECT_TRUE(java_types_.HasTypeByCanonicalName("one.IBar"));
+ EXPECT_TRUE(java_types_.HasTypeByCanonicalName("another.IBar"));
// But if we request just "IBar" we should get our imported one.
- const java::Type* type = java_types_.Find("IBar");
+ AidlType ambiguous_type("IBar", 0, "", false /* not an array */);
+ const java::Type* type = java_types_.Find(ambiguous_type);
ASSERT_TRUE(type);
EXPECT_EQ("one.IBar", type->QualifiedName());
}
@@ -255,7 +254,7 @@
// C++ understands C++ specific stuff
auto cpp_parse_result = Parse(input_path, input, &cpp_types_);
EXPECT_NE(nullptr, cpp_parse_result);
- auto cpp_type = cpp_types_.Find("Bar");
+ auto cpp_type = cpp_types_.FindTypeByCanonicalName("p.Bar");
ASSERT_NE(nullptr, cpp_type);
EXPECT_EQ("::p::Bar", cpp_type->CppType());
set<string> headers;
@@ -266,7 +265,7 @@
// Java ignores C++ specific stuff
auto java_parse_result = Parse(input_path, input, &java_types_);
EXPECT_NE(nullptr, java_parse_result);
- auto java_type = java_types_.Find("Bar");
+ auto java_type = java_types_.FindTypeByCanonicalName("p.Bar");
ASSERT_NE(nullptr, java_type);
EXPECT_EQ("p.Bar", java_type->InstantiableName());
}
diff --git a/type_cpp.cpp b/type_cpp.cpp
index 7666925..e79e440 100644
--- a/type_cpp.cpp
+++ b/type_cpp.cpp
@@ -275,7 +275,7 @@
class StringListType : public Type {
public:
StringListType()
- : Type(ValidatableType::KIND_BUILT_IN, "java.util", "List<String>",
+ : Type(ValidatableType::KIND_BUILT_IN, "java.util", "List<java.lang.String>",
{"utils/String16.h", "vector"},
"::std::vector<::android::String16>",
"readString16Vector", "writeString16Vector",
@@ -372,13 +372,13 @@
"readCharVector", "writeCharVector"));
Type* nullable_string_array_type =
- new ArrayType(ValidatableType::KIND_BUILT_IN, kNoPackage, "String[]",
+ new ArrayType(ValidatableType::KIND_BUILT_IN, "java.lang", "String[]",
{"utils/String16.h", "vector"},
"::std::unique_ptr<::std::vector<::std::unique_ptr<::android::String16>>>",
"readString16Vector", "writeString16Vector");
Type* string_array_type = new ArrayType(ValidatableType::KIND_BUILT_IN,
- kNoPackage, "String[]",
+ "java.lang", "String[]",
{"utils/String16.h", "vector"},
"::std::vector<::android::String16>",
"readString16Vector",
@@ -386,11 +386,11 @@
nullable_string_array_type);
Type* nullable_string_type =
- new Type(ValidatableType::KIND_BUILT_IN, kNoPackage, "String",
+ new Type(ValidatableType::KIND_BUILT_IN, "java.lang", "String",
{"utils/String16.h"}, "::std::unique_ptr<::android::String16>",
"readString16", "writeString16");
- string_type_ = new Type(ValidatableType::KIND_BUILT_IN, kNoPackage, "String",
+ string_type_ = new Type(ValidatableType::KIND_BUILT_IN, "java.lang", "String",
{"utils/String16.h"}, "::android::String16",
"readString16", "writeString16",
string_array_type, nullable_string_type);
@@ -438,7 +438,7 @@
}
bool TypeNamespace::AddListType(const std::string& type_name) {
- const Type* contained_type = Find(type_name);
+ const Type* contained_type = FindTypeByCanonicalName(type_name);
if (!contained_type) {
LOG(ERROR) << "Cannot create List<" << type_name << "> because contained "
"type cannot be found or is invalid.";
diff --git a/type_cpp_unittest.cpp b/type_cpp_unittest.cpp
index 7138d13..35fc3d5 100644
--- a/type_cpp_unittest.cpp
+++ b/type_cpp_unittest.cpp
@@ -33,18 +33,19 @@
};
TEST_F(CppTypeNamespaceTest, HasSomeBasicTypes) {
- EXPECT_TRUE(types_.HasType("byte"));
- EXPECT_TRUE(types_.HasType("int"));
- EXPECT_TRUE(types_.HasType("long"));
- EXPECT_TRUE(types_.HasType("float"));
- EXPECT_TRUE(types_.HasType("double"));
- EXPECT_TRUE(types_.HasType("boolean"));
- EXPECT_TRUE(types_.HasType("char"));
- EXPECT_TRUE(types_.HasType("String"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("byte"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("int"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("long"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("float"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("double"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("boolean"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("char"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("String"));
}
TEST_F(CppTypeNamespaceTest, SupportsListString) {
- EXPECT_TRUE(types_.HasType("List<String>"));
+ EXPECT_TRUE(
+ types_.HasTypeByCanonicalName("java.util.List<java.lang.String>"));
}
} // namespace cpp
diff --git a/type_java.cpp b/type_java.cpp
index febc9cb..f056610 100644
--- a/type_java.cpp
+++ b/type_java.cpp
@@ -702,9 +702,8 @@
void InterfaceType::CreateFromParcel(StatementBlock* addTo, Variable* v,
Variable* parcel, Variable**) const {
// v = Interface.asInterface(parcel.readStrongBinder());
- string stub_type = v->type->QualifiedName() + ".Stub";
addTo->Add(new Assignment(
- v, new MethodCall(m_types->Find(stub_type), "asInterface", 1,
+ v, new MethodCall(stub_, "asInterface", 1,
new MethodCall(parcel, "readStrongBinder"))));
}
@@ -856,17 +855,6 @@
FALSE_VALUE = new LiteralExpression("false");
}
-const Type* JavaTypeNamespace::Find(const char* package,
- const char* name) const {
- string s;
- if (package != nullptr && *package != '\0') {
- s += package;
- s += '.';
- }
- s += name;
- return Find(s);
-}
-
bool JavaTypeNamespace::AddParcelableType(const AidlParcelable& p,
const std::string& filename) {
Type* type =
@@ -897,7 +885,7 @@
}
bool JavaTypeNamespace::AddListType(const std::string& contained_type_name) {
- const Type* contained_type = Find(contained_type_name);
+ const Type* contained_type = FindTypeByCanonicalName(contained_type_name);
if (!contained_type) {
return false;
}
diff --git a/type_java.h b/type_java.h
index bb51ddd..82fb580 100644
--- a/type_java.h
+++ b/type_java.h
@@ -444,10 +444,6 @@
bool AddMapType(const std::string& key_type_name,
const std::string& value_type_name) override;
- using LanguageTypeNamespace<Type>::Find;
- // helper alias for Find(name);
- const Type* Find(const char* package, const char* name) const;
-
const Type* BoolType() const { return m_bool_type; }
const Type* IntType() const { return m_int_type; }
const Type* StringType() const { return m_string_type; }
diff --git a/type_java_unittest.cpp b/type_java_unittest.cpp
index 65687f6..a9df134 100644
--- a/type_java_unittest.cpp
+++ b/type_java_unittest.cpp
@@ -36,27 +36,27 @@
};
TEST_F(JavaTypeNamespaceTest, HasSomeBasicTypes) {
- EXPECT_TRUE(types_.HasType("void"));
- EXPECT_TRUE(types_.HasType("int"));
- EXPECT_TRUE(types_.HasType("String"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("void"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("int"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("java.lang.String"));
}
TEST_F(JavaTypeNamespaceTest, ContainerTypeCreation) {
// We start with no knowledge of parcelables or lists of them.
- EXPECT_FALSE(types_.HasType("Foo"));
- EXPECT_FALSE(types_.HasType("List<Foo>"));
+ EXPECT_FALSE(types_.HasTypeByCanonicalName("Foo"));
+ EXPECT_FALSE(types_.HasTypeByCanonicalName("java.util.List<a.goog.Foo>"));
unique_ptr<AidlParcelable> parcelable(
new AidlParcelable(new AidlQualifiedName("Foo", ""), 0, {"a", "goog"}));
// Add the parcelable type we care about.
EXPECT_TRUE(types_.AddParcelableType(*parcelable.get(), __FILE__));
// Now we can find the parcelable type, but not the List of them.
- EXPECT_TRUE(types_.HasType("Foo"));
- EXPECT_FALSE(types_.HasType("List<Foo>"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("a.goog.Foo"));
+ EXPECT_FALSE(types_.HasTypeByCanonicalName("java.util.List<a.goog.Foo>"));
// But after we add the list explicitly...
AidlType container_type("List<Foo>", 0, "", false /* not array */);
EXPECT_TRUE(types_.MaybeAddContainerType(container_type));
// This should work.
- EXPECT_TRUE(types_.HasType("List<Foo>"));
+ EXPECT_TRUE(types_.HasTypeByCanonicalName("java.util.List<a.goog.Foo>"));
}
} // namespace java
diff --git a/type_namespace.h b/type_namespace.h
index 7865dd3..ae6501a 100644
--- a/type_namespace.h
+++ b/type_namespace.h
@@ -126,22 +126,23 @@
// Get a pointer to an existing type. Searches first by fully-qualified
// name, and then class name (dropping package qualifiers).
- const T* Find(const std::string& name) const;
+ const T* Find(const AidlType& aidl_type) const;
- const ValidatableType* FindTypeByName(
- const std::string& name) const { return Find(name); }
- bool HasType(const std::string& type_name) const {
- return FindTypeByName(type_name) != nullptr;
+ // Find a type by its |name|. If |name| refers to a container type (e.g.
+ // List<String>) you must turn it into a canonical name first (e.g.
+ // java.util.List<java.lang.String>).
+ const T* FindTypeByCanonicalName(const std::string& name) const;
+ bool HasTypeByCanonicalName(const std::string& type_name) const {
+ return FindTypeByCanonicalName(type_name) != nullptr;
}
bool HasImportType(const AidlImport& import) const override {
- return HasType(import.GetNeededClass());
+ return HasTypeByCanonicalName(import.GetNeededClass());
}
const ValidatableType* GetInterfaceType(
const AidlInterface& interface) const override {
- return FindTypeByName(interface.GetCanonicalName());
+ return FindTypeByCanonicalName(interface.GetCanonicalName());
}
-
bool MaybeAddContainerType(const AidlType& aidl_type) override;
// We dynamically create container types as we discover them in the parse
// tree. Returns false if the contained types cannot be canonicalized.
@@ -172,7 +173,7 @@
template<typename T>
bool LanguageTypeNamespace<T>::Add(const T* type) {
- const T* existing = Find(type->QualifiedName());
+ const T* existing = FindTypeByCanonicalName(type->QualifiedName());
if (!existing) {
types_.emplace_back(type);
return true;
@@ -199,13 +200,13 @@
}
template<typename T>
-const T* LanguageTypeNamespace<T>::Find(const std::string& raw_name) const {
+const T* LanguageTypeNamespace<T>::Find(const AidlType& aidl_type) const {
using std::string;
using std::vector;
using android::base::Join;
using android::base::Trim;
- string name = Trim(raw_name);
+ string name = Trim(aidl_type.GetName());
if (IsContainerType(name)) {
vector<string> container_class;
vector<string> contained_type_names;
@@ -213,17 +214,19 @@
&contained_type_names)) {
return nullptr;
}
- for (string& contained_type_name : contained_type_names) {
- const T* contained_type = Find(contained_type_name);
- if (!contained_type) {
- return nullptr;
- }
- contained_type_name = contained_type->QualifiedName();
- }
name = Join(container_class, '.') +
"<" + Join(contained_type_names, ',') + ">";
}
+ // Here, we know that if we have a container
+ return FindTypeByCanonicalName(name);
+}
+template<typename T>
+const T* LanguageTypeNamespace<T>::FindTypeByCanonicalName(
+ const std::string& raw_name) const {
+ using android::base::Trim;
+
+ std::string name = Trim(raw_name);
const T* ret = nullptr;
for (const auto& type : types_) {
// Always prefer a exact match if possible.
@@ -244,8 +247,10 @@
template<typename T>
bool LanguageTypeNamespace<T>::MaybeAddContainerType(
const AidlType& aidl_type) {
+ using android::base::Join;
+
std::string type_name = aidl_type.GetName();
- if (!IsContainerType(type_name) || HasType(type_name)) {
+ if (!IsContainerType(type_name)) {
return true;
}
@@ -256,6 +261,13 @@
return false;
}
+ const std::string canonical_name = Join(container_class, ".") +
+ "<" + Join(contained_type_names, ",") + ">";
+ if (HasTypeByCanonicalName(canonical_name)) {
+ return true;
+ }
+
+
// We only support two types right now and this type is one of them.
switch (contained_type_names.size()) {
case 1:
@@ -310,6 +322,15 @@
std::string remainder = name.substr(opening_brace + 1,
(closing_brace - opening_brace) - 1);
std::vector<std::string> args = Split(remainder, ",");
+ for (auto& type_name: args) {
+ // Here, we are relying on FindTypeByCanonicalName to do its best
+ // when given a non-canonical name.
+ const T* arg_type = FindTypeByCanonicalName(type_name);
+ if (!arg_type) {
+ return false;
+ }
+ type_name = arg_type->QualifiedName();
+ }
// Map the container name to its canonical form for supported containers.
if ((container == "List" || container == "java.util.List") &&
@@ -335,7 +356,7 @@
const AidlType& aidl_type, std::string* error_msg) const {
using android::base::StringPrintf;
- const ValidatableType* type = FindTypeByName(aidl_type.GetName());
+ const ValidatableType* type = Find(aidl_type);
if (type == nullptr) {
*error_msg = "unknown type";
return nullptr;