请求贡献｜Rust 代码审查指南

张汉东

发布于 2023-10-06 12:42:51

2890

发布于 2023-10-06 12:42:51

文章被收录于专栏：Rust 编程

我在 GitHub 开源了一份 Rust 代码审查指南[1]（RCRG,Rust Code Review Guidelines），希望大家可以一起参与维护与完善。

前言

软件系统的代码工程质量和很多方面因素相关，比如代码的可读性、可维护性、性能等等，需要我们综合考虑。

虽然有强大的 Rust 编译器可以在编译期保证 Rust 代码的内存安全，但其实它也是有限制的。前提是你禁止了 Unsafe Code 完全编写 Safe Rust 代码。哪怕只用 Safe Rust，在并发场景下，也会有死锁、lockfree内存顺序指定不当而导致的数据竞争风险。即便内存安全保证了，整个系统还可能有信息安全（Security）风险。

如果仅仅依靠 Rust 编译器、rustfmt、clippy等工具还并不足够能保证最好的工程质量。所以，实际开发中，要保证整个系统代码的工程质量，必须有一套代码审查标准。最好是有一套代码审查的 Checklist 供审查者高效审阅代码，甚至为未来的 AI 审查代码建立一个标准。

Rust 代码审查表：

正确性（Correctness）
- 检查代码可以编译通过，没有警告。修复或文档化任何警告。
- 检查业务逻辑,确认没有错误或边界情况被遗漏。
- 验证错误处理是合适的。
- 确认 Unsafe 代码是正确的，并配备规范的文档。进一步参考安全性。
可读性（Readability）
- 确保代码易于阅读和理解。
- 检查命名是否清晰、描述性强，检查风格和格式是否一致、符合习惯。
- 验证注释解释了意图和复杂的部分。
- 确认代码是合理地组织到函数和模块中的。
可维护性（Maintainability）
- 检查重复逻辑,考虑合并。
- 指出哪些部分可以抽象成通用、可重用的部分。
- 查找抽象不当或过于复杂的情况。
- 确保测试覆盖关键路径和边界情况。
- 验证文档注释解释了实现细节。
- 检查代码组织结构是否合理，是否符合单一指责和开闭原则等
- 检查代码架构耦合性
安全性（Unsafe Safety && Security）
- Unsafe 代码的安全抽象是否规范合理，尤其是 FFI 边界。
- 识别潜在的缓冲区溢出、SQL注入等漏洞。
- 建议输入验证和过滤方法。
- 查找并发场景的静态条件和同步不当的情况，以及Unsafe 代码或 lockfree 场景下内存顺序指定不当而造成的数据竞争风险。
- 确保适当的认证和授权。
- 确保供应链安全，确保使用的外部库是可靠和安全的，避免使用已经被废弃或长时间没有更新的库。
性能（Performance）
- 查找明显的低效情况,如不必要的内存分配。
- 如果适用,建议更优的数据结构或算法。
- 指出哪些地方可以通过延迟计算、异步或并行来优化。
- 确保在性能优化之前有充分的性能基准测试文档以防止性能回退
接口设计（API Design）
- 考虑API的一致性、直观性,和潜在的可用性问题。
- 建议改进 API 命名、接口和文档。
- 指出 API 行为不明确或缺失的部分。
- 识别 Unsafe 代码暴露的隐患。
可调试性（Debuggability）
- 日志记录：代码中是否有足够的日志记录，特别是在关键路径和可能的错误点。日志应该是清晰的、有意义的，并且可以帮助开发者快速定位问题。
- 错误消息：错误消息是否清晰、具体，并提供了足够的上下文信息来帮助定位问题。
- 断言和验证：在关键点使用断言来验证假设，确保代码的行为是预期的。
- 文档和注释：确保复杂的代码段有足够的注释和文档，以帮助其他开发者理解其工作原理。
可观测性（Observability）
- 监控和指标：代码是否集成了监控工具，是否有足够的指标来监控系统的健康状况和性能。
- 追踪：如果应用是分布式的，是否集成了分布式追踪工具，如 OpenTelemetry ，以帮助跟踪请求在系统中的流转。
- 告警：是否设置了合适的告警阈值和通知机制，以便在出现问题时及时通知相关人员。
- 健康检查：对于服务和应用，是否有健康检查端点，以便监控工具和负载均衡器可以检查其健康状况。
其他
- 如果代码需要在多个平台上运行，确保考虑到跨平台的兼容性问题。
- 确保代码可以在 CI/CD 环境中正常编译和测试。