Description
PR #655 added symlink skipping to scripts/license-headers.sh, but the implementation uses a fragile pattern to work around set -e:
validate_file "$file" || validate_result=$?
validate_result=${validate_result:-0}
This relies on a bash subtlety: || validate_result=$? prevents set -e from exiting on non-zero return, but only because the || operator makes the compound command succeed. If someone refactors this to a different structure (e.g., if ! validate_file "$file"; then ...), the set -e behavior changes.
Additionally, the magic return code 2 for symlinks is undocumented at the top of the script.
Proposed Solution
Use explicit conditional instead of || workaround:
# At top of script, document return codes
readonly VALIDATE_OK=0
readonly VALIDATE_ERROR=1
readonly VALIDATE_SYMLINK=2
# In the loop
if validate_file "$file"; then
validate_result=0
else
validate_result=$?
fi
if [[ $validate_result -eq $VALIDATE_SYMLINK ]]; then
echo -e "${YELLOW}⊘${NC} $file (symlink, skipped)"
files_skipped=$((files_skipped + 1))
continue
elif [[ $validate_result -ne $VALIDATE_OK ]]; then
files_failed=$((files_failed + 1))
failed_files+=("$file")
continue
fi
Context
Introduced in #655. The current code works but is fragile. From the Bash manual, set -e has complex interactions with || operators that can surprise maintainers.
Description
PR #655 added symlink skipping to
scripts/license-headers.sh, but the implementation uses a fragile pattern to work aroundset -e:This relies on a bash subtlety:
|| validate_result=$?preventsset -efrom exiting on non-zero return, but only because the||operator makes the compound command succeed. If someone refactors this to a different structure (e.g.,if ! validate_file "$file"; then ...), theset -ebehavior changes.Additionally, the magic return code
2for symlinks is undocumented at the top of the script.Proposed Solution
Use explicit conditional instead of
||workaround:Context
Introduced in #655. The current code works but is fragile. From the Bash manual,
set -ehas complex interactions with||operators that can surprise maintainers.