feat(cmd/gno): perform type checking when calling linter (gnolang#1730)

Depends on (in order): 1. gnolang#1700 2. gnolang#1702 This PR uses the type checker added in gnolang#1702 to perform Gno type checking when calling `gno lint`. Additionally, it adds validation of gno.mod indirectly (the parsed gno mod is used to determine if a package is a draft, and if so skip type checking). Because `gno lint` uses the TestStore, the resulting `MemPackage`s may contain redefinitions, for overwriting standard libraries like `AssertOriginCall`. I changed the type checker to filter out the redefinitions before they reach the Go type checker. Further improvements, which can be done after this: - Add shims for gonative special libraries (`fmt`, `os`...) - This will allow us to fully type check also tests and filetests - Make the type checking on-chain (gnolang#1702) also typecheck tests - as a consequence of the above.
gnoswap-labs · Dec 10, 2024 · 985c262 · 985c262
1 parent 8da4886
commit 985c262
Show file tree

Hide file tree

Showing 68 changed files with 263 additions and 146 deletions.
diff --git a/gnovm/cmd/gno/lint.go b/gnovm/cmd/gno/lint.go
@@ -6,17 +6,19 @@ import (
 	"flag"
 	"fmt"
 	"go/scanner"
+	"go/types"
 	"io"
 	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
 
+	"github.com/gnolang/gno/gnovm"
 	"github.com/gnolang/gno/gnovm/pkg/gnoenv"
 	gno "github.com/gnolang/gno/gnovm/pkg/gnolang"
+	"github.com/gnolang/gno/gnovm/pkg/gnomod"
 	"github.com/gnolang/gno/gnovm/pkg/test"
 	"github.com/gnolang/gno/tm2/pkg/commands"
-	osm "github.com/gnolang/gno/tm2/pkg/os"
 	"go.uber.org/multierr"
 )
 
@@ -50,6 +52,31 @@ func (c *lintCfg) RegisterFlags(fs *flag.FlagSet) {
 	fs.StringVar(&c.rootDir, "root-dir", rootdir, "clone location of github.com/gnolang/gno (gno tries to guess it)")
 }
 
+type lintCode int
+
+const (
+	lintUnknown lintCode = iota
+	lintGnoMod
+	lintGnoError
+	lintParserError
+	lintTypeCheckError
+
+	// TODO: add new linter codes here.
+)
+
+type lintIssue struct {
+	Code       lintCode
+	Msg        string
+	Confidence float64 // 1 is 100%
+	Location   string  // file:line, or equivalent
+	// TODO: consider writing fix suggestions
+}
+
+func (i lintIssue) String() string {
+	// TODO: consider crafting a doc URL based on Code.
+	return fmt.Sprintf("%s: %s (code=%d)", i.Location, i.Msg, i.Code)
+}
+
 func execLint(cfg *lintCfg, args []string, io commands.IO) error {
 	if len(args) < 1 {
 		return flag.ErrHelp
@@ -72,66 +99,69 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error {
 
 	for _, pkgPath := range pkgPaths {
 		if verbose {
-			fmt.Fprintf(io.Err(), "Linting %q...\n", pkgPath)
+			io.ErrPrintln(pkgPath)
+		}
+
+		info, err := os.Stat(pkgPath)
+		if err == nil && !info.IsDir() {
+			pkgPath = filepath.Dir(pkgPath)
 		}
 
 		// Check if 'gno.mod' exists
-		gnoModPath := filepath.Join(pkgPath, "gno.mod")
-		if !osm.FileExists(gnoModPath) {
-			hasError = true
+		gmFile, err := gnomod.ParseAt(pkgPath)
+		if err != nil {
 			issue := lintIssue{
-				Code:       lintNoGnoMod,
+				Code:       lintGnoMod,
 				Confidence: 1,
 				Location:   pkgPath,
-				Msg:        "missing 'gno.mod' file",
+				Msg:        err.Error(),
 			}
-			fmt.Fprint(io.Err(), issue.String()+"\n")
+			io.ErrPrintln(issue)
+			hasError = true
 		}
 
-		// Handle runtime errors
-		hasError = catchRuntimeError(pkgPath, io.Err(), func() {
-			stdout, stdin, stderr := io.Out(), io.In(), io.Err()
-			_, testStore := test.Store(
-				rootDir, false,
-				stdin, stdout, stderr,
-			)
-
-			targetPath := pkgPath
-			info, err := os.Stat(pkgPath)
-			if err == nil && !info.IsDir() {
-				targetPath = filepath.Dir(pkgPath)
+		stdout, stdin, stderr := io.Out(), io.In(), io.Err()
+		_, testStore := test.Store(
+			rootDir, false,
+			stdin, stdout, stderr,
+		)
+
+		memPkg, err := gno.ReadMemPackage(pkgPath, pkgPath)
+		if err != nil {
+			io.ErrPrintln(issueFromError(pkgPath, err).String())
+			hasError = true
+			continue
+		}
+
+		// Run type checking
+		if gmFile == nil || !gmFile.Draft {
+			foundErr, err := lintTypeCheck(io, memPkg, testStore)
+			if err != nil {
+				io.ErrPrintln(err)
+				hasError = true
+			} else if foundErr {
+				hasError = true
 			}
+		} else if verbose {
+			io.ErrPrintfln("%s: module is draft, skipping type check", pkgPath)
+		}
 
-			memPkg := gno.MustReadMemPackage(targetPath, targetPath)
+		// Handle runtime errors
+		hasRuntimeErr := catchRuntimeError(pkgPath, io.Err(), func() {
 			tm := test.Machine(testStore, stdout, memPkg.Path)
 			defer tm.Release()
 
 			// Check package
 			tm.RunMemPackage(memPkg, true)
 
 			// Check test files
-			testfiles := &gno.FileSet{}
-			for _, mfile := range memPkg.Files {
-				if !strings.HasSuffix(mfile.Name, ".gno") {
-					continue // Skip non-GNO files
-				}
+			testFiles := lintTestFiles(memPkg)
 
-				n, _ := gno.ParseFile(mfile.Name, mfile.Body)
-				if n == nil {
-					continue // Skip empty files
-				}
-
-				// XXX: package ending with `_test` is not supported yet
-				if strings.HasSuffix(mfile.Name, "_test.gno") && !strings.HasSuffix(string(n.PkgName), "_test") {
-					// Keep only test files
-					testfiles.AddFiles(n)
-				}
-			}
-
-			tm.RunFiles(testfiles.Files...)
-		}) || hasError
-
-		// TODO: Add more checkers
+			tm.RunFiles(testFiles.Files...)
+		})
+		if hasRuntimeErr {
+			hasError = true
+		}
 	}
 
 	if hasError {
@@ -141,6 +171,66 @@ func execLint(cfg *lintCfg, args []string, io commands.IO) error {
 	return nil
 }
 
+func lintTypeCheck(io commands.IO, memPkg *gnovm.MemPackage, testStore gno.Store) (errorsFound bool, err error) {
+	tcErr := gno.TypeCheckMemPackageTest(memPkg, testStore)
+	if tcErr == nil {
+		return false, nil
+	}
+
+	errs := multierr.Errors(tcErr)
+	for _, err := range errs {
+		switch err := err.(type) {
+		case types.Error:
+			io.ErrPrintln(lintIssue{
+				Code:       lintTypeCheckError,
+				Msg:        err.Msg,
+				Confidence: 1,
+				Location:   err.Fset.Position(err.Pos).String(),
+			})
+		case scanner.ErrorList:
+			for _, scErr := range err {
+				io.ErrPrintln(lintIssue{
+					Code:       lintParserError,
+					Msg:        scErr.Msg,
+					Confidence: 1,
+					Location:   scErr.Pos.String(),
+				})
+			}
+		case scanner.Error:
+			io.ErrPrintln(lintIssue{
+				Code:       lintParserError,
+				Msg:        err.Msg,
+				Confidence: 1,
+				Location:   err.Pos.String(),
+			})
+		default:
+			return false, fmt.Errorf("unexpected error type: %T", err)
+		}
+	}
+	return true, nil
+}
+
+func lintTestFiles(memPkg *gnovm.MemPackage) *gno.FileSet {
+	testfiles := &gno.FileSet{}
+	for _, mfile := range memPkg.Files {
+		if !strings.HasSuffix(mfile.Name, ".gno") {
+			continue // Skip non-GNO files
+		}
+
+		n, _ := gno.ParseFile(mfile.Name, mfile.Body)
+		if n == nil {
+			continue // Skip empty files
+		}
+
+		// XXX: package ending with `_test` is not supported yet
+		if strings.HasSuffix(mfile.Name, "_test.gno") && !strings.HasSuffix(string(n.PkgName), "_test") {
+			// Keep only test files
+			testfiles.AddFiles(n)
+		}
+	}
+	return testfiles
+}
+
 func guessSourcePath(pkg, source string) string {
 	if info, err := os.Stat(pkg); !os.IsNotExist(err) && !info.IsDir() {
 		pkg = filepath.Dir(pkg)
@@ -174,21 +264,21 @@ func catchRuntimeError(pkgPath string, stderr io.WriteCloser, action func()) (ha
 		switch verr := r.(type) {
 		case *gno.PreprocessError:
 			err := verr.Unwrap()
-			fmt.Fprint(stderr, issueFromError(pkgPath, err).String()+"\n")
+			fmt.Fprintln(stderr, issueFromError(pkgPath, err).String())
 		case error:
 			errors := multierr.Errors(verr)
 			for _, err := range errors {
 				errList, ok := err.(scanner.ErrorList)
 				if ok {
 					for _, errorInList := range errList {
-						fmt.Fprint(stderr, issueFromError(pkgPath, errorInList).String()+"\n")
+						fmt.Fprintln(stderr, issueFromError(pkgPath, errorInList).String())
 					}
 				} else {
-					fmt.Fprint(stderr, issueFromError(pkgPath, err).String()+"\n")
+					fmt.Fprintln(stderr, issueFromError(pkgPath, err).String())
 				}
 			}
 		case string:
-			fmt.Fprint(stderr, issueFromError(pkgPath, errors.New(verr)).String()+"\n")
+			fmt.Fprintln(stderr, issueFromError(pkgPath, errors.New(verr)).String())
 		default:
 			panic(r)
 		}
@@ -198,29 +288,6 @@ func catchRuntimeError(pkgPath string, stderr io.WriteCloser, action func()) (ha
 	return
 }
 
-type lintCode int
-
-const (
-	lintUnknown  lintCode = 0
-	lintNoGnoMod lintCode = iota
-	lintGnoError
-
-	// TODO: add new linter codes here.
-)
-
-type lintIssue struct {
-	Code       lintCode
-	Msg        string
-	Confidence float64 // 1 is 100%
-	Location   string  // file:line, or equivalent
-	// TODO: consider writing fix suggestions
-}
-
-func (i lintIssue) String() string {
-	// TODO: consider crafting a doc URL based on Code.
-	return fmt.Sprintf("%s: %s (code=%d).", i.Location, i.Msg, i.Code)
-}
-
 func issueFromError(pkgPath string, err error) lintIssue {
 	var issue lintIssue
 	issue.Confidence = 1

diff --git a/gnovm/cmd/gno/lint_test.go b/gnovm/cmd/gno/lint_test.go
@@ -1,6 +1,9 @@
 package main
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestLintApp(t *testing.T) {
 	tc := []testMainCase{
@@ -9,41 +12,51 @@ func TestLintApp(t *testing.T) {
 			errShouldBe: "flag: help requested",
 		}, {
 			args:                []string{"lint", "../../tests/integ/run_main/"},
-			stderrShouldContain: "./../../tests/integ/run_main: missing 'gno.mod' file (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: "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"},
-			stderrShouldContain: "main.gno:4:2: name fmt not declared (code=2).",
+			stderrShouldContain: "main.gno:4:2: name fmt not declared (code=2)",
 			errShouldBe:         "exit code: 1",
 		}, {
 			args:                []string{"lint", "../../tests/integ/several-lint-errors/main.gno"},
-			stderrShouldContain: "../../tests/integ/several-lint-errors/main.gno:5:5: expected ';', found example (code=2).\n../../tests/integ/several-lint-errors/main.gno:6",
+			stderrShouldContain: "../../tests/integ/several-lint-errors/main.gno:5:5: expected ';', found example (code=2)\n../../tests/integ/several-lint-errors/main.gno:6",
 			errShouldBe:         "exit code: 1",
 		}, {
-			args:                []string{"lint", "../../tests/integ/several-files-multiple-errors/main.gno"},
-			stderrShouldContain: "../../tests/integ/several-files-multiple-errors/file2.gno:3:5: expected 'IDENT', found '{' (code=2).\n../../tests/integ/several-files-multiple-errors/file2.gno:5:1: expected type, found '}' (code=2).\n../../tests/integ/several-files-multiple-errors/main.gno:5:5: expected ';', found example (code=2).\n../../tests/integ/several-files-multiple-errors/main.gno:6:2: expected '}', found 'EOF' (code=2).\n",
-			errShouldBe:         "exit code: 1",
-		}, {
-			args:                []string{"lint", "../../tests/integ/run_main/"},
-			stderrShouldContain: "./../../tests/integ/run_main: missing 'gno.mod' file (code=1).",
-			errShouldBe:         "exit code: 1",
+			args: []string{"lint", "../../tests/integ/several-files-multiple-errors/main.gno"},
+			stderrShouldContain: func() string {
+				lines := []string{
+					"../../tests/integ/several-files-multiple-errors/file2.gno:3:5: expected 'IDENT', found '{' (code=2)",
+					"../../tests/integ/several-files-multiple-errors/file2.gno:5:1: expected type, found '}' (code=2)",
+					"../../tests/integ/several-files-multiple-errors/main.gno:5:5: expected ';', found example (code=2)",
+					"../../tests/integ/several-files-multiple-errors/main.gno:6:2: expected '}', found 'EOF' (code=2)",
+				}
+				return strings.Join(lines, "\n") + "\n"
+			}(),
+			errShouldBe: "exit code: 1",
 		}, {
 			args: []string{"lint", "../../tests/integ/minimalist_gnomod/"},
 			// TODO: raise an error because there is a gno.mod, but no .gno files
 		}, {
 			args: []string{"lint", "../../tests/integ/invalid_module_name/"},
 			// TODO: raise an error because gno.mod is invalid
+		}, {
+			args:                []string{"lint", "../../tests/integ/invalid_gno_file/"},
+			stderrShouldContain: "../../tests/integ/invalid_gno_file/invalid.gno:1:1: expected 'package', found packag (code=2)",
+			errShouldBe:         "exit code: 1",
+		}, {
+			args:                []string{"lint", "../../tests/integ/typecheck_missing_return/"},
+			stderrShouldContain: "../../tests/integ/typecheck_missing_return/main.gno:5:1: missing return (code=4)",
+			errShouldBe:         "exit code: 1",
 		},
 
 		// TODO: 'gno mod' is valid?
-		// TODO: is gno source valid?
 		// TODO: are dependencies valid?
 		// TODO: is gno source using unsafe/discouraged features?
-		// TODO: consider making `gno transpile; go lint *gen.go`
 		// TODO: check for imports of native libs from non _test.gno files
 	}
 	testMainCaseRun(t, tc)

diff --git a/gnovm/cmd/gno/run_test.go b/gnovm/cmd/gno/run_test.go
@@ -1,6 +1,9 @@
 package main
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestRunApp(t *testing.T) {
 	tc := []testMainCase{
@@ -84,9 +87,17 @@ func TestRunApp(t *testing.T) {
 			stdoutShouldContain: "Context worked",
 		},
 		{
-			args:                []string{"run", "../../tests/integ/several-files-multiple-errors/"},
-			stderrShouldContain: "../../tests/integ/several-files-multiple-errors/file2.gno:3:5: expected 'IDENT', found '{' (code=2).\n../../tests/integ/several-files-multiple-errors/file2.gno:5:1: expected type, found '}' (code=2).\n../../tests/integ/several-files-multiple-errors/main.gno:5:5: expected ';', found example (code=2).\n../../tests/integ/several-files-multiple-errors/main.gno:6:2: expected '}', found 'EOF' (code=2).\n",
-			errShouldBe:         "exit code: 1",
+			args: []string{"run", "../../tests/integ/several-files-multiple-errors/"},
+			stderrShouldContain: func() string {
+				lines := []string{
+					"../../tests/integ/several-files-multiple-errors/file2.gno:3:5: expected 'IDENT', found '{' (code=2)",
+					"../../tests/integ/several-files-multiple-errors/file2.gno:5:1: expected type, found '}' (code=2)",
+					"../../tests/integ/several-files-multiple-errors/main.gno:5:5: expected ';', found example (code=2)",
+					"../../tests/integ/several-files-multiple-errors/main.gno:6:2: expected '}', found 'EOF' (code=2)",
+				}
+				return strings.Join(lines, "\n") + "\n"
+			}(),
+			errShouldBe: "exit code: 1",
 		},
 		// TODO: a test file
 		// TODO: args

diff --git a/gnovm/cmd/gno/testdata/gno_fmt/empty.txtar → gnovm/cmd/gno/testdata/fmt/empty.txtar b/gnovm/cmd/gno/testdata/gno_fmt/empty.txtar → gnovm/cmd/gno/testdata/fmt/empty.txtar
diff --git a/...no/testdata/gno_fmt/import_cleaning.txtar → ...md/gno/testdata/fmt/import_cleaning.txtar b/...no/testdata/gno_fmt/import_cleaning.txtar → ...md/gno/testdata/fmt/import_cleaning.txtar
diff --git a/gnovm/cmd/gno/testdata/gno_fmt/include.txtar → gnovm/cmd/gno/testdata/fmt/include.txtar b/gnovm/cmd/gno/testdata/gno_fmt/include.txtar → gnovm/cmd/gno/testdata/fmt/include.txtar
diff --git a/...d/gno/testdata/gno_fmt/multi_import.txtar → ...m/cmd/gno/testdata/fmt/multi_import.txtar b/...d/gno/testdata/gno_fmt/multi_import.txtar → ...m/cmd/gno/testdata/fmt/multi_import.txtar
diff --git a/...no/testdata/gno_fmt/noimport_format.txtar → ...md/gno/testdata/fmt/noimport_format.txtar b/...no/testdata/gno_fmt/noimport_format.txtar → ...md/gno/testdata/fmt/noimport_format.txtar
diff --git a/...md/gno/testdata/gno_fmt/parse_error.txtar → gnovm/cmd/gno/testdata/fmt/parse_error.txtar b/...md/gno/testdata/gno_fmt/parse_error.txtar → gnovm/cmd/gno/testdata/fmt/parse_error.txtar
diff --git a/.../gno/testdata/gno_fmt/shadow_import.txtar → .../cmd/gno/testdata/fmt/shadow_import.txtar b/.../gno/testdata/gno_fmt/shadow_import.txtar → .../cmd/gno/testdata/fmt/shadow_import.txtar