Giter VIP home page Giter VIP logo

zagane's Introduction

zagane

CircleCI GoDoc

zagane is a static analysis tool which can find bugs in spanner's code. zagane consists of several analyzers.

  • unstopiter: it finds iterators which did not stop.
  • unclosetx: it finds transactions which does not close
  • wraperr: it finds (*spanner.Client).ReadWriteTransaction calls which returns wrapped errors

Install

You can get zagane by go get command.

$ go get -u github.com/gcpug/zagane

How to use

zagane run with go vet as below when Go is 1.12 and higher.

$ go vet -vettool=$(which zagane) github.com/gcpug/spshovel/...
# github.com/gcpug/spshovel/spanner
spanner/spanner_service.go:29:29: iterator must be stop

When Go is lower than 1.12, just run zagane command with the package name (import path). But it cannot accept some options such as --tags.

$ zagane github.com/gcpug/spshovel/...
~/go/src/github.com/gcpug/spshovel/spanner/spanner_service.go:29:29: iterator must be stop

Analyzers

unstopiter

unstopiter finds spanner.RowIterator which is not calling Stop method or Do method such as below code.

iter := client.Single().Query(ctx, stmt)
for {
	row, err := iter.Next()
	// ...
}

This code must be fixed as below.

iter := client.Single().Query(ctx, stmt)
defer iter.Stop()
for {
	row, err := iter.Next()
	// ...
}

unclosetx

unclosetx finds spanner.ReadOnlyTransaction which is not calling Close method such as below code.

tx := client.ReadOnlyTransaction()
// ...

This code must be fixed as below.

tx := client.ReadOnlyTransaction()
defer tx.Close()
// ...

When a transaction is created by (*spanner.Client).Single, unclosetx ignore it.

wraperr

wraperr finds ReadWriteTransaction calls which returns wrapped errors such as the below code.

func f(ctx context.Context, client *spanner.Client) {
	client.ReadWriteTransaction(ctx, func(ctx context.Context, txn *spanner.ReadWriteTransaction) error {
		stmt := spanner.Statement{SQL: `SELECT 1`}
		_, err := client.Single().Query(ctx, stmt).Next()
		if err != nil {
			return errors.Wrap(err, "wrapped") // want "must not be wrapped"
		}
		return nil
	})
}

This code must be fixed as below.

func f(ctx context.Context, client *spanner.Client) {
	client.ReadWriteTransaction(ctx, func(ctx context.Context, txn *spanner.ReadWriteTransaction) error {
		stmt := spanner.Statement{SQL: `SELECT 1`}
		_, err := client.Single().Query(ctx, stmt).Next()
		if err != nil {
			return err
		}
		return nil
	})
}

Ignore Checks

Analyzers ignore nodes which are annotated by staticcheck's style comments as belows. A ignore comment includes analyzer names and reason of ignoring checking. If you specify zagane as analyzer name, all analyzers ignore corresponding code.

//lint:ignore zagane reason
var n int

//lint:ignore unstopiter reason
_, _ = client.Single().Query(ctx, stmt).Next()

Analyze with golang.org/x/tools/go/analysis

You can get analyzers of zagane from zagane.Analyzers. And you can use them with unitchecker.

Why name is "zagane"?

"zagane" (座金) means "washer" in Japanese. A washer works between a spanner and other parts. zagane also works between Cloud Spanner and your applications.

zagane's People

Contributors

sinmetal avatar tenntenn avatar vvakame avatar zchee avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

zagane's Issues

Fix version of target spanner package

zagame must fix version of target spanner package because spanner package will update and zagame also will follow it. But some users of zagane may not update spanner package by own reason.

Related issue: #29

bug: panic with type parameters

Go version

$ go version
go version go1.18.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

Try running "zagane" in the following code.

// main.go
package main

import "fmt"

func main() {
	Print([]string{"xxx"})
}

func Print[T any](s []T) {
	for _, v := range s {
		fmt.Println(v)
	}
}

$ go vet -vettool=(which zagane) main.go

What did you expect to see?

No errors.

What did you see?

# command-line-arguments
panic: T

goroutine 17 [running]:
golang.org/x/tools/go/ssa.(*Program).needMethods(0xc0000940d0, {0x132b478?, 0xc0001a21b0?}, 0x0)
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/ssa/methods.go:237 +0x5b1
golang.org/x/tools/go/ssa.(*Program).needMethods(0xc0000940d0, {0x132b400?, 0xc000063d40?}, 0x0)
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/ssa/methods.go:196 +0x347
golang.org/x/tools/go/ssa.(*Program).needMethods(0xc0000940d0, {0x132b450?, 0xc00000d1a0?}, 0x0)
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/ssa/methods.go:233 +0x708
golang.org/x/tools/go/ssa.(*Program).needMethods(0xc0000940d0, {0x132b3d8?, 0xc000198bc0?}, 0x0)
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/ssa/methods.go:209 +0x448
golang.org/x/tools/go/ssa.(*Program).needMethodsOf(0xc0000940d0, {0x132b3d8?, 0xc000198bc0?})
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/ssa/methods.go:145 +0x70
golang.org/x/tools/go/ssa.(*Package).build(0xc000090360)
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/ssa/builder.go:2284 +0x111
sync.(*Once).doSlow(0xc0000940d0?, 0xc0000776d0?)
        /Users/shota/.goenv/versions/1.18.2/src/sync/once.go:68 +0xc2
sync.(*Once).Do(...)
        /Users/shota/.goenv/versions/1.18.2/src/sync/once.go:59
