ARTICLE · INTELLIGENCE

战地情报 · 详情页

来自尧图项目组的一线实战观察与深度解析

基于Git Diff的代码审查辅助工具:设计与CI集成实践

基于Git Diff的代码审查辅助工具:设计与CI集成实践

在团队里做代码审查,最让我头疼的不是审查本身,而是审查之前那一堆机械性工作:打开MR、挨个文件看 diff、找明显的风格问题、检查有没有遗留的 debug 代码、确认关键函数有没有补测试。这些事情不是说多难,而是重复性太高,人一疲劳就容易漏掉。后来我干脆花了两周时间,用 Python 写了一套开源的代码审查辅助工具,取名open-code-review。它不替代人做逻辑判断,而是把“人类审查员不爱干、又容易漏”的那部分活先干完,把 diff 里的可疑点、风格偏差、潜在的坑提前标出来,让真正的人工审查聚焦在逻辑和设计上。

这篇文章就把这个项目从思路、设计到落地全过程拆开来讲,包含完整的规则引擎写法、本地命令行工具的实现思路、CI 集成配置,以及我在真实项目里跑出来的效果数据。不管你是想自己搭一套类似工具,还是纯粹想看看“代码审查自动化”到底能做到什么程度,这篇应该都能给你一些可落地的参考。

1. 整体设计与模块拆分

1.1 项目定位:不是替代审查,而是审查辅助

开工之前我先把目标定清楚。市面上的代码审查工具大致分两类:一类是 SonarQube 这种重型平台,功能全但部署和配置成本高,小团队用起来很重;另一类是 ESLint、Pylint 这类语言级的 lint 工具,只管单文件语法和风格,不管“这次改动到底动了什么”。

我想要的是中间态:基于 git diff 的审查辅助。它不该管缩进是 2 空格还是 4 空格(那是 lint 的活),也不该管模块耦合度、圈复杂度(那是架构工具的活),它只关心一件事——这次代码改动里,有没有常见的、可被规则明确描述的“坏味道”。

定位清楚了,功能边界也就出来了:

  • 输入是 git diff(或者 GitHub/GitLab 的 MR 元数据);
  • 输出是带有文件路径、行号、严重级别和建议的审查意见列表;
  • 规则可配置、可插拔,每一条规则都是独立的 Python 函数;
  • 能被轻松集成进 CI,也能在本地命令行手动跑。

1.2 模块划分与数据流

项目核心分成四个模块,数据流是单向的,一眼就能看明白谁依赖谁:

解析层 → 模型层 → 规则层 → 输出层

先解释每一层干什么:

  • 解析层:读取git diff或统一 diff 格式的文本,拆解成文件维度的变更集合。这一步最关键的是拿到每个变更行的新旧行号。
  • 模型层:定义ChangedFile、ChangedHunk、ChangedLine三个核心数据结构,把 diff 这种“半文本半结构化”的东西,转成规则引擎可以直接查询的结构化对象。
  • 规则层:一堆函数,每个函数接收ChangedFile和ChangedHunk,返回若干条ReviewFinding。规则之间互相独立,互不调用,保证单条规则可测试、可禁用。
  • 输出层:把ReviewFinding列表格式化成终端输出的纯文本、SARIF 或 JSON 格式。JSON 格式主要是为了给 CI 的后续步骤消费。

我自认为这个设计最值钱的地方在于模型层。大多数从零开始的审查工具最后会变成一个“diff 文本正则大杂烩”,就是因为没做结构化建模。你告诉我要“检查新增了print调试代码”,直接用grep当然能查出来,但你拿不到行号,拿不到它属于哪个函数,更没法判断它是在新增代码里还是上下文代码里。有了模型层,规则只用关心“我拿到的是新增行列表”,不用管 diff 的+/-前缀解析——那太容易出边角 bug 了。

1.3 为什么用 Python 而不是 Go 或 Node

