simplify slice expr (avoid len) (#10)

* simplify slice expr (avoid len)

* fix lint

* case specific suggestion output

* disable gosimple check and use our lint

* improve message

.github/workflows/go.yml

@@ -25,7 +25,4 @@ jobs:
       run: go build -v ./...
 
     - name: Test
-      run: go test -v ./...
-
-    - name: Golangci-lint
-      uses: golangci/[email protected]
+      run: go test -v ./...

formatter/fmt.go

@@ -1,14 +1,17 @@
 package formatter
 
 import (
+	"fmt"
 	"strings"
 
+	"github.com/fatih/color"
 	"github.com/gnoswap-labs/lint/internal"
 )
 
 // rule set
 const (
-	UnnecessaryElse = "unnecessary-else"
+	UnnecessaryElse   = "unnecessary-else"
+	SimplifySliceExpr = "simplify-slice-range"
 )
 
 // IssueFormatter is the interface that wraps the Format method.
@@ -36,6 +39,8 @@ func getFormatter(rule string) IssueFormatter {
 	switch rule {
 	case UnnecessaryElse:
 		return &UnnecessaryElseFormatter{}
+	case SimplifySliceExpr:
+		return &SimplifySliceExpressionFormatter{}
 	default:
 		return &GeneralIssueFormatter{}
 	}
@@ -47,3 +52,16 @@ func formatIssueHeader(issue internal.Issue) string {
 	return errorStyle.Sprint("error: ") + ruleStyle.Sprint(issue.Rule) + "\n" +
 		lineStyle.Sprint(" --> ") + fileStyle.Sprint(issue.Filename) + "\n"
 }
+
+func buildSuggestion(result *strings.Builder, issue internal.Issue, lineStyle, suggestionStyle *color.Color) {
+	result.WriteString(suggestionStyle.Sprintf("Suggestion:\n"))
+	result.WriteString(lineStyle.Sprintf("%d | ", issue.Start.Line))
+	result.WriteString(fmt.Sprintf("%s\n", issue.Suggestion))
+	result.WriteString("\n")
+}
+
+func buildNote(result *strings.Builder, issue internal.Issue, suggestionStyle *color.Color) {
+	result.WriteString(suggestionStyle.Sprint("Note: "))
+	result.WriteString(fmt.Sprintf("%s\n", issue.Note))
+	result.WriteString("\n")
+}

formatter/simplify_slice_expr.go

@@ -0,0 +1,35 @@
+package formatter
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/gnoswap-labs/lint/internal"
+)
+
+type SimplifySliceExpressionFormatter struct{}
+
+func (f *SimplifySliceExpressionFormatter) Format(
+	issue internal.Issue,
+	snippet *internal.SourceCode,
+) string {
+	var result strings.Builder
+
+	lineNumberStr := fmt.Sprintf("%d", issue.Start.Line)
+	padding := strings.Repeat(" ", len(lineNumberStr)-1)
+	result.WriteString(lineStyle.Sprintf("  %s|\n", padding))
+
+	line := expandTabs(snippet.Lines[issue.Start.Line-1])
+	result.WriteString(lineStyle.Sprintf("%d | ", issue.Start.Line))
+	result.WriteString(line + "\n")
+
+	visualColumn := calculateVisualColumn(line, issue.Start.Column)
+	result.WriteString(lineStyle.Sprintf("  %s| ", padding))
+	result.WriteString(strings.Repeat(" ", visualColumn))
+	result.WriteString(messageStyle.Sprintf("^ %s\n\n", issue.Message))
+
+	buildSuggestion(&result, issue, lineStyle, suggestionStyle)
+	buildNote(&result, issue, suggestionStyle)
+
+	return result.String()
+}

internal/engine.go

@@ -32,6 +32,7 @@ func (e *Engine) registerDefaultRules() {
 		&GolangciLintRule{},
 		&UnnecessaryElseRule{},
 		&UnusedFunctionRule{},
+		&SimplifySliceExprRule{},
 	)
 }
 

internal/lint.go

