Add code quality checks and formatting configuration

- Introduced a .clang-format file to enforce Google C++ style guidelines across the codebase. - Updated CMakeLists.txt to include custom targets for formatting and format-checking using clang-format. - Added a quality_check.sh script to automate code quality checks, including formatting and static analysis with cppcheck. - Enhanced CMakePresets.json with new macOS-specific configurations for ARM64 and universal binaries, improving build flexibility and support.
2025-09-26 12:57:20 -04:00
parent ec207cae06
commit 22c960eaf3
5 changed files with 248 additions and 24 deletions
--- a/.clang-format
+++ b/.clang-format
@@ -0,0 +1,83 @@
+# Google C/C++ Code Style settings
+# https://clang.llvm.org/docs/ClangFormatStyleOptions.html
+# Author: Kehan Xue, kehan.xue (at) gmail.com
+
+Language: Cpp
+BasedOnStyle: Google
+AccessModifierOffset: -1
+AlignAfterOpenBracket: Align
+AlignConsecutiveAssignments: None
+AlignOperands: Align
+AllowAllArgumentsOnNextLine: true
+AllowAllConstructorInitializersOnNextLine: true
+AllowAllParametersOfDeclarationOnNextLine: false
+AllowShortBlocksOnASingleLine: Empty
+AllowShortCaseLabelsOnASingleLine: false
+AllowShortFunctionsOnASingleLine: Inline
+AllowShortIfStatementsOnASingleLine: Never  # To avoid conflict, set this "Never" and each "if statement" should include brace when coding
+AllowShortLambdasOnASingleLine: Inline
+AllowShortLoopsOnASingleLine: false
+AlwaysBreakAfterReturnType: None
+AlwaysBreakTemplateDeclarations: Yes
+BinPackArguments: true
+BreakBeforeBraces: Custom
+BraceWrapping:
+  AfterCaseLabel: false
+  AfterClass: false
+  AfterStruct: false
+  AfterControlStatement: Never
+  AfterEnum: false
+  AfterFunction: false
+  AfterNamespace: false
+  AfterUnion: false
+  AfterExternBlock: false
+  BeforeCatch: false
+  BeforeElse: false
+  BeforeLambdaBody: false
+  IndentBraces: false
+  SplitEmptyFunction: false
+  SplitEmptyRecord: false
+  SplitEmptyNamespace: false
+BreakBeforeBinaryOperators: None
+BreakBeforeTernaryOperators: true
+BreakConstructorInitializers: BeforeColon
+BreakInheritanceList: BeforeColon
+ColumnLimit: 80
+CompactNamespaces: false
+ContinuationIndentWidth: 4
+Cpp11BracedListStyle: true
+DerivePointerAlignment: false  # Make sure the * or & align on the left
+EmptyLineBeforeAccessModifier: LogicalBlock
+FixNamespaceComments: true
+IncludeBlocks: Preserve
+IndentCaseLabels: true
+IndentPPDirectives: None
+IndentWidth: 2
+KeepEmptyLinesAtTheStartOfBlocks: true
+MaxEmptyLinesToKeep: 1
+NamespaceIndentation: None
+ObjCSpaceAfterProperty: false
+ObjCSpaceBeforeProtocolList: true
+PointerAlignment: Left
+ReflowComments: false
+# SeparateDefinitionBlocks: Always   # Only support since clang-format 14
+SpaceAfterCStyleCast: false
+SpaceAfterLogicalNot: false
+SpaceAfterTemplateKeyword: true
+SpaceBeforeAssignmentOperators: true
+SpaceBeforeCpp11BracedList: false
+SpaceBeforeCtorInitializerColon: true
+SpaceBeforeInheritanceColon: true
+SpaceBeforeParens: ControlStatements
+SpaceBeforeRangeBasedForLoopColon: true
+SpaceBeforeSquareBrackets: false
+SpaceInEmptyParentheses: false
+SpacesBeforeTrailingComments: 2
+SpacesInAngles: false
+SpacesInCStyleCastParentheses: false
+SpacesInContainerLiterals: false
+SpacesInParentheses: false
+SpacesInSquareBrackets: false
+Standard: c++11
+TabWidth: 4
+UseTab: Never
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -127,6 +127,26 @@ if (YAZE_BUILD_TESTS)
 add_subdirectory(test)
 endif()

