Files
Claude-Code-Workflow/.claude/skills/review-code/specs/review-dimensions.md
catlog22 29c8bb7a66 feat: Add orchestrator and state management for code review process
- Implemented orchestrator logic to manage code review phases, including state reading, action selection, and execution loop.
- Defined state schema for review process, including metadata, context, findings, and execution tracking.
- Created action catalog detailing actions for context collection, quick scan, deep review, report generation, and completion.
- Established error recovery strategies and termination conditions for robust review handling.
- Developed issue classification and quality standards documentation to guide review severity and categorization.
- Introduced review dimensions with detailed checklists for correctness, security, performance, readability, testing, and architecture.
- Added templates for issue reporting and review reports to standardize output and improve clarity.
2026-01-13 14:39:16 +08:00

6.8 KiB

Review Dimensions

代码审查维度定义和检查点规范。

When to Use

Phase Usage Section
action-deep-review 获取维度检查规则 All
action-generate-report 维度名称映射 Dimension Names

Dimension Overview

Dimension Weight Focus Key Indicators
Correctness 25% 功能正确性 边界条件、错误处理、类型安全
Security 25% 安全风险 注入攻击、敏感数据、权限
Performance 15% 执行效率 算法复杂度、资源使用
Readability 15% 可维护性 命名、结构、注释
Testing 10% 测试质量 覆盖率、边界测试
Architecture 10% 架构一致性 分层、依赖、模式

1. Correctness (正确性)

检查清单

  • 边界条件处理

    • 空数组/空字符串
    • Null/Undefined
    • 数值边界 (0, 负数, MAX_INT)
    • 集合边界 (首元素, 末元素)
  • 错误处理

    • Try-catch 覆盖
    • 错误不被静默吞掉
    • 错误信息有意义
    • 资源正确释放
  • 类型安全

    • 类型转换正确
    • 避免隐式转换
    • TypeScript strict mode
  • 逻辑完整性

    • If-else 分支完整
    • Switch 有 default
    • 循环终止条件正确

常见问题模式

// ❌ 问题: 未检查 null
function getName(user) {
  return user.name.toUpperCase();  // user 可能为 null
}

// ✅ 修复
function getName(user) {
  return user?.name?.toUpperCase() ?? 'Unknown';
}

// ❌ 问题: 空 catch 块
try {
  await fetchData();
} catch (e) {}  // 错误被静默吞掉

// ✅ 修复
try {
  await fetchData();
} catch (e) {
  console.error('Failed to fetch data:', e);
  throw e;
}

2. Security (安全性)

检查清单

  • 注入防护

    • SQL 注入 (使用参数化查询)
    • XSS (避免 innerHTML)
    • 命令注入 (避免 exec)
    • 路径遍历
  • 认证授权

    • 权限检查完整
    • Token 验证
    • Session 管理
  • 敏感数据

    • 无硬编码密钥
    • 日志不含敏感信息
    • 传输加密
  • 依赖安全

    • 无已知漏洞依赖
    • 版本锁定

常见问题模式

// ❌ 问题: SQL 注入风险
const query = `SELECT * FROM users WHERE id = ${userId}`;

// ✅ 修复: 参数化查询
const query = `SELECT * FROM users WHERE id = ?`;
db.query(query, [userId]);

// ❌ 问题: XSS 风险
element.innerHTML = userInput;

// ✅ 修复
element.textContent = userInput;

// ❌ 问题: 硬编码密钥
const apiKey = 'sk-xxxxxxxxxxxx';

// ✅ 修复
const apiKey = process.env.API_KEY;

3. Performance (性能)

检查清单

  • 算法复杂度

    • 避免 O(n²) 在大数据集
    • 使用合适的数据结构
    • 避免不必要的循环
  • I/O 效率

    • 批量操作 vs 循环单条
    • 避免 N+1 查询
    • 适当使用缓存
  • 资源使用

    • 内存泄漏
    • 连接池使用
    • 大文件流式处理
  • 异步处理

    • 并行 vs 串行
    • Promise.all 使用
    • 避免阻塞

