Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions tools/check_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -143,6 +144,41 @@ def checkNamespace(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 "):
result = PROTO_PACKAGE_REGEX.search(line)
if result is None or len(result.groups()) != 1:
continue

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:
java_package_correct = True
if java_multiple_files and java_package_correct:
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:
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)

return []


def checkJavaProtoOptions(file_path):
java_multiple_files = False
java_package_correct = False
Expand Down Expand Up @@ -383,6 +419,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):
error_messages += fixJavaProtoOptions(file_path)
return error_messages


Expand Down
12 changes: 12 additions & 0 deletions tools/check_format_test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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):
Comment thread
jmarantz marked this conversation as resolved.
os.makedirs(directory)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why this is needed. If there's a filename reaching here, shouldn't we expect the directory to exist?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the time I added it because I had issues with the fact that the test file is api/java_options.proto and requires api to exist, but seems like I can remove this and it still works.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other mkdirs is necessary though to avoid:

diff proto_format.proto /source/tools/testdata/check_format/proto_format.proto.gold
Traceback (most recent call last):
  File "./tools/check_format_test_helper.py", line 215, in <module>
    errors += checkAndFixError("api/java_options.proto", "Java proto option")
  File "./tools/check_format_test_helper.py", line 126, in checkAndFixError
    errors = checkFileExpectingError(filename, expected_substring)
  File "./tools/check_format_test_helper.py", line 121, in checkFileExpectingError
    command, status, stdout = runCheckFormat("check", getInputFile(filename))
  File "./tools/check_format_test_helper.py", line 55, in getInputFile
    shutil.copyfile(infile, filename)
  File "/usr/lib/python2.7/shutil.py", line 83, in copyfile
    with open(dst, 'wb') as fdst:
IOError: [Errno 2] No such file or directory: 'api/java_options.proto'


shutil.copyfile(infile, filename)
command, status, stdout = runCheckFormat("fix", getInputFile(filename))
return (command, infile, filename, status, stdout)
Expand Down Expand Up @@ -194,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")
Expand All @@ -207,6 +218,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")
Expand Down
1 change: 1 addition & 0 deletions tools/testdata/check_format/api/java_options.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package envoy.foo;
3 changes: 3 additions & 0 deletions tools/testdata/check_format/api/java_options.proto.gold
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package envoy.foo;
option java_package = "io.envoyproxy.envoy.foo";
option java_multiple_files = true;
Empty file.