Disallow using new in Setup of GTEST

CHANGELOG.md

@@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.
 
 #### CPP Rules
 
+- Add `noNewInGtestSetup` AST check flagging `new` expressions anywhere inside a member function named `SetUp`, `SetUpTestSuite`, or `SetUpTestCase` — the fixture-setup hooks GTest calls by name. A raw pointer allocated there needs a matching manual release whose timing depends on a separate teardown hook firing correctly later; prefer an RAII/smart-pointer owner instead. Only a literal `new` lexically inside the setup function's own body is flagged, not calls into other functions. Takes no configuration
 - Add `noDeleteInGtestTeardown` AST check flagging `delete`/`delete[]` expressions anywhere inside a member function named `TearDown`, `TearDownTestSuite`, or `TearDownTestCase` — the fixture-teardown hooks GTest calls by name. Manually deleting a raw pointer there is fragile (a `SetUp()` that throws or returns early skips the matching `delete`, and a test body that already freed the pointer causes a double-free); prefer an RAII/smart-pointer owner instead. Only a literal `delete`/`delete[]` lexically inside the teardown function's own body is flagged, not calls into other functions. Takes no configuration
 
 <!-- insertion marker -->

docs/source/sections/ast_checks/index.rst

@@ -97,6 +97,10 @@ Available checks
      - ``delete``/``delete[]`` inside a GTest ``TearDown``/``TearDownTestSuite``/
        ``TearDownTestCase`` function.
      - None
+   * - :doc:`noNewInGtestSetup <no_new_in_gtest_setup>`
+     - ``new`` inside a GTest ``SetUp``/``SetUpTestSuite``/``SetUpTestCase``
+       function.
+     - None
 
 .. toctree::
    :maxdepth: 1
@@ -112,3 +116,4 @@ Available checks
    no_global_using_enum
    no_throw_paren
    no_delete_in_gtest_teardown
+   no_new_in_gtest_setup

docs/source/sections/ast_checks/no_new_in_gtest_setup.rst

@@ -0,0 +1,48 @@
+noNewInGtestSetup
+====================
+
+Flags any ``new`` expression appearing anywhere inside a member function
+named ``SetUp``, ``SetUpTestSuite``, or ``SetUpTestCase`` — the
+fixture-setup hooks GTest calls by name. A raw pointer allocated there needs
+a matching manual release, and whether that release runs depends on a
+separate teardown hook firing later — see also
+:doc:`noDeleteInGtestTeardown <no_delete_in_gtest_teardown>`.
+
+.. code-block:: cpp
+
+   class FooTest : public ::testing::Test {
+   protected:
+       void SetUp() override {
+           ptr_ = new Foo();  // Bad — flagged.
+       }
+
+       Foo* ptr_;
+   };
+
+   class BarTest : public ::testing::Test {
+   protected:
+       void SetUp() override {
+           ptr_ = std::make_unique<Bar>();  // Good — RAII, not flagged.
+       }
+
+       std::unique_ptr<Bar> ptr_;
+   };
+
+Only a literal ``new`` expression lexically inside the setup function's own
+body is flagged — a call to some other function that itself allocates is
+not traced into. A declaration with no body (e.g. a pure-virtual
+``SetUp() = 0``) is never flagged, and neither is a free function of the
+same name that isn't a member function.
+
+Configuration
+--------------
+
+This check takes no configuration.
+
+Disabling
+----------
+
+.. code-block:: toml
+
+   [cpp]
+   ast_check_disabled_ids = ["noNewInGtestSetup"]

src/devops/cpp/ast/checks/no_new_in_gtest_setup.py

