Fix SonarCloud violations

Python fixes:
- Fix _argparse.py: Change return type of _load_from_toml from None to
  argparse.Namespace to match actual return value
- Fix _cmake.py: Reduce cognitive complexity by extracting helper methods
  (_process_keyword_cmake_args, _process_flag_cmake_args,
  _process_platform_cmake_args)
- Fix _cmake_test.py: Reduce cognitive complexity by extracting test helpers
  and rename InterProcessReaderWriterLock variables to follow naming conventions

GitHub Actions security fixes:
- Use environment variables instead of direct interpolation for user-controlled
  data in run commands (pull_request.yml, format.yml, publish_release.yml)
- Add input validation for workflow_dispatch tag parameter (draft_release.yml)

Shell script fixes (run_tests.sh):
- Replace [ ] with [[ ]] for safer conditional tests
- Add explicit return statements at end of functions
- Merge nested if conditions where applicable

Author: claude

.github/workflows/draft_release.yml

@@ -13,7 +13,16 @@ jobs:
   new-release:
     name: "Draft a new release"
     runs-on: ubuntu-latest
+    env:
+      RELEASE_TAG: ${{ github.event.inputs.tag }}
     steps:
+      - name: Validate release tag format
+        run: |
+          if ! echo "$RELEASE_TAG" | grep -qE '^v[0-9]+\.[0-9]+\.[0-9]+$'; then
+            echo "Error: Tag must be in format vXX.YY.ZZ"
+            exit 1
+          fi
+
       - uses: actions/checkout@v4
 
       - name: Setup Python
@@ -26,7 +35,7 @@ jobs:
       - name: Update CHANGELOG
         run: |
           python3 -m pip install mdformat-gfm 'git+https://github.com/Takishima/keepachangelog@v1.0.1'
-          python3 -m keepachangelog release "${{ github.event.inputs.tag }}"
+          python3 -m keepachangelog release "$RELEASE_TAG"
           python3 -m mdformat CHANGELOG.md
 
       - name: Commit changes

.github/workflows/format.yml

@@ -28,16 +28,18 @@ jobs:
           curl -LsSf "${source_url}/${parse_changelog_tag}/parse-changelog-${target}.tar.gz" | tar xzf -
       - name: Extract version from branch name (for release branches)
         if: github.event_name == 'pull_request' && startsWith(github.event.pull_request.head.ref, 'release/')
+        env:
+          BRANCH_NAME: ${{ github.event.pull_request.head.ref }}
         run: |
