update format.sh (#6240)

* update format.sh

add shellcheck to format.sh
add statement macros to .clang-format
add no clang format to format.sh
add changed file list to format.sh diff
rename --cf-version to --print-version in format.sh
lint files

* enable --shell on ci runs

* remove useless semicolons

removes the semicolons after empty function definitions
these semicolons are optional, they don't do anything
this will have functions be consistently formatted
if we want to keep the option to have these on the same line like they
were before we should use the option AllowShortFunctionsOnASingleLine: None

* fix script

* update echo line in lint_cpp.sh which doesn't lint cpp only at all

.ci/docker.sh

@@ -137,10 +137,11 @@ if [[ $SAVE ]]; then
 fi
 
 # Set compile function, runs the compile script on the image, passes arguments to the script
+# shellcheck disable=2120
 function RUN ()
 {
   echo "running image:"
-  if [[ $(docker images) =~ "$IMAGE_NAME" ]]; then
+  if [[ $(docker images) =~ $IMAGE_NAME ]]; then
     local args=(--mount "type=bind,source=$PWD,target=/src")
     args+=(--workdir "/src")
     args+=(--user "$(id -u):$(id -g)")
@@ -151,6 +152,7 @@ function RUN ()
     if [[ -n "$CMAKE_GENERATOR" ]]; then
       args+=(--env "CMAKE_GENERATOR=$CMAKE_GENERATOR")
     fi
+  # shellcheck disable=2086
     docker run "${args[@]}" $RUN_ARGS "$IMAGE_NAME" bash "$BUILD_SCRIPT" $RUN_OPTS "$@"
     return $?
   else
@@ -164,5 +166,6 @@ function RUN ()
 if [[ $INTERACTIVE ]]; then
   export BUILD_SCRIPT="-i"
   export RUN_ARGS="$RUN_ARGS -it"
+  # shellcheck disable=2119
   RUN
 fi

.ci/lint_cpp.sh

@@ -11,11 +11,19 @@ if ! git merge-base origin/master HEAD; then
 fi
 
 # Check formatting using format.sh
-echo "Checking your code using clang-format/cmake-format..."
+echo "Checking your code using format.sh..."
 
-diff="$(./format.sh --diff --cmake --cf-version --branch origin/master)"
+diff="$(./format.sh --diff --cmake --shell --print-version --branch origin/master)"
 err=$?
 
+sep="
+----------
+"
+used_version="${diff%%"$sep"*}"
+diff="${diff#*"$sep"}"
+changes_to_make="${diff%%"$sep"*}"
+files_to_edit="${diff#*"$sep"}"
+
 case $err in
   1)
     cat <<EOM
@@ -33,14 +41,13 @@ case $err in
 ***********************************************************
 
 Used version:
-${diff%%
-----------
-*}
+$used_version
+
+Affected files:
+$files_to_edit
 
 The following changes should be made:
-${diff#*
-----------
-}
+$changes_to_make
 
 Exiting...
 EOM
@@ -58,6 +65,9 @@ EOM
 ***                                                     ***
 ***********************************************************
 
+Used version:
+$used_version
+
 Exiting...
 EOM
     exit 0

.clang-format

@@ -25,6 +25,9 @@ IndentCaseLabels: true
 PointerAlignment: Right
 SortIncludes: true
 IncludeBlocks: Regroup
+StatementAttributeLikeMacros: [emit]
+# requires clang-format 16
+# RemoveSemicolon: true
 ---
 Language: Proto
 AllowShortFunctionsOnASingleLine: None

.github/workflows/desktop-lint.yml

@@ -20,7 +20,7 @@ on:
 
 jobs:
   format:
-    runs-on: ubuntu-22.04
+    runs-on: ubuntu-latest
 
     steps:
     - name: Checkout
@@ -32,7 +32,7 @@ jobs:
       shell: bash
       run: |
         sudo apt-get update
-        sudo apt-get install -y --no-install-recommends clang-format cmake-format
+        sudo apt-get install -y --no-install-recommends clang-format cmake-format shellcheck
 
     - name: Check code formatting
       shell: bash

cockatrice/src/filters/filter_card.h

@@ -52,7 +52,9 @@ private:
     enum Attr a;
 
 public:
-    CardFilter(QString &term, Type type, Attr attr) : trm(term), t(type), a(attr){};
+    CardFilter(QString &term, Type type, Attr attr) : trm(term), t(type), a(attr)
+    {
+    }
 
     Type type() const
     {

cockatrice/src/interface/widgets/tabs/visual_deck_storage/tab_deck_storage_visual.h

@@ -27,7 +27,9 @@ class TabDeckStorageVisual final : public Tab
     Q_OBJECT
 public:
     explicit TabDeckStorageVisual(TabSupervisor *_tabSupervisor);
-    void retranslateUi() override{};
+    void retranslateUi() override
+    {
+    }
     [[nodiscard]] QString getTabText() const override
     {
         return tr("Visual Deck Storage");

format.sh

@@ -2,7 +2,8 @@
 
 # This script will run clang-format on all modified, non-3rd-party C++/Header files.
 # Optionally runs cmake-format on all modified cmake files.
-# Uses clang-format cmake-format git diff find
+# Optionally runs shellcheck on all modified shell files.
+# Uses clang-format cmake-format git diff find shellcheck
 # Never, ever, should this receive a path with a newline in it. Don't bother proofing it for that.
 
 set -o pipefail
@@ -36,7 +37,10 @@ branch="origin/master"
 cmakefile="CMakeLists.txt"
 cmakedir="cmake/.*\\.cmake"
 cmakeinclude=("cmake/gtest-CMakeLists.txt.in")
+scripts="*.sh"
 color="--"
+verbosity=0
+sep="----------"
 
 # parse options
 while [[ $* ]]; do
@@ -102,11 +106,21 @@ OPTIONS:
     -n, --names
         Display a list of filenames that require formatting. Implies --test.
 
+    --no-clang-format
+        Do not check any source files for clang-format.
+
+    --print-version
+        Print the version of clang-format being used before continuing.
+
+    --shell
+        Use shellcheck to lint shell files. Not available in the default inline
+        mode.
+
     -t, --test
         Do not edit files in place. Set exit code to 1 if changes are required.
 
-    --cf-version
-        Print the version of clang-format being used before continuing.
+    -v, --verbose
+        Display output on successes.
 
 EXIT CODES:
     0 on a successful format or if no files require formatting.
@@ -123,7 +137,7 @@ EXAMPLES:
         Tests if the source files in the current directory are correctly
         formatted and prints an error message if formatting is required.
 
-    $0 --cmake --branch "" ""
+    $0 --cmake --branch "" --no-clang-format
         Unconditionally format all cmake files and no source files.
 EOM
       exit 0
@@ -132,12 +146,24 @@ EOM
       mode="name"
       shift
       ;;
+    '--no-clang-format')
+      include=() # do not check any dirs
+      shift
+      ;;
+    '--print-version')
+      print_version=1
+      shift
+      ;;
+    '--shell')
+      do_shell=1
+      shift
+      ;;
     '-t'|'--test')
       mode="code"
       shift
       ;;