golang.org/x/tools/go/ssa.(*Package).Build(...)
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/ssa/builder.go:2272
golang.org/x/tools/go/analysis/passes/buildssa.run(0xc000094000)
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/analysis/passes/buildssa/buildssa.go:72 +0x2ee
golang.org/x/tools/go/analysis/unitchecker.run.func5.1()
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/analysis/unitchecker/unitchecker.go:355 +0x82e
sync.(*Once).doSlow(0x0?, 0x0?)
        /Users/shota/.goenv/versions/1.18.2/src/sync/once.go:68 +0xc2
sync.(*Once).Do(...)
        /Users/shota/.goenv/versions/1.18.2/src/sync/once.go:59
golang.org/x/tools/go/analysis/unitchecker.run.func5(0x149f380?)
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/analysis/unitchecker/unitchecker.go:307 +0x1a5
golang.org/x/tools/go/analysis/unitchecker.run.func6.1(0x0?)
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/analysis/unitchecker/unitchecker.go:367 +0x29
created by golang.org/x/tools/go/analysis/unitchecker.run.func6
        /Users/shota/go/1.18.2/pkg/mod/golang.org/x/[email protected]/go/analysis/unitchecker/unitchecker.go:366 +0x47

Related issues?

proposal: support for BatchReadOnlyTransaction

This sample code become OK.

package a

import (
	"context"

	"cloud.google.com/go/spanner"
)

func f1(ctx context.Context, client *spanner.Client) {
	ro, _ := client.BatchReadOnlyTransaction(ctx, spanner.StrongRead())
	if ro != nil {
	}
}

proposal: rename Analyzer name

I think current name of Analyzer is not straightforward with users.
So I'd like to change the name such as below.

  • unstopiter and unclosetx will merge into sessionleak
  • wraperr will be changed to retryless

unclosetx: not detect about nested struct having Spanner's client

unclosetx cannot detect looks like this sample code.

func (h *Handler) fetchUser(ctx context.Context, userID string) (*database.User, error) {
	txn := h.repository.Client.ReadOnlyTransaction() // h.repository.Client is a Spanner client.
	
	user, err := database.FindUser(ctx, txn, userID)
	if err != nil {
		return nil, err
	}
	return user, nil
}

add document

ユースケースなどのドキュメントを作成する

unclosetx: not detect when a function called with a ReadOnlyTransaction argument

unclosetx cannot detect looks like this sample code.

func f7(ctx context.Context, client *spanner.Client) error {
	ro := client.ReadOnlyTransaction()
	err := Find01(ctx, ro)
	return err
}

func Find01(ctx context.Context, ro *spanner.ReadOnlyTransaction) error {
	row, err := ro.ReadRow(ctx, "DummyTable", spanner.Key{1}, []string{"Id"})
	log.Printf("row: %s", row)
	return err
}

proposal: spanner.CommitTimestamp check

WHAT

Detect that CommitTimestamp contains time.Now () in TIMESTAMP column with allow_commit_timestamp = true

WHY

Unless there are special circumstances, set the commit_timestamp column to
spanner.CommitTimestamp.
If time.Now () is included, Transaction will fail if a future time is set due to the time difference of the execution environment.

It's hard to notice in Test that the time is a few ms in the future, so it would be nice to be able to find it by static analysis.

However, it is difficult to judge whether it is allow_commit_timestamp = true because it is time.Time type in Go source code.
yo will add a new tag to the allow_commit_timestamp column of the generated code.

I can't come up with a good way to determine if a TIMESTAMP column is an allow_commit_timestamp column if I'm still using google-cloud-go 🤔

unclosetx: false positive detection with errgroup

When I use errgroup, zagane reports transaction must be closed. I think that it's false positive detection.

sample code:

func f5(ctx context.Context, client *spanner.Client) error {
	tx := client.ReadOnlyTransaction() // OK
	defer tx.Close()

	var eg errgroup.Group

	eg.Go(func() error {
		_ = tx // use tx
		return nil
	})

	if err := eg.Wait(); err != nil {
		return err
	}
	return nil
}

bug: ignore comments don't work

repro

  1. git clone https://github.com/gcpug/zagane.git
  2. cd zagane
  3. edit test code as follows.
diff --git a/passes/unstopiter/testdata/src/a/a.go b/passes/unstopiter/testdata/src/a/a.go
index 0ad4cf5..c78dd04 100644
--- a/passes/unstopiter/testdata/src/a/a.go
+++ b/passes/unstopiter/testdata/src/a/a.go
@@ -8,7 +8,8 @@ import (

 func f1(ctx context.Context, client *spanner.Client) {
        stmt := spanner.Statement{SQL: `SELECT 1`}
-       _, _ = client.Single().Query(ctx, stmt).Next() // want "iterator must be stopped"
+       //lint:ignore unstopiter reason
+       _, _ = client.Single().Query(ctx, stmt).Next()
        client.Single().Query(ctx, stmt).Stop()        // OK
        defer client.Single().Query(ctx, stmt).Stop()  // OK
 }
  1. go test .\passes\unstopiter\unstopiter_test.go

Expected: Test passes
Actual: Test fails

$ go test .\passes\unstopiter\unstopiter_test.go
--- FAIL: Test (2.40s)
    analysistest.go:251: a/a.go:12:30: unexpected diagnostic: iterator must be stopped
FAIL
FAIL    command-line-arguments  2.550s

This is maybe github.com/gostaticanalysis/comment bug or misusage.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.