claude-skills/engineering-team/skills/code-reviewer
Claude 0e41cb2357
feat(code-reviewer): C-specific smell detector + fixtures
Phase 2 / Tier 1 of the post-#769 audit. Until now, the deterministic
analyzer (scripts/code_quality_checker.py) had language-specific smell
detectors only for C# and Java; C / C++ / Rust / Ruby / PHP / Dart all
fell through to generic checks. This PR brings C onto the same footing
as C# and Java -- the security delta is largest for memory-unsafe
languages, so C goes first per the audit ranking.

What's detected (CERT C + CWE catalogue patterns)

  - Banned functions: gets, strcpy, strcat, sprintf, vsprintf
    (CWE-242 / CWE-120 family -- no bounds check on any of them)
  - Format-string vulnerability: printf(var) / syslog(var) where the
    first arg is a bare identifier instead of a literal (CWE-134).
    Suppressed when the first arg is a literal string.
  - Unbounded scanf: %s without a width specifier (CWE-120).
    Suppressed when a width is present (e.g. %31s).
  - malloc/calloc/realloc result not NULL-checked within 5 lines
    (CWE-690). Recognises if (p == NULL), if (NULL == p), if (!p),
    if (p != NULL).
  - free(p) without setting p = NULL on the next real line
    (CWE-416 use-after-free guardrail). Low severity since some
    style guides skip the zeroing convention.
  - system() with a non-literal argument (CWE-78 command injection).
    Suppressed when the argument is a string literal or NULL.

Implementation

  - New function check_c_specific_smells() in code_quality_checker.py,
    placed after check_java_specific_smells(). Reuses the existing
    _strip_csharp_comments helper -- C, C#, and Java share // and /* */
    comment syntax.
  - Wired into analyze_file() via the existing dispatcher pattern:
    `if language == "c": smells.extend(check_c_specific_smells(content))`.

Fixtures (regression-detection harness)

  - assets/sample_c_smells.c -- 67 lines, every detector pattern
    labelled inline with its CWE. Smells fixture produces 10 C-specific
    detector hits (strcpy fires twice intentionally, once in each
    function). Score: 4/100 (F).
  - assets/sample_c_clean.c -- same surface area refactored per
    rules/universal.md + languages/c.md. Zero C-specific hits.
    Score: 100/100 (A).
  - expected_outputs/sample_c_smells_quality.json and
    expected_outputs/sample_c_clean_quality.json -- committed JSON
    output mirrors the existing C# / Java regression-guard pattern.

Documentation

  - engineering-team/skills/code-reviewer/README.md
    - "Language-specific smell packs" line extended to enumerate the
      6 C-pack patterns alongside the existing C# and Java packs.
    - Bundled-fixtures table adds the 2 new C fixture rows.
  - engineering-team/skills/code-reviewer/SKILL.md
    - "Adding a New Language" step 5 reference: C# and Java -> C#,
      Java, and C.
    - "Regression Fixtures" paragraph reference: C# and Java -> C#,
      Java, and C.
  - docs/skills/engineering-team/code-reviewer.md mirrors the same
    SKILL.md updates.
  - CHANGELOG.md gets a new [Unreleased] section above the existing
    code-reviewer entry, documenting the detector + fixtures.

