From bc5c2cd7dba17c7afae7d4e16f60d24e4070bf14 Mon Sep 17 00:00:00 2001 From: Hannah Kim Date: Tue, 9 Dec 2025 15:41:52 -0500 Subject: [PATCH 1/2] make sure handler functions are properly ignored --- internal/injector/aspect/join/function.go | 33 ++++++++- .../injector/aspect/join/function_test.go | 71 +++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/internal/injector/aspect/join/function.go b/internal/injector/aspect/join/function.go index dbe661f45..95a50efcc 100644 --- a/internal/injector/aspect/join/function.go +++ b/internal/injector/aspect/join/function.go @@ -88,18 +88,29 @@ func (s *functionDeclaration) FileMayMatch(ctx *may.FileContext) may.MatchType { } func (s *functionDeclaration) Matches(ctx context.AspectContext) bool { + node := ctx.Node() + + // Check if this function is being used as an argument to a function call + // and has an ignore directive + chain := ctx.Chain() + if chain.PropertyName() == "Args" { + if hasIgnoreDirective(node) { + return false + } + } + info := functionInformation{ ImportPath: ctx.ImportPath(), typeResolver: ctx, } - if decl, ok := ctx.Node().(*dst.FuncDecl); ok { + if decl, ok := node.(*dst.FuncDecl); ok { if decl.Recv != nil && len(decl.Recv.List) == 1 { info.Receiver = decl.Recv.List[0].Type } info.Name = decl.Name.Name info.Type = decl.Type - } else if lit, ok := ctx.Node().(*dst.FuncLit); ok { + } else if lit, ok := node.(*dst.FuncLit); ok { info.Type = lit.Type } else { return false @@ -496,6 +507,24 @@ func (fo *finalResultImplements) Hash(h *fingerprint.Hasher) error { return h.Named("final-result-implements", fingerprint.String(fo.InterfaceName)) } +// isIgnored returns true if the node is prefixed by an `//orchestrion:ignore` (or the legacy `//dd:ignore`) directive. +func hasIgnoreDirective(node dst.Node) bool { + const ( + orchestrionIgnore = "//orchestrion:ignore" + ddIgnore = "//dd:ignore" + ) + + for _, cmt := range node.Decorations().Start.All() { + if cmt == orchestrionIgnore || strings.HasPrefix(cmt, orchestrionIgnore+" ") { + return true + } + if cmt == ddIgnore || strings.HasPrefix(cmt, ddIgnore+" ") { + return true + } + } + return false +} + // argumentImplements matches functions where at least one argument's type // implements the specified interface. type argumentImplements struct { diff --git a/internal/injector/aspect/join/function_test.go b/internal/injector/aspect/join/function_test.go index bf055a28b..62ccd74a5 100644 --- a/internal/injector/aspect/join/function_test.go +++ b/internal/injector/aspect/join/function_test.go @@ -6,9 +6,12 @@ package join import ( + "go/parser" + "go/token" "testing" "github.com/dave/dst" + "github.com/dave/dst/decorator" "github.com/goccy/go-yaml" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -234,3 +237,71 @@ signature-contains: require.Len(t, signatureContains.Results, 1, "Expected 1 result") assert.Equal(t, "bool", signatureContains.Results[0].Name, "Result should be bool") } + +func TestHasIgnoreDirective(t *testing.T) { + tests := []struct { + name string + source string + shouldMatch bool + }{ + { + name: "without ignore directive", + source: `package main +import "net/http" +func myHandler (w http.ResponseWriter, r *http.Request) {} + +func main() { + http.HandleFunc("/", handler) +}`, + shouldMatch: false, + }, + { + name: "with ignore directive -- net/http", + source: `package main +import "net/http" +//orchestrion:ignore +func myHandler(w http.ResponseWriter, r *http.Request) {} + +func main() { + http.HandleFunc("/", myHandler) +}`, + shouldMatch: true, + }, + { + name: "with ignore directive -- echo", + source: `package main +import "labstack/echo" +//orchestrion:ignore +func myHandler(c echo.Context) error {return c.String(http.StatusOK, "testing")} + +func main() { + e := echo.New() + e.GET("/", myHandler) +}`, + shouldMatch: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fset := token.NewFileSet() + parsedFile, err := parser.ParseFile(fset, "test.go", tt.source, parser.ParseComments) + require.NoError(t, err) + + // Create the decorator. + dec := decorator.NewDecorator(fset) + f, err := dec.DecorateFile(parsedFile) + require.NoError(t, err) + + var funcDecl *dst.FuncDecl + for _, decl := range f.Decls { + if fd, ok := decl.(*dst.FuncDecl); ok && fd.Name.Name == "myHandler" { + funcDecl = fd + break + } + } + assert.NotNil(t, funcDecl) + assert.Equal(t, tt.shouldMatch, hasIgnoreDirective(funcDecl)) + }) + } +} From 9dcb53263cd9dbb3a534262e6f70e3da621c2d88 Mon Sep 17 00:00:00 2001 From: Hannah Kim Date: Tue, 9 Dec 2025 15:42:22 -0500 Subject: [PATCH 2/2] nit: better organization --- internal/injector/aspect/join/function.go | 36 +++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/internal/injector/aspect/join/function.go b/internal/injector/aspect/join/function.go index 95a50efcc..23f7ec15b 100644 --- a/internal/injector/aspect/join/function.go +++ b/internal/injector/aspect/join/function.go @@ -507,24 +507,6 @@ func (fo *finalResultImplements) Hash(h *fingerprint.Hasher) error { return h.Named("final-result-implements", fingerprint.String(fo.InterfaceName)) } -// isIgnored returns true if the node is prefixed by an `//orchestrion:ignore` (or the legacy `//dd:ignore`) directive. -func hasIgnoreDirective(node dst.Node) bool { - const ( - orchestrionIgnore = "//orchestrion:ignore" - ddIgnore = "//dd:ignore" - ) - - for _, cmt := range node.Decorations().Start.All() { - if cmt == orchestrionIgnore || strings.HasPrefix(cmt, orchestrionIgnore+" ") { - return true - } - if cmt == ddIgnore || strings.HasPrefix(cmt, ddIgnore+" ") { - return true - } - } - return false -} - // argumentImplements matches functions where at least one argument's type // implements the specified interface. type argumentImplements struct { @@ -706,3 +688,21 @@ func (o *unmarshalFuncDeclOption) UnmarshalYAML(ctx gocontext.Context, node ast. return nil } + +// isIgnored returns true if the node is prefixed by an `//orchestrion:ignore` (or the legacy `//dd:ignore`) directive. +func hasIgnoreDirective(node dst.Node) bool { + const ( + orchestrionIgnore = "//orchestrion:ignore" + ddIgnore = "//dd:ignore" + ) + + for _, cmt := range node.Decorations().Start.All() { + if cmt == orchestrionIgnore || strings.HasPrefix(cmt, orchestrionIgnore+" ") { + return true + } + if cmt == ddIgnore || strings.HasPrefix(cmt, ddIgnore+" ") { + return true + } + } + return false +}