#57: Define and enforce v2 C++ coding style

doc/changes/unreleased.md

@@ -18,6 +18,7 @@ n/a
 
 ## Internal
 
+* #57: Defined and enforced public v2 C++ coding style
 * #51: Added agent and contributor guidance for v1/v2 development, SLC workflows, CI testing, and PR conventions
 * #56: Restructured the developer guide and synchronized agent guidance
 * Updated Poetry dependencies and added developer guide and added .gitignore

doc/developer_guide/v2/v2.md

@@ -4,6 +4,7 @@ The v2 developer documentation is split into focused guides:
 
 - [Build and test](v2_build_and_test.md) — Bazel setup and test execution.
 - [Code quality](v2_code_quality.md) — clang-tidy and clang-format checks.
+- [Coding style](v2_coding_style.md) — public C++ conventions for v2 code.
 - [Fuzzing](v2_fuzzing.md) — libFuzzer targets, Nox campaigns, and regression
   runs.
 - [Dependency policy](v2_dependency_policy.md) — third-party isolation and

doc/developer_guide/v2/v2_code_quality.md

@@ -13,6 +13,23 @@ bazel build --verbose_failures --config clang-tidy //...
 Run clang-tidy on changed `.cpp` files before submitting code for review to
 catch common issues early.
 
+The Bazel configuration uses `clang-tidy-22` by default. To use another
+installed executable, set `CLANG_TIDY` when invoking Bazel:
+
+```bash
+CLANG_TIDY=clang-tidy bazel build --verbose_failures --config clang-tidy //...
+```
+
+`CLANG_TIDY` may also contain an absolute path to the executable.
+
+The wrapper removes `-fno-canonical-system-headers` from the compiler
+arguments by default. To retain that argument, clear `CLANG_TIDY_REMOVED_ARG`:
+
+```bash
+CLANG_TIDY=clang-tidy CLANG_TIDY_REMOVED_ARG= \
+  bazel build --verbose_failures --config clang-tidy //...
+```
+
 ### Apply clang-tidy fixes
 
 You can run `clang-apply-replacements` with:

doc/developer_guide/v2/v2_coding_style.md

@@ -0,0 +1,94 @@
+# v2 C++ Coding Style
+
+This guide defines the conventions for C++ code under
+[`udf-runner-cpp/v2`](../../../udf-runner-cpp/v2). The checked-in
+[`clang-format` configuration](../../../udf-runner-cpp/v2/tools/clang-format/.clang-format)
+and [`clang-tidy` configuration](../../../udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy)
+are authoritative for automatically checked rules.
+
+## Files and includes
+
+- Use UTF-8 source files.
+- Use `.cc` for implementation files and `.h` or `.hpp` for headers, matching
+  the convention already used by the v2 module.
+- Keep `#include` directives at the top of the file. Do not include headers
+  inside functions unless there is a documented, compelling reason.
+- Include every header required by a file directly; do not rely on transitive
+  includes.
+- Put non-template function definitions in implementation files unless there
+  is a measured performance reason to keep them inline.
+- Keep public headers independent of private implementation details and avoid
+  conditional compilation in headers unless it is required by the public API.
+
+## Names and namespaces
+
+- Use ASCII identifiers and `lower_case` for functions, variables, parameters,
+  and data members.
+- Use `CamelCase` for classes and enum types. Use `CamelCase` for scoped enum
+  values as well.
+- Name factory functions with a `create` prefix and getters/setters with
+  `get_`/`set_` prefixes, for example `get_value()` and `set_value()`.
+- Put file-local functions and types in an unnamed namespace.
+- Put project code in an appropriate `exasol::udf::v2` namespace rather than
+  importing a namespace with `using namespace`.
+- Keep namespace aliases local and descriptive when they improve readability.
+
+The naming policy is enforced by clang-tidy’s
+`readability-identifier-naming` check. Class members have no naming prefix or
+suffix. When a member access would otherwise be ambiguous, qualify it with
+`this->`, for example `this->value`.
+
+Names required by an external ABI or framework are exceptions. For example,
+the libFuzzer entry point `LLVMFuzzerTestOneInput` keeps its required spelling.
+
+## Functions and classes
+
+- Prefer free functions for behavior that does not depend on object state.
+- Avoid operator overloading unless the type has a clear value-like meaning
+  and the overload is required for natural use of the public API.
+- Mark a class `final` when it is not designed for inheritance.
+- Mark overriding methods with `override`.
+- Keep class declarations ordered, where practical, as public, protected, then
+  private; within each section, place types before methods and data members.
+- Avoid ambiguity between constructor parameters and members. Prefer the same
+  descriptive name and qualify member access with `this->`.
+- Separate function definitions with a blank line.
+
+## Types, control flow, and errors
+
+- Prefer fixed-width integer types such as `std::int32_t` and `std::uint64_t`
+  when the width is part of the interface or serialized representation.
+- Use `enum class` for new enumerations.
+- For `std::optional`, use `has_value()` when testing presence and `value()`
+  when explicitly retrieving the contained value. Name the variable after its
+  value, not after the fact that it is optional.
+- Follow the repository formatter for braces and indentation. Keep all code
+  belonging to a `case`, including its terminating `break`, `return`, or
+  fallthrough marker, inside the case body when braces are needed.
+- Prefer safe, expressive casts. If a lower-level cast is required for a
+  measured hot path or ABI boundary, document why it is safe.
+- Report failures caused by external input or environment through the public
+  error mechanism, normally an exception. Use assertions for programmer
+  contract violations and impossible internal states.
+
+## Documentation and cleanup
+
+- Document design decisions close to the code they constrain.
+- Put API documentation in public headers and implementation details near the
+  implementation.
+- Use Doxygen commands with `@`. Prefer `@returns`, `@throws`, and `@see`.
+  Omit `@brief` when the first sentence already provides the brief.
+- Remove commented-out code and avoid `#if 0` or `#if 1` except when a clear,
+  documented temporary or compatibility purpose requires it.
+- Keep comments factual and explain why non-obvious code exists, not what an
+  immediately readable statement does.
+
+## Tests and review
+
+- Add or update tests when changing behavior, public interfaces, parsing,
+  serialization, concurrency, or dependency boundaries.
+- Prefer small, focused tests that make failures easy to diagnose.
+- Run the v2 build and tests, then the `clang-format` and `clang-tidy` checks
+  described in the [code quality guide](v2_code_quality.md).
+- Do not suppress a static-analysis warning without documenting the reason at
+  the suppression site.

