Review: by yuki.ito - Githubissues

marx-saul / go_pure

A static analysis tool for go-lang that ensure some designated functions are pure.

MIT License

1 stars 0 forks source link

Review: by yuki.ito #1

Open 110y opened 4 years ago

110y commented 4 years ago

Goの最新版である1.15を利用したほうが良いと思います
- https://github.com/marx-saul/go-pure/blob/e4c47c3a6e9d5ad228086be2d6752104e90ec448/go.mod#L3-L3
module名は単に pure とするのではなく github.com/marx-saul/go-pure とするべきです
- 単に pure だと標準packageと混同してしまうので
- go.modファイルを go mod init で生成すると自動で適切なmodule名を付与してくれます
- https://github.com/marx-saul/go-pure/blob/e4c47c3a6e9d5ad228086be2d6752104e90ec448/go.mod#L1-L1

110y commented 4 years ago

ここでは strings.Contains を使ったほうが良さそうです
- https://github.com/marx-saul/go-pure/blob/e4c47c3a6e9d5ad228086be2d6752104e90ec448/pure.go#L75-L75
現在の仕様だと例えばこの行のように、コメントの中間に @pure が入っている関数も対象になりますが意図通りですか？
- https://github.com/marx-saul/go-pure/blob/e4c47c3a6e9d5ad228086be2d6752104e90ec448/pure.go#L69-L69

110y commented 4 years ago

Docに @pure が付いているものを探索していますが、Go向けの有名な静的解析ツールであるstaticcheck等が採用しているように、 // lint:pure などのほうが自然かなと思いました。 (@ でアノテートしているLinterはいままで見たことないので） https://staticcheck.io/docs/checks

110y commented 4 years ago

nil pointerによるpanicを回避するために、mapへのアクセス及びtype assertionでは第2戻り値であるboolを受け取って処理するのが良いですね。
- https://github.com/marx-saul/go-pure/blob/e4c47c3a6e9d5ad228086be2d6752104e90ec448/pure.go#L50-L50