Merge pull request chipsalliance#2286 from hellux/fmt-verify

formatter: fix --verify for multiple files

verilog/tools/formatter/BUILD

@@ -78,14 +78,6 @@ sh_test_with_runfiles_lib(
     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",

verilog/tools/formatter/format_file_check_test.sh

@@ -13,29 +13,47 @@
 # 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.
+# Tests verible-verilog-format reading from single or multiple files, checking
+# for formatting changes where changes are needed. This should return 1 if any
+# of the files are incorrectly formatted.
 
 
-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
+# create 1 formatted and 1 unformatted file
+unformatted="${TEST_TMPDIR}/unformatted.sv"
+formatted="${TEST_TMPDIR}/formatted.sv"
+cat >"${unformatted}" <<EOF
   module    m   ;endmodule
 EOF
+cp "$unformatted" "$formatted"
+$formatter --inplace "$formatted"
+
+cases=(
+    "$formatted"
+    "$unformatted"
+    "$unformatted $unformatted"
+    "$unformatted $formatted"
+    "$formatted $unformatted"
+    "$formatted $formatted"
+)
 
-# 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
+for files in "${cases[@]}"; do
+    echo "$files" | grep -q unformatted.sv
+    expected_ret=$((!$?))
+    echo Formatting...
+    ${formatter} --verbose --verify --inplace $files
+    actual_ret=$?
+    if [ "$actual_ret" -ne "$expected_ret" ]; then
+        echo "Expected return code $expected_ret, got $actual_ret"
+        echo "FAIL"
+        exit 1
+    fi
+done
 
 echo "PASS"

verilog/tools/formatter/verilog_format.cc

@@ -137,7 +137,6 @@ static bool formatOneFile(absl::string_view filename,
   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)
@@ -212,11 +211,12 @@ static bool formatOneFile(absl::string_view filename,
   }
 
   // Check if the output is the same as the input.
-  *any_changes = (*content_or != formatted_output);
+  const bool file_changed = (*content_or != formatted_output);
+  *any_changes |= file_changed;
 
   // Don't output or write if --check is set.
   if (check_changes_only) {
-    if (*any_changes) {
+    if (file_changed) {
       FileMsg(filename) << "Needs formatting." << std::endl;
     } else if (absl::GetFlag(FLAGS_verbose)) {
       FileMsg(filename) << "Already formatted, no change." << std::endl;
@@ -226,7 +226,7 @@ static bool formatOneFile(absl::string_view filename,
     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 (file_changed) {
         if (auto status =
                 verible::file::SetContents(filename, formatted_output);
             !status.ok()) {