udf-runner-cpp/v2/.bazelrc

@@ -31,6 +31,8 @@ build:asan-replay --@rules_fuzzing//fuzzing:cc_engine_sanitizer=asan
 
 build:clang-tidy --@rules_clang_tidy//:config=//tools/clang-tidy:config
 build:clang-tidy --@rules_clang_tidy//:clang-tidy=//tools/clang-tidy:wrapper
+build:clang-tidy --action_env=CLANG_TIDY
+build:clang-tidy --action_env=CLANG_TIDY_REMOVED_ARG
 build:clang-tidy --aspects=@rules_clang_tidy//:aspects.bzl%check
 build:clang-tidy --output_groups=report
 build:clang-tidy --remote_download_outputs=toplevel

udf-runner-cpp/v2/arrow_c_data_demo.cc

@@ -18,12 +18,12 @@ namespace
 
 thread_local std::string g_last_error;
 
-void SetLastError(const arrow::Status& status)
+void set_last_error(const arrow::Status& status)
 {
     g_last_error = status.ToString();
 }
 
-arrow::Result<std::shared_ptr<arrow::RecordBatch>> MakeDemoRecordBatch()
+arrow::Result<std::shared_ptr<arrow::RecordBatch>> make_demo_record_batch()
 {
     arrow::Int64Builder id_builder;
     arrow::StringBuilder name_builder;
@@ -51,7 +51,7 @@ arrow::Result<std::shared_ptr<arrow::RecordBatch>> MakeDemoRecordBatch()
     return arrow::RecordBatch::Make(schema, num_rows, {std::move(ids), std::move(names)});
 }
 