@@ -0,0 +1,111 @@
+"""Flag `new` expressions inside a GTest setup function.
+
+A member function named ``SetUp``, ``SetUpTestSuite``, or ``SetUpTestCase``
+— the fixture-setup hooks GTest calls by name (``SetUp()`` before every
+test, the other two once per test suite) — is reported for any ``new``
+expression appearing anywhere in its body. A raw pointer allocated there
+needs a matching manual release, and whether that release actually runs
+depends on a separate teardown hook firing later (see also
+:doc:`noDeleteInGtestTeardown </sections/ast_checks/no_delete_in_gtest_teardown>`)
+— if `SetUp()` itself throws partway through, or a later `SetUp()` call in
+the same fixture leaks the previous allocation, the memory is never freed.
+Prefer ``std::unique_ptr``/``std::shared_ptr`` (or another RAII owner) so
+the allocation's lifetime is tied to the fixture object itself rather than
+to a hook running at the right time.
+
+Only a literal ``new`` expression lexically inside the setup function's own
+body is flagged — a call to some other function that itself allocates is
+not traced into.
+
+To disable this check for a project set::
+
+    [cpp]
+    ast_check_disabled_ids = ["noNewInGtestSetup"]
+"""
+
+from __future__ import annotations
+
+import clang.cindex as clang
+
+from devops.cpp.ast.base import Check, Diagnostic
+
+# GTest calls these fixture methods by name; no base-class relationship is
+# required by the framework itself, so none is checked here either.
+_SETUP_NAMES = frozenset(
+    (
+        "SetUp",
+        "SetUpTestSuite",
+        "SetUpTestCase",
+    )
+)
+
+
+class NoNewInGtestSetup(Check):
+    """Flag `new` expressions inside a GTest setup function."""
+
+    id = "noNewInGtestSetup"
+
+    def visit(self, cursor: clang.Cursor, filename: str) -> list[Diagnostic]:
+        """Flag any new-expression inside a GTest setup function body.
+
+        Parameters
+        ----------
+        cursor: clang.Cursor
+            The AST node currently being visited.
+        filename: str
+            Path of the file being checked.
+
+        Returns
+        -------
+        list[Diagnostic]
+            One diagnostic per `new` expression found inside `cursor`'s
+            body, if `cursor` is a GTest setup function; otherwise an empty
+            list.
+
+        """
+        if cursor.kind != clang.CursorKind.CXX_METHOD:
+            return []
+        if cursor.spelling not in _SETUP_NAMES:
+            return []
+        if not cursor.is_definition():
+            return []
+
+        return [
+            self._make_diagnostic(new_expr, cursor.spelling, filename)
+            for new_expr in cursor.walk_preorder()
+            if new_expr.kind == clang.CursorKind.CXX_NEW_EXPR
+        ]
+
+    def _make_diagnostic(
+        self, new_expr: clang.Cursor, function_name: str, filename: str
+    ) -> Diagnostic:
+        """Build the diagnostic for one flagged new-expression.
+
+        Parameters
+        ----------
+        new_expr: clang.Cursor
+            The `CXX_NEW_EXPR` cursor to report.
+        function_name: str
+            Name of the enclosing setup function, for the message.
+        filename: str
+            Path of the file being checked.
+
+        Returns
+        -------
+        Diagnostic
+            The diagnostic for `new_expr`.
+
+        """
+        loc = new_expr.location
+        return Diagnostic(
+            file=filename,
+            line=loc.line,
+            column=loc.column,
+            message=(
+                f"do not use 'new' inside GTest setup function "
+                f"'{function_name}()' — prefer an RAII/smart-pointer owner "
+                "whose lifetime doesn't depend on a later teardown hook"
+            ),
+            check_id=self.id,
+            severity="style",
+        )

src/devops/cpp/ast/registry.py

@@ -24,6 +24,7 @@
 from devops.cpp.ast.checks.no_delete_in_gtest_teardown import NoDeleteInGtestTeardown
 from devops.cpp.ast.checks.no_global_using import NoGlobalUsing
 from devops.cpp.ast.checks.no_global_using_enum import NoGlobalUsingEnum
+from devops.cpp.ast.checks.no_new_in_gtest_setup import NoNewInGtestSetup
 from devops.cpp.ast.checks.no_throw_paren import NoThrowParen
 from devops.logger import cpp_check_logger
 
@@ -44,6 +45,7 @@
     NoDeleteInGtestTeardown(),
     NoGlobalUsing(),
     NoGlobalUsingEnum(),
+    NoNewInGtestSetup(),
     NoThrowParen(),
 ]
 

tests/cpp/ast/test_no_new_in_gtest_setup.py

