From 955cef7e3e8d12c3f17141072ad57f7fec5a447a Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Mon, 16 Dec 2024 12:00:59 +0100 Subject: [PATCH] feat: convert to an Analyzer (#22) * feat: convert to an Analyzer * chore: update GitHub Actions workflow * refactor: move options --- .github/workflows/go.yml | 47 +++++++ .github/workflows/test.yml | 38 ------ cmd/funlen/main.go | 11 ++ funlen.go | 115 ++++++++++++++++ funlen_test.go | 48 +++++++ go.mod | 9 +- go.sum | 8 ++ main.go | 124 ------------------ main_test.go | 105 --------------- .../src/ignores_comments/ignores_comments.go | 17 +++ testdata/src/too_many_lines/too_many_lines.go | 8 ++ .../too_many_statements.go | 6 + .../too_many_statements_inline_func.go | 13 ++ 13 files changed, 281 insertions(+), 268 deletions(-) create mode 100644 .github/workflows/go.yml delete mode 100644 .github/workflows/test.yml create mode 100644 cmd/funlen/main.go create mode 100644 funlen.go create mode 100644 funlen_test.go create mode 100644 go.sum delete mode 100644 main.go delete mode 100644 main_test.go create mode 100644 testdata/src/ignores_comments/ignores_comments.go create mode 100644 testdata/src/too_many_lines/too_many_lines.go create mode 100644 testdata/src/too_many_statements/too_many_statements.go create mode 100644 testdata/src/too_many_statements_inline_func/too_many_statements_inline_func.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml new file mode 100644 index 0000000..7e9814f --- /dev/null +++ b/.github/workflows/go.yml @@ -0,0 +1,47 @@ +name: CI +on: + push: + branches: + - master + - main + pull_request: + +env: + GO_VERSION: stable + +jobs: + golangci-lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + - name: lint + uses: golangci/golangci-lint-action@v6 + with: + version: latest + + tests: + needs: golangci-lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + - name: Run tests + run: go test -v -cover ./... + + build: + needs: golangci-lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + - name: Build + run: go build -v ./cmd/funlen/ + - name: Run + run: go run ./cmd/funlen/ ./... diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml deleted file mode 100644 index 13ce54d..0000000 --- a/.github/workflows/test.yml +++ /dev/null @@ -1,38 +0,0 @@ -name: CI -on: - push: - branches: - - master - - main - pull_request: - -env: - GO_VERSION: '1.20' - -jobs: - golangci-lint: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - name: Install Go - uses: actions/setup-go@v4 - with: - go-version: ${{ env.GO_VERSION }} - - name: lint - uses: golangci/golangci-lint-action@v3.6.0 - with: - version: latest - # skip cache because of flaky behaviors - skip-build-cache: true - skip-pkg-cache: true - tests: - needs: golangci-lint - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - name: Install Go - uses: actions/setup-go@v4 - with: - go-version: ${{ env.GO_VERSION }} - - name: Run tests - run: go test -v -cover ./... diff --git a/cmd/funlen/main.go b/cmd/funlen/main.go new file mode 100644 index 0000000..a588b3f --- /dev/null +++ b/cmd/funlen/main.go @@ -0,0 +1,11 @@ +package main + +import ( + "golang.org/x/tools/go/analysis/singlechecker" + + "github.com/ultraware/funlen" +) + +func main() { + singlechecker.Main(funlen.NewAnalyzer(0, 0, false)) +} diff --git a/funlen.go b/funlen.go new file mode 100644 index 0000000..9838f7c --- /dev/null +++ b/funlen.go @@ -0,0 +1,115 @@ +package funlen + +import ( + "go/ast" + "go/token" + "reflect" + + "golang.org/x/tools/go/analysis" +) + +const ( + defaultLineLimit = 60 + defaultStmtLimit = 40 +) + +func NewAnalyzer(lineLimit int, stmtLimit int, ignoreComments bool) *analysis.Analyzer { + if lineLimit == 0 { + lineLimit = defaultLineLimit + } + + if stmtLimit == 0 { + stmtLimit = defaultStmtLimit + } + + return &analysis.Analyzer{ + Name: "funlen", + Doc: "Checks for long functions.", + URL: "https://github.com/ultraware/funlen", + Run: func(pass *analysis.Pass) (any, error) { + run(pass, lineLimit, stmtLimit, ignoreComments) + return nil, nil + }, + } +} + +func run(pass *analysis.Pass, lineLimit int, stmtLimit int, ignoreComments bool) { + for _, file := range pass.Files { + cmap := ast.NewCommentMap(pass.Fset, file, file.Comments) + + for _, f := range file.Decls { + decl, ok := f.(*ast.FuncDecl) + if !ok || decl.Body == nil { // decl.Body can be nil for e.g. cgo + continue + } + + if stmtLimit > 0 { + if stmts := parseStmts(decl.Body.List); stmts > stmtLimit { + pass.Reportf(decl.Name.Pos(), "Function '%s' has too many statements (%d > %d)", decl.Name.Name, stmts, stmtLimit) + continue + } + } + + if lineLimit > 0 { + if lines := getLines(pass.Fset, decl, cmap.Filter(decl), ignoreComments); lines > lineLimit { + pass.Reportf(decl.Name.Pos(), "Function '%s' is too long (%d > %d)", decl.Name.Name, lines, lineLimit) + } + } + } + } +} + +func getLines(fset *token.FileSet, f *ast.FuncDecl, cmap ast.CommentMap, ignoreComments bool) int { + lineCount := fset.Position(f.End()).Line - fset.Position(f.Pos()).Line - 1 + + if !ignoreComments { + return lineCount + } + + var commentCount int + + for _, c := range cmap.Comments() { + // If the CommentGroup's lines are inside the function + // count how many comments are in the CommentGroup + if (fset.Position(c.Pos()).Line > fset.Position(f.Pos()).Line) && + (fset.Position(c.End()).Line < fset.Position(f.End()).Line) { + commentCount += len(c.List) + } + } + + return lineCount - commentCount +} + +func parseStmts(s []ast.Stmt) (total int) { + for _, v := range s { + total++ + switch stmt := v.(type) { + case *ast.BlockStmt: + total += parseStmts(stmt.List) - 1 + case *ast.ForStmt, *ast.RangeStmt, *ast.IfStmt, + *ast.SwitchStmt, *ast.TypeSwitchStmt, *ast.SelectStmt: + total += parseBodyListStmts(stmt) + case *ast.CaseClause: + total += parseStmts(stmt.Body) + case *ast.AssignStmt: + total += checkInlineFunc(stmt.Rhs[0]) + case *ast.GoStmt: + total += checkInlineFunc(stmt.Call.Fun) + case *ast.DeferStmt: + total += checkInlineFunc(stmt.Call.Fun) + } + } + return +} + +func checkInlineFunc(stmt ast.Expr) int { + if block, ok := stmt.(*ast.FuncLit); ok { + return parseStmts(block.Body.List) + } + return 0 +} + +func parseBodyListStmts(t any) int { + i := reflect.ValueOf(t).Elem().FieldByName(`Body`).Elem().FieldByName(`List`).Interface() + return parseStmts(i.([]ast.Stmt)) +} diff --git a/funlen_test.go b/funlen_test.go new file mode 100644 index 0000000..1af4a20 --- /dev/null +++ b/funlen_test.go @@ -0,0 +1,48 @@ +package funlen + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAnalyzer(t *testing.T) { + testCases := []struct { + dir string + lineLimit int + stmtLimit int + ignoreComments bool + }{ + { + dir: "too_many_statements", + lineLimit: 1, + stmtLimit: 1, + }, + { + dir: "too_many_lines", + lineLimit: 1, + stmtLimit: 10, + }, + { + dir: "too_many_statements_inline_func", + lineLimit: 1, + stmtLimit: 1, + }, + { + dir: "ignores_comments", + lineLimit: 2, + stmtLimit: 2, + ignoreComments: true, + }, + } + + for _, test := range testCases { + t.Run(test.dir, func(t *testing.T) { + t.Parallel() + + a := NewAnalyzer(test.lineLimit, test.stmtLimit, test.ignoreComments) + + analysistest.Run(t, analysistest.TestData(), a, test.dir) + }) + } +} diff --git a/go.mod b/go.mod index 17fa745..8d3eb88 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,10 @@ module github.com/ultraware/funlen -go 1.20 +go 1.22.0 + +require golang.org/x/tools v0.28.0 + +require ( + golang.org/x/mod v0.22.0 // indirect + golang.org/x/sync v0.10.0 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..e3144c2 --- /dev/null +++ b/go.sum @@ -0,0 +1,8 @@ +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4= +golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= +golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ= +golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/tools v0.28.0 h1:WuB6qZ4RPCQo5aP3WdKZS7i595EdWqWR8vqJTlwTVK8= +golang.org/x/tools v0.28.0/go.mod h1:dcIOrVd3mfQKTgrDVQHqCPMWy6lnhfhtX3hLXYVLfRw= diff --git a/main.go b/main.go deleted file mode 100644 index b68ddb9..0000000 --- a/main.go +++ /dev/null @@ -1,124 +0,0 @@ -package funlen - -import ( - "fmt" - "go/ast" - "go/token" - "reflect" -) - -const ( - defaultLineLimit = 60 - defaultStmtLimit = 40 -) - -// Run runs this linter on the provided code -func Run(file *ast.File, fset *token.FileSet, lineLimit int, stmtLimit int, ignoreComments bool) []Message { - if lineLimit == 0 { - lineLimit = defaultLineLimit - } - if stmtLimit == 0 { - stmtLimit = defaultStmtLimit - } - - cmap := ast.NewCommentMap(fset, file, file.Comments) - - var msgs []Message - for _, f := range file.Decls { - decl, ok := f.(*ast.FuncDecl) - if !ok || decl.Body == nil { // decl.Body can be nil for e.g. cgo - continue - } - - if stmtLimit > 0 { - if stmts := parseStmts(decl.Body.List); stmts > stmtLimit { - msgs = append(msgs, makeStmtMessage(fset, decl.Name, stmts, stmtLimit)) - continue - } - } - - if lineLimit > 0 { - if lines := getLines(fset, decl, cmap.Filter(decl), ignoreComments); lines > lineLimit { - msgs = append(msgs, makeLineMessage(fset, decl.Name, lines, lineLimit)) - } - } - } - - return msgs -} - -// Message contains a message -type Message struct { - Pos token.Position - Message string -} - -func makeLineMessage(fset *token.FileSet, funcInfo *ast.Ident, lines, lineLimit int) Message { - return Message{ - fset.Position(funcInfo.Pos()), - fmt.Sprintf("Function '%s' is too long (%d > %d)\n", funcInfo.Name, lines, lineLimit), - } -} - -func makeStmtMessage(fset *token.FileSet, funcInfo *ast.Ident, stmts, stmtLimit int) Message { - return Message{ - fset.Position(funcInfo.Pos()), - fmt.Sprintf("Function '%s' has too many statements (%d > %d)\n", funcInfo.Name, stmts, stmtLimit), - } -} - -func getLines(fset *token.FileSet, f *ast.FuncDecl, cmap ast.CommentMap, ignoreComments bool) int { // nolint: interfacer - var lineCount int - var commentCount int - - lineCount = fset.Position(f.End()).Line - fset.Position(f.Pos()).Line - 1 - - if !ignoreComments { - return lineCount - } - - for _, c := range cmap.Comments() { - // If the CommenGroup's lines are inside the function - // count how many comments are in the CommentGroup - if (fset.Position(c.Pos()).Line > fset.Position(f.Pos()).Line) && - (fset.Position(c.End()).Line < fset.Position(f.End()).Line) { - commentCount += len(c.List) - } - } - - return lineCount - commentCount -} - -func parseStmts(s []ast.Stmt) (total int) { - for _, v := range s { - total++ - switch stmt := v.(type) { - case *ast.BlockStmt: - total += parseStmts(stmt.List) - 1 - case *ast.ForStmt, *ast.RangeStmt, *ast.IfStmt, - *ast.SwitchStmt, *ast.TypeSwitchStmt, *ast.SelectStmt: - total += parseBodyListStmts(stmt) - case *ast.CaseClause: - total += parseStmts(stmt.Body) - case *ast.AssignStmt: - total += checkInlineFunc(stmt.Rhs[0]) - case *ast.GoStmt: - total += checkInlineFunc(stmt.Call.Fun) - case *ast.DeferStmt: - total += checkInlineFunc(stmt.Call.Fun) - } - } - return -} - -func checkInlineFunc(stmt ast.Expr) int { - if block, ok := stmt.(*ast.FuncLit); ok { - return parseStmts(block.Body.List) - } - return 0 -} - -func parseBodyListStmts(t interface{}) int { - i := reflect.ValueOf(t).Elem().FieldByName(`Body`).Elem().FieldByName(`List`).Interface() - return parseStmts(i.([]ast.Stmt)) -} diff --git a/main_test.go b/main_test.go deleted file mode 100644 index 1e0ad9c..0000000 --- a/main_test.go +++ /dev/null @@ -1,105 +0,0 @@ -package funlen - -import ( - "go/parser" - "go/token" - "strings" - "testing" -) - -func TestRunTable(t *testing.T) { - testcases := map[string]struct { - input string - expected string - lineLimit int - stmtLimit int - }{ - "too-many-statements": { - input: `package main - func main() { - print("Hello, world!") - print("Hello, world!")}`, - expected: "Function 'main' has too many statements (2 > 1)", - lineLimit: 1, - stmtLimit: 1, - }, - "too-many-lines": { - input: `package main - import "fmt" - func main() { - print("main!") - print("is!") - print("too!") - print("long")}`, - expected: "Function 'main' is too long (3 > 1)", - lineLimit: 1, - stmtLimit: 10, - }, - "too-many-statements-inline-func": { - input: `package main - func main() { - print("Hello, world!") - if true { - y := []int{1,2,3,4} - for k, v := range y { - f := func() { print("test") } - f() - } - } - print("Hello, world!")}`, - expected: "Function 'main' has too many statements (8 > 1)", - lineLimit: 1, - stmtLimit: 1, - }, - } - - for name, test := range testcases { - t.Run(name, func(t *testing.T) { - fset := token.NewFileSet() - f, err := parser.ParseFile(fset, "", test.input, parser.ParseComments) - - if err != nil { - t.Error("\nActual: ", err, "\n Did not expected error") - } - - r := Run(f, fset, test.lineLimit, test.stmtLimit, false) - actual := r[0].Message - - if !strings.Contains(actual, test.expected) { - t.Error("\nActual: ", actual, "\nExpected: ", test.expected) - } - }) - } -} - -func TestRunIgnoresComments(t *testing.T) { - - input := `package main - func main() { - // Comment 1 - // Comment 2 - // Comment 3 - print("Hello, world!")} - // Comment Doc - func unittest() { - // Comment 1 - // Comment 2 - print("Hello, world!")} - // Comment 3` - - lineLimit := 2 - stmtLimit := 2 - - fset := token.NewFileSet() - f, err := parser.ParseFile(fset, "", input, parser.ParseComments) - - if err != nil { - t.Error("\nActual: ", err, "\n Did not expected error") - } - - r := Run(f, fset, lineLimit, stmtLimit, true) - - if len(r) > 0 { - t.Error("\nActual: ", r, "\nExpected no lint errors") - } -} diff --git a/testdata/src/ignores_comments/ignores_comments.go b/testdata/src/ignores_comments/ignores_comments.go new file mode 100644 index 0000000..1361504 --- /dev/null +++ b/testdata/src/ignores_comments/ignores_comments.go @@ -0,0 +1,17 @@ +package main + +func main() { + // Comment 1 + // Comment 2 + // Comment 3 + print("Hello, world!") +} + +// Comment Doc +func unittest() { + // Comment 1 + // Comment 2 + print("Hello, world!") +} + +// Comment 3 diff --git a/testdata/src/too_many_lines/too_many_lines.go b/testdata/src/too_many_lines/too_many_lines.go new file mode 100644 index 0000000..7afac00 --- /dev/null +++ b/testdata/src/too_many_lines/too_many_lines.go @@ -0,0 +1,8 @@ +package main + +func main() { // want `Function 'main' is too long \(4 > 1\)` + print("main!") + print("is!") + print("too!") + print("long") +} diff --git a/testdata/src/too_many_statements/too_many_statements.go b/testdata/src/too_many_statements/too_many_statements.go new file mode 100644 index 0000000..b6ee8bd --- /dev/null +++ b/testdata/src/too_many_statements/too_many_statements.go @@ -0,0 +1,6 @@ +package main + +func main() { // want `Function 'main' has too many statements \(2 > 1\)` + print("Hello, world!") + print("Hello, world!") +} diff --git a/testdata/src/too_many_statements_inline_func/too_many_statements_inline_func.go b/testdata/src/too_many_statements_inline_func/too_many_statements_inline_func.go new file mode 100644 index 0000000..3f32cd5 --- /dev/null +++ b/testdata/src/too_many_statements_inline_func/too_many_statements_inline_func.go @@ -0,0 +1,13 @@ +package main + +func main() { // want `Function 'main' has too many statements \(8 > 1\)` + print("Hello, world!") + if true { + y := []int{1, 2, 3, 4} + for k, v := range y { + f := func() { print("test", k, v) } + f() + } + } + print("Hello, world!") +}