Skip to content

Commit

Permalink
fix: don't ignore files without valid package clause
Browse files Browse the repository at this point in the history
Signed-off-by: Norman <[email protected]>
  • Loading branch information
n0izn0iz committed Jan 8, 2025
1 parent c5754b5 commit a73ba04
Show file tree
Hide file tree
Showing 10 changed files with 118 additions and 66 deletions.
34 changes: 16 additions & 18 deletions gnovm/cmd/gno/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ func (i lintIssue) String() string {
location, err = filepath.Rel(wd, i.Location)
if err != nil {
location = i.Location
} else {
location = fmt.Sprintf(".%c%s", filepath.Separator, filepath.Join(location))
}
}
// TODO: consider crafting a doc URL based on Code.
Expand Down Expand Up @@ -133,7 +131,7 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error {
}

// Check if 'gno.mod' exists
if pkg.Root == "" {
if pkg.Root == "" && pkg.ImportPath != "command-line-arguments" {
issue := lintIssue{
Code: lintGnoMod,
Confidence: 1,
Expand All @@ -158,7 +156,7 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error {

// read mempkg
memPkgPath := pkg.ImportPath
if memPkgPath == "" {
if memPkgPath == "" || memPkgPath == "command-line-arguments" {
memPkgPath = pkg.Dir
}
memPkg, err := gno.ReadMemPackage(pkg.Dir, memPkgPath)
Expand Down Expand Up @@ -278,12 +276,12 @@ func lintTestFiles(memPkg *gnovm.MemPackage) *gno.FileSet {
return testfiles
}

func guessSourcePath(pkg, source string) string {
if info, err := os.Stat(pkg); !os.IsNotExist(err) && !info.IsDir() {
pkg = filepath.Dir(pkg)
func guessSourcePath(pkgDir, source string) string {
if info, err := os.Stat(pkgDir); !os.IsNotExist(err) && !info.IsDir() {
pkgDir = filepath.Dir(pkgDir)
}

sourceJoin := filepath.Join(pkg, source)
sourceJoin := filepath.Join(pkgDir, source)
if _, err := os.Stat(sourceJoin); !os.IsNotExist(err) {
return filepath.Clean(sourceJoin)
}
Expand All @@ -292,15 +290,15 @@ func guessSourcePath(pkg, source string) string {
return filepath.Clean(source)
}

return filepath.Clean(pkg)
return filepath.Clean(pkgDir)
}

// reParseRecover is a regex designed to parse error details from a string.
// It extracts the file location, line number, and error message from a formatted error string.
// XXX: Ideally, error handling should encapsulate location details within a dedicated error type.
var reParseRecover = regexp.MustCompile(`^([^:]+)((?::(?:\d+)){1,2}):? *(.*)$`)

func catchRuntimeError(pkgPath string, stderr goio.WriteCloser, action func()) (hasError bool) {
func catchRuntimeError(pkgDir string, stderr goio.WriteCloser, action func()) (hasError bool) {
defer func() {
// Errors catched here mostly come from: gnovm/pkg/gnolang/preprocess.go
r := recover()
Expand All @@ -311,21 +309,21 @@ func catchRuntimeError(pkgPath string, stderr goio.WriteCloser, action func()) (
switch verr := r.(type) {
case *gno.PreprocessError:
err := verr.Unwrap()
fmt.Fprintln(stderr, issueFromError(pkgPath, err).String())
fmt.Fprintln(stderr, issueFromError(pkgDir, err).String())
case error:
errors := multierr.Errors(verr)
for _, err := range errors {
errList, ok := err.(scanner.ErrorList)
if ok {
for _, errorInList := range errList {
fmt.Fprintln(stderr, issueFromError(pkgPath, errorInList).String())
fmt.Fprintln(stderr, issueFromError(pkgDir, errorInList).String())
}
} else {
fmt.Fprintln(stderr, issueFromError(pkgPath, err).String())
fmt.Fprintln(stderr, issueFromError(pkgDir, err).String())
}
}
case string:
fmt.Fprintln(stderr, issueFromError(pkgPath, errors.New(verr)).String())
fmt.Fprintln(stderr, issueFromError(pkgDir, errors.New(verr)).String())
default:
panic(r)
}
Expand All @@ -335,21 +333,21 @@ func catchRuntimeError(pkgPath string, stderr goio.WriteCloser, action func()) (
return
}

func issueFromError(pkgPath string, err error) lintIssue {
func issueFromError(pkgDir string, err error) lintIssue {
var issue lintIssue
issue.Confidence = 1
issue.Code = lintGnoError

parsedError := strings.TrimSpace(err.Error())
parsedError = strings.TrimPrefix(parsedError, pkgPath+"/")
parsedError = strings.TrimPrefix(parsedError, pkgDir+"/")

matches := reParseRecover.FindStringSubmatch(parsedError)
if len(matches) > 0 {
sourcepath := guessSourcePath(pkgPath, matches[1])
sourcepath := guessSourcePath(pkgDir, matches[1])
issue.Location = sourcepath + matches[2]
issue.Msg = strings.TrimSpace(matches[3])
} else {
issue.Location = fmt.Sprintf("%s:0", filepath.Clean(pkgPath))
issue.Location = fmt.Sprintf("%s:0", filepath.Clean(pkgDir))
issue.Msg = err.Error()
}
return issue
Expand Down
4 changes: 2 additions & 2 deletions gnovm/cmd/gno/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ func TestLintApp(t *testing.T) {
errShouldBe: "flag: help requested",
}, {
args: []string{"lint", "../../tests/integ/run_main/"},
stderrShouldContain: "./../../tests/integ/run_main: gno.mod file not found in current or any parent directory (code=1)",
stderrShouldContain: "../../tests/integ/run_main: gno.mod file not found in current or any parent directory (code=1)",
errShouldBe: "exit code: 1",
}, {
args: []string{"lint", "../../tests/integ/undefined_variable_test/undefined_variables_test.gno"},
stderrShouldContain: "../../tests/integ/undefined_variable_test:6:28: name toto not declared (code=2)",
stderrShouldContain: "../../tests/integ/undefined_variable_test/undefined_variables_test.gno:6:28: name toto not declared (code=2)",
errShouldBe: "exit code: 1",
}, {
args: []string{"lint", "../../tests/integ/package_not_declared/main.gno"},
Expand Down
4 changes: 2 additions & 2 deletions gnovm/cmd/gno/mod.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,13 +364,13 @@ func getImportToFilesMap(pkgPath string) (map[string][]string, error) {
if err != nil {
return nil, err
}
imports, err := packages.FileImports(filename, string(data), nil)
imports, err := packages.FileImportsSpecs(filename, string(data), nil)
if err != nil {
return nil, err
}

for _, imp := range imports {
m[imp] = append(m[imp], filename)
m[imp.Path.Value] = append(m[imp.Path.Value], filename)
}
}
return m, nil
Expand Down
8 changes: 6 additions & 2 deletions gnovm/cmd/gno/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,14 @@ func execTest(cfg *testCfg, args []string, io commands.IO) error {
}
packages.Inject(pkgsMap, deps)

memPkg := gno.MustReadMemPackage(pkg.Dir, logName)
memPkg, err := gno.ReadMemPackage(pkg.Dir, logName)
if err != nil {
io.ErrPrintln(err)
return nil
}

startedAt := time.Now()
hasError := catchRuntimeError(logName, io.Err(), func() {
hasError := catchRuntimeError(pkg.Dir, io.Err(), func() {
err = test.Test(memPkg, pkg.Dir, opts)
})

Expand Down
2 changes: 1 addition & 1 deletion gnovm/cmd/gno/testdata/lint/no_gnomod.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ func main() {

-- stdout.golden --
-- stderr.golden --
.: missing 'gno.mod' file (code=1).
.: gno.mod file not found in current or any parent directory (code=1)
22 changes: 8 additions & 14 deletions gnovm/pkg/packages/analyze_packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package packages
import (
"errors"
"fmt"
"go/parser"
"go/token"
"os"
"path"
Expand Down Expand Up @@ -43,20 +42,22 @@ func readCLAPkg(patterns []string, fset *token.FileSet) (*Package, error) {
Files: make(FilesMap),
Imports: make(ImportsMap),
}
var err error

files := []string{}
for _, match := range patterns {
dir := filepath.Dir(match)
dir, base := filepath.Split(match)
dir, err = filepath.Abs(dir)
if err != nil {
return nil, err
}
if pkg.Dir == "" {
pkg.Dir = dir
} else if dir != pkg.Dir {
return nil, fmt.Errorf("named files must all be in one directory; have %s and %s", pkg.Dir, dir)
}
abs, err := filepath.Abs(match)
if err != nil {
return nil, fmt.Errorf("%s: %w", match, err)
}
files = append(files, abs)

files = append(files, filepath.Join(dir, base))
}

return readPkgFiles(pkg, files, fset), nil
Expand Down Expand Up @@ -117,13 +118,6 @@ func readPkgFiles(pkg *Package, files []string, fset *token.FileSet) *Package {
continue
}

// ignore files with invalid package clause
_, err = parser.ParseFile(fset, fpath, nil, parser.PackageClauseOnly)
if err != nil {
pkg.Errors = append(pkg.Errors, err)
continue
}

mempkg.Files = append(mempkg.Files, &gnovm.MemFile{Name: base, Body: body})
pkg.Files[fileKind] = append(pkg.Files[fileKind], base)
}
Expand Down
2 changes: 1 addition & 1 deletion gnovm/pkg/packages/filekind.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func GetFileKind(filename string, body string, fset *token.FileSet) (FileKind, e
}
ast, err := parser.ParseFile(fset, filename, body, parser.PackageClauseOnly)
if err != nil {
return FileKindUnknown, err
return FileKindTest, nil
}
packageName := ast.Name.Name

Expand Down
77 changes: 65 additions & 12 deletions gnovm/pkg/packages/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package packages

import (
"fmt"
"go/ast"
"go/parser"
"go/token"
"sort"
Expand All @@ -11,10 +12,18 @@ import (
"github.com/gnolang/gno/gnovm"
)

func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) (ImportsMap, error) {
specs, err := ImportsSpecs(pkg, fset)
if err != nil {
return nil, err
}
return ImportsMapFromSpecs(specs, fset), nil
}

// Imports returns the list of gno imports from a [gnovm.MemPackage].
// fset is optional.
func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) (ImportsMap, error) {
res := make(ImportsMap, 16)
func ImportsSpecs(pkg *gnovm.MemPackage, fset *token.FileSet) (ImportsSpecsMap, error) {
res := make(ImportsSpecsMap, 16)
seen := make(map[FileKind]map[string]struct{}, 16)

for _, file := range pkg.Files {
Expand All @@ -26,55 +35,71 @@ func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) (ImportsMap, error) {
if err != nil {
return nil, err

Check warning on line 36 in gnovm/pkg/packages/imports.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/packages/imports.go#L36

Added line #L36 was not covered by tests
}
imports, err := FileImports(file.Name, file.Body, fset)
imports, err := FileImportsSpecs(file.Name, file.Body, fset)
if err != nil {
return nil, err
}
for _, im := range imports {
if _, ok := seen[fileKind][im]; ok {
if _, ok := seen[fileKind][im.Path.Value]; ok {
continue
}
res[fileKind] = append(res[fileKind], im)
if _, ok := seen[fileKind]; !ok {
seen[fileKind] = make(map[string]struct{}, 16)
}
seen[fileKind][im] = struct{}{}
seen[fileKind][im.Path.Value] = struct{}{}
}
}

for _, imports := range res {
sortImports(imports)
sortImportsSpecs(imports)
}

return res, nil
}

// FileImports returns the list of gno imports in the given file src.
// The given filename is only used when recording position information.
func FileImports(filename string, src string, fset *token.FileSet) ([]string, error) {
func FileImportsSpecs(filename string, src string, fset *token.FileSet) ([]*ast.ImportSpec, error) {
if fset == nil {
fset = token.NewFileSet()
}
f, err := parser.ParseFile(fset, filename, src, parser.ImportsOnly)
if err != nil {
return nil, err
}
res := make([]string, len(f.Imports))
res := make([]*ast.ImportSpec, len(f.Imports))
for i, im := range f.Imports {
importPath, err := strconv.Unquote(im.Path.Value)
_, err := strconv.Unquote(im.Path.Value)
if err != nil {
// should not happen - parser.ParseFile should already ensure we get
// a valid string literal here.
panic(fmt.Errorf("%v: unexpected invalid import path: %v", fset.Position(im.Pos()).String(), im.Path.Value))
}

res[i] = importPath
res[i] = im
}
return res, nil
}

type ImportsMap map[FileKind][]string

func ImportsMapFromSpecs(specs ImportsSpecsMap, fset *token.FileSet) ImportsMap {
res := make(ImportsMap, len(specs))
for k, v := range specs {
c := make([]string, 0, len(v))
for _, x := range v {
pkgPath, err := strconv.Unquote(x.Path.Value)
if err != nil {
// should not happen - parser.ParseFile should already ensure we get
// a valid string literal here.
panic(fmt.Errorf("%v: unexpected invalid import path: %v", fset.Position(x.Pos()).String(), x.Path.Value))
}
c = append(c, pkgPath)
}
res[k] = c
}
return res
}

// Merge merges imports, it removes duplicates and sorts the result
func (imap ImportsMap) Merge(kinds ...FileKind) []string {
res := make([]string, 0, 16)
Expand All @@ -95,12 +120,40 @@ func (imap ImportsMap) Merge(kinds ...FileKind) []string {
return res
}

type ImportsSpecsMap map[FileKind][]*ast.ImportSpec

// Merge merges imports, it removes duplicates and sorts the result
func (imap ImportsSpecsMap) Merge(kinds ...FileKind) []*ast.ImportSpec {
res := make([]*ast.ImportSpec, 0, 16)
seen := make(map[string]struct{}, 16)

for _, kind := range kinds {
for _, im := range imap[kind] {
if _, ok := seen[im.Path.Value]; ok {
continue
}
seen[im.Path.Value] = struct{}{}

res = append(res, im)
}
}

sortImportsSpecs(res)
return res
}

func sortImports(imports []string) {
sort.Slice(imports, func(i, j int) bool {
return imports[i] < imports[j]
})
}

func sortImportsSpecs(imports []*ast.ImportSpec) {
sort.Slice(imports, func(i, j int) bool {
return imports[i].Path.Value < imports[j].Path.Value
})
}

func FilePackageName(filename string, src string) (string, error) {
fs := token.NewFileSet()
f, err := parser.ParseFile(fs, filename, src, parser.PackageClauseOnly)
Expand Down
Loading

0 comments on commit a73ba04

Please sign in to comment.