Fix various small issues found by clang-tidy and disable some warnings

.clang-tidy

@@ -1,5 +1,5 @@
 ---
-Checks: '*,-android-cloexec-*,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-cstyle-cast,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-static-cast-downcast,-cppcoreguidelines-pro-type-vararg,-fuchsia-*,-google-readability-casting,-google-readability-todo,-hicpp-named-parameter,-hicpp-no-array-decay,-hicpp-vararg,-llvm-include-order,-llvm-header-guard,-llvmlibc-*,-misc-no-recursion,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-named-parameter,-readability-magic-numbers'
+Checks: '*,-android-cloexec-*,-cert-err58-cpp,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-cstyle-cast,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-static-cast-downcast,-cppcoreguidelines-pro-type-vararg,-fuchsia-*,-google-readability-casting,-google-readability-todo,-hicpp-named-parameter,-hicpp-no-array-decay,-hicpp-vararg,-llvm-include-order,-llvm-header-guard,-llvmlibc-*,-misc-no-recursion,-modernize-use-nodiscard,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-named-parameter,-readability-magic-numbers'
 #
 #  cppcoreguidelines-pro-type-cstyle-cast
 #  google-build-using-namespace
@@ -13,6 +13,10 @@ Checks: '*,-android-cloexec-*,-cppcoreguidelines-avoid-magic-numbers,-cppcoregui
 #  android-cloexec-*
 #    O_CLOEXEC isn't available on Windows
 #
+#  cert-err58-cpp
+#    There are many of these in the test code, most of them from the Catch
+#    framework. Difficult to avoid.
+#
 #  cppcoreguidelines-avoid-magic-numbers
 #  readability-magic-numbers
 #    We have a lot of these and should probably at least fix some. But remove
@@ -45,6 +49,10 @@ Checks: '*,-android-cloexec-*,-cppcoreguidelines-avoid-magic-numbers,-cppcoregui
 #  misc-no-recursion
 #    Nothing wrong with recursion
 #
+#  modernize-use-nodiscard
+#    We have a lot of these. Remove it for the time being because there are
+#    too many to fix quickly. (TODO)
+#
 #  modernize-use-trailing-return-type
 #    We are not that modern...
 #

src/flex-table-column.cpp

