Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 70 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
85f62ba
refactor(gnovm): rename precompiler to transpiler, move to own package
thehowl Feb 22, 2024
9409285
move transpiler to pkg
thehowl Feb 22, 2024
cb63cf5
Merge branch 'master' into dev/morgan/precompile-refactor
thehowl Feb 22, 2024
3710ded
Merge branch 'dev/morgan/precompile-refactor' of github.com:gnolang/g…
thehowl Feb 22, 2024
37c9e66
fixup
thehowl Feb 22, 2024
59c5bb1
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/preco…
thehowl Feb 26, 2024
d39292b
Merge branch 'master' into dev/morgan/precompile-refactor
thehowl Feb 27, 2024
a25d076
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/preco…
thehowl Feb 28, 2024
a889dea
begin removing support for linked identifiers
thehowl Feb 22, 2024
aae89db
convert native bindings to never use linked types
thehowl Feb 23, 2024
fd2722e
remove InjectNativeMappings
thehowl Feb 23, 2024
8856225
fix errors in genstd
thehowl Feb 27, 2024
d6955d0
remove AddGo2GnoMapping, nolint on btReadonly
thehowl Feb 28, 2024
c3e8ff8
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/preco…
thehowl Feb 28, 2024
1de00fa
Merge branch 'dev/morgan/precompile-refactor' into dev/morgan/natbind…
thehowl Feb 28, 2024
c0581dd
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/natbi…
thehowl Feb 28, 2024
b3c7adb
fmt + bugfix
thehowl Feb 28, 2024
250e292
feat(gno.land): add go type checking to keeper
thehowl Feb 28, 2024
0d8596d
lint + test fixes
thehowl Feb 28, 2024
10c0434
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/natbi…
thehowl Mar 3, 2024
f641c58
Merge branch 'dev/morgan/natbind-no-linkedtype' into dev/morgan/go-ty…
thehowl Mar 3, 2024
9779ff3
use custom type instead of any in importer
thehowl Mar 3, 2024
7b239df
support multiple errors in type checker
thehowl Mar 3, 2024
1b323a2
correctly return error in vm keeper
thehowl Mar 3, 2024
df1679c
add tests to type checker
thehowl Mar 3, 2024
ee0448e
fixup
thehowl Mar 3, 2024
a169800
add integration test for gno.land
thehowl Mar 3, 2024
d2c3264
feat(cmd/gno): perform type checking when calling linter
thehowl Mar 4, 2024
b320155
fix testscript tests, remove duplicate code;
thehowl Mar 4, 2024
e53aa22
Merge branch 'master' into dev/morgan/go-types-typecheck
thehowl Mar 20, 2024
172e4ae
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/go-ty…
thehowl Mar 27, 2024
0fa34fa
remove newMemPackage
thehowl Mar 27, 2024
8688167
Merge branch 'dev/morgan/go-types-typecheck' of github.com:gnolang/gn…
thehowl Mar 27, 2024
89d9887
remove unused var
thehowl Mar 27, 2024
ccf0ba9
Merge branch 'master' into dev/morgan/go-types-typecheck
thehowl Apr 15, 2024
338454f
remove other usage in gnoclient
thehowl Apr 15, 2024
1c5c14f
Merge branch 'dev/morgan/go-types-typecheck' into dev/morgan/typechec…
thehowl Apr 15, 2024
c2917c6
s/--verbose/-v/g
thehowl Apr 15, 2024
6b4ed9e
remove unused import in test
thehowl Apr 15, 2024
593defa
Merge branch 'dev/morgan/go-types-typecheck' into dev/morgan/typechec…
thehowl Apr 15, 2024
0007e73
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/go-ty…
thehowl Apr 17, 2024
9ccb5ee
bump limits for issue-1786 test
thehowl Apr 17, 2024
e20ebbe
bump up to 4M
thehowl Apr 17, 2024
ad2f0b1
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/go-ty…
thehowl May 6, 2024
66fb953
changes from code review
thehowl May 6, 2024
38cba04
fix gas numbers
thehowl May 7, 2024
bee0f30
add transaction simulation in gnokey
thehowl May 7, 2024
3a82a98
fixup lint
thehowl May 7, 2024
a112b4a
Merge branch 'dev/morgan/go-types-typecheck' into dev/morgan/typechec…
thehowl May 7, 2024
39ae304
update docs
thehowl May 7, 2024
dbde296
partial revert of store changes
thehowl May 9, 2024
9ee3cc6
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/go-ty…
thehowl May 9, 2024
f843acb
fixup
thehowl May 9, 2024
2558b09
Revert "fixup"
thehowl May 10, 2024
cc9d45e
Revert "partial revert of store changes"
thehowl May 10, 2024
09f081c
fixup
thehowl May 10, 2024
38719cd
update docs
thehowl May 10, 2024
815c573
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/go-ty…
thehowl May 13, 2024
6154240
Merge branch 'dev/morgan/go-types-typecheck' into dev/morgan/typechec…
thehowl May 13, 2024
10992d2
please, ci, work
thehowl May 13, 2024
b0dbfe7
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/typec…
thehowl May 13, 2024
c5af2fe
mv
thehowl May 13, 2024
dd64ec1
mv
thehowl May 13, 2024
7d93ab1
codereview changes
thehowl May 13, 2024
0ffda3b
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/typec…
thehowl May 21, 2024
dcb9930
review comments
thehowl May 21, 2024
d5d166c
ignore errorlint on fmt.Errorf
thehowl May 21, 2024
b21a7f5
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/typec…
thehowl May 29, 2024
6530717
Merge branch 'master' of github.com:gnolang/gno into dev/morgan/typec…
thehowl Jun 3, 2024
733fa73
adapt to new master changes
thehowl Jun 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ test:

