diff --git a/.clang-tidy b/.clang-tidy index de4303bf8db..9581f45b814 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,4 +1,85 @@ --- -Checks: '-clang-analyzer-*,google-*,llvm-*,misc-*,readability-*,-google-build-explicit-make-pair,-google-explicit-constructor,-google-readability-braces-around-statements,-google-readability-casting,-google-readability-namespace-comments,-google-readability-function,-google-readability-todo,-google-runtime-int,-llvm-namespace-comment,-llvm-header-guard,-llvm-twine-local,-misc-argument-comment,-readability-braces-around-statements,-readability-identifier-naming' -... +Checks: > + bugprone-*, + -bugprone-narrowing-conversions, + -bugprone-easily-swappable-parameters, + -bugprone-branch-clone, + -bugprone-misplaced-widening-cast, + -bugprone-exception-escape, + -bugprone-implicit-widening-of-multiplication-result, + -bugprone-integer-division, + -bugprone-reserved-identifier, + -bugprone-macro-parentheses, + -bugprone-unhandled-self-assignment, + -bugprone-suspicious-missing-comma, + -bugprone-forward-declaration-namespace, + -bugprone-sizeof-expression, + -clang-analyzer-*, + -clang-diagnostic-unused-local-typedef, + -clang-diagnostic-deprecated-declarations, + google-*, + -google-build-explicit-make-pair, + -google-build-using-namespace, + -google-explicit-constructor, + -google-default-arguments, + -google-readability-braces-around-statements, + -google-readability-casting, + -google-readability-namespace-comments, + -google-readability-function, + -google-readability-todo, + -google-runtime-int, + -google-build-namespaces, + -google-global-names-in-headers, + -google-runtime-references, + -google-readability-function-size, + llvm-*, + -llvm-namespace-comment, + -llvm-qualified-auto, + -llvm-include-order, + -llvm-else-after-return, + -llvm-header-guard, + -llvm-twine-local, + misc-*, + -misc-argument-comment, + -misc-non-private-member-variables-in-classes, + -misc-unused-using-decls, + -misc-unconventional-assign-operator, + -misc-redundant-expression, + -misc-no-recursion, + -misc-misplaced-const, + -misc-definitions-in-headers, + -misc-unused-alias-decls, + -misc-unused-parameters, + readability-*, + -readability-avoid-const-params-in-decls, + -readability-braces-around-statements, + -readability-container-size-empty, + -readability-convert-member-functions-to-static, + -readability-const-return-type, + -readability-function-cognitive-complexity, + -readability-function-size, + -readability-identifier-naming, + -readability-implicit-bool-conversion, + -readability-magic-numbers, + -readability-else-after-return, + -readability-inconsistent-declaration-parameter-name, + -readability-isolate-declaration, + -readability-redundant-declaration, + -readability-uppercase-literal-suffix, + -readability-named-parameter, + -readability-qualified-auto, + -readability-suspicious-call-argument, + -readability-redundant-access-specifiers, + -readability-redundant-member-init, + -readability-static-definition-in-anonymous-namespace, + -readability-use-anyofallof, + -readability-simplify-boolean-expr, + -readability-make-member-function-const, + -readability-redundant-string-init, + -readability-non-const-parameter, + -readability-static-accessed-through-instance + +WarningsAsErrors: '*' +HeaderFilterRegex: '.*' + diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 1d5fbec2d70..5643bb9b790 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -9,7 +9,7 @@ If your PR is still work in progress please attach the relevant label. - [ ] CHANGELOG.md entry ([How to write a changelog entry](http://keepachangelog.com/en/1.0.0/#how)) - [ ] update relevant [Wiki pages](https://github.com/Project-OSRM/osrm-backend/wiki) - - [ ] add tests (see [testing documentation](https://github.com/Project-OSRM/osrm-backend/blob/master/docs/testing.md) + - [ ] add tests (see [testing documentation](https://github.com/Project-OSRM/osrm-backend/blob/master/docs/testing.md)) - [ ] review - [ ] adjust for comments - [ ] cherry pick to release branch diff --git a/.github/workflows/osrm-backend.yml b/.github/workflows/osrm-backend.yml index 2ca8f365a53..5bdac011bf1 100644 --- a/.github/workflows/osrm-backend.yml +++ b/.github/workflows/osrm-backend.yml @@ -85,6 +85,16 @@ jobs: CLANG_VERSION: 5.0.0 CUCUMBER_TIMEOUT: 60000 + - name: clang-11.0-debug-clang-tidy + continue-on-error: false + node: 12 + runs-on: ubuntu-20.04 + BUILD_TOOLS: ON + BUILD_TYPE: Debug + CLANG_VERSION: 11.0.0 + CUCUMBER_TIMEOUT: 60000 + ENABLE_CLANG_TIDY: ON + - name: mason-linux-debug-asan continue-on-error: false node: 12 @@ -365,6 +375,7 @@ jobs: CXXCOMPILER: ${{ matrix.CXXCOMPILER }} CXXFLAGS: ${{ matrix.CXXFLAGS }} ENABLE_ASSERTIONS: ${{ matrix.ENABLE_ASSERTIONS }} + ENABLE_CLANG_TIDY: ${{ matrix.ENABLE_CLANG_TIDY }} ENABLE_COVERAGE: ${{ matrix.ENABLE_COVERAGE }} ENABLE_GLIBC_WORKAROUND: ${{ matrix.ENABLE_GLIBC_WORKAROUND }} ENABLE_MASON: ${{ matrix.ENABLE_MASON }} @@ -490,7 +501,8 @@ jobs: pushd ${OSRM_BUILD_DIR} cmake .. -DCMAKE_BUILD_TYPE=${BUILD_TYPE} \ -DENABLE_MASON=${ENABLE_MASON:-OFF} \ - -DENABLE_ASSERTIONS=${ENABLE_ASSERTIONS:-OFF}} \ + -DENABLE_ASSERTIONS=${ENABLE_ASSERTIONS:-OFF} \ + -DENABLE_CLANG_TIDY=${ENABLE_CLANG_TIDY:-OFF} \ -DBUILD_SHARED_LIBS=${BUILD_SHARED_LIBS:-OFF} \ -DENABLE_COVERAGE=${ENABLE_COVERAGE:-OFF} \ -DENABLE_NODE_BINDINGS=${ENABLE_NODE_BINDINGS:-OFF} \ diff --git a/CHANGELOG.md b/CHANGELOG.md index 7081b636e57..aeef18c7bcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ - API: - FIXED: Fix inefficient osrm-routed connection handling [#6113](https://github.com/Project-OSRM/osrm-backend/pull/6113) - Build: + - CHANGED: Configure clang-tidy job on CI. [#6261](https://github.com/Project-OSRM/osrm-backend/pull/6261) - CHANGED: Use Github Actions for building container images [#6138](https://github.com/Project-OSRM/osrm-backend/pull/6138) - CHANGED: Upgrade Boost dependency to 1.70 [#6113](https://github.com/Project-OSRM/osrm-backend/pull/6113) - CHANGED: Upgrade Ubuntu CI builds to 20.04 [#6119](https://github.com/Project-OSRM/osrm-backend/pull/6119) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3365de910d7..bbfab458952 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,6 +32,18 @@ option(ENABLE_FUZZING "Fuzz testing using LLVM's libFuzzer" OFF) option(ENABLE_GOLD_LINKER "Use GNU gold linker if available" ON) option(ENABLE_NODE_BINDINGS "Build NodeJs bindings" OFF) option(ENABLE_GLIBC_WORKAROUND "Workaround GLIBC symbol exports" OFF) +option(ENABLE_CLANG_TIDY "Enables clang-tidy checks" OFF) + +if (ENABLE_CLANG_TIDY) + find_program(CLANG_TIDY_COMMAND NAMES clang-tidy) + if(NOT CLANG_TIDY_COMMAND) + message(FATAL_ERROR "ENABLE_CLANG_TIDY is ON but clang-tidy is not found!") + else() + message(STATUS "Found clang-tidy at ${CLANG_TIDY_COMMAND}") + set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND};--warnings-as-errors=*") + endif() +endif() + list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") @@ -449,7 +461,7 @@ include_directories(SYSTEM ${VTZERO_INCLUDE_DIR}) set(FLATBUFFERS_SRC_DIR "${CMAKE_CURRENT_SOURCE_DIR}/third_party/flatbuffers") set(FLATBUFFERS_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/third_party/flatbuffers/include") -include_directories(${FLATBUFFERS_INCLUDE_DIR}) +include_directories(SYSTEM ${FLATBUFFERS_INCLUDE_DIR}) add_subdirectory(${FLATBUFFERS_SRC_DIR} ${CMAKE_CURRENT_BINARY_DIR}/flatbuffers-build EXCLUDE_FROM_ALL) diff --git a/include/engine/datafacade/mmap_memory_allocator.hpp b/include/engine/datafacade/mmap_memory_allocator.hpp index fc4b4e24adf..817fcf9db04 100644 --- a/include/engine/datafacade/mmap_memory_allocator.hpp +++ b/include/engine/datafacade/mmap_memory_allocator.hpp @@ -22,7 +22,7 @@ namespace datafacade /** * This allocator uses file backed mmap memory block as the data location. */ -class MMapMemoryAllocator : public ContiguousBlockAllocator +class MMapMemoryAllocator final : public ContiguousBlockAllocator { public: explicit MMapMemoryAllocator(const storage::StorageConfig &config); diff --git a/include/engine/datafacade/process_memory_allocator.hpp b/include/engine/datafacade/process_memory_allocator.hpp index 0dd555e2696..742f20e36d8 100644 --- a/include/engine/datafacade/process_memory_allocator.hpp +++ b/include/engine/datafacade/process_memory_allocator.hpp @@ -20,7 +20,7 @@ namespace datafacade * This class holds a unique_ptr to the memory block, so it * is auto-freed upon destruction. */ -class ProcessMemoryAllocator : public ContiguousBlockAllocator +class ProcessMemoryAllocator final : public ContiguousBlockAllocator { public: explicit ProcessMemoryAllocator(const storage::StorageConfig &config); diff --git a/include/engine/datafacade/shared_memory_allocator.hpp b/include/engine/datafacade/shared_memory_allocator.hpp index 00b37ac7fd5..5b0a0989a67 100644 --- a/include/engine/datafacade/shared_memory_allocator.hpp +++ b/include/engine/datafacade/shared_memory_allocator.hpp @@ -20,7 +20,7 @@ namespace datafacade * Many SharedMemoryDataFacade objects can be created that point to the same shared * memory block. */ -class SharedMemoryAllocator : public ContiguousBlockAllocator +class SharedMemoryAllocator final : public ContiguousBlockAllocator { public: explicit SharedMemoryAllocator( diff --git a/include/engine/geospatial_query.hpp b/include/engine/geospatial_query.hpp index cb7dc7d58ba..a8f4bb75ffe 100644 --- a/include/engine/geospatial_query.hpp +++ b/include/engine/geospatial_query.hpp @@ -467,11 +467,13 @@ template class GeospatialQuery const auto forward_geometry = datafacade.GetUncompressedForwardGeometry(geometry_id); const auto forward_weight_offset = + // NOLINTNEXTLINE(bugprone-fold-init-type) std::accumulate(forward_weights.begin(), forward_weights.begin() + data.fwd_segment_position, EdgeWeight{0}); const auto forward_duration_offset = + // NOLINTNEXTLINE(bugprone-fold-init-type) std::accumulate(forward_durations.begin(), forward_durations.begin() + data.fwd_segment_position, EdgeDuration{0}); diff --git a/include/engine/routing_algorithms/shortest_path_impl.hpp b/include/engine/routing_algorithms/shortest_path_impl.hpp index 05a817a1e55..304f5ee5e5d 100644 --- a/include/engine/routing_algorithms/shortest_path_impl.hpp +++ b/include/engine/routing_algorithms/shortest_path_impl.hpp @@ -304,6 +304,7 @@ InternalRouteResult shortestPathSearch(SearchEngineData &engine_worki { BOOST_ASSERT(target_phantom.IsValidReverseTarget()); new_total_weight_to_reverse = new_total_weight_to_forward; + // NOLINTNEXTLINE(bugprone-use-after-move) packed_leg_to_reverse = std::move(packed_leg_to_forward); new_total_weight_to_forward = INVALID_EDGE_WEIGHT; @@ -354,6 +355,7 @@ InternalRouteResult shortestPathSearch(SearchEngineData &engine_worki { bool forward_to_forward = (new_total_weight_to_forward != INVALID_EDGE_WEIGHT) && + // NOLINTNEXTLINE(bugprone-use-after-move) packed_leg_to_forward.front() == source_phantom.forward_segment_id.id; bool reverse_to_forward = (new_total_weight_to_forward != INVALID_EDGE_WEIGHT) && diff --git a/include/extractor/intersection_bearings_container.hpp b/include/extractor/intersection_bearings_container.hpp index 924ea456238..40746dcf095 100644 --- a/include/extractor/intersection_bearings_container.hpp +++ b/include/extractor/intersection_bearings_container.hpp @@ -55,6 +55,7 @@ template class IntersectionBearingsContainer bearing_classes.end(), bearing_counts.begin(), [](const auto &bearings) { return bearings.getAvailableBearings().size(); }); + // NOLINTNEXTLINE(bugprone-fold-init-type) auto total_bearings = std::accumulate(bearing_counts.begin(), bearing_counts.end(), 0); class_id_to_ranges_table = RangeTable<16>{bearing_counts}; diff --git a/include/guidance/motorway_handler.hpp b/include/guidance/motorway_handler.hpp index 5ecac81705c..05f1c163032 100644 --- a/include/guidance/motorway_handler.hpp +++ b/include/guidance/motorway_handler.hpp @@ -18,7 +18,7 @@ namespace guidance { // Intersection handlers deal with all issues related to intersections. -class MotorwayHandler : public IntersectionHandler +class MotorwayHandler final : public IntersectionHandler { public: MotorwayHandler(const util::NodeBasedDynamicGraph &node_based_graph, diff --git a/include/guidance/roundabout_handler.hpp b/include/guidance/roundabout_handler.hpp index 2d2fe86d0e6..55def24e8d1 100644 --- a/include/guidance/roundabout_handler.hpp +++ b/include/guidance/roundabout_handler.hpp @@ -35,7 +35,7 @@ struct RoundaboutFlags // The roundabout handler processes all roundabout related instructions. // It performs both the distinction between rotaries and roundabouts and // assigns appropriate entry/exit instructions. -class RoundaboutHandler : public IntersectionHandler +class RoundaboutHandler final : public IntersectionHandler { public: RoundaboutHandler(const util::NodeBasedDynamicGraph &node_based_graph, diff --git a/include/guidance/turn_handler.hpp b/include/guidance/turn_handler.hpp index e5078666135..63470f6902e 100644 --- a/include/guidance/turn_handler.hpp +++ b/include/guidance/turn_handler.hpp @@ -23,7 +23,7 @@ namespace guidance { // Intersection handlers deal with all issues related to intersections. -class TurnHandler : public IntersectionHandler +class TurnHandler final : public IntersectionHandler { public: TurnHandler(const util::NodeBasedDynamicGraph &node_based_graph, diff --git a/include/util/typedefs.hpp b/include/util/typedefs.hpp index b3e5a8d461b..b96e954cf5f 100644 --- a/include/util/typedefs.hpp +++ b/include/util/typedefs.hpp @@ -50,8 +50,10 @@ struct duplicated_node }; } // namespace tag using OSMNodeID = osrm::Alias; +// NOLINTNEXTLINE(bugprone-throw-keyword-missing) static_assert(std::is_pod(), "OSMNodeID is not a valid alias"); using OSMWayID = osrm::Alias; +// NOLINTNEXTLINE(bugprone-throw-keyword-missing) static_assert(std::is_pod(), "OSMWayID is not a valid alias"); using DuplicatedNodeID = std::uint64_t; diff --git a/scripts/error_on_dirty.sh b/scripts/error_on_dirty.sh index f0328646d6e..84e4eadf606 100755 --- a/scripts/error_on_dirty.sh +++ b/scripts/error_on_dirty.sh @@ -10,6 +10,7 @@ dirty=$(git ls-files --modified) if [[ $dirty ]]; then echo $MSG echo $dirty + git diff exit 1 else exit 0 diff --git a/src/extractor/intersection/coordinate_extractor.cpp b/src/extractor/intersection/coordinate_extractor.cpp index 2b67d6dcd02..ed0d3dea6ba 100644 --- a/src/extractor/intersection/coordinate_extractor.cpp +++ b/src/extractor/intersection/coordinate_extractor.cpp @@ -892,6 +892,7 @@ CoordinateExtractor::PrepareLengthCache(const std::vector &coo segment_distances.reserve(coordinates.size()); segment_distances.push_back(0); // sentinel + // NOLINTNEXTLINE(bugprone-unused-return-value) std::find_if(std::next(std::begin(coordinates)), std::end(coordinates), [last_coordinate = coordinates.front(), diff --git a/src/extractor/scripting_environment_lua.cpp b/src/extractor/scripting_environment_lua.cpp index 69f1cb0950b..a26f5d0a9a9 100644 --- a/src/extractor/scripting_environment_lua.cpp +++ b/src/extractor/scripting_environment_lua.cpp @@ -884,6 +884,7 @@ void Sol2ScriptingEnvironment::ProcessElements( case osmium::item_type::node: { const auto &node = static_cast(*entity); + // NOLINTNEXTLINE(bugprone-use-after-move) result_node.clear(); if (local_context.has_node_function && (!node.tags().empty() || local_context.properties.call_tagless_node_function)) @@ -896,6 +897,7 @@ void Sol2ScriptingEnvironment::ProcessElements( case osmium::item_type::way: { const osmium::Way &way = static_cast(*entity); + // NOLINTNEXTLINE(bugprone-use-after-move) result_way.clear(); if (local_context.has_way_function) { diff --git a/src/nodejs/CMakeLists.txt b/src/nodejs/CMakeLists.txt index ff16a38569e..fe2a1fff611 100644 --- a/src/nodejs/CMakeLists.txt +++ b/src/nodejs/CMakeLists.txt @@ -15,6 +15,9 @@ message(STATUS "Configuring node_osrm bindings for NodeJs ${NODEJS_VERSION}") add_nodejs_module(node_osrm node_osrm.cpp) set_target_properties(node_osrm PROPERTIES CXX_STANDARD 14) +# TODO: we disable clang-tidy for this target, because it causes errors in third-party NodeJs related headers +set_target_properties(node_osrm PROPERTIES CXX_CLANG_TIDY "") + target_link_libraries(node_osrm osrm) # node_osrm artifacts in ${BINDING_DIR} to depend targets on diff --git a/src/osrm/osrm.cpp b/src/osrm/osrm.cpp index 1c5655f8430..8871aff455e 100644 --- a/src/osrm/osrm.cpp +++ b/src/osrm/osrm.cpp @@ -47,7 +47,7 @@ OSRM::OSRM(engine::EngineConfig &config) engine_ = std::make_unique>(config); break; default: - util::exception("Algorithm not implemented!"); + throw util::exception("Algorithm not implemented!"); } } OSRM::~OSRM() = default; diff --git a/src/updater/updater.cpp b/src/updater/updater.cpp index 8371b3696a5..78a0c19c46c 100644 --- a/src/updater/updater.cpp +++ b/src/updater/updater.cpp @@ -112,6 +112,7 @@ void checkWeightsConsistency( if (geometry_id.forward) { auto range = segment_data.GetForwardWeights(geometry_id.id); + // NOLINTNEXTLINE(bugprone-fold-init-type) EdgeWeight weight = std::accumulate(range.begin(), range.end(), EdgeWeight{0}); if (weight > edge.data.weight) { @@ -122,6 +123,7 @@ void checkWeightsConsistency( else { auto range = segment_data.GetReverseWeights(geometry_id.id); + // NOLINTNEXTLINE(bugprone-fold-init-type) EdgeWeight weight = std::accumulate(range.begin(), range.end(), EdgeWeight{0}); if (weight > edge.data.weight) { @@ -689,6 +691,7 @@ Updater::LoadAndUpdateEdgeExpandedGraph(std::vector &e new_weight += weight; } const auto durations = segment_data.GetForwardDurations(geometry_id.id); + // NOLINTNEXTLINE(bugprone-fold-init-type) new_duration = std::accumulate(durations.begin(), durations.end(), EdgeWeight{0}); } else @@ -704,6 +707,7 @@ Updater::LoadAndUpdateEdgeExpandedGraph(std::vector &e new_weight += weight; } const auto durations = segment_data.GetReverseDurations(geometry_id.id); + // NOLINTNEXTLINE(bugprone-fold-init-type) new_duration = std::accumulate(durations.begin(), durations.end(), EdgeWeight{0}); } return std::make_tuple(new_weight, new_duration); diff --git a/src/util/coordinate_calculation.cpp b/src/util/coordinate_calculation.cpp index 10e64c6cbf5..4aa544b0296 100644 --- a/src/util/coordinate_calculation.cpp +++ b/src/util/coordinate_calculation.cpp @@ -386,7 +386,7 @@ double findClosestDistance(const std::vector &lhs, const std::vector std::min(current_min, findClosestDistance(coordinate, rhs.begin(), rhs.end())); return false; }; - + // NOLINTNEXTLINE(bugprone-unused-return-value) std::find_if(std::begin(lhs), std::end(lhs), compute_minimum_distance_in_rhs); return current_min; }