Integrate clang-tidy (#981)

* configure clang-tidy and run under CI

* pin CI to clang-tidy-17

Author: etwoo

.clang-tidy

@@ -0,0 +1,16 @@
+Checks: >
+  bugprone-*,
+  -bugprone-easily-swappable-parameters,
+  -bugprone-exception-escape,
+  -bugprone-implicit-widening-of-multiplication-result,
+  -bugprone-narrowing-conversions,
+  -bugprone-suspicious-include,
+  -bugprone-unhandled-exception-at-new,
+  clang-analyzer-*,
+
+# Turn all the warnings from the checks above into errors.
+WarningsAsErrors: '*'
+# Check first-party (non-system, non-vendored) headers.
+HeaderFilterRegex: '.*'
+ExcludeHeaderFilterRegex: 'build/_deps/'
+SystemHeaders: false

.github/workflows/lint_and_format_check.yml

@@ -36,3 +36,10 @@ jobs:
         name: Lint with Ruff
         with:
           version: 0.6.0
+
+      - name: Run clang-tidy
+        run: >
+          cmake -B build -DADA_TESTING=ON -DCMAKE_CXX_CLANG_TIDY=clang-tidy-17 &&
+          cmake --build build -j=4
+        env:
+          CXX: clang++-17

cmake/add-cpp-test.cmake

@@ -30,6 +30,7 @@ function(add_cpp_test TEST_NAME)
     add_library(${TEST_NAME} STATIC ${ARGS_SOURCES})
   else(ARGS_LIBRARY)
     add_executable(${TEST_NAME} ${ARGS_SOURCES})
+    set_source_files_properties(${ARGS_SOURCES} PROPERTIES SKIP_LINTING ON)
   endif(ARGS_LIBRARY)
 
   # Add test

include/ada/url-inl.h

@@ -55,6 +55,7 @@ inline std::ostream &operator<<(std::ostream &out, const ada::url &u) {
   out.protocol_end = uint32_t(get_protocol().size());
 
   // Trailing index is always the next character of the current one.
+  // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
   size_t running_index = out.protocol_end;
 
   if (host.has_value()) {

src/ada_idna.cpp

@@ -9374,12 +9374,14 @@ bool is_label_valid(const std::u32string_view label) {
       for (size_t i = 0; i <= last_non_nsm_char; i++) {
         const direction d = find_direction(label[i]);
 
+        // NOLINTBEGIN(bugprone-assignment-in-if-condition)
         // In an RTL label, if an EN is present, no AN may be present, and vice
         // versa.
         if ((d == direction::EN && ((has_en = true) && has_an)) ||
             (d == direction::AN && ((has_an = true) && has_en))) {
           return false;
         }
+        // NOLINTEND(bugprone-assignment-in-if-condition)
 
         if (!(d == direction::R || d == direction::AL || d == direction::AN ||
               d == direction::EN || d == direction::ES || d == direction::CS ||

src/parser.cpp

@@ -394,6 +394,7 @@ result_type parse_url_impl(std::string_view user_input,
 
         // If c is U+002F (/), then set state to relative slash state.
         if ((input_position != input_size) &&
+            // NOLINTNEXTLINE(bugprone-branch-clone)
             (url_data[input_position] == '/')) {
           ada_log(
               "RELATIVE_SCHEME if c is U+002F (/), then set state to relative "

src/unicode.cpp

@@ -476,6 +476,7 @@ bool percent_encode(const std::string_view input, const uint8_t character_set[],
   }
   ada_log("percent_encode appending ", std::distance(input.begin(), pointer),
           " bytes");
+  // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
   out.append(input.data(), std::distance(input.begin(), pointer));
   ada_log("percent_encode processing ", std::distance(pointer, input.end()),
           " bytes");
@@ -510,6 +511,7 @@ bool to_ascii(std::optional<std::string>& out, const std::string_view plain,
 std::string percent_encode(const std::string_view input,
                            const uint8_t character_set[], size_t index) {
   std::string out;
+  // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
   out.append(input.data(), index);
   auto pointer = input.begin() + index;
   for (; pointer != input.end(); pointer++) {

src/url.cpp

@@ -566,6 +566,7 @@ ada_really_inline void url::parse_path(std::string_view input) {
   if (has_search()) {
     answer.append(",\n");
     answer.append("\t\"query\":\"");
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
     helpers::encode_json(query.value(), back);
     answer.append("\"");
   }

tests/CMakeLists.txt

@@ -49,40 +49,27 @@ if(MSVC AND BUILD_SHARED_LIBS)
         "$<TARGET_FILE_DIR:basic_fuzzer>")                 # <--this is out-file path
 endif()
 
+macro(add_gtest_test exe cpp)
+  add_executable(${exe} ${cpp})
+  target_link_libraries(${exe} PRIVATE simdjson GTest::gtest_main)
+  gtest_discover_tests(${exe} PROPERTIES TEST_DISCOVERY_TIMEOUT 600)
+  set_source_files_properties(${cpp} PROPERTIES SKIP_LINTING ON)
+endmacro()
+
 if(MSVC AND BUILD_SHARED_LIBS)
   message(STATUS "For some tests we use Google Test and it fails when building a DLL.")
   message(STATUS "Thus the tests are disabled. Sorry.")
 else()
   include(GoogleTest)
-  add_executable(wpt_url_tests wpt_url_tests.cpp)
-  if(ADA_INCLUDE_URL_PATTERN)
-  add_executable(wpt_urlpattern_tests wpt_urlpattern_tests.cpp)
-  endif()
-  add_executable(url_components url_components.cpp)
-  add_executable(basic_tests basic_tests.cpp)
-  add_executable(from_file_tests from_file_tests.cpp)
-  add_executable(ada_c ada_c.cpp)
-  add_executable(url_search_params url_search_params.cpp)
-
-  target_link_libraries(wpt_url_tests PRIVATE simdjson GTest::gtest_main)
+  add_gtest_test(wpt_url_tests wpt_url_tests.cpp)
   if(ADA_INCLUDE_URL_PATTERN)
-  target_link_libraries(wpt_urlpattern_tests PRIVATE simdjson GTest::gtest_main)
+  add_gtest_test(wpt_urlpattern_tests wpt_urlpattern_tests.cpp)
   endif()
-  target_link_libraries(url_components PRIVATE simdjson GTest::gtest_main)
-  target_link_libraries(basic_tests PRIVATE simdjson GTest::gtest_main)
-  target_link_libraries(from_file_tests PRIVATE simdjson GTest::gtest_main)
-  target_link_libraries(ada_c PRIVATE simdjson GTest::gtest_main)
-  target_link_libraries(url_search_params PRIVATE simdjson GTest::gtest_main)
-
-  gtest_discover_tests(wpt_url_tests PROPERTIES TEST_DISCOVERY_TIMEOUT 600)
-  if(ADA_INCLUDE_URL_PATTERN)
-  gtest_discover_tests(wpt_urlpattern_tests PROPERTIES TEST_DISCOVERY_TIMEOUT 600)
-  endif()
-  gtest_discover_tests(url_components PROPERTIES TEST_DISCOVERY_TIMEOUT 600)
-  gtest_discover_tests(basic_tests PROPERTIES TEST_DISCOVERY_TIMEOUT 600)
-  gtest_discover_tests(from_file_tests PROPERTIES TEST_DISCOVERY_TIMEOUT 600)
-  gtest_discover_tests(ada_c PROPERTIES TEST_DISCOVERY_TIMEOUT 600)
-  gtest_discover_tests(url_search_params PROPERTIES TEST_DISCOVERY_TIMEOUT 600)
+  add_gtest_test(url_components url_components.cpp)
+  add_gtest_test(basic_tests basic_tests.cpp)
+  add_gtest_test(from_file_tests from_file_tests.cpp)
+  add_gtest_test(ada_c ada_c.cpp)
+  add_gtest_test(url_search_params url_search_params.cpp)
 
   if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
     if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9)
@@ -102,5 +89,4 @@ else()
     target_compile_definitions(basic_tests PRIVATE _CRT_SECURE_NO_WARNINGS)
     target_compile_definitions(url_search_params PRIVATE _CRT_SECURE_NO_WARNINGS)
   endif()
-
 endif()