過去数日間の結果を共有したいと思います。これには、いくつかの大規模なGoプロジェクトのgit履歴の分析が含まれています。
記事の第2部では、 go-criticのいくつかの新しい診断について検討します。これにより、エラーを含む可能性が高いコードを見つけることができます。
過去のアイデアを検索
新しいコード検査のアイデアを見つけるには、他のプログラミング言語の静的アナライザーを覗き見たり、オープンソースコードを手動で調べてエラーコードテンプレートを見つけたり、履歴を調べたりすることができます。
Go-Files
プロジェクトをチェックしているとします。 リンター（静的アナライザー）を起動した後、問題はありません;ソーステキストの監査も大きな結果をもたらしませんでした。 急いでGo-Files
片付ける必要がありますか？ そうでもない。
Go-Files
が存在する間、いくつかの簡単に検出可能な欠陥がありましたが、現在は既に修正されています。 必要なのは、gitリポジトリの履歴の分析を開始することです。
バグは近くのどこかにあります。
興味のあるコミットメントのコレクション
問題は次のとおりです。
- たくさんのコミットがある可能性があります
- 同じ改訂版では、物事は互いに無関係である場合があります
- コミットに関するコメントが実際の変更と完全に一致しない場合があります。
- 良いコミットメッセージの使用を過小評価している人もいます（指をさしません）
まず、自動化せずに処理する必要がある情報の量を減らしたいと思います。 偽陰性の応答の問題は、サンプルサイズを増やすことで解決できます（Goグループをさらに制御グループにダウンロードします）。
さまざまな方法で分析できます。コミットメッセージの内容に応じてコミットのスコアを過大評価および過小評価して、一連のテンプレートに対してgit log --grep
を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
でコンパイルするgo build
さえできないその他の問題です。 すべてのプロジェクトがCIまたはpre-commit hookを使用するわけではないため、このような壊れたリビジョンがマスターブランチに到達することがあります。
履歴エラー
gitログの裏で見つかった最も興味深いエラーをいくつか見てみましょう。
実際の結果の代わりにnilを返す
gin-gonic / gin：バグを修正し、boolのバインドに失敗するとerrを返します ：
if err == nil { field.SetBool(boolVal) } - return nil + return err }
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なしでゴルーチンを実行する（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：plumbing / idxfile：MemoryIndexの検索バグを修正 ：
-if low < high { +if low > high { break }
-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：フィルターされたタスクをスライスに含まれるアプリに割り当てます ：
-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)
containous / 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 / tiedot：コレクションスキャンのバグを修正 ：
-ids := make([]uint64, benchSize) +ids := make([]uint64, 0)
上記の修正は元のエラーを修正しますが、次のいずれかのコミットで修正された1つの欠陥があります。
-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 {
すぐにgo-criticはこのクラスのエラーを見つけるのに役立ちます。
その他のエラー
列挙はかなり面倒であることが判明し、残りはネタバレの下に持ち出します。 この部分はオプションであるため、後で使用するか大胆に先に進むことができます。
次のフラグメントは、エラーを修正するという事実にもかかわらず、反復可能なキーの名前が反復可能な値に使用されたのと同じ名前になっているという点で興味深いものです。 新しいコードは正しく見えず、次のプログラマを混乱させる可能性があります。
-for _, el := range *s1 { +for el := range *s1 { if _, ok := (*s2)[el]; !ok { return false }
名前付きの結果はほとんど同じ通常の変数なので、同じルールに従って初期化されます。 ポインターの値はnil
なります。このオブジェクトを使用する場合は、このポインターを明示的に初期化する必要があります（たとえば、 new
）。
dgraph-io / dgraph：server / main.goのnilポインター参照バグを修正 ：
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)
エラーを処理する際に最も頻繁に、そしてほとんどのシャドウイングバイト。
btcsuite / btcd：ブロックチェーン/インデクサー：インデクサー再編成のバグを修正 ：
-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
gin-gonic / gin：Run *関数でのdebugPrintの改行の問題を修正 ：
-debugPrint("Listening and serving HTTP on %s", addr) +debugPrint("Listening and serving HTTP on %s\n", addr)
以下の例のミューテックスの実装は、構造のメンバーとして*sync.Mutex
の使用を報告するチェックを書くというアイデアに*sync.Mutex
ました。 Dave Cheneyは、 常にmutex値へのポインタではなく、 mutex値を使用することをお勧めします。
tealeg / xlsx：スプレッドシートのロード時のバグを修正 ：
styleCache map[int]*Style `-` + lock *sync.RWMutex }
疑わしいコードに対する十字軍
badCond
badCond
チェックは、潜在的に不正な論理式を検出します。
疑わしい論理式の例：
- 結果は常に
true
またはfalse
- この表現は、誤りのあるものと事実上区別できない形式で書かれています
このチェックは、「 x == nil && x == y
」のような操作でも機能します。 1つの簡単な解決策は、「 x == nil && y == nil
」に書き換えることです。 コードの可読性は低下しませんが、リンターはこのコードで疑わしいものを見ることはありません。
badCond
で発見できるバグ修正の例を次に示しbadCond
。
-if err1 := f(); err != nil && err == nil { +if err1 := f(); err1 != nil && err == nil { err = err1 }
トロフィー：
- moby / buildkit：スナップショット：不可能な状態を修正
- tdewolff / parse：util.goのEqualFoldの状態がおかしい
- HouzuoGuo / tiedot：httpapi：srv_test.goの疑わしい状態
weakCond
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：lenチェックなしのnilチェック後のインデックス付け
- moby / moby：レジストリ：lenチェックなしのnilチェック後のインデックス付け
- inkyblackness /ハッキング：ss1 / content / text：lenチェックなしのnilチェック後のインデックス付け
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) }
トロフィー：
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の疑わしい重複したswitch case節
- henrylee2cn / pholcus：common / bytes：重複したスイッチケースを削除
- go-ble / ble：linux / att：重複したスイッチケースエラーを修正
- Docker /ディストリビューション：レジストリ/ストレージ：重複したスイッチケースを修正
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
」は同一です。
トロフィー：
caseOrder
type switch
内のすべてのタイプtype switch
、最初の互換性のあるものに順番にソートされます。 タグ値のタイプが*T
で、 I
インターフェースを実装するcase *T
、「 case I
」のラベルの前に 「 case *T
」のラベルを付ける必要があります。そうでないcase I
、 *T
I
*T
互換性があるため、その逆ではありません）。
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 /達人：describe.goの誤った大文字と小文字の順序を修正
- メッセージ/パイプライン：タイプスイッチケースの順序を修正
- coyove / goflyway：プロキシ：タイプスイッチの大文字と小文字の順序を修正
- go-graphite / carbonapi：pkg / parser：タイプスイッチの大文字小文字の順序を修正
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
は、このようなエラーを見つけるための選択肢が限られていますが、これまでのところ、公開リポジトリに修正は送信されていません。 ストーリーの表示中に見つかった修正を次に示します。
最後に少しの恐怖
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に関するもので、最新リリースの1つにgo-criticが統合されています。 golangci-lint
はgolangci-lint
の類似物です。たとえば、 「Goの静的分析：コードをチェックするときの時間を節約する方法」という記事で、その利点について読むことができます 。
go-critic
は、最近lintpackを使用して書き直されました 。 詳細については、 Go lintpack：a composable linter managerを参照してください 。
まだツールを積極的に使用して、プログラムの潜在的なエラーを文体的および論理的に分析し始めていない場合は、今日、同僚とお茶を飲みながらあなたの行動についてもう一度考えることをお勧めします。 あなたは成功します。
すべてに良い。