Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically detect missing doxygen comments #1226

Merged
merged 18 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
62 changes: 62 additions & 0 deletions .githooks/check-docs
Original file line number Diff line number Diff line change
@@ -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\)")

godexsoft marked this conversation as resolved.
Show resolved Hide resolved
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
99 changes: 99 additions & 0 deletions .githooks/check-format
Original file line number Diff line number Diff line change
@@ -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
93 changes: 3 additions & 90 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
@@ -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
kuznetsss marked this conversation as resolved.
Show resolved Hide resolved
19 changes: 17 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,27 @@ jobs:
- name: Run formatters
id: run_formatters
run: |
./.githooks/pre-commit
./.githooks/check-format
shell: bash

check_docs:
cindyyan317 marked this conversation as resolved.
Show resolved Hide resolved
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:
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/clang-tidy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}
Expand All @@ -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 }}
Expand Down
9 changes: 8 additions & 1 deletion CMake/Docs.cmake
Original file line number Diff line number Diff line change
@@ -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}
Expand Down
6 changes: 6 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Loading
Loading