diff --git a/.github/workflows/go-cross.yml b/.github/workflows/go-cross.yml new file mode 100644 index 0000000..30ba322 --- /dev/null +++ b/.github/workflows/go-cross.yml @@ -0,0 +1,45 @@ +name: Go Matrix +on: [push, pull_request] + +jobs: + + cross: + name: Go + runs-on: ${{ matrix.os }} + env: + CGO_ENABLED: 0 + + strategy: + matrix: + go-version: [ 1.17, 1.18, 1.x ] + os: [ubuntu-latest, macos-latest, windows-latest] + + steps: + # https://github.com/marketplace/actions/setup-go-environment + - name: Set up Go ${{ matrix.go-version }} + uses: actions/setup-go@v2 + with: + go-version: ${{ matrix.go-version }} + + # https://github.com/marketplace/actions/checkout + - name: Checkout code + uses: actions/checkout@v2 + + # https://github.com/marketplace/actions/cache + - name: Cache Go modules + uses: actions/cache@v2 + with: + path: | + ~/go/pkg/mod # Module download cache + ~/.cache/go-build # Build cache (Linux) + ~/Library/Caches/go-build # Build cache (Mac) + '%LocalAppData%\go-build' # Build cache (Windows) + key: ${{ runner.os }}-${{ matrix.go-version }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-${{ matrix.go-version }}-go- + + - name: Test + run: go test -v -cover ./... + + - name: Build + run: go build -v -ldflags "-s -w" -trimpath ./cmd/execinquery/ diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..fea964d --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,50 @@ +name: Main + +on: + push: + branches: + - master + pull_request: + +jobs: + + main: + name: Main Process + runs-on: ubuntu-latest + env: + GO_VERSION: 1.17 + CGO_ENABLED: 0 + + steps: + + # https://github.com/marketplace/actions/setup-go-environment + - name: Set up Go ${{ env.GO_VERSION }} + uses: actions/setup-go@v2 + with: + go-version: ${{ env.GO_VERSION }} + + # https://github.com/marketplace/actions/checkout + - name: Check out code + uses: actions/checkout@v2 + with: + fetch-depth: 0 + + # https://github.com/marketplace/actions/cache + - name: Cache Go modules + uses: actions/cache@v2 + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + + - name: Check and get dependencies + run: | + go mod tidy + git diff --exit-code go.mod + git diff --exit-code go.sum + + - name: golangci-lint + uses: golangci/golangci-lint-action@v2 + with: + version: v1.46.0 diff --git a/.github/workflows/test-and-lint.yml b/.github/workflows/test-and-lint.yml deleted file mode 100644 index 121423c..0000000 --- a/.github/workflows/test-and-lint.yml +++ /dev/null @@ -1,44 +0,0 @@ -name: Go - -on: - push: - branches: [ main ] - -jobs: - test: - name: Test - runs-on: ubuntu-latest - steps: - - name: Set up Go 1.17 - uses: actions/setup-go@v2 - with: - go-version: 1.17 - id: go - - - name: Check out code into the Go module directory - uses: actions/checkout@v2 - - - name: Restore cache - uses: actions/cache@v1 - with: - path: ~/go/pkg/mod - key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} - restore-keys: | - ${{ runner.os }}-go- - - - name: Get dependencies - run: | - go mod download - - - name: Test - run: go test - - lint: - name: Lint - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - name: golangci-lint - uses: golangci/golangci-lint-action@v2 - with: - version: v1.46.0 diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..00e1abc --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +execinquery diff --git a/README.md b/README.md index 00ca3bb..05b67e5 100644 --- a/README.md +++ b/README.md @@ -38,10 +38,10 @@ func main() { ``` ```console -go vet -vettool=(which execinquery) ./... +go vet -vettool=$(which execinquery) ./... # command-line-arguments -./a.go:16:11: It's better to use Execute method instead of Query method to execute `UPDATE` query +./a.go:16:11: Use Exec instead of Query to execute `UPDATE` query ``` ## CI @@ -68,7 +68,7 @@ go vet -vettool=(which execinquery) ./... run: go vet -vettool=`which execinquery` ./... ``` -### License +### License MIT license. diff --git a/execinquery.go b/execinquery.go index 8119b89..6683618 100644 --- a/execinquery.go +++ b/execinquery.go @@ -2,8 +2,8 @@ package execinquery import ( "go/ast" + "regexp" "strings" - "unicode" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -16,13 +16,25 @@ const doc = "execinquery is a linter about query string checker in Query functio var Analyzer = &analysis.Analyzer{ Name: "execinquery", Doc: doc, - Run: run, + Run: newLinter().run, Requires: []*analysis.Analyzer{ inspect.Analyzer, }, } -func run(pass *analysis.Pass) (interface{}, error) { +type linter struct { + commentExp *regexp.Regexp + multilineCommentExp *regexp.Regexp +} + +func newLinter() *linter { + return &linter{ + commentExp: regexp.MustCompile(`--[^\n]*\n`), + multilineCommentExp: regexp.MustCompile(`(?s)/\*.*?\*/`), + } +} + +func (l linter) run(pass *analysis.Pass) (interface{}, error) { result := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) nodeFilter := []ast.Node{ @@ -32,10 +44,6 @@ func run(pass *analysis.Pass) (interface{}, error) { result.Preorder(nodeFilter, func(n ast.Node) { switch n := n.(type) { case *ast.CallExpr: - if len(n.Args) < 1 { - return - } - selector, ok := n.Fun.(*ast.SelectorExpr) if !ok { return @@ -49,53 +57,72 @@ func run(pass *analysis.Pass) (interface{}, error) { return } - var i int + replacement := "Exec" + var i int // the index of the query argument if strings.Contains(selector.Sel.Name, "Context") { + replacement = "ExecContext" i = 1 } - var s string - switch arg := n.Args[i].(type) { - case *ast.BasicLit: - s = strings.Replace(arg.Value, "\"", "", -1) - - case *ast.Ident: - - switch arg2 := arg.Obj.Decl.(type) { - case *ast.AssignStmt: - for _, stmt := range arg2.Rhs { - basicLit, ok := stmt.(*ast.BasicLit) - if !ok { - continue - } - - s = strings.Replace(basicLit.Value, "\"", "", -1) - } - case *ast.ValueSpec: - basicLit, ok := arg2.Values[0].(*ast.BasicLit) - if !ok { - return - } - - s = strings.TrimLeftFunc(basicLit.Value, func(r rune) bool { - return !unicode.IsLetter(r) && !unicode.IsNumber(r) - }) - s = strings.Replace(s, "\"", "", -1) - } - - default: + if len(n.Args) <= i { return } - if strings.HasPrefix(strings.ToLower(s), "select") { + query := l.getQueryString(n.Args[i]) + if query == "" { return } - s = strings.ToTitle(strings.SplitN(s, " ", 2)[0]) + query = strings.TrimSpace(l.cleanValue(query)) + parts := strings.SplitN(query, " ", 2) + cmd := strings.ToUpper(parts[0]) + + if strings.HasPrefix(cmd, "SELECT") { + return + } - pass.Reportf(n.Fun.Pos(), "It's better to use Execute method instead of %s method to execute `%s` query", selector.Sel.Name, s) + pass.Reportf(n.Fun.Pos(), "Use %s instead of %s to execute `%s` query", replacement, selector.Sel.Name, cmd) } }) return nil, nil } + +func (l linter) cleanValue(s string) string { + v := strings.NewReplacer(`"`, "", "`", "").Replace(s) + + v = l.multilineCommentExp.ReplaceAllString(v, "") + + return l.commentExp.ReplaceAllString(v, "") +} + +func (l linter) getQueryString(exp interface{}) string { + switch e := exp.(type) { + case *ast.AssignStmt: + var v string + for _, stmt := range e.Rhs { + v += l.cleanValue(l.getQueryString(stmt)) + } + return v + + case *ast.BasicLit: + return e.Value + + case *ast.ValueSpec: + var v string + for _, value := range e.Values { + v += l.cleanValue(l.getQueryString(value)) + } + return v + + case *ast.Ident: + return l.getQueryString(e.Obj.Decl) + + case *ast.BinaryExpr: + v := l.cleanValue(l.getQueryString(e.X)) + v += l.cleanValue(l.getQueryString(e.Y)) + return v + } + + return "" +} diff --git a/testdata/src/a/a.go b/testdata/src/a/a.go index bcb9a35..00c7a61 100644 --- a/testdata/src/a/a.go +++ b/testdata/src/a/a.go @@ -3,37 +3,59 @@ package a import ( "context" "database/sql" - "testing" ) -const cquery = ` - UPDATE * FROM test where test=? +const selectWithComment = `-- foobar +SELECT * FROM test WHERE test=? ` -func setup() *sql.DB { - db, _ := sql.Open("mysql", "test:test@tcp(test:3306)/test") - return db -} +const deleteWithComment = `-- foobar +-- foobar +-- foobar +DELETE * FROM test WHERE test=? +` -func f(t *testing.T) { - db := setup() - defer db.Close() +const deleteWithCommentMultiline = `/* foobar +-- foobar +-- foobar + */ +DELETE * FROM test WHERE test=? +` +func sample(db *sql.DB) { s := "alice" _ = db.QueryRowContext(context.Background(), "SELECT * FROM test WHERE test=?", s) - _ = db.QueryRowContext(context.Background(), "SELECT * FROM test WHERE test=?", s) + _ = db.QueryRowContext(context.Background(), selectWithComment, s) + _ = db.QueryRowContext(context.Background(), deleteWithComment, s) // want "Use ExecContext instead of QueryRowContext to execute `DELETE` query" + _ = db.QueryRowContext(context.Background(), deleteWithCommentMultiline, s) // want "Use ExecContext instead of QueryRowContext to execute `DELETE` query" - _ = db.QueryRowContext(context.Background(), "DELETE * FROM test WHERE test=?", s) // want "It\\'s better to use Execute method instead of QueryRowContext method to execute `DELETE` query" - _ = db.QueryRowContext(context.Background(), "UPDATE * FROM test WHERE test=?", s) // want "It\\'s better to use Execute method instead of QueryRowContext method to execute `UPDATE` query" + _ = db.QueryRowContext(context.Background(), "DELETE * FROM test WHERE test=?", s) // want "Use ExecContext instead of QueryRowContext to execute `DELETE` query" + _ = db.QueryRowContext(context.Background(), "UPDATE * FROM test WHERE test=?", s) // want "Use ExecContext instead of QueryRowContext to execute `UPDATE` query" - _, _ = db.Query("UPDATE * FROM test WHERE test=?", s) // want "It\\'s better to use Execute method instead of Query method to execute `UPDATE` query" - _, _ = db.QueryContext(context.Background(), "UPDATE * FROM test WHERE test=?", s) // want "It\\'s better to use Execute method instead of QueryContext method to execute `UPDATE` query" - _ = db.QueryRow("UPDATE * FROM test WHERE test=?", s) // want "It\\'s better to use Execute method instead of QueryRow method to execute `UPDATE` query" + _, _ = db.Query("UPDATE * FROM test WHERE test=?", s) // want "Use Exec instead of Query to execute `UPDATE` query" + _, _ = db.QueryContext(context.Background(), "UPDATE * FROM test WHERE test=?", s) // want "Use ExecContext instead of QueryContext to execute `UPDATE` query" + _ = db.QueryRow("UPDATE * FROM test WHERE test=?", s) // want "Use Exec instead of QueryRow to execute `UPDATE` query" query := "UPDATE * FROM test where test=?" - var query1 string = "UPDATE * FROM test where test=?" - _, _ = db.Query(query, s) // want "It\\'s better to use Execute method instead of Query method to execute `UPDATE` query" - _, _ = db.Query(query1, s) // want "It\\'s better to use Execute method instead of Query method to execute `UPDATE` query" - _, _ = db.Query(cquery, s) // want "It\\'s better to use Execute method instead of Query method to execute `UPDATE` query" + _, _ = db.Query(query, s) // want "Use Exec instead of Query to execute `UPDATE` query" + + f1 := ` +UPDATE * FROM test WHERE test=?` + _ = db.QueryRow(f1, s) // want "Use Exec instead of QueryRow to execute `UPDATE` query" + + const f2 = ` +UPDATE * FROM test WHERE test=?` + _ = db.QueryRow(f2, s) // want "Use Exec instead of QueryRow to execute `UPDATE` query" + + f3 := ` +UPDATE * FROM test WHERE test=?` + _ = db.QueryRow(f3, s) // want "Use Exec instead of QueryRow to execute `UPDATE` query" + + f4 := f3 + _ = db.QueryRow(f4, s) // want "Use Exec instead of QueryRow to execute `UPDATE` query" + + f5 := ` +UPDATE * ` + `FROM test` + ` WHERE test=?` + _ = db.QueryRow(f5, s) // want "Use Exec instead of QueryRow to execute `UPDATE` query" } diff --git a/testdata/src/a/go.mod b/testdata/src/a/go.mod index 1204cb9..31d009e 100644 --- a/testdata/src/a/go.mod +++ b/testdata/src/a/go.mod @@ -1,4 +1,3 @@ module a go 1.17 -