mirror of
https://github.com/alirezarezvani/claude-skills.git
synced 2026-10-08 03:07:51 +00:00
0e41cb2357
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
92 lines
4.8 KiB
Markdown
92 lines
4.8 KiB
Markdown
# 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`](./SKILL.md). This README is a quick reference for the 3 bundled scripts.
|
|
|
|
---
|
|
|
|
## How to use
|
|
|
|
### Quick install check
|
|
|
|
```bash
|
|
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
|
|
|
|
```bash
|
|
# 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
|
|
|
|
```bash
|
|
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
|
|
|
|
```bash
|
|
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`](./assets/sample_csharp_smells.cs) | C# file with every C#-specific pattern this skill detects, labelled inline |
|
|
| [`assets/sample_csharp_clean.cs`](./assets/sample_csharp_clean.cs) | Same code refactored per `rules/universal.md` + `languages/csharp.md` |
|
|
| [`assets/sample_java_smells.java`](./assets/sample_java_smells.java) | Java file with every Java-specific pattern this skill detects, labelled inline |
|
|
| [`assets/sample_java_clean.java`](./assets/sample_java_clean.java) | Same code refactored per `rules/universal.md` + `languages/java.md` |
|
|
| [`assets/sample_c_smells.c`](./assets/sample_c_smells.c) | C file with every C-specific pattern this skill detects, labelled inline |
|
|
| [`assets/sample_c_clean.c`](./assets/sample_c_clean.c) | Same code refactored per `rules/universal.md` + `languages/c.md` |
|
|
| [`expected_outputs/*.json`](./expected_outputs/) | Expected `code_quality_checker.py --json` output for each fixture |
|
|
|
|
Use them as a regression-detection harness:
|
|
|
|
```bash
|
|
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`](./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`](./SKILL.md)):
|
|
|
|
- [`rules/universal.md`](./rules/universal.md) — cross-language rules: security, async/concurrency, resource management, exception handling, performance
|
|
- [`languages/`](./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
|