-    '--cf-version')
-      print_version=1
+    '-v'|'--verbose')
+      verbosity=1
       shift
       ;;
     '--')
@@ -191,6 +217,12 @@ if [[ $do_cmake ]] && ! hash cmake-format 2>/dev/null; then
   exit 3
 fi
 
+# check availability of shellcheck
+if [[ $do_shell ]] && ! hash shellcheck 2>/dev/null; then
+  echo "could not find shellcheck" >&2
+  exit 3
+fi
+
 if [[ $branch ]]; then
   # get all dirty files through git
   if ! base=$(git merge-base "$branch" HEAD); then
@@ -224,6 +256,15 @@ if [[ $branch ]]; then
       done
     done
   fi
+  if [[ $do_shell ]]; then
+    shell_names=()
+    for name in "${basenames[@]}"; do
+      filerx="(^|/)$scripts$"
+      if [[ $name =~ $filerx ]]; then
+        shell_names+=("$name")
+      fi
+    done
+  fi
 else
   exts_o=()
   for ext in "${exts[@]}"; do
@@ -235,6 +276,9 @@ else
     mapfile -t cmake_names < <(find . -maxdepth 2 -type f -name "$cmakefile" -o -path "./${cmakedir/.}")
     cmake_names+=("${cmakeinclude[@]}")
   fi
+  if [[ $do_shell ]]; then
+    mapfile -t shell_names < <(find . -maxdepth 5 -type f -name "$scripts")
+  fi
 fi
 
 # filter excludes
@@ -250,14 +294,18 @@ done
 # optionally print version
 if [[ $print_version ]]; then
   $cf_cmd -version
-  [[ $do_cmake ]] && echo "cmake-format $(cmake-format --version)"
-  echo "----------"
+  [[ $do_cmake ]] && echo "cmake-format version $(cmake-format --version)"
+  [[ $do_shell ]] && echo "shellcheck $(shellcheck --version | grep "version:")"
+  echo "$sep"
 fi
 
 if [[ ! ${cmake_names[*]} ]]; then
   unset do_cmake
 fi
-if [[ ! ( ${names[*]} || $do_cmake ) ]]; then
+if [[ ! ${shell_names[*]} ]]; then
+  unset do_shell
+fi
+if [[ ! ( ${names[*]} || $do_cmake || $do_shell ) ]]; then
   exit 0 # nothing to format means format is successful!
 fi
 
@@ -265,16 +313,31 @@ fi
 case $mode in
   diff)
     declare -i code=0
+    files_to_format=()
     for name in "${names[@]}"; do
       if ! $cf_cmd "$name" | diff "$name" - -p "$color"; then
         code=1
+        files_to_format+=("$name")
       fi
     done
     for name in "${cmake_names[@]}"; do
       if ! cmake-format "$name" | diff "$name" - -p "$color"; then
         code=1
+        files_to_format+=("$name")
       fi
     done
