diff --git a/claude/code/commands/review.md b/claude/code/commands/review.md new file mode 100644 index 0000000..f4e0078 --- /dev/null +++ b/claude/code/commands/review.md @@ -0,0 +1,29 @@ +--- +name: review +description: Perform a code review on staged changes, a commit range, or a GitHub PR +args: [--security] +--- + +# Code Review + +Performs a code review on the specified changes. + +## Usage + +Review staged changes: +`/code:review` + +Review a commit range: +`/code:review HEAD~3..HEAD` + +Review a GitHub PR: +`/code:review #123` + +Perform a security-focused review: +`/code:review --security` + +## Action + +```bash +"${CLAUDE_PLUGIN_ROOT}/scripts/code-review.sh" "$@" +``` diff --git a/claude/code/scripts/code-review.sh b/claude/code/scripts/code-review.sh new file mode 100755 index 0000000..8976524 --- /dev/null +++ b/claude/code/scripts/code-review.sh @@ -0,0 +1,187 @@ +#!/bin/bash +# Core code review script + +# --- Result Variables --- +conventions_result="" +debug_result="" +test_coverage_result="" +secrets_result="" +error_handling_result="" +docs_result="" +intensive_security_result="" +suggestions=() + +# --- Check Functions --- + +check_conventions() { + # Placeholder for project convention checks (e.g., linting) + conventions_result="✓ Conventions: UK English, strict types (Placeholder)" +} + +check_debug() { + local diff_content=$1 + if echo "$diff_content" | grep -q -E 'console\.log|print_r|var_dump'; then + debug_result="⚠ No debug statements: Found debug statements." + suggestions+=("Remove debug statements before merging.") + else + debug_result="✓ No debug statements" + fi +} + +check_test_coverage() { + local diff_content=$1 + # This is a simple heuristic and not a replacement for a full test coverage suite. + # It checks if any new files are tests, or if test files were modified. + if echo "$diff_content" | grep -q -E '\+\+\+ b/(tests?|specs?)/'; then + test_coverage_result="✓ Test files modified: Yes" + else + test_coverage_result="⚠ Test files modified: No" + suggestions+=("Consider adding tests for new functionality.") + fi +} + +check_secrets() { + local diff_content=$1 + if echo "$diff_content" | grep -q -i -E 'secret|password|api_key|token'; then + secrets_result="⚠ No secrets detected: Potential hardcoded secrets found." + suggestions+=("Review potential hardcoded secrets for security.") + else + secrets_result="✓ No secrets detected" + fi +} + +intensive_security_check() { + local diff_content=$1 + if echo "$diff_content" | grep -q -E 'eval|dangerouslySetInnerHTML'; then + intensive_security_result="⚠ Intensive security scan: Unsafe functions may be present." + suggestions+=("Thoroughly audit the use of unsafe functions.") + else + intensive_security_result="✓ Intensive security scan: No obvious unsafe functions found." + fi +} + +check_error_handling() { + local diff_content=$1 + # Files with new functions/methods but no error handling + local suspicious_files=$(echo "$diff_content" | grep -E '^\+\+\+ b/' | sed 's/^\+\+\+ b\///' | while read -r file; do + # Heuristic: if a file has added lines with 'function' or '=>' but no 'try'/'catch', it's suspicious. + added_logic=$(echo "$diff_content" | grep -E "^\+.*(function|\=>)" | grep "$file") + added_error_handling=$(echo "$diff_content" | grep -E "^\+.*(try|catch|throw)" | grep "$file") + + if [ -n "$added_logic" ] && [ -z "$added_error_handling" ]; then + line_number=$(echo "$diff_content" | grep -nE "^\+.*(function|\=>)" | grep "$file" | cut -d: -f1 | head -n 1) + echo "$file:$line_number" + fi + done) + + if [ -n "$suspicious_files" ]; then + error_handling_result="⚠ Missing error handling" + for file_line in $suspicious_files; do + suggestions+=("Consider adding error handling in $file_line.") + done + else + error_handling_result="✓ Error handling present" + fi +} + +check_docs() { + local diff_content=$1 + if echo "$diff_content" | grep -q -E '\+\+\+ b/(README.md|docs?)/'; then + docs_result="✓ Documentation updated" + else + docs_result="⚠ Documentation updated: No changes to documentation files detected." + suggestions+=("Update documentation if the changes affect public APIs or user behavior.") + fi +} + +# --- Output Function --- + +print_results() { + local title="Code Review" + if [ -n "$range_arg" ]; then + title="$title: $range_arg" + else + local branch_name=$(git rev-parse --abbrev-ref HEAD 2>/dev/null) + if [ -n "$branch_name" ]; then + title="$title: $branch_name branch" + else + title="$title: Staged changes" + fi + fi + + echo "$title" + echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" + echo "" + + # Print checklist + echo "$conventions_result" + echo "$debug_result" + echo "$test_coverage_result" + echo "$secrets_result" + echo "$error_handling_result" + echo "$docs_result" + if [ -n "$intensive_security_result" ]; then + echo "$intensive_security_result" + fi + echo "" + + # Print suggestions if any + if [ ${#suggestions[@]} -gt 0 ]; then + echo "Suggestions:" + for i in "${!suggestions[@]}"; do + echo "$((i+1)). ${suggestions[$i]}" + done + echo "" + fi + + echo "Overall: Approve with suggestions" +} + +# --- Main Logic --- +security_mode=false +range_arg="" + +for arg in "$@"; do + case $arg in + --security) + security_mode=true + ;; + *) + if [ -n "$range_arg" ]; then echo "Error: Multiple range arguments." >&2; exit 1; fi + range_arg="$arg" + ;; + esac +done + +diff_output="" +if [ -z "$range_arg" ]; then + diff_output=$(git diff --staged) + if [ $? -ne 0 ]; then echo "Error: git diff --staged failed." >&2; exit 1; fi + if [ -z "$diff_output" ]; then echo "No staged changes to review."; exit 0; fi +elif [[ "$range_arg" == \#* ]]; then + pr_number="${range_arg#?}" + if ! command -v gh &> /dev/null; then echo "Error: 'gh' not found." >&2; exit 1; fi + diff_output=$(gh pr diff "$pr_number") + if [ $? -ne 0 ]; then echo "Error: gh pr diff failed. Is the PR number valid?" >&2; exit 1; fi +elif [[ "$range_arg" == *..* ]]; then + diff_output=$(git diff "$range_arg") + if [ $? -ne 0 ]; then echo "Error: git diff failed. Is the commit range valid?" >&2; exit 1; fi +else + echo "Unsupported argument: $range_arg" >&2 + exit 1 +fi + +# Run checks +check_conventions +check_debug "$diff_output" +check_test_coverage "$diff_output" +check_error_handling "$diff_output" +check_docs "$diff_output" +check_secrets "$diff_output" + +if [ "$security_mode" = true ]; then + intensive_security_check "$diff_output" +fi + +# Print the final formatted report +print_results