.PHONY: lint
lint:
go run ../gnovm/cmd/gno lint $(OFFICIAL_PACKAGES)
go run ../gnovm/cmd/gno lint -v $(OFFICIAL_PACKAGES)

.PHONY: test.sync
test.sync:
Expand Down
2 changes: 2 additions & 0 deletions examples/gno.land/r/demo/art/gnoface/gno.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// Draft
zivkovicmilos marked this conversation as resolved.
Show resolved Hide resolved

module gno.land/r/demo/art/gnoface

require (
Expand Down
174 changes: 118 additions & 56 deletions gnovm/cmd/gno/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"flag"
"fmt"
"go/scanner"
"go/types"
"io"
"os"
"path/filepath"
Expand All @@ -14,9 +15,11 @@

"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/tests"
"github.com/gnolang/gno/tm2/pkg/commands"
osm "github.com/gnolang/gno/tm2/pkg/os"
"github.com/gnolang/gno/tm2/pkg/std"
"go.uber.org/multierr"
)

type lintCfg struct {
Expand Down Expand Up @@ -49,6 +52,31 @@
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%
zivkovicmilos marked this conversation as resolved.
Show resolved Hide resolved
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.
thehowl marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -71,66 +99,62 @@

for _, pkgPath := range pkgPaths {
if verbose {
fmt.Fprintf(io.Err(), "Linting %q...\n", pkgPath)
io.ErrPrintln(pkgPath)

Check warning on line 102 in gnovm/cmd/gno/lint.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/lint.go#L102

Added line #L102 was not covered by tests
}

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 := tests.TestStore(
rootDir, "",
stdin, stdout, stderr,
tests.ImportModeStdlibsOnly,
)

targetPath := pkgPath
info, err := os.Stat(pkgPath)
if err == nil && !info.IsDir() {
targetPath = filepath.Dir(pkgPath)
memPkg := gno.ReadMemPackage(pkgPath, pkgPath)

// Run type checking
if gmFile == nil || !gmFile.Draft {
foundErr, err := lintTypeCheck(io, memPkg, testStore)
if err != nil {
io.ErrPrintln(err)
hasError = true

Check warning on line 140 in gnovm/cmd/gno/lint.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/lint.go#L139-L140

Added lines #L139 - L140 were not covered by tests
} else {
hasError = foundErr || hasError
}
} else if verbose {
zivkovicmilos marked this conversation as resolved.
Show resolved Hide resolved
io.ErrPrintfln("%s: module is draft, skipping type check\n", pkgPath)

Check warning on line 145 in gnovm/cmd/gno/lint.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/lint.go#L144-L145

Added lines #L144 - L145 were not covered by tests
}

memPkg := gno.ReadMemPackage(targetPath, targetPath)
tm := tests.TestMachine(testStore, stdout, memPkg.Name)

// 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...)
tm.RunFiles(testFiles.Files...)
}) || hasError

// TODO: Add more checkers
}