选型的时候我犹豫过要不要用 Go 写。Go 的优点是编译成单二进制文件,在 CI 里分发特别方便,不用管 Python 环境依赖。但最终选了 Python,理由有三个:

  1. 规则编写门槛低。团队成员不见得都熟悉 Go 的指针和 goroutine,但绝大多数后端工程师写 Python 规则没有心理负担。工具的长期生命力在于“团队能自己加规则”,而不在于“作者多会选技术栈”。
  2. Python 的 difflib 库原生支持 unified diff 解析,虽然性能一般,但对于一次 MR 的 diff 规模(通常几千行)绰绰有余;Go 反而没有标准库层面的统一 diff 支持,得引入pmezard/go-difflib。
  3. 做原型迭代快。第一个能用的版本我两个晚上就写出来了,如果是 Go,恐怕得一周。

性能方面现在看也没问题。一个 8000 行变动的超大 MR,本地跑完所有规则耗时约 1.8 秒,CI 场景(2 核 4GB 容器)下约 2.5 秒,完全可以接受。

2. 核心机制与规则引擎实现

2.1 diff 解析:行号映射是关键

diff 解析是整个工具的地基。Git 默认的 unified diff 格式长这样:

diff --git a/src/main.py b/src/main.py index 82bf9b1..a17c21f 100644 --- a/src/main.py +++ b/src/main.py @@ -15,7 +15,8 @@ def process_order(order_id: str) -> None: logger.info(f"开始处理订单: {order_id}") if not validate_order(order_id): raise ValueError("无效订单") - total = calc_total(order_id) + # TODO: 接入优惠券逻辑 + total = calc_total(order_id, include_coupon=True) save_order(order_id, total) return

解析这玩意儿你得注意几个坑。首先是@@这一行里不止有区块行号,还有一个可选的section_heading(这里的def process_order...),它是给人类看的上下文提示,不是代码本身,解析时要把@@ -15,7 +15,8 @@和后面的文本拆开。其次是连续多个 hunk 之间可能有若干行没被标识为新增或删除的上下文行,这些行不能混进“新增变更”集合。

更麻烦的是端到端行号映射。规则引擎经常需要回答这样一个问题:“新增的这一行,往前数第一个非空行的内容是什么?”用来判断它是否在某个函数体内部。这要求 hunk 数据结构里不仅要记录新旧行号,还要记录上下文行的归属,否则无法在规则里做“向上查找最近的一个def行”这类操作。

我的实现里,ChangedHunk会维护一份完整的行列表,每一行标记context、added或removed三种类型。added行同时持有新文件行号(用于定位)和它在 hunk 内的索引(用于向前查找)。规则想找“这个新增行属于哪个函数”,就从这个索引往前遍历,找到最近的def行,解析函数名。

2.2 规则注册机制:装饰器与优先级

规则引擎我用了最朴素的“装饰器注册 + 优先级排序”方案。核心实现大概是这样:

# core/registry.py from dataclasses import dataclass, field from typing import Callable, List @dataclass class RuleInfo: rule_id: str name: str description: str severity: str # "error" | "warning" | "info" tags: List[str] = field(default_factory=list) enabled: bool = True priority: int = 100 _RULES: dict[str, RuleInfo] = {} def register_rule(rule_id: str, name: str, description: str, severity: str = "warning", tags: list[str] = None, priority: int = 100): """装饰器工厂:把函数注册成审查规则""" def decorator(func): _RULES[rule_id] = RuleInfo( rule_id=rule_id, name=name, description=description, severity=severity, tags=tags or [], priority=priority, ) func.rule_id = rule_id return func return decorator def get_enabled_rules(config: dict) -> list[tuple[str, Callable]]: """根据配置文件过滤规则,返回 (rule_id, func) 列表""" result = [] for rule_id, info in _RULES.items(): if rule_id in config.get("disabled_rules", []): continue if config.get("disabled_rules_by_tag", []): if info.tags and any(t in config["disabled_rules_by_tag"] for t in info.tags): continue result.append((rule_id, _RULES[rule_id])) result.sort(key=lambda x: x[1].priority) return [(rid, _get_callable(rid)) for rid, _ in result]

每个规则函数的核心签名统一为def check(file: ChangedFile) -> list[ReviewFinding],写起来很简单。从实现到现在,规则库里已经积累了 60 多条规则,从 ASGI 项目特有的“视图函数里禁止裸print”到通用型的“新增了pdb.set_trace()必须拦截”,一条规则平均 30 到 50 行 Python。

2.3 核心规则类型与实现思路

我按实际价值给规则分了三类,每类挑两个最有代表性的拿出来讲讲实现思路。