常见问题模式

// ❌ 问题: N+1 查询
for (const user of users) {
  const posts = await db.query('SELECT * FROM posts WHERE user_id = ?', [user.id]);
}

// ✅ 修复: 批量查询
const userIds = users.map(u => u.id);
const posts = await db.query('SELECT * FROM posts WHERE user_id IN (?)', [userIds]);

// ❌ 问题: 串行执行可并行操作
const a = await fetchA();
const b = await fetchB();
const c = await fetchC();

// ✅ 修复: 并行执行
const [a, b, c] = await Promise.all([fetchA(), fetchB(), fetchC()]);

4. Readability (可读性)

检查清单

  • 命名规范

    • 变量名见名知意
    • 函数名表达动作
    • 常量使用 UPPER_CASE
    • 避免缩写和单字母
  • 函数设计

    • 单一职责
    • 长度 < 50 行
    • 参数 < 5 个
    • 嵌套 < 4 层
  • 代码组织

    • 逻辑分组
    • 空行分隔
    • Import 顺序
  • 注释质量

    • 解释 WHY 而非 WHAT
    • 及时更新
    • 无冗余注释

常见问题模式

// ❌ 问题: 命名不清晰
const d = new Date();
const a = users.filter(x => x.s === 'active');

// ✅ 修复
const currentDate = new Date();
const activeUsers = users.filter(user => user.status === 'active');

// ❌ 问题: 函数过长、职责过多
function processOrder(order) {
  // ... 200 行代码,包含验证、计算、保存、通知
}

// ✅ 修复: 拆分职责
function validateOrder(order) { /* ... */ }
function calculateTotal(order) { /* ... */ }
function saveOrder(order) { /* ... */ }
function notifyCustomer(order) { /* ... */ }

5. Testing (测试)

检查清单

  • 测试覆盖

    • 核心逻辑有测试
    • 边界条件有测试
    • 错误路径有测试
  • 测试质量

    • 测试独立
    • 断言明确
    • Mock 适度
  • 测试可维护性

    • 命名清晰
    • 结构统一
    • 避免重复

常见问题模式

// ❌ 问题: 测试不独立
let counter = 0;
test('increment', () => {
  counter++;  // 依赖外部状态
  expect(counter).toBe(1);
});

// ✅ 修复: 每个测试独立
test('increment', () => {
  const counter = new Counter();
  counter.increment();
  expect(counter.value).toBe(1);
});

// ❌ 问题: 缺少边界测试
test('divide', () => {
  expect(divide(10, 2)).toBe(5);
});

// ✅ 修复: 包含边界情况
test('divide by zero throws', () => {
  expect(() => divide(10, 0)).toThrow();
});

6. Architecture (架构)

检查清单

  • 分层结构

    • 层次清晰
    • 依赖方向正确
    • 无循环依赖
  • 模块化

    • 高内聚低耦合
    • 接口定义清晰
    • 职责单一
  • 设计模式

    • 使用合适的模式
    • 避免过度设计
    • 遵循项目既有模式

常见问题模式

// ❌ 问题: 层次混乱 (Controller 直接操作数据库)
class UserController {
  async getUser(req, res) {
    const user = await db.query('SELECT * FROM users WHERE id = ?', [req.params.id]);
    res.json(user);
  }
}

// ✅ 修复: 分层清晰
class UserController {
  constructor(private userService: UserService) {}
  
  async getUser(req, res) {
    const user = await this.userService.findById(req.params.id);
    res.json(user);
  }
}

// ❌ 问题: 循环依赖
// moduleA.ts
import { funcB } from './moduleB';
// moduleB.ts
import { funcA } from './moduleA';

// ✅ 修复: 提取共享模块或使用依赖注入

Severity Mapping

Severity Criteria
Critical 安全漏洞、数据损坏风险、崩溃风险
High 功能缺陷、性能严重问题、重要边界未处理
Medium 代码质量问题、可维护性问题
Low 风格问题、优化建议
Info 信息性建议、学习机会