Automatically detect missing doxygen comments (#1226)

Fixes #1216

.clang-tidy

@@ -100,7 +100,6 @@ Checks: '-*,
   performance-move-constructor-init,
   performance-no-automatic-move,
   performance-trivially-destructible,
-  readability-avoid-const-params-in-decls,
   readability-braces-around-statements,
   readability-const-return-type,
   readability-container-contains,

.githooks/check-docs

@@ -0,0 +1,62 @@
+#!/bin/bash
+
+# Note: This script is intended to be run from the root of the repository.
+#
+# Not really a hook but should be used to check the completness of documentation for added code, otherwise CI will come for you.
+# It's good to have /tmp as the output so that consecutive runs are fast but no clutter in the repository.
+
+echo "+ Checking documentation..."
+
+ROOT=$(pwd)
+DOXYGEN=$(command -v doxygen)
+TMPDIR=${ROOT}/.cache/doxygen
+TMPFILE=${TMPDIR}/docs.log
+DOCDIR=${TMPDIR}/out
+
+if [ -z "$DOXYGEN" ]; then
+    # No hard error if doxygen is not installed yet
+    cat <<EOF
+
+                                   WARNING
+-----------------------------------------------------------------------------
+        'doxygen' is required to check documentation.
+        Please install it for next time. For the time being it's on CI.
+-----------------------------------------------------------------------------
+
+EOF
+    exit 0
+fi
+
+mkdir -p ${DOCDIR} > /dev/null 2>&1
+pushd ${DOCDIR} > /dev/null 2>&1
+
+cat ${ROOT}/Doxyfile | \
+sed \
+    -e "s/\${LINT}/YES/" \
+    -e "s!\${SOURCE}!${ROOT}!" \
+    -e "s/\${USE_DOT}/NO/" \
+    -e "s/\${EXCLUDES}/impl/" \
+| ${DOXYGEN} - 2> ${TMPFILE} 1> /dev/null
+
+# We don't want to check for default values and typedefs as well as for member variables
+OUT=$(cat ${TMPFILE} \
+    | grep -v "=default" \
+    | grep -v "\(variable\)" \
+    | grep -v "\(typedef\)")
+
+rm -rf ${TMPFILE} > /dev/null 2>&1
+popd > /dev/null 2>&1
+
+if [[ ! -z "$OUT" ]]; then
+    cat <<EOF
+
+                                    ERROR
+-----------------------------------------------------------------------------
+                      Found issues with documentation:
+
+$OUT
+-----------------------------------------------------------------------------
+
+EOF
+    exit 2
+fi

.githooks/check-format

@@ -0,0 +1,99 @@
+#!/bin/bash
+
+# Note: This script is intended to be run from the root of the repository.
+#
+# This script checks the format of the code and cmake files.
+# In many cases it will automatically fix the issues and abort the commit.
+
+echo "+ Checking code format..."
+
+# paths to check and re-format
+sources="src unittests"
+formatter="clang-format -i"
+version=$($formatter --version | grep -o '[0-9\.]*')
+
+if [[ "17.0.0" > "$version" ]]; then
+    cat <<EOF
+
+                                    ERROR
+-----------------------------------------------------------------------------
+            A minimum of version 17 of `which clang-format` is required.
+            Your version is $version.
+            Please fix paths and run again.
+-----------------------------------------------------------------------------
+
+EOF
+    exit 3
+fi
+
+# check there is no .h headers, only .hpp
+wrong_headers=$(find $sources -name "*.h" | sed 's/^/        - /')
+if [[ ! -z "$wrong_headers" ]]; then
+    cat <<EOF
+
+                                    ERROR
+-----------------------------------------------------------------------------
+       Found .h headers in the source code. Please rename them to .hpp:
+
+$wrong_headers
+-----------------------------------------------------------------------------
+
+EOF
+    exit 2
+fi
+
+if ! command -v cmake-format &> /dev/null; then
+    cat <<EOF
+
+                                    ERROR
+-----------------------------------------------------------------------------
+        'cmake-format' is required to run this script.
+        Please install it and run again.
+-----------------------------------------------------------------------------
+
+EOF
+    exit 3
+fi
+
+function grep_code {
+    grep -l "${1}" ${sources} -r --include \*.hpp --include \*.cpp
+}
+
+if [[ "$OSTYPE" == "darwin"* ]]; then
+    # make all includes to be <...> style
+    grep_code '#include ".*"' | xargs sed -i '' -E 's|#include "(.*)"|#include <\1>|g'
+
+    # make local includes to be "..." style
+    main_src_dirs=$(find ./src -maxdepth 1 -type d -exec basename {} \; | tr '\n' '|' | sed 's/|$//' | sed 's/|/\\|/g')
+    grep_code "#include <\($main_src_dirs\)/.*>" | xargs sed -i '' -E "s|#include <(($main_src_dirs)/.*)>|#include \"\1\"|g"
+else
+    # make all includes to be <...> style
+    grep_code '#include ".*"' | xargs sed -i -E 's|#include "(.*)"|#include <\1>|g'
+
+    # make local includes to be "..." style
+    main_src_dirs=$(find ./src -type d -maxdepth 1 -exec basename {} \; | paste -sd '|' | sed 's/|/\\|/g')
+    grep_code "#include <\($main_src_dirs\)/.*>" | xargs sed -i -E "s|#include <(($main_src_dirs)/.*)>|#include \"\1\"|g"
+fi
+
+cmake_dirs=$(echo CMake $sources)
+cmake_files=$(find $cmake_dirs -type f \( -name "CMakeLists.txt" -o -name "*.cmake" \))
+cmake_files=$(echo $cmake_files ./CMakeLists.txt)
+
+first=$(git diff $sources $cmake_files)
+find $sources -type f \( -name '*.cpp' -o -name '*.hpp' -o -name '*.ipp' \) -print0 | xargs -0 $formatter
+cmake-format -i $cmake_files
+second=$(git diff $sources $cmake_files)
+changes=$(diff <(echo "$first") <(echo "$second") | wc -l | sed -e 's/^[[:space:]]*//')
+
+if [ "$changes" != "0" ]; then
+    cat <<\EOF
+
+                                   WARNING
+-----------------------------------------------------------------------------
+  Automatically re-formatted code with 'clang-format' - commit was aborted.
+  Please manually add any updated files and commit again.
+-----------------------------------------------------------------------------
+
+EOF
+    exit 1
+fi

.githooks/pre-commit

@@ -1,93 +1,6 @@
 #!/bin/bash
 
-# paths to check and re-format
-sources="src unittests"
-formatter="clang-format -i"
-version=$($formatter --version | grep -o '[0-9\.]*')
-
-if [[ "17.0.0" > "$version" ]]; then
-    cat <<EOF
-
-                                    ERROR
------------------------------------------------------------------------------
-            A minimum of version 17 of `which clang-format` is required.
-            Your version is $version.
-            Please fix paths and run again.
------------------------------------------------------------------------------
-
-EOF
-    exit 3
-fi
-
-# check there is no .h headers, only .hpp
-wrong_headers=$(find $sources -name "*.h" | sed 's/^/        - /')
-if [[ ! -z "$wrong_headers" ]]; then
-    cat <<EOF
-
-                                    ERROR
------------------------------------------------------------------------------
-       Found .h headers in the source code. Please rename them to .hpp:
-
-$wrong_headers
------------------------------------------------------------------------------
-
-EOF
-    exit 2
-fi
-
-if ! command -v cmake-format &> /dev/null; then
-    cat <<EOF
-
-                                    ERROR
------------------------------------------------------------------------------
-        `cmake-format` is required to run this script.
-        Please install it and run again.
------------------------------------------------------------------------------
-
-EOF
-    exit 3
-fi
-
-function grep_code {
-    grep -l "${1}" ${sources} -r --include \*.hpp --include \*.cpp
-}
-
-if [[ "$OSTYPE" == "darwin"* ]]; then
-    # make all includes to be <...> style
-    grep_code '#include ".*"' | xargs sed -i '' -E 's|#include "(.*)"|#include <\1>|g'
-
-    # make local includes to be "..." style
-    main_src_dirs=$(find ./src -maxdepth 1 -type d -exec basename {} \; | tr '\n' '|' | sed 's/|$//' | sed 's/|/\\|/g')
-    grep_code "#include <\($main_src_dirs\)/.*>" | xargs sed -i '' -E "s|#include <(($main_src_dirs)/.*)>|#include \"\1\"|g"
-else
-    # make all includes to be <...> style
-    grep_code '#include ".*"' | xargs sed -i -E 's|#include "(.*)"|#include <\1>|g'
-
-    # make local includes to be "..." style
-    main_src_dirs=$(find ./src -type d -maxdepth 1 -exec basename {} \; | paste -sd '|' | sed 's/|/\\|/g')
-    grep_code "#include <\($main_src_dirs\)/.*>" | xargs sed -i -E "s|#include <(($main_src_dirs)/.*)>|#include \"\1\"|g"
-fi
-
-cmake_dirs=$(echo CMake $sources)
-cmake_files=$(find $cmake_dirs -type f \( -name "CMakeLists.txt" -o -name "*.cmake" \))
-cmake_files=$(echo $cmake_files ./CMakeLists.txt)
-
-first=$(git diff $sources $cmake_files)
-find $sources -type f \( -name '*.cpp' -o -name '*.hpp' -o -name '*.ipp' \) -print0 | xargs -0 $formatter
-cmake-format -i $cmake_files
-second=$(git diff $sources $cmake_files)
-changes=$(diff <(echo "$first") <(echo "$second") | wc -l | sed -e 's/^[[:space:]]*//')
-
-if [ "$changes" != "0" ]; then
-    cat <<\EOF
-
-                                   WARNING
------------------------------------------------------------------------------
-  Automatically re-formatted code with `clang-format` - commit was aborted.
-  Please manually add any updated files and commit again.
------------------------------------------------------------------------------
-
-EOF
-    exit 1
-fi
+# This script is intended to be run from the root of the repository.
 
+source .githooks/check-format
+source .githooks/check-docs

.github/workflows/build.yml

@@ -17,12 +17,27 @@ jobs:
       - name: Run formatters
         id: run_formatters
         run: |
-          ./.githooks/pre-commit
+          ./.githooks/check-format
+        shell: bash
+
+  check_docs:
+    name: Check documentation
+    runs-on: ubuntu-20.04
+    container:
+      image: rippleci/clio_ci:latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Run linter
+        id: run_linter
+        run: |
+          ./.githooks/check-docs
         shell: bash
 
   build:
     name: Build
-    needs: check_format
+    needs: 
+      - check_format 
+      - check_docs
     strategy:
       fail-fast: false
       matrix:

.github/workflows/clang-tidy.yml

@@ -59,11 +59,11 @@ jobs:
         run: |
           run-clang-tidy-17 -p build -j ${{ steps.number_of_threads.outputs.threads_number }} -fix -quiet 1>output.txt
 
-      - name: Run pre-commit hook
+      - name: Check format
         if: ${{ steps.run_clang_tidy.outcome != 'success' }}
         continue-on-error: true
         shell: bash
-        run: ./.githooks/pre-commit
+        run: ./.githooks/check-format
 
       - name: Print issues found
         if: ${{ steps.run_clang_tidy.outcome != 'success' }}
@@ -88,6 +88,7 @@ jobs:
           rm create_issue.log issue.md
 
       - uses: crazy-max/ghaction-import-gpg@v5
+        if: ${{ steps.run_clang_tidy.outcome != 'success' }}
         with:
           gpg_private_key: ${{ secrets.ACTIONS_GPG_PRIVATE_KEY }}
           passphrase: ${{ secrets.ACTIONS_GPG_PASSPHRASE }}

CMake/Docs.cmake

@@ -1,9 +1,16 @@
 find_package(Doxygen REQUIRED)
 
+# See Doxyfile for these settings:
+set(SOURCE ${CMAKE_CURRENT_SOURCE_DIR})
+set(USE_DOT "YES")
+set(LINT "NO")
+set(EXCLUDES "")
+# ---
+
 set(DOXYGEN_IN ${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile)
 set(DOXYGEN_OUT ${CMAKE_CURRENT_BINARY_DIR}/Doxyfile)
 
-configure_file(${DOXYGEN_IN} ${DOXYGEN_OUT} @ONLY)
+configure_file(${DOXYGEN_IN} ${DOXYGEN_OUT})
 add_custom_target(
   docs
   COMMAND ${DOXYGEN_EXECUTABLE} ${DOXYGEN_OUT}

CONTRIBUTING.md

@@ -24,6 +24,7 @@ git config --local core.hooksPath .githooks
 The pre-commit hook requires `clang-format >= 17.0.0` and `cmake-format` to be installed on your machine.
 `clang-format` can be installed using `brew` on macOS and default package manager on Linux.
 `cmake-format` can be installed using `pip`.
+The hook will also attempt to automatically use `doxygen` to verify that everything public in the codebase is covered by doc comments. If `doxygen` is not installed, the hook will raise a warning suggesting to install `doxygen` for future commits.
 
 ## Git commands
 This sections offers a detailed look at the git commands you will need to use to get your PR submitted. 
@@ -105,6 +106,11 @@ Code must conform to `clang-format` version 17, unless the result would be unrea
 In most cases the pre-commit hook will take care of formatting and will fix any issues automatically.
 To manually format your code, use `clang-format -i <your changed files>` for C++ files and `cmake-format -i <your changed files>` for CMake files.
 
+## Documentation
+All public namespaces, classes and functions must be covered by doc (`doxygen`) comments. Everything that is not within a nested `impl` namespace is considered public.
+
+> **Note:** Keep in mind that this is enforced by Clio's CI and your build will fail if newly added public code lacks documentation.
+
 ## Avoid
 * Proliferation of nearly identical code.
 * Proliferation of new files and classes unless it improves readability or/and compilation time.