Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

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

このコミットは、Go言語の静的解析ツールである govet の内部構造を大幅にリファクタリングするものです。具体的には、これまで govet.go という単一のファイルに集約されていた様々な検査（vetting suite）のロジックを、機能ごとに独立したファイル（method.go, print.go, structtag.go）に分割しています。これにより、コードのモジュール性、可読性、保守性が向上し、将来的な機能追加や変更が容易になることを目的としています。

コミット

  • コミットハッシュ: b618f32687ca1f71c5cc137cee735a2811f74a57
  • 作者: Rob Pike r@golang.org
  • 日付: 2011年12月15日 木曜日 13:44:35 -0800

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

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

元コミット内容

govet: divide the program into one file per vetting suite
Just a rearrangement except for a couple of new functions
and names so govet.go can have all the generic walk routines.

R=rsc
CC=golang-dev
https://golang.org/cl/5489058

変更の背景

govet はGo言語のコードベースにおける一般的なエラーや疑わしい構造を検出するための静的解析ツールです。初期の govet は、その全ての検査ロジックが src/cmd/govet/govet.go という単一のファイルに詰め込まれていました。時間の経過とともに、govet がチェックする項目が増えるにつれて、この単一ファイル構造はコードの肥大化と複雑化を招き、新しい検査の追加や既存の検査の修正が困難になっていました。

このコミットの背景には、govet のコードベースをより整理し、各検査ロジックを独立させることで、以下のメリットを享受しようという意図があります。

  1. モジュール性の向上: 各検査が独立したファイルに分離されることで、それぞれの検査の責任範囲が明確になり、コードの理解が容易になります。
  2. 保守性の向上: 特定の検査にバグが見つかった場合や、そのロジックを変更する必要がある場合に、影響範囲がそのファイル内に限定され、他の検査に意図しない影響を与えるリスクが低減されます。
  3. 拡張性の向上: 新しい種類の検査を追加する際に、既存の govet.go ファイルをさらに複雑にすることなく、新しいファイルとして独立して追加できるようになります。
  4. コードの再利用性: 汎用的なASTウォーク処理を govet.go に集約することで、各検査ファイルは検査ロジックに専念でき、共通処理の重複を避けることができます。

前提知識の解説

このコミットを理解するためには、以下のGo言語の概念とツールに関する知識が役立ちます。

  1. govet: govet はGo言語の標準ツールチェーンに含まれる静的解析ツールです。コンパイルは通るものの、実行時に問題を引き起こす可能性のあるコードパターン（例: Printf フォーマット文字列と引数の不一致、構造体タグの誤り、特定のインターフェースメソッドのシグネチャ不一致など）を検出します。これは、Goのコンパイラが検出できないような、よりセマンティックなエラーを見つけることを目的としています。

  2. GoのAST (Abstract Syntax Tree): Goコンパイラや静的解析ツールは、Goのソースコードを直接解析するのではなく、まずそのコードの抽象構文木（AST）を構築します。ASTは、ソースコードの構造を木構造で表現したもので、プログラムの各要素（関数、変数、式、文など）がノードとして表現されます。go/ast パッケージは、このASTを操作するための型と関数を提供します。

  3. go/parser パッケージ: Goのソースコードを解析し、ASTを生成するためのパッケージです。parser.ParseFile 関数などを使用して、ファイルの内容からASTを構築します。

  4. go/token パッケージ: ソースコード内のトークン（キーワード、識別子、演算子など）や、それらの位置情報（ファイル名、行番号、列番号）を扱うためのパッケージです。ASTノードは、このパッケージの token.Pos 型を使用して、ソースコード内の元の位置を参照します。

  5. go/printer パッケージ: ASTをGoのソースコード形式に整形して出力するためのパッケージです。デバッグやコード生成の際に利用されます。

  6. ast.Walk 関数と ast.Visitor インターフェース: go/ast パッケージの ast.Walk 関数は、ASTのノードを再帰的に走査するためのユーティリティです。ast.Visitor インターフェースを実装したオブジェクトを ast.Walk に渡すことで、ASTの各ノードに到達した際にカスタムロジックを実行できます。Visit メソッドは、ノードを訪れるたびに呼び出され、そのノードの型に基づいて異なる処理を行うことができます。

  7. 静的解析 (Static Analysis): プログラムを実行せずに、ソースコードを分析して潜在的なバグ、脆弱性、またはコードスタイルの違反を検出するプロセスです。govet は静的解析ツールの一例です。

  8. Vetting Suite: govet における「vetting suite」とは、特定の種類の問題を検出するための検査ロジックの集合体を指します。例えば、printf フォーマット文字列の検査、構造体タグの検査、特定のメソッドシグネチャの検査などがそれぞれ独立した「vetting suite」と見なされます。

