diff --git a/README.md b/README.md index 675f7fb..6b7f24a 100644 --- a/README.md +++ b/README.md @@ -20,8 +20,8 @@ Inspired by Rust's [clippy](https://github.com/rust-lang/rust-clippy), tlin aims ## Installation - Requirements: - - Go: 1.22 or higher - - latest version of gno + - Go: 1.22 or higher + - latest version of gno To install tlin CLI, run: @@ -74,33 +74,33 @@ func (e *Engine) registerDefaultRules() { 3. (Optional) if your rule requires special formatting, create a new formatter in the `formatter` package: - a. Create a new file (e.g., `formatter/new_rule.go`). - b. Implement the `IssueFormatter` interface for your new rule: - - ```go - type NewRuleFormatter struct{} - - func (f *NewRuleFormatter) Format( - issue internal.Issue, - snippet *internal.SourceCode, - ) string { - // Implement formatting logic for new rule here. - } - ``` - - c. Add the new formatter to the `GetFormatter` function in `formatter/fmt.go`. - - ```go - func GetFormatter(rule string) IssueFormatter { - switch rule { - // ... - case "new_rule": // Add your new rule here - return &NewRuleFormatter{} - default: - return &DefaultFormatter{} - } - } - ``` + a. Create a new file (e.g., `formatter/new_rule.go`). + b. Implement the `IssueFormatter` interface for your new rule: + + ```go + type NewRuleFormatter struct{} + + func (f *NewRuleFormatter) Format( + issue lints.Issue, + snippet *internal.SourceCode, + ) string { + // Implement formatting logic for new rule here. + } + ``` + + c. Add the new formatter to the `GetFormatter` function in `formatter/fmt.go`. + + ```go + func GetFormatter(rule string) IssueFormatter { + switch rule { + // ... + case "new_rule": // Add your new rule here + return &NewRuleFormatter{} + default: + return &DefaultFormatter{} + } + } + ``` By following these steps, you can add new lint rules and ensure they are properly formatted when displayed in the CLI. diff --git a/cmd/tlin/main.go b/cmd/tlin/main.go index ea16c31..6ad557c 100644 --- a/cmd/tlin/main.go +++ b/cmd/tlin/main.go @@ -8,7 +8,8 @@ import ( "sort" "github.com/gnoswap-labs/lint/formatter" - lint "github.com/gnoswap-labs/lint/internal" + "github.com/gnoswap-labs/lint/internal" + "github.com/gnoswap-labs/lint/internal/lints" ) func main() { @@ -23,13 +24,13 @@ func main() { } rootDir := "." - engine, err := lint.NewEngine(rootDir) + engine, err := internal.NewEngine(rootDir) if err != nil { fmt.Printf("error initializing lint engine: %v\n", err) os.Exit(1) } - var allIssues []lint.Issue + var allIssues []lints.Issue for _, path := range args { info, err := os.Stat(path) if err != nil { @@ -69,7 +70,7 @@ func main() { } } - issuesByFile := make(map[string][]lint.Issue) + issuesByFile := make(map[string][]lints.Issue) for _, issue := range allIssues { issuesByFile[issue.Filename] = append(issuesByFile[issue.Filename], issue) } @@ -82,7 +83,7 @@ func main() { for _, filename := range sortedFiles { issues := issuesByFile[filename] - sourceCode, err := lint.ReadSourceCode(filename) + sourceCode, err := internal.ReadSourceCode(filename) if err != nil { fmt.Printf("error reading source file %s: %v\n", filename, err) continue @@ -96,7 +97,7 @@ func main() { } } -func processFile(engine *lint.Engine, filePath string) ([]lint.Issue, error) { +func processFile(engine *internal.Engine, filePath string) ([]lints.Issue, error) { issues, err := engine.Run(filePath) if err != nil { return nil, fmt.Errorf("error linting %s: %w", filePath, err) diff --git a/formatter/fmt.go b/formatter/fmt.go index 604131c..6e2f0c2 100644 --- a/formatter/fmt.go +++ b/formatter/fmt.go @@ -6,6 +6,7 @@ import ( "github.com/fatih/color" "github.com/gnoswap-labs/lint/internal" + "github.com/gnoswap-labs/lint/internal/lints" ) // rule set @@ -18,12 +19,12 @@ const ( // IssueFormatter is the interface that wraps the Format method. // Implementations of this interface are responsible for formatting specific types of lint issues. type IssueFormatter interface { - Format(issue internal.Issue, snippet *internal.SourceCode) string + Format(issue lints.Issue, snippet *internal.SourceCode) string } // FormatIssuesWithArrows formats a slice of issues into a human-readable string. // It uses the appropriate formatter for each issue based on its rule. -func FormatIssuesWithArrows(issues []internal.Issue, snippet *internal.SourceCode) string { +func FormatIssuesWithArrows(issues []lints.Issue, snippet *internal.SourceCode) string { var builder strings.Builder for _, issue := range issues { builder.WriteString(formatIssueHeader(issue)) @@ -51,19 +52,19 @@ func getFormatter(rule string) IssueFormatter { // formatIssueHeader creates a formatted header string for a given issue. // The header includes the rule and the filename. (e.g. "error: unused-variable\n --> test.go") -func formatIssueHeader(issue internal.Issue) string { +func formatIssueHeader(issue lints.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) { +func buildSuggestion(result *strings.Builder, issue lints.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) { +func buildNote(result *strings.Builder, issue lints.Issue, suggestionStyle *color.Color) { result.WriteString(suggestionStyle.Sprint("Note: ")) result.WriteString(fmt.Sprintf("%s\n", issue.Note)) result.WriteString("\n") diff --git a/formatter/fmt_test.go b/formatter/fmt_test.go index c647988..54e067c 100644 --- a/formatter/fmt_test.go +++ b/formatter/fmt_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/gnoswap-labs/lint/internal" + "github.com/gnoswap-labs/lint/internal/lints" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -24,7 +25,7 @@ func TestFormatIssuesWithArrows(t *testing.T) { }, } - issues := []internal.Issue{ + issues := []lints.Issue{ { Rule: "unused-variable", Filename: "test.go", @@ -106,7 +107,7 @@ func TestFormatIssuesWithArrows_MultipleDigitsLineNumbers(t *testing.T) { }, } - issues := []internal.Issue{ + issues := []lints.Issue{ { Rule: "unused-variable", Filename: "test.go", @@ -170,7 +171,7 @@ func TestFormatIssuesWithArrows_UnnecessaryElse(t *testing.T) { }, } - issues := []internal.Issue{ + issues := []lints.Issue{ { Rule: "unnecessary-else", Filename: "test.go", @@ -300,7 +301,7 @@ func main() { func TestUnnecessaryTypeConversionFormatter(t *testing.T) { formatter := &UnnecessaryTypeConversionFormatter{} - issue := internal.Issue{ + issue := lints.Issue{ Rule: "unnecessary-type-conversion", Filename: "test.go", Start: token.Position{Line: 5, Column: 10}, diff --git a/formatter/general.go b/formatter/general.go index 81fea78..6aa3eca 100644 --- a/formatter/general.go +++ b/formatter/general.go @@ -6,6 +6,7 @@ import ( "github.com/fatih/color" "github.com/gnoswap-labs/lint/internal" + "github.com/gnoswap-labs/lint/internal/lints" ) const tabWidth = 8 @@ -25,7 +26,7 @@ type GeneralIssueFormatter struct{} // Format formats a general lint issue into a human-readable string. // It takes an Issue and a SourceCode snippet as input and returns a formatted string. func (f *GeneralIssueFormatter) Format( - issue internal.Issue, + issue lints.Issue, snippet *internal.SourceCode, ) string { var result strings.Builder diff --git a/formatter/simplify_slice_expr.go b/formatter/simplify_slice_expr.go index c55cafc..7241a18 100644 --- a/formatter/simplify_slice_expr.go +++ b/formatter/simplify_slice_expr.go @@ -5,12 +5,13 @@ import ( "strings" "github.com/gnoswap-labs/lint/internal" + "github.com/gnoswap-labs/lint/internal/lints" ) type SimplifySliceExpressionFormatter struct{} func (f *SimplifySliceExpressionFormatter) Format( - issue internal.Issue, + issue lints.Issue, snippet *internal.SourceCode, ) string { var result strings.Builder diff --git a/formatter/unnecessary_else.go b/formatter/unnecessary_else.go index edc44e1..15ae9ce 100644 --- a/formatter/unnecessary_else.go +++ b/formatter/unnecessary_else.go @@ -5,13 +5,14 @@ import ( "strings" "github.com/gnoswap-labs/lint/internal" + "github.com/gnoswap-labs/lint/internal/lints" ) // UnnecessaryElseFormatter is a formatter specifically designed for the "unnecessary-else" rule. type UnnecessaryElseFormatter struct{} func (f *UnnecessaryElseFormatter) Format( - issue internal.Issue, + issue lints.Issue, snippet *internal.SourceCode, ) string { var result strings.Builder @@ -61,7 +62,7 @@ func calculateMaxLineLength(lines []string, start, end int) int { return maxLen } -func formatSuggestion(issue internal.Issue, improvedSnippet string, startLine int) string { +func formatSuggestion(issue lints.Issue, improvedSnippet string, startLine int) string { var result strings.Builder lines := strings.Split(improvedSnippet, "\n") maxLineNumWidth := calculateMaxLineNumWidth(issue.End.Line) diff --git a/formatter/unnecessary_type_conv.go b/formatter/unnecessary_type_conv.go index 008e3a0..268cbc3 100644 --- a/formatter/unnecessary_type_conv.go +++ b/formatter/unnecessary_type_conv.go @@ -5,12 +5,13 @@ import ( "strings" "github.com/gnoswap-labs/lint/internal" + "github.com/gnoswap-labs/lint/internal/lints" ) type UnnecessaryTypeConversionFormatter struct{} func (f *UnnecessaryTypeConversionFormatter) Format( - issue internal.Issue, + issue lints.Issue, snippet *internal.SourceCode, ) string { var result strings.Builder diff --git a/internal/engine.go b/internal/engine.go index d1e2bef..c323fbf 100644 --- a/internal/engine.go +++ b/internal/engine.go @@ -5,6 +5,8 @@ import ( "os" "path/filepath" "strings" + + "github.com/gnoswap-labs/lint/internal/lints" ) // Engine manages the linting process. @@ -43,14 +45,14 @@ func (e *Engine) AddRule(rule LintRule) { } // Run applies all lint rules to the given file and returns a slice of Issues. -func (e *Engine) Run(filename string) ([]Issue, error) { +func (e *Engine) Run(filename string) ([]lints.Issue, error) { tempFile, err := e.prepareFile(filename) if err != nil { return nil, err } defer e.cleanupTemp(tempFile) - var allIssues []Issue + var allIssues []lints.Issue for _, rule := range e.rules { issues, err := rule.Check(tempFile) if err != nil { @@ -84,8 +86,8 @@ func (e *Engine) cleanupTemp(temp string) { } } -func (e *Engine) filterUndefinedIssues(issues []Issue) []Issue { - var filtered []Issue +func (e *Engine) filterUndefinedIssues(issues []lints.Issue) []lints.Issue { + var filtered []lints.Issue for _, issue := range issues { if issue.Rule == "typecheck" && strings.Contains(issue.Message, "undefined:") { symbol := strings.TrimSpace(strings.TrimPrefix(issue.Message, "undefined:")) @@ -126,6 +128,11 @@ func createTempGoFile(gnoFile string) (string, error) { return tempFile.Name(), nil } +// SourceCode stores the content of a source code file. +type SourceCode struct { + Lines []string +} + // ReadSourceFile reads the content of a file and returns it as a `SourceCode` struct. func ReadSourceCode(filename string) (*SourceCode, error) { content, err := os.ReadFile(filename) diff --git a/internal/fixer.go b/internal/fixer.go index 0207c2a..5de0d64 100644 --- a/internal/fixer.go +++ b/internal/fixer.go @@ -6,6 +6,8 @@ import ( "go/parser" "go/token" "strings" + + "github.com/gnoswap-labs/lint/internal/lints" ) func RemoveUnnecessaryElse(snippet string) (string, error) { @@ -121,7 +123,7 @@ func insertStatementsAfter(block *ast.BlockStmt, target ast.Stmt, stmts []ast.St } } -func ExtractSnippet(issue Issue, code string, startLine, endLine int) string { +func ExtractSnippet(issue lints.Issue, code string, startLine, endLine int) string { lines := strings.Split(code, "\n") // ensure we don't go out of bounds diff --git a/internal/lint.go b/internal/lint.go deleted file mode 100644 index b0e28b2..0000000 --- a/internal/lint.go +++ /dev/null @@ -1,446 +0,0 @@ -package internal - -import ( - "encoding/json" - "fmt" - "go/ast" - "go/importer" - "go/parser" - "go/token" - "go/types" - "os/exec" -) - -/* -* Implement each lint rule as a separate struct - */ - -// LintRule defines the interface for all lint rules. -type LintRule interface { - // Check runs the lint rule on the given file and returns a slice of Issues. - Check(filename string) ([]Issue, error) -} - -type GolangciLintRule struct{} - -func (r *GolangciLintRule) Check(filename string) ([]Issue, error) { - return runGolangciLint(filename) -} - -type golangciOutput struct { - Issues []struct { - FromLinter string `json:"FromLinter"` - Text string `json:"Text"` - Pos struct { - Filename string `json:"Filename"` - Line int `json:"Line"` - Column int `json:"Column"` - } `json:"Pos"` - } `json:"Issues"` -} - -func runGolangciLint(filename string) ([]Issue, error) { - cmd := exec.Command("golangci-lint", "run", "--disable=gosimple", "--out-format=json", filename) - output, _ := cmd.CombinedOutput() - - var golangciResult golangciOutput - if err := json.Unmarshal(output, &golangciResult); err != nil { - return nil, fmt.Errorf("error unmarshaling golangci-lint output: %w", err) - } - - var issues []Issue - for _, gi := range golangciResult.Issues { - issues = append(issues, Issue{ - Rule: gi.FromLinter, - Filename: gi.Pos.Filename, // Use the filename from golangci-lint output - Start: token.Position{Filename: gi.Pos.Filename, Line: gi.Pos.Line, Column: gi.Pos.Column}, - End: token.Position{Filename: gi.Pos.Filename, Line: gi.Pos.Line, Column: gi.Pos.Column + 1}, - Message: gi.Text, - }) - } - - return issues, nil -} - -type UnnecessaryElseRule struct{} - -func (r *UnnecessaryElseRule) Check(filename string) ([]Issue, error) { - engine := &Engine{} - return engine.detectUnnecessaryElse(filename) -} - -// detectUnnecessaryElse detects unnecessary else blocks. -// This rule considers an else block unnecessary if the if block ends with a return statement. -// In such cases, the else block can be removed and the code can be flattened to improve readability. -func (e *Engine) detectUnnecessaryElse(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 { - ifStmt, ok := n.(*ast.IfStmt) - if !ok { - return true - } - - if ifStmt.Else != nil { - blockStmt := ifStmt.Body - if len(blockStmt.List) > 0 { - lastStmt := blockStmt.List[len(blockStmt.List)-1] - if _, isReturn := lastStmt.(*ast.ReturnStmt); isReturn { - issue := Issue{ - Rule: "unnecessary-else", - Filename: filename, - Start: fset.Position(ifStmt.Else.Pos()), - End: fset.Position(ifStmt.Else.End()), - Message: "unnecessary else block", - } - issues = append(issues, issue) - } - } - } - - return true - }) - - return issues, nil -} - -type UnusedFunctionRule struct{} - -func (r *UnusedFunctionRule) Check(filename string) ([]Issue, error) { - engine := &Engine{} - return engine.detectUnusedFunctions(filename) -} - -// detectUnusedFunctions detects functions that are declared but never used. -// This rule reports all unused functions except for the following cases: -// 1. The main function: It's considered "used" as it's the entry point of the program. -// 2. The init function: It's used for package initialization and runs without explicit calls. -// 3. Exported functions: Functions starting with a capital letter are excluded as they might be used in other packages. -// -// This rule helps in code cleanup and improves maintainability. -func (e *Engine) detectUnusedFunctions(filename string) ([]Issue, error) { - fset := token.NewFileSet() - node, err := parser.ParseFile(fset, filename, nil, parser.ParseComments) - if err != nil { - return nil, err - } - - declaredFuncs := make(map[string]*ast.FuncDecl) - calledFuncs := make(map[string]bool) - - ast.Inspect(node, func(n ast.Node) bool { - switch x := n.(type) { - case *ast.FuncDecl: - declaredFuncs[x.Name.Name] = x - case *ast.CallExpr: - if ident, ok := x.Fun.(*ast.Ident); ok { - calledFuncs[ident.Name] = true - } - } - return true - }) - - var issues []Issue - for funcName, funcDecl := range declaredFuncs { - if !calledFuncs[funcName] && funcName != "main" && funcName != "init" && !ast.IsExported(funcName) { - issue := Issue{ - Rule: "unused-function", - Filename: filename, - Start: fset.Position(funcDecl.Pos()), - End: fset.Position(funcDecl.End()), - Message: "function " + funcName + " is declared but not used", - } - issues = append(issues, issue) - } - } - - 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 -} - -type UnnecessaryConversionRule struct{} - -func (r *UnnecessaryConversionRule) Check(filename string) ([]Issue, error) { - engine := &Engine{} - return engine.detectUnnecessaryConversions(filename) -} - -func (e *Engine) detectUnnecessaryConversions(filename string) ([]Issue, error) { - fset := token.NewFileSet() - f, err := parser.ParseFile(fset, filename, nil, parser.ParseComments) - if err != nil { - return nil, err - } - - info := &types.Info{ - Types: make(map[ast.Expr]types.TypeAndValue), - Uses: make(map[*ast.Ident]types.Object), - Defs: make(map[*ast.Ident]types.Object), - } - - conf := types.Config{Importer: importer.Default()} - //! DO NOT CHECK ERROR HERE. - //! error check may broke the lint formatting process. - conf.Check("", fset, []*ast.File{f}, info) - - var issues []Issue - varDecls := make(map[*types.Var]ast.Node) - - // First pass: collect variable declarations - ast.Inspect(f, func(n ast.Node) bool { - switch node := n.(type) { - case *ast.ValueSpec: - for _, name := range node.Names { - if obj := info.Defs[name]; obj != nil { - if v, ok := obj.(*types.Var); ok { - varDecls[v] = node - } - } - } - case *ast.AssignStmt: - for _, lhs := range node.Lhs { - if id, ok := lhs.(*ast.Ident); ok { - if obj := info.Defs[id]; obj != nil { - if v, ok := obj.(*types.Var); ok { - varDecls[v] = node - } - } - } - } - } - return true - }) - - // Second pass: check for unnecessary conversions - ast.Inspect(f, func(n ast.Node) bool { - call, ok := n.(*ast.CallExpr) - if !ok || len(call.Args) != 1 { - return true - } - - ft, ok := info.Types[call.Fun] - if !ok || !ft.IsType() { - return true - } - - at, ok := info.Types[call.Args[0]] - if !ok { - return true - } - - if types.Identical(ft.Type, at.Type) && !isUntypedValue(call.Args[0], info) { - var memo, suggestion string - - // find parent node and retrieve the entire assignment statement - var parent ast.Node - ast.Inspect(f, func(node ast.Node) bool { - if node == n { - return false - } - if contains(node, n) { - parent = node - return false - } - return true - }) - - if assignStmt, ok := parent.(*ast.AssignStmt); ok { - if len(assignStmt.Lhs) > 0 { - lhs := types.ExprString(assignStmt.Lhs[0]) - rhs := types.ExprString(call.Args[0]) - suggestion = fmt.Sprintf("%s := %s", lhs, rhs) - } - } else { - // not an assignment statement - // keep maintaining the original code - suggestion = types.ExprString(call.Args[0]) - } - - if id, ok := call.Args[0].(*ast.Ident); ok { - if obj, ok := info.Uses[id].(*types.Var); ok { - if _, exists := varDecls[obj]; exists { - declType := obj.Type().String() - memo = fmt.Sprintf( - "The variable '%s' is declared as type '%s'. This type conversion appears unnecessary.", - id.Name, declType, - ) - } - } - } - - issues = append(issues, Issue{ - Rule: "unnecessary-type-conversion", - Filename: filename, - Start: fset.Position(call.Pos()), - End: fset.Position(call.End()), - Message: "unnecessary type conversion", - Suggestion: suggestion, - Note: memo, - }) - } - - return true - }) - - return issues, nil -} - -// ref: https://github.com/mdempsky/unconvert/blob/master/unconvert.go#L570 -func isUntypedValue(n ast.Expr, info *types.Info) (res bool) { - switch n := n.(type) { - case *ast.BinaryExpr: - switch n.Op { - case token.SHL, token.SHR: - // Shifts yield an untyped value if their LHS is untyped. - return isUntypedValue(n.X, info) - case token.EQL, token.NEQ, token.LSS, token.GTR, token.LEQ, token.GEQ: - // Comparisons yield an untyped boolean value. - return true - case token.ADD, token.SUB, token.MUL, token.QUO, token.REM, - token.AND, token.OR, token.XOR, token.AND_NOT, - token.LAND, token.LOR: - return isUntypedValue(n.X, info) && isUntypedValue(n.Y, info) - } - case *ast.UnaryExpr: - switch n.Op { - case token.ADD, token.SUB, token.NOT, token.XOR: - return isUntypedValue(n.X, info) - } - case *ast.BasicLit: - // Basic literals are always untyped. - return true - case *ast.ParenExpr: - return isUntypedValue(n.X, info) - case *ast.SelectorExpr: - return isUntypedValue(n.Sel, info) - case *ast.Ident: - if obj, ok := info.Uses[n]; ok { - if obj.Pkg() == nil && obj.Name() == "nil" { - // The universal untyped zero value. - return true - } - if b, ok := obj.Type().(*types.Basic); ok && b.Info()&types.IsUntyped != 0 { - // Reference to an untyped constant. - return true - } - } - case *ast.CallExpr: - if b, ok := asBuiltin(n.Fun, info); ok { - switch b.Name() { - case "real", "imag": - return isUntypedValue(n.Args[0], info) - case "complex": - return isUntypedValue(n.Args[0], info) && isUntypedValue(n.Args[1], info) - } - } - } - - return false -} - -func asBuiltin(n ast.Expr, info *types.Info) (*types.Builtin, bool) { - for { - paren, ok := n.(*ast.ParenExpr) - if !ok { - break - } - n = paren.X - } - - ident, ok := n.(*ast.Ident) - if !ok { - return nil, false - } - - obj, ok := info.Uses[ident] - if !ok { - return nil, false - } - - b, ok := obj.(*types.Builtin) - return b, ok -} - -// contains checks if parent contains child node -func contains(parent, child ast.Node) bool { - found := false - ast.Inspect(parent, func(n ast.Node) bool { - if n == child { - found = true - return false - } - return true - }) - return found -} diff --git a/internal/lints/README.md b/internal/lints/README.md new file mode 100644 index 0000000..0d53fdf --- /dev/null +++ b/internal/lints/README.md @@ -0,0 +1,3 @@ +# internal/lints + +The `internal/lints` directory stores the code that implements the lint rules. diff --git a/internal/lints/default_golangci.go b/internal/lints/default_golangci.go new file mode 100644 index 0000000..72a310f --- /dev/null +++ b/internal/lints/default_golangci.go @@ -0,0 +1,43 @@ +package lints + +import ( + "encoding/json" + "fmt" + "go/token" + "os/exec" +) + +type golangciOutput struct { + Issues []struct { + FromLinter string `json:"FromLinter"` + Text string `json:"Text"` + Pos struct { + Filename string `json:"Filename"` + Line int `json:"Line"` + Column int `json:"Column"` + } `json:"Pos"` + } `json:"Issues"` +} + +func RunGolangciLint(filename string) ([]Issue, error) { + cmd := exec.Command("golangci-lint", "run", "--disable=gosimple", "--out-format=json", filename) + output, _ := cmd.CombinedOutput() + + var golangciResult golangciOutput + if err := json.Unmarshal(output, &golangciResult); err != nil { + return nil, fmt.Errorf("error unmarshaling golangci-lint output: %w", err) + } + + var issues []Issue + for _, gi := range golangciResult.Issues { + issues = append(issues, Issue{ + Rule: gi.FromLinter, + Filename: gi.Pos.Filename, // Use the filename from golangci-lint output + Start: token.Position{Filename: gi.Pos.Filename, Line: gi.Pos.Line, Column: gi.Pos.Column}, + End: token.Position{Filename: gi.Pos.Filename, Line: gi.Pos.Line, Column: gi.Pos.Column + 1}, + Message: gi.Text, + }) + } + + return issues, nil +} diff --git a/internal/lint_test.go b/internal/lints/lint_test.go similarity index 94% rename from internal/lint_test.go rename to internal/lints/lint_test.go index ff4b3de..0a5a29f 100644 --- a/internal/lint_test.go +++ b/internal/lints/lint_test.go @@ -1,4 +1,4 @@ -package internal +package lints import ( "os" @@ -77,8 +77,7 @@ func example2() int { err = os.WriteFile(tmpfile, []byte(tt.code), 0o644) require.NoError(t, err) - engine := &Engine{} - issues, err := engine.detectUnnecessaryElse(tmpfile) + issues, err := DetectUnnecessaryElse(tmpfile) require.NoError(t, err) assert.Equal(t, tt.expected, len(issues), "Number of detected unnecessary else statements doesn't match expected") @@ -182,8 +181,7 @@ func unused2() { err = os.WriteFile(tmpfile, []byte(tt.code), 0o644) require.NoError(t, err) - engine := &Engine{} - issues, err := engine.detectUnusedFunctions(tmpfile) + issues, err := DetectUnusedFunctions(tmpfile) require.NoError(t, err) assert.Equal(t, tt.expected, len(issues), "Number of detected unused functions doesn't match expected") @@ -266,8 +264,7 @@ func main() { err = os.WriteFile(tmpfile, []byte(tt.code), 0o644) require.NoError(t, err) - engine := &Engine{} - issues, err := engine.detectUnnecessarySliceLength(tmpfile) + issues, err := DetectUnnecessarySliceLength(tmpfile) require.NoError(t, err) assert.Equal(t, tt.expected, len(issues), "Number of detected unnecessary slice length doesn't match expected") @@ -365,8 +362,7 @@ func example() { err = os.WriteFile(tmpfile, []byte(tt.code), 0o644) require.NoError(t, err) - engine := &Engine{} - issues, err := engine.detectUnnecessaryConversions(tmpfile) + issues, err := DetectUnnecessaryConversions(tmpfile) require.NoError(t, err) assert.Equal(t, tt.expected, len(issues), "Number of detected unnecessary type conversions doesn't match expected") diff --git a/internal/lints/simplify_slice_expr.go b/internal/lints/simplify_slice_expr.go new file mode 100644 index 0000000..5869d96 --- /dev/null +++ b/internal/lints/simplify_slice_expr.go @@ -0,0 +1,68 @@ +package lints + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" +) + +func 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 +} diff --git a/internal/types.go b/internal/lints/types.go similarity index 67% rename from internal/types.go rename to internal/lints/types.go index 37352bd..30400a2 100644 --- a/internal/types.go +++ b/internal/lints/types.go @@ -1,12 +1,7 @@ -package internal +package lints import "go/token" -// SourceCode stores the content of a source code file. -type SourceCode struct { - Lines []string -} - // Issue represents a lint issue found in the code base. type Issue struct { Rule string diff --git a/internal/lints/unncessary_type_conversion.go b/internal/lints/unncessary_type_conversion.go new file mode 100644 index 0000000..f88c39b --- /dev/null +++ b/internal/lints/unncessary_type_conversion.go @@ -0,0 +1,219 @@ +package lints + +import ( + "fmt" + "go/ast" + "go/importer" + "go/parser" + "go/token" + "go/types" +) + +func DetectUnnecessaryConversions(filename string) ([]Issue, error) { + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, filename, nil, parser.ParseComments) + if err != nil { + return nil, err + } + + info := &types.Info{ + Types: make(map[ast.Expr]types.TypeAndValue), + Uses: make(map[*ast.Ident]types.Object), + Defs: make(map[*ast.Ident]types.Object), + } + + conf := types.Config{Importer: importer.Default()} + //! DO NOT CHECK ERROR HERE. + //! error check may broke the lint formatting process. + conf.Check("", fset, []*ast.File{f}, info) + + var issues []Issue + varDecls := make(map[*types.Var]ast.Node) + + // First pass: collect variable declarations + ast.Inspect(f, func(n ast.Node) bool { + switch node := n.(type) { + case *ast.ValueSpec: + for _, name := range node.Names { + if obj := info.Defs[name]; obj != nil { + if v, ok := obj.(*types.Var); ok { + varDecls[v] = node + } + } + } + case *ast.AssignStmt: + for _, lhs := range node.Lhs { + if id, ok := lhs.(*ast.Ident); ok { + if obj := info.Defs[id]; obj != nil { + if v, ok := obj.(*types.Var); ok { + varDecls[v] = node + } + } + } + } + } + return true + }) + + // Second pass: check for unnecessary conversions + ast.Inspect(f, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || len(call.Args) != 1 { + return true + } + + ft, ok := info.Types[call.Fun] + if !ok || !ft.IsType() { + return true + } + + at, ok := info.Types[call.Args[0]] + if !ok { + return true + } + + if types.Identical(ft.Type, at.Type) && !isUntypedValue(call.Args[0], info) { + var memo, suggestion string + + // find parent node and retrieve the entire assignment statement + var parent ast.Node + ast.Inspect(f, func(node ast.Node) bool { + if node == n { + return false + } + if contains(node, n) { + parent = node + return false + } + return true + }) + + if assignStmt, ok := parent.(*ast.AssignStmt); ok { + if len(assignStmt.Lhs) > 0 { + lhs := types.ExprString(assignStmt.Lhs[0]) + rhs := types.ExprString(call.Args[0]) + suggestion = fmt.Sprintf("%s := %s", lhs, rhs) + } + } else { + // not an assignment statement + // keep maintaining the original code + suggestion = types.ExprString(call.Args[0]) + } + + if id, ok := call.Args[0].(*ast.Ident); ok { + if obj, ok := info.Uses[id].(*types.Var); ok { + if _, exists := varDecls[obj]; exists { + declType := obj.Type().String() + memo = fmt.Sprintf( + "The variable '%s' is declared as type '%s'. This type conversion appears unnecessary.", + id.Name, declType, + ) + } + } + } + + issues = append(issues, Issue{ + Rule: "unnecessary-type-conversion", + Filename: filename, + Start: fset.Position(call.Pos()), + End: fset.Position(call.End()), + Message: "unnecessary type conversion", + Suggestion: suggestion, + Note: memo, + }) + } + + return true + }) + + return issues, nil +} + +// ref: https://github.com/mdempsky/unconvert/blob/master/unconvert.go#L570 +func isUntypedValue(n ast.Expr, info *types.Info) (res bool) { + switch n := n.(type) { + case *ast.BinaryExpr: + switch n.Op { + case token.SHL, token.SHR: + // Shifts yield an untyped value if their LHS is untyped. + return isUntypedValue(n.X, info) + case token.EQL, token.NEQ, token.LSS, token.GTR, token.LEQ, token.GEQ: + // Comparisons yield an untyped boolean value. + return true + case token.ADD, token.SUB, token.MUL, token.QUO, token.REM, + token.AND, token.OR, token.XOR, token.AND_NOT, + token.LAND, token.LOR: + return isUntypedValue(n.X, info) && isUntypedValue(n.Y, info) + } + case *ast.UnaryExpr: + switch n.Op { + case token.ADD, token.SUB, token.NOT, token.XOR: + return isUntypedValue(n.X, info) + } + case *ast.BasicLit: + // Basic literals are always untyped. + return true + case *ast.ParenExpr: + return isUntypedValue(n.X, info) + case *ast.SelectorExpr: + return isUntypedValue(n.Sel, info) + case *ast.Ident: + if obj, ok := info.Uses[n]; ok { + if obj.Pkg() == nil && obj.Name() == "nil" { + // The universal untyped zero value. + return true + } + if b, ok := obj.Type().(*types.Basic); ok && b.Info()&types.IsUntyped != 0 { + // Reference to an untyped constant. + return true + } + } + case *ast.CallExpr: + if b, ok := asBuiltin(n.Fun, info); ok { + switch b.Name() { + case "real", "imag": + return isUntypedValue(n.Args[0], info) + case "complex": + return isUntypedValue(n.Args[0], info) && isUntypedValue(n.Args[1], info) + } + } + } + + return false +} + +func asBuiltin(n ast.Expr, info *types.Info) (*types.Builtin, bool) { + for { + paren, ok := n.(*ast.ParenExpr) + if !ok { + break + } + n = paren.X + } + + ident, ok := n.(*ast.Ident) + if !ok { + return nil, false + } + + obj, ok := info.Uses[ident] + if !ok { + return nil, false + } + + b, ok := obj.(*types.Builtin) + return b, ok +} + +// contains checks if parent contains child node +func contains(parent, child ast.Node) bool { + found := false + ast.Inspect(parent, func(n ast.Node) bool { + if n == child { + found = true + return false + } + return true + }) + return found +} diff --git a/internal/lints/unnecessary_else.go b/internal/lints/unnecessary_else.go new file mode 100644 index 0000000..cb2d639 --- /dev/null +++ b/internal/lints/unnecessary_else.go @@ -0,0 +1,47 @@ +package lints + +import ( + "go/ast" + "go/parser" + "go/token" +) + +// DetectUnnecessaryElse detects unnecessary else blocks. +// This rule considers an else block unnecessary if the if block ends with a return statement. +// In such cases, the else block can be removed and the code can be flattened to improve readability. +func DetectUnnecessaryElse(f string) ([]Issue, error) { + fset := token.NewFileSet() + node, err := parser.ParseFile(fset, f, nil, parser.ParseComments) + if err != nil { + return nil, err + } + + var issues []Issue + ast.Inspect(node, func(n ast.Node) bool { + ifStmt, ok := n.(*ast.IfStmt) + if !ok { + return true + } + + if ifStmt.Else != nil { + blockStmt := ifStmt.Body + if len(blockStmt.List) > 0 { + lastStmt := blockStmt.List[len(blockStmt.List)-1] + if _, isReturn := lastStmt.(*ast.ReturnStmt); isReturn { + issue := Issue{ + Rule: "unnecessary-else", + Filename: f, + Start: fset.Position(ifStmt.Else.Pos()), + End: fset.Position(ifStmt.Else.End()), + Message: "unnecessary else block", + } + issues = append(issues, issue) + } + } + } + + return true + }) + + return issues, nil +} diff --git a/internal/lints/unused_function.go b/internal/lints/unused_function.go new file mode 100644 index 0000000..4198608 --- /dev/null +++ b/internal/lints/unused_function.go @@ -0,0 +1,53 @@ +package lints + +import ( + "go/ast" + "go/parser" + "go/token" +) + +// DetectUnusedFunctions detects functions that are declared but never used. +// This rule reports all unused functions except for the following cases: +// 1. The main function: It's considered "used" as it's the entry point of the program. +// 2. The init function: It's used for package initialization and runs without explicit calls. +// 3. Exported functions: Functions starting with a capital letter are excluded as they might be used in other packages. +// +// This rule helps in code cleanup and improves maintainability. +func DetectUnusedFunctions(filename string) ([]Issue, error) { + fset := token.NewFileSet() + node, err := parser.ParseFile(fset, filename, nil, parser.ParseComments) + if err != nil { + return nil, err + } + + declaredFuncs := make(map[string]*ast.FuncDecl) + calledFuncs := make(map[string]bool) + + ast.Inspect(node, func(n ast.Node) bool { + switch x := n.(type) { + case *ast.FuncDecl: + declaredFuncs[x.Name.Name] = x + case *ast.CallExpr: + if ident, ok := x.Fun.(*ast.Ident); ok { + calledFuncs[ident.Name] = true + } + } + return true + }) + + var issues []Issue + for funcName, funcDecl := range declaredFuncs { + if !calledFuncs[funcName] && funcName != "main" && funcName != "init" && !ast.IsExported(funcName) { + issue := Issue{ + Rule: "unused-function", + Filename: filename, + Start: fset.Position(funcDecl.Pos()), + End: fset.Position(funcDecl.End()), + Message: "function " + funcName + " is declared but not used", + } + issues = append(issues, issue) + } + } + + return issues, nil +} diff --git a/internal/rule_set.go b/internal/rule_set.go new file mode 100644 index 0000000..40a2ab7 --- /dev/null +++ b/internal/rule_set.go @@ -0,0 +1,45 @@ +package internal + +import ( + "github.com/gnoswap-labs/lint/internal/lints" +) + +/* +* Implement each lint rule as a separate struct + */ + +// LintRule defines the interface for all lint rules. +type LintRule interface { + // Check runs the lint rule on the given file and returns a slice of Issues. + Check(filename string) ([]lints.Issue, error) +} + +type GolangciLintRule struct{} + +func (r *GolangciLintRule) Check(filename string) ([]lints.Issue, error) { + return lints.RunGolangciLint(filename) +} + +type UnnecessaryElseRule struct{} + +func (r *UnnecessaryElseRule) Check(filename string) ([]lints.Issue, error) { + return lints.DetectUnnecessaryElse(filename) +} + +type UnusedFunctionRule struct{} + +func (r *UnusedFunctionRule) Check(filename string) ([]lints.Issue, error) { + return lints.DetectUnusedFunctions(filename) +} + +type SimplifySliceExprRule struct{} + +func (r *SimplifySliceExprRule) Check(filename string) ([]lints.Issue, error) { + return lints.DetectUnnecessarySliceLength(filename) +} + +type UnnecessaryConversionRule struct{} + +func (r *UnnecessaryConversionRule) Check(filename string) ([]lints.Issue, error) { + return lints.DetectUnnecessaryConversions(filename) +}