From 85c7028f24f698436a03f0ce93c4b9b39f3180c5 Mon Sep 17 00:00:00 2001 From: Lee ByeongJun Date: Sat, 27 Jul 2024 17:12:11 +0900 Subject: [PATCH 1/2] refactor main --- .github/workflows/go.yml | 2 +- cmd/tlin/main.go | 262 ++++++++++++++++++--------------------- cmd/tlin/main_test.go | 186 +++++++++++++++++++++++++++ go.mod | 3 + go.sum | 6 + 5 files changed, 319 insertions(+), 140 deletions(-) create mode 100644 cmd/tlin/main_test.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 4050875..9dffd37 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -25,4 +25,4 @@ jobs: run: go build -v ./... - name: Test - run: go test -v ./... \ No newline at end of file + run: go test -race -v ./... \ No newline at end of file diff --git a/cmd/tlin/main.go b/cmd/tlin/main.go index 94cc56d..f44cc09 100644 --- a/cmd/tlin/main.go +++ b/cmd/tlin/main.go @@ -8,71 +8,81 @@ import ( "path/filepath" "sort" "strings" + "sync" "time" "github.com/gnoswap-labs/lint/formatter" "github.com/gnoswap-labs/lint/internal" "github.com/gnoswap-labs/lint/internal/lints" tt "github.com/gnoswap-labs/lint/internal/types" + "go.uber.org/zap" ) const defaultTimeout = 5 * time.Minute +type Config struct { + Timeout time.Duration + CyclomaticComplexity bool + CyclomaticThreshold int + IgnoreRules string + Paths []string +} + +type LintEngine interface { + Run(filePath string) ([]tt.Issue, error) + IgnoreRule(rule string) +} + func main() { - timeout := flag.Duration("timeout", defaultTimeout, "Set a timeout for the linter. example: 1s, 1m, 1h") - // verbose := flag.Bool("verbose", false, "Enable verbose output") - // formatJSON := flag.Bool("json", false, "Output results in JSON format") - cyclomaticComplexity := flag.Bool("cyclo", false, "Run cyclomatic complexity analysis") - // |Cyclomatic Complexity | Risk Evaluation | - // |----------------------|-----------------| - // | 1-10 | Low | - // | 11-20 | Moderate | - // | 21-50 | High | - // | 51+ | Very High | - // - // [*] MaCabe's article recommends 10 or less, but up to 15 is acceptable (by Microsoft). - // [*] https://learn.microsoft.com/en-us/visualstudio/code-quality/code-metrics-cyclomatic-complexity?view=vs-2022 - cyclomaticThreshold := flag.Int("threshold", 10, "Cyclomatic complexity threshold") - ignoreRules := flag.String("ignore", "", "Comma-separated list of lint rules to ignore") + logger, _ := zap.NewProduction() + defer logger.Sync() - flag.Parse() + config := parseFlags() - args := flag.Args() - if len(args) == 0 { - fmt.Println("error: Please provide file or directory paths") - os.Exit(1) - } + ctx, cancel := context.WithTimeout(context.Background(), config.Timeout) + defer cancel() - // TODO: Cache the directory tree to avoid re-traversing the same directories. - rootDir := "." - engine, err := internal.NewEngine(rootDir) + engine, err := internal.NewEngine(".") if err != nil { - fmt.Printf("error initializing lint engine: %v\n", err) - os.Exit(1) + logger.Fatal("Failed to initialize lint engine", zap.Error(err)) } - ctx, cancel := context.WithTimeout(context.Background(), *timeout) - defer cancel() - - if *ignoreRules != "" { - rules := strings.Split(*ignoreRules, ",") + if config.IgnoreRules != "" { + rules := strings.Split(config.IgnoreRules, ",") for _, rule := range rules { engine.IgnoreRule(strings.TrimSpace(rule)) } } - // TODO: We might be use the cached directory result when execute analysis functions. - if *cyclomaticComplexity { + if config.CyclomaticComplexity { runWithTimeout(ctx, func() { - runCyclomaticComplexityAnalysis(args, *cyclomaticThreshold) + runCyclomaticComplexityAnalysis(ctx, logger, config.Paths, config.CyclomaticThreshold) }) } else { runWithTimeout(ctx, func() { - runNormalLintProcess(engine, args) + runNormalLintProcess(ctx, logger, engine, config.Paths) }) } } +func parseFlags() Config { + config := Config{} + flag.DurationVar(&config.Timeout, "timeout", defaultTimeout, "Set a timeout for the linter. example: 1s, 1m, 1h") + flag.BoolVar(&config.CyclomaticComplexity, "cyclo", false, "Run cyclomatic complexity analysis") + flag.IntVar(&config.CyclomaticThreshold, "threshold", 10, "Cyclomatic complexity threshold") + flag.StringVar(&config.IgnoreRules, "ignore", "", "Comma-separated list of lint rules to ignore") + + flag.Parse() + + config.Paths = flag.Args() + if len(config.Paths) == 0 { + fmt.Println("error: Please provide file or directory paths") + os.Exit(1) + } + + return config +} + func runWithTimeout(ctx context.Context, f func()) { done := make(chan struct{}) go func() { @@ -89,115 +99,62 @@ func runWithTimeout(ctx context.Context, f func()) { } } -func runNormalLintProcess(engine *internal.Engine, args []string) { - var allIssues []tt.Issue - for _, path := range args { - info, err := os.Stat(path) - if err != nil { - fmt.Printf("error accessing %s: %v\n", path, err) - continue - } - - if info.IsDir() { - err = filepath.Walk(path, func(filePath string, fileInfo os.FileInfo, err error) error { - if err != nil { - return err - } - if !fileInfo.IsDir() && filepath.Ext(filePath) == ".go" || filepath.Ext(filePath) == ".gno" { - issues, err := processFile(engine, filePath) - if err != nil { - fmt.Printf("error processing %s: %v\n", filePath, err) - } else { - allIssues = append(allIssues, issues...) - } - } - return nil - }) - if err != nil { - fmt.Printf("error walking directory %s: %v\n", path, err) - } - } else { - if filepath.Ext(path) == ".go" || filepath.Ext(path) == ".gno" { - issues, err := processFile(engine, path) - if err != nil { - fmt.Printf("error processing %s: %v\n", path, err) - } else { - allIssues = append(allIssues, issues...) - } - } else { - fmt.Printf("skipping non-.co file: %s\n", path) - } - } +func runNormalLintProcess(ctx context.Context, logger *zap.Logger, engine LintEngine, paths []string) { + issues, err := processFiles(ctx, logger, engine, paths, processFile) + if err != nil { + logger.Error("Error processing files", zap.Error(err)) + os.Exit(1) } - issuesByFile := make(map[string][]tt.Issue) - for _, issue := range allIssues { - issuesByFile[issue.Filename] = append(issuesByFile[issue.Filename], issue) - } + printIssues(logger, issues) - var sortedFiles []string - for filename := range issuesByFile { - sortedFiles = append(sortedFiles, filename) + if len(issues) > 0 { + os.Exit(1) } - sort.Strings(sortedFiles) +} - for _, filename := range sortedFiles { - issues := issuesByFile[filename] - sourceCode, err := internal.ReadSourceCode(filename) - if err != nil { - fmt.Printf("error reading source file %s: %v\n", filename, err) - continue - } - output := formatter.FormatIssuesWithArrows(issues, sourceCode) - fmt.Println(output) +func runCyclomaticComplexityAnalysis(ctx context.Context, logger *zap.Logger, paths []string, threshold int) { + issues, err := processFiles(ctx, logger, nil, paths, func(_ LintEngine, path string) ([]tt.Issue, error) { + return processCyclomaticComplexity(path, threshold) + }) + if err != nil { + logger.Error("Error processing files for cyclomatic complexity", zap.Error(err)) + os.Exit(1) } - if len(allIssues) > 0 { + printIssues(logger, issues) + + if len(issues) > 0 { os.Exit(1) } } -func runCyclomaticComplexityAnalysis(paths []string, threshold int) { +func processFiles(ctx context.Context, logger *zap.Logger, engine LintEngine, paths []string, processor func(LintEngine, string) ([]tt.Issue, error)) ([]tt.Issue, error) { var allIssues []tt.Issue + var mutex sync.Mutex + var wg sync.WaitGroup + for _, path := range paths { - issues, err := processCyclomaticComplexity(path, threshold) - if err != nil { - fmt.Printf("error processing %s: %v\n", path, err) - } else { + wg.Add(1) + go func(p string) { + defer wg.Done() + issues, err := processPath(ctx, logger, engine, p, processor) + if err != nil { + logger.Error("Error processing path", zap.String("path", p), zap.Error(err)) + return + } + mutex.Lock() allIssues = append(allIssues, issues...) - } + mutex.Unlock() + }(path) } - issuesByFile := make(map[string][]tt.Issue) - for _, issue := range allIssues { - issuesByFile[issue.Filename] = append(issuesByFile[issue.Filename], issue) - } + wg.Wait() - // sorted by file name - var sortedFiles []string - for filename := range issuesByFile { - sortedFiles = append(sortedFiles, filename) - } - sort.Strings(sortedFiles) - - // apply formatting - for _, filename := range sortedFiles { - issues := issuesByFile[filename] - sourceCode, err := internal.ReadSourceCode(filename) - if err != nil { - fmt.Printf("error reading source file %s: %v\n", filename, err) - continue - } - output := formatter.FormatIssuesWithArrows(issues, sourceCode) - fmt.Println(output) - } - - if len(allIssues) > 0 { - os.Exit(1) - } + return allIssues, nil } -func processCyclomaticComplexity(path string, threshold int) ([]tt.Issue, error) { +func processPath(ctx context.Context, logger *zap.Logger, engine LintEngine, path string, processor func(LintEngine, string) ([]tt.Issue, error)) ([]tt.Issue, error) { info, err := os.Stat(path) if err != nil { return nil, fmt.Errorf("error accessing %s: %w", path, err) @@ -209,38 +166,65 @@ func processCyclomaticComplexity(path string, threshold int) ([]tt.Issue, error) if err != nil { return err } - if !fileInfo.IsDir() && hasDesiredExtension(filePath) { - fileIssue, err := lints.DetectHighCyclomaticComplexity(filePath, threshold) - if err != nil { - fmt.Printf("error processing %s: %v\n", filePath, err) - } else { - issues = append(issues, fileIssue...) + select { + case <-ctx.Done(): + return ctx.Err() + default: + fileIssues, err := processor(engine, filePath) + if err != nil { + logger.Error("Error processing file", zap.String("file", filePath), zap.Error(err)) + } else { + issues = append(issues, fileIssues...) + } } } return nil }) - if err != nil { return nil, fmt.Errorf("error walking directory %s: %w", path, err) } } else if hasDesiredExtension(path) { - fileIssue, err := lints.DetectHighCyclomaticComplexity(path, threshold) + fileIssues, err := processor(engine, path) if err != nil { return nil, err } - issues = append(issues, fileIssue...) + issues = append(issues, fileIssues...) } return issues, nil } -func processFile(engine *internal.Engine, filePath string) ([]tt.Issue, error) { - issues, err := engine.Run(filePath) - if err != nil { - return nil, fmt.Errorf("error linting %s: %w", filePath, err) +func processFile(engine LintEngine, filePath string) ([]tt.Issue, error) { + return engine.Run(filePath) +} + +func processCyclomaticComplexity(path string, threshold int) ([]tt.Issue, error) { + return lints.DetectHighCyclomaticComplexity(path, threshold) +} + +func printIssues(logger *zap.Logger, issues []tt.Issue) { + issuesByFile := make(map[string][]tt.Issue) + for _, issue := range issues { + issuesByFile[issue.Filename] = append(issuesByFile[issue.Filename], issue) + } + + var sortedFiles []string + for filename := range issuesByFile { + sortedFiles = append(sortedFiles, filename) + } + sort.Strings(sortedFiles) + + for _, filename := range sortedFiles { + fileIssues := issuesByFile[filename] + sourceCode, err := internal.ReadSourceCode(filename) + if err != nil { + logger.Error("Error reading source file", zap.String("file", filename), zap.Error(err)) + continue + } + output := formatter.FormatIssuesWithArrows(fileIssues, sourceCode) + fmt.Println(output) } - return issues, nil } func hasDesiredExtension(path string) bool { diff --git a/cmd/tlin/main_test.go b/cmd/tlin/main_test.go new file mode 100644 index 0000000..848e157 --- /dev/null +++ b/cmd/tlin/main_test.go @@ -0,0 +1,186 @@ +package main + +import ( + "context" + "go/token" + "os" + "path/filepath" + "testing" + "time" + + "github.com/gnoswap-labs/lint/internal/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "go.uber.org/zap" +) + +// MockLintEngine is a mock implementation of the LintEngine interface +type MockLintEngine struct { + mock.Mock +} + +func (m *MockLintEngine) Run(filePath string) ([]types.Issue, error) { + args := m.Called(filePath) + return args.Get(0).([]types.Issue), args.Error(1) +} + +func (m *MockLintEngine) IgnoreRule(rule string) { + m.Called(rule) +} + +func TestParseFlags(t *testing.T) { + // Save original os.Args + oldArgs := os.Args + defer func() { os.Args = oldArgs }() + + // Test case + os.Args = []string{"cmd", "-timeout", "10s", "-cyclo", "-threshold", "15", "-ignore", "rule1,rule2", "file1.go", "file2.go"} + + config := parseFlags() + + assert.Equal(t, 10*time.Second, config.Timeout) + assert.True(t, config.CyclomaticComplexity) + assert.Equal(t, 15, config.CyclomaticThreshold) + assert.Equal(t, "rule1,rule2", config.IgnoreRules) + assert.Equal(t, []string{"file1.go", "file2.go"}, config.Paths) +} + +func TestProcessFile(t *testing.T) { + mockEngine := new(MockLintEngine) + expectedIssues := []types.Issue{ + { + Rule: "test-rule", + Filename: "test.go", + Start: token.Position{Filename: "test.go", Offset: 0, Line: 1, Column: 1}, + End: token.Position{Filename: "test.go", Offset: 10, Line: 1, Column: 11}, + Message: "Test issue", + }, + } + mockEngine.On("Run", "test.go").Return(expectedIssues, nil) + + issues, err := processFile(mockEngine, "test.go") + + assert.NoError(t, err) + assert.Equal(t, expectedIssues, issues) + mockEngine.AssertExpectations(t) +} + +func TestProcessPath(t *testing.T) { + logger, _ := zap.NewProduction() + mockEngine := new(MockLintEngine) + ctx := context.Background() + + tempDir, err := os.MkdirTemp("", "test") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + + testFile1 := filepath.Join(tempDir, "test1.go") + testFile2 := filepath.Join(tempDir, "test2.go") + _, err = os.Create(testFile1) + assert.NoError(t, err) + _, err = os.Create(testFile2) + assert.NoError(t, err) + + expectedIssues1 := []types.Issue{ + { + Rule: "rule1", + Filename: testFile1, + Start: token.Position{Filename: testFile1, Offset: 0, Line: 1, Column: 1}, + End: token.Position{Filename: testFile1, Offset: 10, Line: 1, Column: 11}, + Message: "Test issue 1", + }, + } + expectedIssues2 := []types.Issue{ + { + Rule: "rule2", + Filename: testFile2, + Start: token.Position{Filename: testFile2, Offset: 0, Line: 1, Column: 1}, + End: token.Position{Filename: testFile2, Offset: 10, Line: 1, Column: 11}, + Message: "Test issue 2", + }, + } + + mockEngine.On("Run", testFile1).Return(expectedIssues1, nil) + mockEngine.On("Run", testFile2).Return(expectedIssues2, nil) + + issues, err := processPath(ctx, logger, mockEngine, tempDir, processFile) + + assert.NoError(t, err) + assert.Len(t, issues, 2) + assert.Contains(t, issues, expectedIssues1[0]) + assert.Contains(t, issues, expectedIssues2[0]) + mockEngine.AssertExpectations(t) +} + +func TestProcessFiles(t *testing.T) { + logger, _ := zap.NewProduction() + mockEngine := new(MockLintEngine) + ctx := context.Background() + + tempFile1, err := os.CreateTemp("", "test1*.go") + assert.NoError(t, err) + defer os.Remove(tempFile1.Name()) + + tempFile2, err := os.CreateTemp("", "test2*.go") + assert.NoError(t, err) + defer os.Remove(tempFile2.Name()) + + paths := []string{tempFile1.Name(), tempFile2.Name()} + + expectedIssues1 := []types.Issue{ + { + Rule: "rule1", + Filename: tempFile1.Name(), + Start: token.Position{Filename: tempFile1.Name(), Offset: 0, Line: 1, Column: 1}, + End: token.Position{Filename: tempFile1.Name(), Offset: 10, Line: 1, Column: 11}, + Message: "Test issue 1", + }, + } + expectedIssues2 := []types.Issue{ + { + Rule: "rule2", + Filename: tempFile2.Name(), + Start: token.Position{Filename: tempFile2.Name(), Offset: 0, Line: 1, Column: 1}, + End: token.Position{Filename: tempFile2.Name(), Offset: 10, Line: 1, Column: 11}, + Message: "Test issue 2", + }, + } + + mockEngine.On("Run", tempFile1.Name()).Return(expectedIssues1, nil) + mockEngine.On("Run", tempFile2.Name()).Return(expectedIssues2, nil) + + issues, err := processFiles(ctx, logger, mockEngine, paths, processFile) + + assert.NoError(t, err) + assert.Len(t, issues, 2) + assert.Contains(t, issues, expectedIssues1[0]) + assert.Contains(t, issues, expectedIssues2[0]) + mockEngine.AssertExpectations(t) +} + +func TestHasDesiredExtension(t *testing.T) { + assert.True(t, hasDesiredExtension("test.go")) + assert.True(t, hasDesiredExtension("test.gno")) + assert.False(t, hasDesiredExtension("test.txt")) + assert.False(t, hasDesiredExtension("test")) +} + +func TestRunWithTimeout(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + done := make(chan bool) + go func() { + runWithTimeout(ctx, func() { + time.Sleep(1 * time.Second) + done <- true + }) + }() + + select { + case <-done: + // Function completed successfully + case <-ctx.Done(): + t.Fatal("Function timed out unexpectedly") + } +} diff --git a/go.mod b/go.mod index f38f64b..40f3bfc 100644 --- a/go.mod +++ b/go.mod @@ -11,11 +11,14 @@ require ( require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.20 // indirect + github.com/stretchr/objx v0.5.2 // indirect + go.uber.org/multierr v1.10.0 // indirect golang.org/x/sys v0.22.0 // indirect ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + go.uber.org/zap v1.27.0 gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 271bf6a..5e97598 100644 --- a/go.sum +++ b/go.sum @@ -11,8 +11,14 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= +go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= +go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= +go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= From 4935c7c520424ce8607dabd16b21e71e0e76929c Mon Sep 17 00:00:00 2001 From: Lee ByeongJun Date: Sat, 27 Jul 2024 17:13:53 +0900 Subject: [PATCH 2/2] update README --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index d34d63b..a5fc408 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,13 @@ High cyclomatic complexity can make code harder to understand and maintain. Gene To run this analysis, use the `-cyclo` flag along with an optional `-threshold` flag to set the [complexity threshold](https://learn.microsoft.com/en-us/visualstudio/code-quality/code-metrics-cyclomatic-complexity?view=vs-2022). The default threshold is 10. +|Cyclomatic Complexity | Risk Evaluation | +|----------------------|-----------------| +|1-10 | Low | +|11-20 | Moderate | +|21-50 | High | +|>50 | Very High | + ```bash tlin -cyclo -threshold ```