代码审查技能
本技能提供系统化的代码审查方法论和检查清单。
审查流程
1. 快速浏览（2分钟）
先获得整体印象：
# 查看文件结构
ls -la
# 统计代码行数
wc -l *.cs
# 查看最近修改
git log --oneline -10
2. 逐项检查（核心）
按以下顺序检查：
- 安全问题 ← 最高优先级
- 正确性问题
- 性能问题
- 可维护性问题
- 风格问题 ← 最低优先级
安全检查清单
🔴 严重
-
SQL 注入: 是否使用参数化查询？
// ❌ 危险 $"SELECT * FROM Users WHERE Id = {userId}" // ✅ 安全 "SELECT * FROM Users WHERE Id = @Id", new { Id = userId } -
密码处理: 是否安全存储和比较？
// ❌ 危险 if (inputPassword == storedPassword) // ✅ 安全（常量时间比较） if (CryptographicOperations.FixedTimeEquals(hash1, hash2)) -
敏感数据: 日志中是否暴露密码/token？
// ❌ 危险 _logger.LogInfo($"User {user} logged in with password {password}"); // ✅ 安全 _logger.LogInfo($"User {user} logged in");
🟠 重要
- 输入验证: 所有外部输入是否验证？
- 权限检查: API 端点是否有授权？
- CORS 配置: 是否过于宽松？
- 依赖安全: 是否有已知漏洞？
# 检查 NuGet 包漏洞
dotnet list package --vulnerable
正确性检查清单
逻辑错误
-
边界条件: 空值、零、最大值是否处理？
-
异常处理: catch 块是否吞掉错误？
// ❌ 吞掉错误 catch (Exception) { } // ✅ 至少记录 catch (Exception ex) { _logger.LogError(ex, "Operation failed"); throw; } -
资源释放: IDisposable 是否正确处理？
// ❌ 可能泄露 var client = new HttpClient(); // ✅ 正确 using var client = new HttpClient();
并发问题
- 线程安全: 共享状态是否有竞态条件？
- 死锁: async 代码是否有
.Result或.Wait()？// ❌ 可能死锁 var result = GetDataAsync().Result; // ✅ 安全 var result = await GetDataAsync();
性能检查清单
常见问题
-
N+1 查询: 循环中是否有数据库调用？
// ❌ N+1 foreach (var order in orders) order.Customer = await db.GetCustomer(order.CustomerId); // ✅ 批量加载 var customerIds = orders.Select(o => o.CustomerId).Distinct(); var customers = await db.GetCustomers(customerIds); -
内存分配: 是否在热路径上频繁分配？
-
字符串拼接: 是否使用 StringBuilder？
-
LINQ 滥用: 是否多次枚举 IEnumerable？
检测工具
# 使用 BenchmarkDotNet
dotnet add package BenchmarkDotNet
# 内存分析
dotnet-counters monitor --process-id <PID>
可维护性检查清单
代码组织
- 单一职责: 类/方法是否只做一件事？
- 方法长度: 是否超过 50 行？考虑拆分
- 嵌套深度: 是否超过 3 层？考虑提前返回
命名
-
有意义: 名字是否描述意图？
// ❌ 含糊 var d = GetData(); // ✅ 清晰 var activeUsers = GetActiveUsers(); -
一致性: 是否遵循项目约定？
注释
- 必要性: 注释是否解释"为什么"而非"是什么"？
- 准确性: 注释是否与代码一致？
审查报告模板
## 代码审查报告
**文件**: src/Services/AuthService.cs
**审查者**: [你的名字]
**日期**: [日期]
### 🔴 严重问题
1. **第 45 行 - SQL 注入风险**
```csharp
// 当前
var query = $"SELECT * FROM Users WHERE Email = '{email}'";
// 建议
var query = "SELECT * FROM Users WHERE Email = @Email";
🟠 重要问题
- 第 78 行 - 缺少输入验证
用户输入
email未经验证直接使用。
🟡 建议
- 第 120 行 - 方法过长
ProcessOrder方法有 150 行，建议拆分为：ValidateOrder()CalculateTotal()SaveOrder()
✅ 优点
- 良好的异常处理
- 代码结构清晰
## 自动化工具
### 静态分析
```bash
# 使用 .NET Analyzers（内置）
# 在 .csproj 中启用
<PropertyGroup>
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<AnalysisLevel>latest-recommended</AnalysisLevel>
</PropertyGroup>
# 使用 SonarQube
dotnet sonarscanner begin /k:"project-key"
dotnet build
dotnet sonarscanner end
格式化
# 使用 dotnet format
dotnet format
# 检查但不修改
dotnet format --verify-no-changes
常见反模式
1. 上帝类 (God Class)
一个类做太多事情。拆分为多个单一职责的类。
2. 魔法数字
// ❌
if (status == 3) { ... }
// ✅
if (status == OrderStatus.Completed) { ... }
3. 注释掉的代码
删除它。用版本控制恢复。
4. 过早优化
先让它正确工作，再考虑优化。