-          BRANCH_NAME="${{ github.event.pull_request.head.ref }}"
           VERSION=${BRANCH_NAME#release/v}
           echo "RELEASE_VERSION=$VERSION" >> $GITHUB_ENV
           git tag v${RELEASE_VERSION}
 
       - name: Extract version from branch name (for hotfix branches)
         if: github.event_name == 'pull_request'  && startsWith(github.event.pull_request.head.ref, 'hotfix/')
+        env:
+          BRANCH_NAME: ${{ github.event.pull_request.head.ref }}
         run: |
-          BRANCH_NAME="${{ github.event.pull_request.head.ref }}"
           VERSION=${BRANCH_NAME#hotfix/v}
           echo "RELEASE_VERSION=$VERSION" >> $GITHUB_ENV
           git tag v${RELEASE_VERSION}

.github/workflows/publish_release.yml

@@ -39,16 +39,18 @@ jobs:
 
       - name: Extract version from branch name (for release branches)
         if: github.event_name == 'pull_request' && startsWith(github.event.pull_request.head.ref, 'release/')
+        env:
+          BRANCH_NAME: ${{ github.event.pull_request.head.ref }}
         run: |
-          BRANCH_NAME="${{ github.event.pull_request.head.ref }}"
           VERSION=${BRANCH_NAME#release/}
           VERSION=${VERSION#v}
           echo "RELEASE_VERSION=$VERSION" >> $GITHUB_ENV
 
       - name: Extract version from branch name (for hotfix branches)
         if: github.event_name == 'pull_request'  && startsWith(github.event.pull_request.head.ref, 'hotfix/')
+        env:
+          BRANCH_NAME: ${{ github.event.pull_request.head.ref }}
         run: |
-          BRANCH_NAME="${{ github.event.pull_request.head.ref }}"
           VERSION=${BRANCH_NAME#hotfix/}
           VERSION=${VERSION#v}
           echo "RELEASE_VERSION=$VERSION" >> $GITHUB_ENV

.github/workflows/pull_request.yml

@@ -31,8 +31,10 @@ jobs:
           python3 -m pip install 'git+https://github.com/Takishima/pre-commit-changelog-auto-update@v1.0.0'
 
       - name: Run Python script
+        env:
+          PR_BODY: ${{ github.event.pull_request.body }}
         run: |
-          python3 -m update_changelog --pr-body "${{ github.event.pull_request.body }}"
+          python3 -m update_changelog --pr-body "$PR_BODY"
 
       - name: Commit changes
         id: commit

cmake_pc_hooks/_argparse.py

@@ -293,7 +293,7 @@ def _load_from_toml(  # noqa: PLR0913
         path_must_exist: bool = True,
         section_must_exist: bool = True,
         overridable_keys: set | None = None,
-    ) -> None:
+    ) -> argparse.Namespace:
         """
         Load a TOML file and set the attributes within the argparse namespace object.
 

cmake_pc_hooks/_cmake.py

@@ -298,40 +298,8 @@ def resolve_build_directory(self, build_dir_list=None, *, automatic_discovery=Tr
             self.build_dir,
         )
 
-    def setup_cmake_args(self, cmake_args):  # noqa: C901
-        """
-        Setup CMake arguments.
-
-        Args:
-            cmake_args: Dictionary-like data structure with following keys:
-                - 'defines': list[str]
-                - 'undefined': list[str]
-                - 'errors': list[str]
-                - 'no_errors': list[str]
-                - 'generator': str
-                - 'toolset': str
-                - 'platform': str
-                - 'preset': str
-                - 'cmake': str
-                - 'dev_warnings': bool
-                - 'no_dev_warnings': bool
-                - 'linux': list[str]
-                - 'mac': list[str]
-                - 'win': list[str]
-        """
-        self.source_dir = Path(cmake_args.source_dir).resolve()
-        if cmake_args.cmake:
-            self.command = [Path(cmake_args.cmake).resolve()]
-        self.no_cmake_configure = cmake_args.no_cmake_configure
-
-        self.resolve_build_directory(
-            build_dir_list=cmake_args.build_dir,
-            automatic_discovery=cmake_args.automatic_discovery,
-        )
-
-        if cmake_args.detect_configured_files and self.build_dir:
-            self.cmake_trace_log = self.build_dir / self.DEFAULT_TRACE_LOG
-
+    def _process_keyword_cmake_args(self, cmake_args):
+        """Process keyword-style CMake arguments."""
         keyword_args = {
             'defines': ([], '-D{}'),
             'undefines': ([], '-U{}'),
@@ -352,6 +320,8 @@ def setup_cmake_args(self, cmake_args):  # noqa: C901
                     for element in value:
                         self.cmake_args.append(format_str.format(element))
 
+    def _process_flag_cmake_args(self, cmake_args):
+        """Process flag-style CMake arguments."""
         flag_args = {
             'dev_warnings': (False, '-Wdev'),
             'no_dev_warnings': (False, '-Wno_dev'),
@@ -360,18 +330,58 @@ def setup_cmake_args(self, cmake_args):  # noqa: C901
             if getattr(cmake_args, key, default):
                 self.cmake_args.append(flag_str)
 
-        platform_args = {
+    def _process_platform_cmake_args(self, cmake_args):
+        """Process platform-specific CMake arguments."""
+        platform_args_config = {
             'linux': ([], 'Linux'),
             'mac': ([], 'Darwin'),
             'win': ([], 'Windows'),
         }
 
-        for key, (default, platform_name) in platform_args.items():
-            platform_args = getattr(cmake_args, key, default)
-            if platform.system() == platform_name and platform_args:
-                for arg in platform_args:
+        for key, (default, platform_name) in platform_args_config.items():
+            platform_arg_values = getattr(cmake_args, key, default)
+            if platform.system() == platform_name and platform_arg_values:
+                for arg in platform_arg_values:
                     self.cmake_args.append(arg.strip('"\''))
 
+    def setup_cmake_args(self, cmake_args):
+        """
+        Setup CMake arguments.
+
+        Args:
+            cmake_args: Dictionary-like data structure with following keys:
+                - 'defines': list[str]
+                - 'undefined': list[str]
+                - 'errors': list[str]
+                - 'no_errors': list[str]
+                - 'generator': str
+                - 'toolset': str
+                - 'platform': str
+                - 'preset': str
+                - 'cmake': str
+                - 'dev_warnings': bool
+                - 'no_dev_warnings': bool
+                - 'linux': list[str]
+                - 'mac': list[str]
+                - 'win': list[str]
+        """
+        self.source_dir = Path(cmake_args.source_dir).resolve()
+        if cmake_args.cmake:
+            self.command = [Path(cmake_args.cmake).resolve()]
+        self.no_cmake_configure = cmake_args.no_cmake_configure
+
+        self.resolve_build_directory(
+            build_dir_list=cmake_args.build_dir,
+            automatic_discovery=cmake_args.automatic_discovery,
+        )
+
+        if cmake_args.detect_configured_files and self.build_dir:
+            self.cmake_trace_log = self.build_dir / self.DEFAULT_TRACE_LOG
+
+        self._process_keyword_cmake_args(cmake_args)
+        self._process_flag_cmake_args(cmake_args)
+        self._process_platform_cmake_args(cmake_args)
+
     def configure(self, command, *, clean_build=False):
         """
         Run a CMake configure step (multi-process safe).

tests/python/_cmake_test.py

@@ -164,20 +164,27 @@ def test_resolve_build_directory(tmp_path, dir_list, build_dir_tree, ref_path):
     assert cmake.build_dir == tmp_path / cmake.DEFAULT_BUILD_DIR if dir_list is None else dir_list[0]
 
 
-@pytest.mark.parametrize('system', ['Linux', 'Darwin', 'Windows'])
-@pytest.mark.parametrize('no_cmake_configure', [False, True])
-def test_setup_cmake_args(  # noqa: PLR0912, PLR0915, C901
-    mocker, system, no_cmake_configure
-):
-    original_system = platform.system()
-
-    def system_stub():
-        return system
-
-    mocker.patch('platform.system', system_stub)
-
-    cmake = CMakeCommand()
-
+def _assert_args_in_cmake_args(cmake_args, items, format_str='{}'):
+    """Helper to verify items are present in cmake_args."""
+    for item in items:
+        expected = format_str.format(item)
+        assert any(expected in arg for arg in cmake_args), f'Expected {expected} in cmake_args'
+
+
+def _assert_platform_args(cmake_args, args):
+    """Helper to verify platform-specific args."""
+    platform_config = {
+        'Linux': args.linux,
+        'Darwin': args.mac,
+        'Windows': args.win,
+    }
+    current_platform = platform.system()
+    if current_platform in platform_config:
+        _assert_args_in_cmake_args(cmake_args, platform_config[current_platform])
+
+
+def _create_test_args(original_system, no_cmake_configure):
+    """Create test arguments namespace."""
     args = argparse.Namespace()
     args.detect_configured_files = True
     args.no_cmake_configure = no_cmake_configure
@@ -202,6 +209,17 @@ def system_stub():
     args.linux = ['LNX_A', 'LNX_B']
     args.mac = ['MAC_A', 'MAC_B']
     args.win = ['WIN_A', 'WIN_B']
+    return args
+
+
+@pytest.mark.parametrize('system', ['Linux', 'Darwin', 'Windows'])
+@pytest.mark.parametrize('no_cmake_configure', [False, True])
+def test_setup_cmake_args(mocker, system, no_cmake_configure):
+    original_system = platform.system()
+    mocker.patch('platform.system', lambda: system)
+
+    cmake = CMakeCommand()
+    args = _create_test_args(original_system, no_cmake_configure)
 
     assert cmake.source_dir is None
     assert cmake.build_dir is None
@@ -220,32 +238,18 @@ def system_stub():
     assert '-GNinja' in cmake.cmake_args
     assert '-A64' in cmake.cmake_args
     assert '-Tclang-toolset' in cmake.cmake_args
-
     assert '-Wdev' in cmake.cmake_args
     assert '-Wno_dev' in cmake.cmake_args
 
     # The arguments are not added to cmake.cmake_args since we only want to add them during a CMake configure call
     assert '--trace-expand' not in cmake.cmake_args
     assert '--trace-format=json-v1' not in cmake.cmake_args
 
-    for define in args.defines:
-        assert any(define in arg for arg in cmake.cmake_args)
-    for undefine in args.undefines:
-        assert any(undefine in arg for arg in cmake.cmake_args)
-    for error in args.errors:
-        assert any(f'-Werror={error}' in arg for arg in cmake.cmake_args)
-    for no_error in args.no_errors:
-        assert any(f'-Wno-error={no_error}' in arg for arg in cmake.cmake_args)
-
-    if platform.system() == 'Linux':
-        for linux in args.linux:
-            assert any(linux in arg for arg in cmake.cmake_args)
-    elif platform.system() == 'Darwin':
-        for mac in args.mac:
-            assert any(mac in arg for arg in cmake.cmake_args)
-    elif platform.system() == 'Windows':
-        for win in args.win:
-            assert any(win in arg for arg in cmake.cmake_args)
+    _assert_args_in_cmake_args(cmake.cmake_args, args.defines)
+    _assert_args_in_cmake_args(cmake.cmake_args, args.undefines)
+    _assert_args_in_cmake_args(cmake.cmake_args, args.errors, '-Werror={}')
+    _assert_args_in_cmake_args(cmake.cmake_args, args.no_errors, '-Wno-error={}')
+    _assert_platform_args(cmake.cmake_args, args)
 
 
 @pytest.mark.parametrize('detect_configured_files', [False, True], ids=['no_trace', 'w_trace'])
@@ -261,12 +265,10 @@ def test_configure_cmake(  # noqa: PLR0917
     detect_configured_files,
 ):
     sys_exit = mocker.patch('sys.exit')
-    FileLock = mocker.MagicMock(filelock.FileLock)  # noqa: N806
-    mocker.patch(filelock_module_name, FileLock)
-    InterProcessReaderWriterLock = mocker.MagicMock(  # noqa: N806
-        fasteners.InterProcessReaderWriterLock
-    )
-    mocker.patch(interprocess_rw_lock_module_name, InterProcessReaderWriterLock)
+    mock_file_lock = mocker.MagicMock(filelock.FileLock)
+    mocker.patch(filelock_module_name, mock_file_lock)
+    mock_rw_lock = mocker.MagicMock(fasteners.InterProcessReaderWriterLock)
+    mocker.patch(interprocess_rw_lock_module_name, mock_rw_lock)
     configure = mocker.Mock(return_value=returncode)
     mocker.patch(internal_cmake_configure_name, configure)
     parse_log = mocker.patch('cmake_pc_hooks._cmake.CMakeCommand._parse_cmake_trace_log')
@@ -289,17 +291,17 @@ def test_configure_cmake(  # noqa: PLR0917
     # ----------------------------------
 
     if no_cmake_configure:
-        FileLock.assert_not_called()
-        InterProcessReaderWriterLock.assert_not_called()
+        mock_file_lock.assert_not_called()
+        mock_rw_lock.assert_not_called()
         configure.assert_not_called()
         return
 
-    FileLock.assert_called_once_with(build_dir / '_cmake_configure_try_lock')
-    InterProcessReaderWriterLock.assert_called_once_with(build_dir / '_cmake_configure_lock')
+    mock_file_lock.assert_called_once_with(build_dir / '_cmake_configure_try_lock')
+    mock_rw_lock.assert_called_once_with(build_dir / '_cmake_configure_lock')
     configure.assert_called_once_with(
         lock_files=(
-            InterProcessReaderWriterLock.call_args[0][0],
-            FileLock.call_args[0][0],
+            mock_rw_lock.call_args[0][0],
+            mock_file_lock.call_args[0][0],
         ),
         clean_build=clean_build,
     )
@@ -324,14 +326,12 @@ def timeout(blocking):  # noqa: ARG001
 
     args = {'acquire.side_effect': timeout}
     file_lock = mocker.MagicMock(filelock.FileLock, **args)
-    FileLock = mocker.MagicMock(filelock.FileLock, return_value=file_lock)  # noqa: N806
-    mocker.patch(filelock_module_name, FileLock)
+    mock_file_lock = mocker.MagicMock(filelock.FileLock, return_value=file_lock)
+    mocker.patch(filelock_module_name, mock_file_lock)
 
     interprocess_lock = mocker.MagicMock()
-    InterProcessReaderWriterLock = mocker.MagicMock(  # noqa: N806
-        return_value=interprocess_lock
-    )
-    mocker.patch(interprocess_rw_lock_module_name, InterProcessReaderWriterLock)
+    mock_rw_lock = mocker.MagicMock(return_value=interprocess_lock)
+    mocker.patch(interprocess_rw_lock_module_name, mock_rw_lock)
     configure = mocker.Mock(return_value=0)
     mocker.patch(internal_cmake_configure_name, configure)
 
@@ -349,21 +349,19 @@ def timeout(blocking):  # noqa: ARG001
 
     # ----------------------------------
 
-    FileLock.assert_called_once()
-    InterProcessReaderWriterLock.assert_called_once()
+    mock_file_lock.assert_called_once()
+    mock_rw_lock.assert_called_once()
     interprocess_lock.read_lock.assert_called_once()
     configure.assert_not_called()
 
 
 def test_configure_invalid(mocker):
     mocker.patch('sys.exit', side_effect=ExitError)
 
-    FileLock = mocker.MagicMock(filelock.FileLock)  # noqa: N806
-    mocker.patch(filelock_module_name, FileLock)
-    InterProcessReaderWriterLock = mocker.MagicMock(  # noqa: N806
-        fasteners.InterProcessReaderWriterLock
-    )
-    mocker.patch(interprocess_rw_lock_module_name, InterProcessReaderWriterLock)
+    mock_file_lock = mocker.MagicMock(filelock.FileLock)
+    mocker.patch(filelock_module_name, mock_file_lock)
+    mock_rw_lock = mocker.MagicMock(fasteners.InterProcessReaderWriterLock)
+    mocker.patch(interprocess_rw_lock_module_name, mock_rw_lock)
     configure = mocker.Mock(return_value=1)
     mocker.patch(internal_cmake_configure_name, configure)