diff --git a/cmd/cardboard-lint/main.go b/cmd/cardboard-lint/main.go new file mode 100644 index 00000000..25815cb9 --- /dev/null +++ b/cmd/cardboard-lint/main.go @@ -0,0 +1,15 @@ +package main + +import ( + "golang.org/x/tools/go/analysis/multichecker" + + "pkg.package-operator.run/cardboard/internal/analyze/cardboardmeth" + "pkg.package-operator.run/cardboard/internal/analyze/cardboardselfident" +) + +func main() { + multichecker.Main( + cardboardselfident.Analyzer, + cardboardmeth.Analyzer, + ) +} diff --git a/cmd/sample/main.go b/cmd/sample/main.go new file mode 100644 index 00000000..44ae7056 --- /dev/null +++ b/cmd/sample/main.go @@ -0,0 +1,121 @@ +package main + +import ( + "context" + "embed" + "fmt" + "os" + + "pkg.package-operator.run/cardboard/run" +) + +var ( + mgr *run.Manager + + //go:embed *.go + source embed.FS +) + +func main() { + ctx := context.Background() + + mgr = run.New(run.WithSources(source)) + mgr.Register() + if err := mgr.Run(ctx); err != nil { + fmt.Fprintf(os.Stderr, "\n%s\n", err) + os.Exit(1) + } +} + +func FuncTarget(ctx context.Context) error { + self := run.Fn(FuncTarget) + return mgr.ParallelDeps(ctx, self, run.Fn(func() { + fmt.Println("ok") + })) +} + +type ( + empty struct{} + suspect struct{} +) + +func (s *suspect) Meth(ctx context.Context) error { return nil } + +type coll struct{} + +func (c *coll) MethTarget(ctx context.Context, args []string) error { + self := run.Meth1(c, c.MethTarget, args) + return mgr.ParallelDeps(ctx, self, run.Fn(FuncTarget)) +} + +func (c *coll) MethTargetWrongName(ctx context.Context, args []string) error { + this := run.Meth1(c, c.MethTarget, args) + return mgr.ParallelDeps(ctx, this, run.Fn(FuncTarget)) +} + +func (c *coll) MethTargetWrongInline(ctx context.Context, args []string) error { + return mgr.ParallelDeps(ctx, run.Meth1(c, c.MethTarget, args), run.Fn(FuncTarget)) +} + +func (c *coll) MethTargetWrongInlineAndBare(ctx context.Context, args []string) error { + mgr.ParallelDeps(ctx, run.Meth1(c, c.MethTarget, args), run.Fn(FuncTarget)) + return nil +} + +func (c *coll) MethTargetWrongInlineAndIf(ctx context.Context, args []string) error { + if mgr.ParallelDeps(ctx, run.Meth1(c, c.MethTarget, args), run.Fn(FuncTarget)) != nil { + return nil + } + return nil +} + +func (c *coll) MethTargetWrongInlineAndIfBlock(ctx context.Context, args []string) error { + if err := mgr.ParallelDeps(ctx, run.Meth1(c, c.MethTarget, args), run.Fn(FuncTarget)); err != nil { + return err + } + return nil +} + +func (c *coll) MethNoArgs(ctx context.Context) error { + if err := mgr.ParallelDeps(ctx, run.Meth(c, c.MethNoArgs), run.Fn(FuncTarget)); err != nil { + return err + } + return nil +} + +func (c *coll) MethTwoArgs(ctx context.Context, foo, bar string) error { + if err := mgr.ParallelDeps(ctx, run.Meth2(c, c.MethTwoArgs, foo, bar), run.Fn(FuncTarget)); err != nil { + return err + } + return nil +} + +func (c *coll) MethInvalidTwoArgs(ctx context.Context, foo, bar string) error { + if err := mgr.ParallelDeps(ctx, run.Meth2(empty{}, c.MethTwoArgs, foo, bar), run.Fn(FuncTarget)); err != nil { + return err + } + return nil +} + +func (c *coll) MethInvalidWrongReceiverWithSameSignatureReceiverTwoArgs(ctx context.Context, foo, bar string) error { + if err := mgr.ParallelDeps(ctx, run.Meth2(suspect{}, c.MethTwoArgs, foo, bar), run.Fn(FuncTarget)); err != nil { + return err + } + return nil +} + +func (c *coll) MethExtractedTwoArgs(ctx context.Context, foo, bar string) error { + methTwoArgs := c.MethTwoArgs + if err := mgr.ParallelDeps(ctx, run.Meth2(c, methTwoArgs, foo, bar), run.Fn(FuncTarget)); err != nil { + return err + } + return nil +} + +func (c *coll) MethInvalidExtracted(ctx context.Context) error { + meth := (&suspect{}).Meth + if err := mgr.ParallelDeps(ctx, run.Meth(c, meth)); err != nil { + return err + } + return nil +} diff --git a/go.mod b/go.mod index f8d592a7..641d8af0 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( github.com/neilotoole/slogt v1.1.0 github.com/stretchr/testify v1.9.0 github.com/xlab/treeprint v1.2.0 + golang.org/x/tools v0.1.12 ) require ( @@ -23,6 +24,7 @@ require ( github.com/kr/pretty v0.3.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rogpeppe/go-internal v1.12.0 // indirect + golang.org/x/mod v0.9.0 // indirect golang.org/x/sys v0.20.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index 46d5ef01..cc923789 100644 --- a/go.sum +++ b/go.sum @@ -25,9 +25,13 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/xlab/treeprint v1.2.0 h1:HzHnuAF1plUN2zGlAFHbSQP2qJ0ZAD3XF5XD7OesXRQ= github.com/xlab/treeprint v1.2.0/go.mod h1:gj5Gd3gPdKtR1ikdDK6fnFLdmIS0X30kTTuNd/WEJu0= +golang.org/x/mod v0.9.0 h1:KENHtAZL2y3NLMYZeHY9DW8HW8V+kQyJsY/V9JlKvCs= +golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU= +golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= diff --git a/internal/analyze/cardboardmeth/analyzer.go b/internal/analyze/cardboardmeth/analyzer.go new file mode 100644 index 00000000..d8a8acf5 --- /dev/null +++ b/internal/analyze/cardboardmeth/analyzer.go @@ -0,0 +1,99 @@ +package cardboardmeth + +import ( + "bytes" + "go/ast" + "go/printer" + "go/token" + "go/types" + "reflect" + "regexp" + "strconv" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/go/types/typeutil" +) + +const ( + cardboardRunPackage = "pkg.package-operator.run/cardboard/run" +) + +var methFnRegex = regexp.MustCompile("^Meth(.*)") + +var Analyzer = &analysis.Analyzer{ + Name: "cardboardmeth", + Doc: "Checks that cardboard run.MethX calls pass correct struct and method pairs.", + Run: run, + + Requires: []*analysis.Analyzer{inspect.Analyzer}, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + + inspect.Preorder(nodeFilter, func(n ast.Node) { + ce := n.(*ast.CallExpr) + + fn := typeutil.Callee(pass.TypesInfo, ce) + if fn == nil { + return + } + if pkg := fn.Pkg(); pkg == nil || pkg.Path() != cardboardRunPackage { + return // This analyzer is only interested in calls to types from the `cardboardRunPackage` package. + } + + recv := fn.Type().(*types.Signature).Recv() + if recv != nil { + return // This analyzer is only interested in function calls. + } + + captures := methFnRegex.FindStringSubmatch(fn.Name()) + if captures == nil { + return // This analyzer is only interested in calls to run.MethX. + } + + if raw := captures[1]; raw != "" { + parsed, err := strconv.Atoi(raw) + if err != nil { + pass.Reportf(ce.Pos(), "call ro run.MethX with unparseable arity: %s %q", err, render(pass.Fset, ce)) + return + } + if parsed < 1 { + pass.Reportf(ce.Pos(), "call ro run.MethX with arity <1: %q", render(pass.Fset, ce)) + return + } + } + + groupType := pass.TypesInfo.TypeOf(ce.Args[0]) + + var xType types.Type + switch meth := ce.Args[1].(type) { + case *ast.SelectorExpr: + xType = pass.TypesInfo.TypeOf(meth.X) + if !reflect.DeepEqual(xType, groupType) { + pass.Reportf(ce.Args[1].Pos(), "method %q does not belong to type of %q: %q %s", render(pass.Fset, ce.Args[1]), render(pass.Fset, ce.Args[0]), render(pass.Fset, ce), xType) + } + case *ast.Ident: + pass.Reportf(ce.Args[1].Pos(), "identifier %q should be removed and method selector expression inlined: %q", render(pass.Fset, ce.Args[1]), render(pass.Fset, ce)) + default: + pass.Reportf(ce.Args[1].Pos(), "not implemented: %q has method arg node type %T", render(pass.Fset, ce), meth) + } + }) + + return nil, nil +} + +// from https://arslan.io/2019/06/13/using-go-analysis-to-write-a-custom-linter/ +func render(fset *token.FileSet, x interface{}) string { + var buf bytes.Buffer + if err := printer.Fprint(&buf, fset, x); err != nil { + panic(err) + } + return buf.String() +} diff --git a/internal/analyze/cardboardself/analyzer.go b/internal/analyze/cardboardself/analyzer.go new file mode 100644 index 00000000..8daadee8 --- /dev/null +++ b/internal/analyze/cardboardself/analyzer.go @@ -0,0 +1,29 @@ +package cardboardself + +import ( + "go/ast" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +var Analyzer = &analysis.Analyzer{ + Name: "cardboardself", + Doc: "Checks that self is a wrapped version of the caller.", + Run: run, + + Requires: []*analysis.Analyzer{inspect.Analyzer}, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{} + + inspect.WithStack(nodeFilter, func(n ast.Node, _ bool, stack []ast.Node) bool { + return false + }) + + return nil, nil +} diff --git a/internal/analyze/cardboardselfident/analyzer.go b/internal/analyze/cardboardselfident/analyzer.go new file mode 100644 index 00000000..4616160a --- /dev/null +++ b/internal/analyze/cardboardselfident/analyzer.go @@ -0,0 +1,149 @@ +package cardboardselfident + +import ( + "bytes" + "fmt" + "go/ast" + "go/printer" + "go/token" + "go/types" + "reflect" + "regexp" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/go/types/typeutil" +) + +const ( + cardboardRunPackage = "pkg.package-operator.run/cardboard/run" + runManagerTypeName = "*pkg.package-operator.run/cardboard/run.Manager" +) + +var depsFnRegex = regexp.MustCompile("^(Serial|Parallel)Deps$") + +var Analyzer = &analysis.Analyzer{ + Name: "cardboardselfident", + Doc: "Checks that cardboard targets pass correct self arg to their dependencies.", + Run: run, + + Requires: []*analysis.Analyzer{inspect.Analyzer}, +} + +// playground to explore a linter that enforces a separate non-inline `self` argument in calls to mgr.Serial/ParallelDeps + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + + inspect.WithStack(nodeFilter, func(n ast.Node, _ bool, stack []ast.Node) bool { + ce := n.(*ast.CallExpr) + + fn := typeutil.Callee(pass.TypesInfo, ce) + if fn == nil { + return false + } + if pkg := fn.Pkg(); pkg == nil || pkg.Path() != cardboardRunPackage { + return false // This analyzer is only interested in calls to types from the `cardboardRunPackage` package. + } + + recv := fn.Type().(*types.Signature).Recv() + if recv == nil { + return false // This analyzer is only interested in method calls. + } + recvName := recv.Type().Underlying().String() + if recvName != runManagerTypeName { + return false // This analyzer is only interested in calls to the `runManagerTypeName` type. + } + + if !depsFnRegex.MatchString(fn.Name()) { + return false // This analyzer is only interested in calls to Parallel-/SerialDeps. + } + + parentIndex, parentBlock := nextBlock(stack) + + // second arg should be `self` + if selfIdent, ok := ce.Args[1].(*ast.Ident); !ok { + // second arg is not an identifier + invalidSelfArgExpr := ce.Args[1] + offendingCallSrc := render(pass.Fset, ce.Args[1]) + + childStmt := stack[parentIndex+1] + childIndex := -1 + for i, stmt := range parentBlock.List { + if reflect.DeepEqual(stmt, childStmt) { + childIndex = i + break + } + } + + // prepare assignment statement `self := invalidSelfArgExpr` + // https://yuroyoro.github.io/goast-viewer/index.html + // turned out very helpful when I constructed the assignment statement below. + assignment := &ast.AssignStmt{ + Tok: token.DEFINE, + Lhs: []ast.Expr{ast.NewIdent("self")}, + Rhs: []ast.Expr{invalidSelfArgExpr}, + } + + // Inject the assignment just before the statement that contains the invalid call. + list := make([]ast.Stmt, len(parentBlock.List)+1) + copy(list, parentBlock.List[:childIndex]) + list[childIndex] = assignment + copy(list[childIndex+1:], parentBlock.List[childIndex:]) + + // Replace call arg with `self` identifier. + ce.Args[1] = ast.NewIdent("self") + // Replace parent block statement list. + parentBlock.List = list + + pass.Report(analysis.Diagnostic{ + Pos: ce.Pos(), + Message: fmt.Sprintf(`Second arg to %s call should be identifier "self" but is expression %q.`, fn.Name(), offendingCallSrc), + SuggestedFixes: []analysis.SuggestedFix{ + { + Message: fmt.Sprintf(`Separate %q out into separate short variable declaration.`, offendingCallSrc), + TextEdits: []analysis.TextEdit{ + { + Pos: parentBlock.Pos(), + End: parentBlock.End(), + NewText: []byte(render(pass.Fset, parentBlock)), + }, + }, + }, + }, + }) + } else if selfIdent.Name != "self" { + // second arg is an identifier but not named "self" + pass.Reportf(ce.Pos(), `second arg to %s call should be identifier "self" but is "%s".`, fn.Name(), selfIdent.Name) + } + return false + }) + + return nil, nil +} + +// from https://arslan.io/2019/06/13/using-go-analysis-to-write-a-custom-linter/ +func render(fset *token.FileSet, x interface{}) string { + var buf bytes.Buffer + if err := printer.Fprint(&buf, fset, x); err != nil { + panic(err) + } + return buf.String() +} + +// Finds surrounding block statement by travelling up the stack. +func nextBlock(stack []ast.Node) (int, *ast.BlockStmt) { + for i := len(stack) - 1; i >= 0; i-- { + block, ok := stack[i].(*ast.BlockStmt) + if !ok { + continue + } + return i, block + } + return -1, nil +}