chromeos-dbus-bindings: Minor code formatting and comment fixes

Made minor code formatting changes to the generated code and fixed
comment generation from <tp:docstring> element to maintain relative
in-comment indentation as seen in peerd's Service.xml file.

So the comment tag like this:
      <tp:docstring>
        A list of IP addresses, each composed of:
        - an array of bytes containing the octets of the IP
          address (either an IPv4 or an IPv6 address)
        - a 16 bit port number.
      </tp:docstring>

Would become a code comment like this:

  // A list of IP addresses, each composed of:
  // - an array of bytes containing the octets of the IP
  //   address (either an IPv4 or an IPv6 address)
  // - a 16 bit port number.

BUG=None
TEST=FEATURES=test emerge-link chromeos-dbus-bindings
     Manually inspect the generated proxy/adapters for
     existing components (buffet, peerd, privetd).

Change-Id: I794533a26a75991a9a89382f185df721a1b53793
Reviewed-on: https://chromium-review.googlesource.com/232060
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>

5 files changed, 90 insertions, 11 deletions

diff --git a/chromeos-dbus-bindings/adaptor_generator.cc b/chromeos-dbus-bindings/adaptor_generator.cc
index 5ffdf70..8576907 100644
--- a/chromeos-dbus-bindings/adaptor_generator.cc
+++ b/chromeos-dbus-bindings/adaptor_generator.cc
@@ -37,6 +37,7 @@ bool AdaptorGenerator::GenerateAdaptors(
   text.AddLine(StringPrintf("#define %s", header_guard.c_str()));
   text.AddLine("#include <memory>");
   text.AddLine("#include <string>");
+  text.AddLine("#include <tuple>");
   text.AddLine("#include <vector>");
   text.AddBlankLine();
   text.AddLine("#include <base/macros.h>");
@@ -436,11 +437,9 @@ void AdaptorGenerator::AddPropertyDataMembers(const Interface& interface,
     CHECK(signature.Parse(property.type, &type));
     string variable_name = GetPropertyVariableName(property.name);
 
-    block.AddLine(StringPrintf("chromeos::dbus_utils::ExportedProperty<%s>",
-                               type.c_str()));
-    block.PushOffset(kLineContinuationOffset);
-    block.AddLine(StringPrintf("%s_;", variable_name.c_str()));
-    block.PopOffset();
+    block.AddLine(
+        StringPrintf("chromeos::dbus_utils::ExportedProperty<%s> %s_;",
+                     type.c_str(), variable_name.c_str()));
   }
   if (!interface.properties.empty())
     block.AddBlankLine();
diff --git a/chromeos-dbus-bindings/adaptor_generator_unittest.cc b/chromeos-dbus-bindings/adaptor_generator_unittest.cc
index 583b1ea..d63054f 100644
--- a/chromeos-dbus-bindings/adaptor_generator_unittest.cc
+++ b/chromeos-dbus-bindings/adaptor_generator_unittest.cc
@@ -52,6 +52,7 @@ const char kMethod1Name2[] = "Tetsuo2";
 const char kExpectedContent[] = R"literal_string(
 #include <memory>
 #include <string>
+#include <tuple>
 #include <vector>
 
 #include <base/macros.h>
@@ -151,8 +152,7 @@ class TestAdaptor {
       std::vector<dbus::ObjectPath>>;
   std::weak_ptr<SignalMappingType> signal_Mapping_;
 
-  chromeos::dbus_utils::ExportedProperty<std::string>
-      character_name_;
+  chromeos::dbus_utils::ExportedProperty<std::string> character_name_;
 
   TestInterface* interface_;  // Owned by container of this adapter.
 
diff --git a/chromeos-dbus-bindings/indented_text.cc b/chromeos-dbus-bindings/indented_text.cc
index ea6b45a..5a8ed6f 100644
--- a/chromeos-dbus-bindings/indented_text.cc
+++ b/chromeos-dbus-bindings/indented_text.cc
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include <base/logging.h>
+#include <base/strings/string_util.h>
 #include <chromeos/strings/string_utils.h>
 
 using std::string;
@@ -41,10 +42,47 @@ void IndentedText::AddLineWithOffset(const std::string& line, size_t shift) {
 }
 
 void IndentedText::AddComments(const std::string& doc_string) {
-  // Split at \n, trim all whitespaces and eliminate empty strings.
-  auto lines = chromeos::string_utils::Split(doc_string, '\n', true, true);
-  for (const auto& line : lines) {
-    AddLine("// " + line);
+  // Try to retain indentation in the comments. Find the first non-empty line
+  // of the comment and find its whitespace indentation prefix.
+  // For all subsequent lines, remove the same whitespace prefix as found
+  // at the first line of the comment but keep any additional spaces to
+  // maintain the comment layout.
+  auto lines = chromeos::string_utils::Split(doc_string, '\n', false, false);
+  vector<string> lines_out;
+  lines_out.reserve(lines.size());
+  bool first_nonempty_found = false;
+  std::string trim_prefix;
+  for (string line : lines) {
+    base::TrimWhitespaceASCII(line, base::TRIM_TRAILING, &line);
+    if (!first_nonempty_found) {
+      size_t pos = line.find_first_not_of(" \t");
+      if (pos != std::string::npos) {
+        first_nonempty_found = true;
+        trim_prefix = line.substr(0, pos);
+        lines_out.push_back(line.substr(pos));
+      }
+    } else {
+      if (StartsWithASCII(line, trim_prefix, false)) {
+        line = line.substr(trim_prefix.length());
+      } else {
+        base::TrimWhitespaceASCII(line, base::TRIM_LEADING, &line);
+      }
+      lines_out.push_back(line);
+    }
+  }
+
+  // We already eliminated all empty lines at the beginning of the comment
+  // block. Now remove the trailing empty lines.
+  while (!lines_out.empty() && lines_out.back().empty())
+    lines_out.pop_back();
+
+  for (const string& line : lines_out) {
+    const bool all_whitespace = (line.find_first_not_of(" \t") == string::npos);
+    if (all_whitespace) {
+      AddLine("//");
+    } else {
+      AddLine("// " + line);
+    }
   }
 }
 
diff --git a/chromeos-dbus-bindings/indented_text_unittest.cc b/chromeos-dbus-bindings/indented_text_unittest.cc
index ef60082..d615770 100644
--- a/chromeos-dbus-bindings/indented_text_unittest.cc
+++ b/chromeos-dbus-bindings/indented_text_unittest.cc
@@ -125,4 +125,45 @@ TEST_F(IndentedTextTest, Reset) {
   EXPECT_TRUE(GetHistory().empty());
 }
 
+TEST_F(IndentedTextTest, AddComments_Empty) {
+  text_.AddComments("");
+  EXPECT_EQ("", text_.GetContents());
+}
+
+TEST_F(IndentedTextTest, AddComments_WhitespaceOnly) {
+  text_.AddComments("  \n \t  \n");
+  EXPECT_EQ("", text_.GetContents());
+}
+
+TEST_F(IndentedTextTest, AddComments_EmptyLines) {
+  string comment_block = R"(
+
+    line1
+
+    line2
+
+
+  )";
+  text_.AddComments(comment_block);
+  EXPECT_EQ("// line1\n"
+            "//\n"
+            "// line2\n", text_.GetContents());
+}
+
+TEST_F(IndentedTextTest, AddComments_Indentation) {
+  string comment_block = R"(
+    line1
+      - bullet1
+        line2
+      - bullet2
+  line3
+  )";
+  text_.AddComments(comment_block);
+  EXPECT_EQ("// line1\n"
+            "//   - bullet1\n"
+            "//     line2\n"
+            "//   - bullet2\n"
+            "// line3\n", text_.GetContents());
+}
+
 }  // namespace chromeos_dbus_bindings
diff --git a/chromeos-dbus-bindings/proxy_generator.cc b/chromeos-dbus-bindings/proxy_generator.cc
index 17d299a..01f2a75 100644
--- a/chromeos-dbus-bindings/proxy_generator.cc
+++ b/chromeos-dbus-bindings/proxy_generator.cc
@@ -86,6 +86,7 @@ void ProxyGenerator::GenerateInterfaceProxy(const ServiceConfig& config,
 
   text->AddLine(StringPrintf("// Interface proxy for %s.",
                              GetFullClassName(namespaces, itf_name).c_str()));
+  text->AddComments(interface.doc_string);
   text->AddLine(StringPrintf("class %s final {", proxy_name.c_str()));
   text->AddLineWithOffset("public:", kScopeOffset);
   text->PushOffset(kBlockOffset);