Merge pull request #2062 from kbrunham-intel/feat/add_dryrun_option
Added --verify option to verilog-formatter.
diff --git a/verilog/tools/formatter/BUILD b/verilog/tools/formatter/BUILD
index d1467b3..693f280 100644
--- a/verilog/tools/formatter/BUILD
+++ b/verilog/tools/formatter/BUILD
@@ -68,6 +68,22 @@
)
sh_test_with_runfiles_lib(
+ name = "format-file-check_test",
+ size = "small",
+ srcs = ["format_file_check_test.sh"],
+ args = ["$(location :verible-verilog-format)"],
+ data = [":verible-verilog-format"],
+)
+
+sh_test_with_runfiles_lib(
+ name = "format-file-nochange-check_test",
+ size = "small",
+ srcs = ["format_file_check_nochange_test.sh"],
+ args = ["$(location :verible-verilog-format)"],
+ data = [":verible-verilog-format"],
+)
+
+sh_test_with_runfiles_lib(
name = "format-file-lex-error_test",
size = "small",
srcs = ["format_file_lex_error_test.sh"],
diff --git a/verilog/tools/formatter/format_file_check_nochange_test.sh b/verilog/tools/formatter/format_file_check_nochange_test.sh
new file mode 100755
index 0000000..7a73fdb
--- /dev/null
+++ b/verilog/tools/formatter/format_file_check_nochange_test.sh
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# Copyright 2017-2020 The Verible Authors.
+#
+# 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.
+
+# Tests verible-verilog-format reading from a file, checking for formatting
+# changes where no changes are needed. This should return 0.
+
+declare -r MY_OUTPUT_FILE="${TEST_TMPDIR}/myoutput.txt"
+
+# Get tool from argument
+[[ "$#" == 1 ]] || {
+ echo "Expecting 1 positional argument, verible-verilog-format path."
+ exit 1
+}
+formatter="$(rlocation ${TEST_WORKSPACE}/$1)"
+
+# Will overwrite this file in-place.
+cat >${MY_OUTPUT_FILE} <<EOF
+ module m ;endmodule
+EOF
+
+# Run formatter.
+${formatter} --inplace ${MY_OUTPUT_FILE} || exit 1
+
+# Now run the verify mode, which should return 0 since changes
+# are already made
+${formatter} --verify ${MY_OUTPUT_FILE}
+if [ "$?" -neq 0 ]; then
+ echo "No changes should result in 0 error code"
+ echo "FAIL"
+ exit 1
+fi
+
+echo "PASS"
diff --git a/verilog/tools/formatter/format_file_check_test.sh b/verilog/tools/formatter/format_file_check_test.sh
new file mode 100755
index 0000000..bcb101a
--- /dev/null
+++ b/verilog/tools/formatter/format_file_check_test.sh
@@ -0,0 +1,41 @@
+#!/usr/bin/env bash
+# Copyright 2017-2020 The Verible Authors.
+#
+# 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.
+
+# Tests verible-verilog-format reading from a file, checking for formatting
+# changes where changes are needed. This should return 1.
+
+
+declare -r MY_INPUT_FILE="${TEST_TMPDIR}/myinput.txt"
+
+# Get tool from argument
+[[ "$#" == 1 ]] || {
+ echo "Expecting 1 positional argument, verible-verilog-format path."
+ exit 1
+}
+formatter="$(rlocation ${TEST_WORKSPACE}/${1})"
+
+cat >${MY_INPUT_FILE} <<EOF
+ module m ;endmodule
+EOF
+
+# Run formatter.
+${formatter} --verbose --verify ${MY_INPUT_FILE}
+if [ "$?" -eq 0 ]; then
+ echo "Changes should produce non-zero error code"
+ echo "FAIL"
+ exit 1
+fi
+
+echo "PASS"
diff --git a/verilog/tools/formatter/verilog_format.cc b/verilog/tools/formatter/verilog_format.cc
index fd6c514..170cdf7 100644
--- a/verilog/tools/formatter/verilog_format.cc
+++ b/verilog/tools/formatter/verilog_format.cc
@@ -85,6 +85,11 @@
// TODO(fangism): Provide -i alias, as it is canonical to many formatters
ABSL_FLAG(bool, inplace, false,
"If true, overwrite the input file on successful conditions.");
+ABSL_FLAG(
+ bool, verify, false,
+ "If true, only checks if formatting would be done. Return code 0 means "
+ "no files would change. Return code 1 means some files would "
+ "be reformatted.");
ABSL_FLAG(std::string, stdin_name, "<stdin>",
"When using '-' to read from stdin, this gives an alternate name for "
"diagnostic purposes. Otherwise this is ignored.");
@@ -125,11 +130,15 @@
return std::cerr;
}
+// TODO: Refactor and simplify
static bool formatOneFile(absl::string_view filename,
- const LineNumberSet& lines_to_format) {
+ const LineNumberSet& lines_to_format,
+ bool* any_changes) {
const bool inplace = absl::GetFlag(FLAGS_inplace);
+ const bool check_changes_only = absl::GetFlag(FLAGS_verify);
const bool is_stdin = filename == "-";
const auto& stdin_name = absl::GetFlag(FLAGS_stdin_name);
+ *any_changes = false;
if (inplace && is_stdin) {
FileMsg(filename)
@@ -203,21 +212,34 @@
return absl::GetFlag(FLAGS_failsafe_success);
}
- // Safe to write out result, having passed above verification.
- if (inplace && !is_stdin) {
- // Don't write if the output is exactly as the input, so that we don't mess
- // with tools that look for timestamp changes (such as make).
- if (*content_or != formatted_output) {
- if (auto status = verible::file::SetContents(filename, formatted_output);
- !status.ok()) {
- FileMsg(filename) << "error writing result " << status << std::endl;
- return false;
- }
+ // Check if the output is the same as the input.
+ *any_changes = (*content_or != formatted_output);
+
+ // Don't output or write if --check is set.
+ if (check_changes_only) {
+ if (*any_changes) {
+ FileMsg(filename) << "Needs formatting." << std::endl;
} else if (absl::GetFlag(FLAGS_verbose)) {
FileMsg(filename) << "Already formatted, no change." << std::endl;
}
} else {
- std::cout << formatted_output;
+ // Safe to write out result, having passed above verification.
+ if (inplace && !is_stdin) {
+ // Don't write if the output is exactly as the input, so that we don't
+ // mess with tools that look for timestamp changes (such as make).
+ if (*any_changes) {
+ if (auto status =
+ verible::file::SetContents(filename, formatted_output);
+ !status.ok()) {
+ FileMsg(filename) << "error writing result " << status << std::endl;
+ return false;
+ }
+ } else if (absl::GetFlag(FLAGS_verbose)) {
+ FileMsg(filename) << "Already formatted, no change." << std::endl;
+ }
+ } else {
+ std::cout << formatted_output;
+ }
}
return true;
@@ -260,11 +282,19 @@
}
bool all_success = true;
+ bool any_changes = false;
// All positional arguments are file names. Exclude program name.
for (const absl::string_view filename :
verible::make_range(file_args.begin() + 1, file_args.end())) {
- all_success &= formatOneFile(filename, lines_to_format);
+ all_success &= formatOneFile(filename, lines_to_format, &any_changes);
}
- return all_success ? 0 : 1;
+ int ret_val = 0;
+ if (absl::GetFlag(FLAGS_verify)) {
+ ret_val = any_changes;
+ } else {
+ ret_val = all_success ? 0 : 1;
+ }
+
+ return ret_val;
}
