diff --git a/compiler/astutil/astutil.go b/compiler/astutil/astutil.go index 30febe1cb..abe17d94a 100644 --- a/compiler/astutil/astutil.go +++ b/compiler/astutil/astutil.go @@ -5,7 +5,7 @@ import ( "go/ast" "go/token" "go/types" - "strings" + "regexp" ) func RemoveParens(e ast.Expr) ast.Expr { @@ -82,15 +82,7 @@ func FuncKey(d *ast.FuncDecl) string { // such as code expecting ints to be 64-bit. It should be used with caution // since it may create unused imports in the original source file. func PruneOriginal(d *ast.FuncDecl) bool { - if d.Doc == nil { - return false - } - for _, c := range d.Doc.List { - if strings.HasPrefix(c.Text, "//gopherjs:prune-original") { - return true - } - } - return false + return hasDirective(d, `prune-original`) } // KeepOriginal returns true if gopherjs:keep-original directive is present @@ -102,15 +94,38 @@ func PruneOriginal(d *ast.FuncDecl) bool { // function in the original called `foo`, it will be accessible by the name // `_gopherjs_original_foo`. func KeepOriginal(d *ast.FuncDecl) bool { - if d.Doc == nil { - return false - } - for _, c := range d.Doc.List { - if strings.HasPrefix(c.Text, "//gopherjs:keep-original") { - return true + return hasDirective(d, `keep-original`) +} + +// directiveMatcher is a regex which matches a GopherJS directive +// and finds the directive action. +var directiveMatcher = regexp.MustCompile(`^\/(?:\/|\*)gopherjs:([\w-]+)`) + +// hasDirective returns true if the associated documentation +// or line comments for the given node have the given directive action. +// +// All GopherJS-specific directives must start with `//gopherjs:` or +// `/*gopherjs:` and followed by an action without any whitespace. The action +// must be one or more letter, decimal, underscore, or hyphen. +// +// see https://pkg.go.dev/cmd/compile#hdr-Compiler_Directives +func hasDirective(node ast.Node, directiveAction string) bool { + foundDirective := false + ast.Inspect(node, func(n ast.Node) bool { + switch a := n.(type) { + case *ast.Comment: + m := directiveMatcher.FindStringSubmatch(a.Text) + if len(m) == 2 && m[1] == directiveAction { + foundDirective = true + } + return false + case *ast.CommentGroup: + return !foundDirective + default: + return n == node } - } - return false + }) + return foundDirective } // FindLoopStmt tries to find the loop statement among the AST nodes in the diff --git a/compiler/astutil/astutil_test.go b/compiler/astutil/astutil_test.go index a996ae73f..80b689b5f 100644 --- a/compiler/astutil/astutil_test.go +++ b/compiler/astutil/astutil_test.go @@ -1,6 +1,7 @@ package astutil import ( + "go/ast" "go/token" "testing" @@ -130,6 +131,347 @@ func TestPruneOriginal(t *testing.T) { } } +func TestHasDirectiveOnDecl(t *testing.T) { + tests := []struct { + desc string + src string + want bool + }{ + { + desc: `no comment on function`, + src: `package testpackage; + func foo() {}`, + want: false, + }, { + desc: `no directive on function with comment`, + src: `package testpackage; + // foo has no directive + func foo() {}`, + want: false, + }, { + desc: `wrong directive on function`, + src: `package testpackage; + //gopherjs:wrong-directive + func foo() {}`, + want: false, + }, { + desc: `correct directive on function`, + src: `package testpackage; + //gopherjs:do-stuff + // foo has a directive to do stuff + func foo() {}`, + want: true, + }, { + desc: `correct directive in multiline comment on function`, + src: `package testpackage; + /*gopherjs:do-stuff + foo has a directive to do stuff + */ + func foo() {}`, + want: true, + }, { + desc: `invalid directive in multiline comment on function`, + src: `package testpackage; + /* + gopherjs:do-stuff + */ + func foo() {}`, + want: false, + }, { + desc: `prefix directive on function`, + src: `package testpackage; + //gopherjs:do-stuffs + func foo() {}`, + want: false, + }, { + desc: `multiple directives on function`, + src: `package testpackage; + //gopherjs:wrong-directive + //gopherjs:do-stuff + //gopherjs:another-directive + func foo() {}`, + want: true, + }, { + desc: `directive with explanation on function`, + src: `package testpackage; + //gopherjs:do-stuff 'cause we can + func foo() {}`, + want: true, + }, { + desc: `no directive on type declaration`, + src: `package testpackage; + // Foo has a comment + type Foo int`, + want: false, + }, { + desc: `directive on type declaration`, + src: `package testpackage; + //gopherjs:do-stuff + type Foo int`, + want: true, + }, { + desc: `directive on specification, not on declaration`, + src: `package testpackage; + type ( + Foo int + + //gopherjs:do-stuff + Bar struct{} + )`, + want: false, + }, { + desc: `no directive on const declaration`, + src: `package testpackage; + const foo = 42`, + want: false, + }, { + desc: `directive on const documentation`, + src: `package testpackage; + //gopherjs:do-stuff + const foo = 42`, + want: true, + }, { + desc: `no directive on var declaration`, + src: `package testpackage; + var foo = 42`, + want: false, + }, { + desc: `directive on var documentation`, + src: `package testpackage; + //gopherjs:do-stuff + var foo = 42`, + want: true, + }, { + desc: `no directive on var declaration`, + src: `package testpackage; + import _ "embed"`, + want: false, + }, { + desc: `directive on var documentation`, + src: `package testpackage; + //gopherjs:do-stuff + import _ "embed"`, + want: true, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + const action = `do-stuff` + decl := srctesting.ParseDecl(t, test.src) + if got := hasDirective(decl, action); got != test.want { + t.Errorf(`hasDirective(%T, %q) returned %t, want %t`, decl, action, got, test.want) + } + }) + } +} + +func TestHasDirectiveOnSpec(t *testing.T) { + tests := []struct { + desc string + src string + want bool + }{ + { + desc: `no directive on type specification`, + src: `package testpackage; + type Foo int`, + want: false, + }, { + desc: `directive on declaration, not on specification`, + src: `package testpackage; + //gopherjs:do-stuff + type Foo int`, + want: false, + }, { + desc: `directive in doc on type specification`, + src: `package testpackage; + type ( + //gopherjs:do-stuff + Foo int + )`, + want: true, + }, { + desc: `directive in line on type specification`, + src: `package testpackage; + type Foo int //gopherjs:do-stuff`, + want: true, + }, { + desc: `no directive on const specification`, + src: `package testpackage; + const foo = 42`, + want: false, + }, { + desc: `directive in doc on const specification`, + src: `package testpackage; + const ( + //gopherjs:do-stuff + foo = 42 + )`, + want: true, + }, { + desc: `directive in line on const specification`, + src: `package testpackage; + const foo = 42 //gopherjs:do-stuff`, + want: true, + }, { + desc: `no directive on var specification`, + src: `package testpackage; + var foo = 42`, + want: false, + }, { + desc: `directive in doc on var specification`, + src: `package testpackage; + var ( + //gopherjs:do-stuff + foo = 42 + )`, + want: true, + }, { + desc: `directive in line on var specification`, + src: `package testpackage; + var foo = 42 //gopherjs:do-stuff`, + want: true, + }, { + desc: `no directive on import specification`, + src: `package testpackage; + import _ "embed"`, + want: false, + }, { + desc: `directive in doc on import specification`, + src: `package testpackage; + import ( + //gopherjs:do-stuff + _ "embed" + )`, + want: true, + }, { + desc: `directive in line on import specification`, + src: `package testpackage; + import _ "embed" //gopherjs:do-stuff`, + want: true, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + const action = `do-stuff` + spec := srctesting.ParseSpec(t, test.src) + if got := hasDirective(spec, action); got != test.want { + t.Errorf(`hasDirective(%T, %q) returned %t, want %t`, spec, action, got, test.want) + } + }) + } +} + +func TestHasDirectiveOnFile(t *testing.T) { + tests := []struct { + desc string + src string + want bool + }{ + { + desc: `no directive on file`, + src: `package testpackage; + //gopherjs:do-stuff + type Foo int`, + want: false, + }, { + desc: `directive on file`, + src: `//gopherjs:do-stuff + package testpackage; + type Foo int`, + want: true, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + const action = `do-stuff` + fset := token.NewFileSet() + file := srctesting.Parse(t, fset, test.src) + if got := hasDirective(file, action); got != test.want { + t.Errorf(`hasDirective(%T, %q) returned %t, want %t`, file, action, got, test.want) + } + }) + } +} + +func TestHasDirectiveOnField(t *testing.T) { + tests := []struct { + desc string + src string + want bool + }{ + { + desc: `no directive on struct field`, + src: `package testpackage; + type Foo struct { + bar int + }`, + want: false, + }, { + desc: `directive in doc on struct field`, + src: `package testpackage; + type Foo struct { + //gopherjs:do-stuff + bar int + }`, + want: true, + }, { + desc: `directive in line on struct field`, + src: `package testpackage; + type Foo struct { + bar int //gopherjs:do-stuff + }`, + want: true, + }, { + desc: `no directive on interface method`, + src: `package testpackage; + type Foo interface { + Bar(a int) int + }`, + want: false, + }, { + desc: `directive in doc on interface method`, + src: `package testpackage; + type Foo interface { + //gopherjs:do-stuff + Bar(a int) int + }`, + want: true, + }, { + desc: `directive in line on interface method`, + src: `package testpackage; + type Foo interface { + Bar(a int) int //gopherjs:do-stuff + }`, + want: true, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + const action = `do-stuff` + spec := srctesting.ParseSpec(t, test.src) + tspec := spec.(*ast.TypeSpec) + var field *ast.Field + switch typeNode := tspec.Type.(type) { + case *ast.StructType: + field = typeNode.Fields.List[0] + case *ast.InterfaceType: + field = typeNode.Methods.List[0] + default: + t.Errorf(`unexpected node type, %T, when finding field`, typeNode) + return + } + if got := hasDirective(field, action); got != test.want { + t.Errorf(`hasDirective(%T, %q) returned %t, want %t`, field, action, got, test.want) + } + }) + } +} + func TestEndsWithReturn(t *testing.T) { tests := []struct { desc string diff --git a/internal/srctesting/srctesting.go b/internal/srctesting/srctesting.go index 1d9cecd20..4e374845e 100644 --- a/internal/srctesting/srctesting.go +++ b/internal/srctesting/srctesting.go @@ -53,17 +53,45 @@ func Check(t *testing.T, fset *token.FileSet, files ...*ast.File) (*types.Info, // // Fails the test if there isn't exactly one function declared in the source. func ParseFuncDecl(t *testing.T, src string) *ast.FuncDecl { + t.Helper() + decl := ParseDecl(t, src) + fdecl, ok := decl.(*ast.FuncDecl) + if !ok { + t.Fatalf("Got %T decl, expected *ast.FuncDecl", decl) + } + return fdecl +} + +// ParseDecl parses source with a single declaration and +// returns that declaration AST. +// +// Fails the test if there isn't exactly one declaration in the source. +func ParseDecl(t *testing.T, src string) ast.Decl { t.Helper() fset := token.NewFileSet() file := Parse(t, fset, src) if l := len(file.Decls); l != 1 { - t.Fatalf("Got %d decls in the sources, expected exactly 1", l) + t.Fatalf(`Got %d decls in the sources, expected exactly 1`, l) } - fdecl, ok := file.Decls[0].(*ast.FuncDecl) + return file.Decls[0] +} + +// ParseSpec parses source with a single declaration containing +// a single specification and returns that specification AST. +// +// Fails the test if there isn't exactly one declaration and +// one specification in the source. +func ParseSpec(t *testing.T, src string) ast.Spec { + t.Helper() + decl := ParseDecl(t, src) + gdecl, ok := decl.(*ast.GenDecl) if !ok { - t.Fatalf("Got %T decl, expected *ast.FuncDecl", file.Decls[0]) + t.Fatalf("Got %T decl, expected *ast.GenDecl", decl) } - return fdecl + if l := len(gdecl.Specs); l != 1 { + t.Fatalf(`Got %d spec in the sources, expected exactly 1`, l) + } + return gdecl.Specs[0] } // Format AST node into a string.