@@ -0,0 +1,139 @@
+"""Tests for the NoNewInGtestSetup AST check."""
+
+from __future__ import annotations
+
+import typing
+
+import pytest
+
+from devops.cpp.ast.checks.no_new_in_gtest_setup import NoNewInGtestSetup
+from devops.cpp.ast.engine import run_ast_checks
+
+if typing.TYPE_CHECKING:
+    from pathlib import Path
+
+pytest.importorskip("clang.cindex")
+
+_CHECK = [NoNewInGtestSetup()]
+_ARGS = ["-std=c++17"]
+
+
+def _diags(code: str, tmp_path: Path) -> list[str]:
+    p = tmp_path / "test.cpp"
+    p.write_text(code)
+    return [d.message for d in run_ast_checks(p, code, _ARGS, checks=_CHECK)]
+
+
+class TestNewInSetupFlagged:
+    """`new` in a recognised setup hook is flagged."""
+
+    def test_new_in_setup_flagged(self, tmp_path: Path) -> None:
+        """Test new in SetUp flagged."""
+        code = "struct FooTest {\n  void SetUp() {\n    int* p = new int(1);\n  }\n};\n"
+        diags = _diags(code, tmp_path)
+        assert len(diags) == 1
+        assert "'new'" in diags[0]
+        assert "SetUp()" in diags[0]
+
+    def test_new_array_in_setup_flagged(self, tmp_path: Path) -> None:
+        """Test array new in SetUp flagged."""
+        code = "struct FooTest {\n  void SetUp() {\n    int* p = new int[3];\n  }\n};\n"
+        diags = _diags(code, tmp_path)
+        assert len(diags) == 1
+
+    def test_new_in_setup_test_suite_flagged(self, tmp_path: Path) -> None:
+        """Test new in SetUpTestSuite flagged."""
+        code = (
+            "struct FooTest {\n"
+            "  static void SetUpTestSuite() {\n"
+            "    shared_ = new int(1);\n"
+            "  }\n"
+            "  static int* shared_;\n"
+            "};\n"
+        )
+        diags = _diags(code, tmp_path)
+        assert len(diags) == 1
+        assert "SetUpTestSuite()" in diags[0]
+
+    def test_new_in_setup_test_case_flagged(self, tmp_path: Path) -> None:
+        """Test new in SetUpTestCase flagged."""
+        code = (
+            "struct FooTest {\n"
+            "  static void SetUpTestCase() {\n"
+            "    shared_ = new int(1);\n"
+            "  }\n"
+            "  static int* shared_;\n"
+            "};\n"
+        )
+        diags = _diags(code, tmp_path)
+        assert len(diags) == 1
+        assert "SetUpTestCase()" in diags[0]
+
+    def test_multiple_news_all_flagged(self, tmp_path: Path) -> None:
+        """Test multiple news all flagged."""
+        code = (
+            "struct FooTest {\n"
+            "  void SetUp() {\n"
+            "    int* a = new int(1);\n"
+            "    int* b = new int(2);\n"
+            "  }\n"
+            "};\n"
+        )
+        assert len(_diags(code, tmp_path)) == 2
+
+    def test_new_in_nested_lambda_flagged(self, tmp_path: Path) -> None:
+        """Test new inside a lambda defined in SetUp is still flagged."""
+        code = (
+            "struct FooTest {\n"
+            "  void SetUp() {\n"
+            "    auto f = []() { return new int(1); };\n"
+            "    f();\n"
+            "  }\n"
+            "};\n"
+        )
+        assert len(_diags(code, tmp_path)) == 1
+
+
+class TestNewElsewhereAllowed:
+    """`new` outside a recognised setup hook is not flagged."""
+
+    def test_new_in_tear_down_allowed(self, tmp_path: Path) -> None:
+        """Test new in TearDown allowed."""
+        code = (
+            "struct FooTest {\n  void TearDown() {\n    int* p = new int(1);\n  }\n};\n"
+        )
+        assert _diags(code, tmp_path) == []
+
+    def test_new_in_unrelated_method_allowed(self, tmp_path: Path) -> None:
+        """Test new in an unrelated method allowed."""
+        code = "struct S {\n  void Init() {\n    int* p = new int(1);\n  }\n};\n"
+        assert _diags(code, tmp_path) == []
+
+    def test_new_in_free_function_named_setup_allowed(self, tmp_path: Path) -> None:
+        """Test new in a free function (not a method) named SetUp allowed."""
+        code = "void SetUp() {\n  int* p = new int(1);\n}\n"
+        assert _diags(code, tmp_path) == []
+
+    def test_setup_declaration_without_body_allowed(self, tmp_path: Path) -> None:
+        """Test a SetUp declaration with no body is not flagged."""
+        code = "struct Base {\n  virtual void SetUp() = 0;\n};\n"
+        assert _diags(code, tmp_path) == []
+
+    def test_clean_setup_produces_no_diagnostics(self, tmp_path: Path) -> None:
+        """Test clean file produces no diagnostics."""
+        code = (
+            "struct FooTest {\n"
+            "  void SetUp() {\n"
+            "    ptr_ = std::make_unique<int>(1);\n"
+            "  }\n"
+            "};\n"
+        )
+        assert _diags(code, tmp_path) == []
+
+
+class TestNoNewInGtestSetupCheckId:
+    """The check id is correct and the check is selectable."""
+
+    def test_check_id(self) -> None:
+        """Test check id."""
+        assert NoNewInGtestSetup().id == "noNewInGtestSetup"