Skip to content

Commit 403d2bd

Browse files
isidorowiczdevisidorowiczdev
authored
fix: address clang-tidy 17 errors (#1723)
* fix: address clang-tidy 17 errors CI enables clang-tidy with -warnings-as-errors, and three checks were failing the build: - clang-analyzer-cplusplus.NewDeleteLeaks (24+ sites across TokenEndpointImpl, IndexLayerClientImpl, VolatileLayerClientImpl, VersionedLayerClientImpl, StreamLayerClientImpl): Every flagged call path was traced end-to-end and contains zero raw new/delete; ownership is shared_ptr/std::function throughout, so each site is suppressed with NOLINTNEXTLINE - clang-analyzer-deadcode.DeadStores (Crypto.cpp:171): ComputeSha256 mutated `value` via `>>=` when packing the last output byte, a redundant shift whose result was never read; changed to non-mutating `>>`. - clang-analyzer-optin.cplusplus.VirtualCall (SignInResultImpl.cpp:95): the constructor called virtual IsValid() instead of reading is_valid_ directly Resolves: DATASDK-104 Signed-off-by: Iwo Sidorowicz <iwo.sidorowicz@here.com> * Minor: add clang-tidy job Add clang-tidy 17 job to the pipeline Resolves: DATASDK-104 Signed-off-by: Iwo Sidorowicz <iwo.sidorowicz@here.com> * Minor: chmod fix Chmod fix for clang-tidy script Resolves: DATASDK-104 Signed-off-by: Iwo Sidorowicz <iwo.sidorowicz@here.com> * Minor: reposition suppresion lines Move each comment directly above the `[=](...)`. Resolves: DATASDK-104 Signed-off-by: Iwo Sidorowicz <iwo.sidorowicz@here.com> * Minor: revert formatting Revert formatting Resolves: DATASDK-104 Signed-off-by: Iwo Sidorowicz <iwo.sidorowicz@here.com> * Minor: add copyrights Add copyrights to the changed files. Resolves: DATASDK-104 Signed-off-by: Iwo Sidorowicz <iwo.sidorowicz@here.com> --------- Signed-off-by: Iwo Sidorowicz <iwo.sidorowicz@here.com>
1 parent 2ce7d25 commit 403d2bd

10 files changed

Lines changed: 140 additions & 7 deletions

File tree

‎.clang-tidy‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Checks: 'boost-*,bugprone-*,clang-diagnostic*,cppcoreguidelines-*,modernize-*,misc-*,performance-*,readability-*,-bugprone-easily-swappable-parameters,-cppcoreguidelines-avoid-do-while,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-vararg,-modernize-use-trailing-return-type,-misc-include-cleaner,-misc-non-private-member-variables-in-classes'
2+
WarningsAsErrors: "*"
3+
HeaderFilterRegex: '.*\/(olp-cpp-sdk-core|olp-cpp-sdk-authentication|olp-cpp-sdk-dataservice-read|olp-cpp-sdk-dataservice-write)\/.*'
4+
FormatStyle: "file"
5+
CheckOptions:
6+
- key: readability-function-cognitive-complexity.IgnoreMacros
7+
value: true
8+
- key: readability-identifier-naming.ClassCase
9+
value: CamelCase
10+
- key: readability-identifier-naming.MethodCase
11+
value: CamelCase
12+
- key: readability-identifier-naming.MemberCase
13+
value: lower_case
14+
- key: readability-identifier-naming.PrivateMemberSuffix
15+
value: _
16+
- key: readability-identifier-naming.ProtectedMemberSuffix
17+
value: _
18+
- key: readability-identifier-naming.FunctionCase
19+
value: CamelCase
20+
- key: readability-identifier-naming.ConstexprVariableCase
21+
value: CamelCase
22+
- key: readability-identifier-naming.ConstexprVariablePrefix
23+
value: k
24+
- key: readability-identifier-naming.StaticConstantCase
25+
value: CamelCase
26+
- key: readability-identifier-naming.StaticConstantPrefix
27+
value: k
28+
- key: readability-identifier-naming.GlobalConstantCase
29+
value: CamelCase
30+
- key: readability-identifier-naming.GlobalConstantPrefix
31+
value: k
32+
- key: readability-identifier-naming.EnumConstantCase
33+
value: CamelCase
34+
- key: readability-identifier-naming.EnumConstantPrefix
35+
value: k
36+
- key: readability-identifier-naming.ParameterCase
37+
value: lower_case
38+
- key: readability-identifier-naming.VariableCase
39+
value: lower_case

‎.github/workflows/psv_pipelines.yml‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,24 @@ jobs:
2626
run: ./scripts/misc/cpplint_ci.sh
2727
shell: bash
2828

29+
psv-linux-24-04-clang17-build-clang-tidy:
30+
name: PSV.Linux.24.04.clang17.ClangTidy
31+
if: github.event_name == 'pull_request'
32+
runs-on: ubuntu-24.04
33+
steps:
34+
- name: Check out repository
35+
uses: actions/checkout@v7
36+
- name: Install dependencies
37+
run: |
38+
sudo apt-get update
39+
sudo apt-get install -y \
40+
libboost-all-dev \
41+
libcurl4-openssl-dev
42+
shell: bash
43+
- name: Run clang-tidy
44+
run: ./scripts/misc/clang-tidy-17-check.sh
45+
shell: bash
46+
2947
psv-linux-22-04-gcc9-build-test-codecov:
3048
name: PSV.Linux.22.04.gcc9.Tests.CodeCov
3149
runs-on: ubuntu-22.04
@@ -377,4 +395,4 @@ jobs:
377395
echo "Then run: git apply $CLANG_FORMAT_FILE"
378396
exit 1
379397
fi
380-
shell: bash
398+
shell: bash

‎olp-cpp-sdk-authentication/src/Crypto.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2023 HERE Europe B.V.
2+
* Copyright (C) 2019-2026 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -168,7 +168,7 @@ Crypto::Sha256Digest ComputeSha256(const std::vector<unsigned char>& src) {
168168
auto v3 = (unsigned char)value;
169169
auto v2 = (unsigned char)(value >>= 8);
170170
auto v1 = (unsigned char)(value >>= 8);
171-
ret[j + 0] = (unsigned char)(value >>= 8);
171+
ret[j + 0] = (unsigned char)(value >> 8);
172172
ret[j + 1] = v1;
173173
ret[j + 2] = v2;
174174
ret[j + 3] = v3;

‎olp-cpp-sdk-authentication/src/SignInResultImpl.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ SignInResultImpl::SignInResultImpl(
9292

9393
// Extra response data if no errors reported
9494
if (!HasError()) {
95-
if (!IsValid()) {
95+
if (!is_valid_) {
9696
status_ = http::HttpStatusCode::SERVICE_UNAVAILABLE;
9797
error_.message = Constants::ERROR_HTTP_SERVICE_UNAVAILABLE;
9898
} else {

‎olp-cpp-sdk-authentication/src/TokenEndpointImpl.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ client::CancellationToken TokenEndpointImpl::RequestToken(
192192
properties.scope = scope_;
193193
return auth_client_.SignInClient(
194194
credentials_, properties,
195+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
195196
[callback](
196197
const AuthenticationClient::SignInClientResponse& sign_in_response) {
197198
if (!sign_in_response) {

‎olp-cpp-sdk-dataservice-write/src/IndexLayerClientImpl.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2024 HERE Europe B.V.
2+
* Copyright (C) 2019-2026 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -242,6 +242,7 @@ client::CancellationToken IndexLayerClientImpl::DeleteIndexData(
242242
auto cancel_context = std::make_shared<client::CancellationContext>();
243243
auto self = shared_from_this();
244244

245+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
245246
auto cancel_function = [=]() {
246247
self->tokenList_.RemoveTask(op_id);
247248
callback(DeleteIndexDataResponse(client::ApiError(
@@ -302,12 +303,14 @@ client::CancellationToken IndexLayerClientImpl::UpdateIndex(
302303
auto self = shared_from_this();
303304

304305
auto op_id = tokenList_.GetNextId();
306+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
305307
auto cancel_function = [=]() {
306308
self->tokenList_.RemoveTask(op_id);
307309
callback(UpdateIndexResponse(client::ApiError(
308310
client::ErrorCode::Cancelled, "Operation cancelled.", true)));
309311
};
310312

313+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
311314
auto updateIndex_callback = [=](UpdateIndexResponse update_index_response) {
312315
self->tokenList_.RemoveTask(op_id);
313316
if (!update_index_response.IsSuccessful()) {

‎olp-cpp-sdk-dataservice-write/src/StreamLayerClientImpl.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ olp::client::CancellationToken StreamLayerClientImpl::Flush(
214214
// invocation: one during execution phase and other when `Flush` is cancelled.
215215
auto exec_started = std::make_shared<std::atomic_bool>(false);
216216

217+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
217218
auto task_context = client::TaskContext::Create(
218219
[=](client::CancellationContext context) -> EmptyFlushApiResponse {
219220
exec_started->exchange(true);
@@ -249,6 +250,7 @@ olp::client::CancellationToken StreamLayerClientImpl::Flush(
249250
callback(responses);
250251
return EmptyFlushApiResponse{};
251252
},
253+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
252254
[=](EmptyFlushApiResponse /*response*/) {
253255
// we don't need to notify user 2 times, cause we already invoke a
254256
// callback in the execution function:

‎olp-cpp-sdk-dataservice-write/src/VersionedLayerClientImpl.cpp‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2021 HERE Europe B.V.
2+
* Copyright (C) 2019-2026 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -224,13 +224,15 @@ olp::client::CancellationToken VersionedLayerClientImpl::GetBaseVersion(
224224
auto cancel_context = std::make_shared<client::CancellationContext>();
225225
auto id = tokenList_.GetNextId();
226226

227+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
227228
auto cancel_function = [=]() {
228229
self->tokenList_.RemoveTask(id);
229230
callback(client::ApiError(client::ErrorCode::Cancelled,
230231
"Operation cancelled.", true));
231232
};
232233

233234
auto getBaseVersion_callback =
235+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
234236
[=](MetadataApi::CatalogVersionResponse response) {
235237
self->tokenList_.RemoveTask(id);
236238
if (!response.IsSuccessful()) {
@@ -247,12 +249,14 @@ olp::client::CancellationToken VersionedLayerClientImpl::GetBaseVersion(
247249
}
248250
};
249251

252+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
250253
auto getBaseVersion_function = [=]() -> client::CancellationToken {
251254
return MetadataApi::GetLatestCatalogVersion(*self->apiclient_metadata_, -1,
252255
olp::porting::none,
253256
getBaseVersion_callback);
254257
};
255258

259+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
256260
cancel_context->ExecuteOrCancelled(
257261
[=]() -> client::CancellationToken {
258262
return self->InitApiClients(
@@ -298,13 +302,15 @@ olp::client::CancellationToken VersionedLayerClientImpl::GetBatch(
298302
auto cancel_context = std::make_shared<client::CancellationContext>();
299303
auto id = tokenList_.GetNextId();
300304

305+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
301306
auto cancel_function = [=]() {
302307
self->tokenList_.RemoveTask(id);
303308
callback(client::ApiError(client::ErrorCode::Cancelled,
304309
"Operation cancelled.", true));
305310
};
306311

307312
auto getPublication_callback =
313+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
308314
[=](GetPublicationResponse getPublicationResponse) {
309315
self->tokenList_.RemoveTask(id);
310316
if (!getPublicationResponse.IsSuccessful()) {
@@ -314,12 +320,14 @@ olp::client::CancellationToken VersionedLayerClientImpl::GetBatch(
314320
}
315321
};
316322

323+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
317324
auto getPublication_function = [=]() -> client::CancellationToken {
318325
return PublishApi::GetPublication(*self->apiclient_publish_, publicationId,
319326
olp::porting::none,
320327
getPublication_callback);
321328
};
322329

330+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
323331
cancel_context->ExecuteOrCancelled(
324332
[=]() -> client::CancellationToken {
325333
return self->InitApiClients(
@@ -567,6 +575,7 @@ client::CancellationToken VersionedLayerClientImpl::CheckDataExists(
567575
auto id = tokenList_.GetNextId();
568576

569577
auto check_data_exists_callback =
578+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
570579
[=](CheckDataExistsResponse check_data_exists_response) {
571580
self->tokenList_.RemoveTask(id);
572581
if (!check_data_exists_response.IsSuccessful()) {
@@ -576,17 +585,20 @@ client::CancellationToken VersionedLayerClientImpl::CheckDataExists(
576585
}
577586
};
578587

588+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
579589
auto check_data_exists_function = [=]() -> client::CancellationToken {
580590
return BlobApi::checkBlobExists(*self->apiclient_blob_, layer_id,
581591
data_handle, olp::porting::none,
582592
check_data_exists_callback);
583593
};
584594

595+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
585596
auto cancel_function = [callback]() {
586597
callback(client::ApiError(client::ErrorCode::Cancelled,
587598
"Operation cancelled.", true));
588599
};
589600

601+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
590602
cancel_context->ExecuteOrCancelled(
591603
[=]() -> client::CancellationToken {
592604
return self->InitApiClients(

‎olp-cpp-sdk-dataservice-write/src/VolatileLayerClientImpl.cpp‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2021 HERE Europe B.V.
2+
* Copyright (C) 2019-2026 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -193,6 +193,7 @@ client::CancellationToken VolatileLayerClientImpl::GetBaseVersion(
193193
};
194194

195195
auto getBaseVersion_callback =
196+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
196197
[=](MetadataApi::CatalogVersionResponse response) {
197198
self->tokenList_.RemoveTask(id);
198199
if (!response.IsSuccessful()) {
@@ -209,12 +210,14 @@ client::CancellationToken VolatileLayerClientImpl::GetBaseVersion(
209210
}
210211
};
211212

213+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
212214
auto getBaseVersion_function = [=]() -> client::CancellationToken {
213215
return MetadataApi::GetLatestCatalogVersion(*self->apiclient_metadata_, -1,
214216
olp::porting::none,
215217
getBaseVersion_callback);
216218
};
217219

220+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
218221
cancel_context->ExecuteOrCancelled(
219222
[=]() -> client::CancellationToken {
220223
return self->InitApiClients(
@@ -264,6 +267,7 @@ client::CancellationToken VolatileLayerClientImpl::StartBatch(
264267
}
265268
};
266269

270+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
267271
auto init_publication_function = [=]() -> client::CancellationToken {
268272
model::Publication pub;
269273
pub.SetLayerIds(request.GetLayers().value_or(std::vector<std::string>()));
@@ -275,12 +279,14 @@ client::CancellationToken VolatileLayerClientImpl::StartBatch(
275279
init_publication_callback);
276280
};
277281

282+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
278283
auto cancel_function = [=]() {
279284
self->tokenList_.RemoveTask(id);
280285
callback(client::ApiError(client::ErrorCode::Cancelled,
281286
"Operation cancelled.", true));
282287
};
283288

289+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
284290
cancel_context->ExecuteOrCancelled(
285291
[=]() -> client::CancellationToken {
286292
return self->InitApiClients(
@@ -426,6 +432,7 @@ client::CancellationToken VolatileLayerClientImpl::GetBatch(
426432
};
427433

428434
auto getPublication_callback =
435+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
429436
[=](GetPublicationResponse getPublicationResponse) {
430437
self->tokenList_.RemoveTask(id);
431438
if (!getPublicationResponse.IsSuccessful()) {
@@ -435,12 +442,14 @@ client::CancellationToken VolatileLayerClientImpl::GetBatch(
435442
}
436443
};
437444

445+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
438446
auto getPublication_function = [=]() -> client::CancellationToken {
439447
return PublishApi::GetPublication(*self->apiclient_publish_, publicationId,
440448
olp::porting::none,
441449
getPublication_callback);
442450
};
443451

452+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
444453
cancel_context->ExecuteOrCancelled(
445454
[=]() -> client::CancellationToken {
446455
return self->InitApiClients(
@@ -572,13 +581,15 @@ client::CancellationToken VolatileLayerClientImpl::PublishToBatch(
572581
auto self = shared_from_this();
573582
auto id = tokenList_.GetNextId();
574583

584+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
575585
auto cancel_function = [=]() {
576586
self->tokenList_.RemoveTask(id);
577587
callback(client::ApiError(client::ErrorCode::Cancelled,
578588
"Operation cancelled.", true));
579589
};
580590

581591
auto upload_partitions_callback =
592+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
582593
[=](UploadPartitionsResponse upload_partitions_response) {
583594
self->tokenList_.RemoveTask(id);
584595
if (!upload_partitions_response.IsSuccessful()) {
@@ -588,6 +599,7 @@ client::CancellationToken VolatileLayerClientImpl::PublishToBatch(
588599
}
589600
};
590601

602+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
591603
auto upload_partitions_function = [=]() -> client::CancellationToken {
592604
std::vector<model::PublishPartition> pub_partition_list;
593605
for (const auto& partition_request : partitions) {
@@ -610,6 +622,7 @@ client::CancellationToken VolatileLayerClientImpl::PublishToBatch(
610622
upload_partitions_callback);
611623
};
612624

625+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
613626
cancel_context->ExecuteOrCancelled(
614627
[=]() -> client::CancellationToken {
615628
return self->InitApiClients(
@@ -654,13 +667,15 @@ client::CancellationToken VolatileLayerClientImpl::CompleteBatch(
654667
auto self = shared_from_this();
655668
auto cancel_context = std::make_shared<client::CancellationContext>();
656669
auto id = tokenList_.GetNextId();
670+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
657671
auto cancel_function = [=]() {
658672
self->tokenList_.RemoveTask(id);
659673
callback(client::ApiError(client::ErrorCode::Cancelled,
660674
"Operation cancelled.", true));
661675
};
662676

663677
auto completePublication_callback =
678+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
664679
[=](SubmitPublicationResponse submitPublicationResponse) {
665680
self->tokenList_.RemoveTask(id);
666681
if (!submitPublicationResponse.IsSuccessful()) {
@@ -670,12 +685,14 @@ client::CancellationToken VolatileLayerClientImpl::CompleteBatch(
670685
}
671686
};
672687

688+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
673689
auto completePublication_function = [=]() -> client::CancellationToken {
674690
return PublishApi::SubmitPublication(*self->apiclient_publish_,
675691
publicationId, olp::porting::none,
676692
completePublication_callback);
677693
};
678694

695+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): false
679696
cancel_context->ExecuteOrCancelled(
680697
[=]() -> client::CancellationToken {
681698
return self->InitApiClients(

0 commit comments

Comments
 (0)