技術的詳細

このコミットの主要な技術的変更は、govet の検査ロジックのモジュール化です。

  1. ファイルの分割:

    • src/cmd/govet/govet.go: 汎用的なAST走査ロジックと、ファイル全体の処理を管理する中心的な役割を担うようになりました。
    • src/cmd/govet/method.go (新規): Goの標準ライブラリで定義されている特定のインターフェース（例: io.Reader, fmt.Formatter など）のメソッドが、正しいシグネチャで実装されているかをチェックするロジックが移動されました。これは、動的なインターフェースチェックではコンパイル時に検出できないシグネチャの不一致を検出するために重要です。
    • src/cmd/govet/print.go (新規): fmt.Printf, fmt.Println などのフォーマット付き/なし出力関数呼び出しにおける引数の数や型、フォーマット文字列の整合性をチェックするロジックが移動されました。これは、Printf のフォーマット文字列と引数が一致しない場合に発生する実行時パニックを防ぐために役立ちます。
    • src/cmd/govet/structtag.go (新規): 構造体のフィールドに付与されるタグ（例: json:"name", xml:"id" など）の構文が正しいか、reflect.StructTag.Get で解析可能かをチェックするロジックが移動されました。不正なタグは、encoding/json や encoding/xml などのパッケージで予期せぬ動作を引き起こす可能性があります。

  2. File 構造体の Visit メソッドの変更: govet.go 内の File 構造体は、ASTを走査するための ast.Visitor インターフェースを実装しています。以前は、Visit メソッド内で全ての検査ロジックが直接呼び出されていました。この変更により、Visit メソッドは各ノードタイプに対応する汎用的な walk メソッド（例: walkCallExpr, walkFieldTag, walkMethodDecl, walkInterfaceType）を呼び出すようになりました。これらの walk メソッドは、さらに具体的な検査ロジック（例: checkFmtPrintfCall, checkCanonicalFieldTag, checkCanonicalMethod）を、新しく分割されたファイル内の関数に委譲します。

  3. Makefile の更新: 新しいGoソースファイルが追加されたため、src/cmd/govet/Makefile が更新され、これらのファイルが govet バイナリのビルド対象に含まれるようになりました。

  4. 依存関係の整理: govet.go から、各検査ロジックに特化した import 文（例: reflect, unicode/utf8, go/printer）が削除され、それぞれの新しいファイルに移動されました。これにより、govet.go の依存関係が最小限に抑えられ、よりクリーンな状態になりました。

このリファクタリングは、govet の内部アーキテクチャを改善し、各検査の独立性を高めることで、将来的な開発とメンテナンスを容易にするための重要なステップです。

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

src/cmd/govet/Makefile

--- a/src/cmd/govet/Makefile
+++ b/src/cmd/govet/Makefile
@@ -7,6 +7,9 @@ include ../../Make.inc
 TARG=govet
 GOFILES=\
 	govet.go\
+	method.go\
+	print.go\
+	structtag.go\
 
 include ../../Make.cmd
  • method.go, print.go, structtag.go が GOFILES に追加され、ビルド対象に含まれるようになりました。

src/cmd/govet/govet.go

--- a/src/cmd/govet/govet.go
+++ b/src/cmd/govet/govet.go
@@ -12,19 +12,15 @@ import (
 	"fmt"
 	"go/ast"
 	"go/parser"
-	"go/printer"
 	"go/token"
 	"io"
 	"os"
 	"path/filepath"
-	"reflect"
 	"strconv"
 	"strings"
-	"unicode/utf8"
 )
 
 var verbose = flag.Bool("v", false, "verbose")
-var printfuncs = flag.String("printfuncs", "", "comma-separated list of print function names to check")
 var exitCode = 0
 
 // setExit sets the value for os.Exit when it is called, later.  It
@@ -102,7 +98,7 @@ func doFile(name string, reader io.Reader) {
 		return
 	}
 	file := &File{fset: fs, file: parsedFile}
