# 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 - 循环终止条件正确 ### 常见问题模式 ```javascript // ❌ 问题: 未检查 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 管理 - [ ] **敏感数据** - 无硬编码密钥 - 日志不含敏感信息 - 传输加密 - [ ] **依赖安全** - 无已知漏洞依赖 - 版本锁定 ### 常见问题模式 ```javascript // ❌ 问题: 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 使用 - 避免阻塞 ### 常见问题模式 ```javascript // ❌ 问题: 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 - 及时更新 - 无冗余注释 ### 常见问题模式 ```javascript // ❌ 问题: 命名不清晰 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 适度 - [ ] **测试可维护性** - 命名清晰 - 结构统一 - 避免重复 ### 常见问题模式 ```javascript // ❌ 问题: 测试不独立 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 (架构) ### 检查清单 - [ ] **分层结构** - 层次清晰 - 依赖方向正确 - 无循环依赖 - [ ] **模块化** - 高内聚低耦合 - 接口定义清晰 - 职责单一 - [ ] **设计模式** - 使用合适的模式 - 避免过度设计 - 遵循项目既有模式 ### 常见问题模式 ```javascript // ❌ 问题: 层次混乱 (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** | 信息性建议、学习机会 |