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 13 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
61 changes: 61 additions & 0 deletions .githooks/check-docs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#!/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=/tmp
TMPFILE=${TMPDIR}/docs.log
DOCDIR=${TMPDIR}/out

if [[ ! -z "$DOXYGEN" ]]; then
mkdir -p ${DOCDIR} > /dev/null
pushd ${DOCDIR} > /dev/null

sed \
-e "s/\${LINT}/YES/" \
-e "s!\${SOURCE}!${ROOT}!" \
-e "s/\${USE_DOT}/NO/" \
-e "s/\${EXCLUDES}/impl/" \
${ROOT}/Doxyfile > ./Doxyfile

${DOXYGEN} ./Doxyfile 2> ${TMPFILE} 1> /dev/null
godexsoft marked this conversation as resolved.
Show resolved Hide resolved

# 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
popd > /dev/null

if [[ ! -z "$OUT" ]]; then
cat <<EOF

ERROR
-----------------------------------------------------------------------------
Found issues with documentation:

$OUT
-----------------------------------------------------------------------------

EOF
exit 2
fi
else
# 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
fi
godexsoft marked this conversation as resolved.
Show resolved Hide resolved
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
30 changes: 15 additions & 15 deletions Doxyfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
PROJECT_NAME = "Clio"
PROJECT_LOGO = ../docs/img/xrpl-logo.svg
PROJECT_NUMBER = @DOC_CLIO_VERSION@
PROJECT_LOGO = ${SOURCE}/docs/img/xrpl-logo.svg
PROJECT_NUMBER = ${DOC_CLIO_VERSION}
PROJECT_BRIEF = The XRP Ledger API server.

EXTRACT_ALL = NO
Expand All @@ -12,16 +12,16 @@ EXTRACT_ANON_NSPACES = NO

SORT_MEMBERS_CTORS_1ST = YES

INPUT = ../src
EXCLUDE_SYMBOLS = impl
INPUT = ${SOURCE}/src
EXCLUDE_SYMBOLS = ${EXCLUDES}
RECURSIVE = YES
HAVE_DOT = YES
HAVE_DOT = ${USE_DOT}

QUIET = YES
WARNINGS = YES
WARN_NO_PARAMDOC = YES
WARN_IF_INCOMPLETE_DOC = YES
WARN_IF_UNDOCUMENTED = YES
WARNINGS = ${LINT}
WARN_NO_PARAMDOC = ${LINT}
WARN_IF_INCOMPLETE_DOC = ${LINT}
WARN_IF_UNDOCUMENTED = ${LINT}

GENERATE_LATEX = NO
GENERATE_HTML = YES
Expand All @@ -31,12 +31,12 @@ SORT_MEMBERS_CTORS_1ST = YES
GENERATE_TREEVIEW = YES
DISABLE_INDEX = NO
FULL_SIDEBAR = NO
HTML_HEADER = ../docs/doxygen-awesome-theme/header.html
HTML_EXTRA_STYLESHEET = ../docs/doxygen-awesome-theme/doxygen-awesome.css \
../docs/doxygen-awesome-theme/doxygen-awesome-sidebar-only.css \
../docs/doxygen-awesome-theme/doxygen-awesome-sidebar-only-darkmode-toggle.css
HTML_EXTRA_FILES = ../docs/doxygen-awesome-theme/doxygen-awesome-darkmode-toggle.js \
../docs/doxygen-awesome-theme/doxygen-awesome-interactive-toc.js
HTML_HEADER = ${SOURCE}/docs/doxygen-awesome-theme/header.html
HTML_EXTRA_STYLESHEET = ${SOURCE}/docs/doxygen-awesome-theme/doxygen-awesome.css \
${SOURCE}/docs/doxygen-awesome-theme/doxygen-awesome-sidebar-only.css \
${SOURCE}/docs/doxygen-awesome-theme/doxygen-awesome-sidebar-only-darkmode-toggle.css
HTML_EXTRA_FILES = ${SOURCE}/docs/doxygen-awesome-theme/doxygen-awesome-darkmode-toggle.js \
${SOURCE}/docs/doxygen-awesome-theme/doxygen-awesome-interactive-toc.js

HTML_COLORSTYLE = LIGHT
HTML_COLORSTYLE_HUE = 209
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/BookChangesHelper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ class BookChanges final {
/**
* @brief Implementation of value_from for BookChange type.
*
* @param jv The JSON value to populate
* @param [out] jv The JSON value to populate
* @param change The BookChange to serialize
*/
inline void
Expand Down
Loading
Loading