@@ -38,7 +38,7 @@ type golangciOutput struct {
 }
 
 func runGolangciLint(filename string) ([]Issue, error) {
-	cmd := exec.Command("golangci-lint", "run", "--out-format=json", filename)
+	cmd := exec.Command("golangci-lint", "run", "--disable=gosimple", "--out-format=json", filename)
 	output, _ := cmd.CombinedOutput()
 
 	var golangciResult golangciOutput
@@ -159,3 +159,70 @@ func (e *Engine) detectUnusedFunctions(filename string) ([]Issue, error) {
 
 	return issues, nil
 }
+
+type SimplifySliceExprRule struct{}
+
+func (r *SimplifySliceExprRule) Check(filename string) ([]Issue, error) {
+	engine := &Engine{}
+	return engine.detectUnnecessarySliceLength(filename)
+}
+
+func (e *Engine) detectUnnecessarySliceLength(filename string) ([]Issue, error) {
+	fset := token.NewFileSet()
+	node, err := parser.ParseFile(fset, filename, nil, parser.ParseComments)
+	if err != nil {
+		return nil, err
+	}
+
+	var issues []Issue
+	ast.Inspect(node, func(n ast.Node) bool {
+		sliceExpr, ok := n.(*ast.SliceExpr)
+		if !ok {
+			return true
+		}
+
+		if callExpr, ok := sliceExpr.High.(*ast.CallExpr); ok {
+			if ident, ok := callExpr.Fun.(*ast.Ident); ok && ident.Name == "len" {
+				if arg, ok := callExpr.Args[0].(*ast.Ident); ok {
+					if sliceExpr.X.(*ast.Ident).Name == arg.Name {
+						var suggestion, detailedMessage string
+						baseMessage := "unnecessary use of len() in slice expression, can be simplified"
+
+						if sliceExpr.Low == nil {
+							suggestion = fmt.Sprintf("%s[:]", arg.Name)
+							detailedMessage = fmt.Sprintf(
+								"%s\nIn this case, `%s[:len(%s)]` is equivalent to `%s[:]`. "+
+									"The full length of the slice is already implied when omitting both start and end indices.",
+								baseMessage, arg.Name, arg.Name, arg.Name)
+						} else if basicLit, ok := sliceExpr.Low.(*ast.BasicLit); ok {
+							suggestion = fmt.Sprintf("%s[%s:]", arg.Name, basicLit.Value)
+							detailedMessage = fmt.Sprintf("%s\nHere, `%s[%s:len(%s)]` can be simplified to `%s[%s:]`. "+
+								"When slicing to the end of a slice, using len() is unnecessary.",
+								baseMessage, arg.Name, basicLit.Value, arg.Name, arg.Name, basicLit.Value)
+						} else if lowIdent, ok := sliceExpr.Low.(*ast.Ident); ok {
+							suggestion = fmt.Sprintf("%s[%s:]", arg.Name, lowIdent.Name)
+							detailedMessage = fmt.Sprintf("%s\nIn this instance, `%s[%s:len(%s)]` can be written as `%s[%s:]`. "+
+								"The len() function is redundant when slicing to the end, regardless of the start index.",
+								baseMessage, arg.Name, lowIdent.Name, arg.Name, arg.Name, lowIdent.Name)
+						}
+
+						issue := Issue{
+							Rule:       "simplify-slice-range",
+							Filename:   filename,
+							Start:      fset.Position(sliceExpr.Pos()),
+							End:        fset.Position(sliceExpr.End()),
+							Message:    baseMessage,
+							Suggestion: suggestion,
+							Note:       detailedMessage,
+						}
+						issues = append(issues, issue)
+					}
+				}
+			}
+		}
+
+		return true
+	})
+
+	return issues, nil
+}

internal/lint_test.go

@@ -197,3 +197,91 @@ func unused2() {
 		})
 	}
 }
+
+func TestDetectUnnecessarySliceLength(t *testing.T) {
+	baseMsg := "unnecessary use of len() in slice expression, can be simplified"
+	tests := []struct {
+		name     string
+		code     string
+		expected int
+		message  string
+	}{
+		{
+			name: "suggests to use slice[:]",
+			code: `
+package main
+
+func main() {
+	slice := []int{1, 2, 3}
+	_ = slice[:len(slice)]
+}`,
+			expected: 1,
+			message:  baseMsg,
+		},
+		{
+			name: "suggests to use slice[a:]",
+			code: `
+package main
+
+func main() {
+	slice := []int{1, 2, 3}
+	_ = slice[1:len(slice)]
+}`,
+			expected: 1,
+			message:  baseMsg,
+		},
+		{
+			name: "Unnecessary slice length",
+			code: `
+package main
+
+func main() {
+	slice := []int{1, 2, 3}
+	_ = slice[:]
+}`,
+			expected: 0,
+		},
+		{
+			name: "slice[a:len(slice)] -> slice[a:] (a: variable)",
+			code: `
+package main
+
+func main() {
+	slice := []int{1, 2, 3}
+	a := 1
+	_ = slice[a:len(slice)]
+}`,
+			expected: 1,
+			message:  baseMsg,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			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)
+
+			engine := &Engine{}
+			issues, err := engine.detectUnnecessarySliceLength(tmpfile)
+			require.NoError(t, err)
+
+			assert.Equal(t, tt.expected, len(issues), "Number of detected unnecessary slice length doesn't match expected")
+
+			if len(issues) > 0 {
+				for _, issue := range issues {
+					assert.Equal(t, "simplify-slice-range", issue.Rule)
+					assert.Equal(
+						t,
+						tt.message,
+						issue.Message,
+					)
+				}
+			}
+		})
+	}
+}

internal/types.go

@@ -9,9 +9,11 @@ type SourceCode struct {
 
 // Issue represents a lint issue found in the code base.
 type Issue struct {
-	Rule     string
-	Filename string
-	Start    token.Position
-	End      token.Position
-	Message  string
+	Rule       string
+	Filename   string
+	Start      token.Position
+	End        token.Position
+	Message    string
+	Suggestion string
+	Note       string
 }

testdata/slice0.gno

@@ -0,0 +1,16 @@
+package slice
+
+var slice = []int{1, 2, 3}
+
+func foo() {
+	_ = slice[1:len(slice)]
+}
+
+func bar() {
+	_ = slice[:len(slice)]
+}
+
+func baz() {
+	a := 1
+	_ = slice[a:len(slice)]
+}