Merge branch 'master' into 6269/ci/strip-fat-bins

.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
+    -v, --verbose
-        Print the version of clang-format being used before continuing.
+        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')
+    '-v'|'--verbose')
-      print_version=1
+      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)"
+  [[ $do_cmake ]] && echo "cmake-format version $(cmake-format --version)"
-  echo "----------"
+  [[ $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/email_parser.cpp

@@ -25,7 +25,7 @@ QPair<QString, QString> EmailParser::parseEmailAddress(const QString &dirtyEmail
 
     // Trim out dots and pluses from Google/Gmail domains
     if (capturedEmailAddressDomain.toLower() == "gmail.com") {
-        // Remove all content after first plus sign (as unnecessary with gmail)
+        // Remove all content after the first plus sign (as unnecessary with gmail)
         // https://gmail.googleblog.com/2008/03/2-hidden-ways-to-get-more-from-your.html
         const auto firstPlusSign = capturedEmailUser.indexOf("+");
         if (firstPlusSign != -1) {
@@ -36,6 +36,13 @@ QPair<QString, QString> EmailParser::parseEmailAddress(const QString &dirtyEmail
         // https://gmail.googleblog.com/2008/03/2-hidden-ways-to-get-more-from-your.html
         capturedEmailUser.replace(".", "");
     }
+    // Trim out minuses from Yahoo domains
+    else if (capturedEmailAddressDomain.toLower() == "yahoo.com") {
+        const auto firstMinusSign = capturedEmailUser.indexOf("-");
+        if (firstMinusSign != -1) {
+            capturedEmailUser = capturedEmailUser.left(firstMinusSign);
+        }
+    }
 
     return {capturedEmailUser, capturedEmailAddressDomain};
 }

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);