@@ -53,6 +53,8 @@ static std::array<column_type_lookup, 25> const column_types = {
 static table_column_type
 get_column_type_from_string(std::string const &type)
 {
+    // Because it doesn't work with MSVC:
+    // NOLINTNEXTLINE(llvm-qualified-auto,readability-qualified-auto)
     auto const it =
         std::find_if(std::begin(column_types), std::end(column_types),
                      [&type](column_type_lookup name_type) {

src/format.hpp

@@ -13,6 +13,7 @@
 #define FMT_HEADER_ONLY
 #include <fmt/format.h>
 
-using namespace fmt::literals; // NOLINT(google-global-names-in-headers)
+// NOLINTNEXTLINE(google-global-names-in-headers,google-build-using-namespace)
+using namespace fmt::literals;
 
 #endif // OSM2PGSQL_FORMAT_HPP

src/middle-pgsql.cpp

@@ -15,13 +15,12 @@
  * emit the final geometry-enabled output formats
 */
 
-#include <stdexcept>
-#include <unordered_map>
-
 #include <cassert>
 #include <cstdlib>
-#include <functional>
 #include <memory>
+#include <stdexcept>
+#include <string>
+#include <unordered_map>
 #include <utility>
 
 #include <osmium/builder/osm_object_builder.hpp>
@@ -643,9 +642,9 @@ void middle_pgsql_t::stop()
     } else if (!m_options->append) {
         // Building the indexes takes time, so do it asynchronously.
         for (auto &table : m_tables) {
-            table.task_set(thread_pool().submit(
-                std::bind(&middle_pgsql_t::table_desc::build_index, &table,
-                          m_options->database_options.conninfo())));
+            table.task_set(thread_pool().submit([&]() {
+                table.build_index(m_options->database_options.conninfo());
+            }));
         }
     }
 }

src/options.cpp

@@ -101,7 +101,7 @@ struct option const long_options[] = {
     {"with-forward-dependencies", required_argument, nullptr, 217},
     {nullptr, 0, nullptr, 0}};
 
-static void long_usage(char const *arg0, bool verbose)
+void long_usage(char const *arg0, bool verbose)
 {
     char const *const name = program_name(arg0);
 

src/pgsql.cpp

@@ -89,17 +89,17 @@ void pg_conn_t::copy_data(std::string const &sql,
     log_sql_data("Copy data to '{}':\n{}", context, sql);
     int const r = PQputCopyData(m_conn.get(), sql.c_str(), (int)sql.size());
 
-    if (r == 1) {
-        return; // success
-    }
-
     switch (r) {
     case 0: // need to wait for write ready
         log_error("{} - COPY unexpectedly busy", context);
         break;
+    case 1: // success
+        return;
     case -1: // error occurred
         log_error("{} - error on COPY: {}", context, error_msg());
         break;
+    default: // unexpected result
+        break;
     }
 
     if (sql.size() < 1100) {

src/tagtransform-c.cpp

@@ -322,15 +322,12 @@ bool c_tagtransform_t::filter_rel_member_tags(
                 out_tags.add_tag_if_not_exists("nwn_ref", *relref);
             }
         }
-    } else if (is_boundary) {
+    } else if (is_boundary || (is_multipolygon && out_tags.contains("boundary"))) {
         /* Boundaries will get converted into multiple geometries:
          - Linear features will end up in the line and roads tables (useful for admin boundaries)
          - Polygon features also go into the polygon table (useful for national_forests)
          The edges of the polygon also get treated as linear fetaures allowing these to be rendered seperately. */
         *make_boundary = true;
-    } else if (is_multipolygon && out_tags.contains("boundary")) {
-        /* Treat type=multipolygon exactly like type=boundary if it has a boundary tag. */
-        *make_boundary = true;
     } else if (is_multipolygon) {
         *make_polygon = true;
     }

src/wkb.cpp

@@ -46,7 +46,7 @@ enum wkb_byte_order_type_t : uint8_t
 };
 
 template <typename T>
-static void str_push(std::string *data, T value)
+void str_push(std::string *data, T value)
 {
     data->append(reinterpret_cast<char const *const>(&value), sizeof(T));
 }
@@ -58,7 +58,7 @@ static void str_push(std::string *data, T value)
  *
  * \pre \code data != nullptr \endcode
  */
-static void write_header(std::string *data, geometry_type type, uint32_t srid)
+void write_header(std::string *data, geometry_type type, uint32_t srid)
 {
     str_push(data, Endian);
     if (srid) {
@@ -76,12 +76,12 @@ static void write_header(std::string *data, geometry_type type, uint32_t srid)
  *
  * \pre \code data != nullptr \endcode
  */
-static void write_length(std::string *data, std::size_t length)
+void write_length(std::string *data, std::size_t length)
 {
     str_push(data, static_cast<uint32_t>(length));
 }
 
-static void write_points(std::string *data, geom::point_list_t const &points)
+void write_points(std::string *data, geom::point_list_t const &points)
 {
     write_length(data, points.size());
     for (auto const &point : points) {
@@ -90,7 +90,7 @@ static void write_points(std::string *data, geom::point_list_t const &points)
     }
 }
 
-static void write_linestring(std::string *data, geom::linestring_t const &geom,
+void write_linestring(std::string *data, geom::linestring_t const &geom,
                              uint32_t srid)
 {
     assert(data);
@@ -99,7 +99,7 @@ static void write_linestring(std::string *data, geom::linestring_t const &geom,
     write_points(data, geom);
 }
 
-static void write_polygon(std::string *data, geom::polygon_t const &geom,
+void write_polygon(std::string *data, geom::polygon_t const &geom,
                           uint32_t srid)
 {
     assert(data);
@@ -343,7 +343,7 @@ private:
     {
         check_bytes(sizeof(double) * 2);
 
-        std::array<double, 2> data;
+        std::array<double, 2> data{};
         std::memcpy(&data[0], m_it, sizeof(double) * 2);
         m_it += sizeof(double) * 2;
 

tests/test-expire-tiles.cpp

@@ -9,6 +9,7 @@
 
 #include <catch.hpp>
 
+#include <random>
 #include <set>
 
 #include "expire-tiles.hpp"
@@ -43,13 +44,18 @@ struct tile_output_set
 
     static tile_output_set generate_random(uint32_t zoom, size_t count)
     {
+        // Use a random device with a fixed seed. We don't really care about
+        // the quality of random numbers here, we just need to generate valid
+        // OSM test data. The fixed seed ensures that the results are
+        // reproducible.
+        // NOLINTNEXTLINE(cert-msc32-c,cert-msc51-cpp)
+        static std::mt19937_64 rng{47382};
+
+        std::uniform_int_distribution<uint32_t> dist{0, (1U << zoom) - 1U};
         tile_output_set set;
-        auto const cmask = (1UL << zoom) - 1U;
 
         do {
-            set.output_dirty_tile(
-                tile_t{zoom, static_cast<uint32_t>(rand() & cmask),
-                       static_cast<uint32_t>(rand() & cmask)});
+            set.output_dirty_tile(tile_t{zoom, dist(rng), dist(rng)});
         } while (set.tiles.size() < count);
 
         return set;

tests/test-middle.cpp

@@ -857,17 +857,17 @@ TEMPLATE_TEST_CASE("middle: add relation with attributes", "",
 
     // Create some relations we'll use for the tests.
     test_buffer_t buffer;
-    auto &relation30 = buffer.add_relation(
+    auto const &relation30 = buffer.add_relation(
         "r30 v123 c456 i789 t2009-02-13T23:31:30Z Mw10@outer,w11@inner "
         "Ttype=multipolygon,name=Penguin_Park");
 
     // The same relation but with default attributes.
-    auto &relation30_no_attr = buffer.add_relation(
+    auto const &relation30_no_attr = buffer.add_relation(
         "r30 Mw10@outer,w11@inner Ttype=multipolygon,name=Penguin_Park");
 
     // The same relation but with attributes in tags.
     // The order of the tags is important here!
-    auto &relation30_attr_tags = buffer.add_relation(
+    auto const &relation30_attr_tags = buffer.add_relation(
         "r30 Mw10@outer,w11@inner "
         "Ttype=multipolygon,name=Penguin_Park,osm_user=,osm_uid=789,"
         "osm_version=123,osm_timestamp=2009-02-13T23:31:30Z,osm_changeset=456");

tests/test-output-gazetteer.cpp

@@ -21,7 +21,7 @@ static testing::db::import_t db;
 // Use a random device with a fixed seed. We don't really care about
 // the quality of random numbers here, we just need to generate valid
 // OSM test data. The fixed seed ensures that the results are reproducible.
-static std::mt19937_64 rng{47382}; // NOLINT(cert-msc32-c)
+static std::mt19937_64 rng{47382}; // NOLINT(cert-msc32-c,cert-msc51-cpp)
 
 class node_opl_t
 {

tests/test-output-pgsql-style-file.cpp

@@ -62,7 +62,7 @@ TEST_CASE("Parse style file with single node entry")
     REQUIRE(enable_way_area);
 
     REQUIRE(exlist.get(osmium::item_type::node).size() == 1);
-    REQUIRE(exlist.get(osmium::item_type::way).size() == 0);
+    REQUIRE(exlist.get(osmium::item_type::way).empty());
 
     auto const &ex = exlist.get(osmium::item_type::node).front();
     REQUIRE(ex.name == "access");
@@ -219,7 +219,7 @@ TEST_CASE("Parse style file with invalid data types")
 
     REQUIRE(enable_way_area);
 
-    REQUIRE(exlist.get(osmium::item_type::node).size() == 0);
+    REQUIRE(exlist.get(osmium::item_type::node).empty());
     REQUIRE(exlist.get(osmium::item_type::way).size() == 1);
 
     auto const &ways = exlist.get(osmium::item_type::way);