[WIP] Error check

internal/engine.go

@@ -52,6 +52,7 @@ var allRuleConstructors = ruleMap{
 	"gno-mod-tidy":                NewMissingModPackageRule,
 	"slice-bounds-check":          NewSliceBoundCheckRule,
 	"const-error-declaration":     NewConstErrorDeclarationRule,
+	"error-check":                 NewErrCheckRule,
 }
 
 func (e *Engine) applyRules(rules map[string]tt.ConfigRule) {

internal/lints/err_check.go

@@ -0,0 +1,186 @@
+package lints
+
+import (
+	"go/ast"
+	"go/token"
+	"strings"
+
+	tt "github.com/gnolang/tlin/internal/types"
+)
+
+func DetectErrCheck(filename string, node *ast.File, fset *token.FileSet, severity tt.Severity) ([]tt.Issue, error) {
+	v := &errCheckVisitor{
+		filename:    filename,
+		fset:        fset,
+		severity:    severity,
+		imports:     make(map[string]bool),
+		errorVars:   make(map[string]bool),
+		errorChecks: make(map[string]bool),
+	}
+
+	// 1st pass: collect error variable declarations and checks
+	ast.Inspect(node, func(n ast.Node) bool {
+		switch x := n.(type) {
+		case *ast.AssignStmt:
+			v.collectErrorVars(x)
+		case *ast.IfStmt:
+			v.collectErrorChecks(x)
+		}
+		return true
+	})
+
+	// 2nd pass: check for unchecked errors
+	ast.Walk(v, node)
+	return v.issues, nil
+}
+
+type errCheckVisitor struct {
+	filename    string
+	fset        *token.FileSet
+	severity    tt.Severity
+	issues      []tt.Issue
+	imports     map[string]bool
+	errorVars   map[string]bool // tracks declared error variables
+	errorChecks map[string]bool // tracks which error vars have been checked
+	currentFunc *ast.FuncDecl   // tracks current function being analyzed
+}
+
+func (v *errCheckVisitor) collectErrorVars(assign *ast.AssignStmt) {
+	for _, lhs := range assign.Lhs {
+		if ident, ok := lhs.(*ast.Ident); ok {
+			if ident.Name == "err" || strings.HasSuffix(ident.Name, "Error") {
+				v.errorVars[ident.Name] = true
+			}
+		}
+	}
+}
+
+func (v *errCheckVisitor) collectErrorChecks(ifStmt *ast.IfStmt) {
+	// check `err != nil` pattern
+	if binExpr, ok := ifStmt.Cond.(*ast.BinaryExpr); ok {
+		if ident, ok := binExpr.X.(*ast.Ident); ok {
+			if v.errorVars[ident.Name] {
+				v.errorChecks[ident.Name] = true
+			}
+		}
+	}
+}
+
+func (v *errCheckVisitor) Visit(node ast.Node) ast.Visitor {
+	if node == nil {
+		return v
+	}
+
+	switch n := node.(type) {
+	case *ast.FuncDecl:
+		v.currentFunc = n
+		defer func() { v.currentFunc = nil }()
+	case *ast.AssignStmt:
+		v.checkAssignment(n)
+	case *ast.ExprStmt:
+		v.checkExprStmt(n)
+	}
+
+	return v
+}
+
+func (v *errCheckVisitor) checkAssignment(assign *ast.AssignStmt) {
+	// multi-return cases
+	if len(assign.Rhs) == 1 && len(assign.Lhs) > 1 {
+		if call, ok := assign.Rhs[0].(*ast.CallExpr); ok {
+			v.checkMultiValueAssign(assign.Lhs, call)
+		}
+		return
+	}
+
+	// single assignments
+	for i, rhs := range assign.Rhs {
+		if i >= len(assign.Lhs) {
+			continue
+		}
+		if call, ok := rhs.(*ast.CallExpr); ok && v.isLikelyErrorReturn(call) {
+			lhs := assign.Lhs[i]
+			if isBlankIdentifier(lhs) {
+				v.addIssue(
+					assign.Pos(), assign.End(),
+					"error return value is ignored with blank identifier",
+					"consider handling the error value",
+				)
+			}
+		}
+	}
+}
+
+func (v *errCheckVisitor) checkMultiValueAssign(lhs []ast.Expr, call *ast.CallExpr) {
+	if !v.isLikelyErrorReturn(call) {
+		return
+	}
+
+	// check if the last return value (typically error) is properly handled
+	if len(lhs) >= 2 {
+		lastExpr := lhs[len(lhs)-1]
+		if isBlankIdentifier(lastExpr) {
+			v.addIssue(
+				call.Pos(), call.End(),
+				"error return value is ignored with blank identifier",
+				"consider handling the error value",
+			)
+		}
+	}
+}
+
+func (v *errCheckVisitor) checkExprStmt(expr *ast.ExprStmt) {
+	if call, ok := expr.X.(*ast.CallExpr); ok && v.isLikelyErrorReturn(call) {
+		// ignore certain method calls that are commonly used without error checking
+		if sel, ok := call.Fun.(*ast.SelectorExpr); ok {
+			if sel.Sel.Name == "Close" || sel.Sel.Name == "Print" || sel.Sel.Name == "Println" {
+				return
+			}
+		}
+
+		v.addIssue(
+			expr.Pos(), expr.End(),
+			"error-returning function call's result is ignored",
+			"add error handling or explicitly assign the error",
+		)
+	}
+}
+
+func (v *errCheckVisitor) isLikelyErrorReturn(call *ast.CallExpr) bool {
+	switch fun := call.Fun.(type) {
+	case *ast.Ident:
+		// common functions that don't return errors
+		switch fun.Name {
+		case "print", "println", "len", "cap", "append", "recover", "panic":
+			return false
+		}
+	case *ast.SelectorExpr:
+		// common error-returning methods
+		switch fun.Sel.Name {
+		case "Error", "String", "GoString":
+			return false
+		case "Close", "Write", "Read", "Scan", "Parse", "Open", "Create",
+			"Delete", "Update", "Exec", "Query":
+			return true
+		}
+	}
+	return true
+}
+
+func isBlankIdentifier(expr ast.Expr) bool {
+	ident, ok := expr.(*ast.Ident)
+	return ok && ident.Name == "_"
+}
+
+func (v *errCheckVisitor) addIssue(start, end token.Pos, msg, note string) {
+	issue := tt.Issue{
+		Rule:     "error-check",
+		Filename: v.filename,
+		Start:    v.fset.Position(start),
+		End:      v.fset.Position(end),
+		Message:  msg,
+		Note:     note,
+		Severity: v.severity,
+	}
+	v.issues = append(v.issues, issue)
+}

internal/lints/err_check_test.go

@@ -0,0 +1,154 @@
+package lints
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/gnolang/tlin/internal/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDetectErrCheck(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		code     string
+		expected int
+		messages []string
+	}{
+		{
+			name: "Simple error ignored",
+			code: `
+package main
+import "os"
+func main() {
+    f, _ := os.Open("file.txt")
+    defer f.Close()
+}`,
+			expected: 1,
+			messages: []string{
+				"error return value is ignored with blank identifier",
+			},
+		},
+		{
+			name: "Simple error ignored 2",
+			code: `
+package main
+
+import "errors"
+
+func foo() (int, error) {
+	return 0, errors.New("test")
+}
+
+func main() {
+    v, _ := foo()
+	println(v)
+}`,
+			expected: 1,
+			messages: []string{
+				"error return value is ignored with blank identifier",
+			},
+		},
+		{
+			name: "Error not checked after assignment",
+			code: `
+package main
+import "os"
+func main() {
+    var err error
+    f, err := os.Open("file.txt")
+    f.Write([]byte("test"))
+    defer f.Close()
+}`,
+			expected: 1,
+			messages: []string{
+				"error-returning function call's result is ignored",
+			},
+		},
+		{
+			name: "Proper error handling",
+			code: `
+package main
+
+import "errors"
+
+func foo() (int, error) {
+	return 0, errors.New("test")
+}
+
+func main() {
+    v, err := foo()
+    if err != nil {
+		panic(err)
+	}
+	println(v)
+}`,
+			expected: 0,
+			messages: []string{},
+		},
+		{
+			name: "Multiple error ignoring patterns",
+			code: `
+package main
+import (
+    "os"
+    "encoding/json"
+)
+func main() {
+    f, _ := os.Open("file.txt")
+    json.Unmarshal([]byte("{}"), &struct{}{})
+    f.Write([]byte("test"))
+}`,
+			expected: 3,
+			messages: []string{
+				"error return value is ignored with blank identifier",
+				"error-returning function call's result is ignored",
+				"error-returning function call's result is ignored",
+			},
+		},
+		{
+			name: "Non-error returning functions",
+			code: `
+package main
+func main() {
+    println("test")
+    x := len("test")
+    _ = cap([]int{1,2,3})
+}`,
+			expected: 0,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			tmpDir, err := os.MkdirTemp("", "lint-test")
+			require.NoError(t, err)
+			defer os.RemoveAll(tmpDir)
+
+			tmpfile := filepath.Join(tmpDir, "test.go")
+			err = os.WriteFile(tmpfile, []byte(tt.code), 0o644)
+			require.NoError(t, err)
+
+			node, fset, err := ParseFile(tmpfile, nil)
+			require.NoError(t, err)
+
+			issues, err := DetectErrCheck(tmpfile, node, fset, types.SeverityError)
+			require.NoError(t, err)
+
+			assert.Equal(t, tt.expected, len(issues),
+				"Number of detected issues doesn't match expected")
+
+			if len(issues) > 0 {
+				for i, issue := range issues {
+					assert.Equal(t, "error-check", issue.Rule)
+					assert.Contains(t, issue.Message, tt.messages[i])
+				}
+			}
+		})
+	}
+}

internal/rule_set.go

@@ -365,6 +365,32 @@ func (r *ConstErrorDeclarationRule) Severity() tt.Severity {
 	return r.severity
 }
 
+type ErrCheckRule struct {
+	severity tt.Severity
+}
+
+func NewErrCheckRule() LintRule {
+	return &ErrCheckRule{
+		severity: tt.SeverityWarning,
+	}
+}
+
+func (r *ErrCheckRule) Check(filename string, node *ast.File, fset *token.FileSet) ([]tt.Issue, error) {
+	return lints.DetectErrCheck(filename, node, fset, r.severity)
+}
+
+func (r *ErrCheckRule) Name() string {
+	return "error-check"
+}
+
+func (r *ErrCheckRule) Severity() tt.Severity {
+	return r.severity
+}
+
+func (r *ErrCheckRule) SetSeverity(severity tt.Severity) {
+	r.severity = severity
+}
+
 // -----------------------------------------------------------------------------
 // Regex related rules
 

testdata/error-check/ec0.gno

@@ -0,0 +1,12 @@
+package main
+
+import "errors"
+
+func foo() (int, error) {
+	return 0, errors.New("test")
+}
+
+func main() {
+	v, _ := foo()
+	println(v)
+}