[インデックス 16994] ファイルの概要

このコミットは、Go言語の標準ライブラリであるfmtパッケージに対するクリーンアップ作業を目的としています。具体的には、go vetツールによって検出された潜在的なエラーや、未使用のコード要素を削除することで、コードの品質と保守性を向上させています。

コミット

commit fba7b04dcb0dbda681fbbb50ff258f0b4a5b6615
Author: Rob Pike <r@golang.org>
Date:   Fri Aug 2 11:38:19 2013 +1000

    fmt: clean up some errors found by vet
    Includes deleting some unused items.
    
    R=golang-dev, adg
    CC=golang-dev
    https://golang.org/cl/12305043

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/fba7b04dcb0dbda681fbbb50ff258f0b4a5b6615

元コミット内容

fmt: clean up some errors found by vet Includes deleting some unused items.

R=golang-dev, adg CC=golang-dev https://golang.org/cl/12305043

変更の背景

このコミットの主な背景は、Go言語の公式ツールであるgo vetの活用です。go vetは、Goのソースコードを静的に解析し、コンパイラでは検出できないが、実行時に問題を引き起こす可能性のある疑わしいコード構造を報告するツールです。

この時期のGo開発では、コードベース全体の品質と堅牢性を高めるために、go vetの導入と適用が進められていました。fmtパッケージはGoのI/Oフォーマットを扱う非常に重要なパッケージであり、その正確性と安定性はGoプログラム全体の信頼性に直結します。したがって、go vetによって検出された問題点を修正し、未使用のコードを削除することで、fmtパッケージのコードベースをよりクリーンで、理解しやすく、将来の変更に対してより堅牢なものにすることが目的でした。

特に、String()メソッドのレシーバの変更は、go vetが検出する一般的な問題の一つである、値レシーバのメソッド内でレシーバのフィールドが変更されるケースに関連している可能性があります。このような場合、メソッドがレシーバのコピーに対して動作するため、元の値は変更されず、意図しない動作につながることがあります。

前提知識の解説

`fmt`パッケージ

fmtパッケージは、Go言語におけるフォーマットI/O（入力/出力）を実装するためのパッケージです。C言語のprintfやscanfに似た機能を提供し、文字列のフォーマット、標準出力への出力、文字列からのスキャンなど、多岐にわたる機能を提供します。fmt.Printf、fmt.Sprintf、fmt.Scanfなどが代表的な関数です。

`go vet`ツール

go vetは、Go言語の標準ツールチェーンに含まれる静的解析ツールです。コンパイラが検出できないような、しかし実行時にバグや予期せぬ動作を引き起こす可能性のあるコードパターンを検出します。以下はその例です。

Printfフォーマット文字列の誤り: フォーマット指定子と引数の型が一致しない場合など。
構造体のタグの誤り: jsonやxmlなどのエンコーディング/デコーディングで使用される構造体タグの構文エラー。
メソッドのレシーバの誤用: 値レシーバのメソッド内で、レシーバのフィールドを変更しようとする場合。これは、メソッドがレシーバのコピーに対して動作するため、元の値は変更されないというGoのセマンティクスに反する可能性があります。
到達不能なコード: 常にreturnやpanicの後に続くコードなど。
ロックの誤用: sync.Mutexなどのロックが正しく使用されていない場合。

go vetは、開発者がコードレビューやテストでは見落としがちな潜在的な問題を早期に発見し、コードの品質と信頼性を向上させるのに役立ちます。CI/CDパイプラインに組み込むことで、コードがコミットされる前にこれらの問題を自動的にチェックすることが一般的です。

Go言語のメソッドレシーバ（値レシーバ vs. ポインタレシーバ）

Go言語では、メソッドを定義する際にレシーバを「値レシーバ」または「ポインタレシーバ」として指定できます。

値レシーバ (func (t T) MethodName(...)): メソッドが呼び出されると、レシーバの型の値のコピーがメソッドに渡されます。メソッド内でレシーバのフィールドを変更しても、元の値には影響しません。これは、メソッドが元の値のコピーに対して動作するためです。
ポインタレシーバ (func (t *T) MethodName(...)): メソッドが呼び出されると、レシーバの型の値へのポインタがメソッドに渡されます。メソッド内でレシーバのフィールドを変更すると、元の値も変更されます。これは、メソッドが元の値が格納されているメモリ位置を直接参照するためです。