+# Code quality targets
+find_program(CLANG_FORMAT NAMES clang-format clang-format-14 clang-format-15 clang-format-16 clang-format-17 clang-format-18)
+if(CLANG_FORMAT)
+  file(GLOB_RECURSE ALL_SOURCE_FILES 
+       "${CMAKE_SOURCE_DIR}/src/*.cc" 
+       "${CMAKE_SOURCE_DIR}/src/*.h"
+       "${CMAKE_SOURCE_DIR}/test/*.cc"
+       "${CMAKE_SOURCE_DIR}/test/*.h")
+  
+  add_custom_target(format
+    COMMAND ${CLANG_FORMAT} -i --style=Google ${ALL_SOURCE_FILES}
+    COMMENT "Running clang-format on source files"
+  )
+  
+  add_custom_target(format-check
+    COMMAND ${CLANG_FORMAT} --dry-run --Werror --style=Google ${ALL_SOURCE_FILES}
+    COMMENT "Checking code format"
+  )
+endif()
+
 # Packaging configuration
 include(cmake/packaging.cmake)

--- a/CMakePresets.json
+++ b/CMakePresets.json
@@ -10,7 +10,7 @@
      "name": "default",
      "displayName": "Default Config",
      "description": "Default build configuration",
-      "generator": "Ninja",
+      "generator": "Unix Makefiles",
      "binaryDir": "${sourceDir}/build",
      "cacheVariables": {
        "CMAKE_BUILD_TYPE": "RelWithDebInfo",
@@ -52,6 +52,16 @@
        "YAZE_TEST_ROM_PATH": "${sourceDir}/build/bin/zelda3.sfc"
      }
    },
