Skip to content

Commit

Permalink
chore: fixup minor problems
Browse files Browse the repository at this point in the history
  • Loading branch information
notJoon committed Jul 31, 2024
1 parent 99fa317 commit 5e3275d
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 89 deletions.
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
2 changes: 0 additions & 2 deletions formatter/format_emit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion formatter/general.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions formatter/slice_bound.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
1 change: 1 addition & 0 deletions internal/lints/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
165 changes: 83 additions & 82 deletions internal/lints/slice_bound.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
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
}

0 comments on commit 5e3275d

Please sign in to comment.