refactor(pose_initializer): apply static analysis #7355

YamatoAndo · 2024-06-07T05:29:16Z

Description

Fixed all of the points raised by clang-tidy, cpplint and cppcheck.

Tests performed

Checked with clang-tidy and cppcheck (v2.14.1)

check_linter.sh

#!/bin/bash
set -eux

TARGET_DIR=$1

current_dir=$(basename $(pwd))
if [[ ! $current_dir =~ ^(autoware|pilot-auto) ]]; then
    echo "This script must be run in a directory with a prefix of autoware or pilot-auto."
    exit 1
fi

set +eux
export CPLUS_INCLUDE_PATH=/usr/include/c++/11:/usr/include/x86_64-linux-gnu/c++/11:$CPLUS_INCLUDE_PATH
set -eux

fdfind -e cpp -e hpp --full-path ${TARGET_DIR} | xargs -P $(nproc) -I{} cpplint {}
fdfind -e cpp -e hpp --full-path ${TARGET_DIR} | xargs -P $(nproc) -I{} clang-tidy -p build/ {}

before PR

output from above commands

$ ./check_linter.sh localization/pose_initializer
+ TARGET_DIR=localization/pose_initializer
+++ pwd
++ basename /home/yamato_ando/autoware
+ current_dir=autoware
+ [[ ! autoware =~ ^(autoware|pilot-auto) ]]
+ set +eux
+ fdfind -e cpp -e hpp --full-path localization/pose_initializer
++ nproc
+ xargs -P 20 '-I{}' cpplint '{}'
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/gnss_module.cpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/ndt_localization_trigger_module.hpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/ndt_localization_trigger_module.cpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/gnss_module.hpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/stop_check_module.cpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/ekf_localization_trigger_module.hpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/ndt_module.cpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/ekf_localization_trigger_module.cpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/test/test_copy_vector_to_array.cpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/yabloc_module.cpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/ndt_module.hpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/stop_check_module.hpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/yabloc_module.hpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/copy_vector_to_array.hpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.hpp
Done processing ./src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.cpp
+ fdfind -e cpp -e hpp --full-path localization/pose_initializer
++ nproc
+ xargs -P 20 '-I{}' clang-tidy -p build/ '{}'
3778 warnings generated.
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/copy_vector_to_array.hpp:43:3: warning: uninitialized record type: 'covariance' [cppcoreguidelines-pro-type-member-init]
  std::array<double, 36> covariance;
  ^
                                   {}
