feat(gnovm): enforce formatting of added packages (gnolang#2464)

This PR introduces the concept of automatically formatting packages
during AddPkg. The advantage is that it's relatively inexpensive since
we were already parsing the submitted source code AST to apply some
verification.

I believe that having a consistent official formatting for the Go
language is one of the things that makes it better, simpler, and more
readable. I think Gno should strive to be as good as or better than Go
in this regard.

Another justification for this approach is that Gno is not just the
language but a platform (gno.land), like a PAAS, and AddPkg can be
considered the "CD" process, while auto-formatting could be the "CI"
component, which often go hand-in-hand in "CI/CD".

The biggest drawback I can see is that in practice, the code content may
not be the same on the publisher's computer and on-chain.

---------

Signed-off-by: moul <[email protected]>
Co-authored-by: Morgan <[email protected]>

gno.land/pkg/sdk/vm/keeper.go

@@ -388,7 +388,8 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) (err error) {
 	}
 
 	// Validate Gno syntax and type check.
-	if err := gno.TypeCheckMemPackage(memPkg, gnostore); err != nil {
+	format := true
+	if err := gno.TypeCheckMemPackage(memPkg, gnostore, format); err != nil {
 		return ErrTypeCheck(err)
 	}
 
@@ -593,7 +594,8 @@ func (vm *VMKeeper) Run(ctx sdk.Context, msg MsgRun) (res string, err error) {
 	}
 
 	// Validate Gno syntax and type check.
-	if err = gno.TypeCheckMemPackage(memPkg, gnostore); err != nil {
+	format := false
+	if err = gno.TypeCheckMemPackage(memPkg, gnostore, format); err != nil {
 		return "", ErrTypeCheck(err)
 	}
 

gno.land/pkg/sdk/vm/keeper_test.go

@@ -17,6 +17,7 @@ import (
 func TestVMKeeperAddPackage(t *testing.T) {
 	env := setupTestEnv()
 	ctx := env.ctx
+	vmk := env.vmk
 
 	// Give "addr1" some gnots.
 	addr := crypto.AddressFromPreimage([]byte("addr1"))
@@ -30,10 +31,7 @@ func TestVMKeeperAddPackage(t *testing.T) {
 		{
 			Name: "test.gno",
 			Body: `package test
-
-func Echo() string {
-	return "hello world"
-}`,
+func Echo() string {return "hello world"}`,
 		},
 	}
 	pkgPath := "gno.land/r/test"
@@ -49,6 +47,16 @@ func Echo() string {
 
 	assert.Error(t, err)
 	assert.True(t, errors.Is(err, InvalidPkgPathError{}))
+
+	// added package is formatted
+	store := vmk.getGnoStore(ctx)
+	memFile := store.GetMemFile("gno.land/r/test", "test.gno")
+	assert.NotNil(t, memFile)
+	expected := `package test
+
+func Echo() string { return "hello world" }
+`
+	assert.Equal(t, expected, memFile.Body)
 }
 
 // Sending total send amount succeeds.

gnovm/pkg/gnolang/go2gno.go

@@ -31,8 +31,10 @@ package gnolang
 */
 
 import (
+	"bytes"
 	"fmt"
 	"go/ast"
+	"go/format"
 	"go/parser"
 	"go/token"
 	"go/types"
@@ -491,7 +493,10 @@ type MemPackageGetter interface {
 // mempkg. To retrieve dependencies, it uses getter.
 //
 // The syntax checking is performed entirely using Go's go/types package.
-func TypeCheckMemPackage(mempkg *std.MemPackage, getter MemPackageGetter) error {
+//
+// If format is true, the code will be automatically updated with the
+// formatted source code.
+func TypeCheckMemPackage(mempkg *std.MemPackage, getter MemPackageGetter, format bool) error {
 	var errs error
 	imp := &gnoImporter{
 		getter: getter,
@@ -504,7 +509,7 @@ func TypeCheckMemPackage(mempkg *std.MemPackage, getter MemPackageGetter) error
 	}
 	imp.cfg.Importer = imp
 
-	_, err := imp.parseCheckMemPackage(mempkg)
+	_, err := imp.parseCheckMemPackage(mempkg, format)
 	// prefer to return errs instead of err:
 	// err will generally contain only the first error encountered.
 	if errs != nil {
@@ -545,12 +550,13 @@ func (g *gnoImporter) ImportFrom(path, _ string, _ types.ImportMode) (*types.Pac
 		g.cache[path] = gnoImporterResult{err: err}
 		return nil, err
 	}
-	result, err := g.parseCheckMemPackage(mpkg)
+	fmt := false
+	result, err := g.parseCheckMemPackage(mpkg, fmt)
 	g.cache[path] = gnoImporterResult{pkg: result, err: err}
 	return result, err
 }
 
-func (g *gnoImporter) parseCheckMemPackage(mpkg *std.MemPackage) (*types.Package, error) {
+func (g *gnoImporter) parseCheckMemPackage(mpkg *std.MemPackage, fmt bool) (*types.Package, error) {
 	fset := token.NewFileSet()
 	files := make([]*ast.File, 0, len(mpkg.Files))
 	var errs error
@@ -567,6 +573,17 @@ func (g *gnoImporter) parseCheckMemPackage(mpkg *std.MemPackage) (*types.Package
 			continue
 		}
 
+		// enforce formatting
+		if fmt {
+			var buf bytes.Buffer
+			err = format.Node(&buf, fset, f)
+			if err != nil {
+				errs = multierr.Append(errs, err)
+				continue
+			}
+			file.Body = buf.String()
+		}
+
 		files = append(files, f)
 	}
 	if errs != nil {

gnovm/pkg/gnolang/go2gno_test.go

@@ -324,7 +324,8 @@ func TestTypeCheckMemPackage(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			err := TypeCheckMemPackage(tc.pkg, tc.getter)
+			format := false
+			err := TypeCheckMemPackage(tc.pkg, tc.getter, format)
 			if tc.check == nil {
 				assert.NoError(t, err)
 			} else {
@@ -333,3 +334,46 @@ func TestTypeCheckMemPackage(t *testing.T) {
 		})
 	}
 }
+
+func TestTypeCheckMemPackage_format(t *testing.T) {
+	t.Parallel()
+
+	input := `
+	package hello
+		func Hello(name string) string   {return "hello"  + name
+}
+
+
+
+`
+
+	pkg := &std.MemPackage{
+		Name: "hello",
+		Path: "gno.land/p/demo/hello",
+		Files: []*std.MemFile{
+			{
+				Name: "hello.gno",
+				Body: input,
+			},
+		},
+	}
+
+	mpkgGetter := mockPackageGetter{}
+	format := false
+	err := TypeCheckMemPackage(pkg, mpkgGetter, format)
+	assert.NoError(t, err)
+	assert.Equal(t, input, pkg.Files[0].Body) // unchanged
+
+	expected := `package hello
+
+func Hello(name string) string {
+	return "hello" + name
+}
+`
+
+	format = true
+	err = TypeCheckMemPackage(pkg, mpkgGetter, format)
+	assert.NoError(t, err)
+	assert.NotEqual(t, input, pkg.Files[0].Body)
+	assert.Equal(t, expected, pkg.Files[0].Body)
+}