我想分享最近几天的结果，其中包括对一些大型Go项目的git历史的分析，目的是找到可以更正错误的提交，然后对其进行形式化以进行检测，以防它们出现在新代码中。

在本文的第二部分中，我们将考虑go-critic中的一些新诊断方法，这些方法可以查找包含错误可能性很高的代码。

搜寻过去的想法

要找到进行新代码检查的想法，您可以查看其他编程语言的静态分析器，可以手动检查开源代码以尝试在其中找到错误的代码模板，或者可以查看历史记录。

假设您正在检查Go-Files项目。 启动lint（静态分析器）后，没有任何问题；对源文本进行审核也未带来很好的结果。 我应该急于将Go-Files放在一边吗？ 不完全是

在存在期间， Go-Files具有许多易于检测的缺陷，但是现在它们已得到修复。 我们需要做的是开始分析git存储库的历史记录。

虫子在附近。

收集我们感兴趣的承诺

问题：

• 可能有很多提交
• 有时在同一修订版中，事情彼此无关
• 有时，对提交的评论与实际更改不符。
• 有些人低估了良好提交消息的使用（我们不要指责）

首先，我们希望减少在没有自动化的情况下必须处理的信息量。 可以通过增加样本大小（将更多的Go数据包下载到对照组）来解决假阴性反应的问题。

您可以用不同的方式对其进行分析，我只是为一组模板设置了git log --grep ，根据其提交消息的内容高估或低估了提交的分数。 如果提交过大，您可以立即将其丢弃，因为它很可能弄清楚发生了什么，将是不平凡的。

我还想补充一点，在更正或发现错误旁边，对补丁的描述通常不是很乐观，有时也不是很贴切：

 - "We check length later, but we assumed it was always 1 bytes long. Not always the case. I'm a little depressed that this bug was there" - "Really fixed the bug now" - "WTF, the changes that actually fixed the bug for the accesspattern wasn't actually committed" - "how do i pronounce this damn project" ( traefic) - "damnit travis" - "one f*cking dot" (./.. => ./...  .travis.yml) 

最无用的“错误修复”是编辑语法错误或其他阻止项目甚至无法通过go build编译的问题。 并非所有项目都使用CI或pre-commit挂钩 ，因此，有时这样的无效修订最终会在master分支中结束。

历史错误

让我们看一下在git日志的掩盖下发现的一些最有趣的错误。

返回nil而不是实际结果

gin-gonic / gin：修复错误，绑定bool失败时返回err ：

  if err == nil { field.SetBool(boolVal) } - return nil + return err } 

go-pg / pg：AppendParam错误修复 ：

  return method.AppendValue(b, m.strct.Addr(), 1), true } - return nil, false + return b, false } 

快速路径缺乏回报

gonum / gonum：修复了dswap快速路径中的错误 ：

  for i := 0; i < n; i++ { x[i], y[i] = y[i], x[i] } + return } 

在没有wg.Add的情况下运行goroutines（1）

btcsuite / btcd：修复RPC服务器关闭中的几个错误 ：

 +s.wg.Add(1) go s.walletListenerDuplicator() 

无效的逻辑条件

