diff --git a/README.md b/README.md index 389b3c1..b8dbe16 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ See [documentation](https://clickhouse.com/docs/integrations/go#the-clickhouse-g The linter detects 2 common implementation mistakes: - forgetting to call `rows.Err()` after calling `rows.Next()` -- forgetting to call `defer batch.Close()` when using `Batch` +- forgetting to call `defer xxx.Close()` when using `Batch` or `Rows` These implementation mistakes are often observed in codebases using the clickhouse-go driver. They result in incomplete data and hard-to-troubleshoot bugs. @@ -55,6 +55,7 @@ Incorrect code: ```go parsedRows := []... rows, _ := conn.Query(ctx, "SELECT ...") +defer rows.Close() for rows.Next() { parsedRow, err = .... if err { @@ -74,6 +75,7 @@ return parsedRows, nil ```go parsedRows := []... rows, _ := conn.Query(ctx, "SELECT ...") +defer rows.Close() for rows.Next() { parsed, err = .... if err { @@ -130,8 +132,10 @@ There are some limitations: --> in this case the linter will not be able to detect that `.Err()` may be called too late. -## 2. chbatchclose -Detect when a `github.com/ClickHouse/clickhouse-go/v2/lib/driver` `Batch` variable is used but no associated `defer batch.Close()` is called. +## 2. chclose +Detect when a `github.com/ClickHouse/clickhouse-go/v2/lib/driver`: `Batch` / `Rows` variable is used but no associated `defer xxx.Close()` is called. + +*All examples below are given for `Batch` but apply the same way for `Rows`.* Calling `defer batch.Close()` is highly recommended (see [here](https://clickhouse.com/docs/integrations/go#batch-insert)). While it is possible to not use this statement, code that do not use it often end up with subtle leaks, as the @@ -180,15 +184,21 @@ return batch.Send() ### Rule details +*All examples below are given for `Batch` but apply the same way for `Rows`.* + The linter goes through every function. If a clickhouse driver `Batch` is instantiated and is not part of the values returned by the function, a `defer batch.Close()` must be found. Also, assigning a `Batch` to the blank identifier `_` is flagged. -Variable re-assignments and intertwined variable are supported. See [testcases.go](passes/chbatchclose/testdata/src/testcases/testcases.go). +Variable re-assignments and intertwined variable are supported. See [testcases.go](passes/chclose/testdata/src/testcases/testcases.go). There are some limitations: -- except for looking into defer blocks;, the linter does not cross function block boundaries. If a `Batch` variable is instantiated and `batch.Close()` is called in a - closure inside the defer call, the linter will not be able to associate the `batch.Close()` to the variable. - (note: in most cases such pattern is a bad idea). See `deferCloseIsInClosure` test case. +- linting of structs wrapping or embedding a `Batch` is not supported +- tracking across function calls is not supported: + - if a `Batch` variable is instantiated and returned by a function call `return logAndReturn(batch)`, + the linter cannot know the batch is part of the values returned. + - If a `Batch` variable is instantiated and `batch.Close()` is called in a + closure inside the defer call, the linter will not be able to associate the `batch.Close()` to the variable. + (note: in most cases such pattern is a bad idea). See `deferCloseIsInClosure` test case. - `defer batch.Close()` must be called after checking that the `PrepareBatch` call returned no error. incorrect: ``` diff --git a/main.go b/main.go index 9eb2036..a43a3ae 100644 --- a/main.go +++ b/main.go @@ -1,11 +1,11 @@ package main import ( - "github.com/ClickHouse/clickhouse-go-linter/passes/chbatchclose" + "github.com/ClickHouse/clickhouse-go-linter/passes/chclose" "github.com/ClickHouse/clickhouse-go-linter/passes/chrowserr" "golang.org/x/tools/go/analysis/multichecker" ) func main() { - multichecker.Main(chrowserr.NewAnalyzer(), chbatchclose.NewAnalyzer()) + multichecker.Main(chrowserr.NewAnalyzer(), chclose.NewAnalyzer()) } diff --git a/passes/chbatchclose/chbatchclose.go b/passes/chbatchclose/chbatchclose.go deleted file mode 100644 index 3da2c55..0000000 --- a/passes/chbatchclose/chbatchclose.go +++ /dev/null @@ -1,182 +0,0 @@ -package chbatchclose - -import ( - "go/ast" - "go/token" - "os" - "strconv" - - "github.com/ClickHouse/clickhouse-go-linter/internal/util" - - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" - "golang.org/x/tools/go/ast/inspector" -) - -type analyzer struct { - // if true, report valid usages and log spurious but valid cases. - debug bool -} - -func NewAnalyzer() *analysis.Analyzer { - debug, _ := strconv.ParseBool(os.Getenv("CH_GO_LINTER_DEBUG")) - a := analyzer{ - debug: debug, - } - return &analysis.Analyzer{ - Name: "chbatchclosecheck", - Doc: "chbatchclosecheck checks whether defer batch.Close() is called on ClickHouse driver Batch variables", - Run: a.run, - Requires: []*analysis.Analyzer{inspect.Analyzer}, - } -} - -func (a *analyzer) run(pass *analysis.Pass) (any, error) { - insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) - - nodeFilter := []ast.Node{ - (*ast.FuncDecl)(nil), - (*ast.FuncLit)(nil), - } - - insp.Preorder(nodeFilter, func(n ast.Node) { - var body *ast.BlockStmt - switch fn := n.(type) { - case *ast.FuncDecl: - body = fn.Body - case *ast.FuncLit: - body = fn.Body - } - - if body == nil { - return - } - a.checkFunc(pass, body) - }) - - return nil, nil -} - -// batchUsage tracks whether a driver.Batch variable has a defer Close/Abort or is returned. -type batchUsage struct { - assignPos token.Pos - deferredClose bool - returned bool -} - -func (b *batchUsage) report(varName string, pass *analysis.Pass, debug bool) { - if b.assignPos == token.NoPos { - // no usage of Batch - return - } - if !b.deferredClose && !b.returned { - pass.Reportf(b.assignPos, - "clickhouse Batch %s must be closed defensively with defer %s.Close() after successful instantiation", - varName, varName) - } else if debug { - if b.deferredClose { - pass.Reportf(b.assignPos, - "clickhouse Batch %s is properly closed defensively after successful instantiation [valid]", - varName) - } else { - pass.Reportf(b.assignPos, - "clickhouse Batch %s is returned by the function [valid]", - varName) - } - } -} - -// checkFunc analyzes a single function/closure body. -// It does a single-pass collection of Batch assignments, defer Close/Abort calls, and return statements. -// It does not descend into nested closures (they are handled as separate units by the Preorder visitor above). -func (a *analyzer) checkFunc(pass *analysis.Pass, body *ast.BlockStmt) { - usages := map[string]*batchUsage{} - - ast.Inspect(body, func(n ast.Node) bool { - if n == nil { - return false - } - // don't descend into nested closures - if n != body { - if _, ok := n.(*ast.FuncLit); ok { - return false - } - } - - switch node := n.(type) { - case *ast.AssignStmt: - a.handleAssign(pass, node, usages) - case *ast.DeferStmt: - handleDefer(node, usages) - case *ast.ReturnStmt: - handleReturn(node, usages) - } - - return true - }) - - // remaining usages that were not flushed - for varName, u := range usages { - u.report(varName, pass, a.debug) - } -} - -// handleAssign checks if any LHS variable in the assignment is of type driver.Batch. -// If a tracked variable is reassigned, it flushes/reports the previous tracking first. -func (a *analyzer) handleAssign(pass *analysis.Pass, assign *ast.AssignStmt, usages map[string]*batchUsage) { - for _, lhs := range assign.Lhs { - name := util.IdentName(lhs) - if name == "" { - continue - } - if name == "_" && util.IsChObj(pass, lhs, "Batch") { - pass.Reportf(assign.Pos(), "clickhouse Batch assigned to blank identifier. Connection leak. clickhouse Batch must be instantiated and closed defensively with defer batch.Close() after successful instantiation") - continue - } - - // if this var was already tracked, flush previous usage before re-tracking - if u, ok := usages[name]; ok { - u.report(name, pass, a.debug) - delete(usages, name) - } - - if util.IsChObj(pass, lhs, "Batch") { - usages[name] = &batchUsage{assignPos: assign.Pos()} - } - } -} - -// handleDefer checks if a defer statement calls Close() or Abort() on a tracked Batch variable. -func handleDefer(deferStmt *ast.DeferStmt, usages map[string]*batchUsage) { - call := deferStmt.Call - sel, ok := call.Fun.(*ast.SelectorExpr) - if !ok { - return - } - varName := util.IdentName(sel.X) - if varName == "" { - return - } - u, exists := usages[varName] - if !exists { - return - } - - switch sel.Sel.Name { - case "Close": - u.deferredClose = true - } -} - -// handleReturn checks if any return value is a tracked Batch variable. -func handleReturn(ret *ast.ReturnStmt, usages map[string]*batchUsage) { - for _, result := range ret.Results { - name := util.IdentName(result) - if name == "" { - continue - } - if u, exists := usages[name]; exists { - u.returned = true - } - } -} diff --git a/passes/chbatchclose/chbatchclose_test.go b/passes/chbatchclose/chbatchclose_test.go deleted file mode 100644 index 2d92ac2..0000000 --- a/passes/chbatchclose/chbatchclose_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package chbatchclose_test - -import ( - "testing" - - "golang.org/x/tools/go/analysis/analysistest" - - "github.com/ClickHouse/clickhouse-go-linter/passes/chbatchclose" -) - -func TestAnalyzer(t *testing.T) { - analysistest.Run(t, analysistest.TestData(), chbatchclose.NewAnalyzer(), "testcases") -} diff --git a/passes/chclose/chclose.go b/passes/chclose/chclose.go new file mode 100644 index 0000000..b2c2f70 --- /dev/null +++ b/passes/chclose/chclose.go @@ -0,0 +1,281 @@ +package chclose + +import ( + "go/ast" + "go/token" + "os" + "strconv" + + "github.com/ClickHouse/clickhouse-go-linter/internal/util" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +type analyzer struct { + // if true, report valid usages and log spurious but valid cases. + debug bool +} + +func NewAnalyzer() *analysis.Analyzer { + debug, _ := strconv.ParseBool(os.Getenv("CH_GO_LINTER_DEBUG")) + a := analyzer{ + debug: debug, + } + return &analysis.Analyzer{ + Name: "chclosecheck", + Doc: "chclosecheck checks whether defer xxx.Close() is called on ClickHouse driver Batch/Rows variables", + Run: a.run, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + } +} + +func (a *analyzer) run(pass *analysis.Pass) (any, error) { + insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.FuncDecl)(nil), + (*ast.FuncLit)(nil), + } + + insp.Preorder(nodeFilter, func(n ast.Node) { + var body *ast.BlockStmt + switch fn := n.(type) { + case *ast.FuncDecl: + body = fn.Body + case *ast.FuncLit: + body = fn.Body + } + + if body == nil { + return + } + a.checkFunc(pass, body) + }) + + return nil, nil +} + +// closableTypes lists the ClickHouse driver types that must be closed defensively. +var closableTypes = []string{"Batch", "Rows"} + +// closableUsage tracks whether a driver.Batch / driver.Rows variable has a defer Close() or is returned. +type closableUsage struct { + typeName string // "Batch" or "Rows" + assignPos token.Pos + deferredClose bool + returned bool +} + +func (u *closableUsage) report(varName string, pass *analysis.Pass, debug bool) { + if u.assignPos == token.NoPos { + // no usage of Batch + return + } + if !u.deferredClose && !u.returned { + pass.Reportf(u.assignPos, + "clickhouse %s %s must be closed defensively with defer %s.Close() after successful instantiation", + u.typeName, varName, varName) + } else if debug { + if u.deferredClose { + pass.Reportf(u.assignPos, + "clickhouse %s %s is properly closed defensively after successful instantiation [valid]", + u.typeName, varName) + } else { + pass.Reportf(u.assignPos, + "clickhouse %s %s is returned by the function [valid]", + u.typeName, varName) + } + } +} + +// checkFunc analyzes a single function/closure body. +// It does a single-pass collection of Batch assignments, defer Close/Abort calls, and return statements. +// It does not descend into nested closures (they are handled as separate units by the Preorder visitor above). +func (a *analyzer) checkFunc(pass *analysis.Pass, body *ast.BlockStmt) { + usages := map[string]*closableUsage{} + + ast.Inspect(body, func(n ast.Node) bool { + if n == nil { + return false + } + // don't descend into nested closures + if n != body { + if _, ok := n.(*ast.FuncLit); ok { + return false + } + } + + switch node := n.(type) { + case *ast.AssignStmt: + a.handleAssign(pass, node, usages) + case *ast.DeferStmt: + handleDefer(node, usages) + case *ast.ReturnStmt: + handleReturn(node, usages) + } + + return true + }) + + // for violations, check if the tracked var was stored in a struct literal whose result was returned + for varName, u := range usages { + if !u.deferredClose && !u.returned && isStoredInStructAndReturned(body, varName) { + u.returned = true + } + u.report(varName, pass, a.debug) + } +} + +// isStoredInStructAndReturned checks if varName appears as a value in a composite literal +// assigned to some variable, and that variable is later returned. +// This handles: l := &Wrapper{rows}; return l, nil +func isStoredInStructAndReturned(body *ast.BlockStmt, varName string) bool { + // step 1: find assignments where varName is inside a struct literal on the RHS + derivedVars := map[string]bool{} + ast.Inspect(body, func(n ast.Node) bool { + assign, ok := n.(*ast.AssignStmt) + if !ok { + return true + } + for i, rhs := range assign.Rhs { + if !compositeLiteralContains(rhs, varName) { + continue + } + if i < len(assign.Lhs) { + if lhsName := util.IdentName(assign.Lhs[i]); lhsName != "" { + derivedVars[lhsName] = true + } + } + } + return true + }) + if len(derivedVars) == 0 { + return false + } + + // step 2: check if any derived variable is returned + found := false + ast.Inspect(body, func(n ast.Node) bool { + if found { + return false + } + ret, ok := n.(*ast.ReturnStmt) + if !ok { + return true + } + for _, result := range ret.Results { + if name := util.IdentName(result); name != "" && derivedVars[name] { + found = true + return false + } + } + return true + }) + return found +} + +// compositeLiteralContains checks if an expression is (or contains via unary &) a composite literal +// that references varName as a value. +func compositeLiteralContains(expr ast.Expr, varName string) bool { + // unwrap &Wrapper{...} + if unary, ok := expr.(*ast.UnaryExpr); ok { + expr = unary.X + } + lit, ok := expr.(*ast.CompositeLit) + if !ok { + return false + } + for _, elt := range lit.Elts { + switch e := elt.(type) { + case *ast.Ident: + if e.Name == varName { + return true + } + case *ast.KeyValueExpr: + if id, ok := e.Value.(*ast.Ident); ok && id.Name == varName { + return true + } + } + } + return false +} + +// handleAssign checks if any LHS variable in the assignment is a closable ClickHouse driver type. +// If a tracked variable is reassigned, it flushes/reports the previous tracking first. +func (a *analyzer) handleAssign(pass *analysis.Pass, assign *ast.AssignStmt, usages map[string]*closableUsage) { + for _, lhs := range assign.Lhs { + name := util.IdentName(lhs) + if name == "" { + continue + } + + for _, typeName := range closableTypes { + if !util.IsChObj(pass, lhs, typeName) { + continue + } + + if name == "_" { + pass.Reportf(assign.Pos(), "clickhouse %s assigned to blank identifier. Connection leak. clickhouse %s must be instantiated and closed defensively with defer %s.Close() after successful instantiation", + typeName, typeName, typeName) + break + } + + // if this var was already tracked, flush previous usage before re-tracking + if u, ok := usages[name]; ok { + u.report(name, pass, a.debug) + delete(usages, name) + } + + usages[name] = &closableUsage{typeName: typeName, assignPos: assign.Pos()} + break + } + } +} + +// handleDefer checks if a defer statement calls Close() or Abort() on a tracked Batch variable. +func handleDefer(deferStmt *ast.DeferStmt, usages map[string]*closableUsage) { + call := deferStmt.Call + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return + } + varName := util.IdentName(sel.X) + if varName == "" { + return + } + u, exists := usages[varName] + if !exists { + return + } + + switch sel.Sel.Name { + case "Close": + u.deferredClose = true + } +} + +// handleReturn checks if any return expression contains a tracked variable. +// It recognizes direct returns (return rows, nil) and struct literal wrapping (return &Wrapper{rows}, nil). +// Function calls in return expressions are NOT walked into (we cannot distinguish +// wrapping (ownership transfer) from consuming (just reads)). +func handleReturn(ret *ast.ReturnStmt, usages map[string]*closableUsage) { + for _, result := range ret.Results { + ast.Inspect(result, func(n ast.Node) bool { + switch n.(type) { + case *ast.CallExpr: + // don't descend into function calls — opaque ownership semantics + return false + } + id, ok := n.(*ast.Ident) + if !ok { + return true + } + if u, exists := usages[id.Name]; exists { + u.returned = true + } + return true + }) + } +} diff --git a/passes/chclose/chclose_test.go b/passes/chclose/chclose_test.go new file mode 100644 index 0000000..287a749 --- /dev/null +++ b/passes/chclose/chclose_test.go @@ -0,0 +1,13 @@ +package chclose_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/ClickHouse/clickhouse-go-linter/passes/chclose" +) + +func TestAnalyzer(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), chclose.NewAnalyzer(), "testcases") +} diff --git a/passes/chbatchclose/testdata/src/github.com/ClickHouse/clickhouse-go/v2/lib/driver/driver.go b/passes/chclose/testdata/src/github.com/ClickHouse/clickhouse-go/v2/lib/driver/driver.go similarity index 50% rename from passes/chbatchclose/testdata/src/github.com/ClickHouse/clickhouse-go/v2/lib/driver/driver.go rename to passes/chclose/testdata/src/github.com/ClickHouse/clickhouse-go/v2/lib/driver/driver.go index 6e96b25..3e0e7ad 100644 --- a/passes/chbatchclose/testdata/src/github.com/ClickHouse/clickhouse-go/v2/lib/driver/driver.go +++ b/passes/chclose/testdata/src/github.com/ClickHouse/clickhouse-go/v2/lib/driver/driver.go @@ -10,6 +10,19 @@ type Batch interface { Rows() int } +type Rows interface { + Next() bool + Scan(dest ...any) error + ScanStruct(dest any) error + // ColumnTypes() []ColumnType + Totals(dest ...any) error + Columns() []string + Close() error + Err() error + HasData() bool +} + type Conn interface { PrepareBatch(ctx any, query string, opts ...any) (Batch, error) + Query(ctx any, query string, args ...any) (Rows, error) } diff --git a/passes/chbatchclose/testdata/src/testcases/testcases.go b/passes/chclose/testdata/src/testcases/testcases.go similarity index 53% rename from passes/chbatchclose/testdata/src/testcases/testcases.go rename to passes/chclose/testdata/src/testcases/testcases.go index 7c0b951..8f16ccb 100644 --- a/passes/chbatchclose/testdata/src/testcases/testcases.go +++ b/passes/chclose/testdata/src/testcases/testcases.go @@ -9,6 +9,8 @@ import ( var conn driver.Conn var ctx = context.Background() +// ===== Batch tests ===== + // valid: defer Close() after PrepareBatch func validDeferClose() { batch, err := conn.PrepareBatch(ctx, "INSERT INTO t") @@ -47,6 +49,33 @@ func helperReturningBatch() (driver.Batch, error) { return conn.PrepareBatch(ctx, "INSERT INTO t") } +type batchWrapper struct { + driver.Batch +} + +func wrapBatch(b driver.Batch) *batchWrapper { + return &batchWrapper{b} +} + +// known false positive: batch returned wrapped in a function call. +// we cannot statically distinguish wrapping (ownership transfer) from consuming. +func falsePositiveReturnWrappedInCall() (*batchWrapper, error) { + batch, err := conn.PrepareBatch(ctx, "INSERT INTO t") //want `clickhouse Batch batch must be closed defensively with defer batch\.Close\(\) after successful instantiation` + if err != nil { + return nil, err + } + return wrapBatch(batch), nil +} + +// valid: batch returned wrapped in a struct literal +func validReturnWrappedInStruct() (*batchWrapper, error) { + batch, err := conn.PrepareBatch(ctx, "INSERT INTO t") + if err != nil { + return nil, err + } + return &batchWrapper{batch}, nil +} + // valid: defer Close() after Batch is instantiated from helper method func validFromHelper() { batch, err := helperReturningBatch() @@ -61,7 +90,7 @@ func validFromHelper() { // invalid assignment to blank identifier. // unlikely to exist in real code base but results in a connection leak. func toBlankIdentifier() { - _, err := conn.PrepareBatch(ctx, "INSERT INTO t") // want `clickhouse Batch assigned to blank identifier. Connection leak. clickhouse Batch must be instantiated and closed defensively with defer batch\.Close\(\) after successful instantiation` + _, err := conn.PrepareBatch(ctx, "INSERT INTO t") // want `clickhouse Batch assigned to blank identifier. Connection leak. clickhouse Batch must be instantiated and closed defensively with defer Batch\.Close\(\) after successful instantiation` if err != nil { return } @@ -209,3 +238,164 @@ func invalidNotDefensive() error { } return nil } + +// ===== Rows tests ===== + +// valid: defer Close() after Query +func rowsValidDeferClose() { + rows, err := conn.Query(ctx, "SELECT 1") + if err != nil { + return + } + defer rows.Close() + for rows.Next() { + var v int + _ = rows.Scan(&v) + } +} + +// valid: rows returned to caller +func rowsValidReturn() (driver.Rows, error) { + rows, err := conn.Query(ctx, "SELECT 1") + if err != nil { + return nil, err + } + return rows, nil +} + +// valid: rows returned to caller "without instantiation" +func helperReturningRows() (driver.Rows, error) { + return conn.Query(ctx, "SELECT 1") +} + +type rowsWrapper struct { + driver.Rows +} + +func wrapRows(r driver.Rows) *rowsWrapper { + return &rowsWrapper{r} +} + +// known false positive: rows returned wrapped in a function call. +// we cannot statically distinguish wrapping (ownership transfer) from consuming. +func rowsFalsePositiveReturnWrappedInCall() (*rowsWrapper, error) { + rows, err := conn.Query(ctx, "SELECT 1") //want `clickhouse Rows rows must be closed defensively with defer rows\.Close\(\) after successful instantiation` + if err != nil { + return nil, err + } + return wrapRows(rows), nil +} + +// valid: rows returned wrapped in a struct literal +func rowsValidReturnWrappedInStruct() (*rowsWrapper, error) { + rows, err := conn.Query(ctx, "SELECT 1") + if err != nil { + return nil, err + } + return &rowsWrapper{rows}, nil +} + +// known false positive: rows wrapped via function call into intermediate variable then returned. +// we cannot statically distinguish wrapping (ownership transfer) from consuming (just reads rows). +func rowsFalsePositiveWrappedViaFuncCall() (*rowsWrapper, error) { + rows, err := conn.Query(ctx, "SELECT 1") //want `clickhouse Rows rows must be closed defensively with defer rows\.Close\(\) after successful instantiation` + if err != nil { + return nil, err + } + l := wrapRows(rows) + return l, nil +} + +// valid: rows stored in struct literal via intermediate variable then returned. +// struct literals are unambiguous — rows is a field in the returned struct, so ownership is transferred. +func rowsValidWrappedViaStructLiteral() (*rowsWrapper, error) { + rows, err := conn.Query(ctx, "SELECT 1") + if err != nil { + return nil, err + } + l := &rowsWrapper{rows} + return l, nil +} + +// valid: defer Close() after Rows instantiated from helper method +func rowsValidFromHelper() { + rows, err := helperReturningRows() + if err != nil { + return + } + defer rows.Close() + for rows.Next() { + var v int + _ = rows.Scan(&v) + } +} + +// invalid: no defer, no return +func rowsInvalidNoDeferNoReturn() { + rows, err := conn.Query(ctx, "SELECT 1") //want `clickhouse Rows rows must be closed defensively with defer rows\.Close\(\) after successful instantiation` + if err != nil { + return + } + for rows.Next() { + var v int + _ = rows.Scan(&v) + } +} + +// invalid assignment to blank identifier +func rowsToBlankIdentifier() { + _, err := conn.Query(ctx, "SELECT 1") // want `clickhouse Rows assigned to blank identifier. Connection leak. clickhouse Rows must be instantiated and closed defensively with defer Rows\.Close\(\) after successful instantiation` + if err != nil { + return + } +} + +// invalid: rows from helper function, no defer, no return +func rowsInvalidFromHelper() { + rows, err := helperReturningRows() //want `clickhouse Rows rows must be closed defensively with defer rows\.Close\(\) after successful instantiation` + if err != nil { + return + } + for rows.Next() { + var v int + _ = rows.Scan(&v) + } +} + +// reassignment: first has no defer (invalid), second has defer (valid) +func rowsReassignInvalidValid() { + rows, err := conn.Query(ctx, "SELECT 1") //want `clickhouse Rows rows must be closed defensively with defer rows\.Close\(\) after successful instantiation` + if err != nil { + return + } + _ = rows.Err() + + rows, err = conn.Query(ctx, "SELECT 2") + if err != nil { + return + } + defer rows.Close() +} + +// reassignment: first has defer (valid), second has no defer (invalid) +func rowsReassignValidInvalid() { + rows, err := conn.Query(ctx, "SELECT 1") + if err != nil { + return + } + defer rows.Close() + + rows, err = conn.Query(ctx, "SELECT 2") //want `clickhouse Rows rows must be closed defensively with defer rows\.Close\(\) after successful instantiation` + if err != nil { + return + } +} + +// closures are not supported +func rowsDeferCloseIsInClosure() { + rows, err := conn.Query(ctx, "SELECT 1") //want `clickhouse Rows rows must be closed defensively with defer rows\.Close\(\) after successful instantiation` + if err != nil { + return + } + defer func() { rows.Close() }() +}