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

このコミットは、Go言語の静的解析ツールである cmd/vet における誤検知（false positive）を修正するものです。具体的には、スライスや配列の複合リテラルが、タグ付けされていないフィールドを持つ構造体リテラルとして誤って警告される問題を解消します。この修正は、go/types パッケージを利用して、複合リテラルの正確な型情報を取得することで実現されています。

コミット

commit 83e22f1a47ee3bb7ceb1ceb5fbe1c5e13f7fe131
Author: Rob Pike <r@golang.org>
Date:   Sun Feb 24 13:18:21 2013 -0800

    cmd/vet: eliminate false positives for slices in untagged literal test
    Made possible by go/types, as long as the package type-checks OK.
    
    Fixes #4684.
    
    R=golang-dev
    CC=golang-dev
    https://golang.org/cl/7407045

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

https://github.com/golang/go/commit/83e22f1a47ee3bb7ceb1ceb5fbe1c5e13f7fe131

元コミット内容

cmd/vet: eliminate false positives for slices in untagged literal test Made possible by go/types, as long as the package type-checks OK.

Fixes #4684.

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

変更の背景

Go言語の vet ツールは、コード内の疑わしい構造を報告する静的解析ツールです。その機能の一つに、構造体リテラルでフィールド名を明示的に指定しない「タグ付けされていないフィールド（untagged fields）」の使用を警告するものがあります。これは、構造体のフィールド順序が変更された場合に、コードが意図せず壊れる可能性を防ぐためのものです。

しかし、このチェックはスライスや配列の複合リテラルに対しても誤って適用されていました。スライスや配列は、構造体のように名前付きフィールドを持たないため、「タグ付けされていないフィールド」という概念自体が当てはまりません。例えば、[]int{1, 2, 3} のようなスライスリテラルは、その性質上、常に「タグ付けされていない」形式で記述されますが、これが vet によって不必要に警告されてしまうという問題がありました（Issue #4684）。

このコミットは、この誤検知を解消し、vet がスライスや配列の複合リテラルに対しては警告を発しないようにすることで、ツールの精度と利便性を向上させることを目的としています。

前提知識の解説

• cmd/vet: Go言語の標準的な静的解析ツールの一つ。コンパイルは通るが、潜在的なバグや疑わしいコードパターンを検出します。例えば、到達不能なコード、誤ったフォーマット文字列、ロックの誤用などを検出します。
• 複合リテラル (Composite Literals): Go言語で構造体、配列、スライス、マップなどの複合型を初期化するための構文です。
  • 構造体リテラル: struct { X int; Y int }{X: 1, Y: 2} のようにフィールド名を明示的に指定する方法（タグ付けされたフィールド）と、struct { X int; Y int }{1, 2} のようにフィールド名を省略して順序で指定する方法（タグ付けされていないフィールド）があります。vet は後者に対して警告を出すことがあります。
  • スライスリテラル: []int{1, 2, 3} のように要素を列挙して初期化します。スライスはインデックスでアクセスされるため、構造体のような「フィールド名」の概念はありません。
  • 配列リテラル: [3]int{1, 2, 3} のように要素を列挙して初期化します。配列もインデックスでアクセスされるため、構造体のような「フィールド名」の概念はありません。
• go/ast パッケージ: Goのソースコードを抽象構文木（AST: Abstract Syntax Tree）として表現するためのパッケージです。コンパイラや静的解析ツールがGoコードを解析する際に使用します。ast.CompositeLit は複合リテラルを表すASTノードです。
• go/types パッケージ: Goの型システムに関する情報を提供するパッケージです。Goプログラムの型チェックを行い、式や宣言の型情報を取得できます。このパッケージは、コンパイル時に利用可能な正確な型情報を提供するため、vet のようなツールがより賢い判断を下すのに役立ちます。例えば、go/types.Slice はスライス型を、go/types.Array は配列型を、go/types.Struct は構造体型を表します。
• 誤検知 (False Positive): 静的解析ツールやセキュリティツールが、実際には問題のないコードを問題があると誤って報告すること。

技術的詳細

このコミットの核心は、cmd/vet の taglit.go ファイルにある checkUntaggedLiteral 関数に、go/types パッケージを用いた型チェックロジックを追加した点です。

