java-code-review

What the source does

• FilesystemNone observedNo reads or writes in the source
• NetworkNone observedNo outbound calls in the source
• ShellNone observedNo commands run in the source
• DependenciesPinnedNothing floats
• SecretsNone foundNo keys in source

Five audit layers

Gates applied: no behavioural pass.

.claude/skills

Audit 0.4.1 · source sha 75a5ffc454a4 · 20 Oct 2026Report an issue or request a re-scan

• Publisherdecebals GitHub userOwner of the repository this skill is read from
• Source folderdecebals/claude-code-java/skills/java-code-reviewThe exact folder we audited
• LicenceMITAs the repository states it
• Commit pinned for this verdict75a5ffc454a4A new audit runs when the source changes
• Repository statusNot archivedLast push 43 days ago
• Sandboxed runNot runSkills are read, not executed; layer L4 is skipped

Its description: Systematic code review for Java with null safety, exception handling, concurrency, and performance checks. Use when user says "review code", "check this PR", "code review", or before merging changes.

---
name: java-code-review
description: Systematic code review for Java with null safety, exception handling, concurrency, and performance checks. Use when user says "review code", "check this PR", "code review", or before merging changes.
license: MIT
---

# Java Code Review Skill

Systematic code review checklist for Java projects.

## When to Use
- User says "review this code" / "check this PR" / "code review"
- Before merging a PR
- After implementing a feature

## Review Strategy

1. **Quick scan** - Understand intent, identify scope
2. **Checklist pass** - Go through each category below
3. **Summary** - List findings by severity (Critical → Minor)

## Output Format

```markdown
## Code Review: [file/feature name]

### Critical
- [Issue description + line reference + suggestion]

### Improvements
- [Suggestion + rationale]

### Minor/Style
- [Nitpicks, optional improvements]

### Good Practices Observed
- [Positive feedback - important for morale]
```

---

## Review Checklist

### 1. Null Safety

**Check for:**
```java
// ❌ NPE risk
String name = user.getName().toUpperCase();

// ✅ Safe
String name = Optional.ofNullable(user.getName())
    .map(String::toUpperCase)
    .orElse("");

// ✅ Also safe (early return)
if (user.getName() == null) {
    return "";
}
return user.getName().toUpperCase();
```

**Flags:**
- Chained method calls without null checks
- Missing `@Nullable` / `@NonNull` annotations on public APIs
- `Optional.get()` without `isPresent()` check
- Returning `null` from methods that could return `Optional` or empty collection

**Suggest:**
- Use `Optional` for return types that may be absent
- Use `Objects.requireNonNull()` for constructor/method params
- Return empty collections instead of null: `Collections.emptyList()`

### 2. Exception Handling

**Check for:**
```java
// ❌ Swallowing exceptions
try {
    process();
} catch (Exception e) {
    // silently ignored
}

// ❌ Catching too broad
catch (Exception e) { }
catch (Throwable t) { }

// ❌ Losing stack trace
catch (IOException e) {
    throw new RuntimeException(e.getMessage());
}

// ✅ Proper handling
catch (IOException e) {
    log.error("Failed to process file: {}", filename, e);
    throw new ProcessingException("File processing failed", e);
}
```

**Flags:**
- Empty catch blocks
- Catching `Exception` or `Throwable` broadly
- Losing original exception (not chaining)
- Using exceptions for flow control
- Checked exceptions leaking through API boundaries

**Suggest:**
- Log with context AND stack trace
- Use specific exception types
- Chain exceptions with `cause`
- Consider custom exceptions for domain errors

### 3. Collections & Streams

**Check for:**
```java
// ❌ Modifying while iterating
for (Item item : items) {
    if (item.isExpired()) {
        items.remove(item);  // ConcurrentModificationException
    }
}

// ✅ Use removeIf
items.removeIf(Item::isExpired);

// ❌ Stream for simple operations
list.stream().forEach(System.out::println);

// ✅ Simple loop is cleaner
for (Item item : list) {
    System.out.println(item);
}

// ❌ Collecting to modify
List<String> names = users.stream()
    .map(User::getName)
    .collect(Collectors.toList());
names.add("extra");  // Might be immutable!

// ✅ Explicit mutable list
List<String> names = users.stream()
    .map(User::getName)
    .collect(Collectors.toCollection(ArrayList::new));
```

**Flags:**
- Modifying collections during iteration
- Overusing streams for simple operations
- Assuming `Collectors.toList()` returns mutable list
- Not using `List.of()`, `Set.of()`, `Map.of()` for immutable collections
- Parallel streams without understanding implications

**Suggest:**
- `List.copyOf()` for defensive copies
- `removeIf()` instead of iterator removal
- Streams for transformations, loops for side effects

### 4. Concurrency

**Check for:**
```java
// ❌ Not thread-safe
private Map<String, User> cache = new HashMap<>();

// ✅ Thread-safe
private Map<String, User> cache = new ConcurrentHashMap<>();

// ❌ Check-then-act race condition
if (!map.containsKey(key)) {
    map.put(key, computeValue());
}

// ✅ Atomic operation
map.computeIfAbsent(key, k -> computeValue());

// ❌ Double-checked locking (broken without volatile)
if (instance == null) {
    synchronized(this) {
        if (instance == null) {
            instance = new Instance();
        }
    }
}
```

**Flags:**
- Shared mutable state without synchronization
- Check-then-act patterns without atomicity
- Missing `volatile` on shared variables
- Synchronized on non-final objects
- Thread-unsafe lazy initialization

**Suggest:**
- Prefer immutable objects
- Use `java.util.concurrent` classes
- `AtomicReference`, `AtomicInteger` for simple cases
- Consider `@ThreadSafe` / `@NotThreadSafe` annotations

### 5. Java Idioms

**equals/hashCode:**
```java
// ❌ Only equals without hashCode
@Override
public boolean equals(Object o) { ... }
// Missing hashCode!

// ❌ Mutable fields in hashCode
@Override
public int hashCode() {
    return Objects.hash(id, mutableField);  // Breaks HashMap
}

// ✅ Use immutable fields, implement both
@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof User user)) return false;
    return Objects.equals(id, user.id);
}

@Override
public int hashCode() {
    return Objects.hash(id);
}
```

**toString:**
```java
// ❌ Missing - hard to debug
// No toString()

// ❌ Including sensitive data
return "User{password='" + password + "'}";

// ✅ Useful for debugging
@Override
public String toString() {
    return "User{id=" + id + ", name='" + name + "'}";
}
```

**Builders:**
```java
// ✅ For classes with many optional parameters
User user = User.builder()
    .name("John")
    .email("[email protected]")
    .build();
```

**Flags:**
- `equals` without `hashCode`
- Mutable fields in `hashCode`
- Missing `toString` on domain objects
- Constructors with > 3-4 parameters (suggest builder)
- Not using `instanceof` pattern matching (Java 16+)

### 6. Res

Excerpt, first 6,000 characters of SKILL.md, verbatim from the audited commit. It is text from someone else’s repository, quoted so you can judge it before your agent reads it.

git clone https://github.com/decebals/claude-code-java
git -C claude-code-java checkout 75a5ffc454a4

Works in

1. 2026-10-20
   ✓SAFE B 89 / 100

   first audit · source 75a5ffc454a4

Questions about java-code-review

• github.com/decebals/claude-code-java/tree/main/skills/java-code- — the audited source at commit 75a5ffc454a4 and every finding aboveObserved · 20 Oct 2026

Spotted something wrong? Report a correction · Methodology