欲善其事, 必利其器之Code Quality Checks

February 11, 2019 Leo

进入工业界写代码一转眼就三年多了，在Google写了一年半的Python，C++和borg, 都是本地用Sublime或者用内部的基于web的编辑器cider写，来Squarespace后转到了product engineering，开始大量地写JS和Java，编辑器换成了VSCode和Intellij IDEA。前后差不多都是一年半多的工作时间，但仔细一比发现后一段工作经历无论代码数量还是质量上都远远超过前一段。原因挺简单的：项目决定一切。在Squarespace做的几个项目都是我们Commerce部门里最核心和迫切的几个项目，impact够大，活也够多，能写的代码很多，能学到的东西也就很多了。

另一方面，代码写得多并不代表写得好。经常能在代码库git history里看到有人一日几十个commit，每个commit的描述都是“fix”, “typo”, “test”, “asdf”之类的不成体统的话，点进去一看代码风格和质量也是拆强人意。这种风格的代码放在Google别说merge，申请review都不行，因为好多pre-merge checks都会挂掉。可是在startup，速度即一切，launch是王道，公司还处于苦苦摸索product market fit的阶段，谁会愿意花一个小时争论该用var好还是const好。FB的motto “Move fast and break things” 可谓一针见血。可是随着公司的成长，工程师团队也迅速地扩大，如果大家在Code Review过程中对代码质量的把控不严的话，长期以往整个代码库就变得良莠不齐，各种tech debt渐渐累积，重构的难度也会日益加大。

其实中间有不少现成的工具可以用来帮助整个工程团队维护整体的代码质量，同时大大减轻code reviewer的工作负担。以Java为例，在业界比较常用的代码质量校验工具包括：

checkstyle用来维护代码风格一致性和可读性，常用的代码风格规范包括Google Java Style, 定义了诸如变量命名规范、格式化。千里之行，始于足下，一致的代码分格对于多人的工程项目是必不可少的，不然长期以往写的人和读的人都很痛苦，总是处在接受别人的风格和坚持自己的风格中挣扎。万一checkstyle报错了呢，你可以手动修正风格错误，也可以用像google-java-format这样的工具自动按照Google Java Style格式化修正。

findbugs是基于Java字节码层面的静态分析工具，是一个从学术界诞生的开源工具。正如其名，它的作用就是帮助工程师发现他们代码中的常见错误和可能的bug。它支持的bug类别主要有：

Bad practice: 不好的编程习惯，比如类实现了equals方法却没有实现hashcode方法
Correctness：正确性，最经典就是通过@Nullable 和 @NoneNull提供更准确的Null Pointer分析
Internationalization：国际化，主要是针对String的转换和格式化时一些默认编码和Locale的提醒
Malicious code vulnerability：恶意攻击脆弱性，经典的例子就是public类成员是可变的(mutable 或者not final
Multithreaded correctness：多线程正确性，比如在父类constructor里就调用Thread.start()会导致在子类初始化完成前线程就开始了
Performance：性能问题，比如内部类没有使用static关键词会导致类实例体积变大
Security：安全性问题，比如在源代码中裸用数据库密码
Dodgy code：孤僻代码，比如根本没被调用的类方法或者没被使用的类成员

和checkstyle比，findbugs是真刀真枪地检查代码里的问题。即使是写了Java十年的资深工程师有时候也会“栽”在它的坑里，可想而知如果用在一个相对年轻的团队中使用它的作用会有多大。很多bug就可以在被其他人review前就被解决了，code review的效率自然也得到了相应的提高。

error-prone和findbugs类似，不过是基于源代码的编译时bug分析工具，由Google的Java Build System组出品。和findbugs比，它的优势有：

代码质量高，且有专门的Google组维护，能看到他们在加入更多的bug-pattern。findbugs的社区支持度就相对较弱了。
提供bug fix的建议：因为是基于源代码的分析工具，error-prone有能力基于有bug的源代码提供bug fix的建议。这个功能的开发者体验远胜于findbugs，因为一般findbugs发现一个bug，你还得查他的bug类别文档来理解这个bug pattern然后才能去fix。
基于javac运行，提供了一个error-prone-javac版本来增强标准的Java编译器，这样大部分项目无需很多额外配置就能立刻使用。如果用findbugs还得额外设置编译之外的新任务来跑。
完美支持基于Guava的代码分析，毕竟都是狗家的开源项目。

举一个官方的例子：

import java.util.Set;
import java.util.HashSet;

public class ShortSet {
  public static void main (String[] args) {
    Set<Short> s = new HashSet<>();
    for (short i = 0; i < 100; i++) {
      s.add(i);
      s.remove(i - 1);
    }
    System.out.println(s.size());
  }
}

---------------------------------------------------------------------------------------------------------
$ bazel build :hello
ERROR: example/myproject/BUILD:29:1: Java compilation in rule '//example/myproject:hello'
ShortSet.java:6: error: [CollectionIncompatibleType] Argument 'i - 1' should not be passed to this method;
its type int is not compatible with its collection's type argument Short
      s.remove(i - 1);
              ^
    (see http://errorprone.info/bugpattern/CollectionIncompatibleType)
1 error

error-prone发现了循环里面的类型不匹配和越界问题。

PMD也是基于源代码的分析工具，而且支持多种编程语言：Java, C, C++, JavaScript, Python, PHP等。但可想而知，通用化和特例化就像天平的两端，你只能取其一。相比较findbugs和error-prone，它在Java领域的bug检测能力明显就技不如人。但如果你的代码库是个多种编程语言构成的monorepo，PMD不妨一试。

jacoco和其他提到的工具不同，它是用来生成Java代码测试覆盖率报告(test coverage report)的。我之所以把它并列其中是因为代码测试覆盖率也是代码质量的一个很重要的指标。它支持的覆盖率指标包括: 1) 函数覆盖率 2）行覆盖率 3) 分支覆盖率 4）代码库整体覆盖率。支持JUnit和TestNG两大测试框架，和各种IDE和持续集成系统都良好兼容。如果你用SonarQube的服务的话，能在code review的时候看到每行源代码的测试覆盖程度，非常实用。

如果你用Gradle等构建系统，你还能用jacoco定义最低的测试覆盖率，比如0.7意为着如果测试覆盖率低于70%这个build就会失败。这个最低测试覆盖率听起来有点教条主义了，不过对于有些非常核心的模块可能是必不可少的。

说了这么多，希望能为感兴趣的读者起到初步的启迪作用。有这些这些代码质量检测工具还只是第一步，更重要的是如果塑造一个关心代码质量、追求代码质量的工程师团队文化。只有写代码的人用心了，加上这些工具的辅助，我们的代码才能达到可读、可用、可测试、可拓展、可维护的理想境界。

2019.1.16