以前の checkUntaggedLiteral 関数は、複合リテラルが「タグ付けされていないフィールド」を持つかどうかを、そのAST構造のみに基づいて判断していました。しかし、ASTだけでは、その複合リテラルが構造体の初期化なのか、それともスライスや配列の初期化なのかを区別することは困難でした。

新しいロジックでは、以下の手順で型を特定します。

1. f.pkg.types[c] を使用して、現在の複合リテラル c の型情報を取得します。ここで f.pkg は、解析中のパッケージの型情報（go/types.Package）を保持しています。
2. 取得した型 typ が nil でないことを確認します。nil の場合、型情報が利用できない（例えば、パッケージが型チェックに失敗している）ため、従来のロジックにフォールバックします。
3. 型が *types.NamedType（例えば、type MySlice []int のような名前付き型）である場合、その基底型（Underlying）を取得します。これにより、名前付き型であってもその実体がスライスや配列であるかを正確に判断できます。
4. 基底型が *types.Slice または *types.Array のインスタンスである場合、その複合リテラルはスライスまたは配列の初期化であることが確定します。この場合、タグ付けされていないフィールドの警告は不要であるため、関数は即座に return します。
5. 上記のいずれにも該当しない場合（つまり、型が構造体であるか、型情報が利用できないため判断できない場合）、従来のロジックに従ってタグ付けされていないフィールドのチェックを続行します。

また、テストの柔軟性を高めるために、compositewhitelist という新しいフラグが追加されました。これは、untaggedLiteralWhitelist の適用を制御するためのもので、テスト時に特定の警告を意図的に有効/無効にするために使用されます。Makefile の変更は、この新しいフラグをテスト実行時に false に設定することで、スライス/配列の誤検知修正が正しく機能するかを検証しています。

警告メッセージも「%s struct literal uses untagged fields」から「%s composite literal uses untagged fields」に変更され、より一般的な「複合リテラル」という表現を用いることで、警告の対象が構造体に限定されないことを示唆しています。ただし、このコミットの目的はスライス/配列の誤検知をなくすことなので、実質的には構造体リテラルにのみ警告が発せられるようになります。

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

src/cmd/vet/Makefile

--- a/src/cmd/vet/Makefile
+++ b/src/cmd/vet/Makefile
@@ -4,5 +4,5 @@
 
 test testshort:
 	go build -tags unsafe
-	../../../test/errchk ./vet -printfuncs='Warn:1,Warnf:1' *.go
+	../../../test/test/errchk ./vet -compositewhitelist=false -printfuncs='Warn:1,Warnf:1' *.go

test コマンドの実行時に、vet に -compositewhitelist=false オプションが追加されました。これは、テスト時にホワイトリスト機能を無効にし、新しい型チェックロジックが正しく機能するかを検証するためのものです。

src/cmd/vet/taglit.go

--- a/src/cmd/vet/taglit.go
+++ b/src/cmd/vet/taglit.go
@@ -7,18 +7,42 @@
 package main
 
 import (
+\t"flag"
 \t"go/ast"
+\t"go/types"
 \t"strings"
 
-\t"flag" // for test
+\t"go/scanner" // for test; chosen because it's already linked in.
 )
 
