基于diff的代码审查工具:从原理到CI集成实践
1. 我到底为什么又写了一个代码审查工具1.1 先说说手动 review 里最磨人的那部分我是一个开源项目的维护者仓库不大不小刚好卡在“一个人忙得过来但 PR 多起来就烦躁”的规模。每天打开 GitHub 的 Pull Requests 列表我一眼就能看出哪些 PR 是认真做了自测的哪些只是把代码从编辑器里复制过来就点了 create pull request。最典型的例子是函数名把复数写成单数import顺序乱得没法看日志级别全是print()明明项目里统一用了 Pydantic 却有人传 dict 进来。这些不是架构问题不是逻辑问题甚至很难算“bug”但它们每一件都需要我打开文件、看上下文、写评论、等提交人改掉、再重新 review。一个下午就这么没了。但是真让我绝望的不是这些小事而是这些小事里的“重复性”。我几乎是在用人工的方式做一台正则机器该做的事情。每次看到同类问题我都有同一个念头这种东西不应该是人肉查的。我需要的不是又一个 linter因为 linter 管的是整个文件而 review 的时候我只关心这次改动动了哪几行我更想要的是一套“挂在 diff 上的审查助手”它能看到我改了哪里、动了哪些符号、有没有把不该来的代码顺手带进来然后在我点开 PR 之前先替我过一遍。1.2 现有工具和我的需求之间差了什么当时我不是没试过别的方案。GitHub 自带 code scanning、CodeQL 的规则确实很强大但它的配置成本和学习曲线摆在那里属于“三个月用一次”的工具。市面上也有一些能把 AI 拉进 code review 的 SaaS 服务但它们的问题是代码要出网我对这个比较敏感而且价格也不便宜。更现实的一个痛点是我希望 review 规则能跟着自己团队的项目规范走比如“核心包不允许新增第三方依赖”“错误处理必须返回项目自定义的AppError”“所有文件名都应该是小写下划线”。这些东西并不通用别人帮不了我必须自己写。于是我把需求收敛成了三点第一工具必须能只看 diff而不是整个仓库这样跑得才快、噪音才少第二规则必须能通过插件方式扩展我用 Python 写那么规则插件最好也是 Python 函数第三输出必须既给人看又给 CI 用最好能直接生成 Markdown 报告或者小规模自定义 annotations。按这个思路我折腾出了一个人的开源项目名叫open-code-review。项目的定位是一个“跑在本地跑在 CI 里的轻量代码变更审查器”。它不替代人工 review但它可以代替你去翻那些一眼就能看到底的细节问题把你从琐碎里解放出来让你把脑力留给真正的设计评审。如果你也在维护一个有点规模的仓库或者你团队里想要统一但一直没统一的代码审查基线这个工具的使用思路应该是能给你一些启发的。1.3 三个设计原则diff 输入、插件式检查器、可解释输出动手写代码前我先把硬约束列了出来。第一原则是open-code-review只基于 diff 运行。我会把基础分支和目标分支之间的改动解析成结构化数据所有检查器都只看到这些改动而不是整个仓库文件。第二原则是插件式检查器。工具自己带的规则库很少因为我清楚每个团队都有自己的偏好统一内置一套“全人类都应该这么写”的规则是胡说八道。所以核心引擎负责解析、上下文收集和报告输出规则全部作为插件存在。第三原则是输出必须可解释。检查器输出的是“哪个文件、哪一行、为什么报、参考文档是什么”而不是一个干巴巴的错误码。做到这一点后续无论是人工处理还是接入 CI 门禁都方便得多。有了这三个原则我在后面写代码时就不用老纠结架构了。2. open-code-review 的架构把 diff 变成一条审查流水线2.1 以 diff 为中心的设计到底好在哪如果你之前写过静态分析工具你大概率是从“分析整棵 AST”入手的。但如果目标是代码审查一上来就分析全库反而会导致两个问题第一是性能太差一个大仓库全量解析动不动就几十秒放在 CI 里无法接受第二是误报率奇高因为那些“本来就在仓库里存在的问题”会一起蹦出来把真正的增量问题淹没掉。open-code-review把 diff 作为一切检查的输入源头。它会先调用 git 拿到改动的文件列表和每个文件的具体行号范围然后让各个检查器只针对这些行号去定位问题。比如一个检查器叫no-debug-print它不会检查整个项目有没有console.log它只会检查本次改动新增的代码里有没有console.log。这就非常符合人工 review 的习惯你不是去翻整个文件你是看着绿绿红红的 diff 判断这段新代码是否合格。这个思路救了我很多次尤其在大规模重构的 PR 中仓库里原本的旧问题一大堆而我只关心这次联调动了什么。为了确保检查器拿到的“改动上下文”足够丰富引擎还会计算每个 hunk 的前后文。也就是说虽然检查器只检查新增行但它可以看到这个新增行所在的函数名、类名、是否是函数签名的一部分这能帮助规则做出更合理的判断减少指鹿为马的情况。2.2 检查器到底能拿到哪些信息很多人会问如果只看 diff是不是能做的事太少比如说某个改动会导致类型错误但类型信息藏在另一个文件里光看 diff 根本不知道。我的回答是不要试图让检查器取代编译器它能拿到的信息既能保证“快”又能保证“够用”。open-code-review传给检查器的结构大致包含三类信息。第一类是文件信息包括路径、语言类型、当前分支上的相对路径第二类是 diff hunk 信息包含每个 hunk 的旧行号新行号、改动类型新增、删除、修改、前后若干行的原文第三类是轻量符号表引擎会对改动涉及的目标文件做一次快速 AST 扫描提取出当前文件里的函数、类、变量定义和 import 关系但不做跨文件的类型推导。这就是整个上下文容器的范围听起来朴素但对绝大多数规则来说已经够了。举个例子我写过一个规则叫no-sleep-in-tests它会抓time.sleep(的出现。仅凭 diff hunk它就能判断这个新增调用出现在测试目录还是生产代码目录。又比如规则avoid-todo-leak它会在 App 代码的新增行里匹配TODO注释。这些都不是什么高深能力但组合起来之后覆盖面相当可观。2.3 所有诊断信息统一成同一种数据结构检查器的执行结果会统一转换成诊断对象。这个对象包含file_path、line、column、severity、message、rule_id、suggestion和confidence。我特别重视confidence这个字段因为代码审查不是编译报错它本身就是概率性的同一段代码可能在不同团队里有不同风格所以一个置信度字段能帮助下游决定是该拦截还是仅提醒。统一的诊断结构带来的最大好处是“输出层可以随便换”。引擎底层只需要收集一个个 Diagnostic 对象然后在渲染阶段把它们转成 Markdown、JSON 或 SARIF。我最初只做了 JSON 输出后来因为要在 GitHub Actions 里展示 annotations所以补了 SARIF 渲染器。如果没有统一的结构这种扩展会非常痛苦。2.4 为什么选择了 Python 而不是 Go 或 Rust选语言这件事被朋友问过很多次。其实我最开始也考虑过 Go毕竟单二进制部署确实很香但最后还是选了 Python。原因很现实我是用 Python 做日常脚本和维护的遇到新规则需求我可以用最短时间写出来。另外 Python 内置了difflib第三方的tree_sitter绑定也用得顺手生态足够让我快速迭代。当然坏处也明显分发依赖 Python 环境跑起来比 Go 慢。但考虑到这个工具定位是“CI 中的前置过滤网”而不是“大规模静态分析集群”Python 的性能完全足够。如果你也想做类似的东西我的建议是除非你一开始就知道性能和分发是硬指标否则选择自己最熟的语言去把逻辑跑通远比选一个看起来很酷的语言然后中途放弃重要。3. 拿一条真实提交把流程跑通你才知道它值不值3.1 安装与初始化配置我习惯把工具做成标准的 Python CLI。安装就是常见的pip install open-code-review然后命令行里执行open-code-review init它会生成一个open-code-review.toml作为默认配置。这个配置文件负责三件事声明要启用哪些规则、告诉引擎要忽略哪些路径、以及设置规则的参数。一个最小配置大概长这样[core] reporter markdown root_dir . ignore_paths [vendor/, dist/, node_modules/] [checkers] enabled [ no_debug_print, no_sleep_in_tests, avoid_todo_leak, require_error_handler, ] [checkers.no_sleep_in_tests] test_paths [tests/, src/__tests__/]配置文件的格式本身没什么魔力但设计它的过程让我意识到一件事规则参数最好是显式的。像test_paths这种字段如果我写死在规则里那么换一个项目整个规则就得复制一遍但如果把它放出来规则就能做到“一码多用”。这正是插件化框架需要优先考虑的把可变的部分暴露给开发配置而不是写进代码里。3.2 命令行参数的设计取舍命令行主入口是open-code-review review。我通常会传两个参数--base和--head或者在不传参数时自动用origin/main和当前分支做 diff。还有--format用于指定输出格式--output指定输出文件--fail-on用于设置门禁级别比如error或warning。我还加了一个很有意思的参数叫--context-size用来控制每个检查器拿到的前后文行数默认是 3 行。有人可能会觉得这参数太底层了但实际使用中非常有用。比如写“检测日志里是否含用户敏感信息”的规则时上下文行数多一点才不至于把多行拼接的日志漏掉而在性能敏感的大仓库上把上下文缩到 1 行能明显提速。命令长这样open-code-review review --base origin/main --format markdown --output report.md如果审查结果里没有任何问题进程退出码是 0如果存在 error 级别问题退出码是 1。这个语义很简单任何 CI 脚本都能直接吃。3.3 一个真实场景下跑出来的报告我拿一个改造前的旧模块测试过。改动内容非常典型有人在一个工具函数里加了print(fdebug: {data})来排查问题然后忘记删同时他在测试文件里写了一个time.sleep(2)来等异步结果。这些都逃过了普通 linter 的眼睛因为 linter 默认运行在整个文件上并没有“这条调试语句是这次变更新增的”这个维度。跑一次open-code-review后Markdown 报告里会出现这样的片段| rule_id | file | line | severity | message | | --- | --- | --- | --- | --- | | no_debug_print | src/utils/helpers.py | 42 | error | 新增代码中包含了调试用 print请删除或用日志对象替代 | | no_sleep_in_tests | tests/test_async_utils.py | 18 | warning | 测试代码中出现固定 sleep容易造成 CI 不稳定建议使用 wait 条件 |这个报告让我眼前一亮的地方不是它抓到了问题而是它抓到了“新增代码里的问题”。如果我人工 review也要把 diff 移到那一行才能发现。而现在机器人已经帮我在提交阶段把所有这个泪点扫过一遍了。3.4 自动修复与手动 apply 的边界为了一些简单规则我还实现了open-code-review fix子命令。它能够对诊断对象里的suggestion字段做“机器可读补丁”处理。比如去掉print(行、把time.sleep替换成wait_until这类明确的转换。但修复原则非常克制只修“语义百分百不会变”的改动其余一律只能给出建议文案绝不自动改。我自己在设计中坚持了一条原则自动修复属于“锦上添花”不属于核心功能。因为代码审查的任务场景非常复杂同一个代码片段可能在团队里有特殊含义盲目自动修复比不修更可怕。如果你也在做类似工具我建议你一开始不要做自动修复先把诊断能力做扎实后面再考虑“哪些建议可以安全地自动执行”。4. 接入 GitHub Actions从本地扫描到 PR 自动评论4.1 一个实际在用的 workflow 长什么样本地跑通了下一步自然是让它进 CI。我不想每次手动拉分支跑命令而是希望每次 PR 打开或更新时工具自动对两个分支之间的 diff 执行审查然后以机器人的身份在 PR 底下留言。我使用的是 GitHub Actions。一个最小可用的 workflow 我贴在这里name: open-code-review on: pull_request: types: [opened, synchronize, reopened] jobs: code-review: runs-on: ubuntu-latest steps: - uses: actions/checkoutv4 with: fetch-depth: 0 - uses: actions/setup-pythonv5 with: python-version: 3.11 - run: pip install open-code-review - run: open-code-review review --base origin/main --format markdown --output report.md - name: Read report id: report run: | echo report_md$(cat report.md) $GITHUB_OUTPUT - name: Comment PR uses: actions/github-scriptv7 with: script: | const body process.env.REPORT_MD; await github.rest.issues.createComment({ issue_number: context.issue.number, owner: context.repo.owner, repo: context.repo.repo, body: body, }); env: REPORT_MD: ${{ steps.report.outputs.report_md }}关键点是actions/checkout一定要加fetch-depth: 0否则没有完整 git 历史git diff拿不到正确的 base 和 head。这一点我踩过坑第一次没写 fetch-depth结果工具永远拿到的是半截 diff报了一堆莫名其妙的错误。4.2 用GITHUB_TOKEN评论 PR还是用独立 bot我见过很多自动评论工具会要求你单独申请一个 GitHub App 或者 bot 账号理由是可以绕过GITHUB_TOKEN的权限限制。但对代码审查这个场景来说GITHUB_TOKEN完全够用而且它不需要额外配置密钥安全上也省心。唯一需要注意的是用GITHUB_TOKEN发评论时评论者显示为 “github-actions[bot]”无法在评论里 具体的人。如果你想在评论里提到提交者必须在脚本里显式读取 PR 的提交人信息再拼进 Markdown否则大家只能看到一条匿名机器人回复。另外如果希望 PR 不被机器人刷爆我会建议把汇总评论而不是逐行内联评论作为默认策略。原因在于逐行评论在某些仓库里被 REVIEWERS 视为“噪音”而且一旦提交人针对每条评论做了回复评论区会变得非常长。汇总评论恰好能一屏展示所有问题适合用来粗筛。4.3 汇总评论和内联评论我的最终取舍这里我算是想明白了关键不是内联好不好而是你要解决什么问题。如果你团队的目标是“合并代码前必须把所有 block 问题改掉”那汇总评论足够了因为提交人可以对照清单逐项修如果你希望机器人直接对某一行提出具体修改建议那内联评论更高效。open-code-review本身就支持输出github-action格式也就是把诊断对象映射成 GitHub 的 workflow command::error filesrc/utils/helpers.py,line42,titleno_debug_print::新增代码中包含了调试用 print这种格式可以直接被 GitHub 解析成 PR 文件行上的 annotation不需要调用任何 API。我在实践中是“内联 annotation 汇总评论”组合用内联负责定位问题汇总用来写总结意见。二者互相补充体验反而最好。4.4 把它当门禁用的时候务必要留后门把 open-code-review 集成到 CI 之后我一度想过如果 error 级别问题存在就不允许合并是否过于严厉后来发现过度严格的门禁一定会有人想办法绕过最常见的办法是直接 close 再开一个新的 PR把旧评论全部洗掉毕竟 review 工具不是强制流程。所以我最后采用的是“warning 不拦截error 拦截但允许维护者手动 override”.在 workflow 中我加了一个判断只有 PR 改动者不包括在某个admin-paths列表时才启用阻断。这种方式显然不是完美的自动门禁但它在“自动化”和“人的可干预性”之间找到了一种平衡。你可以在自己的工具里复制这个模式让门禁策略是可配置的而不是硬编码成一个二选一。5. 自定义检查器的真实开发流程从需求到落地5.1 检查器的三步约定open-code-review的插件体系非常简单约定大于配置。每个检查器是一个 Python 函数它接收一个ReviewContext返回一个Diagnostic列表。三段式结构非常容易记忆。from open_code_review import register_checker, ReviewContext, Diagnostic def check(context: ReviewContext) - list[Diagnostic]: diags [] for file_change in context.changed_files: if not file_change.path.endswith(.py): continue # 检查逻辑 return diags register_checker(my_custom_rule, check, defaults{severity: warning})这个设计我尽力让它保持直白。没有复杂的事件系统没有装饰器注册链就是一个普通函数挂到register_checker上。好处是新人接手自定义规则几乎零成本坏处是表达能力有限但“有限”有时候反而是控制的优点。5.2 用一个真实需求做例子禁止某个依赖不经过批准升级我举一个实际团队里会用到的例子管理员希望锁定requests库的版本不允许 PR 直接把版本从 2.28 升到 2.31除非有人在审查配置里显式声明允许。规则逻辑很简单对每个变更文件过滤出requirements.txt或pyproject.toml再匹配requests相关的依赖行。如果这行是新增的且版本号不在allowed_versions列表里就返回一条 error 诊断。import re from open_code_review import register_checker, ReviewContext, Diagnostic ALLOWED_REQUESTS {2.28.0, 2.28.1} def check_requests_pinned(context: ReviewContext) - list[Diagnostic]: diags [] for fc in context.changed_files: if fc.path not in (requirements.txt, pyproject.toml): continue for hunk in fc.hunks: for line in hunk.added_lines: if line.startswith(requests) and requests in line: version re.search(rrequests[^\d]*([\d.]), line) if version and version.group(1) not in ALLOWED_REQUESTS: diags.append(Diagnostic( file_pathfc.path, linehunk.new_start hunk.added_lines.index(line), severityerror, messagerequests 库版本未在允许升级列表中如需升级请联系维护者, rule_idrequests_pinned, suggestionf使用允许的版本之一: {, .join(sorted(ALLOWED_REQUESTS))}, )) return diags register_checker(requests_pinned, check_requests_pinned, defaults{severity: error})这个例子最值得看的其实不是正则本身而是“为什么规则只关心新增行”。如果旧版本本来就不符合规范升级 PR 没动它那处理它只会制造噪音。这一点思路可以说是我整个工具的哲学。5.3 给规则写单元测试的三种姿势规则一旦多了回归测试就成了刚需。open-code-review内置了一个小型的测试辅助模块让你可以不用启动完整 CLI就能直接调用检查器。第一种姿势是造一个最简单的ReviewContext只包含一个 hunk。第二种是使用真实仓库 fixture跑完整流程然后快照输出。第三种是针对纯函数单测比如把过滤逻辑抽出来单独测。def test_deny_requests_upgrade(): context make_context_from_diff( diff --git a/requirements.txt b/requirements.txt --- a/requirements.txt b/requirements.txt -10 10 -requests2.28.1 requests2.31.0 ) result check_requests_pinned(context) assert len(result) 1 assert result[0].rule_id requests_pinned如果你也想做类似项目我强烈建议早一点把这种测试通路铺好。规则引擎最怕的不是逻辑写得差而是改了一个通用辅助函数后几十个规则一起挂而你只能靠盲摸去定位。5.4 避免重复造轮子内置规则怎么复用我最初设计了一些内置规则但内置规则数量很少。不是我不想内置而是如果内置规则和自定义规则用完全不同的代码路径那自定义的灵活性就会大打折扣。因此我把内置规则也写成插件放在open_code_review.contrib.rules里。用户可以直接继承一个基础规则类也可以在自己规则文件里配置内置规则的参数。比如说你只需要把no_debug_print的检测对象从print扩展到pprint你不需要复制代码而只需要在配置里传一个[checkers.no_debug_print] expressions [print, pprint]这个参数会进入诊断对象里的rule_meta让大家一眼就能看出该规则的实际配置。规则复用做到这个程度团队内部定制成本基本就降到最低了。6. 项目用了大半年之后我总结出的几条实用经验6.1 误报率永远比漏报率更值得关注工具上线后我收到的第一个反馈不是“抓得好”而是“它怎么连这种东西都报”。如果有人对工具产生了不信任后面再想让团队接受就难了。所以我在设计上刻意让多数规则默认朝着“保守”走拿不准的问题只报 warning不报 error只对新增行做报告不对历史坏账多做纠缠。有个很典型的例子no_debug_print在开发期经常被误报因为有人会写print在业务逻辑里并不是调试而是命令行工具的正常输出。后来我给规则加了一个目录黑名单默认对cli/和scripts/目录放行这才把误报率降到可接受范围。没有完美的规则只有愿意倾听反馈并主动调整规则边界的维护者。6.2 它和 linter 不是替代关系而是上下层关系有人会问我项目里已经有ruff、ESLint、golangci-lint了为什么还要用open-code-review我的回答是它不是 linter 的替代品而是 linter 的“调度器”和“过滤器”。linter 的扫描范围是整个文件open-code-review会先把变更行提取出来再在变更行上精准地执行所有规则因此即使底层复用了一些开源 lint 引擎的规则也能跑得比“全量 lint 一遍再 diff 结果”更快、更准确。我在实际项目里是三层流水线第一层是 pre-commit 钩子里的快速 lint第二层是open-code-review的 diff 审查第三层才是人工的架构评审。每一层负责的事都不一样大概率可以做到不重叠。6.3 大 PR 和超大重构是这个工具目前最大的软肋如果 PR 里改了 500 个文件且大部分文件都是机械变更比如加函数签名参数、换包名、调缩进那么基于 diff 的引擎会被大量 hunk 淹没性能会明显下降。为此我做了一个硬性限制默认只处理前 200 个变化文件超出部分会在报告里提示“本次审查为部分覆盖”。这个局限让我意识到自动代码审查工具能解决的最舒服的场景是“小步快跑的日常 PR”而不是“蓄力三个月的一次大重构”。后者更适合交给人力去 review因为语义上的完整性判断目前还是人比工具强。如果你想在自己的项目里采用同类工具我给你的建议是先在小而频繁的 PR 里使用不要指望它一次解决巨型重构。6.4 从 open-code-review 延伸出来的两个方向这半年里我从项目里剥离出了两个值得继续的方向。第一个是把它内置的规则接口做成独立的规则包仓库允许团队上传自己的规则然后通过open-code-review install-rules package安装到本地配置。这样可以形成一个规则市场大家就不用把自己辛苦调出来的规范藏在公司代码库里。第二个是把诊断对象导出成与 AI 对话友好的结构化 JSON未来可以让大模型基于这些诊断结果做更深层的语义审查比如“这个问题是否与函数的设计意图相冲突”。我目前已经把这些想法整理进项目 roadmap但还没正式做到。如果你感兴趣可以在你自己的工具里试试类似思路先把规则引擎做薄把规则包做厚后续再叠加高级分析能力。这样做的最直接好处是即使某一天你想换掉底层分析引擎上面的规则和下游的评论流程都还能继续用。对我个人而言open-code-review带来的最大改变是让我重新理解了自己在 review 里到底该干什么。琐碎的机械检查交给脚本之后我反而愿意多花时间看调用关系、设计边界和测试覆盖率——这些才是一个维护者真正不可替代的价值。