code-reviewer

name	code-reviewer
description	审查Go代码的最佳实践、bug、并发安全和业务逻辑。当用户要求代码审查或完成重要代码更改后调用。

代码审查器

此技能用于对遵循项目规范和最佳实践的Go项目进行全面的代码审查。

调用时机

• 用户要求代码审查
• 完成重要代码更改后
• 合并代码更改前
• 用户想要检查代码质量

审查清单

1. 代码风格和格式

• 变量、函数和类型名称是否清晰并遵循Go规范？
• 是否有不必要的代码注释或缺少关键注释？
• 代码是否使用gofmt格式化？

2. 错误处理

• 错误是否正确检查（if err != nil）？
• 错误消息是否有足够的上下文（例如：fmt.Errorf("xxx: %v", err)）？
• panic和recover是否使用得当（避免滥用）？

3. 并发安全

• 对共享变量的访问是否使用互斥锁（sync.Mutex）或原子操作保护？
• 通道是否正确用于goroutine同步？
• 是否存在潜在的死锁、竞态条件或goroutine泄漏？
• sync.WaitGroup和context.Context是否正确用于并发控制？

4. 性能和资源管理

• 是否有不必要的内存分配（例如：使用+连接字符串而不是strings.Builder）？
• 资源是否正确关闭（文件、网络连接、HTTP响应体）？适当时使用defer。
• 循环内的重复计算是否可以提取到外部？
• 是否可以使用sync.Pool减少对象分配？

5. 代码组织和可读性

• 函数是否过长？是否可以拆分为更小的函数？
• 是否遵循"提前返回"原则以减少嵌套？
• 包结构是否清晰且职责单一？
• 全局变量和包级状态是否使用得当？

6. 测试

• 测试中的错误是否正确处理（使用t.Fatal而不是panic）？

7. 常见陷阱

• 是否在循环内使用defer？（可能导致资源释放延迟，考虑使用匿名函数）
• 是否错误地复制了不可复制的类型，如sync.Mutex、sync.WaitGroup？
• 是否错误使用time.After导致内存泄漏？

8. 业务逻辑检查

• 客户端请求消息字段是否完全验证？
  • 整数字段：负值、过大的值
  • 数组：空检查
• 数值溢出问题：int32算术运算是否会超过最大/最小值？
• 除零问题：分母是否可能为零？
• 购买/兑换逻辑：先扣除成本，再发放物品
• 数组索引越界风险
• 空指针解引用风险
• 是否错误修改了cfg/data_mgr.go管理的配置数据,除了预处理配置数据据和重新加载配置数据,业务逻辑不能修改配置数据的值,否则就是严重错误

审查流程

1. 阅读代码文件以理解实现
2. 检查每个项目在审查清单中
3. 识别问题并按严重程度分类：
   • 严重：安全漏洞、数据损坏、崩溃
   • 重要：逻辑错误、资源泄漏、并发问题
   • 次要：代码风格、性能优化、可读性
4. 提供反馈，包括：
   • 问题描述
   • 文件位置和行号
   • 建议的修复或改进
   • 适用时提供代码示例

输出格式

## 代码审查报告

### 概述
代码质量和主要发现的简要概述。

### 严重问题
1. [文件:行号] 问题描述
   - 建议修复：...

### 重要问题
1. [文件:行号] 问题描述
   - 建议修复：...

### 次要问题
1. [文件:行号] 问题描述
   - 建议修复：...

### 建议
- 不是bug但可以提高代码质量的改进建议。

### 积极方面
- 突出代码中发现的好做法。

问题示例

空指针检查

// 错误
hero := bagHero.GetElem(req.GetId())
hero.Level += 1  // 潜在的空指针解引用

// 正确
hero := bagHero.GetElem(req.GetId())
if hero == nil {
    l.Error("hero not found", "heroId", req.GetId())
    return nil, errors.New("heroNotFound")
}
hero.Level += 1

整数溢出

// 错误
total := a + b  // 可能溢出int32

// 正确
if int64(a) + int64(b) > math.MaxInt32 {
    return errors.New("overflow")
}
total := a + b

资源管理

// 错误
file, _ := os.Open(path)
data, _ := io.ReadAll(file)

// 正确
file, err := os.Open(path)
if err != nil {
    return err
}
defer file.Close()
data, err := io.ReadAll(file)
if err != nil {
    return err
}

业务逻辑顺序

// 错误：先发放，后扣除（可能导致漏洞）
bag.AddItem(item)
bag.DelItems(cost)

// 正确：先扣除，后发放
if !bag.IsEnough(cost) {
    return errors.New("notEnough")
}
bag.DelItems(cost)
bag.AddItem(item)

注意事项

• 专注于可操作的反馈
• 优先考虑安全性和正确性而非风格
• 考虑项目现有的模式和规范
• 提供问题的上下文说明
• 建议具体的解决方案，而不仅仅是问题