+var compositeWhiteList = flag.Bool("compositewhitelist", true, "use composite white list; for testing only")
+
 // checkUntaggedLiteral checks if a composite literal is an struct literal with
 // untagged fields.
 func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) {
  if !vet("composites") {
  	return
  }
+\
+\t// Check that the CompositeLit's type is a slice or array (which need no tag), if possible.
+\tif f.pkg != nil {
+\t\ttyp := f.pkg.types[c]
+\t\tif typ != nil {
+\t\t\t// If it's a named type, pull out the underlying type.
+\t\t\tif namedType, ok := typ.(*types.NamedType); ok {
+\t\t\t\ttyp = namedType.Underlying
+\t\t\t}\
+\t\t\tswitch typ.(type) {
+\t\t\tcase *types.Slice:
+\t\t\t\treturn
+\t\t\tcase *types.Array:
+\t\t\t\treturn
+\t\t\t}\
+\t\t}\
+\t}\
+\
+\t// It's a struct, or we can't tell it's not a struct because we don't have types.
+\
  // Check if the CompositeLit contains an untagged field.
  allKeyValue := true
  for _, e := range c.Elts {
@@ -48,11 +72,11 @@ func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) {\
  	return
  }
  typ := path + "." + s.Sel.Name
-\tif untaggedLiteralWhitelist[typ] {\n+\tif *compositeWhiteList && untaggedLiteralWhitelist[typ] {\n  	return
  }
  
-\tf.Warnf(c.Pos(), "%s struct literal uses untagged fields", typ)\n+\tf.Warnf(c.Pos(), "%s composite literal uses untagged fields", typ)", typ)\n }
 
 // pkgPath returns the import path "image/png" for the package name "png".
@@ -125,9 +149,17 @@ var untaggedLiteralWhitelist = map[string]bool{\
  	"image.Rectangle":     true,\
  }\
  
+// Testing is awkward because we need to reference things from a separate package
+// to trigger the warnings.
+\
  var BadStructLiteralUsedInTests = flag.Flag{ // ERROR "untagged fields"\
  	"Name",
  	"Usage",
  	nil, // Value
  	"DefValue",
  }\
+\
+// Used to test the check for slices and arrays: If that test is disabled and
+// vet is run with --compositewhitelist=false, this line triggers an error.
+// Clumsy but sufficient.
+var scannerErrorListTest = scanner.ErrorList{nil, nil}

コアとなるコードの解説

taglit.go の checkUntaggedLiteral 関数内の変更がこのコミットの主要な部分です。

1. go/types のインポート: import ("go/types") が追加され、Goの型情報にアクセスできるようになりました。
2. compositeWhiteList フラグの追加:

   var compositeWhiteList = flag.Bool("compositewhitelist", true, "use composite white list; for testing only")
   

   これは、untaggedLiteralWhitelist の適用を制御するためのブール型フラグです。デフォルトは true で、ホワイトリストが有効になります。テスト時に false に設定することで、ホワイトリストを無効にした状態での動作を検証できます。
3. 型チェックロジックの追加:

   	if f.pkg != nil {
   		typ := f.pkg.types[c]
   		if typ != nil {
   			// If it's a named type, pull out the underlying type.
   			if namedType, ok := typ.(*types.NamedType); ok {
   				typ = namedType.Underlying
   			}
   			switch typ.(type) {
   			case *types.Slice:
   				return
   			case *types.Array:
   				return
   			}
   		}
   	}
   

   このブロックが、複合リテラル c の型を f.pkg.types[c] で取得し、それがスライス (*types.Slice) または配列 (*types.Array) であれば、警告をスキップ (return) します。*types.NamedType の場合は Underlying 型をチェックすることで、type MySlice []int のようなカスタム型にも対応しています。これにより、スライスや配列の複合リテラルに対する誤検知が排除されます。
4. ホワイトリスト適用条件の変更:

   	if *compositeWhiteList && untaggedLiteralWhitelist[typ] {
   		return
   	}
   

   untaggedLiteralWhitelist の適用が、新しく追加された compositeWhiteList フラグの値に依存するようになりました。これにより、テスト時にホワイトリストの挙動を細かく制御できます。
5. 警告メッセージの変更:

   	f.Warnf(c.Pos(), "%s composite literal uses untagged fields", typ)
   

   警告メッセージが「struct literal」から「composite literal」に変更されました。これは、警告の対象が構造体に限定されないことを示すための一般的な表現ですが、このコミットの変更により、実質的には構造体リテラルにのみ警告が発せられるようになります。
6. テスト用変数の追加:

   var scannerErrorListTest = scanner.ErrorList{nil, nil}
   

   これは、スライスと配列のチェックが正しく機能するかをテストするための補助的な変数です。--compositewhitelist=false で vet を実行した場合に、この行が意図的にエラーをトリガーするように設計されています。

これらの変更により、cmd/vet は複合リテラルの型を正確に識別し、スライスや配列の初期化に対しては不必要な警告を発しなくなりました。

関連リンク

• Go Issue #4684: cmd/vet: untagged struct literal check should ignore slices and arrays
• Go CL 7407045: https://golang.org/cl/7407045 (このコミットに対応するGerritの変更リスト)

参考にした情報源リンク

• Go言語の公式ドキュメント (go/ast, go/types パッケージに関するもの)
• Go言語の vet ツールのドキュメント
• Go言語の複合リテラルに関する解説記事
• Go言語のIssueトラッカー (特に #4684)