第一类是“抓调试残留”型。这类规则最简单,价值却特别直接,因为人翻 MR 时最容易扫一眼就过,而测试环境跑了print或者breakpoint()进了生产代码,后果可大可小。

核心实现是扫描新增行,看有没有匹配正则:

# rules/debug_residual.py import re from core.models import ChangedFile, ReviewFinding from core.registry import register_rule _DEBUG_PATTERNS = [ (r"^\s*print\s*\(.*", "info", "存在 print 调试输出"), (r"^\s*breakpoint\s*\(.*", "error", "存在 breakpoint 调试断点"), (r"^\s*pdb\.set_trace\s*\(.*", "error", "存在 pdb 调试断点"), (r"^\s*console\.log\s*\(.*", "warning", "存在 console.log 调试输出"), (r"^\s*(logging\.)?debug\s*\(.*", "info", "存在 debug 级别日志输出"), ] @register_rule("debug_no_print", "禁止新增调试输出", "扫描新增代码中的 print/breakpoint/console.log 等调试残留", severity="warning", tags=["debug"], priority=10) def check_debug_residual(file: ChangedFile) -> list[ReviewFinding]: findings = [] for hunk in file.hunks: for idx, line in enumerate(hunk.lines): if line.type != "added": continue for pattern, severity, message in _DEBUG_PATTERNS: if re.match(pattern, line.content): findings.append(ReviewFinding( rule_id="debug_no_print", file_path=file.new_path, line_number=line.new_number, severity=severity, message=message, suggestion="删除调试代码,改用结构化日志系统(如 structlog)" )) break return findings

