From a73ba043ef4c06388ac67f4b66c567cab2ad375b Mon Sep 17 00:00:00 2001 From: Norman Date: Wed, 8 Jan 2025 16:59:29 +0100 Subject: [PATCH] fix: don't ignore files without valid package clause Signed-off-by: Norman --- gnovm/cmd/gno/lint.go | 34 +++++---- gnovm/cmd/gno/lint_test.go | 4 +- gnovm/cmd/gno/mod.go | 4 +- gnovm/cmd/gno/test.go | 8 ++- gnovm/cmd/gno/testdata/lint/no_gnomod.txtar | 2 +- gnovm/pkg/packages/analyze_packages.go | 22 +++--- gnovm/pkg/packages/filekind.go | 2 +- gnovm/pkg/packages/imports.go | 77 +++++++++++++++++---- gnovm/pkg/packages/imports_test.go | 14 ++-- gnovm/pkg/test/imports.go | 17 +++-- 10 files changed, 118 insertions(+), 66 deletions(-) diff --git a/gnovm/cmd/gno/lint.go b/gnovm/cmd/gno/lint.go index 183ed004aaf..3fdb8411daa 100644 --- a/gnovm/cmd/gno/lint.go +++ b/gnovm/cmd/gno/lint.go @@ -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. @@ -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, @@ -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) @@ -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) } @@ -292,7 +290,7 @@ 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. @@ -300,7 +298,7 @@ func guessSourcePath(pkg, source string) 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() @@ -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) } @@ -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 diff --git a/gnovm/cmd/gno/lint_test.go b/gnovm/cmd/gno/lint_test.go index e1a730130d2..72ca51f240e 100644 --- a/gnovm/cmd/gno/lint_test.go +++ b/gnovm/cmd/gno/lint_test.go @@ -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"}, diff --git a/gnovm/cmd/gno/mod.go b/gnovm/cmd/gno/mod.go index 63c5c506e0a..18d583c5810 100644 --- a/gnovm/cmd/gno/mod.go +++ b/gnovm/cmd/gno/mod.go @@ -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 diff --git a/gnovm/cmd/gno/test.go b/gnovm/cmd/gno/test.go index e3d0dc46a3c..2d1d4a2347a 100644 --- a/gnovm/cmd/gno/test.go +++ b/gnovm/cmd/gno/test.go @@ -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) }) diff --git a/gnovm/cmd/gno/testdata/lint/no_gnomod.txtar b/gnovm/cmd/gno/testdata/lint/no_gnomod.txtar index 775c921aee2..072e7cca551 100644 --- a/gnovm/cmd/gno/testdata/lint/no_gnomod.txtar +++ b/gnovm/cmd/gno/testdata/lint/no_gnomod.txtar @@ -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) diff --git a/gnovm/pkg/packages/analyze_packages.go b/gnovm/pkg/packages/analyze_packages.go index 7453927eaee..9ae9009d047 100644 --- a/gnovm/pkg/packages/analyze_packages.go +++ b/gnovm/pkg/packages/analyze_packages.go @@ -3,7 +3,6 @@ package packages import ( "errors" "fmt" - "go/parser" "go/token" "os" "path" @@ -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 @@ -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) } diff --git a/gnovm/pkg/packages/filekind.go b/gnovm/pkg/packages/filekind.go index 382d1d12af7..cbad0cc6c1e 100644 --- a/gnovm/pkg/packages/filekind.go +++ b/gnovm/pkg/packages/filekind.go @@ -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 diff --git a/gnovm/pkg/packages/imports.go b/gnovm/pkg/packages/imports.go index 4820dc03b7a..6a684bcc57a 100644 --- a/gnovm/pkg/packages/imports.go +++ b/gnovm/pkg/packages/imports.go @@ -2,6 +2,7 @@ package packages import ( "fmt" + "go/ast" "go/parser" "go/token" "sort" @@ -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 { @@ -26,32 +35,30 @@ func Imports(pkg *gnovm.MemPackage, fset *token.FileSet) (ImportsMap, error) { if err != nil { return nil, err } - 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() } @@ -59,22 +66,40 @@ func FileImports(filename string, src string, fset *token.FileSet) ([]string, er 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) @@ -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) diff --git a/gnovm/pkg/packages/imports_test.go b/gnovm/pkg/packages/imports_test.go index 0267f46e17a..9a396d1ac40 100644 --- a/gnovm/pkg/packages/imports_test.go +++ b/gnovm/pkg/packages/imports_test.go @@ -1,6 +1,7 @@ package packages_test import ( + "go/token" "os" "path/filepath" "testing" @@ -146,18 +147,13 @@ func TestImports(t *testing.T) { pkg, err := gnolang.ReadMemPackage(tmpDir, "test") require.NoError(t, err) - importsMap, err := Imports(pkg, nil) + fset := token.NewFileSet() + + importsSpec, err := ImportsSpecs(pkg, nil) require.NoError(t, err) // ignore specs - got := map[FileKind][]string{} - for key, vals := range importsMap { - stringVals := make([]string, len(vals)) - for i, val := range vals { - stringVals[i] = val - } - got[key] = stringVals - } + got := ImportsMapFromSpecs(importsSpec, fset) require.Equal(t, expected, got) } diff --git a/gnovm/pkg/test/imports.go b/gnovm/pkg/test/imports.go index fc81cab17d6..8b96400ead8 100644 --- a/gnovm/pkg/test/imports.go +++ b/gnovm/pkg/test/imports.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "runtime/debug" + "strconv" "strings" "time" @@ -246,20 +247,26 @@ func LoadImports(store gno.Store, memPkg *gnovm.MemPackage) (err error) { }() fset := token.NewFileSet() - importsMap, err := packages.Imports(memPkg, fset) + importsMap, err := packages.ImportsSpecs(memPkg, fset) if err != nil { return err } imports := importsMap.Merge(packages.FileKindPackageSource, packages.FileKindTest, packages.FileKindXTest) - for _, imp := range imports { - if gno.IsRealmPath(imp) { + for _, im := range imports { + pkgPath, 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)) + } + if gno.IsRealmPath(pkgPath) { // Don't eagerly load realms. // Realms persist state and can change the state of other realms in initialization. continue } - pkg := store.GetPackage(imp, true) + pkg := store.GetPackage(pkgPath, true) if pkg == nil { - return fmt.Errorf("unknown import path %v", imp) + return fmt.Errorf("%s: unknown import path %s", fset.Position(im.Pos()).String(), pkgPath) } } return nil