From a417ace1057c5424418d1a5652798e46d08c11b8 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Tue, 16 Jun 2026 09:05:11 +0200 Subject: [PATCH] refactor: align identifier naming with house style + re-enable the gate Correct the readability-identifier-naming policy to match the documented house style and re-enable the check as a WarningsAsErrors gate (it was "temporarily disabled during rename migration"). The SDK already follows the policy closely; this is a config alignment plus a small internal-only cleanup. .clang-tidy amendments: - LocalConstant/LocalVariable = lower_case: const/constexpr *locals* stay lower_case; the k-prefix Constant policy applies to file-scope/global constants only (without this, const locals falsely flag en masse). - NamespaceIgnoredRegexp '^(PJ|Ui)$': the flat `PJ` namespace (house style) and uic-generated `Ui`. - C-ABI / canonical-schema / STL exemptions (the plugin binary + wire contract): * Struct/Enum/EnumConstant/Function/GlobalConstant `PJ_`/`pj_` extern "C" C-ABI identifiers, plus the camelBack vtable member `fetchMessageData`. * PublicMember single-capital `D K R P` (ROS/OpenCV CameraInfo/DepthImage intrinsics -- a wire-format field contract). * Method has_value/value_or/error_or (std::expected drop-in interface) and load_config/save_config/ui_content/widget_data/trampoline_* (plugin-author and C-ABI-bridge methods every compiled plugin links by name). * TypeAlias `json` (nlohmann alias). Recasing any of these breaks wire compat, STL-shaped generic code, or the plugin ABI -- they are grandfathered (revisit the snake_case plugin virtuals in a future coordinated 1.0.0). - StructIgnoredRegexp `overloaded` (std::visit idiom). - ParameterIgnoredRegexp '.*_': a trailing underscore on a parameter is the idiomatic shadow-avoidance form (`PayloadView(Span bytes_) : bytes(bytes_)`); stripping it reintroduces a -Wshadow error, so trailing-underscore params are exempt. Internal-only renames (no public API/ABI change -- local variables, file-local helpers, test/example code): flatten_impl/count_leaf_fields_impl -> camelBack; test helpers (ColorEq, ColorNear, schema_release, array_release, stream_release, make_robot_pose) -> camelBack; local vars (requiredString/withPath/writeField/...) -> lower_case; named constants (ns_per_second, default_a_/default_b_) -> k-prefix. Versioning: config + invisible internal renames only; no installed public API, ABI struct, or wire-format change, and abi/baseline.abi is untouched -> no version bump warranted. Co-Authored-By: Claude Opus 4.8 (1M context) --- .clang-tidy | 45 ++++++- pj_base/src/type_tree.cpp | 16 +-- pj_base/tests/arrow_holders_test.cpp | 12 +- .../tests/image_annotations_codec_test.cpp | 18 +-- pj_base/tests/protobuf_wire_test_helpers.hpp | 8 +- pj_base/tests/scene_entities_codec_test.cpp | 8 +- pj_base/tests/type_tree_test.cpp | 8 +- .../pj_plugins/sdk/dialog_plugin_base.hpp | 6 +- .../tests/dialog_plugin_typed_test.cpp | 116 +++++++++--------- .../tests/plugin_lifecycle_test.cpp | 8 +- pj_plugins/examples/mock_schema_parser.cpp | 8 +- pj_plugins/src/plugin_catalog.cpp | 24 ++-- 12 files changed, 160 insertions(+), 117 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index dc2c46f7..7d173268 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -45,7 +45,7 @@ WarningsAsErrors: > modernize-use-nullptr, modernize-use-override, performance-move-const-arg, - # readability-identifier-naming, # temporarily disabled during rename migration + readability-identifier-naming, HeaderFilterRegex: '.*' @@ -98,6 +98,49 @@ CheckOptions: value: CamelCase - key: readability-identifier-naming.TemplateParameterCase value: CamelCase + # const/constexpr LOCALS follow lower_case (house style); the k-prefixed + # Constant policy above applies to file-scope/global constants only. + - key: readability-identifier-naming.LocalConstantCase + value: lower_case + - key: readability-identifier-naming.LocalConstantPrefix + value: '' + - key: readability-identifier-naming.LocalVariableCase + value: lower_case + # `PJ` is the documented flat project namespace; `Ui` is uic-generated. + - key: readability-identifier-naming.NamespaceIgnoredRegexp + value: '^(PJ|Ui)$' + # extern "C" C-ABI plugin contract: PJ_* structs/enums/enum-constants/entry + # points and pj_* exported globals are the plugin binary ABI — recasing them + # breaks every compiled plugin. Also exempt Google Benchmark BM_ functions and + # the std::visit `overloaded` idiom. + - key: readability-identifier-naming.FunctionIgnoredRegexp + value: '(BM_|PJ_|pj_).*' + - key: readability-identifier-naming.StructIgnoredRegexp + value: '(overloaded|PJ_.*)' + - key: readability-identifier-naming.EnumIgnoredRegexp + value: 'PJ_.*' + - key: readability-identifier-naming.EnumConstantIgnoredRegexp + value: 'PJ_.*' + - key: readability-identifier-naming.GlobalConstantIgnoredRegexp + value: 'pj_.*' + # Canonical schema and STL-/plugin-facing API are grandfathered: the ROS/OpenCV + # CameraInfo/DepthImage intrinsics (single-capital D/K/R/P) are a wire-format + # field contract; `has_value`/`value_or`/`error_or` are the std::expected drop-in + # interface; and `load_config`/`save_config`/`trampoline_*` are plugin-author or + # C-ABI-bridge methods that every compiled plugin links by name. Recasing any of + # these breaks wire compat, STL-shaped generic code, or the plugin ABI. (Revisit + # the snake_case plugin virtuals in a future coordinated 1.0.0.) + - key: readability-identifier-naming.PublicMemberIgnoredRegexp + value: '([A-Z]|fetchMessageData)' + - key: readability-identifier-naming.MethodIgnoredRegexp + value: '(has_value|value_or|error_or|load_config|save_config|ui_content|widget_data|trampoline_.*)' + - key: readability-identifier-naming.TypeAliasIgnoredRegexp + value: 'json' + # A trailing underscore on a *parameter* is the idiomatic shadow-avoidance form + # (e.g. `PayloadView(Span bytes_) : bytes(bytes_)` where `bytes` is a member); + # stripping it reintroduces a -Wshadow error. Exempt trailing-underscore params. + - key: readability-identifier-naming.ParameterIgnoredRegexp + value: '.*_' # --- Function size limits --- - key: readability-function-size.LineThreshold diff --git a/pj_base/src/type_tree.cpp b/pj_base/src/type_tree.cpp index bdaeb99f..f0e0eb37 100644 --- a/pj_base/src/type_tree.cpp +++ b/pj_base/src/type_tree.cpp @@ -11,20 +11,20 @@ namespace PJ { namespace { -void flatten_impl(const TypeTreeNode& node, std::string_view prefix, std::vector& out) { +void flattenImpl(const TypeTreeNode& node, std::string_view prefix, std::vector& out) { std::string current_path = prefix.empty() ? node.name : std::string(prefix) + "." + node.name; switch (node.kind) { case TypeKind::kStruct: for (const auto& child : node.children) { - flatten_impl(*child, current_path, out); + flattenImpl(*child, current_path, out); } break; case TypeKind::kArray: if (node.element_type && node.fixed_array_size.has_value()) { for (uint32_t idx = 0; idx < *node.fixed_array_size; ++idx) { std::string indexed = current_path + "[" + std::to_string(idx) + "]"; - flatten_impl(*node.element_type, indexed, out); + flattenImpl(*node.element_type, indexed, out); } } // Dynamic arrays (no fixed_array_size) produce 0 paths @@ -36,18 +36,18 @@ void flatten_impl(const TypeTreeNode& node, std::string_view prefix, std::vector } } -std::size_t count_leaf_fields_impl(const TypeTreeNode& node) { +std::size_t countLeafFieldsImpl(const TypeTreeNode& node) { switch (node.kind) { case TypeKind::kStruct: { std::size_t count = 0; for (const auto& child : node.children) { - count += count_leaf_fields_impl(*child); + count += countLeafFieldsImpl(*child); } return count; } case TypeKind::kArray: if (node.element_type && node.fixed_array_size.has_value()) { - return *node.fixed_array_size * count_leaf_fields_impl(*node.element_type); + return *node.fixed_array_size * countLeafFieldsImpl(*node.element_type); } return 0; // dynamic array: 0 columns until expanded default: @@ -118,13 +118,13 @@ std::vector flattenFieldPaths(const TypeTreeNode& root) { } // Skip root struct name -- children use empty prefix for (const auto& child : root.children) { - flatten_impl(*child, "", result); + flattenImpl(*child, "", result); } return result; } std::size_t countLeafFields(const TypeTreeNode& root) { - return count_leaf_fields_impl(root); + return countLeafFieldsImpl(root); } } // namespace PJ diff --git a/pj_base/tests/arrow_holders_test.cpp b/pj_base/tests/arrow_holders_test.cpp index ce3a9ed1..0f5f422f 100644 --- a/pj_base/tests/arrow_holders_test.cpp +++ b/pj_base/tests/arrow_holders_test.cpp @@ -37,36 +37,36 @@ int& streamReleaseCount() { return count; } -void schema_release(::ArrowSchema* s) { +void schemaRelease(::ArrowSchema* s) { ++schemaReleaseCount(); std::memset(s, 0, sizeof(*s)); // spec: release sets fields to null/0 } -void array_release(::ArrowArray* a) { +void arrayRelease(::ArrowArray* a) { ++arrayReleaseCount(); std::memset(a, 0, sizeof(*a)); } -void stream_release(::ArrowArrayStream* s) { +void streamRelease(::ArrowArrayStream* s) { ++streamReleaseCount(); std::memset(s, 0, sizeof(*s)); } ::ArrowSchema makeLiveSchema() { ::ArrowSchema s{}; - s.release = schema_release; + s.release = schemaRelease; return s; } ::ArrowArray makeLiveArray() { ::ArrowArray a{}; - a.release = array_release; + a.release = arrayRelease; return a; } ::ArrowArrayStream makeLiveStream() { ::ArrowArrayStream s{}; - s.release = stream_release; + s.release = streamRelease; return s; } diff --git a/pj_base/tests/image_annotations_codec_test.cpp b/pj_base/tests/image_annotations_codec_test.cpp index c480f743..88d68b6c 100644 --- a/pj_base/tests/image_annotations_codec_test.cpp +++ b/pj_base/tests/image_annotations_codec_test.cpp @@ -34,7 +34,7 @@ sdk::ImageAnnotations roundTrip(const sdk::ImageAnnotations& input) { // Compare two ColorRGBA values allowing 1-LSB drift on each channel from the // double-quantization round-trip (uint8 -> double in [0,1] -> uint8). -::testing::AssertionResult ColorEq(const ColorRGBA& a, const ColorRGBA& b) { +::testing::AssertionResult colorEq(const ColorRGBA& a, const ColorRGBA& b) { auto near = [](uint8_t x, uint8_t y) { return x > y ? (x - y) <= 1 : (y - x) <= 1; }; if (near(a.r, b.r) && near(a.g, b.g) && near(a.b, b.b) && near(a.a, b.a)) { return ::testing::AssertionSuccess(); @@ -150,8 +150,8 @@ TEST(ImageAnnotationCodecTest, RoundTrip_LineLoopFourPoints) { ASSERT_EQ(out.points.size(), 1u); EXPECT_EQ(out.points[0].topology, AnnotationTopology::kLineLoop); EXPECT_EQ(out.points[0].points, in.points[0].points); - EXPECT_TRUE(ColorEq(in.points[0].color, out.points[0].color)); - EXPECT_TRUE(ColorEq(in.points[0].fill_color, out.points[0].fill_color)); + EXPECT_TRUE(colorEq(in.points[0].color, out.points[0].color)); + EXPECT_TRUE(colorEq(in.points[0].fill_color, out.points[0].fill_color)); EXPECT_DOUBLE_EQ(out.points[0].thickness, 2.5); } @@ -190,8 +190,8 @@ TEST(ImageAnnotationCodecTest, RoundTrip_CirclePreservesDiameterRadiusInverse) { EXPECT_DOUBLE_EQ(out.circles[0].center.y, 60.0); EXPECT_DOUBLE_EQ(out.circles[0].radius, 10.0); EXPECT_DOUBLE_EQ(out.circles[0].thickness, 1.5); - EXPECT_TRUE(ColorEq(in.circles[0].color, out.circles[0].color)); - EXPECT_TRUE(ColorEq(in.circles[0].fill_color, out.circles[0].fill_color)); + EXPECT_TRUE(colorEq(in.circles[0].color, out.circles[0].color)); + EXPECT_TRUE(colorEq(in.circles[0].fill_color, out.circles[0].fill_color)); } TEST(ImageAnnotationCodecTest, RoundTrip_TextUtf8) { @@ -209,7 +209,7 @@ TEST(ImageAnnotationCodecTest, RoundTrip_TextUtf8) { EXPECT_DOUBLE_EQ(out.texts[0].position.x, 320.5); EXPECT_DOUBLE_EQ(out.texts[0].position.y, 240.25); EXPECT_DOUBLE_EQ(out.texts[0].font_size, 14.0); - EXPECT_TRUE(ColorEq(in.texts[0].color, out.texts[0].color)); + EXPECT_TRUE(colorEq(in.texts[0].color, out.texts[0].color)); } TEST(ImageAnnotationCodecTest, RoundTrip_FullImageAnnotationAllThreeKinds) { @@ -291,9 +291,9 @@ TEST(ImageAnnotationCodecTest, RoundTrip_PerVertexColors) { auto out = roundTrip(in); ASSERT_EQ(out.points.size(), 1u); ASSERT_EQ(out.points[0].colors.size(), 3u); - EXPECT_TRUE(ColorEq(in.points[0].colors[0], out.points[0].colors[0])); - EXPECT_TRUE(ColorEq(in.points[0].colors[1], out.points[0].colors[1])); - EXPECT_TRUE(ColorEq(in.points[0].colors[2], out.points[0].colors[2])); + EXPECT_TRUE(colorEq(in.points[0].colors[0], out.points[0].colors[0])); + EXPECT_TRUE(colorEq(in.points[0].colors[1], out.points[0].colors[1])); + EXPECT_TRUE(colorEq(in.points[0].colors[2], out.points[0].colors[2])); } } // namespace diff --git a/pj_base/tests/protobuf_wire_test_helpers.hpp b/pj_base/tests/protobuf_wire_test_helpers.hpp index 0bdff288..8d591259 100644 --- a/pj_base/tests/protobuf_wire_test_helpers.hpp +++ b/pj_base/tests/protobuf_wire_test_helpers.hpp @@ -73,12 +73,12 @@ inline void appendBytes(std::vector& out, const uint8_t* data, size_t s // each builds the inner message body (sans length prefix) for the proto type. inline std::vector encodeTimestamp(Timestamp timestamp_ns) { - constexpr int64_t ns_per_second = 1'000'000'000LL; - int64_t seconds = timestamp_ns / ns_per_second; - int32_t nanos = static_cast(timestamp_ns % ns_per_second); + constexpr int64_t kNsPerSecond = 1'000'000'000LL; + int64_t seconds = timestamp_ns / kNsPerSecond; + int32_t nanos = static_cast(timestamp_ns % kNsPerSecond); if (nanos < 0) { --seconds; - nanos += static_cast(ns_per_second); + nanos += static_cast(kNsPerSecond); } std::vector body; appendTag(body, 1, 0); diff --git a/pj_base/tests/scene_entities_codec_test.cpp b/pj_base/tests/scene_entities_codec_test.cpp index e4efc45b..15f02273 100644 --- a/pj_base/tests/scene_entities_codec_test.cpp +++ b/pj_base/tests/scene_entities_codec_test.cpp @@ -35,7 +35,7 @@ namespace pb = ::PJ::test_pb; // Compare two ColorRGBA values allowing 1-LSB drift on each channel from the // uint8 -> double in [0,1] -> uint8 round-trip in the codec. -::testing::AssertionResult ColorNear(const ColorRGBA& a, const ColorRGBA& b) { +::testing::AssertionResult colorNear(const ColorRGBA& a, const ColorRGBA& b) { auto near = [](uint8_t x, uint8_t y) { return x > y ? (x - y) <= 1 : (y - x) <= 1; }; if (near(a.r, b.r) && near(a.g, b.g) && near(a.b, b.b) && near(a.a, b.a)) { return ::testing::AssertionSuccess(); @@ -161,7 +161,7 @@ TEST(SceneEntitiesCodecTest, RoundTripOneEntityWithEachPrimitive) { ASSERT_EQ(dst.axes.size(), 1u); EXPECT_EQ(dst.arrows.front().pose, src.arrows.front().pose); - EXPECT_TRUE(ColorNear(src.arrows.front().color, dst.arrows.front().color)); + EXPECT_TRUE(colorNear(src.arrows.front().color, dst.arrows.front().color)); EXPECT_EQ(dst.cubes.front().size, src.cubes.front().size); EXPECT_DOUBLE_EQ(dst.cylinders.front().top_scale, src.cylinders.front().top_scale); EXPECT_EQ(dst.lines.front().type, src.lines.front().type); @@ -196,7 +196,7 @@ TEST(SceneEntitiesCodecTest, RoundTripLineWithPerVertexColorsAndIndices) { EXPECT_EQ(dst_line.type, LineType::kLineList); EXPECT_EQ(dst_line.indices, std::vector({0, 1, 2, 3})); ASSERT_EQ(dst_line.colors.size(), 4u); - EXPECT_TRUE(ColorNear(ColorRGBA{0, 255, 0, 255}, dst_line.colors[1])); + EXPECT_TRUE(colorNear(ColorRGBA{0, 255, 0, 255}, dst_line.colors[1])); } TEST(SceneEntitiesCodecTest, RoundTripModelPrimitive) { @@ -228,7 +228,7 @@ TEST(SceneEntitiesCodecTest, RoundTripModelPrimitive) { const auto& dst = out->entities.front().models.front(); EXPECT_EQ(dst.pose, src.pose); EXPECT_EQ(dst.scale, src.scale); - EXPECT_TRUE(ColorNear(src.color, dst.color)); + EXPECT_TRUE(colorNear(src.color, dst.color)); EXPECT_TRUE(dst.override_color); EXPECT_EQ(dst.url, src.url); EXPECT_EQ(dst.media_type, src.media_type); diff --git a/pj_base/tests/type_tree_test.cpp b/pj_base/tests/type_tree_test.cpp index c27a6ee2..97f94c62 100644 --- a/pj_base/tests/type_tree_test.cpp +++ b/pj_base/tests/type_tree_test.cpp @@ -19,7 +19,7 @@ namespace { // position: struct {x: float32, y: float32, z: float32} // rotation: struct {w: float32, x: float32, y: float32, z: float32} // semantic_tags = {"quaternion"} -std::shared_ptr make_robot_pose() { +std::shared_ptr makeRobotPose() { auto position = makeStruct( "position", { makePrimitive("x", PrimitiveType::kFloat32), @@ -51,7 +51,7 @@ std::shared_ptr make_robot_pose() { // ---------- Test 1: flatten_field_paths on robot_pose ---------- TEST(TypeTreeTest, FlattenFieldPathsRobotPose) { - auto pose = make_robot_pose(); + auto pose = makeRobotPose(); auto paths = flattenFieldPaths(*pose); const std::vector expected = { @@ -67,7 +67,7 @@ TEST(TypeTreeTest, FlattenFieldPathsRobotPose) { // ---------- Test 2: count_leaf_fields on robot_pose ---------- TEST(TypeTreeTest, CountLeafFieldsRobotPose) { - auto pose = make_robot_pose(); + auto pose = makeRobotPose(); EXPECT_EQ(countLeafFields(*pose), 8u); } @@ -110,7 +110,7 @@ TEST(TypeTreeTest, MakeEnumSetsKind) { // ---------- Test 4: semantic tags are preserved ---------- TEST(TypeTreeTest, SemanticTagsPreserved) { - auto pose = make_robot_pose(); + auto pose = makeRobotPose(); // rotation is the third child (index 2) ASSERT_GE(pose->children.size(), 3u); const auto& rotation = pose->children[2]; diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp index 7f5a84c5..872ff1b1 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp @@ -85,7 +85,7 @@ class DialogPluginBase { return; } out_error->code = code; - auto writeField = [](char* dest, std::size_t dest_size, std::string_view src) { + auto write_field = [](char* dest, std::size_t dest_size, std::string_view src) { if (dest == nullptr || dest_size == 0) { return; } @@ -93,8 +93,8 @@ class DialogPluginBase { std::memcpy(dest, src.data(), n); dest[n] = '\0'; }; - writeField(out_error->domain, sizeof(out_error->domain), domain); - writeField(out_error->message, sizeof(out_error->message), message); + write_field(out_error->domain, sizeof(out_error->domain), domain); + write_field(out_error->message, sizeof(out_error->message), message); // Clear the v3.1 growth-path slots so a reused error struct does not // carry a stale pointer from a previous call. Matches sdk::fillError. out_error->extended = nullptr; diff --git a/pj_plugins/dialog_protocol/tests/dialog_plugin_typed_test.cpp b/pj_plugins/dialog_protocol/tests/dialog_plugin_typed_test.cpp index d62d8764..602a22c7 100644 --- a/pj_plugins/dialog_protocol/tests/dialog_plugin_typed_test.cpp +++ b/pj_plugins/dialog_protocol/tests/dialog_plugin_typed_test.cpp @@ -137,128 +137,128 @@ bool dispatch(PJ::DialogPluginBase& plugin, std::string_view widget, std::string class TypedDispatchTest : public ::testing::Test { protected: - RecordingPlugin plugin; + RecordingPlugin plugin_; }; // --- Individual dispatch tests --- TEST_F(TypedDispatchTest, TextChanged) { - EXPECT_TRUE(dispatch(plugin, "my_input", R"({"text": "hello"})")); - EXPECT_EQ(plugin.last_handler, "text_changed"); - EXPECT_EQ(plugin.last_widget, "my_input"); - EXPECT_EQ(plugin.last_text, "hello"); + EXPECT_TRUE(dispatch(plugin_, "my_input", R"({"text": "hello"})")); + EXPECT_EQ(plugin_.last_handler, "text_changed"); + EXPECT_EQ(plugin_.last_widget, "my_input"); + EXPECT_EQ(plugin_.last_text, "hello"); } TEST_F(TypedDispatchTest, IndexChanged) { - EXPECT_TRUE(dispatch(plugin, "combo", R"({"current_index": 3})")); - EXPECT_EQ(plugin.last_handler, "index_changed"); - EXPECT_EQ(plugin.last_int, 3); + EXPECT_TRUE(dispatch(plugin_, "combo", R"({"current_index": 3})")); + EXPECT_EQ(plugin_.last_handler, "index_changed"); + EXPECT_EQ(plugin_.last_int, 3); } TEST_F(TypedDispatchTest, Toggled) { - EXPECT_TRUE(dispatch(plugin, "checkbox", R"({"checked": true})")); - EXPECT_EQ(plugin.last_handler, "toggled"); - EXPECT_TRUE(plugin.last_bool); + EXPECT_TRUE(dispatch(plugin_, "checkbox", R"({"checked": true})")); + EXPECT_EQ(plugin_.last_handler, "toggled"); + EXPECT_TRUE(plugin_.last_bool); } TEST_F(TypedDispatchTest, ToggledFalse) { - EXPECT_TRUE(dispatch(plugin, "checkbox", R"({"checked": false})")); - EXPECT_EQ(plugin.last_handler, "toggled"); - EXPECT_FALSE(plugin.last_bool); + EXPECT_TRUE(dispatch(plugin_, "checkbox", R"({"checked": false})")); + EXPECT_EQ(plugin_.last_handler, "toggled"); + EXPECT_FALSE(plugin_.last_bool); } TEST_F(TypedDispatchTest, ValueInt) { - EXPECT_TRUE(dispatch(plugin, "spinbox", R"({"value": 42})")); - EXPECT_EQ(plugin.last_handler, "value_int"); - EXPECT_EQ(plugin.last_int, 42); + EXPECT_TRUE(dispatch(plugin_, "spinbox", R"({"value": 42})")); + EXPECT_EQ(plugin_.last_handler, "value_int"); + EXPECT_EQ(plugin_.last_int, 42); } TEST_F(TypedDispatchTest, ValueDouble) { - EXPECT_TRUE(dispatch(plugin, "dspinbox", R"({"value": 3.14})")); - EXPECT_EQ(plugin.last_handler, "value_double"); - EXPECT_DOUBLE_EQ(plugin.last_double, 3.14); + EXPECT_TRUE(dispatch(plugin_, "dspinbox", R"({"value": 3.14})")); + EXPECT_EQ(plugin_.last_handler, "value_double"); + EXPECT_DOUBLE_EQ(plugin_.last_double, 3.14); } TEST_F(TypedDispatchTest, Clicked) { - EXPECT_TRUE(dispatch(plugin, "btn", R"({"clicked": true})")); - EXPECT_EQ(plugin.last_handler, "clicked"); - EXPECT_EQ(plugin.last_widget, "btn"); + EXPECT_TRUE(dispatch(plugin_, "btn", R"({"clicked": true})")); + EXPECT_EQ(plugin_.last_handler, "clicked"); + EXPECT_EQ(plugin_.last_widget, "btn"); } TEST_F(TypedDispatchTest, FileSelected) { - EXPECT_TRUE(dispatch(plugin, "file_btn", R"({"file_selected": "/tmp/data.csv"})")); - EXPECT_EQ(plugin.last_handler, "file_selected"); - EXPECT_EQ(plugin.last_text, "/tmp/data.csv"); + EXPECT_TRUE(dispatch(plugin_, "file_btn", R"({"file_selected": "/tmp/data.csv"})")); + EXPECT_EQ(plugin_.last_handler, "file_selected"); + EXPECT_EQ(plugin_.last_text, "/tmp/data.csv"); } TEST_F(TypedDispatchTest, SelectionChanged) { - EXPECT_TRUE(dispatch(plugin, "list", R"({"selected_items": ["a", "b"]})")); - EXPECT_EQ(plugin.last_handler, "selection_changed"); - ASSERT_EQ(plugin.last_strings.size(), 2u); - EXPECT_EQ(plugin.last_strings[0], "a"); - EXPECT_EQ(plugin.last_strings[1], "b"); + EXPECT_TRUE(dispatch(plugin_, "list", R"({"selected_items": ["a", "b"]})")); + EXPECT_EQ(plugin_.last_handler, "selection_changed"); + ASSERT_EQ(plugin_.last_strings.size(), 2u); + EXPECT_EQ(plugin_.last_strings[0], "a"); + EXPECT_EQ(plugin_.last_strings[1], "b"); } TEST_F(TypedDispatchTest, TabChanged) { - EXPECT_TRUE(dispatch(plugin, "tabs", R"({"tab_index": 2})")); - EXPECT_EQ(plugin.last_handler, "tab_changed"); - EXPECT_EQ(plugin.last_int, 2); + EXPECT_TRUE(dispatch(plugin_, "tabs", R"({"tab_index": 2})")); + EXPECT_EQ(plugin_.last_handler, "tab_changed"); + EXPECT_EQ(plugin_.last_int, 2); } TEST_F(TypedDispatchTest, DateRangeChanged) { - EXPECT_TRUE(dispatch(plugin, "picker", R"({"date_from_iso": "2016-04-29T00:00:00", "date_to_iso": ""})")); - EXPECT_EQ(plugin.last_handler, "date_range_changed"); - EXPECT_EQ(plugin.last_widget, "picker"); - EXPECT_EQ(plugin.last_date_from, "2016-04-29T00:00:00"); - EXPECT_EQ(plugin.last_date_to, ""); + EXPECT_TRUE(dispatch(plugin_, "picker", R"({"date_from_iso": "2016-04-29T00:00:00", "date_to_iso": ""})")); + EXPECT_EQ(plugin_.last_handler, "date_range_changed"); + EXPECT_EQ(plugin_.last_widget, "picker"); + EXPECT_EQ(plugin_.last_date_from, "2016-04-29T00:00:00"); + EXPECT_EQ(plugin_.last_date_to, ""); } // --- Edge cases --- TEST_F(TypedDispatchTest, UnrecognizedFieldReturnsFalse) { - EXPECT_FALSE(dispatch(plugin, "widget", R"({"unknown_field": 123})")); - EXPECT_TRUE(plugin.last_handler.empty()); + EXPECT_FALSE(dispatch(plugin_, "widget", R"({"unknown_field": 123})")); + EXPECT_TRUE(plugin_.last_handler.empty()); } TEST_F(TypedDispatchTest, EmptyJsonReturnsFalse) { - EXPECT_FALSE(dispatch(plugin, "widget", "{}")); - EXPECT_TRUE(plugin.last_handler.empty()); + EXPECT_FALSE(dispatch(plugin_, "widget", "{}")); + EXPECT_TRUE(plugin_.last_handler.empty()); } TEST_F(TypedDispatchTest, InvalidJsonReturnsFalse) { - EXPECT_FALSE(dispatch(plugin, "widget", "not json")); - EXPECT_TRUE(plugin.last_handler.empty()); + EXPECT_FALSE(dispatch(plugin_, "widget", "not json")); + EXPECT_TRUE(plugin_.last_handler.empty()); } // --- Priority tests (multi-field events) --- TEST_F(TypedDispatchTest, TextTakesPriorityOverClicked) { // text is checked before clicked in the dispatch chain - EXPECT_TRUE(dispatch(plugin, "w", R"({"text": "x", "clicked": true})")); - EXPECT_EQ(plugin.last_handler, "text_changed"); + EXPECT_TRUE(dispatch(plugin_, "w", R"({"text": "x", "clicked": true})")); + EXPECT_EQ(plugin_.last_handler, "text_changed"); } TEST_F(TypedDispatchTest, FileSelectedTakesPriorityOverClicked) { // file_selected is checked before clicked - EXPECT_TRUE(dispatch(plugin, "w", R"({"file_selected": "/a", "clicked": true})")); - EXPECT_EQ(plugin.last_handler, "file_selected"); + EXPECT_TRUE(dispatch(plugin_, "w", R"({"file_selected": "/a", "clicked": true})")); + EXPECT_EQ(plugin_.last_handler, "file_selected"); } TEST_F(TypedDispatchTest, CurrentIndexTakesPriorityOverValue) { // current_index is checked before value - EXPECT_TRUE(dispatch(plugin, "w", R"({"current_index": 1, "value": 5})")); - EXPECT_EQ(plugin.last_handler, "index_changed"); + EXPECT_TRUE(dispatch(plugin_, "w", R"({"current_index": 1, "value": 5})")); + EXPECT_EQ(plugin_.last_handler, "index_changed"); } TEST_F(TypedDispatchTest, CodeChangedCarriesCursorToTypedHandler) { - EXPECT_TRUE(dispatch(plugin, "editor", R"({"code_changed": "robot ==", "code_cursor": 8})")); - EXPECT_EQ(plugin.last_handler, "code_changed"); - EXPECT_EQ(plugin.last_text, "robot =="); - EXPECT_EQ(plugin.last_int, 8); + EXPECT_TRUE(dispatch(plugin_, "editor", R"({"code_changed": "robot ==", "code_cursor": 8})")); + EXPECT_EQ(plugin_.last_handler, "code_changed"); + EXPECT_EQ(plugin_.last_text, "robot =="); + EXPECT_EQ(plugin_.last_int, 8); } TEST_F(TypedDispatchTest, CodeChangedWithoutCursorPassesNegativeOne) { - EXPECT_TRUE(dispatch(plugin, "editor", R"({"code_changed": "x"})")); - EXPECT_EQ(plugin.last_handler, "code_changed"); - EXPECT_EQ(plugin.last_int, -1); + EXPECT_TRUE(dispatch(plugin_, "editor", R"({"code_changed": "x"})")); + EXPECT_EQ(plugin_.last_handler, "code_changed"); + EXPECT_EQ(plugin_.last_int, -1); } diff --git a/pj_plugins/dialog_protocol/tests/plugin_lifecycle_test.cpp b/pj_plugins/dialog_protocol/tests/plugin_lifecycle_test.cpp index ad18d695..6f6d7948 100644 --- a/pj_plugins/dialog_protocol/tests/plugin_lifecycle_test.cpp +++ b/pj_plugins/dialog_protocol/tests/plugin_lifecycle_test.cpp @@ -187,8 +187,8 @@ TEST_F(PluginLifecycleTest, SaveLoadConfigRoundTrip) { } TEST_F(PluginLifecycleTest, LoadConfigWithInvalidJson) { - const char kBad[] = "not valid json"; - PJ_string_view_t sv{kBad, sizeof(kBad) - 1}; + const char k_bad[] = "not valid json"; + PJ_string_view_t sv{k_bad, sizeof(k_bad) - 1}; bool loaded = vt_->load_config(ctx_, sv, nullptr); EXPECT_FALSE(loaded); } @@ -196,8 +196,8 @@ TEST_F(PluginLifecycleTest, LoadConfigWithInvalidJson) { TEST_F(PluginLifecycleTest, LoadConfigWithWrongTypes) { // name as int instead of string — should not crash, should still return true // (type-safe loading just skips invalid fields) - const char kJson[] = R"({"name": 42, "count": "not_int"})"; - PJ_string_view_t sv{kJson, sizeof(kJson) - 1}; + const char k_json[] = R"({"name": 42, "count": "not_int"})"; + PJ_string_view_t sv{k_json, sizeof(k_json) - 1}; bool loaded = vt_->load_config(ctx_, sv, nullptr); EXPECT_TRUE(loaded); // Verify name was NOT overwritten (was string, got int — skipped) diff --git a/pj_plugins/examples/mock_schema_parser.cpp b/pj_plugins/examples/mock_schema_parser.cpp index 1b981664..c9d7aee4 100644 --- a/pj_plugins/examples/mock_schema_parser.cpp +++ b/pj_plugins/examples/mock_schema_parser.cpp @@ -87,8 +87,8 @@ class MockSchemaParser : public PJ::MessageParserPluginBase { private: PJ::Status bindFields() { - const std::string& name_a = field_names_.empty() ? default_a_ : field_names_[0]; - const std::string& name_b = field_names_.size() < 2 ? default_b_ : field_names_[1]; + const std::string& name_a = field_names_.empty() ? kDefaultA : field_names_[0]; + const std::string& name_b = field_names_.size() < 2 ? kDefaultB : field_names_[1]; auto handle_a = writeHost().ensureField(name_a, PJ::PrimitiveType::kFloat64); if (!handle_a) { @@ -110,8 +110,8 @@ class MockSchemaParser : public PJ::MessageParserPluginBase { std::vector field_handles_; bool fields_bound_ = false; - static inline const std::string default_a_ = "x"; - static inline const std::string default_b_ = "y"; + static inline const std::string kDefaultA = "x"; + static inline const std::string kDefaultB = "y"; }; } // namespace diff --git a/pj_plugins/src/plugin_catalog.cpp b/pj_plugins/src/plugin_catalog.cpp index 82deac97..97da5b8e 100644 --- a/pj_plugins/src/plugin_catalog.cpp +++ b/pj_plugins/src/plugin_catalog.cpp @@ -196,14 +196,14 @@ Expected decodeManifest( return unexpected("plugin embedded manifest must be a JSON object"); } - auto requiredString = [&](std::string_view key) -> Expected { + auto required_string = [&](std::string_view key) -> Expected { const auto it = j.find(std::string(key)); if (it == j.end() || !it->is_string() || it->get().empty()) { return unexpected(fmt::format("plugin embedded manifest missing required string key: {}", key)); } return it->get(); }; - auto optionalString = [&](std::string_view key) -> Expected { + auto optional_string = [&](std::string_view key) -> Expected { const auto it = j.find(std::string(key)); if (it == j.end()) { return std::string{}; @@ -219,15 +219,15 @@ Expected decodeManifest( d.abi_major = PJ_ABI_VERSION; d.family = family; - auto id = requiredString("id"); + auto id = required_string("id"); if (!id) { return unexpected(id.error()); } - auto name = requiredString("name"); + auto name = required_string("name"); if (!name) { return unexpected(name.error()); } - auto version = requiredString("version"); + auto version = required_string("version"); if (!version) { return unexpected(version.error()); } @@ -236,11 +236,11 @@ Expected decodeManifest( d.name = *name; d.version = *version; - auto description = optionalString("description"); + auto description = optional_string("description"); if (!description) { return unexpected(description.error()); } - auto category = optionalString("category"); + auto category = optional_string("category"); if (!category) { return unexpected(category.error()); } @@ -292,26 +292,26 @@ Expected inspectPluginDso(const std::filesystem::path& dso_pat if (!hasDsoSuffix(dso_path)) { return unexpected(fmt::format("not a platform plugin DSO: {}", dso_path.string())); } - auto withPath = [&](const std::string& error) { return fmt::format("{}: {}", dso_path.string(), error); }; + auto with_path = [&](const std::string& error) { return fmt::format("{}: {}", dso_path.string(), error); }; auto handle = detail::loadLibraryHandle(dso_path.string()); if (!handle) { - return unexpected(withPath(handle.error())); + return unexpected(with_path(handle.error())); } std::unique_ptr library(*handle); if (auto abi = detail::checkPluginAbiVersion(library.get()); !abi) { - return unexpected(withPath(abi.error())); + return unexpected(with_path(abi.error())); } auto candidate = findEmbeddedManifest(library.get()); if (!candidate) { - return unexpected(withPath(candidate.error())); + return unexpected(with_path(candidate.error())); } auto descriptor = decodeManifest(dso_path, candidate->family, candidate->manifest_json); if (!descriptor) { - return unexpected(withPath(descriptor.error())); + return unexpected(with_path(descriptor.error())); } return *descriptor; }