# 代码审查标准掌握全面的代码审查实践，在关键问题进入生产之前发现它们。该技能涵盖安全漏洞、代码质量指标、性能优化、测试要求和文档标准，以确保每个拉取请求都符合专业的工程标准。 ## 简介代码审查是针对错误、安全漏洞和技术债务的最后一道防线。彻底的审查过程可以防止生产事故，维护代码质量，并在整个团队中传递知识。 **回顾哲学**： * **行为重于实现** \- 关注代码的作用，而不仅仅是代码的编写方式 * **安全第一的心态** \- 始终在代码质量之前检查漏洞 * **建设性反馈** \- 解释每条评论背后的“原因” * **问题假设** \- 如果有不清楚的地方，请在批准前询问 **何时审核与自动批准**： * ✅ **始终审查**：安全更改、身份验证、数据处理、数据库迁移、API 更改 * ✅ **始终审查**：复杂的业务逻辑、性能关键的代码、公共 API * ⚠️ **轻度审查**：文档更新、简单的拼写错误修复、依赖项更新（检查更改日志） * ❌ **绝不自动批准**：任何您不明白的地方 - 提出问题 ** ** ## 安全审查清单安全漏洞是代码审查中的最高优先级。系统地检查这些类别。 **有关包含示例的全面安全审查指南，请参阅**： * [references/security-review.md]() \- OWASP Top 10 深入探讨漏洞示例 * [examples/security-issues.md]() \- 真实世界的安全漏洞及修复 ### 身份验证和授权快速检查 **要检查什么**： * JWT 验证和令牌过期处理 * 会话管理和安全cookie配置 * RBAC（基于角色的访问控制）实施 * 密码散列和凭证存储（bcrypt、argon2） * OAuth/SSO 实施的正确性 **复习问题**： * 所有受保护的路由都位于身份验证中间件后面吗？ * 代码在敏感操作之前是否检查用户权限？ * 令牌是否针对每个请求进行验证（而不仅仅是缓存）？ * 密码重置流程安全吗（令牌、过期、速率限制）？ * 凭证是否从未硬编码在代码中？ ### 输入验证和清理快速检查 **要检查什么**： * SQL注入预防（仅限参数化查询） * XSS预防（正确的输出编码） * 防止命令注入（使用execFile，而不是shell命令） * 路径遍历预防 * 文件上传验证（类型、大小、内容扫描） **复习问题**： * 所有数据库查询是否参数化（无字符串连接）？ * 用户输入是否根据白名单（不仅仅是黑名单）进行验证？ * 文件上传是否受到类型、大小和恶意软件扫描的限制？ * 输出是否针对上下文（HTML、URL、JS）正确转义？ * 是否避免或正确清理了 shell 命令？ ### 数据保护快速检查 **要检查什么**： * 静态加密（数据库中的敏感数据） * 传输加密（HTTPS、TLS） * PII（个人身份信息）处理 * 秘密管理（环境变量，从不硬编码） * 数据保留和删除政策 **复习问题**： * 敏感数据在静态和传输过程中是否都经过加密？ * PII 字段是否得到正确识别和保护（GDPR 合规性）？ * 秘密是否从环境变量加载（从不提交到 git）？ * 代码是否符合数据保留政策？ * 敏感数据从未被记录吗？ ### OWASP Top 10 快速参考为了进行全面的安全审查，请验证代码不存在以下 OWASP 漏洞： 1. **访问控制损坏** \- 检查每个受保护资源的授权 2. **加密失败** \- 验证强加密，无弱算法 3. **注入** \- SQL、NoSQL、命令、LDAP注入预防 4. **不安全设计** \- 威胁建模、安全架构 5. **安全配置错误** \- 检查默认配置，错误消息不会泄露信息 6. **易受攻击的组件** \- 检查依赖项版本、已知 CVE 7. **身份验证失败** \- MFA、撞库防护、弱密码策略 8. **软件/数据完整性** \- CI/CD 安全性、未签名的软件包 9. **记录/监控失败** \- 记录安全事件，配置警报 10. **SSRF** \- 服务器端请求伪造预防 **请参阅 [references/security-review.md]() 了解详细示例和修复模式。** ** ** ## 代码质量标准除了安全性之外，代码还必须可维护、可读并遵循最佳实践。 ### 复杂性指标 **循环复杂度**：目标是复杂度 < 10 的函数。高复杂度（>15）是一种代码味道。 ❌ **不好**：高复杂性（15 个以上分支）函数 processOrder(订单、用户、库存) { if (order.type === '标准') { 如果（用户.isPremium）{ if (库存.库存 > 0) { if (订单.数量 < 库存.库存) { if (user. paymentMethod === '卡') { // ... 10 个以上的嵌套条件 } } } } } // 复杂度：20+，无法测试 } ✅ **好**：提取的函数（每个复杂度 < 10）函数 processOrder(订单、用户、库存) { 验证订单（订单）； checkInventory（订单，库存）； processPayment（订单，用户）； updateStock(订单、库存); } 函数 validateOrder(订单) { if (!order.type || !order.quantity) { throw new ValidationError("订单无效"); } } // 每个函数：复杂度3-5，易于测试 **复习问题**： * 函数是否简短且重点突出（理想情况是 < 50 行）？ * 嵌套深度是否合理（< 4 层）？ * 复杂的函数可以分解成更小的函数吗？ * 提前返回是否用于减少嵌套？ ### 代码异味 **需要标记的常见气味**： ❌ **重复代码**：在多个地方重复相同的逻辑 // 在组件A中 const Total = items.reduce((sum, item) => sum + item.price * item.quantity, 0); // 在组件B中 const Total = items.reduce((sum, item) => sum + item.price * item.quantity, 0); ✅ **好**：提取到共享实用程序 // 实用程序/cart.ts 导出函数calculateTotal(items) { return items.reduce((sum, item) => sum + item.price * item.quantity, 0); } ❌ **幻数**：无法解释的常量 if (user.age > 65) { ... } // 为什么是 65？ setTimeout(pollAPI, 5000); // 为什么是 5 秒？ ✅ **好**：命名常量常量退休年龄 = 65; 常量 API_POLL_INTERVAL_MS = 5000; if (用户.年龄 > 退休年龄) { ... } setTimeout(pollAPI, API_POLL_INTERVAL_MS); ❌ **长参数列表** : > 4 个参数是一种味道函数 createUser(姓名、电子邮件、密码、年龄、国家/地区、城市、邮政编码) { ... } ✅ **好**：对象参数函数 createUser({ 姓名、电子邮件、密码、个人资料 }: CreateUserParams) { ... } ❌ **God Objects/Functions** ：类/函数做了太多事情类用户服务{ 验证（）{...} 发送电子邮件() { ... } 处理付款() { ... } generateReport() { ... } // 职责太多 } ✅ **好**：单一职责原则类 AuthService { 验证（） { ... } } 类 EmailService { send() { ... } } 类 PaymentService { process() { ... } } 类 ReportService { 生成（） { ... } } ### 命名约定 **变量和函数**： * 使用描述性名称（除非众所周知，否则避免使用缩写） * 布尔变量应该是问题：`isValid`、`hasPermission`、`canEdit` * 函数应该是动词：`getUserById`、`calculateTotal`、`validateEmail` ❌ **坏**：模糊的名称 const d = new Date(); //“d”是什么意思？ function proc(x) { ... } // 什么是 proc？ x 是什么？ const temp = fetchData(); // 临时做什么？ ✅ **好**：描述性名称 const currentDate = new Date(); 函数 processPayment(订单) { ... } const userData = 等待 fetchUserProfile(); ### 代码组织（SOLID 原则） **单一职责**：每个类/函数都应该有一个更改的理由 **开放/封闭** ：开放用于扩展，封闭用于修改（使用组合/继承） **里氏替换**：子类型应该可以替换基本类型 **接口隔离**：许多特定接口>一个通用接口 **依赖倒置**：依赖于抽象，而不是具体 **复习问题**： * 每个函数/类是否都有一个明确的目的？ * 是否可以在不修改现有代码的情况下扩展设计？ * 是否注入了依赖项（不是硬编码）？请参阅 [examples/code-quality-examples.md]() 了解 15-20 个重构示例。 ** ** ## 性能考虑因素查找可能导致负载下生产问题的性能问题。 ### 数据库优化 **N+1 查询问题**：最常见的性能问题 ❌ **不好**：N+1 次查询（1 个帖子 + N 个作者） const posts =等待db.query('SELECT * FROM posts'); for (const post of posts) { post.author = wait db.query('SELECT * FROM users WHERE id = ?', [post.authorId]); // 这会执行 N 个额外的查询！ } ✅ **好**：使用 JOIN 进行单个查询 const posts = 等待 db.query(` SELECT p.*, u.name 作为作者姓名，u.email 作为作者电子邮件来自帖子 p 加入用户 u ON p.authorId = u.id `); **缺少索引**：检查未索引列上的查询 ❌ **坏**：全表扫描 SELECT * FROM 订单 WHERE customer_email = 'user@example.com'; -- 如果电子邮件未编入索引，则会扫描所有行 ✅ **好**：索引列在订单上创建索引 idx_orders_email(customer_email); SELECT * FROM 订单 WHERE customer_email = 'user@example.com'; **大型结果集**：始终分页 ❌ **Bad** ：返回所有结果 const allUsers = wait db.query('SELECT * FROM users'); // 可能有数百万！ ✅ **好**：分页 const 用户 = 等待 db.query( '从用户限制中选择*？偏移？'， [页面大小，页面*页面大小] ）；有关全面的数据库优化模式，请参阅 [references/performance-review.md]()。 ### 前端性能 **不必要的重新渲染**： ❌ **坏**：在渲染中创建新对象函数组件() { const 选项 = { key: 'value' }; // 每次渲染都会有新对象！返回 <子选项={选项} />; // 导致子进程重新渲染 } ✅ **好**：记忆函数组件() { const options = useMemo(() => ({ key: 'value' }), []); 返回 <子选项={选项} />; } **Bundle Size** ：检查大的依赖项 ❌ **不好**：导入整个库从'lodash'导入_； // 一个函数 70KB+ 从“时刻”导入时刻； // 68KB 用于日期格式 ✅ **好**：Tree-shakeable 导入从“lodash/debounce”导入去抖； // 2KB 从 'date-fns' 导入 { formatDistance }； // 2-10KB ### API设计 **高效的数据传输**： ❌ **不好**：没有分页，过度获取 app.get('/api/products', async (req, res) => { const 产品 = 等待 db.getAllProducts(); // 所有 10,000 种产品！ res.json（产品）； }); ✅ **好**：分页+选择性字段 app.get('/api/products', async (req, res) => { const { page = 1, limit = 20, fields = 'id,name,price' } = req.query; const products = wait db.getProducts({ page, limit, fields }); res.json（产品）； }); **复习问题**： * 数据库查询是否经过优化（索引、JOIN 而不是循环）？ * 列表端点是否实现了分页？ * 大型计算是否被缓存或记忆？ * 图像和代码是否使用延迟加载？ ** ** ## 测试要求代码审查应验证全面的测试覆盖率。 ### 覆盖阈值 **最低覆盖目标**： * **关键路径**（身份验证、支付、数据完整性）：100% * **业务逻辑**：90%+ * **用户界面组件**：70-80% * **实用程序/助手**：85%+ **覆盖率是必要的，但还不够**：100% 的覆盖率并不能保证代码没有错误。测试质量比数量更重要。 ### 测试质量（AAA 模式）良好的测试遵循**安排、执行、断言**： ❌ **不好**：测试实现细节 test('计数器增量', () => { 常量计数器 = 新计数器(); 计数器.值 = 5; // 直接设置内部状态期望(counter.value).toBe(5); // 测试实现，而不是行为 }); ✅ **好**：测试行为 test('点击按钮时计数器增加', () => { // 排列常量计数器 = 新计数器(); // 行动 counter.increment(); counter.increment(); // 断言期望(counter.getValue()).toBe(2); }); ❌ **Bad** ：没有明确含义的多个断言 test('用户创建', async () => { const user = wait createUser({ name: 'John' }); 期望（用户）.toBeDefined（）；期望(user.id).toBeTruthy(); Expect(用户名).toBe('约翰'); 期望(user.createdAt).toBeTruthy(); // 实际测试的是什么？ }); ✅ **好**：有明确意图的集中测试 test('createUser 为新用户分配唯一 ID', async () => { const user = wait createUser({ name: 'John' }); Expect(user.id).toMatch(/^[a-f0-9-]{36}$/); //UUID格式 }); test('createUser 设置createdAt 时间戳', async () => { const before = Date.now(); const user = wait createUser({ name: 'John' }); const after = Date.now(); 期望(user.createdAt).toBeGreaterThanOrEqual(之前); 期望(user.createdAt).toBeLessThanOrEqual(之后); }); ### 所需的测试类型 **何时需要每种类型**： **单元测试**（始终需要）： * 所有业务逻辑功能 * 实用程序和助手 * 数据验证 * 计算和转换 **集成测试**（需要）： * API端点 * 数据库操作 * 外部服务集成 * 多组件工作流程 **E2E 测试**（需要）： * 关键用户流程（注册、结帐、付款） * 复杂的多页面工作流程 * 涉及多种服务的功能 **复习问题**： * 测试涵盖所有新功能吗？ * 测试是否涵盖边缘情况和错误情况？ * 测试的重点是行为，而不是实施吗？ * 集成测试是否验证 API 合约？请参阅 [references/test-review.md]() 以获取全面的测试指南。 ** ** ## 文档期望文档应该解释_为什么_，而不仅仅是_什么_。 ### 何时需要文档 **始终记录**： * 公共 API（导出的函数、类、端点） * 复杂的算法或业务逻辑 *非显而易见的设计决策 * 安全考虑 * 性能优化 **可选文档**： * 不言自明的代码（好的命名使注释变得多余） * 明显的实现（标准CRUD操作） ### 记录什么 **函数/方法文档**： ❌ **不好**：多余的评论 // 通过ID获取用户函数 getUserById(id) { ... } ✅ **好**：有意义的文档 /** * 获取具有急切加载的首选项的用户配置文件。 * * @param id - 用户UUID * @returns 有偏好的用户，如果没有找到则为 null * 如果连接失败则@抛出DatabaseError * * 注意：此函数将结果缓存 5 分钟以减少数据库负载。 * 如果您需要最新数据，请使用 getUserByIdFresh()。 */ 异步函数 getUserById(id: string): Promise { ... } **复杂逻辑文档**： ❌ **不好**：没有解释常量价格 = 基价 * (1 + 税) * (1 - 折扣) + 运费； ✅ **好**：解释计算 // 计算最终价格，包括税费（加法）、折扣（乘法）和运费 // 公式：（基数×（1+税%）×（1-折扣%））+运费 // 示例：$100 × 1.08 × 0.90 + $5 = $102.20 常量价格 = 基价 * (1 + 税) * (1 - 折扣) + 运费； ### 文档格式 **使用适当的上下文格式**： * **JSDoc/TSDoc** : 对于 TypeScript/JavaScript 函数和类 * **内联注释** ：用于函数内的复杂逻辑 * **自述文件/文档**：用于架构、设置、API 指南 **复习问题**： * 公共 API 是否记录了参数、返回值和示例？ * 是否解释了复杂的算法（“为什么”，而不仅仅是“什么”）？ * 注释是否会增加价值（而不仅仅是重述代码）？ ** ** ## 审核流程最佳实践 ### 复习礼仪 **具有建设性**： * ✅“考虑将其提取到辅助函数中以实现可重用性” * ❌“这段代码太糟糕了” **提问**： * ✅“为什么你选择 X 方法而不是 Y 方法？” * ❌“这是错误的”（没有解释） **解释推理**： * ✅“参数化查询防止 SQL 注入 (OWASP #3)” * ❌“使用参数化查询”（无上下文） **表彰出色的工作**： * ✅“对这个边缘情况进行了很好的测试覆盖！” * ✅“重构得很好，现在更清晰了” 有关模板审核评论，请参阅 [examples/review-comments.md]()。 ### 常见评论 **安全**： *“🔒该端点需要身份验证中间件” *“🔒在数据库查询之前应验证用户输入” *“🔒不应记录敏感数据” **代码质量**： *“♻️考虑将此逻辑提取到可重用的函数中” *“♻️幻数 - 定义为命名常量” *“♻️可以通过提前返回来简化” **性能**： *“⚡检测到N+1查询-使用JOIN或急切加载” *“⚡这可以被记住以避免重新计算” *“⚡考虑对大型结果集进行分页” **测试**： *“🧪 缺少错误条件的测试用例” *“🧪这个测试很脆弱——测试实现，而不是行为” *“🧪此API端点需要集成测试” ### 何时批准与请求更改 **✅ 批准** : * 解决了所有关键问题（安全性、正确性） * 注意到一些小问题，但不要阻止（风格偏好） * 测试通过且覆盖范围足够 * 文档足够 **🔄 请求更改** : * 存在安全漏洞 * 测试缺失或失败 * 严重错误或逻辑错误 * 可能导致生产问题的性能问题 **💬 仅限评论**（无批准/拒绝）： * 建议改进但不阻止 * 提出问题以求澄清 * 分享知识或背景 ** ** ## 快速参考 ### 安全检查表 * 强制执行身份验证和授权 * 所有用户输入均经过验证和清理 * SQL 查询参数化（无字符串连接） * 敏感数据在静态和传输过程中加密 * 环境变量中的秘密（未硬编码） * 检查了 OWASP Top 10 漏洞 * 日志中没有敏感数据 ### 代码质量检查表 * 函数 < 50 行，复杂度 < 10 * 没有重复代码（DRY原则） * 有意义的变量/函数名称 * 幻数替换为命名常量 * 遵循坚实的原则 * 代码可读，没有过多注释 ### 绩效检查表 * 没有 N+1 查询（使用 JOIN 或预加载） * 查询列存在数据库索引 * 大型结果集分页 * 记忆昂贵的计算 * 大依赖树摇动 * 图片和代码延迟加载 ### 测试清单 * 覆盖率达到阈值（90%+ 业务逻辑） * 测试遵循 AAA 模式（安排、行动、断言） * 测试验证行为，而不是实现 * 涵盖边缘情况和错误情况 * API 集成测试 * 关键流程的端到端测试 ### 文档清单 * 记录公共 API (JSDoc/TSDoc) * 复杂算法解释 *非显而易见的决定是否合理 * 如果需要更新自述文件 * 评论增加价值（不多余） ** ** ## 相关技能 * `testing-strategy` \- 测试金字塔和综合测试模式 * `github-workflow-best-practices` \- PR 工作流程和审核流程 * `agency-workflow-patterns` \- 多代理代码审查协调（Reality Checker 代理）

代码审查标准

包含技能