Implement sysprop type checker am: 472fb63273 am: e50548bdbe

Change-Id: I3fcc0bbc1bcada87f9c52ddcab700d9d4c63fe31

9 files changed, 420 insertions, 6 deletions

diff --git a/Android.bp b/Android.bp
index 2804fb8..2768156 100644
--- a/Android.bp
+++ b/Android.bp
@@ -18,7 +18,7 @@ cc_defaults {
     name: "sysprop-defaults",
     srcs: ["sysprop.proto", "Common.cpp", "CodeWriter.cpp"],
     shared_libs: ["libbase", "liblog"],
-    static_libs: ["libc++fs"],
+    static_libs: ["libc++fs", "libpropertyinfoserializer"],
     proto: {
         type: "full",
     },
@@ -49,12 +49,19 @@ cc_binary_host {
     srcs: ["ApiDumpMain.cpp"],
 }
 
+cc_binary_host {
+    name: "sysprop_type_checker",
+    defaults: ["sysprop-defaults"],
+    srcs: ["TypeChecker.cpp", "TypeCheckerMain.cpp"],
+}
+
 cc_test_host {
     name: "sysprop_test",
     defaults: ["sysprop-defaults"],
     srcs: ["ApiChecker.cpp",
-            "CppGen.cpp",
+           "CppGen.cpp",
            "JavaGen.cpp",
+           "TypeChecker.cpp",
            "tests/*.cpp"],
     test_suites: ["general-tests"],
 }
diff --git a/Common.cpp b/Common.cpp
index 2278e04..f81aa59 100644
--- a/Common.cpp
+++ b/Common.cpp
@@ -274,6 +274,10 @@ std::string GetModuleName(const sysprop::Properties& props) {
   return module.substr(module.rfind('.') + 1);
 }
 
+std::vector<std::string> ParseEnumValues(const std::string& enum_values) {
+  return android::base::Split(enum_values, "|");
+}
+
 Result<sysprop::Properties> ParseProps(const std::string& input_file_path) {
   sysprop::Properties ret;
   std::string file_contents;
diff --git a/CppGen.cpp b/CppGen.cpp
index 842bd5d..51e2f51 100644
--- a/CppGen.cpp
+++ b/CppGen.cpp
@@ -338,8 +338,7 @@ std::string GenerateSource(const sysprop::Properties& props,
     writer.Write("constexpr const std::pair<const char*, %s> %s_list[] = {\n",
                  enum_name.c_str(), prop_id.c_str());
     writer.Indent();
-    for (const std::string& name :
-         android::base::Split(prop.enum_values(), "|")) {
+    for (const std::string& name : ParseEnumValues(prop.enum_values())) {
       writer.Write("{\"%s\", %s::%s},\n", name.c_str(), enum_name.c_str(),
                    ToUpper(name).c_str());
     }
diff --git a/JavaGen.cpp b/JavaGen.cpp
index 7d7a9f0..5912607 100644
--- a/JavaGen.cpp
+++ b/JavaGen.cpp
@@ -317,8 +317,7 @@ std::string GenerateJavaClass(const sysprop::Properties& props,
       writer.Write("public static enum %s {\n",
                    GetJavaEnumTypeName(prop).c_str());
       writer.Indent();
-      std::vector<std::string> values =
-          android::base::Split(prop.enum_values(), "|");
+      std::vector<std::string> values = ParseEnumValues(prop.enum_values());
       for (int i = 0; i < values.size(); ++i) {
         const std::string& name = values[i];
         writer.Write("%s(\"%s\")", ToUpper(name).c_str(), name.c_str());
diff --git a/TypeChecker.cpp b/TypeChecker.cpp
new file mode 100644
index 0000000..70fd395
--- /dev/null
+++ b/TypeChecker.cpp
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "TypeChecker.h"
+
+#include <algorithm>
+#include <string>
+#include <unordered_map>
+
+#include <android-base/strings.h>
+
+#include "Common.h"
+
+using android::base::Result;
+using android::properties::ParsePropertyInfoFile;
+using android::properties::PropertyInfoEntry;
+
+namespace {
+std::string SyspropTypeToContextType(const sysprop::Property& prop) {
+  switch (prop.type()) {
+    case sysprop::Integer:
+    case sysprop::Long:
+      return "int";
+    case sysprop::Double:
+      return "double";
+    case sysprop::Boolean:
+      return "bool";
+    case sysprop::Enum: {
+      // Sort both values and then join
+      auto prop_values = android::base::Split(prop.enum_values(), "|");
+      std::sort(prop_values.begin(), prop_values.end());
+      return "enum " + android::base::Join(prop_values, " ");
+    }
+    default:
+      // All other types (string, all kinds of list) should fall here
+      return "string";
+  }
+}
+
+bool IsCompatible(const sysprop::Property& prop, const std::string& ctx_type) {
+  if (prop.type() == sysprop::Enum) {
+    // special case: we need to first sort values and then compare
+    auto prop_values = ParseEnumValues(prop.enum_values());
+    std::sort(prop_values.begin(), prop_values.end());
+
+    // ctx_type must be "enum [value1] [value2] ..."
+    auto ctx_values = android::base::Split(ctx_type, " ");
+    if (ctx_values.empty() || ctx_values[0] != "enum") {
+      return false;
+    }
+    ctx_values.erase(ctx_values.begin());
+    std::sort(ctx_values.begin(), ctx_values.end());
+
+    return prop_values == ctx_values;
+  }
+
+  return SyspropTypeToContextType(prop) == ctx_type;
+}
+
+std::string GetTypeName(const sysprop::Property& prop) {
+  if (prop.type() == sysprop::Enum) {
+    return "Enum " + prop.enum_values();
+  }
+
+  return sysprop::Type_Name(prop.type());
+}
+}  // namespace
+
+Result<void> CheckPropertyTypes(const sysprop::SyspropLibraryApis& api,
+                                const std::vector<PropertyInfoEntry>& entries) {
+  std::string err;
+
+  // map from exact property names to types in property_contexts
+  std::unordered_map<std::string, std::string> types;
+  for (auto& entry : entries) {
+    // skip prefix entries.
+    if (!entry.exact_match) continue;
+
+    // Duplicated prop check is intentionally skipped.
+    // Build will fail if any duplication happens.
+    types.emplace(entry.name, entry.type);
+  }
+
+  for (auto& props : api.props()) {
+    for (auto& prop : props.prop()) {
+      // Skip check if there is no matched property.
+      auto itr = types.find(prop.prop_name());
+      if (itr == types.end()) continue;
+
+      if (!IsCompatible(prop, itr->second)) {
+        if (!err.empty()) err += "\n";
+        err += "Type of prop '" + prop.prop_name() +
+               "' is incompatible with property_contexts\n";
+        err += "In sysprop_library: " + GetTypeName(prop) + "\n";
+        err += "In property_contexts: " + itr->second + " (should be '" +
+               SyspropTypeToContextType(prop) + "')\n";
+      }
+    }
+  }
+
+  if (err.empty())
+    return {};
+  else
+    return Errorf("{}", err);
+}
diff --git a/TypeCheckerMain.cpp b/TypeCheckerMain.cpp
new file mode 100644
index 0000000..4a63257
--- /dev/null
+++ b/TypeCheckerMain.cpp
@@ -0,0 +1,128 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define LOG_TAG "sysprop_type_checker_main"
+
+#include <android-base/file.h>
+#include <android-base/logging.h>
+#include <cstdio>
+#include <cstdlib>
+#include <string>
+#include <vector>
+
+#include <getopt.h>
+
+#include "Common.h"
+#include "TypeChecker.h"
+
+using android::base::Result;
+using android::properties::ParsePropertyInfoFile;
+using android::properties::PropertyInfoEntry;
+
+namespace {
+
+struct Arguments {
+  std::vector<std::string> api_paths;
+  std::vector<std::string> context_paths;
+};
+
+[[noreturn]] void PrintUsage(const char* exe_name) {
+  std::printf("Usage: %s [--api api_path]... [--context context_path]...\n",
+              exe_name);
+  std::exit(EXIT_FAILURE);
+}
+
+Result<Arguments> ParseArgs(int argc, char* argv[]) {
+  Arguments ret;
+  for (;;) {
+    static struct option long_options[] = {
+        {"api", required_argument, 0, 'a'},
+        {"context", required_argument, 0, 'c'},
+    };
+
+    int opt = getopt_long_only(argc, argv, "", long_options, nullptr);
+    if (opt == -1) break;
+
+    switch (opt) {
+      case 'a':
+        ret.api_paths.emplace_back(optarg);
+        break;
+      case 'c':
+        ret.context_paths.emplace_back(optarg);
+        break;
+      default:
+        PrintUsage(argv[0]);
+    }
+  }
+
+  if (optind < argc) {
+    return Errorf("Unknown arguments");
+  }
+
+  if (ret.api_paths.empty() || ret.context_paths.empty()) {
+    return Errorf("both api files and context files must be specified");
+  }
+
+  return ret;
+}
+}  // namespace
+
+int main(int argc, char* argv[]) {
+  Arguments args;
+  if (auto res = ParseArgs(argc, argv); res.ok()) {
+    args = std::move(*res);
+  } else {
+    LOG(ERROR) << argv[0] << ": " << res.error();
+    PrintUsage(argv[0]);
+  }
+
+  sysprop::SyspropLibraryApis api;
+
+  // read all api files and merge them into one SyspropLibraryApis
+  for (auto& api_path : args.api_paths) {
+    if (auto res = ParseApiFile(api_path); res.ok()) {
+      api.MergeFrom(*res);
+    } else {
+      LOG(FATAL) << "parsing sysprop_library API file " << api_path
+                 << " failed: " << res.error();
+    }
+  }
+
+  std::vector<PropertyInfoEntry> entries;
+
+  // read all context files and parse entries
+  for (auto& context_path : args.context_paths) {
+    std::string contents;
+    if (!android::base::ReadFileToString(context_path, &contents)) {
+      LOG(FATAL) << "Could not read properties from '" << context_path << "'";
+    }
+
+    std::vector<std::string> errors;
+    ParsePropertyInfoFile(contents, true, &entries, &errors);
+    if (!errors.empty()) {
+      for (const auto& error : errors) {
+        LOG(ERROR) << "Could not read line from '" << context_path
+                   << "': " << error;
+      }
+      LOG(FATAL) << "Could not parse properties from '" << context_path << "'";
+    }
+  }
+
+  if (auto res = CheckPropertyTypes(api, entries); !res.ok()) {
+    LOG(ERROR) << "sysprop_library type check failed:\n\n" << res.error();
+    return EXIT_FAILURE;
+  }
+}
diff --git a/include/Common.h b/include/Common.h
index 41a3f0a..8ad6fb1 100644
--- a/include/Common.h
+++ b/include/Common.h
@@ -26,6 +26,7 @@ inline static constexpr const char* kGeneratedFileFooterComments =
 std::string ApiNameToIdentifier(const std::string& name);
 std::string GetModuleName(const sysprop::Properties& props);
 bool IsListProp(const sysprop::Property& prop);
+std::vector<std::string> ParseEnumValues(const std::string& enum_values);
 android::base::Result<sysprop::Properties> ParseProps(
     const std::string& file_path);
 android::base::Result<sysprop::SyspropLibraryApis> ParseApiFile(
diff --git a/include/TypeChecker.h b/include/TypeChecker.h
new file mode 100644
index 0000000..0212828
--- /dev/null
+++ b/include/TypeChecker.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <vector>
+
+#include <android-base/result.h>
+#include <property_info_serializer/property_info_serializer.h>
+#include "sysprop.pb.h"
+
+android::base::Result<void> CheckPropertyTypes(
+    const sysprop::SyspropLibraryApis& api,
+    const std::vector<android::properties::PropertyInfoEntry>& entries);
diff --git a/tests/TypeCheckerTest.cpp b/tests/TypeCheckerTest.cpp
new file mode 100644
index 0000000..21d9089
--- /dev/null
+++ b/tests/TypeCheckerTest.cpp
@@ -0,0 +1,131 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <string>
+
+#include <android-base/test_utils.h>
+#include <gtest/gtest.h>
+
+#include "Common.h"
+#include "TypeChecker.h"
+
+using android::properties::ParsePropertyInfoFile;
+using android::properties::PropertyInfoEntry;
+
+namespace {
+
+constexpr const char* kApi =
+    R"(
+props {
+    owner: Platform
+    module: "android.platprop"
+    prop {
+        api_name: "prop1"
+        type: Long
+        scope: Public
+        access: ReadWrite
+        prop_name: "prop1"
+    }
+    prop {
+        api_name: "prop2"
+        type: Enum
+        scope: Public
+        access: Writeonce
+        enum_values: "a|b|c"
+        prop_name: "ro.public.prop2"
+    }
+    prop {
+        api_name: "prop3"
+        type: Boolean
+        scope: Public
+        access: ReadWrite
+        prop_name: "ctl.start$prop3"
+    }
+    prop {
+        api_name: "prop4"
+        type: String
+        scope: Public
+        access: Readonly
+        prop_name: "ro.prop4"
+        deprecated: true
+    }
+
+}
+)";
+
+constexpr const char* kContexts =
+    R"(
+prop1           u:object_r:default_prop:s0 exact int
+ro.public.prop2 u:object_r:foo_prop:s0 exact enum c b a
+ctl.start$prop3 u:object_r:ctl_prop:s0 exact bool
+ro.prop4        u:object_r:bar_prop:s0 exact string
+)";
+
+constexpr const char* kBadContexts =
+    R"(
+prop1           u:object_r:default_prop:s0 exact double
+ro.public.prop2 u:object_r:foo_prop:s0 exact enum a c
+ctl.start$prop3 u:object_r:ctl_prop:s0 exact string
+ro.prop4        u:object_r:bar_prop:s0 exact int
+)";
+
+}  // namespace
+
+TEST(SyspropTest, TypeCheckerTest) {
+  TemporaryFile api_file;
+  close(api_file.fd);
+  api_file.fd = -1;
+  ASSERT_TRUE(android::base::WriteStringToFile(kApi, api_file.path));
+
+  std::string err;
+  auto api = ParseApiFile(api_file.path);
+  ASSERT_RESULT_OK(api);
+
+  // Good property_contexts tests
+  std::vector<PropertyInfoEntry> entries;
+  std::vector<std::string> errors;
+  ParsePropertyInfoFile(kContexts, true, &entries, &errors);
+  ASSERT_TRUE(errors.empty());
+
+  auto res = CheckPropertyTypes(*api, entries);
+  EXPECT_TRUE(res.ok());
+
+  // Bad property_contexts tests
+  std::vector<PropertyInfoEntry> bad_entries;
+  ParsePropertyInfoFile(kBadContexts, true, &bad_entries, &errors);
+  ASSERT_TRUE(errors.empty());
+
+  auto bad_res = CheckPropertyTypes(*api, bad_entries);
+  EXPECT_FALSE(bad_res.ok());
+
+  EXPECT_EQ(
+      bad_res.error().message(),
+      "Type of prop 'prop1' is incompatible with property_contexts\n"
+      "In sysprop_library: Long\n"
+      "In property_contexts: double (should be 'int')\n"
+      "\n"
+      "Type of prop 'ro.public.prop2' is incompatible with property_contexts\n"
+      "In sysprop_library: Enum a|b|c\n"
+      "In property_contexts: enum a c (should be 'enum a b c')\n"
+      "\n"
+      "Type of prop 'ctl.start$prop3' is incompatible with property_contexts\n"
+      "In sysprop_library: Boolean\n"
+      "In property_contexts: string (should be 'bool')\n"
+      "\n"
+      "Type of prop 'ro.prop4' is incompatible with property_contexts\n"
+      "In sysprop_library: String\n"
+      "In property_contexts: int (should be 'string')\n");
+}