Suppressed 3777 warnings (3777 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
8854 warnings generated.
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/test/test_copy_vector_to_array.cpp:22:3: warning: uninitialized record type: 'array' [cppcoreguidelines-pro-type-member-init]
  std::array<int, 5> array;
  ^
                          {}
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/test/test_copy_vector_to_array.cpp:31:3: warning: uninitialized record type: 'array' [cppcoreguidelines-pro-type-member-init]
  std::array<int, 0> array;
  ^
                          {}
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/test/test_copy_vector_to_array.cpp:39:5: warning: uninitialized record type: 'array' [cppcoreguidelines-pro-type-member-init]
    std::array<int, 6> array;
    ^
                            {}
Suppressed 8881 warnings (8851 in non-user code, 30 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13352 warnings generated.
Suppressed 13363 warnings (13352 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13657 warnings generated.
Suppressed 13668 warnings (13657 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13657 warnings generated.
Suppressed 13668 warnings (13657 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13352 warnings generated.
Suppressed 13363 warnings (13352 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13668 warnings generated.
Suppressed 13679 warnings (13668 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13349 warnings generated.
Suppressed 13360 warnings (13349 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13668 warnings generated.
Suppressed 13679 warnings (13668 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13297 warnings generated.
Suppressed 13308 warnings (13297 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13297 warnings generated.
Suppressed 13308 warnings (13297 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
13821 warnings generated.
Suppressed 13832 warnings (13821 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
14338 warnings generated.
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.hpp:34:7: warning: class 'PoseInitializer' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class PoseInitializer : public rclcpp::Node
      ^
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.hpp:38:3: error: annotate this function with 'override' or (rarely) 'final' [modernize-use-override,-warnings-as-errors]
  ~PoseInitializer();
  ^
                     override
Suppressed 14347 warnings (14336 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
1 warning treated as error
16591 warnings and 2 errors generated.
Error while processing /home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.cpp.
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.cpp:29:1: warning: constructor does not initialize these fields: output_pose_covariance_, gnss_particle_covariance_ [cppcoreguidelines-pro-type-member-init]
PoseInitializer::PoseInitializer(const rclcpp::NodeOptions & options)
^
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.cpp:70:7: warning: do not use 'else' after 'throw' [readability-else-after-return]
    } else if (
      ^~~~~~~~~
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.cpp:89:18: error: use '= default' to define a trivial destructor [modernize-use-equals-default,-warnings-as-errors]
PoseInitializer::~PoseInitializer()
                 ^
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.cpp:178:36: error: format string is not a string literal (potentially insecure) [clang-diagnostic-format-security]
        RCLCPP_ERROR(get_logger(), message.str().c_str());
                                   ^
/home/yamato_ando/autoware/install/rclcpp/include/rclcpp/rclcpp/logging.hpp:1407:7: note: expanded from macro 'RCLCPP_ERROR'
      __VA_ARGS__); \
      ^~~~~~~~~~~
/opt/ros/humble/include/rcutils/rcutils/logging_macros.h:997:5: note: expanded from macro 'RCUTILS_LOG_ERROR_NAMED'
    __VA_ARGS__)
    ^~~~~~~~~~~
/opt/ros/humble/include/rcutils/rcutils/logging_macros.h:72:64: note: expanded from macro 'RCUTILS_LOG_COND_NAMED'
      rcutils_log(&__rcutils_logging_location, severity, name, __VA_ARGS__); \
                                                               ^~~~~~~~~~~
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.cpp:178:36: note: treat the string as an argument to avoid this
        RCLCPP_ERROR(get_logger(), message.str().c_str());
                                   ^
                                   "%s", 
/home/yamato_ando/autoware/install/rclcpp/include/rclcpp/rclcpp/logging.hpp:1407:7: note: expanded from macro 'RCLCPP_ERROR'
      __VA_ARGS__); \
      ^
/opt/ros/humble/include/rcutils/rcutils/logging_macros.h:997:5: note: expanded from macro 'RCUTILS_LOG_ERROR_NAMED'
    __VA_ARGS__)
    ^
/opt/ros/humble/include/rcutils/rcutils/logging_macros.h:72:64: note: expanded from macro 'RCUTILS_LOG_COND_NAMED'
      rcutils_log(&__rcutils_logging_location, severity, name, __VA_ARGS__); \
                                                               ^
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.cpp:189:34: error: format string is not a string literal (potentially insecure) [clang-diagnostic-format-security]
      RCLCPP_ERROR(get_logger(), message.str().c_str());
                                 ^
/home/yamato_ando/autoware/install/rclcpp/include/rclcpp/rclcpp/logging.hpp:1407:7: note: expanded from macro 'RCLCPP_ERROR'
      __VA_ARGS__); \
      ^~~~~~~~~~~
/opt/ros/humble/include/rcutils/rcutils/logging_macros.h:997:5: note: expanded from macro 'RCUTILS_LOG_ERROR_NAMED'
    __VA_ARGS__)
    ^~~~~~~~~~~
/opt/ros/humble/include/rcutils/rcutils/logging_macros.h:72:64: note: expanded from macro 'RCUTILS_LOG_COND_NAMED'
      rcutils_log(&__rcutils_logging_location, severity, name, __VA_ARGS__); \
                                                               ^~~~~~~~~~~
/home/yamato_ando/autoware/src/universe/autoware.universe/localization/pose_initializer/src/pose_initializer/pose_initializer_core.cpp:189:34: note: treat the string as an argument to avoid this
      RCLCPP_ERROR(get_logger(), message.str().c_str());
                                 ^
                                 "%s", 
/home/yamato_ando/autoware/install/rclcpp/include/rclcpp/rclcpp/logging.hpp:1407:7: note: expanded from macro 'RCLCPP_ERROR'
      __VA_ARGS__); \
      ^
/opt/ros/humble/include/rcutils/rcutils/logging_macros.h:997:5: note: expanded from macro 'RCUTILS_LOG_ERROR_NAMED'
    __VA_ARGS__)
    ^
/opt/ros/humble/include/rcutils/rcutils/logging_macros.h:72:64: note: expanded from macro 'RCUTILS_LOG_COND_NAMED'
      rcutils_log(&__rcutils_logging_location, severity, name, __VA_ARGS__); \
                                                               ^
Suppressed 16599 warnings (16588 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
1 warning treated as error
16019 warnings generated.
Suppressed 16030 warnings (16019 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
16343 warnings generated.
Suppressed 16354 warnings (16343 in non-user code, 11 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

output from cppcheck

$ cppcheck --enable=warning --enable=style --enable=performance --enable=portability --enable=unusedFunction --inconclusive --check-level=exhaustive .
Checking src/pose_initializer/ekf_localization_trigger_module.cpp ...
1/8 files checked 11% done
Checking src/pose_initializer/gnss_module.cpp ...
src/pose_initializer/gnss_module.cpp:25:81: performance: Variable 'pose_' is assigned in constructor body. Consider performing initialization in initialization list. [useInitializationList]
    "gnss_pose_cov", 1, [this](PoseWithCovarianceStamped::ConstSharedPtr msg) { pose_ = msg; });
                                                                                ^
src/pose_initializer/gnss_module.cpp:27:3: performance: Variable 'clock_' is assigned in constructor body. Consider performing initialization in initialization list. [useInitializationList]
  clock_ = node->get_clock();
  ^
2/8 files checked 19% done
Checking src/pose_initializer/ndt_localization_trigger_module.cpp ...
3/8 files checked 30% done
Checking src/pose_initializer/ndt_module.cpp ...
src/pose_initializer/ndt_module.cpp:28:3: performance: Variable 'cli_align_' is assigned in constructor body. Consider performing initialization in initialization list. [useInitializationList]
  cli_align_ = node->create_client<RequestPoseAlignment>("ndt_align");
  ^
4/8 files checked 39% done
Checking src/pose_initializer/pose_initializer_core.cpp ...
5/8 files checked 78% done
Checking src/pose_initializer/stop_check_module.cpp ...
6/8 files checked 83% done
Checking src/pose_initializer/yabloc_module.cpp ...
src/pose_initializer/yabloc_module.cpp:28:3: performance: Variable 'cli_align_' is assigned in constructor body. Consider performing initialization in initialization list. [useInitializationList]
  cli_align_ = node->create_client<RequestPoseAlignment>("yabloc_align");
  ^
7/8 files checked 92% done
Checking test/test_copy_vector_to_array.cpp ...
8/8 files checked 100% done

after PR

clear all warnings

Effects on system behavior

Not applicable.

Interface changes

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

I've confirmed the contribution guidelines.
The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

The PR follows the pull request guidelines.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: Yamato Ando <yamato.ando@tier4.jp>

SakodaShintaro

I have confirmed that logging_simulator works well.
Looks Good To Me

SakodaShintaro · 2024-06-10T07:47:12Z

I just merged it into the latest autoware:main locally and confirmed that logging_simulator works. I will merge it.

* apply clang-tidy and cpplint Signed-off-by: Yamato Ando <yamato.ando@tier4.jp> * apply cppcheck Signed-off-by: Yamato Ando <yamato.ando@tier4.jp> * style(pre-commit): autofix --------- Signed-off-by: Yamato Ando <yamato.ando@tier4.jp> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

YamatoAndo added 2 commits June 7, 2024 14:09

apply clang-tidy and cpplint

cffb289

Signed-off-by: Yamato Ando <yamato.ando@tier4.jp>

apply cppcheck

6d85ba9

Signed-off-by: Yamato Ando <yamato.ando@tier4.jp>

YamatoAndo requested review from anhnv3991, isamu-takagi, KYabuuchi, Motsu-san, RyuYamamoto, SakodaShintaro and TaikiYamada4 as code owners June 7, 2024 05:29

github-actions bot added the component:localization Vehicle's position determination in its environment. (auto-assigned) label Jun 7, 2024

style(pre-commit): autofix

cb2ba79

SakodaShintaro approved these changes Jun 7, 2024

View reviewed changes

SakodaShintaro added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jun 7, 2024

SakodaShintaro merged commit badc3e7 into autowarefoundation:main Jun 10, 2024
32 of 33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(pose_initializer): apply static analysis #7355

refactor(pose_initializer): apply static analysis #7355

YamatoAndo commented Jun 7, 2024

SakodaShintaro left a comment

SakodaShintaro commented Jun 10, 2024

refactor(pose_initializer): apply static analysis #7355

refactor(pose_initializer): apply static analysis #7355

Conversation

YamatoAndo commented Jun 7, 2024

Description

Tests performed

Effects on system behavior

Interface changes

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

SakodaShintaro left a comment

Choose a reason for hiding this comment

SakodaShintaro commented Jun 10, 2024