Fix for BatchReadOnlyTransaction check #48
Conversation
passes/unclosetx/testdata/src/a/a.go
Outdated
|
|
||
| for { | ||
| _, err := iter.Next() | ||
| if err == iterator.Done { |
There was a problem hiding this comment.
実際に実行するわけじゃないのだろうけど、err != nilだったら、一生このループ抜けられないのか・・・ｗって思ってしまったｗ
| @@ -73,3 +73,22 @@ func f6(ctx context.Context, client *spanner.Client) error { | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| func f7(ctx context.Context, client *spanner.Client) error { | |||
There was a problem hiding this comment.
Could you add a test case which does not call Close method with a // want comment.
There was a problem hiding this comment.
f5 is that the testcase.
zagane/passes/unclosetx/testdata/src/a/a.go
Lines 54 to 57 in effcadc
| @@ -59,7 +68,7 @@ func run(pass *analysis.Pass) (interface{}, error) { | |||
| // skip this | |||
| continue | |||
| } | |||
| instrs := analysisutil.NotCalledIn(f, txTyp, methods...) | |||
| instrs := analysisutil.NotCalledIn(f, txTypeRo, methods...) | |||
There was a problem hiding this comment.
Because the second argument of NotCalledIn is a receiver of the methods, it cannot find txTypeBatch's methods.
I think you should call NotCalledIn two times for txTypRo and txTypeBatch and merge theses return values into instrs.
There was a problem hiding this comment.
When I use this:
instrs := append(
analysisutil.NotCalledIn(f, txTypeRo, methods...),
analysisutil.NotCalledIn(f, txTypeBatch, methods...)...,
)
for _, instr := range instrs {
...
The test says:
/my/go/path/root/sters/zagane/passes/unclosetx/analysistest.go:446: a/a.go:55:42: unexpected diagnostic: transaction must be closed
it points to this test case:
I have no idea to solve this test failure. 🤔
|
Let me close this PR. Feel free to pick my commit. 🙏 |
I have a weird case of using
BatchReadOnlyTransaction, please check f7 testcase.I guess the reason is
BatchReadOnlyTransaction.CloseandReadOnlyTransaction.Closeare different.https://github.com/googleapis/google-cloud-go/blob/main/spanner/batch.go#L246-L251
So, I added BatchReadOnlyTransaction.Close to close method list.
(I don't have confidence on my changes.)