Skip to content

Commit e775917

Browse files
refactor: align identifier naming with house style + re-enable the gate (#124)
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) <noreply@anthropic.com>
1 parent 8f485e5 commit e775917

12 files changed

Lines changed: 160 additions & 117 deletions

.clang-tidy

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ WarningsAsErrors: >
4545
modernize-use-nullptr,
4646
modernize-use-override,
4747
performance-move-const-arg,
48-
# readability-identifier-naming, # temporarily disabled during rename migration
48+
readability-identifier-naming,
4949
5050
HeaderFilterRegex: '.*'
5151

@@ -98,6 +98,49 @@ CheckOptions:
9898
value: CamelCase
9999
- key: readability-identifier-naming.TemplateParameterCase
100100
value: CamelCase
101+
# const/constexpr LOCALS follow lower_case (house style); the k-prefixed
102+
# Constant policy above applies to file-scope/global constants only.
103+
- key: readability-identifier-naming.LocalConstantCase
104+
value: lower_case
105+
- key: readability-identifier-naming.LocalConstantPrefix
106+
value: ''
107+
- key: readability-identifier-naming.LocalVariableCase
108+
value: lower_case
109+
# `PJ` is the documented flat project namespace; `Ui` is uic-generated.
110+
- key: readability-identifier-naming.NamespaceIgnoredRegexp
111+
value: '^(PJ|Ui)$'
112+
# extern "C" C-ABI plugin contract: PJ_* structs/enums/enum-constants/entry
113+
# points and pj_* exported globals are the plugin binary ABI — recasing them
114+
# breaks every compiled plugin. Also exempt Google Benchmark BM_ functions and
115+
# the std::visit `overloaded` idiom.
116+
- key: readability-identifier-naming.FunctionIgnoredRegexp
117+
value: '(BM_|PJ_|pj_).*'
118+
- key: readability-identifier-naming.StructIgnoredRegexp
119+
value: '(overloaded|PJ_.*)'
120+
- key: readability-identifier-naming.EnumIgnoredRegexp
121+
value: 'PJ_.*'
122+
- key: readability-identifier-naming.EnumConstantIgnoredRegexp
123+
value: 'PJ_.*'
124+
- key: readability-identifier-naming.GlobalConstantIgnoredRegexp
125+
value: 'pj_.*'
126+
# Canonical schema and STL-/plugin-facing API are grandfathered: the ROS/OpenCV
127+
# CameraInfo/DepthImage intrinsics (single-capital D/K/R/P) are a wire-format
128+
# field contract; `has_value`/`value_or`/`error_or` are the std::expected drop-in
129+
# interface; and `load_config`/`save_config`/`trampoline_*` are plugin-author or
130+
# C-ABI-bridge methods that every compiled plugin links by name. Recasing any of
131+
# these breaks wire compat, STL-shaped generic code, or the plugin ABI. (Revisit
132+
# the snake_case plugin virtuals in a future coordinated 1.0.0.)
133+
- key: readability-identifier-naming.PublicMemberIgnoredRegexp
134+
value: '([A-Z]|fetchMessageData)'
135+
- key: readability-identifier-naming.MethodIgnoredRegexp
136+
value: '(has_value|value_or|error_or|load_config|save_config|ui_content|widget_data|trampoline_.*)'
137+
- key: readability-identifier-naming.TypeAliasIgnoredRegexp
138+
value: 'json'
139+
# A trailing underscore on a *parameter* is the idiomatic shadow-avoidance form
140+
# (e.g. `PayloadView(Span bytes_) : bytes(bytes_)` where `bytes` is a member);
141+
# stripping it reintroduces a -Wshadow error. Exempt trailing-underscore params.
142+
- key: readability-identifier-naming.ParameterIgnoredRegexp
143+
value: '.*_'
101144

102145
# --- Function size limits ---
103146
- key: readability-function-size.LineThreshold