どちらのレシーバを使用するかは、メソッドがレシーバの状態を変更する必要があるか、またはレシーバが大きな構造体でありコピーのオーバーヘッドを避けたいか、といった要因によって決定されます。String()メソッドのように、レシーバの値を文字列として表現するだけであれば値レシーバでも問題ありませんが、レシーバの状態を変更する可能性がある場合はポインタレシーバを使用するのが適切です。

技術的詳細

このコミットは、主に以下の2種類の変更を含んでいます。

go vetによって検出された問題の修正: src/pkg/fmt/fmt_test.goにおけるRecur構造体のString()メソッドのレシーバの変更がこれに該当します。
未使用コードの削除: src/pkg/fmt/format.go、src/pkg/fmt/print.go、src/pkg/fmt/scan.go、src/pkg/fmt/scan_test.goにおける未使用の変数や関数の削除がこれに該当します。

`Recur`構造体の`String()`メソッドのレシーバ変更

src/pkg/fmt/fmt_test.goでは、Recur構造体のString()メソッドのレシーバがRecur（値レシーバ）から*Recur（ポインタレシーバ）に変更されました。

変更前:

func (r Recur) String() string {
    if recurCount++; recurCount > 10 {
        *r.failed = true
        return "FAIL"
    }
    // ...
}

変更後:

func (r *Recur) String() string {
    if recurCount++; recurCount > 10 {
        *r.failed = true
        return "FAIL"
    }
    // ...
}

この変更の理由は、String()メソッド内で*r.failed = trueという操作が行われているためです。r.failedは*bool型であり、Recur構造体のフィールドとして定義されています。もしレシーバが値レシーバRecurのままだと、String()メソッドに渡されるrは元のRecur構造体のコピーになります。そのコピーのfailedフィールドが指すbool値は変更されますが、元のRecur構造体のfailedフィールドが指すbool値は変更されません。これは、TestBadVerbRecursionテストの意図（再帰が深くなりすぎた場合にfailedフラグを立てる）と矛盾し、テストが正しく機能しない可能性があります。

ポインタレシーバ*Recurに変更することで、String()メソッドは元のRecur構造体へのポインタを受け取ります。これにより、*r.failed = trueという操作が元のRecur構造体のfailedフィールドが指すbool値を正しく変更できるようになり、テストのロジックが期待通りに動作するようになります。go vetはこのような潜在的な問題を検出する能力を持っています。

また、TestBadVerbRecursion内のRecur構造体の初期化も、ポインタを渡すように変更されています。

変更前:

r := Recur{3, &failed}
// ...
r = Recur{4, &failed}

変更後:

r := &Recur{3, &failed}
// ...
r = &Recur{4, &failed}

これは、Sprintf関数にRecurのポインタを渡す場合と値を渡す場合の両方をテストしているため、String()メソッドのレシーバがポインタになったことに合わせて、テストコードもポインタを生成するように変更されています。

未使用コードの削除

以下の未使用の変数や関数が削除されました。これらはコードベースの肥大化を防ぎ、可読性を向上させます。

src/pkg/fmt/format.go:
- var newline = []byte{'\n'}: この変数はどこからも参照されていなかったため削除されました。
src/pkg/fmt/print.go:
- floatBitsとcomplexBits変数: これらの変数は定義されていましたが、コード内で使用されていなかったため削除されました。
src/pkg/fmt/scan.go:
- typeError関数: この関数は定義されていましたが、どこからも呼び出されていなかったため削除されました。
src/pkg/fmt/scan_test.go:
- stringVal1変数: この変数はテスト内で定義されていましたが、使用されていなかったため削除されました。

これらの削除は、コードのデッドコードを排除し、コンパイルされたバイナリサイズをわずかに削減し、将来のメンテナンスを容易にする効果があります。

コアとなるコードの変更箇所

`src/pkg/fmt/fmt_test.go`

--- a/src/pkg/fmt/fmt_test.go
+++ b/src/pkg/fmt/fmt_test.go
@@ -906,7 +906,7 @@ type Recur struct {
 	failed *bool
 }
 
