bookworm/bookworm-smart-assistant
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
131840c962
bookworm-smart-assistant/skills/reviewer-expert/references/review-checklist.md

10 KiB
Raw Blame History

代码审查清单 (Code Review Checklist)

按语言和框架分类的审查要点，以及 PR 审查流程 SOP。

TypeScript/JavaScript 审查清单

类型安全

  • 禁止裸 any：所有 any 必须有注释说明原因，或替换为具体类型 / unknown
  • 类型断言最小化：as 断言需要注释理由；优先使用类型守卫 (type guard)
  • 泛型正确性：泛型参数有约束 (extends)，不使用无约束的 <T>
  • 严格模式兼容：strictNullChecks 下无隐式 undefined 访问
  • 联合类型穷举：switch 对联合类型使用 exhaustive check（never 兜底）
  • 外部数据验证：API 响应、用户输入使用 Zod / io-ts 运行时校验
// ❌ 裸 any，无校验
function handleResponse(data: any) {
  return data.user.name;
}

// ✅ 运行时校验 + 类型推导
const ResponseSchema = z.object({
  user: z.object({ name: z.string() }),
});
function handleResponse(raw: unknown) {
  const data = ResponseSchema.parse(raw);
  return data.user.name; // 类型安全
}

异步安全

  • Promise 错误处理：每个 await 都在 try-catch 中，或链式 .catch()
  • 竞态条件：组件卸载后不更新 state（AbortController / cleanup function）
  • 并发控制：批量请求使用 Promise.allSettled 而非 Promise.all（避免一个失败全部丢弃）
  • 超时处理：网络请求设置合理的 timeout
  • 重试机制：幂等操作配备指数退避 (exponential backoff)
// ❌ 竞态条件：组件卸载后仍设置 state
useEffect(() => {
  fetchData().then(data => setData(data));
}, [id]);

// ✅ 使用 AbortController 取消
useEffect(() => {
  const controller = new AbortController();
  fetchData(id, { signal: controller.signal })
    .then(data => setData(data))
    .catch(err => {
      if (!controller.signal.aborted) reportError(err);
    });
  return () => controller.abort();
}, [id]);

React 特定

  • Hooks 依赖数组：useEffect / useMemo / useCallback 依赖完整且无多余项
  • key 属性：列表渲染使用稳定唯一 key，禁止用 index 作 key（排序/删除场景）
  • 不必要的 re-render：大组件拆分，使用 React.memo / useMemo 防止子树重渲染
  • 状态提升 vs 下沉：state 放在最近的共同祖先，不过度提升到顶层
  • Context 粒度：避免巨大 Context 导致不相关组件重渲染
  • 受控 vs 非受控：表单组件模式一致，不混用

Next.js 特定

  • Server / Client 边界：'use client' 声明仅在必要时添加，Server Component 优先
  • 数据获取位置：Server Component 中直接 fetch，不在 Client Component 中请求可在服务端获取的数据
  • 缓存策略：fetch 的 cache / revalidate 选项合理；理解 ISR vs SSR vs SSG
  • Metadata：页面导出 metadata 或 generateMetadata 以支持 SEO
  • Loading / Error 边界：loading.tsx 和 error.tsx 文件完整
  • Route Handler 安全：API route 有鉴权中间件，参数经过校验

Python 审查清单

类型标注

  • typing 模块使用：函数签名完整标注参数和返回值类型
  • Pydantic model 验证：API 入参使用 Pydantic BaseModel，字段有 Field 约束
  • Optional 显式标注：可为 None 的字段使用 Optional[T] 或 T | None
  • 泛型容器：使用 list[str] 而非 List（Python 3.9+）
  • TypeAlias：复杂类型定义使用 TypeAlias 提高可读性
# ❌ 无类型标注，无验证
def create_user(data):
    return db.execute(f"INSERT INTO users VALUES ({data['name']})")

# ✅ Pydantic 验证 + 类型标注
class CreateUserRequest(BaseModel):
    name: str = Field(..., min_length=1, max_length=100)
    email: EmailStr

async def create_user(request: CreateUserRequest) -> UserResponse:
    return await user_service.create(request)

FastAPI 特定

  • 依赖注入：数据库 session、当前用户等通过 Depends() 注入
  • 错误处理：使用 HTTPException 返回合适状态码；全局异常处理器兜底
  • 中间件链：CORS、日志、限流中间件顺序正确
  • 路由组织：使用 APIRouter 按模块分组；路径命名 RESTful
  • 响应模型：response_model 显式声明，避免泄露内部字段
  • 后台任务：耗时操作使用 BackgroundTasks 或 Celery

异步安全

  • asyncio 使用：不在 async 函数中调用同步阻塞 I/O（使用 run_in_executor）
  • 连接池管理：数据库连接池大小合理；async session 正确关闭
  • 并发限制：使用 asyncio.Semaphore 控制并发数
  • 上下文变量：使用 contextvars 而非全局变量传递请求级数据

Go 审查清单

