name: code-review-analysis description: 执行全面的代码审查，包括最佳实践、安全检查和建设性反馈。在审查拉取请求、分析代码质量、检查安全漏洞或提供代码改进建议时使用。

代码审查分析

概览

系统化的代码审查流程，涵盖代码质量、安全性、性能、可维护性和最佳实践，遵循行业标准。

使用场景

审查拉取请求和合并请求
在合并前分析代码质量
识别安全漏洞
向开发者提供建设性反馈
确保编码标准合规性
通过代码审查进行指导

指令

1. 初始评估

# 检查变更
git diff main...feature-branch

# 审查文件变更
git diff --stat main...feature-branch

# 检查提交历史
git log main...feature-branch --oneline

快速检查清单：

[ ] PR描述清晰完整
[ ] 变更与声明的目的相符
[ ] 未包含无关变更
[ ] 包含测试
[ ] 文档已更新

2. 代码质量分析

可读性

# ❌ 可读性差
def p(u,o):
    return u['t']*o['q'] if u['s']=='a' else 0

# ✅ 可读性好
def calculate_order_total(user: User, order: Order) -> float:
    """根据用户特定定价计算订单总额。"""
    if user.status == 'active':
        return user.tier_price * order.quantity
    return 0

复杂度

// ❌ 认知复杂度高
function processData(data) {
  if (data) {
    if (data.type === 'user') {
      if (data.status === 'active') {
        if (data.permissions && data.permissions.length > 0) {
          // 深层嵌套逻辑
        }
      }
    }
  }
}

// ✅ 通过早期返回降低复杂度
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;

  // 主逻辑在顶层
}

3. 安全审查

常见漏洞

SQL注入

# ❌ 易受SQL注入攻击
query = f"SELECT * FROM users WHERE email = '{user_email}'"

# ✅ 参数化查询
query = "SELECT * FROM users WHERE email = ?"
cursor.execute(query, (user_email,))

XSS防护

// ❌ XSS漏洞
element.innerHTML = userInput;

// ✅ 安全渲染
element.textContent = userInput;
// 或使用框架转义：{{ userInput }} 在模板中

认证与授权

// ❌ 缺少授权检查
app.delete('/api/users/:id', async (req, res) => {
  await deleteUser(req.params.id);
  res.json({ success: true });
});

// ✅ 适当的授权
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 });
});

4. 性能审查

// ❌ N+1查询问题
const users = await User.findAll();
for (const user of users) {
  user.orders = await Order.findAll({ where: { userId: user.id } });
}

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

# ❌ 低效列表操作
result = []
for item in large_list:
    if item % 2 == 0:
        result.append(item * 2)

# ✅ 列表推导
result = [item * 2 for item in large_list if item % 2 == 0]

5. 测试审查

测试覆盖率

describe('User Service', () => {
  // ✅ 测试边缘情况
  it('should handle empty input', () => {
    expect(processUser(null)).toBeNull();
  });

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

  // ✅ 测试正常路径
  it('should process valid user', () => {
    const result = processUser(validUserData);
    expect(result.id).toBeDefined();
  });
});

检查：

[ ] 新函数的单元测试
[ ] 新特性的集成测试
[ ] 覆盖边缘情况
[ ] 测试错误情况
[ ] 适当使用模拟/桩

6. 最佳实践

错误处理

// ❌ 静默失败
try {
  await saveData(data);
} catch (e) {
  // 空catch
}

// ✅ 适当的错误处理
try {
  await saveData(data);
} catch (error) {
  logger.error('Failed to save data', { error, data });
  throw new DataSaveError('Could not save data', { cause: error });
}

资源管理

# ❌ 资源未关闭
file = open('data.txt')
data = file.read()
process(data)

# ✅ 适当的清理
with open('data.txt') as file:
    data = file.read()
    process(data)

审查反馈模板

## 代码审查：[PR标题]

### 概要
变更的简要概述和整体评估。

### ✅ 优点
- 良好的错误处理结构
- 全面的测试覆盖
- 清晰的文档

### 🔍 发现的问题

#### 🔴 严重（必须修复）
1. **安全**：用户查询中的SQL注入漏洞（第45行）
   ```python
   # 当前代码
   query = f"SELECT * FROM users WHERE id = '{user_id}'"

   # 建议修复
   query = "SELECT * FROM users WHERE id = ?"
   cursor.execute(query, (user_id,))

🟡 中等（应该修复）

性能：N+1查询问题（第78-82行）
- 建议使用预加载以减少数据库查询

🟢 次要（考虑）

风格：考虑提取这个函数以提高可测试性
命名：proc_data可以更具描述性，如processUserData

💡 建议

考虑添加输入验证
可以增加额外的边缘情况测试
文档可以包括使用示例

📋 检查清单

[ ] 安全漏洞已解决
[ ] 测试已添加并通过
[ ] 文档已更新
[ ] 无console.log或调试语句
[ ] 错误处理适当

裁决

✅ 批准，有小建议 | ⏸️ 需要更改 | ❌ 需要重大修订


## 常见问题检查清单

### 安全
- [ ] 无SQL注入漏洞
- [ ] XSS防护到位
- [ ] 需要时的CSRF保护
- [ ] 认证/授权检查
- [ ] 无暴露的秘密或凭证
- [ ] 实施了输入验证
- [ ] 应用了输出编码

### 代码质量
- [ ] 函数专注且小
- [ ] 名称具有描述性
- [ ] 无代码重复
- [ ] 适当的注释
- [ ] 一致的风格
- [ ] 无魔术数字
- [ ] 错误消息有帮助

### 性能
- [ ] 无N+1查询
- [ ] 适当的索引
- [ ] 高效的算法
- [ ] 无不必要的计算
- [ ] 适当的缓存，如果有益
- [ ] 资源清理

### 测试
- [ ] 新代码包含测试
- [ ] 覆盖边缘情况
- [ ] 测试错误情况
- [ ] 如果需要，进行集成测试
- [ ] 测试可维护
- [ ] 无不稳定测试

### 可维护性
- [ ] 代码自解释
- [ ] 复杂逻辑已解释
- [ ] 无过早优化
- [ ] 遵循SOLID原则
- [ ] 依赖适当
- [ ] 考虑向后兼容性

## 工具

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

## 最佳实践

### ✅ 要做
- 建设性和尊重
- 解释建议背后的"为什么"
- 提供代码示例
- 如果不清楚，提问
- 承认好的做法
- 专注于重要问题
- 考虑上下文
- 复杂问题提供结对编程

### ❌ 不要做
- 过于批评或个人
- 吹毛求疵的小风格问题（使用自动化工具）
- 阻碍主观偏好
- 一次审查太多变更（>400行）
- 忘记检查测试
- 忽视安全影响
- 匆忙审查

## 示例

查看重构遗留代码技能，了解在代码审查中经常应用的详细重构示例。