第二类是“异常处理缺失”型。这种规则稍微复杂一点,因为它需要跨行判断。比如检查“新增调用了外部 API,但没有 try/excess 捕获”。实现思路是:先匹配新增行里是否有requests.get(、httpx.get(、await client.post(等调用;如果有,从这一行开始向下遍历接下来 10 行内新增的代码,找有没有except关键字。找不到就报一个 warning。

这种“基于滑动窗口”的规则在本地 CLI 工具里表现不错,准确率大约在 85% 左右。误报主要来自调用处和 try 块不在同一个 hunk 里的情况,比如函数开头 try,调用在 30 行之后,或者调用结果被包了一个工具函数。这种误报在人工复审时一眼就能看出来,问题不大。

第三类是“变更范围异常”型。这是我觉得最有想象力的一类。它不和代码内容较劲,而是和“diff 本身”较劲。比如:

  • 一个 MR 里同时改了超过 20 个文件 → 建议拆分;
  • 一个文件的新增行数超过 500 行 → 提示“这个 PR 太大,能不能拆”;
  • 一个 hunk 里既有新增又有删除,且两边的公共部分超过 30 行 → 提示“可能只是改了个缩进”。

这类规则的作用不是找代码问题,而是给人工审查者提供全局视角。人看 MR 时很容易陷入细节,忘了退一步看全局。这类规则相当于在 MR 顶上加了一个横幅:“这个改动面有点大哦”。

2.4 误报抑制:路径白名单与行级注释

规则引擎做出来后,最大的痛点不是查不出来,而是查得太“吵”。有时候团队明确知道某条规则会有误报,但又不想直接禁用(因为大部分时候有用),所以需要更细粒度的抑制机制。

我实现了两种机制:

路径级白名单。在review_config.yaml里可以写:

rules: debug_no_print: enabled: true exclude_paths: - "tests/**" - "scripts/generate_fixture.py"

这解决的是“测试代码里用 print 完全合理”这样的诉求。

行级抑制注释。团队约定,如果新增了一行代码,而这行代码会被某条规则误报,就在行尾加一个注释:

total = calc_total(order_id, include_coupon=True) # no-review: debug_no_print

规则引擎在每条规则执行前,先检查该行代码是否以no-review: <rule_id>结尾,如果有就跳过。这个机制很像 ESLint 的eslint-disable-next-line,但在 review 工具里少了很多人会觉得烦。我最初担心团队会滥用,实际跑了两周后发现,团队成员比我想象中诚实,大多数时候宁可改代码也不会加抑制注释——因为加了还得解释为什么,反而更麻烦。

3. 与 CI/CD 集成及实操配置

3.1 本地命令行用法:先让人用顺手

工具如果只能在 CI 里跑,开发者在本地没法提前知道结果,使用率会大打折扣。所以我把 CLI 做了三个子命令:

# 1. 查看当前分支相对 main 的审查结果 open-code-review review --base main # 2. 只审查某个文件 open-code-review review --file src/models/order.py # 3. 输出 JSON,方便在终端里用 jq 做过滤 open-code-review review --base main --format json | jq '.findings[] | select(.severity == "error")'

本地使用体验的关键在于终端输出要给出行号和代码内容。我用 ANSI 转义序列给严重级别上了颜色:error 红色、warning 黄色、info 蓝色。这些细节虽然很小,但决定了一个工具是“偶尔想起才用”还是“每次提交前都想跑一下”。

CLI 实现的公共参数部分大概长这样:

# cli.py 核心逻辑 import argparse import sys from core.diff_parser import parse_git_diff from core.git import get_diff_from_base from core.registry import get_enabled_rules from core.review_runner import run_rules from output.console import format_findings_console def main(): parser = argparse.ArgumentParser(description="open-code-review: 基于 git diff 的代码审查辅助工具") parser.add_argument("--base", default="main", help="对比基准分支,默认 main") parser.add_argument("--file", help="只审查指定文件") parser.add_argument("--format", choices=["console", "json"], default="console") parser.add_argument("--config", default="review_config.yaml", help="配置文件路径") args = parser.parse_args() diff_text = get_diff_from_base(args.base, target_file=args.file) changed_files = parse_git_diff(diff_text) rules = get_enabled_rules(load_config(args.config)) findings = run_rules(changed_files, rules) if args.format == "console": sys.stdout.write(format_findings_console(findings)) sys.exit(1 if any(f.severity == "error" for f in findings) else 0) else: import json sys.stdout.write(json.dumps([f.to_dict() for f in findings], indent=2)) sys.exit(1 if any(f.severity == "error" for f in findings) else 0)

3.2 GitHub Actions 集成:一次配置,长期生效

本地能跑顺之后,接着就是进 CI。GitHub Actions 里的配置非常直接,我用的是自定义 Python 脚本方式,没有引入额外的 Action:

# .github/workflows/code-review.yml name: open-code-review on: pull_request: types: [opened, synchronize, reopened] jobs: auto-review: runs-on: ubuntu-latest permissions: contents: read pull-requests: write steps: - name: Checkout uses: actions/checkout@v4 with: fetch-depth: 0 - name: Setup Python uses: actions/setup-python@v5 with: python-version: "3.11" cache: "pip" - name: Install open-code-review run: | pip install open-code-review - name: Run review id: review run: | open-code-review review --base origin/main --format json > review_result.json echo "error_count=$(python -c " import json data = json.load(open('review_result.json')) errors = [f for f in data if f['severity'] == 'error'] print(len(errors)) ")" >> "$GITHUB_OUTPUT" - name: Comment on PR if: ${{ success() }} uses: actions/github-script@v7 with: script: | const fs = require('fs'); const findings = JSON.parse(fs.readFileSync('review_result.json', 'utf8')); if (findings.length === 0) return; const body = findings .filter(f => f.severity === 'error' || f.severity === 'warning') .slice(0, 30) .map(f => `**${f.severity.toUpperCase()}** \`${f.file_path}:${f.line_number}\` ${f.message}`) .join('\n'); await github.rest.issues.createComment({ issue_number: context.issue.number, owner: context.repo.owner, repo: context.repo.repo, body: `## 🤖 open-code-review 自动审查结果\n${body}` });

需要注意一个坑:actions/checkout默认只拉取当前分支的浅克隆,fetch-depth: 0必须加上,否则open-code-review review --base origin/main在拿不到 main 分支历史的时候会直接报错。我在最早一版配置里就忘了这个,跑了两次 CI 失败才反应过来。

3.3 配置文件管理与团队个性化

配置文件支持全局和项目两级合并。优先级是:项目里的review_config.yaml> 用户目录的~/.open-code-review.yaml> 内置默认配置。这样团队统一默认,个人可按需调整。

一个典型的项目配置长这样:

# review_config.yaml project: order-service disabled_rules: - debug_no_console_log_info disabled_rules_by_tag: - suggestions severity: warning # 即使有 warning 也不阻断 CI,只有 error 才阻断 rules: api_without_timeout: enabled: true include_paths: - "app/**" timeout_threshold_ms: 3000 scan_limits: max_files: 30 max_lines_per_file: 500 max_total_lines: 10000 # 超出范围后自动跳过规则分析,只返回提示

第 14 行那个scan_limits是我后来加的,因为确实碰到过超大 MR 把工具跑挂了的情况。限制加得比较保守:单文件超 500 行新增就直接提示“文件过大,建议拆分”,不跑规则;总新增超 10000 行就直接提示“建议基于主干切分支做增量提交”,全量跳过。这种设计符合“审查辅助工具不该成为 MR 合并的阻碍”的定位。

4. 落地效果与实际收益

4.1 在真实项目里的运行数据

工具从写好到现在,在我们组的 order-service 项目(Python + FastAPI)和账户中心项目(Node.js + TypeScript)上跑了大概 6 周。我拉了一组数据,虽然不是严格的对照组实验,但趋势足够说明问题。

order-service 项目,6 周内累计审查了 47 个 MR,共发现 218 条有效问题:

严重级别数量常见类型
error级27breakpoint 残留、未接数据库超时配置、API 调用无异常捕获
warning级86print 调试输出、日志级别过高、新增了 TODO 但无关联 issue
info级105文件过大建议拆分、函数过长建议重构、MR 修改文件数超过 20

error 级问题的命中率很稳定:平均每个 MR 0.57 个,也就是说大约每两个 MR 里就有一个会踩中至少一个 error 级问题。印象最深的一次是有人往一个处理用户校验的公共模块里加了一行print(json.dumps(user_info))。那个字段里带了用户手机号,如果没人发现合到 main 再部署,线上日志就会泄露个人信息。工具在 MR 的 review 评论里直接标了 error 级“存在 print 调试输出”,开发者在合并前就删掉了。

Node.js 项目那边的数据少一些,总共 23 个 MR、86 条问题。样式问题的占比明显更高,尤其console.log残留非常普遍。这也符合 JavaScript 生态里调试输出习惯比较多的现实。

4.2 人工审查时长变化的观察

我让团队的 6 名核心开发者在工具上线前后分别记录了自己“每次 MR 花在代码审查上的时间”。这个不是严格受控的实验,但变化趋势让我对工具的价值有了更清楚的判断:

  • 上线前:一个 300 到 500 行的小型 MR,平均审查时间约 15 到 25 分钟;
  • 上线后:同样的 MR,如果工具没有报任何 error 级问题,审查时间普遍降到 8 到 12 分钟;
  • 如果工具报了 error 级问题,审查者会先看工具标出的点,再决定是自己查看还是直接让开发者改,整体时间在 10 到 16 分钟之间。

我分析时间缩短的主要原因不是“不用看代码了”,而是工具帮审查者建立了“先看风险点,再看整体逻辑”的顺序。过去人会从头到尾一行行扫,担心漏掉什么;现在工具已经把明显问题筛出来了,人的注意力可以更放心地放在“这段逻辑是否合理”“这个接口设计是否优雅”这些更需要人类判断力的事情上。

4.3 团队反馈与迭代方向

工具上线第三周,我收集了一次团队反馈,比较重要的三条是:

第一点,“这个工具能不能提醒我‘函数多了个参数但没更新所有调用方’”?这个需求其实是 IDE 重构能力范畴,但说明团队已经开始把工具当成“程序分析助手”来用了。我考虑后续接入 AST 级别的分析,而不只停留在 diff 文本层面。

第二点,“如果我改了 A 文件,但 B 文件里有对应的测试没改,能不能提醒我”?这是个很有价值的跨文件关联需求。实现思路是维护一个“实现文件到测试文件的映射表”,发现修改了实现文件但没改对应测试文件时给一条 warning。这个我先做了个简化版:扫描 diff 中的实现文件列表,检查同目录test_*.py或*.test.ts是否也出现在 diff 里,没出现就提示。

第三点,“能不能把 INFO 级别的提示默认关掉,我只需要看 error 和 warning”。需求合理,我在review_config.yaml里加了min_severity配置项,同时在 CI 的 PR 评论里也默认只展示 warning 及以上级别。

5. 常见问题与排查实录

5.1 高频报错与解决方案速查表

自己写工具最大的幸福感之一,就是被团队用起来后,会收到各种各样的“报错”。我整理了一份高频问题表,后面维护的人可以直接照方抓药。

症状可能原因解决方案
fatal: Not a git repository在非 git 仓库目录下运行了工具先git init或在仓库目录下运行
拿到空 diff(什么都没审出来)当前分支和 base 分支指向同一个 commit确认执行前已经提交或至少git add了改动
CI 报错但本地跑没问题CI 里 checkout 没有fetch-depth: 0,浅克隆导致拿不到 base 分支在 checkout 步骤加with: fetch-depth: 0
中文字段名在终端显示乱码终端编码不是 UTF-8设置环境变量PYTHONIOENCODING=utf-8
ModuleNotFoundError: No module named 'yaml'本地翻译后依赖未安装完整pip install -e .或pip install pyyaml
超大 MR 执行时间超过 30 秒diff 过大且规则过多开启scan_limits限制,或按文件路径配置规则范围

5.2 规则误报的一个经典案例

有段时间团队总反应“新增 redis 调用的地方老是报 API 没有异常处理的 warning”。我一开始以为是规则写得太激进,后来查了实际报错内容,发现规则匹配了redis_client.get(key),但代码里明明有用 try/excess 包着。

盘了半小时才找到原因:规则检查“新增行之后 10 行内有没有 except”,而实际代码的结构是这样的:

def get_user_token(user_id: str) -> Optional[str]: try: token = cache.get(f"token:{user_id}") except RedisError: logger.warning("redis 查询失败") return None return token

问题出在def get_user_token这一个函数在 diff 里是整段新增的,但git diff默认会把新增代码和文化上相邻的少量上下文行放在同一个 hunk 里,try和except都在新增行范围里,规则应该能找到 —— 但我写 hunk 行号映射的时候,把except所在行的类型标错了,标成了context,而规则只在新added行里找except,自然找不到。

修法是:在 hunk 里保留完整的行序列,except行虽然不作为“新增变更”报告,但它依然是新增行队列的一部分,技术上用了 next-line 的回溯。这个 bug 让我意识到,diff 解析器里的“新增行”集合和“真正要审查的行”集合并不是一回事,后者需要包含 hunk 内的所有新增代码,包括结构化关键字。

5.3 想加自定义规则?从这三点开始

如果你拿到这个工具想加自己的规则,我的建议是从这三个方向入手,收益最高、门槛最低:

  1. 调试残留类:正则搜索新增代码里的console.log、print、debugger、TODO、FIXME。这类规则最容易写,团队接受度最高,几乎不会误报。
  2. 防御性编程检查:判断新增代码里有没有调用外部 IO(HTTP/RPC/DB)却没有超时参数或异常捕获。这类规则价值高,但需要你根据自己项目的框架和习惯做适配。
  3. 变更度量类:文件数量、新增行数超过阈值时提醒。这类规则基本没有误报,因为它不是指责“代码写得不好”,而是建议“PR 拆小一点”。

写规则的时候记住一个原则:规则宁缺毋滥。每加一条规则,你都要权衡它会不会带来持续性的误报。如果一个规则的命中率低于 60%,团队就会开始无视它的输出,进而你整套工具的信任度都会下降。我建议新规则先放到 warning 级别,跑两周,统计一下命中率,再决定是否升级到 error。

6. 从 v0.1 到 v0.5 的三个关键迭代

6.1 从“玩具脚本”到“命令行工具”的跃迁

v0.1 是一个 200 行的 Python 脚本,所有逻辑都塞在一个文件里,main()函数下面开着七八个笔记。它当时只能做一件事:解析git diff,检查有没有print和breakpoint。虽然简单,但团队用了之后觉得“挺有用”,我才决定继续往下走。

v0.2 最大的改变是引入Configuration类,支持 YAML 配置。这一版开始它能被不同团队使用了。v0.3 重构了 diff 解析,引入了 hunk 和 line 的建模,从此规则不再需要和裸文本作斗争。v0.4 加入了 CI 集成和 JSON 输出,它才真正成为“能在 MR 评论里说话的机器人”。v0.5 修复了多个误报问题,性能优化了 40%,同时增加了scan_limits保护机制。

回看这三个版本,我最大的体会是:别想着第一版就完美。如果我在最开始就规划好所有功能,可能这个工具现在还躺在我的笔记本里没写完,因为“规划”阶段永远会比“动手实现”阶段长。反而是先做出一个能解决身边小问题的脚本,让真实用户(哪怕只有一个)使用起来,才逼出了后续所有更有价值的功能。

6.2 性能优化:从 6 秒到 1.8 秒

v0.4 在老版本上有一次明显的性能问题:解析一个 3000 行 diff 要花 6 秒。分析下来,瓶颈不在 diff 解析,而在每条规则的输出都单独格式化了一次文件路径和行号的字符串拼接。

优化方案是给规则层传入一个延迟求值的上下文对象,规则只需要返回finding_id和参数,输出层在最终打印时才拼接完整消息:

@dataclass class FindingTemplate: message_key: str params: dict # 输出层统一渲染 MESSAGES = { "debug_no_print": "第 {line} 行存在调试输出 {expr},建议移除", "api_without_timeout": "第 {line} 行调用 {api} 未指定超时时间", # ... }

第二处优化是缓存在hunk里的“上一个非空行索引”,避免每条规则反复遍历行列表。改动后,3000 行 diff 的耗时从 6 秒降到了 1.8 秒,效率高了不少。

7. 如果你也想写一套类似的工具,我的建议

7.1 不要一开始就追求“覆盖所有语言”

我一开始幻想做一个多语言通用的审查工具,后来发现这是个大坑:Python 的“print 是坏的”和 JavaScript 的“console.log 是坏的”在语义上完全不同;Java 的异常处理机制和 Go 的 error 返回模式也完全不是一个套路。如果你做通用规则,最后只能停留在“检查 TODO、检查调试输出”这种低价值层面。

正确的策略是:选一个你团队当前最核心的语言,把那个语言相关的规则做深做扎实。比如我们团队是做 Python 的,就优先把“异常处理缺失”“资源未关闭”“日志级别不规范”这几点做好。把 Python 做好之后,再有精力才考虑其他语言。

7.2 规则引擎的注册机制务必保持简单

我见过不少审查工具项目,死因就是“规则机制设计得太复杂”。规则引擎该有的功能是:注册(让工具知道有这个规则)、开关(团队能关掉不想要的)、参数配置(支持按项目微调)。

如果你往里加“规则依赖”“规则之间的组合机制”“DSL 脚本语言支持”,规则编写的门槛就上去了,团队里不会再有第二个人写规则了。保持简单,让规则就是一个接收ChangedFile返回list[ReviewFinding]的普通函数,这才是让项目有生命力的核心。

7.3 工具的价值最终还是靠人定义

写代码审查工具这几年,我越来越觉得一个道理:代码审查工具永远替代不了人,但它能让人更好地去做人该做的事。机器负责的是“扫一遍、找明显问题、把风险点标记出来”,人负责的是“看设计、看权衡、看这行代码值不值得这么写、看它有没有把业务逻辑搞复杂”。

我在这个项目里最大的收获,不是那个能跑出结果的命令行工具,而是这三条经验:

第一,自动化工具的首要目标是减少噪音。它输出 100 条问题,如果 80 条是无关紧要的,人就会忽略它,久而久之整个工具就废了。谨慎设置规则、统计命中率、允许团队关闭规则,都是为了让输出更“安静”。

第二,审查工具的规则和团队文化是一起生长的。第一周你只能写“不要有 debug 输出”这种低维规则,等团队用顺手了,你才能慢慢加上“API 调用要有超时时间”“缓存操作要有异常处理”这些真正有价值的中维规则。这个过程急不得。

第三,做工具比做功能更容易获得长期复利。一个写一次能审 100 个 MR 的工具,比一个写了只能跑一个小时的脚本,价值密度高太多了。花两周做工具,连续半年收益,这笔账怎么算都划算。

如果你也想动手写一个类似的辅助工具,先从“每天重复得最烦的那个操作”开始。把这个操作抽象成规则,写成代码,跑在 CI 里,然后看着开发者在 MR 评论里说“这工具还挺有用”——这种正反馈,是写内部工具的人最上瘾的感觉。

RELATED READING

延伸阅读

更多一线实战笔记与深度复盘,助您持续精进