explore writing linters for cardboard users #229
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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, | ||
| ) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } | ||
|
Comment on lines
+73
to
+86
There was a problem hiding this comment. 🧩 Analysis chainUse Comparing A possible refactor (core part within the current block): - groupType := pass.TypesInfo.TypeOf(ce.Args[0])
-
- var xType types.Type
- switch meth := ce.Args[1].(type) {
+ if len(ce.Args) < 2 {
+ // Signature of run.MethX requires at least two arguments; be defensive.
+ 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)
- }
+ xType = pass.TypesInfo.TypeOf(meth.X)
+ if !types.Identical(xType, groupType) {
+ pass.Reportf(
+ ce.Args[1].Pos(),
+ "method %q does not belong to type of %q: %q (receiver type: %v, group type: %v)",
+ render(pass.Fset, ce.Args[1]),
+ render(pass.Fset, ce.Args[0]),
+ render(pass.Fset, ce),
+ xType,
+ groupType,
+ )
+ }After this change, you can also drop the now-unused I'll verify the review comment by examining the code file and checking the context of the suggested changes. 🏁 Script executed: #!/bin/bash
# Examine the file at the specified lines
echo "=== File content at lines 60-100 ==="
sed -n '60,100p' internal/analyze/cardboardmeth/analyzer.go
echo ""
echo "=== Check imports in the file ==="
head -30 internal/analyze/cardboardmeth/analyzer.go | grep -E "^import|reflect|types"Length of output: 1484 🏁 Script executed: #!/bin/bash
# Search for reflect.DeepEqual usage in the file
echo "=== reflect.DeepEqual usage in analyzer.go ==="
grep -n "reflect\.DeepEqual" internal/analyze/cardboardmeth/analyzer.go
echo ""
echo "=== All reflect usage in analyzer.go ==="
grep -n "reflect\." internal/analyze/cardboardmeth/analyzer.goLength of output: 184 🏁 Script executed: #!/bin/bash
# Search for MethX to understand the context
echo "=== MethX references in the codebase ==="
rg -n "MethX" --type go -C 3Length of output: 1853 🏁 Script executed: #!/bin/bash
# Check if types.Identical is standard in go/types package
echo "=== Checking if go/types package exists and types.Identical is documented ==="
# We'll search the codebase for existing uses of types package functions
echo "Existing types package usage in codebase:"
rg -n "types\." internal/analyze/cardboardmeth/analyzer.go | head -20Length of output: 194 🏁 Script executed: #!/bin/bash
# Check the full function context where this code appears
echo "=== Full context around the analyzed code (wider range) ==="
sed -n '50,120p' internal/analyze/cardboardmeth/analyzer.goLength of output: 1688 🌐 Web query: 💡 Result: Summary — go/types.Identical
References
Replace The review comment is correct and identifies a legitimate major issue. Verification confirms:
Apply the suggested changes:
🤖 Prompt for AI Agents |
||
| }) | ||
|
|
||
| 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() | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Fix typo in diagnostics for MethX arity errors
Both diagnostics use “call ro run.MethX”; fix spelling to “call to run.MethX” for cleaner user-facing messages.
You can apply this minimal change:
If you rely on any specific wording conventions for other analyzers, please align these messages accordingly.
🏁 Script executed:
Length of output: 286
🏁 Script executed:
Length of output: 0
Fix typo in diagnostics for MethX arity errors
Both diagnostics contain "call ro run.MethX"; fix spelling to "call to run.MethX" at lines 64 and 68:
📝 Committable suggestion
🤖 Prompt for AI Agents