From d00c72d7acc33cd5106f935ceffa262fec266d78 Mon Sep 17 00:00:00 2001
From: Noah Hellman <noah@hllmn.net>
Date: Fri, 1 Nov 2024 19:40:58 +0100
Subject: [PATCH 1/2] format_file_check_test: extend to test multiple files

generalize format_file_check_test.sh to test both formatted and
unformatted files, single or multiple files at a time

test that --verify returns correct return code for each case

(currently fails for multiple files)

remove format_file_check_nochange_test.sh because its test case is also
covered
---
 verilog/tools/formatter/BUILD                 |  8 ----
 .../format_file_check_nochange_test.sh        | 45 -------------------
 .../tools/formatter/format_file_check_test.sh | 42 ++++++++++++-----
 3 files changed, 30 insertions(+), 65 deletions(-)
 delete mode 100755 verilog/tools/formatter/format_file_check_nochange_test.sh

diff --git a/verilog/tools/formatter/BUILD b/verilog/tools/formatter/BUILD
index b2d404618..85bd5a308 100644
--- a/verilog/tools/formatter/BUILD
+++ b/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",
diff --git a/verilog/tools/formatter/format_file_check_nochange_test.sh b/verilog/tools/formatter/format_file_check_nochange_test.sh
deleted file mode 100755
index 7a73fdbe8..000000000
--- a/verilog/tools/formatter/format_file_check_nochange_test.sh
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/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
index bcb101a01..50979a3a7 100755
--- a/verilog/tools/formatter/format_file_check_test.sh
+++ b/verilog/tools/formatter/format_file_check_test.sh
@@ -13,12 +13,11 @@
 # 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."
@@ -26,16 +25,35 @@ declare -r MY_INPUT_FILE="${TEST_TMPDIR}/myinput.txt"
 }
 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"

From ab2f4e99d00e1ed41e02d5f38198329c092da5ff Mon Sep 17 00:00:00 2001
From: Noah Hellman <noah@hllmn.net>
Date: Fri, 1 Nov 2024 18:49:40 +0100
Subject: [PATCH 2/2] verilog_format: fix --verify for multiple files

change state was previously overwritten for each file so the return type
only indicated if last file was formatted correctly
---
 verilog/tools/formatter/verilog_format.cc | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/verilog/tools/formatter/verilog_format.cc b/verilog/tools/formatter/verilog_format.cc
index 88995529a..29418e691 100644
--- a/verilog/tools/formatter/verilog_format.cc
+++ b/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()) {