Skip to content

[CI] Restore the per-function Doxygen check #688

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
myungjoo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[CI] Restore the per-function Doxygen check #688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 54 additions & 8 deletions .github/workflows/static.check.scripts/doxygen-tag.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,42 @@ fi

files=$1
failed=0
report_path=${report_path:-/dev/null}

if [ ! -f $files ]; then
echo "::error The file $files does not exists."
exit 1
fi

##
# @brief Tell whether the declaration starting at a line carries a trailing
# "/**<" Doxygen comment, on the declaration itself or right after it.
# @param $1 1-based line number where ctags placed the function
# @return 0 when such a comment is found, 1 otherwise
# Reads the global array file_lines (0-based). A declaration ends at the first
# ';' outside braces, or at the '}' that closes an inline body. The comment may
# also open the next line; a "/**<" later on that line belongs to whatever is
# declared there, not to this one.
has_trailing_doc() {
local -i i=$1-1
local -i last=${#file_lines[@]}-1
local -i depth=0
local l opens closes
while [[ $i -le $last ]]; do
l=${file_lines[$i]}
[[ $l == *"/**<"* ]] && return 0
opens=${l//[^\{]/}
closes=${l//[^\}]/}
depth+=${#opens}-${#closes}
if [[ $depth -le 0 && ( $l == *";"* || $l == *"}"* ) ]]; then
[[ $i -lt $last && ${file_lines[$i+1]} =~ ^[[:space:]]*/\*\*\< ]] && return 0
return 1
fi
i+=1
done
return 1
}

echo "::group::Doxygen tag check started"

for file in `cat $files`; do
Expand Down Expand Up @@ -69,14 +99,20 @@ for file in `cat $files`; do
# Checking tags for each function
if [[ $advanced == 1 ]]; then
declare -i idx=0
brief=0
in_comment=0
function_positions="" # Line number of functions.
structure_positions="" # Line number of structure.
mapfile -t file_lines < "$file"

local function_check_flag="f+p" # check document for function and prototype of the function

if [[ $pr_doxygen_check_skip_function_definition == 1 && $file != *.h ]]; then
function_check_flag="p" # check document for only prototypes of the function for non-header file
fi
# Definitions are checked everywhere. Prototypes are checked
# only in headers, where the declaration is the documented
# form; a forward declaration in a .c/.cc is documented at
# its definition.
case $file in
*.h|*.hh|*.hpp ) function_check_flag="f+p" ;;
* ) function_check_flag="f" ;;
esac

# Find line number of functions using ctags, and append them.
while IFS='' read -r line || [[ -n "$line" ]]; do
Expand All @@ -97,7 +133,7 @@ for file in `cat $files`; do
# Check if a function has @brief tag or not.
# To pass correct line number not sub number, keep space " $idx ".
# ex) want to pass 143 not 14, 43, 1, 3, 4
if [[ $function_positions =~ " $idx " && $brief -eq 0 ]]; then
if [[ $function_positions =~ " $idx " && $brief -eq 0 ]] && ! has_trailing_doc $idx; then
echo "[ERROR] File name: $file, $idx line, `echo $line | cut -d ' ' -f1` function needs @brief tag "
failed=1
fi
Expand All @@ -113,11 +149,21 @@ for file in `cat $files`; do
# Find brief or copydoc tag in the comments between the codes.
if [[ $line =~ "@brief" || $line =~ "@copydoc" ]]; then
brief=1
# Doxygen tags become zero in code section.
elif [[ $line != *"*"* && ( $line =~ ";" || $line =~ "}" || $line =~ "#") ]]; then
# Doxygen tags become zero in code section. Lines inside a
# block comment, or opening one, keep the pending tag; a
# code line that merely contains '*' (a pointer) does not.
elif [[ $in_comment -eq 0 && ! $line =~ ^[[:space:]]*(/\*|//) && ( $line =~ ";" || $line =~ "}" || $line =~ "#") ]]; then
brief=0
fi

# Track block comments so that their continuation lines are
# never mistaken for code, whatever character they start with.
if [[ $in_comment -eq 1 ]]; then
[[ $line == *"*/"* ]] && in_comment=0
elif [[ $line == *"/*"* && ${line##*/\*} != *"*/"* ]]; then
in_comment=1
fi

# Check a comment statement that begins with '/*'.
# Note that doxygen does not recognize a comment statement that start with '/*'.
# Let's skip the doxygen tag inspection such as "/**" in case of a single line comment.
Expand Down
254 changes: 254 additions & 0 deletions .github/workflows/static.check.scripts/test_doxygen_tag.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,254 @@
#!/usr/bin/env bash

##
# Copyright (c) 2026 Samsung Electronics Co., Ltd. All Rights Reserved.
#
# @file test_doxygen_tag.sh
# @brief Self-test for doxygen-tag.sh in the advanced (per-function) mode.
# @see https://github.com/nnstreamer/nnstreamer
# @author MyungJoo Ham <myungjoo.ham@samsung.com>
#
# Runs doxygen-tag.sh the way static.check.yml does, with a changed-file list
# and the advanced flag. The per-function check had passed vacuously for two
# years because a stray "local" emptied the ctags kind list, so the first
# property pinned here is the negative control: an undocumented function
# definition must fail. The rest pin the scope of the check (definitions
# everywhere, prototypes only in headers), the trailing "/**<" form, and the
# per-file reset of the tag state.
#
# Fixtures are generated into a temporary directory rather than committed, so
# that the intentionally undocumented ones never appear in a changed-file list.

set -u

SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd)
REPO_ROOT=$(cd "${SCRIPT_DIR}/../../.." && pwd)
CHECKER="${SCRIPT_DIR}/doxygen-tag.sh"
workdir=$(mktemp -d)
failed=0

trap 'rm -rf "$workdir"' EXIT

if ! command -v ctags > /dev/null 2>&1; then
echo "::error ctags is required by doxygen-tag.sh but is not installed."
exit 1
fi

# File-level tags every C/C++ fixture needs so that only the per-function
# rules decide the outcome. The include line matters: the checker treats a
# @brief as pending until the next code line, and a real file always has one
# between the header comment and the first function.
header_block() {
printf '/**\n * @file %s\n * @brief fixture\n * @author fixture\n * @bug none\n */\n#include <stddef.h>\n' "$1"
}

# write_fixture <name> <content>
# Creates workdir/<name> with the standard file header followed by content.
write_fixture() {
local name=$1 content=$2
{ header_block "$name"; printf '%s\n' "$content"; } > "${workdir}/${name}"
}

# run_checker <expected 0|1> <description> <fixture>...
# Runs the checker on the listed fixtures in advanced mode, from the repo
# root, and compares the exit status. Also fails on any stderr output, which
# is how the broken "local" and the unset report_path used to show up.
run_checker() {
local expected=$1 desc=$2
shift 2
local list errfile actual f

list=$(mktemp -p "$workdir")
errfile=$(mktemp -p "$workdir")
for f in "$@"; do
printf '%s\n' "${workdir}/${f}" >> "$list"
done

(cd "$REPO_ROOT" && bash "$CHECKER" "$list" 1) > /dev/null 2> "$errfile" < /dev/null
actual=$?
[[ $actual -ne 0 ]] && actual=1

if [[ "$actual" != "$expected" ]]; then
echo "FAIL: ${desc} expected exit ${expected}, got ${actual}"
failed=1
elif [[ -s "$errfile" ]]; then
echo "FAIL: ${desc} wrote to stderr:"
sed 's/^/ /' "$errfile"
failed=1
else
echo "PASS: ${desc} (exit ${actual})"
fi
}

write_fixture documented.c '/**
* @brief add one
*/
int
add_one (int x)
{
return x + 1;
}'
run_checker 0 "documented definition passes" documented.c

write_fixture undocumented.c 'int
add_one (int x)
{
return x + 1;
}'
run_checker 1 "undocumented definition fails" undocumented.c

write_fixture forward_decl.c 'static int add_one (int x);

/**
* @brief add one
*/
static int
add_one (int x)
{
return x + 1;
}'
run_checker 0 "undocumented forward declaration in a .c passes" forward_decl.c

write_fixture undocumented_proto.h 'int add_one (int x);'
run_checker 1 "undocumented prototype in a .h fails" undocumented_proto.h

write_fixture documented_proto.h '/**
* @brief add one
*/
int add_one (int x);'
run_checker 0 "documented prototype in a .h passes" documented_proto.h

write_fixture trailing_same_line.hh '/** @brief adder */
class adder
{
public:
/** @brief the class */
adder ();
int add_one (int x); /**< add one */
};'
run_checker 0 "trailing /**< on the declaration line passes" trailing_same_line.hh

write_fixture trailing_next_line.hh '/** @brief adder */
class adder
{
public:
/** @brief the class */
adder ();
virtual int add_one (int x) = 0;
/**< add one */
virtual int add_two (int x,
int y) = 0;
/**< add two, declared over two lines */
};'
run_checker 0 "trailing /**< on the following line passes" trailing_next_line.hh

write_fixture trailing_after_body.hh '/** @brief adder */
class adder
{
public:
/** @brief the class */
adder ();
virtual int add_one (int x)
{
return x + 1;
}
/**< add one, documented after the inline body */
};'
run_checker 0 "trailing /**< after an inline body passes" trailing_after_body.hh

write_fixture trailing_missing.hh '/** @brief adder */
class adder
{
public:
/** @brief the class */
adder ();
int add_one (int x);
int add_two (int x);
/**< documents add_two only */
};'
run_checker 1 "declaration without any doc between documented ones fails" trailing_missing.hh

write_fixture trailing_belongs_to_next.hh '/** @brief adder */
class adder
{
public:
/** @brief the class */
adder ();
int add_one (int x);
int count; /**< documents count, not add_one */
};'
run_checker 1 "a /**< on the next declaration does not cover this one" trailing_belongs_to_next.hh

write_fixture undocumented_struct.h 'struct point
{
int x;
};'
run_checker 1 "undocumented struct fails" undocumented_struct.h

# The tag must not survive a code line just because that line has a '*' in
# it: a pointer parameter is not a comment.
write_fixture pointer_line.h '/**
* @brief add one in place
*/
void add_one (int *x);
void add_two (int *x);'
run_checker 1 "a brief does not carry over a pointer-typed declaration" pointer_line.h

# A wrapped parameter list can put the '*' at the start of a line; that is
# still code, not a comment continuation.
write_fixture wrapped_pointer.h '/**
* @brief add one in place
*/
void add_one (int
*x);
void add_two (int *x);'
run_checker 1 "a brief does not carry over a wrapped pointer parameter" wrapped_pointer.h

# The converse: a ';' inside the comment block must not clear the tag.
write_fixture semicolon_in_comment.h '/**
* @brief add one in place; the caller owns x
* usage: add_one (&x);
*/
void add_one (int *x);'
run_checker 0 "a semicolon inside the comment block keeps the brief" semicolon_in_comment.h

write_fixture ends_in_brief.c '/**
* @brief add one
*/
int
add_one (int x)
{
return x + 1;
}
/**
* @brief a dangling comment that leaves the tag state set
*/'
run_checker 0 "file ending in a brief passes on its own" ends_in_brief.c

# The file-level tags sit at the bottom here so that nothing resets the tag
# state before the first function; only the per-file reset can fail it.
{ printf 'int
add_one (int x)
{
return x + 1;
}
'; header_block tail_header.c; } > "${workdir}/tail_header.c"
run_checker 1 "undocumented first function fails on its own" tail_header.c
run_checker 1 "tag state does not leak into the next file" ends_in_brief.c tail_header.c

write_fixture no_author.c '/**
* @brief add one
*/
int
add_one (int x)
{
return x + 1;
}'
sed -i '/@author/d' "${workdir}/no_author.c"
run_checker 1 "missing file-level @author fails" no_author.c

if [[ "$failed" != "0" ]]; then
echo "::error test_doxygen_tag.sh failed."
exit 1
fi
echo "test_doxygen_tag.sh: all checks passed."
17 changes: 17 additions & 0 deletions .github/workflows/static.check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,23 @@ jobs:
# Need "grep"
run: |
bash .github/workflows/static.check.scripts/doxygen-tag.sh $changed_file_list 1
- name: /Checker/ doxygen-tag self-test
# Runs a repository script unrelated to $changed_file_list. The step
# list comes from the merge ref while the checkout above is the PR
# head, so a tree that lacks this script - whether the branch
# predates it or a rename/deletion left it out without updating this
# step - would fail with "No such file or directory". Skip only when
# this PR did not remove it (see the elif below).
run: |
selftest=.github/workflows/static.check.scripts/test_doxygen_tag.sh
if [ -f "$selftest" ]; then
bash "$selftest"
elif git show --pretty="format:" --name-only --no-renames --diff-filter=D ${{ github.event.pull_request.head.sha }} -${{ github.event.pull_request.commits }} | grep -qxF "$selftest"; then
echo "::error::This PR removes $selftest; remove its workflow step too, or restore the script (also update this step if the script moved)."
exit 1
else
echo "Skipped: $selftest is not in this branch's tree and this PR does not remove it."
fi
- name: /Checker/ Indent check
# Originally from "pr-prebuild-indent"
# Need "indent"
Expand Down
Loading