+    for name in "${shell_names[@]}"; do
+      if ! shellcheck "$name"; then
+        code=1
+        files_to_format+=("$name")
+      fi
+    done
+    if (( code>0 )); then
+      echo "$sep"
+      for name in "${files_to_format[@]}"; do
+        echo "$name"
+      done
+    fi
     exit $code
     ;;
   name)
@@ -291,6 +354,12 @@ case $mode in
         code=1
       fi
     done
+    for name in "${shell_names[@]}"; do
+      if ! shellcheck "$name" >/dev/null; then
+        echo "$name"
+        code=1
+      fi
+    done
     exit $code
     ;;
   code)
@@ -300,6 +369,9 @@ case $mode in
     for name in "${cmake_names[@]}"; do
       cmake-format "$name" --check || exit 1
     done
+    for name in "${shell_names[@]}"; do
+      shellcheck "$name" >/dev/null || exit 1
+    done
     ;;
   *)
     if [[ "${names[*]}" ]]; then
@@ -308,5 +380,16 @@ case $mode in
     if [[ $do_cmake ]]; then
       cmake-format -i "${cmake_names[@]}"
     fi
+    if [[ $do_shell ]]; then
+      echo "warning: --shell is not compatible with the current mode but shell files were modified!" >&2
+      echo "recommendation: try $0 --diff --shell" >&2
+    fi
+    if (( verbosity>0 )); then
+      count="${#names[*]}"
+      if [[ $do_cmake ]]; then
+        (( count+=${#cmake_names[*]} ))
+      fi
+      echo "parsed $count files that differ from base $branch"
+    fi
     ;;
 esac

libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h

@@ -146,7 +146,9 @@ public:
                             const QString & /* logMessage */,
                             LogMessage_TargetType /* targetType */,
                             const int /* targetId */,
-                            const QString & /* targetName */){};
+                            const QString & /* targetName */)
+    {
+    }
     virtual bool checkUserIsBanned(Server_ProtocolHandler * /* session */,
                                    QString & /* banReason */,
                                    int & /* banSecondsRemaining */)

oracle/src/oraclewizard.h

@@ -182,7 +182,9 @@ class LoadSpoilersPage : public SimpleDownloadFilePage
 {
     Q_OBJECT
 public:
-    explicit LoadSpoilersPage(QWidget * = nullptr){};
+    explicit LoadSpoilersPage(QWidget * = nullptr)
+    {
+    }
     void retranslateUi() override;
 
 protected:
@@ -197,7 +199,9 @@ class LoadTokensPage : public SimpleDownloadFilePage
 {
     Q_OBJECT
 public:
-    explicit LoadTokensPage(QWidget * = nullptr){};
+    explicit LoadTokensPage(QWidget * = nullptr)
+    {
+    }
     void retranslateUi() override;
 
 protected:

oracle/src/pagetemplates.h

@@ -13,7 +13,9 @@ class OracleWizardPage : public QWizardPage
 {
     Q_OBJECT
 public:
-    explicit OracleWizardPage(QWidget *parent = nullptr) : QWizardPage(parent){};
+    explicit OracleWizardPage(QWidget *parent = nullptr) : QWizardPage(parent)
+    {
+    }
     virtual void retranslateUi() = 0;
 
 signals:

servatrice/check_schema_version.sh

@@ -6,6 +6,7 @@ version_line="$(grep 'INSERT INTO cockatrice_schema_version' servatrice/servatri
 version_line="${version_line#*VALUES(}"
 declare -i schema_ver="${version_line%%)*}"
 
+# shellcheck disable=2012
 latest_migration="$(ls -1 servatrice/migrations/ | tail -n1)"
 xtoysql="${latest_migration#servatrice_}"
 xtoy="${xtoysql%.sql}"
@@ -23,7 +24,7 @@ if ((schema_ver != new_ver)); then
 fi
 
 expected_sql="^UPDATE cockatrice_schema_version SET version=${new_ver} WHERE version=${old_ver};$"
-if ! grep -q "$expected_sql" servatrice/migrations/$latest_migration; then
+if ! grep -q "$expected_sql" "servatrice/migrations/$latest_migration"; then
     echo "$latest_migration does not contain expected sql: $expected_sql"
     exit 1
 fi

servatrice/src/serversocketinterface.h

@@ -146,7 +146,9 @@ public:
     AbstractServerSocketInterface(Servatrice *_server,
                                   Servatrice_DatabaseInterface *_databaseInterface,
                                   QObject *parent = 0);
-    ~AbstractServerSocketInterface(){};
+    ~AbstractServerSocketInterface()
+    {
+    }
     bool initSession();
 
     virtual QHostAddress getPeerAddress() const = 0;

servatrice/src/signalhandler.h

@@ -10,7 +10,9 @@ class SignalHandler : public QObject
     Q_OBJECT
 public:
     SignalHandler(QObject *parent = 0);
-    ~SignalHandler(){};
+    ~SignalHandler()
+    {
+    }
     static void sigHupHandler(int /* sig */);
     static void sigSegvHandler(int sig);