code-review

Code Review

Systematic code review methodology for identifying issues, security vulnerabilities, and optimization opportunities.

Team Review Guidelines

Communication Style

When providing review feedback:

Be friendly and constructive

Good: "What do you think about using X here? It might help with Y."
Good: "Nice solution! One small suggestion..."
Avoid: "This is wrong."
Avoid: "You should have done X."

Ask before assuming

Good: "I'm curious about this approach - could you explain the reasoning?"
Avoid: "Why didn't you just...?"

Acknowledge good work

"Great catch on the edge case!"
"Clean and readable!"
"I learned something from this."

Double Verification Process

Every PR requires TWO passes:

Pass 1 - Security (Blocking)

All security checklist items MUST pass
Any security issue = Request Changes

Pass 2 - Quality (Advisory)

Standards, performance, maintainability
These are suggestions, not blockers (unless severe)

Review Phases

Phase 1: Preparation

Understand Context

Read PR description and linked issues
Understand the feature/fix being implemented
Check if there are related changes in other PRs
Review the commit history for context

Identify Standards

Check project's coding standards
Review existing patterns in the codebase
Note any specific requirements (performance, security)

Phase 2: Systematic Analysis

Work through each category methodically:

Security Review

Input Validation

// BAD: No validation
app.post('/user', (req, res) => {
db.query(`SELECT * FROM users WHERE id = ${req.body.id}`);
});

// GOOD: Parameterized + validated
app.post('/user', (req, res) => {
const schema = z.object({ id: z.number().positive() });
const { id } = schema.parse(req.body);
db.query('SELECT * FROM users WHERE id = ?', [id]);
});

Authentication & Authorization

// BAD: Missing auth check
app.delete('/api/posts/:id', async (req, res) => {
await Post.findByIdAndDelete(req.params.id);
});

// GOOD: Verify ownership
app.delete('/api/posts/:id', authenticate, async (req, res) => {
const post = await Post.findById(req.params.id);
if (post.authorId !== req.user.id &#x26;&#x26; !req.user.isAdmin) {
return res.status(403).json({ error: 'Forbidden' });
}
await post.delete();
});

Security Checklist

SQL/NoSQL injection prevented (parameterized queries)
XSS prevented (output encoding, CSP headers)
CSRF protection in place
Sensitive data not logged or exposed
API keys/secrets not hardcoded
File uploads validated and sanitized
Rate limiting on sensitive endpoints
Proper error messages (no stack traces in production)

Performance Review

N+1 Query Detection

// BAD: N+1 queries
const posts = await Post.find();
for (const post of posts) {
const author = await User.findById(post.authorId); // Query per post!
}

// GOOD: Eager loading
const posts = await Post.find().populate('author');

Performance Checklist

No N+1 queries (use eager loading)
Expensive operations cached appropriately
Database queries use proper indexes
Large datasets paginated
Async operations properly awaited
No unnecessary re-renders (React)
Memory leaks prevented (cleanup handlers)
Heavy computations memoized or offloaded

Code Quality Checklist

Clear, descriptive naming conventions
Functions are single-purpose (< 30 lines ideal)
Cyclomatic complexity reasonable (< 10)
No code duplication (DRY principle)
Proper error handling throughout
Edge cases considered
Magic numbers/strings extracted to constants
Comments explain "why", not "what"

Edge Cases Checklist

Always verify these scenarios are handled:

Data Edge Cases

Empty arrays/collections
Null or undefined values
Zero values (especially in division)
Negative numbers where only positive expected
Empty strings vs null vs undefined
Unicode and special characters in strings

User Input Edge Cases

Missing required fields
Fields with only whitespace
Duplicate submissions (double-click)
Concurrent updates to same resource
Invalid date formats
Timezone handling
File upload with no file selected

API Edge Cases

Network timeout handling
Rate limit exceeded responses
API returns unexpected data structure
Partial success in batch operations
Graceful degradation when service unavailable

WordPress-Specific Edge Cases

Post with no featured image
User with no display name
Taxonomy with no posts
Widget in empty sidebar
Shortcode with no attributes
Multisite subdirectory vs subdomain
Translation missing for locale
Cron job running during high traffic
Option not yet saved (first install)
Meta value of 0 vs meta not existing
get_post() returning null
WP_Query with no results
Current user not logged in (ID = 0)

Language-Specific Checks

PHP/WordPress

Escaping output (esc_html, esc_attr, etc.)
Nonces for form submissions
Capability checks for actions
Prepared statements for queries
WordPress coding standards followed
Cron hooks: No name collision between cron hook and internal do_action()
Cron scheduling: Events scheduled on activation, cleared on deactivation
Hook callbacks: No recursive/self-triggering patterns
Settings sanitization handles unchecked checkboxes
Transient expiration set appropriately
Object cache checked before expensive queries
REST endpoints have permission_callback
AJAX handlers verify nonce and capability

JavaScript/TypeScript

=== used instead of ==
Proper TypeScript types (no any abuse)
async/await error handling
No prototype pollution risks
ESLint/Prettier rules followed

React

Hooks rules followed
Keys provided for lists
useEffect dependencies correct
No state mutations
Proper component composition

Review Output Format

Structure feedback as:

Critical Issues (Must Fix)

Security vulnerabilities
Data loss risks
Breaking changes without migration
Failing tests

Warnings (Should Fix)

Performance concerns
Code maintainability
Missing error handling

Suggestions (Nice to Have)

Refactoring opportunities
Better naming
Additional documentation

Positive Notes

Good patterns followed
Clean code
Thorough testing

Safety Notice

Copy this and send it to your AI assistant to learn

Source Transparency

Related Skills

security

wp-security-review

php