-	file.checkFile(name, parsedFile)
+	file.walkFile(name, parsedFile)
 }
 
 func visit(path string, f os.FileInfo, err error) error {
@@ -168,8 +164,8 @@ func (f *File) Warnf(pos token.Pos, format string, args ...interface{}) {
 	fmt.Fprintf(os.Stderr, loc+format+"\\n", args...)
 }
 
-// checkFile checks all the top-level declarations in a file.
-func (f *File) checkFile(name string, file *ast.File) {
+// walkFile walks the file's tree.
+func (f *File) walkFile(name string, file *ast.File) {
 	Println("Checking file", name)
 	ast.Walk(f, file)
 }
@@ -178,396 +174,59 @@ func (f *File) Visit(node ast.Node) ast.Visitor {
 func (f *File) Visit(node ast.Node) ast.Visitor {
 	switch n := node.(type) {
 	case *ast.CallExpr:
-		f.checkCallExpr(n)
+		f.walkCallExpr(n)
 	case *ast.Field:
-		f.checkFieldTag(n)
+		f.walkFieldTag(n)
 	case *ast.FuncDecl:
-		f.checkMethodDecl(n)
+		f.walkMethodDecl(n)
 	case *ast.InterfaceType:
-		f.checkInterfaceType(n)
+		f.walkInterfaceType(n)
 	}
 	return f
 }
 
-// checkMethodDecl checks for canonical method signatures
-// in method declarations.
-func (f *File) checkMethodDecl(d *ast.FuncDecl) {
-	if d.Recv == nil {
-		// not a method
-		return
-	}
-
-	f.checkMethod(d.Name, d.Type)
-}
-
-// checkInterfaceType checks for canonical method signatures
-// in interface definitions.
-func (f *File) checkInterfaceType(t *ast.InterfaceType) {
-	for _, field := range t.Methods.List {
-		for _, id := range field.Names {
-			f.checkMethod(id, field.Type.(*ast.FuncType))
-		}
-	}
-}
-
-type MethodSig struct {
-	args    []string
-	results []string
-}
-
-// canonicalMethods lists the input and output types for Go methods
-// that are checked using dynamic interface checks.  Because the
-// checks are dynamic, such methods would not cause a compile error
-// if they have the wrong signature: instead the dynamic check would
-// fail, sometimes mysteriously.  If a method is found with a name listed
-// here but not the input/output types listed here, govet complains.
-//
-// A few of the canonical methods have very common names.
-// For example, a type might implement a Scan method that
-// has nothing to do with fmt.Scanner, but we still want to check
-// the methods that are intended to implement fmt.Scanner.
-// To do that, the arguments that have a + prefix are treated as
-// signals that the canonical meaning is intended: if a Scan
-// method doesn't have a fmt.ScanState as its first argument,
-// we let it go.  But if it does have a fmt.ScanState, then the
-// rest has to match.
-var canonicalMethods = map[string]MethodSig{
-	// "Flush": {{}, {"error"}}, // http.Flusher and jpeg.writer conflict
-	"Format":        {[]string{"=fmt.State", "rune"}, []string{}},            // fmt.Formatter
-	"GobDecode":     {[]string{"[]byte"}, []string{"error"}},                 // gob.GobDecoder
-	"GobEncode":     {[]string{}, []string{"[]byte", "error"}},               // gob.GobEncoder
-	"MarshalJSON":   {[]string{}, []string{"[]byte", "error"}},               // json.Marshaler
-	"MarshalXML":    {[]string{}, []string{"[]byte", "error"}},               // xml.Marshaler
-	"Peek":          {[]string{"=int"}, []string{"[]byte", "error"}},         // image.reader (matching bufio.Reader)
-	"ReadByte":      {[]string{}, []string{"byte", "error"}},                 // io.ByteReader
-	"ReadFrom":      {[]string{"=io.Reader"}, []string{"int64", "error"}},    // io.ReaderFrom
-	"ReadRune":      {[]string{}, []string{"rune", "int", "error"}},          // io.RuneReader
-	"Scan":          {[]string{"=fmt.ScanState", "rune"}, []string{"error"}}, // fmt.Scanner
-	"Seek":          {[]string{"=int64", "int"}, []string{"int64", "error"}}, // io.Seeker
-	"UnmarshalJSON": {[]string{"[]byte"}, []string{"error"}},                 // json.Unmarshaler
-	"UnreadByte":    {[]string{}, []string{"error"}},
-	"UnreadRune":    {[]string{}, []string{"error"}},
-	"WriteByte":     {[]string{"byte"}, []string{"error"}},                // jpeg.writer (matching bufio.Writer)
-	"WriteTo":       {[]string{"=io.Writer"}, []string{"int64", "error"}}, // io.WriterTo
+// walkCall walks a call expression.
+func (f *File) walkCall(call *ast.CallExpr, name string) {
+	f.checkFmtPrintfCall(call, name)
 }
 
-func (f *File) checkMethod(id *ast.Ident, t *ast.FuncType) {
-	// Expected input/output.
-	expect, ok := canonicalMethods[id.Name]
-	if !ok {
-		return
-	}
-
-	// Actual input/output
-	args := typeFlatten(t.Params.List)
-	var results []ast.Expr
-	if t.Results != nil {
-		results = typeFlatten(t.Results.List)
-	}
-
-	// Do the =s (if any) all match?
-	if !f.matchParams(expect.args, args, "=") || !f.matchParams(expect.results, results, "=") {
-		return
-	}
-
-	// Everything must match.
-	if !f.matchParams(expect.args, args, "") || !f.matchParams(expect.results, results, "") {
-		expectFmt := id.Name + "(" + argjoin(expect.args) + ")"
-		if len(expect.results) == 1 {
-			expectFmt += " " + argjoin(expect.results)
-		} else if len(expect.results) > 1 {
-			expectFmt += " (" + argjoin(expect.results) + ")"
-		}
-
-		f.b.Reset()
-		if err := printer.Fprint(&f.b, f.fset, t); err != nil {
-			fmt.Fprintf(&f.b, "<%s>", err)
-		}
-		actual := f.b.String()
-		if strings.HasPrefix(actual, "func(") {
-			actual = actual[4:]
-		}
-		actual = id.Name + actual
-
-		f.Warnf(id.Pos(), "method %s should have signature %s", actual, expectFmt)
-	}
+// walkFieldTag walks a struct field tag.
+func (f *File) walkFieldTag(field *ast.Field) {
+	if field.Tag == nil {
+		return
+	}
+	f.checkCanonicalFieldTag(field)
 }
 
-func argjoin(x []string) string {
-	y := make([]string, len(x))
-	for i, s := range x {
-		if s[0] == '=' {
-			s = s[1:]
-		}
-		y[i] = s
-	}
-	return strings.Join(y, ", ")
+// walkMethodDecl walks the method's signature.
+func (f *File) walkMethod(id *ast.Ident, t *ast.FuncType) {
+	f.checkCanonicalMethod(id, t)
 }
 
-// Turn parameter list into slice of types
-// (in the ast, types are Exprs).
-// Have to handle f(int, bool) and f(x, y, z int)
-// so not a simple 1-to-1 conversion.
-func typeFlatten(l []*ast.Field) []ast.Expr {
-	var t []ast.Expr
-	for _, f := range l {
-		if len(f.Names) == 0 {
-			t = append(t, f.Type)
-			continue
-		}
-		for _ = range f.Names {
-			t = append(t, f.Type)
-		}
+// walkMethodDecl walks the method signature in the declaration.
+func (f *File) walkMethodDecl(d *ast.FuncDecl) {
+	if d.Recv == nil {
+		// not a method
+		return
 	}
-	return t
+	f.walkMethod(d.Name, d.Type)
 }
 
-// Does each type in expect with the given prefix match the corresponding type in actual?
-func (f *File) matchParams(expect []string, actual []ast.Expr, prefix string) bool {
-	for i, x := range expect {
-		if !strings.HasPrefix(x, prefix) {
-			continue
-		}
-		if i >= len(actual) {
-			return false
-		}
-		if !f.matchParamType(x, actual[i]) {
-			return false
+// walkInterfaceType walks the method signatures of an interface.
+func (f *File) walkInterfaceType(t *ast.InterfaceType) {
+	for _, field := range t.Methods.List {
+		for _, id := range field.Names {
+			f.walkMethod(id, field.Type.(*ast.FuncType))
 		}
 	}
-	if prefix == "" && len(actual) > len(expect) {
-		return false
-	}
-	return true
 }
 
-// Does this one type match?
-func (f *File) matchParamType(expect string, actual ast.Expr) bool {
-	if strings.HasPrefix(expect, "=") {
-		expect = expect[1:]
-	}
-	// Strip package name if we're in that package.
-	if n := len(f.file.Name.Name); len(expect) > n && expect[:n] == f.file.Name.Name && expect[n] == '.' {
-		expect = expect[n+1:]
-	}
-
-	// Overkill but easy.
-	f.b.Reset()
-	printer.Fprint(&f.b, f.fset, actual)
-	return f.b.String() == expect
-}
-
-// checkField checks a struct field tag.
-func (f *File) checkFieldTag(field *ast.Field) {
-	if field.Tag == nil {
-		return
-	}
-
-	tag, err := strconv.Unquote(field.Tag.Value)
-	if err != nil {
-		f.Warnf(field.Pos(), "unable to read struct tag %s", field.Tag.Value)
-		return
-	}
-
-	// Check tag for validity by appending
-	// new key:value to end and checking that
-	// the tag parsing code can find it.
-	if reflect.StructTag(tag+` _gofix:"_magic"`).Get("_gofix") != "_magic" {
-		f.Warnf(field.Pos(), "struct field tag %s not compatible with reflect.StructTag.Get", field.Tag.Value)
-		return
-	}
-}
-
-// checkCallExpr checks a call expression.
-func (f *File) checkCallExpr(call *ast.CallExpr) {
+// walkCallExpr walks a call expression.
+func (f *File) walkCallExpr(call *ast.CallExpr) {
 	switch x := call.Fun.(type) {
 	case *ast.Ident:
-		f.checkCall(call, x.Name)
+		f.walkCall(call, x.Name)
 	case *ast.SelectorExpr:
-		f.checkCall(call, x.Sel.Name)
-	}
-}
-
-// printfList records the formatted-print functions. The value is the location
-// of the format parameter. Names are lower-cased so the lookup is
-// case insensitive.
-var printfList = map[string]int{
-	"errorf":  0,
-	"fatalf":  0,
-	"fprintf": 1,
-	"panicf":  0,
-	"printf":  0,
-	"sprintf": 0,
-}
-
-// printList records the unformatted-print functions. The value is the location
-// of the first parameter to be printed.  Names are lower-cased so the lookup is
-// case insensitive.
-var printList = map[string]int{
-	"error":  0,
-	"fatal":  0,
-	"fprint": 1, "fprintln": 1,
-	"panic": 0, "panicln": 0,
-	"print": 0, "println": 0,
-	"sprint": 0, "sprintln": 0,
-}
-
-// checkCall triggers the print-specific checks if the call invokes a print function.
-func (f *File) checkCall(call *ast.CallExpr, Name string) {
-	name := strings.ToLower(Name)
-	if skip, ok := printfList[name]; ok {
-		f.checkPrintf(call, Name, skip)
-		return
-	}
-	if skip, ok := printList[name]; ok {
-		f.checkPrint(call, Name, skip)
-		return
-	}
-}
-
-// checkPrintf checks a call to a formatted print routine such as Printf.
-// The skip argument records how many arguments to ignore; that is,
-// call.Args[skip] is (well, should be) the format argument.
-func (f *File) checkPrintf(call *ast.CallExpr, name string, skip int) {
-	if len(call.Args) <= skip {
-		return
-	}
-	// Common case: literal is first argument.
-	arg := call.Args[skip]
-	lit, ok := arg.(*ast.BasicLit)
-	if !ok {
-		// Too hard to check.
-		if *verbose {
-			f.Warn(call.Pos(), "can't check args for call to", name)
-		}
-		return
-	}
-	if lit.Kind == token.STRING {
-		if !strings.Contains(lit.Value, "%") {
-			if len(call.Args) > skip+1 {
-				f.Badf(call.Pos(), "no formatting directive in %s call", name)
-			}
-			return
-		}
-	}
-	// Hard part: check formats against args.
-	// Trivial but useful test: count.
-	numArgs := 0
-	for i, w := 0, 0; i < len(lit.Value); i += w {
-		w = 1
-		if lit.Value[i] == '%' {
-			nbytes, nargs := parsePrintfVerb(lit.Value[i:])
-			w = nbytes
-			numArgs += nargs
-		}
-	}
-	expect := len(call.Args) - (skip + 1)
-	if numArgs != expect {
-		f.Badf(call.Pos(), "wrong number of args in %s call: %d needed but %d args", name, numArgs, expect)
-	}
-}
-
-// parsePrintfVerb returns the number of bytes and number of arguments
-// consumed by the Printf directive that begins s, including its percent sign
-// and verb.
-func parsePrintfVerb(s string) (nbytes, nargs int) {
-	// There's guaranteed a percent sign.
-	nbytes = 1
-	end := len(s)
-	// There may be flags.
-FlagLoop:
-	for nbytes < end {
-		switch s[nbytes] {
-		case '#', '0', '+', '-', ' ':
-			nbytes++
-		default:
-			break FlagLoop
-		}
-	}
-	getNum := func() {
-		if nbytes < end && s[nbytes] == '*' {
-			nbytes++
-			nargs++
-		} else {
-			for nbytes < end && '0' <= s[nbytes] && s[nbytes] <= '9' {
-				nbytes++
-			}
-		}
-	}
-	// There may be a width.
-	getNum()
-	// If there's a period, there may be a precision.
-	if nbytes < end && s[nbytes] == '.' {
-		nbytes++
-		getNum()
-	}
-	// Now a verb.
-	c, w := utf8.DecodeRuneInString(s[nbytes:])
-	nbytes += w
-	if c != '%' {
-		nargs++
-	}
-	return
-}
-
-// checkPrint checks a call to an unformatted print routine such as Println.
-// The skip argument records how many arguments to ignore; that is,
-// call.Args[skip] is the first argument to be printed.
-func (f *File) checkPrint(call *ast.CallExpr, name string, skip int) {
-	isLn := strings.HasSuffix(name, "ln")
-	args := call.Args
-	if len(args) <= skip {
-		if *verbose && !isLn {
-			f.Badf(call.Pos(), "no args in %s call", name)
-		}
-		return
-	}
-	arg := args[skip]
-	if lit, ok := arg.(*ast.BasicLit); ok && lit.Kind == token.STRING {
-		if strings.Contains(lit.Value, "%") {
-			f.Badf(call.Pos(), "possible formatting directive in %s call", name)
-		}
-	}
-	if isLn {
-		// The last item, if a string, should not have a newline.
-		arg = args[len(call.Args)-1]
-		if lit, ok := arg.(*ast.BasicLit); ok && lit.Kind == token.STRING {
-			if strings.HasSuffix(lit.Value, `\\n"`) {
-				f.Badf(call.Pos(), "%s call ends with newline", name)
-			}
-		}
-	}
-}
-
-// This function never executes, but it serves as a simple test for the program.
-// Test with make test.
-func BadFunctionUsedInTests() {
-	fmt.Println()                      // not an error
-	fmt.Println("%s", "hi")            // ERROR "possible formatting directive in Println call"
-	fmt.Printf("%s", "hi", 3)          // ERROR "wrong number of args in Printf call"
-	fmt.Printf("%s%%%d", "hi", 3)      // correct
-	fmt.Printf("%.*d", 3, 3)           // correct
-	fmt.Printf("%.*d", 3, 3, 3)        // ERROR "wrong number of args in Printf call"
-	printf("now is the time", "buddy") // ERROR "no formatting directive"
-	Printf("now is the time", "buddy") // ERROR "no formatting directive"
-	Printf("hi")                       // ok
-	f := new(File)
-	f.Warn(0, "%s", "hello", 3)  // ERROR "possible formatting directive in Warn call"
-	f.Warnf(0, "%s", "hello", 3) // ERROR "wrong number of args in Warnf call"
-}
-
-type BadTypeUsedInTests struct {
-	X int "hello" // ERROR "struct field tag"
-}
-
-func (t *BadTypeUsedInTests) Scan(x fmt.ScanState, c byte) { // ERROR "method Scan[(]x fmt.ScanState, c byte[)] should have signature Scan[(]fmt.ScanState, rune[)] error"
-}
-
-type BadInterfaceUsedInTests interface {
-	ReadByte() byte // ERROR "method ReadByte[(][)] byte should have signature ReadByte[(][)] [(]byte, error[)]"
-}
-
-// printf is used by the test.
-func printf(format string, args ...interface{}) {
-	panic("don't call - testing only")
-}
+		f.walkCall(call, x.Sel.Name)
+	}
+}
  • go/printer, reflect, unicode/utf8 のインポートが削除されました。
  • checkFile 関数が walkFile にリネームされ、AST走査の汎用的な役割を強調しています。
  • File.Visit メソッド内の checkCallExpr, checkFieldTag, checkMethodDecl, checkInterfaceType の呼び出しが、それぞれ walkCallExpr, walkFieldTag, walkMethodDecl, walkInterfaceType に変更されました。
  • 以前 govet.go にあった canonicalMethods マップ、printfList マップ、printList マップ、およびそれらに関連するメソッド（checkMethod, argjoin, typeFlatten, matchParams, matchParamType, checkFieldTag, checkCall, checkPrintf, parsePrintfVerb, checkPrint）が全て削除されました。これらのロジックは新しいファイルに移動されました。
  • テスト用の関数 BadFunctionUsedInTests, BadTypeUsedInTests, BadInterfaceUsedInTests, printf も削除されました。

src/cmd/govet/method.go (新規ファイル)

// Copyright 2010 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// This file contains the code to check canonical methods.

package main

import (
	"fmt"
	"go/ast"
	"go/printer"
	"strings"
)

type MethodSig struct {
	args    []string
	results []string
}

// canonicalMethods lists the input and output types for Go methods
// that are checked using dynamic interface checks.  Because the
// checks are dynamic, such methods would not cause a compile error
// if they have the wrong signature: instead the dynamic check would
// fail, sometimes mysteriously.  If a method is found with a name listed
// here but not the input/output types listed here, govet complains.
//
// A few of the canonical methods have very common names.
// For example, a type might implement a Scan method that
// has nothing to do with fmt.Scanner, but we still want to check
// the methods that are intended to implement fmt.Scanner.
// To do that, the arguments that have a + prefix are treated as
// signals that the canonical meaning is intended: if a Scan
// method doesn't have a fmt.ScanState as its first argument,
// we let it go.  But if it does have a fmt.ScanState, then the
// rest has to match.
var canonicalMethods = map[string]MethodSig{
	// "Flush": {{}, {"error"}}, // http.Flusher and jpeg.writer conflict
	"Format":        {[]string{"=fmt.State", "rune"}, []string{}},            // fmt.Formatter
	"GobDecode":     {[]byte{}, []string{"error"}},                 // gob.GobDecoder
	"GobEncode":     {[]string{}, []byte{}, "error"}},               // gob.GobEncoder
	"MarshalJSON":   {[]string{}, []byte{}, "error"}},               // json.Marshaler
	"MarshalXML":    {[]string{}, []byte{}, "error"}},               // xml.Marshaler
	"Peek":          {[]string{"=int"}, []byte{}, "error"}},         // image.reader (matching bufio.Reader)
	"ReadByte":      {[]string{}, []string{"byte", "error"}},                 // io.ByteReader
	"ReadFrom":      {[]string{"=io.Reader"}, []string{"int64", "error"}},    // io.ReaderFrom
	"ReadRune":      {[]string{}, []string{"rune", "int", "error"}},          // io.RuneReader
	"Scan":          {[]string{"=fmt.ScanState", "rune"}, []string{"error"}}, // fmt.Scanner
	"Seek":          {[]string{"=int64", "int"}, []string{"int64", "error"}}, // io.Seeker
	"UnmarshalJSON": {[]string{"[]byte"}, []string{"error"}},                 // json.Unmarshaler
	"UnreadByte":    {[]string{}, []string{"error"}},
	"UnreadRune":    {[]string{}, []string{"error"}},
	"WriteByte":     {[]string{"byte"}, []string{"error"}},                // jpeg.writer (matching bufio.Writer)
	"WriteTo":       {[]string{"=io.Writer"}, []string{"int64", "error"}}, // io.WriterTo
}

func (f *File) checkCanonicalMethod(id *ast.Ident, t *ast.FuncType) {
	// ... (canonical method checking logic) ...
}

func argjoin(x []string) string {
	// ... (argument joining logic) ...
}

func typeFlatten(l []*ast.Field) []ast.Expr {
	// ... (type flattening logic) ...
}

func (f *File) matchParams(expect []string, actual []ast.Expr, prefix string) bool {
	// ... (parameter matching logic) ...
}

func (f *File) matchParamType(expect string, actual ast.Expr) bool {
	// ... (parameter type matching logic) ...
}
  • canonicalMethods マップと、それに関連するメソッド（checkCanonicalMethod, argjoin, typeFlatten, matchParams, matchParamType）が govet.go からこのファイルに移動されました。
  • これらの関数は、Goの標準インターフェースで定義されているメソッドのシグネチャが正しく実装されているかを検証します。

src/cmd/govet/print.go (新規ファイル)

// Copyright 2010 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// This file contains the printf-checker.

package main

import (
	"flag"
	"fmt"
	"go/ast"
	"go/token"
	"strings"
	"unicode/utf8"
)

var printfuncs = flag.String("printfuncs", "", "comma-separated list of print function names to check")

// printfList records the formatted-print functions. The value is the location
// of the format parameter. Names are lower-cased so the lookup is
// case insensitive.
var printfList = map[string]int{
	"errorf":  0,
	"fatalf":  0,
	"fprintf": 1,
	"panicf":  0,
	"printf":  0,
	"sprintf": 0,
}

// printList records the unformatted-print functions. The value is the location
// of the first parameter to be printed.  Names are lower-cased so the lookup is
// case insensitive.
var printList = map[string]int{
	"error":  0,
	"fatal":  0,
	"fprint": 1, "fprintln": 1,
	"panic": 0, "panicln": 0,
	"print": 0, "println": 0,
	"sprint": 0, "sprintln": 0,
}

// checkCall triggers the print-specific checks if the call invokes a print function.
func (f *File) checkFmtPrintfCall(call *ast.CallExpr, Name string) {
	// ... (print function call checking logic) ...
}

// checkPrintf checks a call to a formatted print routine such as Printf.
func (f *File) checkPrintf(call *ast.CallExpr, name string, skip int) {
	// ... (formatted print checking logic) ...
}

// parsePrintfVerb returns the number of bytes and number of arguments
// consumed by the Printf directive that begins s, including its percent sign
// and verb.
func parsePrintfVerb(s string) (nbytes, nargs int) {
	// ... (printf verb parsing logic) ...
}

// checkPrint checks a call to an unformatted print routine such as Println.
func (f *File) checkPrint(call *ast.CallExpr, name string, skip int) {
	// ... (unformatted print checking logic) ...
}

// This function never executes, but it serves as a simple test for the program.
// Test with make test.
func BadFunctionUsedInTests() {
	// ... (test functions) ...
}

type BadTypeUsedInTests struct {
	X int "hello" // ERROR "struct field tag"
}

func (t *BadTypeUsedInTests) Scan(x fmt.ScanState, c byte) { // ERROR "method Scan[(]x fmt.ScanState, c byte[)] should have signature Scan[(]fmt.ScanState, rune[)] error"
}

type BadInterfaceUsedInTests interface {
	ReadByte() byte // ERROR "method ReadByte[(][)] byte should have signature ReadByte[(][)] [(]byte, error[)]"
}

// printf is used by the test.
func printf(format string, args ...interface{}) {
	panic("don't call - testing only")
}
  • printfuncs フラグ、printfList マップ、printList マップ、およびそれらに関連するメソッド（checkFmtPrintfCall, checkPrintf, parsePrintfVerb, checkPrint）が govet.go からこのファイルに移動されました。
  • これらの関数は、fmt.Printf や fmt.Println などの関数呼び出しにおけるフォーマット文字列と引数の整合性を検証します。
  • テスト用の関数 BadFunctionUsedInTests, BadTypeUsedInTests, BadInterfaceUsedInTests, printf もこのファイルに移動されました。

src/cmd/govet/structtag.go (新規ファイル)

// Copyright 2010 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// This file contains the test for canonical struct tags.

package main

import (
	"go/ast"
	"reflect"
	"strconv"
)

// checkField checks a struct field tag.
func (f *File) checkCanonicalFieldTag(field *ast.Field) {
	// ... (struct tag checking logic) ...
}
  • checkCanonicalFieldTag 関数が govet.go からこのファイルに移動されました。
  • この関数は、構造体フィールドのタグが reflect.StructTag で正しく解析できるかを検証します。

コアとなるコードの解説

このコミットの核となる変更は、govet の静的解析ロジックを、その機能に基づいて複数のファイルに分割したことです。

以前の govet.go では、File 構造体の Visit メソッドがASTを走査し、その中でメソッドシグネチャのチェック、printf フォーマットのチェック、構造体タグのチェックなど、全ての検査ロジックを直接呼び出していました。これは、単一責任の原則に反し、コードの凝集度を低下させていました。

今回の変更では、govet.go はASTの走査と、各ノードタイプに応じた汎用的な「ウォーク」処理（walkCallExpr, walkFieldTag, walkMethodDecl, walkInterfaceType）に特化しました。これらの「ウォーク」処理は、具体的な検査ロジックを、新しく作成された method.go, print.go, structtag.go ファイル内の対応する check 関数（checkCanonicalMethod, checkFmtPrintfCall, checkCanonicalFieldTag）に委譲します。

例えば、govet.go の File.Visit メソッドが ast.CallExpr ノードに遭遇した場合、以前は直接 f.checkCallExpr(n) を呼び出していましたが、変更後は f.walkCallExpr(n) を呼び出します。そして、walkCallExpr の中で、print.go に移動された f.checkFmtPrintfCall(call, name) が呼び出され、printf 関連の検査が実行される、という流れになります。

このアーキテクチャ変更により、各ファイルは特定の検査ロジックに集中できるようになり、以下の利点が得られます。

  • 関心の分離 (Separation of Concerns): 各ファイルが特定の「vetting suite」のロジックのみを扱うため、コードの理解と変更が容易になります。
  • 再利用性の向上: 汎用的なAST走査ロジックが govet.go に集約されたことで、新しい検査を追加する際に、既存の走査メカニズムを再利用できます。
  • テストの容易性: 各検査ロジックが独立した関数として存在するため、単体テストが書きやすくなります。
  • 並行開発の促進: 異なる開発者が異なる検査ロジックを同時に開発・修正する際に、コードの競合が減少します。

このコミットは、govet の長期的な保守性と拡張性を確保するための、基盤となる重要なリファクタリングと言えます。

関連リンク

参考にした情報源リンク

  • Go言語の公式ドキュメント (go/ast, go/parser, go/token パッケージ): https://pkg.go.dev/go/ast, https://pkg.go.dev/go/parser, https://pkg.go.dev/go/token
  • govet の概要に関する情報 (Go公式ブログなど): https://blog.golang.org/go-vet (一般的な情報源として、具体的な記事はコミット時点のものではない可能性がありますが、govet の目的を理解するのに役立ちます。)
  • 静的解析に関する一般的な情報: https://en.wikipedia.org/wiki/Static_program_analysis
  • GoのASTウォークに関するチュートリアルや記事 (例: https://yourbasic.org/golang/go-ast-example/ など、一般的なASTウォークの概念を理解するためのもの)