diff --git a/formatter/general.go b/formatter/general.go index 68e30d7..f8cf121 100644 --- a/formatter/general.go +++ b/formatter/general.go @@ -43,28 +43,19 @@ func (f *GeneralIssueFormatter) Format( 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)) - if len(snippet.Lines) > 0 { - line := expandTabs(snippet.Lines[lineIndex]) - result.WriteString(lineStyle.Sprintf("%d | ", issue.Start.Line)) - result.WriteString(line + "\n") + line := expandTabs(snippet.Lines[lineIndex]) + 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)) - } else { - result.WriteString(messageStyle.Sprintf("Unable to display line. File might be empty.\n")) - result.WriteString(messageStyle.Sprintf("Issue: %s\n\n", issue.Message)) - } + 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)) + } else { + result.WriteString(messageStyle.Sprintf("Unable to display line. File might be empty.\n")) + result.WriteString(messageStyle.Sprintf("Issue: %s\n\n", issue.Message)) + } return result.String() } diff --git a/internal/engine.go b/internal/engine.go index e6c1f16..3f528b5 100644 --- a/internal/engine.go +++ b/internal/engine.go @@ -11,9 +11,9 @@ import ( // Engine manages the linting process. type Engine struct { - SymbolTable *SymbolTable - rules []LintRule - ignoredRules map[string]bool + SymbolTable *SymbolTable + rules []LintRule + ignoredRules map[string]bool } // NewEngine creates a new lint engine. diff --git a/internal/lints/gno_analyzer.go b/internal/lints/gno_analyzer.go index 28e28ea..fdcdc2f 100644 --- a/internal/lints/gno_analyzer.go +++ b/internal/lints/gno_analyzer.go @@ -6,7 +6,6 @@ import ( "go/parser" "go/token" "os" - "path/filepath" "strings" tt "github.com/gnoswap-labs/lint/internal/types" @@ -17,38 +16,22 @@ const ( GNO_STD_PACKAGE = "std" ) -// Dependency represents an imported package and its usage status. type Dependency struct { ImportPath string IsGno bool IsUsed bool - IsIgnored bool // alias with `_` should be ignored + IsIgnored bool // aliased as `_` } -type ( - // Dependencies is a map of import paths to their Dependency information. - Dependencies map[string]*Dependency +type Dependencies map[string]*Dependency - // FileMap is a map of filenames to their parsed AST representation. - FileMap map[string]*ast.File -) - -// Package represents a Go/Gno package with its name and files. -type Package struct { - Name string - Files FileMap -} - -// DetectGnoPackageImports analyzes the given file for Gno package imports and returns any issues found. func DetectGnoPackageImports(filename string) ([]tt.Issue, error) { - dir := filepath.Dir(filename) - - pkg, deps, err := analyzePackage(dir) + file, deps, err := analyzeFile(filename) if err != nil { - return nil, fmt.Errorf("error analyzing package: %w", err) + return nil, fmt.Errorf("error analyzing file: %w", err) } - issues := runGnoPackageLinter(pkg, deps) + issues := runGnoPackageLinter(file, deps) for i := range issues { issues[i].Filename = filename @@ -57,83 +40,52 @@ func DetectGnoPackageImports(filename string) ([]tt.Issue, error) { return issues, nil } -// parses all gno files and collect their imports and usage. -func analyzePackage(dir string) (*Package, Dependencies, error) { - pkg := &Package{ - Files: make(FileMap), +func analyzeFile(filename string) (*ast.File, Dependencies, error) { + content, err := os.ReadFile(filename) + if err != nil { + return nil, nil, err } - deps := make(Dependencies) - files, err := filepath.Glob(filepath.Join(dir, "*.gno")) + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, filename, content, parser.ParseComments) if err != nil { return nil, nil, err } - // 1. Parse all file contents and collect dependencies - for _, file := range files { - f, err := parseFile(file) - if err != nil { - return nil, nil, err - } - - pkg.Files[file] = f - if pkg.Name == "" { - pkg.Name = f.Name.Name - } - - for _, imp := range f.Imports { - impPath := strings.Trim(imp.Path.Value, `"`) - if _, exists := deps[impPath]; !exists { - deps[impPath] = &Dependency{ - ImportPath: impPath, - IsGno: isGnoPackage(impPath), - IsUsed: false, - IsIgnored: imp.Name != nil && imp.Name.Name == "_", - } - } + deps := make(Dependencies) + for _, imp := range file.Imports { + impPath := strings.Trim(imp.Path.Value, `"`) + deps[impPath] = &Dependency{ + ImportPath: impPath, + IsGno: isGnoPackage(impPath), + IsUsed: false, + IsIgnored: imp.Name != nil && imp.Name.Name == "_", } } - // 2. Determine which dependencies are used - for _, file := range pkg.Files { - ast.Inspect(file, func(n ast.Node) bool { - switch x := n.(type) { - case *ast.SelectorExpr: - if ident, ok := x.X.(*ast.Ident); ok { - for _, imp := range file.Imports { - if imp.Name != nil && imp.Name.Name == ident.Name { - deps[strings.Trim(imp.Path.Value, `"`)].IsUsed = true - } else if lastPart := getLastPart(strings.Trim(imp.Path.Value, `"`)); lastPart == ident.Name { - deps[strings.Trim(imp.Path.Value, `"`)].IsUsed = true - } + // Determine which dependencies are used in this file + ast.Inspect(file, func(n ast.Node) bool { + switch x := n.(type) { + case *ast.SelectorExpr: + if ident, ok := x.X.(*ast.Ident); ok { + for _, imp := range file.Imports { + if imp.Name != nil && imp.Name.Name == ident.Name { + deps[strings.Trim(imp.Path.Value, `"`)].IsUsed = true + } else if lastPart := getLastPart(strings.Trim(imp.Path.Value, `"`)); lastPart == ident.Name { + deps[strings.Trim(imp.Path.Value, `"`)].IsUsed = true } } } - return true - }) - } + } + return true + }) - return pkg, deps, nil + return file, deps, nil } -func runGnoPackageLinter(pkg *Package, deps Dependencies) []tt.Issue { +func runGnoPackageLinter(_ *ast.File, deps Dependencies) []tt.Issue { var issues []tt.Issue - for _, file := range pkg.Files { - ast.Inspect(file, func(n ast.Node) bool { - switch x := n.(type) { - case *ast.SelectorExpr: - // check unused imports - if ident, ok := x.X.(*ast.Ident); ok { - if dep, exists := deps[ident.Name]; exists { - dep.IsUsed = true - } - } - } - return true - }) - } - for impPath, dep := range deps { if !dep.IsUsed && !dep.IsIgnored { issue := tt.Issue{ @@ -151,16 +103,6 @@ func isGnoPackage(importPath string) bool { return strings.HasPrefix(importPath, GNO_PKG_PREFIX) || importPath == GNO_STD_PACKAGE } -func parseFile(filename string) (*ast.File, error) { - content, err := os.ReadFile(filename) - if err != nil { - return nil, err - } - - fset := token.NewFileSet() - return parser.ParseFile(fset, filename, content, parser.ParseComments) -} - func getLastPart(path string) string { parts := strings.Split(path, "/") return parts[len(parts)-1] diff --git a/internal/lints/gno_analyzer_test.go b/internal/lints/gno_analyzer_test.go index 6aadc18..69ba337 100644 --- a/internal/lints/gno_analyzer_test.go +++ b/internal/lints/gno_analyzer_test.go @@ -15,46 +15,78 @@ func TestRunLinter(t *testing.T) { testDir := filepath.Join(filepath.Dir(current), "..", "..", "testdata", "pkg") - pkg, deps, err := analyzePackage(testDir) - require.NoError(t, err) - require.NotNil(t, pkg) - require.NotNil(t, deps) - - issues := runGnoPackageLinter(pkg, deps) - - expectedIssues := []struct { - rule string - message string + tests := []struct { + filename string + expectedIssues []struct { + rule string + message string + } + expectedDeps map[string]struct { + isGno bool + isUsed bool + isIgnored bool + } }{ - {"unused-import", "unused import: strings"}, + { + filename: filepath.Join(testDir, "pkg0.gno"), + expectedIssues: []struct { + rule string + message string + }{ + {"unused-import", "unused import: strings"}, + }, + expectedDeps: map[string]struct { + isGno bool + isUsed bool + isIgnored bool + }{ + "fmt": {false, true, false}, + "gno.land/p/demo/ufmt": {true, true, false}, + "strings": {false, false, false}, + "std": {true, true, false}, + "gno.land/p/demo/diff": {true, false, true}, + }, + }, + { + filename: filepath.Join(testDir, "pkg1.gno"), + expectedIssues: []struct { + rule string + message string + }{}, + expectedDeps: map[string]struct { + isGno bool + isUsed bool + isIgnored bool + }{ + "strings": {false, true, false}, + }, + }, } - assert.Equal(t, len(expectedIssues), len(issues), "Number of issues doesn't match expected") + for _, tc := range tests { + t.Run(filepath.Base(tc.filename), func(t *testing.T) { + file, deps, err := analyzeFile(tc.filename) + require.NoError(t, err) + require.NotNil(t, file) - for i, expected := range expectedIssues { - assert.Equal(t, expected.rule, issues[i].Rule, "Rule doesn't match for issue %d", i) - assert.Contains(t, issues[i].Message, expected.message, "Message doesn't match for issue %d", i) - } + issues := runGnoPackageLinter(file, deps) - expectedDeps := map[string]struct { - isGno bool - isUsed bool - isIgnored bool - }{ - "fmt": {false, true, false}, - "gno.land/p/demo/ufmt": {true, true, false}, - "strings": {false, false, false}, - "std": {true, true, false}, - "gno.land/p/demo/diff": {true, false, true}, - } + assert.Equal(t, len(tc.expectedIssues), len(issues), "Number of issues doesn't match expected for %s", tc.filename) - for importPath, expected := range expectedDeps { - dep, exists := deps[importPath] - assert.True(t, exists, "Dependency %s not found", importPath) - if exists { - assert.Equal(t, expected.isGno, dep.IsGno, "IsGno mismatch for %s", importPath) - assert.Equal(t, expected.isUsed, dep.IsUsed, "IsUsed mismatch for %s", importPath) - assert.Equal(t, expected.isIgnored, dep.IsIgnored, "IsIgnored mismatch for %s", importPath) - } + for i, expected := range tc.expectedIssues { + assert.Equal(t, expected.rule, issues[i].Rule, "Rule doesn't match for issue %d in %s", i, tc.filename) + assert.Contains(t, issues[i].Message, expected.message, "Message doesn't match for issue %d in %s", i, tc.filename) + } + + for importPath, expected := range tc.expectedDeps { + dep, exists := deps[importPath] + assert.True(t, exists, "Dependency %s not found in %s", importPath, tc.filename) + if exists { + assert.Equal(t, expected.isGno, dep.IsGno, "IsGno mismatch for %s in %s", importPath, tc.filename) + assert.Equal(t, expected.isUsed, dep.IsUsed, "IsUsed mismatch for %s in %s", importPath, tc.filename) + assert.Equal(t, expected.isIgnored, dep.IsIgnored, "IsIgnored mismatch for %s in %s", importPath, tc.filename) + } + } + }) } } diff --git a/internal/lints/loop_allocation.go b/internal/lints/loop_allocation.go index a32bb54..1bd185a 100644 --- a/internal/lints/loop_allocation.go +++ b/internal/lints/loop_allocation.go @@ -25,7 +25,7 @@ func DetectLoopAllocation(filename string) ([]tt.Issue, error) { case *ast.CallExpr: if isAllocationFunction(innerNode) { issues = append(issues, tt.Issue{ - Rule: "loop-allocation", + Rule: "loop-allocation", Message: "Potential unnecessary allocation inside loop", Start: fset.Position(innerNode.Pos()), End: fset.Position(innerNode.End()), diff --git a/testdata/pkg/pkg1.gno b/testdata/pkg/pkg1.gno new file mode 100644 index 0000000..6bc2f8f --- /dev/null +++ b/testdata/pkg/pkg1.gno @@ -0,0 +1,9 @@ +package main + +import ( + "strings" +) + +func main() { + strings.Contains("foo", "o") +} \ No newline at end of file