错误处理

  • error wrapping：使用 fmt.Errorf("context: %w", err) 包装错误，保留链路
  • sentinel errors：预定义错误使用 errors.New 并以 Err 前缀命名
  • 自定义 error：复杂错误实现 Error() 接口，携带结构化上下文
  • errors.Is / As：错误判断使用 errors.Is 而非 ==
  • 不忽略错误：_ = someFunc() 必须有注释说明原因
// ❌ 错误被忽略，无上下文
data, _ := json.Marshal(user)
db.Exec("INSERT INTO users VALUES (?)", data)

// ✅ 错误包装，逐层传递
data, err := json.Marshal(user)
if err != nil {
    return fmt.Errorf("序列化用户数据失败: %w", err)
}
if _, err := db.Exec("INSERT INTO users VALUES (?)", data); err != nil {
    return fmt.Errorf("插入用户记录失败: %w", err)
}

并发安全

  • goroutine 泄漏：每个 goroutine 有明确退出条件（context cancel / done channel）
  • mutex 使用：共享数据用 sync.Mutex 或 sync.RWMutex 保护；锁粒度合理
  • channel 模式：有缓冲 vs 无缓冲选择合理；select 配 default 或 context.Done()
  • sync.WaitGroup：Add 在 goroutine 外调用；Done 使用 defer
  • data race：运行 go test -race 检测竞态

Gin 特定

  • 中间件：鉴权、日志、Recovery 中间件顺序正确
  • 参数绑定：使用 ShouldBindJSON / ShouldBindQuery 而非手动取值
  • 响应格式：统一使用 c.JSON() 返回标准格式 { code, message, data }
  • 路由分组：v1 := r.Group("/api/v1") 按版本和模块分组
  • 优雅关停：使用 signal.Notify + srv.Shutdown(ctx) 处理退出

PR 审查流程 SOP

审查前准备

  1. 理解上下文：阅读 PR 描述和关联 issue，明确变更目的
  2. 检查 CI 状态：确认自动化测试通过、lint 无报错
  3. 浏览变更范围：先看 file changes 概览，判断影响面
  4. 本地检出（可选）：复杂变更建议本地运行验证

审查步骤（按优先级）

Step 1: 架构审查
  - 变更是否符合系统架构设计？
  - 模块职责划分是否合理？
  - 依赖方向是否正确（不反向依赖）？

Step 2: 逻辑正确性
  - 业务逻辑是否正确？边界条件是否覆盖？
  - 数据流转是否完整？状态管理是否一致？

Step 3: 安全检查
  - 输入是否经过验证和清理？
  - 是否存在注入、XSS、CSRF 风险？
  - 敏感数据是否加密 / 脱敏？

Step 4: 性能评估
  - 是否引入 N+1 查询、大数据量循环？
  - 缓存策略是否合理？
  - 数据库索引是否匹配新查询？

Step 5: 代码风格
  - 命名是否清晰、符合团队约定？
  - 代码组织是否合理？是否过度抽象？

反馈规范

  • 区分严重度：每条反馈标注级别（Blocker / Warning / Suggestion / Nit）
  • 提供替代方案：指出问题的同时给出建议的修改代码
  • 解释原因：说明为什么需要修改，而不是只说"改一下"
  • 肯定优点：发现好的设计和实现也要给予正面反馈
  • 对事不对人：审查代码而非审查人，使用"这段代码"而非"你的代码"

严重度分类标准

级别 标记 定义 示例 处理要求
Critical [BLOCKER] 必须修复才能合并。存在正确性、安全性或数据完整性问题 SQL 注入漏洞；竞态条件导致数据不一致；未处理的空指针异常 必须修复后重新审查
Warning [WARNING] 强烈建议修复。存在性能隐患、可维护性问题或潜在 Bug N+1 查询问题；缺少错误处理；过大的函数需要拆分 应当修复，可协商延期
Suggestion [SUGGESTION] 改进建议。提升代码质量但不影响功能 可以使用更清晰的命名；建议提取公共方法；增加类型约束 可选，鼓励采纳
Nit [NIT] 细微问题。风格偏好或微小优化 空行数量；import 排序；注释措辞 可选，不阻塞合并

审查反馈模板

**[BLOCKER]** SQL 注入风险
- 文件: `src/services/user.ts:42`
- 问题: 用户输入直接拼接到 SQL 查询字符串
- 建议:
  ```typescript
  // 使用参数化查询
  const user = await db.query('SELECT * FROM users WHERE id = $1', [userId]);

[SUGGESTION] 建议提取复用函数

  • 文件: src/utils/format.ts:15-28
  • 问题: 格式化逻辑在多处重复
  • 建议: 提取为 formatCurrency() 工具函数

---

## 审查自检清单（审查者使用）

### 提交审查前自检

- [ ] 是否审查了所有变更文件？
- [ ] 是否理解了变更的业务目的？
- [ ] Critical 问题是否都已标注？
- [ ] 反馈是否具有可操作性（有建议、有代码）？
- [ ] 是否检查了测试覆盖？
- [ ] 是否有正面反馈？