★ FEATURED ARTICLE 1. 从零搭建代码评审机器人的真实动机代码评审这件事做过团队协作的人都有体会。一个人写完功能提交合并请求然后眼巴巴等着同事来审。同事手头一堆活儿可能拖到第二天才看看完提几条意见改完再等一轮。一个不大的改动来回折腾两三天是常事。更麻烦的是有些问题其实很机械——变量命名不规范、函数太长、缺少边界判断、日志打得不合理——这些完全可以用工具自动查出来却占用了大量人工评审时间。我所在的团队大概十来个人后端前端都有项目迭代节奏快的时候一天能提十几个合并请求。评审压力最大的时候资深工程师半天时间全花在看别人的代码上自己的活儿只能加班干。时间一长大家都有怨气。我就琢磨着能不能做一个自动化的代码评审工具把那些“看一眼就知道有问题”的毛病先过滤掉让人工评审只关注真正需要判断力的部分比如架构设计是否合理、业务逻辑是否有漏洞、接口定义是否清晰。这个想法就是“open-code-review”项目的起点。它的核心定位很明确一个开源的、可自行部署的代码评审辅助系统对接代码托管平台的合并请求自动拉取差异内容跑一遍预设的检查规则把发现的问题以评论形式回写到合并请求里。它不追求替代人工评审而是做人工评审的“第一道筛子”。适合谁来参考这篇内容呢如果你是小团队的技术负责人正被评审效率问题困扰或者你是对自动化工具感兴趣的开发者想了解这类系统怎么从零搭起来又或者你只是想给自己的项目加一个自动检查的环节那接下来的内容应该都能帮到你。我会从整体设计思路讲起然后拆解核心模块的实现方式再分享实际跑起来之后踩过的坑和调整过程。2. 整体架构与核心模块拆解2.1 为什么选择“事件驱动 规则引擎”的组合做自动化评审工具第一个要决定的就是触发方式。最简单的做法是定时轮询每隔几分钟去代码托管平台拉一次最新的合并请求列表发现有新的就处理。但这种方式有两个明显问题一是延迟高提交之后可能要等好几分钟才有反馈二是浪费资源大部分轮询都是空跑。所以我选了事件驱动的方式。代码托管平台基本都支持Webhook当有新的合并请求创建或者更新时平台会主动发一个HTTP请求到我们配置的地址。我们的服务收到请求后解析出项目标识、合并请求编号、源分支和目标分支这些信息然后触发后续的评审流程。这样响应及时资源消耗也小。但光有事件驱动还不够评审逻辑本身需要一套规则引擎来支撑。规则引擎的作用是把“检查什么”和“怎么检查”分开。检查什么由规则定义比如“函数行数不超过80行”“禁止使用某些不推荐的API”“新增代码必须有对应的单元测试文件”。怎么检查由引擎负责它遍历所有启用的规则对差异内容逐条执行收集结果。这种分离带来的好处是扩展性强。后面想加新规则只需要写一个规则描述文件注册到引擎里就行不用改核心流程。团队里不同的人可以负责不同的规则集比如后端组维护Java相关的规则前端组维护JavaScript和CSS的规则互不干扰。2.2 差异内容获取与解析的关键细节评审的基础是拿到代码差异。代码托管平台的API通常提供两种获取差异的方式一种是直接返回统一格式的diff文本另一种是返回结构化的文件变更列表。我一开始用的是diff文本因为看起来简单直接解析就行。但实际用下来发现坑不少。diff文本的格式虽然标准但不同平台在细节上有差异。比如有的平台在文件头会加额外的元信息有的平台对重命名文件的表示方式不一样还有的平台在二进制文件变更时给出的标记不统一。更麻烦的是当差异内容很大时diff文本会非常长解析起来性能很差。后来我改成了用结构化接口。以常见的代码托管平台为例获取合并请求的变更列表时返回的是一个数组每个元素包含文件路径、变更类型新增、修改、删除、重命名、新增行数、删除行数以及具体的差异块。每个差异块里又有旧文件行号、新文件行号、变更内容这些字段。这样解析起来就清晰多了不用去猜文本格式。拿到结构化差异之后还需要做一层过滤。不是所有文件都需要评审。比如自动生成的代码、第三方库文件、编译产物、锁文件这些通常不需要人工看也不应该触发规则检查。我在配置里加了一个忽略列表支持按文件路径模式匹配比如*.min.js、package-lock.json、dist/**这些。这个忽略列表是可以动态调整的不同项目可以有不同的配置。还有一个细节是差异块的上下文行数。代码托管平台默认返回的差异块通常只包含变更行前后各三行上下文。对于简单的规则检查够用但有些规则需要更多上下文才能准确判断。比如检查“函数是否过长”如果只看到函数中间几行根本不知道函数从哪里开始到哪里结束。这种情况就需要在规则执行时按需拉取完整文件内容来做分析。我的做法是规则可以声明自己需要“完整文件”还是“仅差异块”引擎根据声明来决定是否额外拉取文件内容。2.3 规则引擎的设计与规则定义方式规则引擎是整个系统的核心。我设计的时候参考了静态代码分析工具的思路但做了简化因为目标不是做全量代码扫描而是针对变更内容做增量检查。每条规则是一个独立的配置单元包含几个关键字段规则标识、规则描述、适用文件类型、检查逻辑、严重级别。规则标识是唯一的名字比如function-length-check。适用文件类型用扩展名或路径模式表示比如.java、.py、src/**/*.js。检查逻辑是规则的核心我用了两种实现方式一种是基于正则表达式的简单匹配适合检查命名规范、禁止使用的API这类另一种是基于语法树的分析适合检查函数长度、圈复杂度、重复代码这类需要理解代码结构的。严重级别分三档错误、警告、建议。错误级别的问题会阻塞合并请求必须修复才能继续警告级别的问题会显示在评论里但不阻塞建议级别的问题只是提示开发者可以自行决定是否处理。这个分级很重要如果所有问题都阻塞开发者会很快失去耐心觉得工具在找茬。规则的定义我用了YAML格式因为可读性好非开发人员也能看懂和修改。一个典型的规则定义长这样rule_id: function-length-check description: 函数体行数不应超过80行 severity: warning file_patterns: - **/*.java - **/*.py check_type: ast params: max_lines: 80引擎启动时会加载所有规则文件按文件类型分组。处理一个合并请求时先根据变更文件的类型筛选出适用的规则然后对每个文件的每个差异块执行规则检查。检查结果收集到一个列表里最后统一格式化成评论内容。这里有个性能上的考虑。如果变更文件很多规则也很多逐个执行可能会慢。我的优化方式是并行处理把文件列表分成多个批次每个批次在一个独立的线程里执行规则检查最后合并结果。线程数根据CPU核心数动态调整避免过度切换。实测下来一个包含20个文件变更、启用了30条规则的合并请求完整检查耗时在3秒左右完全可以接受。2.4 评论回写与结果呈现的取舍检查出问题之后怎么把结果告诉开发者这个环节的设计直接影响工具的接受度。最直接的方式是在合并请求里发一条评论把所有问题列出来。但这样做有几个问题一是评论太长几十个问题堆在一起看着就头疼二是问题和代码位置对不上开发者还得自己去找是哪一行三是如果每次提交都发一条新评论评论区很快就乱了。我最终采用的方案是“行内评论 汇总评论”的组合。行内评论是针对具体代码行的代码托管平台的API通常支持在差异的指定行上创建评论。这样开发者直接在代码旁边就能看到问题修改起来很方便。汇总评论是每个合并请求只发一条内容包括检查概况总共发现多少问题、各严重级别多少、规则执行统计、以及一个提示“详细问题请查看行内评论”。汇总评论在每次检查后更新而不是新发一条这样评论区始终保持整洁。行内评论的定位需要小心处理。差异块里的行号有旧文件行号和新文件行号之分评论要挂在新增行上所以要用新文件行号。但有些问题可能出现在删除行上比如“删除了必要的空行检查”这种情况没法挂行内评论只能放在汇总评论里说明。另外如果同一个位置有多条规则都报了问题需要合并成一条评论避免同一行出现多条评论显得杂乱。还有一个体验上的细节评论内容要简洁明了。我见过一些工具报错信息写了一大段又是规则说明又是修复建议开发者根本没耐心看。我的做法是每条评论只包含三部分问题描述一句话、严重级别、规则标识。比如“函数体行数超过80行当前92行 [警告] [function-length-check]”。修复建议放在规则文档里评论里只给一个链接。这样评论短小精悍开发者扫一眼就知道什么问题。3. 规则集的设计与持续调优3.1 从“什么都查”到“只查值得查的”项目刚跑起来的时候我恨不得把所有能想到的检查都加上。命名规范、代码格式、注释覆盖率、函数长度、圈复杂度、重复代码、魔法数字、空指针风险、资源未关闭、日志规范……林林总总列了四五十条规则。结果第一次跑真实项目一个合并请求报了三百多个问题开发者直接炸了说这工具没法用。我后来反思问题出在“什么都查”的思路不对。自动化评审工具的价值在于过滤机械性问题而不是制造噪音。如果一个规则报出来的问题开发者大概率会忽略或者觉得无所谓那这个规则就不应该默认开启。比如“注释覆盖率”这条规则很多团队根本不要求每个函数都写注释报了也是白报。调整之后我把规则分成了三组核心组、推荐组、实验组。核心组是默认开启的只包含那些“几乎所有人都认同应该修”的规则比如语法错误、明显的空指针风险、资源泄漏、安全漏洞。推荐组是建议开启的包含代码风格、复杂度相关的规则团队可以根据自己的规范选择。实验组是默认关闭的包含一些还在验证中的规则比如“这个函数可能应该拆分”这种带有主观判断的。这个分组策略效果很好。核心组规则数量控制在15条以内一个典型的合并请求通常只会报出3到5个问题开发者觉得有帮助而不是骚扰。推荐组由团队自行决定我们团队后来开启了函数长度和圈复杂度两条因为大家确实觉得长函数和复杂逻辑容易出问题。3.2 误报处理规则引擎最耗精力的部分误报是规则引擎最大的敌人。一个规则如果误报率高开发者很快就会失去信任连带对整个工具产生抵触。我在实际运行中遇到最多的误报来自几个方面。第一类是语法树解析的边界情况。比如检查“函数长度”时匿名函数、Lambda表达式、内部类的方法这些算不算独立函数一开始我没区分导致一个很长的Lambda被当成独立函数报了问题但实际上它只是某个方法里的一小段逻辑。后来我在规则里加了过滤条件只对具名方法做长度检查匿名函数和Lambda跳过。第二类是框架特有的写法。比如某些框架的依赖注入字段看起来没有被赋值但实际上框架会在运行时注入。如果规则检查“未使用的字段”就会误报。这类问题没有通用解法只能针对具体框架加白名单或者特殊处理。我的做法是在规则配置里加一个exclude_patterns字段支持按注解、按继承关系、按导入包来排除。第三类是差异块上下文不足导致的误判。比如检查“变量未使用”如果变量在差异块之外被使用了但差异块里只看到声明没看到使用就会误报。这种情况需要拉取完整文件做分析但完整文件分析又可能引入新的问题——比如文件太大导致分析超时。我的折中方案是对于这类需要全局信息的规则先做一次快速扫描如果差异块内没有发现使用再拉取完整文件确认。这样大部分情况不需要拉完整文件性能可以接受。误报处理是一个持续的过程。我建了一个误报记录表每次开发者反馈误报就记录下来分析原因然后决定是调整规则参数、加排除条件还是干脆下线这条规则。运行三个月之后核心组规则的误报率降到了5%以下开发者基本不再抱怨了。3.3 规则执行性能的优化实践规则引擎的性能直接影响用户体验。如果检查要等半分钟才出结果开发者早就切到别的页面去了。我一开始的实现比较粗暴拿到差异内容后对每个文件、每条规则依次执行串行处理。一个中等规模的合并请求大概要跑十几秒。虽然不算太慢但还有优化空间。第一个优化是缓存。很多规则需要解析语法树而语法树解析是比较耗时的操作。如果同一个文件被多条规则检查没必要重复解析。我在引擎里加了一个语法树缓存以文件路径和文件内容哈希为键解析结果缓存起来。这样多条规则共享同一棵语法树解析次数从“规则数乘以文件数”降到了“文件数”。第二个优化是规则分组执行。有些规则只需要看差异块有些需要看完整文件。我把规则分成两组先执行只需要差异块的规则这些规则速度快能快速给出初步结果。然后再执行需要完整文件的规则这些规则慢一些但数量少。这样即使完整文件分析还没完成初步结果已经可以展示了。第三个优化是增量检查。如果一个合并请求更新了只检查变更的部分而不是全量重新检查。代码托管平台的Webhook会带上本次更新的差异信息我利用这个信息只对新增的变更块执行规则之前已经检查过且没有变化的块跳过。这个优化在合并请求频繁更新的场景下效果很明显检查时间从十几秒降到了两三秒。还有一个细节是超时控制。有些规则在极端情况下可能跑很久比如检查一个几千行的文件。我给每条规则设了超时时间默认5秒超时就跳过并记录日志。这样避免一条规则卡住整个检查流程。4. 部署运行与踩坑实录4.1 服务部署方式的选择与配置要点这个工具本身是一个HTTP服务需要接收Webhook请求同时要能访问代码托管平台的API。部署方式我试过两种直接跑在物理机或虚拟机上以及用容器编排。物理机部署最简单装好运行时环境把服务启动起来配一个反向代理处理HTTPS然后在代码托管平台配置Webhook地址就行。适合小规模使用维护成本低。但缺点是扩容麻烦如果团队规模变大合并请求量上去了单机可能扛不住。容器化部署更适合有一定规模的团队。我把服务打包成容器镜像用编排工具管理。好处是扩容方便副本数调一下就行配置可以通过环境变量注入不同环境用不同配置日志收集和监控也更容易集成。缺点是初期搭建成本高一些需要维护容器编排环境。我最终选了容器化方案因为团队本身就有容器编排环境复用现有设施比较方便。配置方面有几个关键环境变量代码托管平台的API地址和访问令牌、Webhook签名密钥、规则文件目录、日志级别、并行线程数。访问令牌的权限要控制好只需要合并请求的读取和评论写入权限不要给仓库的完整权限避免安全风险。Webhook签名验证是一个容易忽略的安全点。代码托管平台在发送Webhook请求时通常会带一个签名头用预共享密钥对请求体做哈希。服务端收到请求后用同样的密钥和算法计算哈希和签名头比对不一致就拒绝。这一步能防止伪造请求。我一开始没做这个验证后来安全审计的时候被指出来了才补上。4.2 实际运行中遇到的典型问题与排查过程问题一Webhook请求丢失。有开发者反馈提交了合并请求但工具没反应。我去查日志发现根本没有收到Webhook请求。排查下来发现是反向代理的配置问题——代理层对请求体大小有限制而有些Webhook请求体比较大包含完整的变更信息超过了限制被直接拒绝了。调整代理配置把请求体大小限制调大之后解决了。问题二评论重复发送。有时候同一个合并请求会收到多条汇总评论而不是更新同一条。排查发现是并发问题——Webhook可能因为重试机制被重复发送两个请求同时处理都去创建了新评论。解决方案是加一个幂等键用合并请求编号加提交哈希作为唯一标识处理前先检查是否已经处理过处理中则加锁。问题三规则执行超时导致检查不完整。有一个合并请求改了十几个文件其中一个文件有三千多行语法树解析花了十几秒超过了单条规则5秒的超时限制导致这个文件的所有规则都被跳过。后来我调整了策略对于大文件先做快速的行数判断如果文件太大就只做基于正则的简单检查跳过语法树分析。同时把超时时间改成可配置的不同规则可以设不同的超时。问题四评论内容中的特殊字符导致格式错乱。代码里经常有反引号、星号、下划线这些Markdown特殊字符直接放在评论里会把格式搞乱。比如变量名user_name会被渲染成斜体。解决方案是对评论内容做转义处理把特殊字符用反斜杠转义或者用代码块包裹。我选的是用行内代码块包裹变量名和代码片段这样既清晰又不会格式错乱。问题五访问令牌过期导致评论发送失败。代码托管平台的访问令牌通常有有效期过期后API调用会返回认证失败。我一开始没做令牌刷新令牌过期后工具就静默失败了开发者以为没问题实际上评论根本没发出去。后来加了令牌有效期监控快过期时自动刷新同时如果API返回认证失败会在日志里打错误并发送告警通知。4.3 团队接受度与工作流融合的经验工具做得再好如果团队不用也是白搭。我在推广这个工具的过程中有几个体会比较深。第一不要一上来就强制。我一开始把核心组规则设成阻塞级别结果开发者怨声载道觉得被工具卡脖子。后来改成第一个月只警告不阻塞让大家先熟悉第二个月开始只有错误级别的问题阻塞警告和建议不阻塞第三个月才把部分核心规则升级为阻塞。这个渐进过程给了团队适应时间抵触情绪小了很多。第二要让开发者能方便地反馈。我在汇总评论里加了一行提示“如果认为某条规则误报请回复本条评论并说明原因。”这样开发者有意见可以直接说我定期收集这些反馈来调优规则。比让他们去提工单或者发邮件方便多了。第三规则要透明。每条规则在评论里都带规则标识开发者可以点进去看规则说明了解为什么会有这条规则、检查逻辑是什么。透明带来信任如果规则藏着掖着开发者会觉得是黑箱不信任检查结果。第四和现有工作流融合。我们团队本来就有合并请求模板要求填写变更说明和测试情况。我把自动检查的结果也整合到模板里在合并请求页面直接显示检查状态。这样开发者不用切到别的页面就能看到检查结果体验更顺畅。5. 规则扩展与二次开发指南5.1 自定义规则的编写方法与示例内置规则不可能覆盖所有场景团队需要能自己加规则。我把规则定义做成了插件式的新增规则只需要在规则目录下放一个YAML文件重启服务或者触发配置重载就能生效。写一条自定义规则首先要确定检查类型。目前支持三种正则匹配、语法树查询、外部命令调用。正则匹配最简单适合检查文本模式比如“禁止使用System.out.println”。语法树查询适合需要理解代码结构的检查比如“检查所有公开方法的参数个数”。外部命令调用适合集成已有的检查工具比如把某个静态分析工具的输出解析成规则结果。以“禁止使用System.out.println”为例规则文件这样写rule_id: no-system-out-println description: 禁止在生产代码中使用System.out.println请使用日志框架 severity: warning file_patterns: - **/*.java check_type: regex params: pattern: System\\.out\\.println exclude_patterns: - **/test/**这条规则会对所有Java文件做正则匹配发现System.out.println就报警告但测试目录下的文件排除在外。exclude_patterns这个字段很实用可以排除测试代码、生成代码、示例代码这些不需要严格检查的地方。再举一个语法树查询的例子检查“公开方法参数超过5个”rule_id: max-method-params description: 公开方法参数不应超过5个过多参数建议封装成对象 severity: suggestion file_patterns: - **/*.java check_type: ast params: query: method_declaration[modifierpublic][params.length 5] message: 方法 {method_name} 有 {param_count} 个参数建议重构语法树查询的语法我设计得比较简单支持按节点类型、修饰符、属性条件来筛选。{method_name}和{param_count}是占位符执行时会被替换成实际值。这样评论内容可以动态生成更具体。外部命令调用的规则适合集成已有工具。比如团队已经在用某个代码格式化工具可以写一条规则调用它把格式化差异作为检查结果。配置里指定命令路径、参数模板、输出解析方式就行。这种方式的好处是复用现有工具链不用重复造轮子。5.2 规则优先级与冲突处理当多条规则同时命中同一个位置时需要决定怎么展示。我的处理原则是错误级别优先于警告警告优先于建议同级别下规则标识按字母序排列。这样保证重要的先展示开发者不会漏掉关键问题。但有时候规则之间会有冲突。比如一条规则说“函数不超过80行”另一条规则说“函数必须包含完整的错误处理”一个很长的函数可能同时触发这两条规则但修复方向是矛盾的——拆函数可能让错误处理不完整加错误处理可能让函数更长。这种情况我目前的做法是都展示让开发者自己判断。因为工具没法理解业务上下文强行合并或者抑制反而可能掩盖问题。还有一种情况是规则重复。比如两条规则都在检查命名规范只是检查的方面不同。这种情况我会在规则设计阶段就避免确保每条规则的检查范围不重叠。如果发现重叠就合并成一条规则用多个检查条件来实现。5.3 从单仓库到多仓库的配置管理一开始这个工具只服务一个仓库配置直接写在代码里。后来团队项目多了需要服务多个仓库配置管理就成了问题。不同仓库的规则集可能不一样忽略列表也不一样访问令牌也可能不同。我的解决方案是引入“项目配置”的概念。每个仓库对应一个配置文件包含该仓库的规则启用列表、忽略路径、严重级别覆盖、访问令牌引用等。服务启动时加载所有项目配置Webhook请求进来后根据仓库标识找到对应的配置用该配置来执行检查。配置文件的存储方式我选了对象存储因为可以版本化修改有记录回滚也方便。配置更新后通过一个管理接口触发重载不需要重启服务。这样运维人员改配置不用找开发自己就能操作。多仓库还带来一个问题是资源隔离。如果某个仓库的合并请求特别多可能会占用大量线程影响其他仓库的检查。我加了一个简单的限流机制每个仓库最多同时处理N个合并请求超出的排队等待。N的值可以在项目配置里调整默认是3。这样避免单个仓库把资源吃光。6. 运行数据与效果复盘工具跑了大概半年积累了一些数据可以拿出来说说实际效果。从检查量来看平均每天处理40到50个合并请求每个请求平均涉及8个文件变更。规则引擎每天执行大约1200次规则检查其中核心组规则占70%推荐组占25%实验组占5%。核心组规则平均每个合并请求报出2.3个问题推荐组报出4.1个实验组报出1.8个。从问题类型分布来看排在前几位的分别是日志使用不规范占18%、空指针风险占15%、资源未关闭占12%、函数过长占11%、命名不规范占9%。这几类加起来占了65%说明大部分问题确实集中在少数几个方面规则集的设计方向是对的。从修复率来看错误级别的问题修复率最高达到92%因为会阻塞合并不修没法继续。警告级别的问题修复率在60%左右开发者会根据自己的判断决定是否处理。建议级别的问题修复率只有25%大部分被忽略了。这个数据也验证了严重级别分级的必要性——如果所有问题都设成错误级别开发者会被迫处理大量低价值问题体验很差。从时间节省来看我粗略估算过一个合并请求如果全靠人工评审平均需要15到20分钟。引入自动检查后人工评审时间降到了8到10分钟因为机械性问题已经被过滤掉了评审者只需要关注逻辑和设计。按每天40个合并请求算每天节省的评审时间大概在4到5个小时相当于半个多人天。对于十来个人的团队来说这个收益还是很可观的。当然也有做得不够好的地方。比如跨文件的依赖检查目前还比较弱一个文件改了接口定义另一个文件还在用旧接口这种问题工具查不出来。还有业务逻辑层面的问题比如边界条件处理、并发安全这些需要理解业务上下文自动化工具很难覆盖。所以这个工具始终是辅助不能替代人工评审。后续我打算在几个方向继续完善一是增加跨文件分析能力能追踪接口变更的影响范围二是引入简单的机器学习模型对历史评审评论做学习自动识别哪些问题更可能被开发者接受动态调整严重级别三是支持更多代码托管平台目前只对接了一个平台通用性还不够。这些方向都还在探索阶段有进展再分享。
阅读完成 · 觉得有帮助？
