From b1169ce0b9aa2745dcce7300be01dbe462b396ab Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 7 Jan 2019 11:15:51 -0500 Subject: [PATCH 1/6] format: add format fix for java proto options Allows running tools/check_format.py fix to insert the necessary java proto options. Signed-off-by: Snow Pettersen --- tools/check_format.py | 26 +++++++++++++++++++ tools/check_format_test_helper.py | 8 ++++++ .../check_format/api/java_options.proto | 1 + .../check_format/api/java_options.proto.gold | 3 +++ 4 files changed, 38 insertions(+) create mode 100644 tools/testdata/check_format/api/java_options.proto create mode 100644 tools/testdata/check_format/api/java_options.proto.gold diff --git a/tools/check_format.py b/tools/check_format.py index 79951a18192e1..673737e4eceae 100755 --- a/tools/check_format.py +++ b/tools/check_format.py @@ -142,6 +142,30 @@ def checkNamespace(file_path): return ["Unable to find Envoy namespace or NOLINT(namespace-envoy) for file: %s" % file_path] return [] +def fixJavaProtoOptions(file_path): + java_multiple_files = False + java_package_correct = False + package_name = None + for line in fileinput.FileInput(file_path): + if line.startswith("package "): + package_name = re.compile("package (.*);").search(line).group(1) + if "option java_multiple_files = true;" in line: + java_multiple_files = True + if "option java_package = \"io.envoyproxy.envoy" in line: + java_package_correct = True + if java_multiple_files and java_package_correct: + return + + to_add = "" + if not java_package_correct: + to_add = to_add + "option java_package = \"io.envoyproxy.{}\";\n".format(package_name) + if not java_multiple_files: + to_add = to_add + "option java_multiple_files = true;\n" + + for line in fileinput.FileInput(file_path, inplace=True): + if line.startswith("package "): + line = line.replace(line, line + to_add) + sys.stdout.write(line) def checkJavaProtoOptions(file_path): java_multiple_files = False @@ -383,6 +407,8 @@ def fixSourcePath(file_path): if not file_path.endswith(PROTO_SUFFIX): error_messages += fixHeaderOrder(file_path) error_messages += clangFormat(file_path) + if file_path.endswith(PROTO_SUFFIX) and isApiFile(file_path): + fixJavaProtoOptions(file_path) return error_messages diff --git a/tools/check_format_test_helper.py b/tools/check_format_test_helper.py index 879f82097ca6a..a278af58dfee5 100755 --- a/tools/check_format_test_helper.py +++ b/tools/check_format_test_helper.py @@ -49,6 +49,9 @@ def runCheckFormat(operation, filename): def getInputFile(filename): infile = os.path.join(src, filename) + directory = os.path.dirname(filename) + if not directory == '' and not os.path.isdir(directory): + os.makedirs(directory) shutil.copyfile(infile, filename) return filename @@ -58,6 +61,10 @@ def getInputFile(filename): # code. def fixFileHelper(filename): infile = os.path.join(src, filename) + directory = os.path.dirname(filename) + if not directory == '' and not os.path.isdir(directory): + os.makedirs(directory) + shutil.copyfile(infile, filename) command, status, stdout = runCheckFormat("fix", getInputFile(filename)) return (command, infile, filename, status, stdout) @@ -207,6 +214,7 @@ def checkFileExpectingOK(filename): errors += checkAndFixError("license.BUILD", "envoy_build_fixer check failed") errors += checkAndFixError("bad_envoy_build_sys_ref.BUILD", "Superfluous '@envoy//' prefix") errors += checkAndFixError("proto_format.proto", "clang-format check failed") + errors += checkAndFixError("api/java_options.proto", "Java proto option") errors += checkFileExpectingOK("real_time_source_override.cc") errors += checkFileExpectingOK("time_system_wait_for.cc") diff --git a/tools/testdata/check_format/api/java_options.proto b/tools/testdata/check_format/api/java_options.proto new file mode 100644 index 0000000000000..a979cc6d23747 --- /dev/null +++ b/tools/testdata/check_format/api/java_options.proto @@ -0,0 +1 @@ +package envoy.foo; diff --git a/tools/testdata/check_format/api/java_options.proto.gold b/tools/testdata/check_format/api/java_options.proto.gold new file mode 100644 index 0000000000000..aaf202ec0aae0 --- /dev/null +++ b/tools/testdata/check_format/api/java_options.proto.gold @@ -0,0 +1,3 @@ +package envoy.foo; +option java_package = "io.envoyproxy.envoy.foo"; +option java_multiple_files = true; From 461a379304e5c40ba812808914d188fba555e069 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 7 Jan 2019 11:27:55 -0500 Subject: [PATCH 2/6] format Signed-off-by: Snow Pettersen --- tools/check_format.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/check_format.py b/tools/check_format.py index 673737e4eceae..3b831abe6eb7c 100755 --- a/tools/check_format.py +++ b/tools/check_format.py @@ -142,6 +142,7 @@ def checkNamespace(file_path): return ["Unable to find Envoy namespace or NOLINT(namespace-envoy) for file: %s" % file_path] return [] + def fixJavaProtoOptions(file_path): java_multiple_files = False java_package_correct = False @@ -154,19 +155,20 @@ def fixJavaProtoOptions(file_path): if "option java_package = \"io.envoyproxy.envoy" in line: java_package_correct = True if java_multiple_files and java_package_correct: - return + return to_add = "" if not java_package_correct: - to_add = to_add + "option java_package = \"io.envoyproxy.{}\";\n".format(package_name) + to_add = to_add + "option java_package = \"io.envoyproxy.{}\";\n".format(package_name) if not java_multiple_files: - to_add = to_add + "option java_multiple_files = true;\n" + to_add = to_add + "option java_multiple_files = true;\n" for line in fileinput.FileInput(file_path, inplace=True): if line.startswith("package "): line = line.replace(line, line + to_add) sys.stdout.write(line) + def checkJavaProtoOptions(file_path): java_multiple_files = False java_package_correct = False From bc04ed3804765923d3449247e8bde0b2574729eb Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 16 Jan 2019 19:45:25 -0500 Subject: [PATCH 3/6] pr feedback Signed-off-by: Snow Pettersen --- tools/check_format.py | 3 ++- tools/check_format_test_helper.py | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/check_format.py b/tools/check_format.py index 3b831abe6eb7c..c1a85e95f512d 100755 --- a/tools/check_format.py +++ b/tools/check_format.py @@ -48,6 +48,7 @@ SUBDIR_SET = set(common.includeDirOrder()) INCLUDE_ANGLE = "#include <" INCLUDE_ANGLE_LEN = len(INCLUDE_ANGLE) +PROTO_PACKAGE_REGEX = re.compile("package (.*);") # yapf: disable PROTOBUF_TYPE_ERRORS = { @@ -149,7 +150,7 @@ def fixJavaProtoOptions(file_path): package_name = None for line in fileinput.FileInput(file_path): if line.startswith("package "): - package_name = re.compile("package (.*);").search(line).group(1) + package_name = PROTO_PACKAGE_REGEX.search(line).group(1) if "option java_multiple_files = true;" in line: java_multiple_files = True if "option java_package = \"io.envoyproxy.envoy" in line: diff --git a/tools/check_format_test_helper.py b/tools/check_format_test_helper.py index a278af58dfee5..7e41b17b3f52c 100755 --- a/tools/check_format_test_helper.py +++ b/tools/check_format_test_helper.py @@ -62,8 +62,6 @@ def getInputFile(filename): def fixFileHelper(filename): infile = os.path.join(src, filename) directory = os.path.dirname(filename) - if not directory == '' and not os.path.isdir(directory): - os.makedirs(directory) shutil.copyfile(infile, filename) command, status, stdout = runCheckFormat("fix", getInputFile(filename)) From 8d0974079984d765cad116c389a1ce7c8366fe05 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 16 Jan 2019 20:32:01 -0500 Subject: [PATCH 4/6] check result of regex, add test for failure case Signed-off-by: Snow Pettersen --- tools/check_format.py | 13 +++++++++++-- tools/check_format_test_helper.py | 6 ++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/tools/check_format.py b/tools/check_format.py index c1a85e95f512d..d217bc34a2294 100755 --- a/tools/check_format.py +++ b/tools/check_format.py @@ -150,13 +150,20 @@ def fixJavaProtoOptions(file_path): package_name = None for line in fileinput.FileInput(file_path): if line.startswith("package "): + result = PROTO_PACKAGE_REGEX.search(line) + if result is None or len(result.groups()) != 1: + continue + package_name = PROTO_PACKAGE_REGEX.search(line).group(1) if "option java_multiple_files = true;" in line: java_multiple_files = True if "option java_package = \"io.envoyproxy.envoy" in line: java_package_correct = True if java_multiple_files and java_package_correct: - return + return [] + + if package_name is None: + return ["Unable to find package name for proto file: %s" % file_path] to_add = "" if not java_package_correct: @@ -169,6 +176,8 @@ def fixJavaProtoOptions(file_path): line = line.replace(line, line + to_add) sys.stdout.write(line) + return [] + def checkJavaProtoOptions(file_path): java_multiple_files = False @@ -411,7 +420,7 @@ def fixSourcePath(file_path): error_messages += fixHeaderOrder(file_path) error_messages += clangFormat(file_path) if file_path.endswith(PROTO_SUFFIX) and isApiFile(file_path): - fixJavaProtoOptions(file_path) + error_messages += fixJavaProtoOptions(file_path) return error_messages diff --git a/tools/check_format_test_helper.py b/tools/check_format_test_helper.py index 7e41b17b3f52c..6ffae4a8b73cd 100755 --- a/tools/check_format_test_helper.py +++ b/tools/check_format_test_helper.py @@ -62,6 +62,8 @@ def getInputFile(filename): def fixFileHelper(filename): infile = os.path.join(src, filename) directory = os.path.dirname(filename) + if not directory == '' and not os.path.isdir(directory): + os.makedirs(directory) shutil.copyfile(infile, filename) command, status, stdout = runCheckFormat("fix", getInputFile(filename)) @@ -199,6 +201,10 @@ def checkFileExpectingOK(filename): errors += checkUnfixableError("testing_test.cc", "Don't use 'using testing::Test;, elaborate the type instead") + errors += fixFileExpectingFailure( + "api/missing_package.proto", + "Unable to find package name for proto file: ./api/missing_package.proto") + # The following files have errors that can be automatically fixed. errors += checkAndFixError("over_enthusiastic_spaces.cc", "./over_enthusiastic_spaces.cc:3: over-enthusiastic spaces") From 67bab61449c1246b3f0aab39f2c1affa759651ce Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 17 Jan 2019 10:24:41 -0500 Subject: [PATCH 5/6] add mising testdata Signed-off-by: Snow Pettersen --- tools/testdata/check_format/api/missing_package.proto | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tools/testdata/check_format/api/missing_package.proto diff --git a/tools/testdata/check_format/api/missing_package.proto b/tools/testdata/check_format/api/missing_package.proto new file mode 100644 index 0000000000000..e69de29bb2d1d From 30ede57070216775e544f07f21150049f51f3694 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 17 Jan 2019 10:47:33 -0500 Subject: [PATCH 6/6] resuse result Signed-off-by: Snow Pettersen --- tools/check_format.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/check_format.py b/tools/check_format.py index d217bc34a2294..f0f849c2551b5 100755 --- a/tools/check_format.py +++ b/tools/check_format.py @@ -154,7 +154,7 @@ def fixJavaProtoOptions(file_path): if result is None or len(result.groups()) != 1: continue - package_name = PROTO_PACKAGE_REGEX.search(line).group(1) + package_name = result.group(1) if "option java_multiple_files = true;" in line: java_multiple_files = True if "option java_package = \"io.envoyproxy.envoy" in line: