Skip to content

Commit

Permalink
fix: handle gno specific path imports
Browse files Browse the repository at this point in the history
  • Loading branch information
notJoon committed Jul 25, 2024
1 parent 66f7295 commit 399d946
Show file tree
Hide file tree
Showing 6 changed files with 250 additions and 5 deletions.
2 changes: 1 addition & 1 deletion internal/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (e *Engine) registerDefaultRules() {
&UnnecessaryConversionRule{},
&LoopAllocationRule{},
&DetectCycleRule{},
// &CyclomaticComplexityRule{Threshold: 10},
&GnoSpecificRule{},
)
}

Expand Down
9 changes: 5 additions & 4 deletions internal/lints/default_golangci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package lints

import (
"encoding/json"
"fmt"
_ "fmt"
"go/token"
"os/exec"

Expand All @@ -26,9 +26,10 @@ func RunGolangciLint(filename string) ([]tt.Issue, error) {
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)
}

// @notJoon: Ignore Unmarshal error. We cannot unmarshal the output of golangci-lint
// when source code contains gno package imports (i.e. p/demo, r/demo, std). [07/25/24]
json.Unmarshal(output, &golangciResult)

var issues []tt.Issue
for _, gi := range golangciResult.Issues {
Expand Down
159 changes: 159 additions & 0 deletions internal/lints/gno_analyzer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package lints

import (
"fmt"
"go/ast"
"go/parser"
"go/token"
"os"
"path/filepath"
"strings"

tt "github.com/gnoswap-labs/lint/internal/types"
)

const (
GNO_PKG_PREFIX = "gno.land/"
GNO_STD_PACKAGE = "std"
)

type Dependency struct {
ImportPath string
IsGno bool
IsUsed bool
}

type (
Dependencies map[string]*Dependency
FileMap map[string]*ast.File
)

type Package struct {
Name string
Files FileMap
}

func DetectGnoPackageImports(filename string) ([]tt.Issue, error) {
dir := filepath.Dir(filename)

pkg, deps, err := analyzePackage(dir)
if err != nil {
return nil, fmt.Errorf("error analyzing package: %w", err)
}

issues := runGnoPackageLinter(pkg, deps)

for i := range issues {
issues[i].Filename = filename
}

return issues, nil
}

func analyzePackage(dir string) (*Package, Dependencies, error) {
pkg := &Package{
Files: make(FileMap),
}
deps := make(Dependencies)

files, err := filepath.Glob(filepath.Join(dir, "*.gno"))
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,
}
}
}
}

// 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
}
}
}
}
return true
})
}

return pkg, deps, nil
}

func runGnoPackageLinter(pkg *Package, 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:
if ident, ok := x.X.(*ast.Ident); ok {
if dep, exists := deps[ident.Name]; exists {
dep.IsUsed = true
}
}
}
return true
})
}

// TODO: if unused package has `_` alias, it should be ignored
// TODO: or throw a warning
for impPath, dep := range deps {
if !dep.IsUsed {
issue := tt.Issue{
Rule: "unused-import",
Message: fmt.Sprintf("unused import: %s", impPath),
}
issues = append(issues, issue)
}
}

return issues
}

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]
}
57 changes: 57 additions & 0 deletions internal/lints/gno_analyzer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package lints

import (
"path/filepath"
"runtime"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestRunLinter(t *testing.T) {
_, current, _, ok := runtime.Caller(0)
require.True(t, ok)

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
}{
{"unused-import", "unused import: strings"},
}

assert.Equal(t, len(expectedIssues), len(issues), "Number of issues doesn't match expected")

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)
}

expectedDeps := map[string]struct {
isGno bool
isUsed bool
}{
"fmt": {false, true},
"gno.land/p/demo/ufmt": {true, true},
"strings": {false, false},
"std": {true, true},
}

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)
}
}
}
9 changes: 9 additions & 0 deletions internal/rule_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,12 @@ type CyclomaticComplexityRule struct {
func (r *CyclomaticComplexityRule) Check(filename string) ([]tt.Issue, error) {
return lints.DetectHighCyclomaticComplexity(filename, r.Threshold)
}

// -----------------------------------------------------------------------------

// GnoSpecificRule checks for gno-specific package imports. (p, r and std)
type GnoSpecificRule struct{}

func (r *GnoSpecificRule) Check(filename string) ([]tt.Issue, error) {
return lints.DetectGnoPackageImports(filename)
}
19 changes: 19 additions & 0 deletions testdata/pkg/pkg0.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package main

import (
"fmt"
"std"
"strings" // unused import

"gno.land/p/demo/ufmt"
)

func main() {
fmt.Println("Hello, World!")
ufmt.Printf("Formatted: %s\n", "test")

std.Emit(
"foo",
"1", "2",
)
}

0 comments on commit 399d946

Please sign in to comment.