diff --git a/Makefile b/Makefile index 8fd7eef..0e2c9c3 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,9 @@ BINARY_MAC=$(BINARY_NAME)_mac # Main package path MAIN_PACKAGE=./cmd/tlin +install: + go install $(MAIN_PACKAGE) + # Build the project all: test build diff --git a/README.md b/README.md index 01aea3b..0769aef 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ Inspired by Rust's [clippy](https://github.com/rust-lang/rust-clippy), tlin aims - Requirements: - Go: 1.22 or higher - latest version of gno + - GNU Make 3.81 or higher (for building) To install tlin CLI, follow these steps: diff --git a/formatter/format_emit.go b/formatter/format_emit.go index 020cee9..fd1ca64 100644 --- a/formatter/format_emit.go +++ b/formatter/format_emit.go @@ -16,8 +16,6 @@ func (f *EmitFormatFormatter) Format( ) string { var result strings.Builder - result.WriteString(formatIssueHeader(issue)) - maxLineNumWidth := calculateMaxLineNumWidth(issue.End.Line) padding := strings.Repeat(" ", maxLineNumWidth+1) diff --git a/formatter/general.go b/formatter/general.go index b84c7aa..492a947 100644 --- a/formatter/general.go +++ b/formatter/general.go @@ -13,7 +13,7 @@ const tabWidth = 8 var ( errorStyle = color.New(color.FgRed, color.Bold) - warningStyle = color.New(color.FgHiYellow, color.Bold) + warningStyle = color.New(color.FgHiYellow, color.Bold) ruleStyle = color.New(color.FgYellow, color.Bold) fileStyle = color.New(color.FgCyan, color.Bold) lineStyle = color.New(color.FgBlue, color.Bold) diff --git a/formatter/slice_bound.go b/formatter/slice_bound.go index 2d7aac3..9996062 100644 --- a/formatter/slice_bound.go +++ b/formatter/slice_bound.go @@ -34,10 +34,10 @@ func (f *SliceBoundsCheckFormatter) Format( result.WriteString(warningStyle.Sprint("warning: ")) if issue.Category == "index-access" { - result.WriteString("Index access without bounds checking can lead to runtime panics.\n") - } else if issue.Category == "slice-expression" { - result.WriteString("Slice expressions without proper length checks may cause unexpected behavior.\n\n") - } + result.WriteString("Index access without bounds checking can lead to runtime panics.\n") + } else if issue.Category == "slice-expression" { + result.WriteString("Slice expressions without proper length checks may cause unexpected behavior.\n\n") + } return result.String() } diff --git a/internal/lints/lint_test.go b/internal/lints/lint_test.go index b984422..5dcf2aa 100644 --- a/internal/lints/lint_test.go +++ b/internal/lints/lint_test.go @@ -83,6 +83,7 @@ func example2() int { require.NoError(t, err) node, fset, err := ParseFile(tmpfile) + assert.NoError(t, err) issues, err := DetectUnnecessaryElse(tmpfile, node, fset) require.NoError(t, err) diff --git a/internal/lints/slice_bound.go b/internal/lints/slice_bound.go index 93f6034..0c34751 100644 --- a/internal/lints/slice_bound.go +++ b/internal/lints/slice_bound.go @@ -19,24 +19,25 @@ func DetectSliceBoundCheck(filename string, node *ast.File, fset *token.FileSet) } if assignStmt, ok := findAssignmentForIdent(node, ident); ok { - if callExpr, ok := assignStmt.Rhs[0].(*ast.CallExpr); ok { - if fun, ok := callExpr.Fun.(*ast.Ident); ok && fun.Name == "make" { - // make로 생성된 슬라이스의 경우, 초기 길이가 0이면 위험할 수 있음 - if len(callExpr.Args) >= 2 { - if lit, ok := callExpr.Args[1].(*ast.BasicLit); ok && lit.Value == "0" { - issue := createIssue(x, ident, filename, fset) - issues = append(issues, issue) - return true - } - } - } - } - } - - if !isWithinSafeContext(node, x) && !isWithinBoundsCheck(node, x, ident) { - issue := createIssue(x, ident, filename, fset) - issues = append(issues, issue) - } + if callExpr, ok := assignStmt.Rhs[0].(*ast.CallExpr); ok { + if fun, ok := callExpr.Fun.(*ast.Ident); ok && fun.Name == "make" { + // Slice created with make, it can be dangerous if the initial length is 0 + // because the slice will be nil and accessing an index will panic + if len(callExpr.Args) >= 2 { + if lit, ok := callExpr.Args[1].(*ast.BasicLit); ok && lit.Value == "0" { + issue := createIssue(x, ident, filename, fset) + issues = append(issues, issue) + return true + } + } + } + } + } + + if !isWithinSafeContext(node, x) && !isWithinBoundsCheck(node, x, ident) { + issue := createIssue(x, ident, filename, fset) + issues = append(issues, issue) + } } return true }) @@ -45,34 +46,34 @@ func DetectSliceBoundCheck(filename string, node *ast.File, fset *token.FileSet) } func createIssue(node ast.Node, ident *ast.Ident, filename string, fset *token.FileSet) tt.Issue { - var category, message, suggestion, note string + var category, message, suggestion, note string - switch x := node.(type) { - case *ast.IndexExpr: + switch x := node.(type) { + case *ast.IndexExpr: if isConstantIndex(x.Index) { return tt.Issue{} } category = "index-access" - message = "Potential out of bounds array/slice index access" - suggestion = fmt.Sprintf("if i < len(%s) { value := %s[i] }", ident.Name, ident.Name) - note = "Always check the length of the array/slice before accessing an index to prevent runtime panics." - case *ast.SliceExpr: + message = "Potential out of bounds array/slice index access" + suggestion = fmt.Sprintf("if i < len(%s) { value := %s[i] }", ident.Name, ident.Name) + note = "Always check the length of the array/slice before accessing an index to prevent runtime panics." + case *ast.SliceExpr: category = "slice-expression" - message = "Potential out of bounds slice expression" - suggestion = fmt.Sprintf("%s = append(%s, newElement)", ident.Name, ident.Name) - note = "Consider using append() for slices to automatically handle capacity and prevent out of bounds errors." - } + message = "Potential out of bounds slice expression" + suggestion = fmt.Sprintf("%s = append(%s, newElement)", ident.Name, ident.Name) + note = "Consider using append() for slices to automatically handle capacity and prevent out of bounds errors." + } - return tt.Issue{ - Rule: "slice-bounds-check", + return tt.Issue{ + Rule: "slice-bounds-check", Category: category, - Filename: filename, - Start: fset.Position(node.Pos()), - End: fset.Position(node.End()), - Message: message, - Suggestion: suggestion, - Note: note, - } + Filename: filename, + Start: fset.Position(node.Pos()), + End: fset.Position(node.End()), + Message: message, + Suggestion: suggestion, + Note: note, + } } // getIdentForSliceOrArr checks if the node is within an if statement @@ -155,35 +156,35 @@ func isCapOrLenCallWithIdent(call *ast.CallExpr, ident *ast.Ident) bool { // 4. If a safe context is found, it immediately stops traversal and returns true. // // Notes: -// - This function may not cover all possible safe usage scenarios. -// - Complex nested structures or indirect access through function calls may be difficult to analyze accurately. +// - This function may not cover all possible safe usage scenarios. +// - Complex nested structures or indirect access through function calls may be difficult to analyze accurately. func isWithinSafeContext(file *ast.File, node ast.Node) bool { - var safeContext bool - ast.Inspect(file, func(n ast.Node) bool { - if n == node { - return false - } - switch x := n.(type) { - case *ast.RangeStmt: - if containsNode(x.Body, node) { + var safeContext bool + ast.Inspect(file, func(n ast.Node) bool { + if n == node { + return false + } + switch x := n.(type) { + case *ast.RangeStmt: + if containsNode(x.Body, node) { // inside a range statement, but check if the index expression is the range variable - if indexExpr, ok := node.(*ast.IndexExpr); ok { - if ident, ok := indexExpr.X.(*ast.Ident); ok { + if indexExpr, ok := node.(*ast.IndexExpr); ok { + if ident, ok := indexExpr.X.(*ast.Ident); ok { // accessing a different slice/array than the range variable is not safe - safeContext = (ident.Name == x.Key.(*ast.Ident).Name) - } - } - return false - } - case *ast.ForStmt: - if isForWithLenCheck(x) && containsNode(x.Body, node) { - safeContext = true - return false - } - } - return true - }) - return safeContext + safeContext = (ident.Name == x.Key.(*ast.Ident).Name) + } + } + return false + } + case *ast.ForStmt: + if isForWithLenCheck(x) && containsNode(x.Body, node) { + safeContext = true + return false + } + } + return true + }) + return safeContext } func isForWithLenCheck(forStmt *ast.ForStmt) bool { @@ -217,21 +218,21 @@ func isBinaryExprLenCheck(expr *ast.BinaryExpr) bool { } func findAssignmentForIdent(file *ast.File, ident *ast.Ident) (*ast.AssignStmt, bool) { - var assignStmt *ast.AssignStmt - var found bool - - ast.Inspect(file, func(n ast.Node) bool { - if assign, ok := n.(*ast.AssignStmt); ok { - for _, lhs := range assign.Lhs { - if id, ok := lhs.(*ast.Ident); ok && id.Name == ident.Name { - assignStmt = assign - found = true - return false - } - } - } - return true - }) - - return assignStmt, found -} \ No newline at end of file + var assignStmt *ast.AssignStmt + var found bool + + ast.Inspect(file, func(n ast.Node) bool { + if assign, ok := n.(*ast.AssignStmt); ok { + for _, lhs := range assign.Lhs { + if id, ok := lhs.(*ast.Ident); ok && id.Name == ident.Name { + assignStmt = assign + found = true + return false + } + } + } + return true + }) + + return assignStmt, found +}