code-review-analysis

仓库: aj-geddes/useful-ai-prompts
安装量: 1.5K
排名: #981

安装

npx skills add https://github.com/aj-geddes/useful-ai-prompts --skill code-review-analysis

Code Review Analysis Overview

Systematic code review process covering code quality, security, performance, maintainability, and best practices following industry standards.

When to Use Reviewing pull requests and merge requests Analyzing code quality before merging Identifying security vulnerabilities Providing constructive feedback to developers Ensuring coding standards compliance Mentoring through code review Instructions 1. Initial Assessment

Check the changes

git diff main...feature-branch

Review file changes

git diff --stat main...feature-branch

Check commit history

git log main...feature-branch --oneline

Quick Checklist:

PR description is clear and complete Changes match the stated purpose No unrelated changes included Tests are included Documentation is updated 2. Code Quality Analysis Readability

❌ Poor readability

def p(u,o): return u['t']*o['q'] if u['s']=='a' else 0

✅ Good readability

def calculate_order_total(user: User, order: Order) -> float: """Calculate order total with user-specific pricing.""" if user.status == 'active': return user.tier_price * order.quantity return 0

Complexity // ❌ High cognitive complexity function processData(data) { if (data) { if (data.type === 'user') { if (data.status === 'active') { if (data.permissions && data.permissions.length > 0) { // deeply nested logic } } } } }

// ✅ Reduced complexity with early returns function processData(data) { if (!data) return null; if (data.type !== 'user') return null; if (data.status !== 'active') return null; if (!data.permissions?.length) return null;

// main logic at top level }

  1. Security Review Common Vulnerabilities

SQL Injection

❌ Vulnerable to SQL injection

query = f"SELECT * FROM users WHERE email = '{user_email}'"

✅ Parameterized query

query = "SELECT * FROM users WHERE email = ?" cursor.execute(query, (user_email,))

XSS Prevention

// ❌ XSS vulnerable element.innerHTML = userInput;

// ✅ Safe rendering element.textContent = userInput; // or use framework escaping: {{ userInput }} in templates

Authentication & Authorization

// ❌ Missing authorization check app.delete('/api/users/:id', async (req, res) => { await deleteUser(req.params.id); res.json({ success: true }); });

// ✅ Proper authorization app.delete('/api/users/:id', requireAuth, async (req, res) => { if (req.user.id !== req.params.id && !req.user.isAdmin) { return res.status(403).json({ error: 'Forbidden' }); } await deleteUser(req.params.id); res.json({ success: true }); });

  1. Performance Review // ❌ N+1 query problem const users = await User.findAll(); for (const user of users) { user.orders = await Order.findAll({ where: { userId: user.id } }); }

// ✅ Eager loading const users = await User.findAll({ include: [{ model: Order }] });

❌ Inefficient list operations

result = [] for item in large_list: if item % 2 == 0: result.append(item * 2)

✅ List comprehension

result = [item * 2 for item in large_list if item % 2 == 0]

  1. Testing Review

Test Coverage

describe('User Service', () => { // ✅ Tests edge cases it('should handle empty input', () => { expect(processUser(null)).toBeNull(); });

it('should handle invalid data', () => { expect(() => processUser({})).toThrow(ValidationError); });

// ✅ Tests happy path it('should process valid user', () => { const result = processUser(validUserData); expect(result.id).toBeDefined(); }); });

Check for:

Unit tests for new functions Integration tests for new features Edge cases covered Error cases tested Mock/stub usage is appropriate 6. Best Practices Error Handling // ❌ Silent failures try { await saveData(data); } catch (e) { // empty catch }

// ✅ Proper error handling try { await saveData(data); } catch (error) { logger.error('Failed to save data', { error, data }); throw new DataSaveError('Could not save data', { cause: error }); }

Resource Management

❌ Resources not closed

file = open('data.txt') data = file.read() process(data)

✅ Proper cleanup

with open('data.txt') as file: data = file.read() process(data)

Review Feedback Template

Code Review: [PR Title]

Summary

Brief overview of changes and overall assessment.

✅ Strengths

  • Well-structured error handling
  • Comprehensive test coverage
  • Clear documentation

🔍 Issues Found

🔴 Critical (Must Fix)

  1. Security: SQL injection vulnerability in user query (line 45) ```python # Current code query = f"SELECT * FROM users WHERE id = '{user_id}'"

# Suggested fix query = "SELECT * FROM users WHERE id = ?" cursor.execute(query, (user_id,))

🟡 Moderate (Should Fix) Performance: N+1 query problem (lines 78-82) Suggest using eager loading to reduce database queries 🟢 Minor (Consider) Style: Consider extracting this function for better testability Naming: proc_data could be more descriptive as processUserData 💡 Suggestions Consider adding input validation Could benefit from additional edge case tests Documentation could include usage examples 📋 Checklist Security vulnerabilities addressed Tests added and passing Documentation updated No console.log or debug statements Error handling is appropriate Verdict

✅ Approved with minor suggestions | ⏸️ Needs changes | ❌ Needs major revision

Common Issues Checklist

Security

  • [ ] No SQL injection vulnerabilities
  • [ ] XSS prevention in place
  • [ ] CSRF protection where needed
  • [ ] Authentication/authorization checks
  • [ ] No exposed secrets or credentials
  • [ ] Input validation implemented
  • [ ] Output encoding applied

Code Quality

  • [ ] Functions are focused and small
  • [ ] Names are descriptive
  • [ ] No code duplication
  • [ ] Appropriate comments
  • [ ] Consistent style
  • [ ] No magic numbers
  • [ ] Error messages are helpful

Performance

  • [ ] No N+1 queries
  • [ ] Appropriate indexing
  • [ ] Efficient algorithms
  • [ ] No unnecessary computations
  • [ ] Proper caching where beneficial
  • [ ] Resource cleanup

Testing

  • [ ] Tests included for new code
  • [ ] Edge cases covered
  • [ ] Error cases tested
  • [ ] Integration tests if needed
  • [ ] Tests are maintainable
  • [ ] No flaky tests

Maintainability

  • [ ] Code is self-documenting
  • [ ] Complex logic is explained
  • [ ] No premature optimization
  • [ ] Follows SOLID principles
  • [ ] Dependencies are appropriate
  • [ ] Backwards compatibility considered

Tools

  • Linters: ESLint, Pylint, RuboCop
  • Security: Snyk, OWASP Dependency Check, Bandit
  • Code Quality: SonarQube, Code Climate
  • Coverage: Istanbul, Coverage.py
  • Static Analysis: TypeScript, Flow, mypy

Best Practices

✅ DO

  • Be constructive and respectful
  • Explain the "why" behind suggestions
  • Provide code examples
  • Ask questions if unclear
  • Acknowledge good practices
  • Focus on important issues
  • Consider the context
  • Offer to pair program on complex issues

❌ DON'T

  • Be overly critical or personal
  • Nitpick minor style issues (use automated tools)
  • Block on subjective preferences
  • Review too many changes at once (>400 lines)
  • Forget to check tests
  • Ignore security implications
  • Rush the review

Examples

See the refactor-legacy-code skill for detailed refactoring examples that often apply during code review.

← 返回排行榜