pj_base/src/type_tree.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,20 @@
1111
namespace PJ {
1212
namespace {
1313

14-
void flatten_impl(const TypeTreeNode& node, std::string_view prefix, std::vector<std::string>& out) {
14+
void flattenImpl(const TypeTreeNode& node, std::string_view prefix, std::vector<std::string>& out) {
1515
std::string current_path = prefix.empty() ? node.name : std::string(prefix) + "." + node.name;
1616

1717
switch (node.kind) {
1818
case TypeKind::kStruct:
1919
for (const auto& child : node.children) {
20-
flatten_impl(*child, current_path, out);
20+
flattenImpl(*child, current_path, out);
2121
}
2222
break;
2323
case TypeKind::kArray:
2424
if (node.element_type && node.fixed_array_size.has_value()) {
2525
for (uint32_t idx = 0; idx < *node.fixed_array_size; ++idx) {
2626
std::string indexed = current_path + "[" + std::to_string(idx) + "]";
27-
flatten_impl(*node.element_type, indexed, out);
27+
flattenImpl(*node.element_type, indexed, out);
2828
}
2929
}
3030
// 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
3636
}
3737
}
3838

39-
std::size_t count_leaf_fields_impl(const TypeTreeNode& node) {
39+
std::size_t countLeafFieldsImpl(const TypeTreeNode& node) {
4040
switch (node.kind) {
4141
case TypeKind::kStruct: {
4242
std::size_t count = 0;
4343
for (const auto& child : node.children) {
44-
count += count_leaf_fields_impl(*child);
44+
count += countLeafFieldsImpl(*child);
4545
}
4646
return count;
4747
}
4848
case TypeKind::kArray:
4949
if (node.element_type && node.fixed_array_size.has_value()) {
50-
return *node.fixed_array_size * count_leaf_fields_impl(*node.element_type);
50+
return *node.fixed_array_size * countLeafFieldsImpl(*node.element_type);
5151
}
5252
return 0; // dynamic array: 0 columns until expanded
5353
default:
@@ -118,13 +118,13 @@ std::vector<std::string> flattenFieldPaths(const TypeTreeNode& root) {
118118
}
119119
// Skip root struct name -- children use empty prefix
120120
for (const auto& child : root.children) {
121-
flatten_impl(*child, "", result);
121+
flattenImpl(*child, "", result);
122122
}
123123
return result;
124124
}
125125

126126
std::size_t countLeafFields(const TypeTreeNode& root) {
127-
return count_leaf_fields_impl(root);
127+
return countLeafFieldsImpl(root);
128128
}
129129

130130
} // namespace PJ

pj_base/tests/arrow_holders_test.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,36 +37,36 @@ int& streamReleaseCount() {
3737
return count;
3838
}
3939

40-
void schema_release(::ArrowSchema* s) {
40+
void schemaRelease(::ArrowSchema* s) {
4141
++schemaReleaseCount();
4242
std::memset(s, 0, sizeof(*s)); // spec: release sets fields to null/0
4343
}
4444

45-
void array_release(::ArrowArray* a) {
45+
void arrayRelease(::ArrowArray* a) {
4646
++arrayReleaseCount();
4747
std::memset(a, 0, sizeof(*a));
4848
}
4949

50-
void stream_release(::ArrowArrayStream* s) {
50+
void streamRelease(::ArrowArrayStream* s) {
5151
++streamReleaseCount();
5252
std::memset(s, 0, sizeof(*s));
5353
}
5454

5555
::ArrowSchema makeLiveSchema() {
5656
::ArrowSchema s{};
57-
s.release = schema_release;
57+
s.release = schemaRelease;
5858
return s;
5959
}
6060

6161
::ArrowArray makeLiveArray() {
6262
::ArrowArray a{};
63-
a.release = array_release;
63+
a.release = arrayRelease;
6464
return a;
6565
}
6666

6767
::ArrowArrayStream makeLiveStream() {
6868
::ArrowArrayStream s{};
69-
s.release = stream_release;
69+
s.release = streamRelease;
7070
return s;
7171
}
7272

pj_base/tests/image_annotations_codec_test.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ sdk::ImageAnnotations roundTrip(const sdk::ImageAnnotations& input) {
3434

3535
// Compare two ColorRGBA values allowing 1-LSB drift on each channel from the
3636
// double-quantization round-trip (uint8 -> double in [0,1] -> uint8).
37-
::testing::AssertionResult ColorEq(const ColorRGBA& a, const ColorRGBA& b) {
37+
::testing::AssertionResult colorEq(const ColorRGBA& a, const ColorRGBA& b) {
3838
auto near = [](uint8_t x, uint8_t y) { return x > y ? (x - y) <= 1 : (y - x) <= 1; };
3939
if (near(a.r, b.r) && near(a.g, b.g) && near(a.b, b.b) && near(a.a, b.a)) {
4040
return ::testing::AssertionSuccess();
@@ -150,8 +150,8 @@ TEST(ImageAnnotationCodecTest, RoundTrip_LineLoopFourPoints) {
150150
ASSERT_EQ(out.points.size(), 1u);
151151
EXPECT_EQ(out.points[0].topology, AnnotationTopology::kLineLoop);
152152
EXPECT_EQ(out.points[0].points, in.points[0].points);
153-
EXPECT_TRUE(ColorEq(in.points[0].color, out.points[0].color));
154-
EXPECT_TRUE(ColorEq(in.points[0].fill_color, out.points[0].fill_color));
153+
EXPECT_TRUE(colorEq(in.points[0].color, out.points[0].color));
154+
EXPECT_TRUE(colorEq(in.points[0].fill_color, out.points[0].fill_color));
155155
EXPECT_DOUBLE_EQ(out.points[0].thickness, 2.5);
156156
}
157157

@@ -190,8 +190,8 @@ TEST(ImageAnnotationCodecTest, RoundTrip_CirclePreservesDiameterRadiusInverse) {
190190
EXPECT_DOUBLE_EQ(out.circles[0].center.y, 60.0);
191191
EXPECT_DOUBLE_EQ(out.circles[0].radius, 10.0);
192192
EXPECT_DOUBLE_EQ(out.circles[0].thickness, 1.5);
193-
EXPECT_TRUE(ColorEq(in.circles[0].color, out.circles[0].color));
194-
EXPECT_TRUE(ColorEq(in.circles[0].fill_color, out.circles[0].fill_color));
193+
EXPECT_TRUE(colorEq(in.circles[0].color, out.circles[0].color));
194+
EXPECT_TRUE(colorEq(in.circles[0].fill_color, out.circles[0].fill_color));
195195
}
196196

197197
TEST(ImageAnnotationCodecTest, RoundTrip_TextUtf8) {
@@ -209,7 +209,7 @@ TEST(ImageAnnotationCodecTest, RoundTrip_TextUtf8) {
209209
EXPECT_DOUBLE_EQ(out.texts[0].position.x, 320.5);
210210
EXPECT_DOUBLE_EQ(out.texts[0].position.y, 240.25);
211211
EXPECT_DOUBLE_EQ(out.texts[0].font_size, 14.0);
212-
EXPECT_TRUE(ColorEq(in.texts[0].color, out.texts[0].color));
212+
EXPECT_TRUE(colorEq(in.texts[0].color, out.texts[0].color));
213213
}
214214

215215
TEST(ImageAnnotationCodecTest, RoundTrip_FullImageAnnotationAllThreeKinds) {
@@ -291,9 +291,9 @@ TEST(ImageAnnotationCodecTest, RoundTrip_PerVertexColors) {
291291
auto out = roundTrip(in);
292292
ASSERT_EQ(out.points.size(), 1u);
293293
ASSERT_EQ(out.points[0].colors.size(), 3u);
294-
EXPECT_TRUE(ColorEq(in.points[0].colors[0], out.points[0].colors[0]));
295-
EXPECT_TRUE(ColorEq(in.points[0].colors[1], out.points[0].colors[1]));
296-
EXPECT_TRUE(ColorEq(in.points[0].colors[2], out.points[0].colors[2]));
294+
EXPECT_TRUE(colorEq(in.points[0].colors[0], out.points[0].colors[0]));
295+
EXPECT_TRUE(colorEq(in.points[0].colors[1], out.points[0].colors[1]));
296+
EXPECT_TRUE(colorEq(in.points[0].colors[2], out.points[0].colors[2]));
297297
}
298298

299299
} // namespace

pj_base/tests/protobuf_wire_test_helpers.hpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,12 @@ inline void appendBytes(std::vector<uint8_t>& out, const uint8_t* data, size_t s
7373
// each builds the inner message body (sans length prefix) for the proto type.
7474

7575
inline std::vector<uint8_t> encodeTimestamp(Timestamp timestamp_ns) {
76-
constexpr int64_t ns_per_second = 1'000'000'000LL;
77-
int64_t seconds = timestamp_ns / ns_per_second;
78-
int32_t nanos = static_cast<int32_t>(timestamp_ns % ns_per_second);
76+
constexpr int64_t kNsPerSecond = 1'000'000'000LL;
77+
int64_t seconds = timestamp_ns / kNsPerSecond;
78+
int32_t nanos = static_cast<int32_t>(timestamp_ns % kNsPerSecond);
7979
if (nanos < 0) {
8080
--seconds;
81-
nanos += static_cast<int32_t>(ns_per_second);
81+
nanos += static_cast<int32_t>(kNsPerSecond);
8282
}
8383
std::vector<uint8_t> body;
8484
appendTag(body, 1, 0);

pj_base/tests/scene_entities_codec_test.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ namespace pb = ::PJ::test_pb;
3535

3636
// Compare two ColorRGBA values allowing 1-LSB drift on each channel from the
3737
// uint8 -> double in [0,1] -> uint8 round-trip in the codec.
38-
::testing::AssertionResult ColorNear(const ColorRGBA& a, const ColorRGBA& b) {
38+
::testing::AssertionResult colorNear(const ColorRGBA& a, const ColorRGBA& b) {
3939
auto near = [](uint8_t x, uint8_t y) { return x > y ? (x - y) <= 1 : (y - x) <= 1; };
4040
if (near(a.r, b.r) && near(a.g, b.g) && near(a.b, b.b) && near(a.a, b.a)) {
4141
return ::testing::AssertionSuccess();
@@ -161,7 +161,7 @@ TEST(SceneEntitiesCodecTest, RoundTripOneEntityWithEachPrimitive) {
161161
ASSERT_EQ(dst.axes.size(), 1u);
162162

163163
EXPECT_EQ(dst.arrows.front().pose, src.arrows.front().pose);
164-
EXPECT_TRUE(ColorNear(src.arrows.front().color, dst.arrows.front().color));
164+
EXPECT_TRUE(colorNear(src.arrows.front().color, dst.arrows.front().color));
165165
EXPECT_EQ(dst.cubes.front().size, src.cubes.front().size);
166166
EXPECT_DOUBLE_EQ(dst.cylinders.front().top_scale, src.cylinders.front().top_scale);
167167
EXPECT_EQ(dst.lines.front().type, src.lines.front().type);
@@ -196,7 +196,7 @@ TEST(SceneEntitiesCodecTest, RoundTripLineWithPerVertexColorsAndIndices) {
196196
EXPECT_EQ(dst_line.type, LineType::kLineList);
197197
EXPECT_EQ(dst_line.indices, std::vector<uint32_t>({0, 1, 2, 3}));
198198
ASSERT_EQ(dst_line.colors.size(), 4u);
199-
EXPECT_TRUE(ColorNear(ColorRGBA{0, 255, 0, 255}, dst_line.colors[1]));
199+
EXPECT_TRUE(colorNear(ColorRGBA{0, 255, 0, 255}, dst_line.colors[1]));
200200
}
201201

202202
TEST(SceneEntitiesCodecTest, RoundTripModelPrimitive) {
@@ -228,7 +228,7 @@ TEST(SceneEntitiesCodecTest, RoundTripModelPrimitive) {
228228
const auto& dst = out->entities.front().models.front();
229229
EXPECT_EQ(dst.pose, src.pose);
230230
EXPECT_EQ(dst.scale, src.scale);
231-
EXPECT_TRUE(ColorNear(src.color, dst.color));
231+
EXPECT_TRUE(colorNear(src.color, dst.color));
232232
EXPECT_TRUE(dst.override_color);
233233
EXPECT_EQ(dst.url, src.url);
234234
EXPECT_EQ(dst.media_type, src.media_type);

pj_base/tests/type_tree_test.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace {
1919
// position: struct {x: float32, y: float32, z: float32}
2020
// rotation: struct {w: float32, x: float32, y: float32, z: float32}
2121
// semantic_tags = {"quaternion"}
22-
std::shared_ptr<TypeTreeNode> make_robot_pose() {
22+
std::shared_ptr<TypeTreeNode> makeRobotPose() {
2323
auto position = makeStruct(
2424
"position", {
2525
makePrimitive("x", PrimitiveType::kFloat32),
@@ -51,7 +51,7 @@ std::shared_ptr<TypeTreeNode> make_robot_pose() {
5151
// ---------- Test 1: flatten_field_paths on robot_pose ----------
5252

5353
TEST(TypeTreeTest, FlattenFieldPathsRobotPose) {
54-
auto pose = make_robot_pose();
54+
auto pose = makeRobotPose();
5555
auto paths = flattenFieldPaths(*pose);
5656

5757
const std::vector<std::string> expected = {
@@ -67,7 +67,7 @@ TEST(TypeTreeTest, FlattenFieldPathsRobotPose) {
6767
// ---------- Test 2: count_leaf_fields on robot_pose ----------
6868

6969
TEST(TypeTreeTest, CountLeafFieldsRobotPose) {
70-
auto pose = make_robot_pose();
70+
auto pose = makeRobotPose();
7171
EXPECT_EQ(countLeafFields(*pose), 8u);
7272
}
7373

@@ -110,7 +110,7 @@ TEST(TypeTreeTest, MakeEnumSetsKind) {
110110
// ---------- Test 4: semantic tags are preserved ----------
111111

112112
TEST(TypeTreeTest, SemanticTagsPreserved) {
113-
auto pose = make_robot_pose();
113+
auto pose = makeRobotPose();
114114
// rotation is the third child (index 2)
115115
ASSERT_GE(pose->children.size(), 3u);
116116
const auto& rotation = pose->children[2];

pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,16 @@ class DialogPluginBase {
8585
return;
8686
}
8787
out_error->code = code;
88-
auto writeField = [](char* dest, std::size_t dest_size, std::string_view src) {
88+
auto write_field = [](char* dest, std::size_t dest_size, std::string_view src) {
8989
if (dest == nullptr || dest_size == 0) {
9090
return;
9191
}
9292
std::size_t n = src.size() < dest_size - 1 ? src.size() : dest_size - 1;
9393
std::memcpy(dest, src.data(), n);
9494
dest[n] = '\0';
9595
};
96-
writeField(out_error->domain, sizeof(out_error->domain), domain);
97-
writeField(out_error->message, sizeof(out_error->message), message);
96+
write_field(out_error->domain, sizeof(out_error->domain), domain);
97+
write_field(out_error->message, sizeof(out_error->message), message);
9898
// Clear the v3.1 growth-path slots so a reused error struct does not
9999
// carry a stale pointer from a previous call. Matches sdk::fillError.
100100
out_error->extended = nullptr;

0 commit comments

Comments
 (0)