if hasError {
Expand All @@ -140,6 +164,67 @@
return nil
}

func lintTypeCheck(io commands.IO, memPkg *std.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:

Check warning on line 199 in gnovm/cmd/gno/lint.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/lint.go#L192-L199

Added lines #L192 - L199 were not covered by tests
//nolint
return false, fmt.Errorf("unexpected error type: %T", err)

Check warning on line 201 in gnovm/cmd/gno/lint.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/lint.go#L201

Added line #L201 was not covered by tests
}
}
return true, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the situation that errorsFound is true while err is nil?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None, really: further above, we check that tcErr == nil. So, at this point, there will always be at least an error that was printed.

}

func lintTestFiles(memPkg *std.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

Check warning on line 216 in gnovm/cmd/gno/lint.go

View check run for this annotation

Codecov / codecov/patch

gnovm/cmd/gno/lint.go#L216

Added line #L216 was not covered by tests
}

// 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)
}
Comment on lines +219 to +223
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code block should be moved before gno.ParseFile(...) line 214.
I saw somewhere else in this PR that there is also some code to match against *_filetest.gno. Should it be added here, or better, get rid of special treatment for *_filetest.gno. Why should it be different from *_test.gno?

}
return testfiles
}

func guessSourcePath(pkg, source string) string {
if info, err := os.Stat(pkg); !os.IsNotExist(err) && !info.IsDir() {
pkg = filepath.Dir(pkg)
Expand Down Expand Up @@ -191,29 +276,6 @@
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
Expand Down
16 changes: 9 additions & 7 deletions gnovm/cmd/gno/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ 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"},
Expand All @@ -23,23 +23,25 @@ func TestLintApp(t *testing.T) {
args: []string{"lint", "../../tests/integ/several-lint-errors/main.gno"},
stderrShouldContain: "../../tests/integ/several-lint-errors/main.gno:5: expected ';', found example (code=2).\n../../tests/integ/several-lint-errors/main.gno:6",
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/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: 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)
Expand Down
36 changes: 36 additions & 0 deletions gnovm/cmd/gno/scripts_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package main

import (
"os"
"path/filepath"
"strconv"
"testing"

"github.com/gnolang/gno/gnovm/pkg/integration"
"github.com/rogpeppe/go-internal/testscript"
"github.com/stretchr/testify/require"
)

func TestScript(t *testing.T) {
updateScripts, _ := strconv.ParseBool(os.Getenv("UPDATE_SCRIPTS"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already have a flag for this, -update-golden-files or -update-golden-tests iirc. it should be exposed as a global unexported var.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we do this in another PR? this was carried over from the existing files (test_test.go, transpile_test.go) and I think there are probably more tests to update to use that flag

dirs, err := os.ReadDir("testdata")
require.NoError(t, err)
for _, dir := range dirs {
t.Run(dir.Name(), func(t *testing.T) {
p := testscript.Params{
UpdateScripts: updateScripts,
Dir: filepath.Join("testdata", dir.Name()),
}

if coverdir, ok := integration.ResolveCoverageDir(); ok {
err := integration.SetupTestscriptsCoverage(&p, coverdir)
require.NoError(t, err)
}

err := integration.SetupGno(&p, t.TempDir())
require.NoError(t, err)

testscript.Run(t, p)
})
}
}
29 changes: 0 additions & 29 deletions gnovm/cmd/gno/test_test.go

This file was deleted.

20 changes: 0 additions & 20 deletions gnovm/cmd/gno/testdata/gno_test/lint_file_error_txtar

This file was deleted.

Loading
Loading