-func (r Recur) String() string {
+func (r *Recur) String() string {
 	if recurCount++; recurCount > 10 {
 		*r.failed = true
 		return "FAIL"
@@ -919,13 +919,13 @@ func (r Recur) String() string {
 
 func TestBadVerbRecursion(t *testing.T) {
 	failed := false
-	r := Recur{3, &failed}
+	r := &Recur{3, &failed}
 	Sprintf("recur@%p value: %d\n", &r, r.i)
 	if failed {
 		t.Error("fail with pointer")
 	}
 	failed = false
-	r = Recur{4, &failed}
+	r = &Recur{4, &failed}
 	Sprintf("recur@%p, value: %d\n", r, r.i)
 	if failed {
 		t.Error("fail with value")

`src/pkg/fmt/format.go`

--- a/src/pkg/fmt/format.go
+++ b/src/pkg/fmt/format.go
@@ -24,8 +24,6 @@ const (
 var padZeroBytes = make([]byte, nByte)
 var padSpaceBytes = make([]byte, nByte)
 
-var newline = []byte{'\n'}
-
 func init() {
 	for i := 0; i < nByte; i++ {
 		padZeroBytes[i] = '0'

`src/pkg/fmt/print.go`

--- a/src/pkg/fmt/print.go
+++ b/src/pkg/fmt/print.go
@@ -641,8 +641,6 @@ func (p *pp) fmtPointer(value reflect.Value, verb rune, goSyntax bool) {
 
 var (
 	intBits     = reflect.TypeOf(0).Bits()
-	floatBits   = reflect.TypeOf(0.0).Bits()
-	complexBits = reflect.TypeOf(1i).Bits()
 	uintptrBits = reflect.TypeOf(uintptr(0)).Bits()
 )

`src/pkg/fmt/scan.go`

--- a/src/pkg/fmt/scan.go
+++ b/src/pkg/fmt/scan.go
@@ -479,11 +479,6 @@ func (s *ss) token(skipSpace bool, f func(rune) bool) []byte {
 	return s.buf
 }
 
-// typeError indicates that the type of the operand did not match the format
-func (s *ss) typeError(arg interface{}, expected string) {
-	s.errorString("expected argument of type pointer to " + expected + "; found " + reflect.TypeOf(arg).String())
-}
-
 var complexError = errors.New("syntax error scanning complex number")
 var boolError = errors.New("syntax error scanning boolean")

`src/pkg/fmt/scan_test.go`

--- a/src/pkg/fmt/scan_test.go
+++ b/src/pkg/fmt/scan_test.go
@@ -54,7 +54,6 @@ var (
 	float32Val           float32
 	float64Val           float64
 	stringVal            string
-	stringVal1           string
 	bytesVal             []byte
 	runeVal              rune
 	complex64Val         complex64

コアとなるコードの解説

`fmt_test.go`の変更

Recur構造体のString()メソッドのレシーバがRecurから*Recurに変更されました。これにより、メソッド内でr.failedが指すbool値が正しく更新されるようになります。これは、fmt.SprintfがString()メソッドを呼び出す際に、レシーバのコピーではなく元のオブジェクトへのポインタを渡すことで、failedフラグの更新が意図通りに機能するようにするためです。
TestBadVerbRecursion関数内で、Recur構造体のインスタンスを生成する際に、直接値ではなくポインタを生成するように変更されました（例: r := &Recur{3, &failed}）。これは、String()メソッドのレシーバがポインタになったことに対応し、テストの整合性を保つための変更です。

`format.go`、`print.go`、`scan.go`、`scan_test.go`の変更

これらのファイルでは、コードベースから未使用の変数や関数が削除されました。

format.goからnewline変数が削除されました。
print.goからfloatBitsとcomplexBits変数が削除されました。
scan.goからtypeError関数が削除されました。
scan_test.goからstringVal1変数が削除されました。

これらの削除は、コードの冗長性を減らし、fmtパッケージのコードベースをより簡潔で効率的なものにすることを目的としています。go vetのような静的解析ツールは、このようなデッドコードを特定するのに非常に有効です。

参考にした情報源リンク

Go言語の公式ドキュメント
go vetに関する一般的な情報源
Go言語のメソッドレシーバに関する解説記事
GitHubのgolang/goリポジトリのコミット履歴
https://golang.org/cl/12305043 (GoのコードレビューシステムGerritの変更リスト)

comemo