+    {
+      "name": "macos-dev",
+      "displayName": "macOS Development (ARM64)",
+      "description": "macOS ARM64 development build with ROM testing",
+      "inherits": "macos-debug",
+      "cacheVariables": {
+        "YAZE_ENABLE_ROM_TESTS": "ON",
+        "YAZE_TEST_ROM_PATH": "${sourceDir}/build/bin/zelda3.sfc"
+      }
+    },
    {
      "name": "ci",
      "displayName": "Continuous Integration",
@@ -64,8 +74,38 @@
    },
    {
      "name": "macos-debug",
-      "displayName": "macOS Debug",
-      "description": "macOS-specific debug configuration",
+      "displayName": "macOS Debug (ARM64)",
+      "description": "macOS ARM64-specific debug configuration",
+      "inherits": "debug",
+      "condition": {
+        "type": "equals",
+        "lhs": "${hostSystemName}",
+        "rhs": "Darwin"
+      },
+      "cacheVariables": {
+        "CMAKE_OSX_DEPLOYMENT_TARGET": "11.0",
+        "CMAKE_OSX_ARCHITECTURES": "arm64"
+      }
+    },
+    {
+      "name": "macos-release",
+      "displayName": "macOS Release (ARM64)",
+      "description": "macOS ARM64-specific release configuration",
+      "inherits": "release",
+      "condition": {
+        "type": "equals",
+        "lhs": "${hostSystemName}",
+        "rhs": "Darwin"
+      },
+      "cacheVariables": {
+        "CMAKE_OSX_DEPLOYMENT_TARGET": "11.0",
+        "CMAKE_OSX_ARCHITECTURES": "arm64"
+      }
+    },
+    {
+      "name": "macos-debug-universal",
+      "displayName": "macOS Debug (Universal)",
+      "description": "macOS universal binary debug configuration",
      "inherits": "debug",
      "condition": {
        "type": "equals",
@@ -78,9 +118,9 @@
      }
    },
    {
-      "name": "macos-release",
-      "displayName": "macOS Release",
-      "description": "macOS-specific release configuration",
+      "name": "macos-release-universal",
+      "displayName": "macOS Release (Universal)",
+      "description": "macOS universal binary release configuration",
      "inherits": "release",
      "condition": {
        "type": "equals",
@@ -184,6 +224,11 @@
      "configurePreset": "dev",
      "displayName": "Development Build"
    },
+    {
+      "name": "macos-dev",
+      "configurePreset": "macos-dev",
+      "displayName": "macOS Development Build (ARM64)"
+    },
    {
      "name": "ci",
      "configurePreset": "ci",
@@ -197,7 +242,17 @@
    {
      "name": "macos-release",
      "configurePreset": "macos-release",
-      "displayName": "macOS Release Build"
+      "displayName": "macOS Release Build (ARM64)"
+    },
+    {
+      "name": "macos-debug-universal",
+      "configurePreset": "macos-debug-universal",
+      "displayName": "macOS Debug Build (Universal)"
+    },
+    {
+      "name": "macos-release-universal",
+      "configurePreset": "macos-release-universal",
+      "displayName": "macOS Release Build (Universal)"
    },
    {
      "name": "fast",
@@ -293,7 +348,12 @@
    {
      "name": "macos",
      "configurePreset": "macos-release",
-      "displayName": "macOS Package"
+      "displayName": "macOS Package (ARM64)"
+    },
+    {
+      "name": "macos-universal",
+      "configurePreset": "macos-release-universal",
+      "displayName": "macOS Package (Universal)"
    }
  ],
  "workflowPresets": [
--- a/scripts/quality_check.sh
+++ b/scripts/quality_check.sh
@@ -0,0 +1,54 @@
+#!/bin/bash
+
+# Quality check script for YAZE codebase
+# This script runs various code quality checks to ensure CI/CD pipeline passes
+
+set -e
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+PROJECT_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)"
+
+cd "${PROJECT_ROOT}"
+
+echo "🔍 Running code quality checks for YAZE..."
+
+# Check if required tools are available
+command -v clang-format >/dev/null 2>&1 || { echo "❌ clang-format not found. Please install it."; exit 1; }
+command -v cppcheck >/dev/null 2>&1 || { echo "❌ cppcheck not found. Please install it."; exit 1; }
+
+# Create .clang-format config if it doesn't exist
+if [ ! -f .clang-format ]; then
+    echo "📝 Creating .clang-format configuration..."
+    clang-format --style=Google --dump-config > .clang-format
+fi
+
+echo "✅ Code formatting check..."
+# Check formatting without modifying files
+FORMATTING_ISSUES=$(find src test -name "*.cc" -o -name "*.h" | head -50 | xargs clang-format --dry-run --Werror --style=Google 2>&1 || true)
+if [ -n "$FORMATTING_ISSUES" ]; then
+    echo "⚠️  Formatting issues found. Run 'make format' to fix them."
+    echo "$FORMATTING_ISSUES" | head -20
+else
+    echo "✅ All files are properly formatted"
+fi
+
+echo "🔍 Running static analysis..."
+# Run cppcheck on main source directories
+cppcheck --enable=all --error-exitcode=0 \
+    --suppress=missingIncludeSystem \
+    --suppress=unusedFunction \
+    --suppress=unmatchedSuppression \
+    --suppress=unreadVariable \
+    --suppress=cstyleCast \
+    --suppress=variableScope \
+    src/ 2>&1 | head -30
+
+echo "✅ Quality checks completed!"
+echo ""
+echo "💡 To fix formatting issues automatically, run:"
+echo "   find src test -name '*.cc' -o -name '*.h' | xargs clang-format -i --style=Google"
+echo ""
+echo "💡 For CI/CD pipeline compatibility, ensure:"
+echo "   - All formatting issues are resolved"
+echo "   - absl::Status return values are handled with RETURN_IF_ERROR() or PRINT_IF_ERROR()"
+echo "   - Use Google C++ style for consistency"
--- a/src/app/test/test_manager.h
+++ b/src/app/test/test_manager.h
@@ -18,7 +18,7 @@ namespace yaze {
 namespace editor {
 class EditorManager;
 }
-}
+}  // namespace yaze

 #if defined(YAZE_ENABLE_IMGUI_TEST_ENGINE) && YAZE_ENABLE_IMGUI_TEST_ENGINE
 #include "imgui_test_engine/imgui_te_engine.h"
@@ -167,36 +167,43 @@ class TestManager {
  void DrawTestDashboard(bool* show_dashboard = nullptr);

  // ROM-dependent testing
-  void SetCurrentRom(Rom* rom) { 
+  void SetCurrentRom(Rom* rom) {
    util::logf("TestManager::SetCurrentRom called with ROM: %p", (void*)rom);
    if (rom) {
-      util::logf("ROM title: '%s', loaded: %s", rom->title().c_str(), rom->is_loaded() ? "true" : "false");
+      util::logf("ROM title: '%s', loaded: %s", rom->title().c_str(),
+                 rom->is_loaded() ? "true" : "false");
    }
-    current_rom_ = rom; 
+    current_rom_ = rom;
  }
  Rom* GetCurrentRom() const { return current_rom_; }
-  void RefreshCurrentRom(); // Refresh ROM pointer from editor manager
+  void RefreshCurrentRom();  // Refresh ROM pointer from editor manager
  // Remove EditorManager dependency to avoid circular includes
-  
+
  // Enhanced ROM testing
  absl::Status LoadRomForTesting(const std::string& filename);
  void ShowRomComparisonResults(const Rom& before, const Rom& after);
-  
+
  // Test ROM management
-  absl::Status CreateTestRomCopy(Rom* source_rom, std::unique_ptr<Rom>& test_rom);
+  absl::Status CreateTestRomCopy(Rom* source_rom,
+                                 std::unique_ptr<Rom>& test_rom);
  std::string GenerateTestRomFilename(const std::string& base_name);
  void OfferTestSessionCreation(const std::string& test_rom_path);
-  
+
 public:
  // ROM testing methods (work on copies, not originals)
  absl::Status TestRomSaveLoad(Rom* rom);
  absl::Status TestRomDataIntegrity(Rom* rom);
-  absl::Status TestRomWithCopy(Rom* source_rom, std::function<absl::Status(Rom*)> test_function);
-  
+  absl::Status TestRomWithCopy(Rom* source_rom,
+                               std::function<absl::Status(Rom*)> test_function);
+
  // Test configuration management
-  void DisableTest(const std::string& test_name) { disabled_tests_[test_name] = true; }
-  void EnableTest(const std::string& test_name) { disabled_tests_[test_name] = false; }
-  bool IsTestEnabled(const std::string& test_name) const { 
+  void DisableTest(const std::string& test_name) {
+    disabled_tests_[test_name] = true;
+  }
+  void EnableTest(const std::string& test_name) {
+    disabled_tests_[test_name] = false;
+  }
+  bool IsTestEnabled(const std::string& test_name) const {
    auto it = disabled_tests_.find(test_name);
    return it == disabled_tests_.end() || !it->second;
  }
@@ -245,7 +252,7 @@ class TestManager {
  // ROM-dependent testing
  Rom* current_rom_ = nullptr;
  // Removed editor_manager_ to avoid circular dependency
-  
+
  // UI state
  bool show_google_tests_ = false;
  bool show_rom_test_results_ = false;
@@ -253,7 +260,7 @@ class TestManager {
  bool show_test_session_dialog_ = false;
  bool show_test_configuration_ = false;
  std::string test_rom_path_for_session_;
-  
+
  // Test selection and configuration
  std::unordered_map<std::string, bool> disabled_tests_;
 };