From 11e65d194b09412a4e55e3da0505545aefb47d50 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Mon, 2 Apr 2018 18:09:38 -0400
Subject: [PATCH 1/2] formatting: reject incorrect protobuf namespaces

Add some checks to the formatting script so that protobuf-related type
references use the correct namespaces. This should prevent the need for
later fixups like #2963.

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 source/server/http/admin.cc |  2 +-
 tools/check_format.py       | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc
index b71a0d9a62f8..77764af32c15 100644
--- a/source/server/http/admin.cc
+++ b/source/server/http/admin.cc
@@ -285,7 +285,7 @@ Http::Code AdminImpl::handlerConfigDump(absl::string_view, Http::HeaderMap&,
   for (const auto& key_callback_pair : config_tracker_.getCallbacksMap()) {
     ProtobufTypes::MessagePtr message = key_callback_pair.second();
     RELEASE_ASSERT(message);
-    Protobuf::Any any_message;
+    ProtobufWkt::Any any_message;
     any_message.PackFrom(*message);
     config_dump_map[key_callback_pair.first] = any_message;
   }
diff --git a/tools/check_format.py b/tools/check_format.py
index c65ee16206e6..791c291e9ee2 100755
--- a/tools/check_format.py
+++ b/tools/check_format.py
@@ -68,6 +68,25 @@ def checkProtobufExternalDepsBuild(file_path):
 def checkProtobufExternalDeps(file_path):
   if whitelistedForProtobufDeps(file_path):
     return True
+  protobuf_type_errors = {
+      # Well-known types should be referenced from the ProtobufWkt namespace.
+      "Protobuf::Any":                    "ProtobufWkt::Any",
+      "Protobuf::Empty":                  "ProtobufWkt::Empty",
+      "Protobuf::ListValue":              "ProtobufWkt:ListValue",
+      "Protobuf::NULL_VALUE":             "ProtobufWkt::NULL_VALUE",
+      "Protobuf::StringValue":            "ProtobufWkt::StringValue",
+      "Protobuf::Struct":                 "ProtobufWkt::Struct",
+      "Protobuf::Value":                  "ProtobufWkt::Value",
+
+      # Maps including strings should use the protobuf string types.
+      "Protobuf::MapPair<std::string":    "Protobuf::MapPair<Envoy::ProtobufTypes::String",
+
+      # Other common mis-namespacing of protobuf types.
+      "ProtobufWkt::Map":                 "Protobuf::Map",
+      "ProtobufWkt::MapPair":             "Protobuf::MapPair",
+      "ProtobufUtil::MessageDifferencer": "Protobuf::util::MessageDifferencer"
+  }
+
   with open(file_path) as f:
     text = f.read()
     if '"google/protobuf' in text or "google::protobuf" in text:
@@ -75,6 +94,10 @@ def checkProtobufExternalDeps(file_path):
           "%s has unexpected direct dependency on google.protobuf, use "
           "the definitions in common/protobuf/protobuf.h instead." % file_path)
       return False
+    for error, replacement in protobuf_type_errors.items():
+      if error in text:
+        printError("%s uses an unexpected protobuf namespace reference; instead of %s, use %s"
+                   % (file_path, error, replacement))
     return True
 
 

From ce230240e5bdac3a10f7a2f71afd15898929ff09 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Mon, 2 Apr 2018 20:44:23 -0400
Subject: [PATCH 2/2] Auto-fix proto namespace errors

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 tools/check_format.py | 50 ++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/tools/check_format.py b/tools/check_format.py
index 791c291e9ee2..6108b58b6f8d 100755
--- a/tools/check_format.py
+++ b/tools/check_format.py
@@ -23,6 +23,25 @@
 HEADER_ORDER_PATH = os.path.join(
     os.path.dirname(os.path.abspath(sys.argv[0])), "header_order.py")
 
+PROTOBUF_TYPE_ERRORS = {
+    # Well-known types should be referenced from the ProtobufWkt namespace.
+    "Protobuf::Any":                    "ProtobufWkt::Any",
+    "Protobuf::Empty":                  "ProtobufWkt::Empty",
+    "Protobuf::ListValue":              "ProtobufWkt:ListValue",
+    "Protobuf::NULL_VALUE":             "ProtobufWkt::NULL_VALUE",
+    "Protobuf::StringValue":            "ProtobufWkt::StringValue",
+    "Protobuf::Struct":                 "ProtobufWkt::Struct",
+    "Protobuf::Value":                  "ProtobufWkt::Value",
+
+    # Maps including strings should use the protobuf string types.
+    "Protobuf::MapPair<std::string":    "Protobuf::MapPair<Envoy::ProtobufTypes::String",
+
+    # Other common mis-namespacing of protobuf types.
+    "ProtobufWkt::Map":                 "Protobuf::Map",
+    "ProtobufWkt::MapPair":             "Protobuf::MapPair",
+    "ProtobufUtil::MessageDifferencer": "Protobuf::util::MessageDifferencer"
+}
+
 found_error = False
 
 
@@ -68,25 +87,6 @@ def checkProtobufExternalDepsBuild(file_path):
 def checkProtobufExternalDeps(file_path):
   if whitelistedForProtobufDeps(file_path):
     return True
-  protobuf_type_errors = {
-      # Well-known types should be referenced from the ProtobufWkt namespace.
-      "Protobuf::Any":                    "ProtobufWkt::Any",
-      "Protobuf::Empty":                  "ProtobufWkt::Empty",
-      "Protobuf::ListValue":              "ProtobufWkt:ListValue",
-      "Protobuf::NULL_VALUE":             "ProtobufWkt::NULL_VALUE",
-      "Protobuf::StringValue":            "ProtobufWkt::StringValue",
-      "Protobuf::Struct":                 "ProtobufWkt::Struct",
-      "Protobuf::Value":                  "ProtobufWkt::Value",
-
-      # Maps including strings should use the protobuf string types.
-      "Protobuf::MapPair<std::string":    "Protobuf::MapPair<Envoy::ProtobufTypes::String",
-
-      # Other common mis-namespacing of protobuf types.
-      "ProtobufWkt::Map":                 "Protobuf::Map",
-      "ProtobufWkt::MapPair":             "Protobuf::MapPair",
-      "ProtobufUtil::MessageDifferencer": "Protobuf::util::MessageDifferencer"
-  }
-
   with open(file_path) as f:
     text = f.read()
     if '"google/protobuf' in text or "google::protobuf" in text:
@@ -94,10 +94,6 @@ def checkProtobufExternalDeps(file_path):
           "%s has unexpected direct dependency on google.protobuf, use "
           "the definitions in common/protobuf/protobuf.h instead." % file_path)
       return False
-    for error, replacement in protobuf_type_errors.items():
-      if error in text:
-        printError("%s uses an unexpected protobuf namespace reference; instead of %s, use %s"
-                   % (file_path, error, replacement))
     return True
 
 
@@ -117,7 +113,13 @@ def fixFileContents(file_path):
   for line in fileinput.input(file_path, inplace=True):
     # Strip double space after '.'  This may prove overenthusiastic and need to
     # be restricted to comments and metadata files but works for now.
-    sys.stdout.write("%s" % (line.replace('.  ', '. ')))
+    line = line.replace('.  ', '. ')
+
+    # Fix incorrect protobuf namespace references.
+    for error, replacement in PROTOBUF_TYPE_ERRORS.items():
+      line = line.replace(error, replacement)
+
+    sys.stdout.write(str(line))
 
 
 def checkFilePath(file_path):