gorgonia / gorgonia：还修复了一些错误 ：

 -for i := 0; i > start; i++ { +for i := 0; i < start; i++ { retVal[i] = 0 } 

src-d / go-git：管道/ idxfile：修复在MemoryIndex中搜索错误 ：

 -if low < high { +if low > high { break } 

go-xorm / xorm：修复总和上的错误 ：

 -if !strings.Contains(colName, " ") && strings.Contains(colName, "(") { +if !strings.Contains(colName, " ") && !strings.Contains(colName, "(") { 

btcsuite / btcd：服务器：修复了在filteradd上断开对等连接的错误 ：

 -if sp.filter.IsLoaded() { +if !sp.filter.IsLoaded() { 

btcsuite / btcd：使用最大操作处理来修复反向测试错误 ：

 -if pop.opcode.value < OP_16 { +if pop.opcode.value > OP_16 { s.numOps++ 

通过值更新接收者（this / self）

经典版 在方法内部修改对象的副本，因此调用者将看不到预期的结果。

修复了堆栈/队列/双端队列错误（指针接收器） ：

 -func (s Stack) Push(x interface{}) { - s = append(s, x) +func (s *Stack) Push(x interface{}) { + *s = append(*s, x) } 

更新循环中的对象副本

containous / traefik：将过滤后的任务分配给slice中包含的应用程序 ：

 -for _, app := range filteredApps { - app.Tasks = fun.Filter(func(task *marathon.Task) bool { +for i, app := range filteredApps { + filteredApps[i].Tasks = fun.Filter(func(task *marathon.Task) bool { return p.taskFilter(*task, app) }, app.Tasks).([]*marathon.Task) 

容器/ traefik：添加用于安全包装的单元测试 ：

 -for _, routine := range p.routines { +for i := range p.routines { p.waitGroup.Add(1) - routine.stop = make(chan bool, 1) + p.routines[i].stop = make(chan bool, 1) Go(func() { - routine.goroutine(routine.stop) + p.routines[i].goroutine(p.routines[i].stop) p.waitGroup.Done() }) } 

不使用包装功能的结果。

gorgonia / gorgonia：修复了Grad（）中的一个微小的无关紧要的错误 ：

 if !n.isInput() { - errors.Wrapf(err, ...) - // return + err = errors.Wrapf(err, ...) + return nil, err } 

使用make的工作不正确

有时候，年轻的地鼠会“ make([]T, count) ”，然后在循环内append 。 此处正确的选项是“ make([]T, 0, count) ”。

HouzuoGuo / tiedo：修复集合扫描错误 ：

 -ids := make([]uint64, benchSize) +ids := make([]uint64, 0) 

上面的修复程序纠正了原始错误，但是在以下提交之一中已解决了一个缺陷：

 -ids := make([]uint64, 0) +ids := make([]uint64, 0, benchSize) 

但是某些操作（例如copy或ScanSlice可能需要“正确”的长度。

go-xorm / xorm：SumsInt的错误修复返回空片 ：

 -var res = make([]int64, 0, len(columnNames)) +var res = make([]int64, len(columnNames), len(columnNames)) if session.IsAutoCommit { err = session.DB().QueryRow(sqlStr, args...).ScanSlice(&res) } else { 

很快，批评家会帮助您发现此类错误。

其他错误

枚举结果相当麻烦，我将其余的内容删除了。 这部分是可选的，因此您可以将其留待以后或大胆地继续。

 -for _, el := range *s1 { +for el := range *s1 { if _, ok := (*s2)[el]; !ok { return false } 

 func (s *server) Query(ctx context.Context, - req *graph.Request) (resp *graph.Response, err error) { + req *graph.Request) (*graph.Response, error) { + resp := new(graph.Response) 

 -block, err := btcutil.NewBlockFromBytes(blockBytes) +block, err = btcutil.NewBlockFromBytes(blockBytes) if err != nil { return err } 

 -err = session.Rollback() +err1 := session.Rollback() +if err1 == nil { + return lastId, err +} +err = err1 

 -debugPrint("Listening and serving HTTP on %s", addr) +debugPrint("Listening and serving HTTP on %s\n", addr) 

  styleCache map[int]*Style `-` + lock *sync.RWMutex } 

反对可疑代码

badCond

badCond检查将发现可能不正确的逻辑表达式。

可疑逻辑表达式的示例：

• 结果始终是true还是false
• 该表达式以几乎与错误几乎无法区分的形式编写

此检查还适用于“ x == nil && x == y ”之类的操作。 一种简单的解决方案是将其重写为“ x == nil && y == nil ”。 该代码的可读性不会受到影响，但是在该代码中，lint不会看到任何可疑的东西。

这是badCond可以发现的错误修复的badCond ：

 -if err1 := f(); err != nil && err == nil { +if err1 := f(); err1 != nil && err == nil { err = err1 } 

奖杯：

• Moby / buildkit：快照：修复不可能的情况
• tdewolff /解析：来自util.go的EqualFold条件异常
• HouzuoGuo /绑匪：httpapi：srv_test.go中的可疑情况

弱电

weakCond与weakCond相似，但是警报的置信度略低。

弱条件是不能充分完全覆盖输入的条件。

弱（不足）条件的一个很好的例子是检查切片是否为nil ，然后使用第一个元素。 这里的条件“ s != nil ”还不够。 正确的条件是“ len(s) != 0 ”或其等效值：

 func addRequiredHeadersToRedirectedRequests( req *http.Request, via []*http.Request) error { - if via != nil && via[0] != nil { + if len(via) != 0 && via[0] != nil { 

奖杯：

• etcd-io / etcd：etcdserver / api / v2v3：nil检查后建立索引而不进行len检查
• Moby / Moby：注册表：在无len检查后索引而不进行len检查
• inkyblackness / hacked：ss1 /内容/文本：nil检查后索引而没有len检查

dupArg

对于某些函数，传递与多个参数相同的值（或变量）没有多大意义。 通常，这表示复制/粘贴错误。

这种功能的一个例子是copy 。 表达式copy(xs, xs)没有意义。

 -if !bytes.Equal(i2.Value, i2.Value) { +if !bytes.Equal(i1.Value, i2.Value) { return fmt.Errorf("tries differ at key %x", i1.Key) } 

奖杯：

• 以太坊/去以太坊：浅：以字节为单位修复重复的参数。
• openshift /原点：修复reflect.DeepEqual（x，x）错误

dupCase

您可能知道在Go中不能在switch使用重复的case值。

以下示例无法编译 ：

 switch x := 0; x { case 1: fmt.Println("first") case 1: fmt.Println("second") } 

编译错误：开关中的情况1重复。

但是，如果我们举一个更有趣的例子 ：

 one := 1 switch x := 1; x { case one: fmt.Println("first") case one: fmt.Println("second") } 

变量允许重复的值。 此外， switch true为错误的可能性更大：

 switch x := 0; true { case x == 1: fmt.Println("first") case x == 1: fmt.Println("second") } 

这是修复实际错误的示例：

 switch { case m < n: // Upper triangular matrix. //   . case m == n: // Diagonal matrix. //   . -case m < n: // Lower triangular matrix. +case m > n: // Lower triangular matrix. //   . } 

奖杯：

• mellium / sasl：删除重复的开关盒
• gonum / gonum：修复重复的开关盒
• pact-foundation / pact-go：删除重复的开关盒
• minio / minio：cmd / object-api-error.go中可疑的重复开关案例子句
• henrylee2cn / pholcus：常见/字节：删除重复的开关盒
• go-ble / ble：linux / att：修复重复的开关大小写错误
• 泊坞窗/分配：注册表/存储：修复重复的开关盒

dupBranchBody

条件语句（例如if/else和switch具有要执行的主体。 当满足条件时，控制权将转移到相关的操作单元。 当其他诊断程序验证这些块的条件不同时， dupBranchBody检查可执行块本身是否完全不相同。

除非它是type switch ，否则条件语句中是否存在重复主体至少是可疑的。

 -if s, err := r.ReceivePack(context.Background(), req); err != nil { - return s, err -} else { - return s, err -} +return r.ReceivePack(context.Background(), req) 

有关以下代码的正确性的判断留给读者：

 if field.IsMessage() || p.IsGroup(field) { //   . } else if field.IsString() { if nullable && !proto3 { p.generateNullableField(fieldname, verbose) } else { pP(`if this.`, fieldname, ` != that1.`, fieldname, `{`) } } else { if nullable && !proto3 { p.generateNullableField(fieldname, verbose) } else { pP(`if this.`, fieldname, ` != that1.`, fieldname, `{`) } } } 

“ if field.IsString() ”内部的主体和关联的“ else ”内部是相同的。

奖杯：

• src -d / go-git：可疑的重复分支主体

caseOrder

type switch内的所有类型type switch按顺序排序，直到第一个兼容。 如果标记值的类型为*T ，并且实现了接口I ，则您需要在带有“ case I ” 的标签之前放置一个带有“ case *T ” 的标签，否则将仅执行“ case I ”，因为*T与I兼容（但是反之亦然）。

 case string: res = append(res, NewTargetExpr(v).toExpr().(*expr)) -case Expr: - res = append(res, v.toExpr().(*expr)) case *expr: res = append(res, v) +case Expr: + res = append(res, v.toExpr().(*expr)) 

奖杯：

• cmd / guru：修复describe.go中错误的案例顺序
• 消息/管道：修复类型切换案例顺序
• coyove / goflyway：代理：在类型切换中修复案例顺序
• 石墨/ carbonapi：pkg /解析器：固定类型开关盒顺序

offBy1

单位错误非常普遍，甚至有自己的维基百科页面 。

 if len(optArgs) > 1 { return nil, ErrTooManyOptArgs } -node = optArgs[1] +node = optArgs[0] 

 -last := xs[len(xs)] +last := xs[len(xs) - 1] 

go-critic对于发现此类错误的选择有限，但到目前为止，尚未将任何修复程序发送到公共存储库。 以下是在查看故事时发现的一些修复程序：

• btcsuite / btcd：测试并修复getaddednodeinfo中的错误
• btcsuite / btcd：提高测试覆盖率并修复测试发现的一些错误

最后有点恐怖

go vet对“ x != a || x != b ”等表达式进行了很好的检查。 似乎人们可能并不了解gometalinter ，但是go vet几乎启动了所有人，对吗？

使用gogrep实用程序，我将一小段相似的表达式组合在一起，这些表达式仍在某些项目的主分支中。

 cloud.google.com/go/trace/trace_test.go:943:7: expectTraceOption != ((o2&1) != 0) || expectTraceOption != ((o3&1) != 0) github.com/Shopify/sarama/mocks/sync_producer_test.go:36:5: offset != 1 || offset != msg.Offset github.com/Shopify/sarama/mocks/sync_producer_test.go:44:5: offset != 2 || offset != msg.Offset github.com/docker/libnetwork/api/api_test.go:376:5: id1 != i2e(i2).ID || id1 != i2e(i3).ID github.com/docker/libnetwork/api/api_test.go:408:5: "sh" != epList[0].Network || "sh" != epList[1].Network github.com/docker/libnetwork/api/api_test.go:1196:5: ep0.ID() != ep1.ID() || ep0.ID() != ep2.ID() github.com/docker/libnetwork/api/api_test.go:1467:5: ep0.ID() != ep1.ID() || ep0.ID() != ep2.ID() github.com/docker/libnetwork/ipam/allocator_test.go:1261:5: len(indices) != len(allocated) || len(indices) != num github.com/esimov/caire/grayscale_test.go:27:7: r != g || r != b github.com/gogo/protobuf/test/bug_test.go:99:5: protoSize != mSize || protoSize != lenData github.com/gogo/protobuf/test/combos/both/bug_test.go:99:5: protoSize != mSize || protoSize != lenData github.com/gogo/protobuf/test/combos/marshaler/bug_test.go:99:5: protoSize != mSize || protoSize != lenData github.com/gogo/protobuf/test/combos/unmarshaler/bug_test.go:99:5: protoSize != mSize || protoSize != lenData github.com/gonum/floats/floats.go:65:5: len(dst) != len(s) || len(dst) != len(y) github.com/gonum/lapack/testlapack/dhseqr.go:139:7: wr[i] != h.Data[i*h.Stride+i] || wr[i] != h.Data[(i+1)*h.Stride+i+1] github.com/gonum/stat/stat.go:1053:5: len(x) != len(labels) || len(x) != len(weights) github.com/hashicorp/go-sockaddr/ipv4addr_test.go:659:27: sockaddr.IPPort(p) != test.z16_portInt || sockaddr.IPPort(p) != test.z16_portInt github.com/hashicorp/go-sockaddr/ipv6addr_test.go:430:27: sockaddr.IPPort(p) != test.z16_portInt || sockaddr.IPPort(p) != test.z16_portInt github.com/nats-io/gnatsd/server/monitor_test.go:1863:6: v.ID != c.ID || v.ID != r.ID github.com/nbutton23/zxcvbn-go/adjacency/adjcmartix.go:85:7: char != "" || char != " " github.com/openshift/origin/pkg/oc/cli/admin/migrate/migrator_test.go:85:7: expectedInfos != writes || expectedInfos != saves github.com/openshift/origin/test/integration/project_request_test.go:120:62: added != deleted || added != 1 github.com/openshift/origin/test/integration/project_request_test.go:126:64: added != deleted || added != 4 github.com/openshift/origin/test/integration/project_request_test.go:132:62: added != deleted || added != 1 gonum.org/v1/gonum/floats/floats.go:60:5: len(dst) != len(s) || len(dst) != len(y) gonum.org/v1/gonum/lapack/testlapack/dhseqr.go:139:7: wr[i] != h.Data[i*h.Stride+i] || wr[i] != h.Data[(i+1)*h.Stride+i+1] gonum.org/v1/gonum/stat/stat.go:1146:5: len(x) != len(labels) || len(x) != len(weights) 

从别人的错误中学习

不，即使在最后一部分， 机器学习也一无所获。

但是这里要写的是有关golangci-lint的 ，该语言将go-critic集成在最新版本之一中。 golangci-lint是golangci-lint的类似物，您可以阅读它的优点，例如，在文章“ Go中的静态分析：如何在检查代码时节省时间”。

最近，使用lintpack重写了go-critic 。 可以在Go lintpack：可组合的linter管理器中找到详细信息。

如果您尚未开始主动使用工具来分析程序中的潜在错误（无论是样式错误还是逻辑错误），今天，我建议您再次与同事一起思考一下您喝茶的行为。 你会成功的。

对所有人都好。