-arrow::Status ExportDemoRecordBatch(ArrowArray* out_array, ArrowSchema* out_schema)
+arrow::Status export_demo_record_batch(ArrowArray* out_array, ArrowSchema* out_schema)
 {
     if (out_array == nullptr || out_schema == nullptr)
     {
@@ -62,7 +62,7 @@ arrow::Status ExportDemoRecordBatch(ArrowArray* out_array, ArrowSchema* out_sche
     std::memset(out_array, 0, sizeof(*out_array));
     std::memset(out_schema, 0, sizeof(*out_schema));
 
-    auto maybe_batch = MakeDemoRecordBatch();
+    auto maybe_batch = make_demo_record_batch();
     if (!maybe_batch.ok())
     {
         return maybe_batch.status();
@@ -71,10 +71,10 @@ arrow::Status ExportDemoRecordBatch(ArrowArray* out_array, ArrowSchema* out_sche
     return arrow::Status::OK();
 }
 
-arrow::Status ConsumeDemoRecordBatch(ArrowArray* array,
-                                     ArrowSchema* schema,
-                                     int64_t* out_row_count,
-                                     int64_t* out_id_sum)
+arrow::Status consume_demo_record_batch(ArrowArray* array,
+                                        ArrowSchema* schema,
+                                        int64_t* out_row_count,
+                                        int64_t* out_id_sum)
 {
     if (array == nullptr || schema == nullptr || out_row_count == nullptr || out_id_sum == nullptr)
     {
@@ -111,10 +111,10 @@ arrow::Status ConsumeDemoRecordBatch(ArrowArray* array,
 extern "C" UDF_RUNNER_CPP_V2_EXPORT int udf_runner_cpp_v2_demo_export_record_batch(
     ArrowArray* out_array, ArrowSchema* out_schema)
 {
-    const arrow::Status status = ExportDemoRecordBatch(out_array, out_schema);
+    const arrow::Status status = export_demo_record_batch(out_array, out_schema);
     if (!status.ok())
     {
-        SetLastError(status);
+        set_last_error(status);
         return 1;
     }
     g_last_error.clear();
@@ -124,10 +124,11 @@ extern "C" UDF_RUNNER_CPP_V2_EXPORT int udf_runner_cpp_v2_demo_export_record_bat
 extern "C" UDF_RUNNER_CPP_V2_EXPORT int udf_runner_cpp_v2_demo_consume_record_batch(
     ArrowArray* array, ArrowSchema* schema, int64_t* out_row_count, int64_t* out_id_sum)
 {
-    const arrow::Status status = ConsumeDemoRecordBatch(array, schema, out_row_count, out_id_sum);
+    const arrow::Status status =
+        consume_demo_record_batch(array, schema, out_row_count, out_id_sum);
     if (!status.ok())
     {
-        SetLastError(status);
+        set_last_error(status);
         return 1;
     }
     g_last_error.clear();

udf-runner-cpp/v2/arrow_c_data_demo_test.cc

@@ -25,8 +25,8 @@ using export_fn_t  = int (*)(ArrowArray*, ArrowSchema*);
 using consume_fn_t = int (*)(ArrowArray*, ArrowSchema*, int64_t*, int64_t*);
 using error_fn_t   = const char* (*)();
 
-constexpr std::string_view kArrowMangledPrefix = "_ZN5arrow";
-constexpr std::string_view kDemoExportedPrefix = "udf_runner_cpp_v2_demo_";
+constexpr std::string_view arrow_mangled_prefix = "_ZN5arrow";
+constexpr std::string_view demo_exported_prefix = "udf_runner_cpp_v2_demo_";
 
 [[noreturn]] void fail(const std::string& message)
 {
@@ -140,11 +140,11 @@ void verify_symbols(const std::string& library_path)
 
         const std::string name = read_string(file, string_table.sh_offset + symbol.st_name,
                                              string_table.sh_size - symbol.st_name);
-        if (name.compare(0, kArrowMangledPrefix.size(), kArrowMangledPrefix) == 0)
+        if (name.compare(0, arrow_mangled_prefix.size(), arrow_mangled_prefix) == 0)
         {
             fail("shared library exports an Arrow C++ symbol: " + name);
         }
-        if (name.compare(0, kDemoExportedPrefix.size(), kDemoExportedPrefix) == 0)
+        if (name.compare(0, demo_exported_prefix.size(), demo_exported_prefix) == 0)
         {
             found_demo_symbol = true;
         }
@@ -227,6 +227,9 @@ int main(int argc, char** argv)
         assert(names->GetString(0) == "alpha");
         assert(names->GetString(3) == "delta");
 
+        assert(export_batch(nullptr, nullptr) != 0);
+        assert(std::string(last_error()).find("must not be null") != std::string::npos);
+
         ArrowArray second_array{};
         ArrowSchema second_schema{};
         if (export_batch(&second_array, &second_schema) != 0)
@@ -242,6 +245,9 @@ int main(int argc, char** argv)
         }
         assert(row_count == 4);
         assert(id_sum == 10);
+
+        assert(consume_batch(nullptr, nullptr, nullptr, nullptr) != 0);
+        assert(std::string(last_error()).find("must not be null") != std::string::npos);
     }
     catch (const std::exception& error)
     {

udf-runner-cpp/v2/call_metadata_fuzz_test.cc

@@ -1,8 +1,7 @@
 #include "test_utils/json_schema_fuzzing.hpp"
 
-extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data,
-                                      std::size_t size) {
-  exasol::udf::v2::fuzzing::FuzzJsonSchema(
-      data, size, "json_schema/call_metadata.schema.json");
-  return 0;
+extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size)
+{
+    exasol::udf::v2::fuzzing::fuzz_json_schema(data, size, "json_schema/call_metadata.schema.json");
+    return 0;
 }

udf-runner-cpp/v2/connection_information_fuzz_test.cc

@@ -1,8 +1,8 @@
 #include "test_utils/json_schema_fuzzing.hpp"
 
-extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data,
-                                      std::size_t size) {
-  exasol::udf::v2::fuzzing::FuzzJsonSchema(
-      data, size, "json_schema/connection_information.schema.json");
-  return 0;
+extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size)
+{
+    exasol::udf::v2::fuzzing::fuzz_json_schema(data, size,
+                                               "json_schema/connection_information.schema.json");
+    return 0;
 }

udf-runner-cpp/v2/export_specification_fuzz_test.cc

@@ -1,8 +1,8 @@
 #include "test_utils/json_schema_fuzzing.hpp"
 
-extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data,
-                                      std::size_t size) {
-  exasol::udf::v2::fuzzing::FuzzJsonSchema(
-      data, size, "json_schema/export_specification.schema.json");
-  return 0;
+extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size)
+{
+    exasol::udf::v2::fuzzing::fuzz_json_schema(data, size,
+                                               "json_schema/export_specification.schema.json");
+    return 0;
 }

udf-runner-cpp/v2/frame_fuzz_test.cc

@@ -3,8 +3,8 @@
 
 #include "udf_protocol.hpp"
 
-extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data,
-                                      std::size_t size) {
-  exasol::udf::protocol::VerifyFrameBuffer(data, size);
-  return 0;
+extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size)
+{
+    exasol::udf::protocol::verify_frame_buffer(data, size);
+    return 0;
 }

udf-runner-cpp/v2/import_specification_fuzz_test.cc

@@ -1,8 +1,8 @@
 #include "test_utils/json_schema_fuzzing.hpp"
 
-extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data,
-                                      std::size_t size) {
-  exasol::udf::v2::fuzzing::FuzzJsonSchema(
-      data, size, "json_schema/import_specification.schema.json");
-  return 0;
+extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size)
+{
+    exasol::udf::v2::fuzzing::fuzz_json_schema(data, size,
+                                               "json_schema/import_specification.schema.json");
+    return 0;
 }

udf-runner-cpp/v2/json_schema_symbol_leak_test.cc

@@ -18,8 +18,8 @@ namespace isolated_nlohmann = exasol::udf::v2::third_party::nlohmann;
 namespace
 {
 
-constexpr std::string_view kGlobalNamespacePrefix   = "_ZN8nlohmann";
-constexpr std::string_view kIsolatedNamespacePrefix = "_ZN6exasol3udf2v211third_party8nlohmann";
+constexpr std::string_view global_namespace_prefix   = "_ZN8nlohmann";
+constexpr std::string_view isolated_namespace_prefix = "_ZN6exasol3udf2v211third_party8nlohmann";
 
 [[noreturn]] void fail(const std::string& message)
 {
@@ -133,11 +133,11 @@ void verify_symbols(const std::string& library_path)
 
         const std::string name = read_string(file, string_table.sh_offset + symbol.st_name,
                                              string_table.sh_size - symbol.st_name);
-        if (name.compare(0, kGlobalNamespacePrefix.size(), kGlobalNamespacePrefix) == 0)
+        if (name.compare(0, global_namespace_prefix.size(), global_namespace_prefix) == 0)
         {
             fail("validator exports a global nlohmann symbol: " + name);
         }
-        if (name.compare(0, kIsolatedNamespacePrefix.size(), kIsolatedNamespacePrefix) == 0)
+        if (name.compare(0, isolated_namespace_prefix.size(), isolated_namespace_prefix) == 0)
         {
             found_isolated_symbol = true;
         }

udf-runner-cpp/v2/moodycamel_symbol_leak_test.cc

@@ -15,8 +15,8 @@
 namespace
 {
 
-constexpr std::string_view kGlobalNamespacePrefix   = "_ZN10moodycamel";
-constexpr std::string_view kIsolatedNamespacePrefix = "_ZN6exasol3udf2v211third_party10moodycamel";
+constexpr std::string_view global_namespace_prefix   = "_ZN10moodycamel";
+constexpr std::string_view isolated_namespace_prefix = "_ZN6exasol3udf2v211third_party10moodycamel";
 
 [[noreturn]] void fail(const std::string& message)
 {
@@ -97,11 +97,11 @@ void verify_symbols(const std::string& path)
         }
         const std::string name = read_string(file, string_table.sh_offset + symbol.st_name,
                                              string_table.sh_size - symbol.st_name);
-        if (name.starts_with(kGlobalNamespacePrefix))
+        if (name.starts_with(global_namespace_prefix))
         {
             fail("queue library exports a global moodycamel symbol: " + name);
         }
-        if (name.starts_with(kIsolatedNamespacePrefix))
+        if (name.starts_with(isolated_namespace_prefix))
         {
             found_isolated_symbol = true;
         }