Go 1.17 Release祝い 細かすぎて伝わらないMinor change in flag package

Go 1.17 のリリースノートが出ましたね。

https://tip.golang.org/doc/go1.17

Go 1.17 is not yet released. These are work-in-progress release notes. Go 1.17 is expected to be released in August 2021.

もうすぐですね。Minor changesを眺めるのが好きなのですが、とっても内部コードを読むのに良さそうな物を見つけました。

flag packageでinvalidなflag名指定の場合のハンドリングに修正が入る

こんな変更が入ったらしい。

Flag declarations now panic if an invalid name is specified.

https://tip.golang.org/doc/go1.17#flag

たどってみるとこのChangesだねってわかる。

https://go-review.googlesource.com/c/go/+/271788/

変更は説明通りpanicで落とすようにしたよっていう感じのもの。テストコードを見ると期待値がわかりやすい。

https://go-review.googlesource.com/c/go/+/271788/6/src/flag/flag_test.go

		{
			flag:     "-foo",
			errorMsg: "flag \"-foo\" begins with -",
		},
		{
			flag:     "foo=bar",
			errorMsg: "flag \"foo=bar\" contains =",
		},

flag自体に-が入ってたり=が入っている場合にこれまで通っていたけどpanicにするよっていう子のようだ。

実際にflag_test.goから該当テストを単体ファイルで動作可能な感じで持ってくるとこんなテスト

package main

import (
	"bytes"
	"flag"
	"fmt"
	"testing"
)

type flagVar []string

func (f *flagVar) String() string {
	return fmt.Sprint([]string(*f))
}

func (f *flagVar) Set(value string) error {
	*f = append(*f, value)
	return nil
}

func TestInvalidFlags(t *testing.T) {
	tests := []struct {
		flag     string
		errorMsg string
	}{
		{
			flag:     "-foo",
			errorMsg: "flag \"-foo\" begins with -",
		},
		{
			flag:     "foo=bar",
			errorMsg: "flag \"foo=bar\" contains =",
		},
	}

	for _, test := range tests {
		testName := fmt.Sprintf("FlagSet.Var(&v, %q, \"\")", test.flag)

		fs := flag.NewFlagSet("", flag.ContinueOnError)
		buf := bytes.NewBuffer(nil)
		fs.SetOutput(buf)

		var v flagVar
		fs.Var(&v, test.flag, "")
		if msg := test.errorMsg + "\n"; msg != buf.String() {
			t.Errorf("%s\n: unexpected output: expected %q, bug got %q", testName, msg, buf)
		}
	}
}

これをGo1.16の動作環境で動かすとpanicは順当におこらない。

$ go version
go version go1.16.1 darwin/amd64
$ go test main_test.go 
--- FAIL: TestInvalidFlags (0.00s)
    main_test.go:46: FlagSet.Var(&v, "-foo", "")
        : unexpected output: expected "flag \"-foo\" begins with -\n", bug got ""
    main_test.go:46: FlagSet.Var(&v, "foo=bar", "")
        : unexpected output: expected "flag \"foo=bar\" contains =\n", bug got ""
FAIL
FAIL    command-line-arguments  0.096s
FAIL

Go1.17ならどうかな？

来る金曜日のGo 1.17 Release PartyではありがたいことにサクッとコピペできるGo 1.17 downloadできるコマンドを張ってくださっている。ホスピタリティが高くてすごい...。

https://gocon.connpass.com/event/216361/

$ go get golang.org/dl/go1.17beta1
$ go1.17beta1 download
$ go1.17beta1 version
go version go1.17beta1 darwin/amd64

さぁ実行してみよう

$ go1.17beta1 test main_test.go
--- FAIL: TestInvalidFlags (0.00s)
panic: flag "-foo" begins with - [recovered]
        panic: flag "-foo" begins with -

goroutine 18 [running]:

(omit)
FAIL    command-line-arguments  0.207s
FAIL

これでテストの期待値で見てもわかるがflag -foo なんてもんはinvalidなんなんだぜってことがわかる。

どうしてこの変更が入った？

さてなんでこの変更が入ったのかfixしにいったissueを見に行く。

https://github.com/golang/go/issues/41792

Currently, the flag package allows any string to be used as the name of the flag. It only validates whether the name of the flag is already registered. (code)

But, if the name of the flag is (1) empty, (2) starts with a hyphen, or (3) contains an equal sign, the flag can not be parsed properly. If you try to use the flag as described in the usage message, it will fail. (code)

もともと設定できるかどうかでは重複しているかどうかくらいを確認していた。
しかし-や=はParseがうまくいかない。

Parseがうまくいかんっつうのはどういうことかっていうと

	_ = fs.Parse([]string{"--hyphen"}) // bad flag syntax: -=foobar

ってやったとてflag provided but not defined: -hyphenっていうエラーになるし、

	_ = fs.Parse([]string{"-=foobar"}) // bad flag syntax: -=foobar

ってやったとしてbad flag syntax: -=foobarってなるわけでParse時に期待したものが出てこない。CLI作成者とかが期待したflag設定がこの時点で来ていないのでどこかでValidateしてあげなあかんのちゃう？ってのがProposalの課題意識。

ゆえに設定時のValidationをしようよっていうのがProposalだった。

Issue上でのDiscussionの末approveされGo 1.17 でお披露目されたのであった。

https://github.com/golang/go/issues/41792#issuecomment-725589312

Go 1.17楽しいね

https://gocon.connpass.com/event/216361/ 金曜日頑張って時間開けてリリパ参加しよ...。

後この話はBASE BANK, Inc.のGo Code Reading Partyで話題にしてもらって調べてみた子でした（わいわい）。

https://github.com/basebank/gophers-code-reading-party/issues/9

Check basebank-code-reading-ja yeah.