Regression

  - All 6 fixtures (C# / Java / C x smells / clean) pass byte-for-byte
    against expected_outputs/*.json. No drift introduced in C# or Java
    behaviour.

Not in this PR (Phase 2 audit, subsequent PRs)

  - check_<name>_specific_smells for C++, Rust, Python, Kotlin, PHP,
    Ruby, Dart, Go, Swift, TypeScript, JavaScript. C++ and Rust are
    the next-highest-leverage targets (smart-pointer ownership,
    unsafe block discipline). Same fixture + expected_outputs pattern
    will apply to each.

https://claude.ai/code/session_01SnXMhpyuAwrws26Wy4fizz
2026-05-28 14:27:26 +00:00
..
assets feat(code-reviewer): C-specific smell detector + fixtures 2026-05-28 14:27:26 +00:00
expected_outputs feat(code-reviewer): C-specific smell detector + fixtures 2026-05-28 14:27:26 +00:00
languages feat(code-reviewer): add C, C++, Rust, Ruby, PHP, and Dart/Flutter language support (#769) 2026-05-28 08:26:14 +02:00
rules refactor(code-reviewer): — universal rules + per-language files 2026-05-25 13:15:37 +01:00
scripts feat(code-reviewer): C-specific smell detector + fixtures 2026-05-28 14:27:26 +00:00
README.md feat(code-reviewer): C-specific smell detector + fixtures 2026-05-28 14:27:26 +00:00
SKILL.md feat(code-reviewer): C-specific smell detector + fixtures 2026-05-28 14:27:26 +00:00

code-reviewer

Code review automation for TypeScript, JavaScript, Python, Go, Swift, Kotlin, C#, .NET, Java, C, C++, Rust, Ruby, PHP, and Dart/Flutter. Analyzes PRs for complexity and risk, checks code quality for SOLID violations and code smells, and generates review reports.

The full skill spec is SKILL.md. This README is a quick reference for the 3 bundled scripts.


How to use

Quick install check

python scripts/pr_analyzer.py --help
python scripts/code_quality_checker.py --help
python scripts/review_report_generator.py --help

All three scripts are stdlib-only — no pip install required.

Example 1 — review a pull request

# From inside the repo you want to analyze:
python /path/to/skills/code-reviewer/scripts/pr_analyzer.py . --base main --head HEAD

Outputs: complexity score (1-10), risk categorization (critical / high / medium / low), prioritized review order, commit-message validation.

Example 2 — score a directory's code quality

python scripts/code_quality_checker.py /path/to/code

# Filter by language
python scripts/code_quality_checker.py /path/to/code --language csharp

# Machine-readable
python scripts/code_quality_checker.py /path/to/code --json

Outputs: quality score (0-100), letter grade, detected code smells, SOLID violations.

Example 3 — combine into a review report

python scripts/review_report_generator.py /path/to/repo --format markdown --output review.md

Outputs: review verdict (approve / request changes / block), score, prioritized action items.


Examples bundled with the skill

File Purpose
assets/sample_csharp_smells.cs C# file with every C#-specific pattern this skill detects, labelled inline
assets/sample_csharp_clean.cs Same code refactored per rules/universal.md + languages/csharp.md
assets/sample_java_smells.java Java file with every Java-specific pattern this skill detects, labelled inline
assets/sample_java_clean.java Same code refactored per rules/universal.md + languages/java.md
assets/sample_c_smells.c C file with every C-specific pattern this skill detects, labelled inline
assets/sample_c_clean.c Same code refactored per rules/universal.md + languages/c.md
expected_outputs/*.json Expected code_quality_checker.py --json output for each fixture

Use them as a regression-detection harness:

python scripts/code_quality_checker.py assets/sample_java_smells.java --json > /tmp/check.json
diff /tmp/check.json expected_outputs/sample_java_smells_quality.json
# silence means the detector still behaves as documented

What it detects

See SKILL.md for the full pattern list, severity tiers, and references. Quick summary:

  • PR Analyzer (scripts/pr_analyzer.py): hardcoded secrets / connection strings, SQL injection, debug statements (console.* / System.out / printStackTrace), analyzer suppressions (ESLint / Roslyn / @SuppressWarnings), any / dynamic overuse, TODO/FIXME, unsafe blocks, null-forgiving !, async void, blocking on Task.
  • Code Quality Checker (scripts/code_quality_checker.py): long methods, large files, god classes, deep nesting, too many parameters, high cyclomatic complexity, swallowed exceptions, missing await, undisposed IDisposable, new HttpClient() in method body, unused using directives. Language-specific smell packs for C# (async void, blocking on Task), Java (empty catch, printStackTrace, swallowed InterruptedException, unclosed resources, per-call ObjectMapper / Gson), and C (banned functions gets/strcpy/strcat/sprintf/vsprintf, format-string vulnerability printf(var), unbounded scanf("%s"), malloc-without-NULL-check, free-without-zeroing, system() with non-literal argument).
  • Review Report Generator (scripts/review_report_generator.py): combines the above into a single markdown or JSON verdict.

Review rules

Rules are split so every review loads exactly two files — the cross-language baseline plus one language guide (see the dispatch table in SKILL.md):

  • rules/universal.md — cross-language rules: security, async/concurrency, resource management, exception handling, performance
  • languages/ — one self-contained guide per language (python, typescript, go, swift, kotlin, csharp, java, c, cpp, rust, ruby, php, dart), each with Security / Async / Resource Management / Exception Handling / Performance / Idioms sections